#18405 closed enhancement (fixed)

Check that MAR signing is done properly

Reported by: gk Owned by: gk
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-security TorBrowserTeam201603R GeorgKoppen201603
Cc: brade, mcs Actual Points:
Parent ID: Points:
Reviewer: Sponsor: SponsorU

Description

We are checking that the authenticode signing got done properly with a script. We should do the same with our MAR files.

Child Tickets

Change History (9)

comment:1 Changed 20 months ago by gk

Keywords: SponsorU removed
Sponsor: SponsorU

comment:2 Changed 20 months ago by gk

Keywords: TorBrowserTeam201603 added

comment:3 Changed 20 months ago by gk

Keywords: GeorgKoppen201603 added

comment:4 Changed 20 months ago by gk

Keywords: TorBrowserTeam201603R added; TorBrowserTeam201603 removed
Status: newneeds_review

bug_18405 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_18405) in my branch has a script that is modeled after the one we already have for authenticode signature checking. Please review.

comment:5 Changed 20 months ago by gk

Okay, I cleaned the script a bit up, bug_18405_v2 (https://gitweb.torproject.org/user/gk/tor-browser-bundle.git/commit/?h=bug_18405_v2&id=8f364f1ce08c74bd47510d586a70d87307981d5f) has the latest version for review now.

comment:6 Changed 20 months ago by mcs

Cc: brade mcs added

comment:7 Changed 20 months ago by boklm

Related to this, I opened #18497 to do the same on the files available in the update responses.

comment:8 Changed 20 months ago by boklm

The script looks good. I used it to check the mar files of 5.5.3, 6.0a3 and 6.0a3-hardened.

Minor thing: maybe we could add the mar-tools directory to the PATH and use signmar from the PATH instead of using the SIGNMAR environment variable, but it doesn't make a big difference.

comment:9 Changed 20 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Maybe. I'd have to think about this a bit more, though, especially as I'd like to take the setup of random MAR signing checking people into account. My feeling is it is less error-prone if one needs to specify both env variables as done. Thus, I take the patch as is, thanks. Commit 8f364f1ce08c74bd47510d586a70d87307981d5f has it.

Note: See TracTickets for help on using tickets.