get rid of cert pinning for application updates

RESOLVED DUPLICATE of bug 1189843

Status

RESOLVED DUPLICATE of bug 1189843
2 years ago
2 years ago

People

(Reporter: bhearsum, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Cert pinning of aus*.mozilla.org allows us to ensure users are not man-in-the-middled as part of the update process, but it eventually prevents us from being able to ship updates to them. Eg: aus3.mozilla.org's cert will be expiring in a few months, and we cannot acquire a new certificate that meets the pinning requirements.

The new way of guarding against MitM is to sign the MARs with a cert that we embed into the browser. We should do this for Thunderbird (if we're not already), and remove the pinning.

Comment 1

2 years ago
I think you know more than anyone else on the Thunderbird team (or more than all team members combined even) about this. Can you give us a hand?
I don't have time to help with this. https://bugzilla.mozilla.org/show_bug.cgi?id=973933 is where much of the work for Firefox happened though. Basically, once MAR signing is enabled for all platforms you can remove the pinning requirements.
I checked the patches from bug 973933 and saw the only changes in /browser/confvars.sh are adding for Linux and Darwin (and removing again for Linux) MOZ_VERIFY_MAR_SIGNATURE=1 and adding MOZ_ENABLE_SIGNMAR=1 for all. On the actual confvars.sh there is no more MOZ_VERIFY_MAR_SIGNATURE=1 in it.

In mail/confvars.sh we have already added MOZ_ENABLE_SIGNMAR=1 and we have 

  if ! test "$HAVE_64BIT_BUILD"; then
    MOZ_VERIFY_MAR_SIGNATURE=1
  fi

Now my question, is this enough and do we still need the MOZ_VERIFY_MAR_SIGNATURE=1 ?
Flags: needinfo?(bhearsum)
(In reply to Richard Marti (:Paenglab) from comment #3)
> I checked the patches from bug 973933 and saw the only changes in
> /browser/confvars.sh are adding for Linux and Darwin (and removing again for
> Linux) MOZ_VERIFY_MAR_SIGNATURE=1 and adding MOZ_ENABLE_SIGNMAR=1 for all.
> On the actual confvars.sh there is no more MOZ_VERIFY_MAR_SIGNATURE=1 in it.
> 
> In mail/confvars.sh we have already added MOZ_ENABLE_SIGNMAR=1 and we have 
> 
>   if ! test "$HAVE_64BIT_BUILD"; then
>     MOZ_VERIFY_MAR_SIGNATURE=1
>   fi
> 
> Now my question, is this enough and do we still need the
> MOZ_VERIFY_MAR_SIGNATURE=1 ?

I don't know these build system flags well enough to say for sure, but I would guess yes. I believe MOZ_ENABLE_SIGNMAR  enables the signature bits for the updater and mar binaries,and MOZ_VERIFY_MAR_SIGNATURE actually turns on mar verification before applying updates. Probably this can be verified by setting app.update.log = True and looking to see if the mars are verified on all platforms?

And just be clear: I'm not pushing for this, but having improves your ability to update old users in the long run. If that's not as important to you as other things, there's no need to prioritize this.
Flags: needinfo?(bhearsum)
Created attachment 8876482 [details]
Update log Windows

Update log Windows
Created attachment 8876483 [details]
Update log Linux

Update log Linux
Created attachment 8876484 [details]
Update log Mac

Update log Mac

Ben, this and the two above are update logs from all platforms. Please can you say if the mars are verified?

The gap in the logs is the restart of TB.
Flags: needinfo?(bhearsum)
Attachment #8876482 - Attachment mime type: text/x-log → text/plain
Flags: needinfo?(bhearsum)
Attachment #8876483 - Attachment mime type: text/x-log → text/plain
Attachment #8876484 - Attachment mime type: text/x-log → text/plain
These look very similar to my Firefox update logs, which makes me think there isn't anything special logged when the signature is verified (unless it fails, maybe).
Thank you, Ben.

So I think we have nothing to do except to remove the

  if ! test "$HAVE_64BIT_BUILD"; then
    MOZ_VERIFY_MAR_SIGNATURE=1
  fi

part we have in our confvars.sh FX doesn't have.
Just to be clear: based on the logs I do not know whether or not the verification is happening.

(In reply to Richard Marti (:Paenglab) from comment #9)
> Thank you, Ben.
> 
> So I think we have nothing to do except to remove the
> 
>   if ! test "$HAVE_64BIT_BUILD"; then
>     MOZ_VERIFY_MAR_SIGNATURE=1
>   fi
> 
> part we have in our confvars.sh FX doesn't have.

It sounds like this is preventing some platforms from doing mar signature verification, so I'd guess you need to do this. But I wouldn't ship anything with this change without doing some explicit testing that everything is working. If you ship it, and it's broken in some way, you strand users.

Once all of that is done, and you're certain mar verification is all working, solving this bug is a matter of actually disabling the pinning with something like: https://bug1151485.bmoattachments.org/attachment.cgi?id=8631918
The update log on Windows was done with x64 build. I tried the update now with the 32 bit build which is the only with "MOZ_VERIFY_MAR_SIGNATURE=1" which preventing the verification.

The only difference in the log is the line:
   AUS:SVC Downloader:_verifyDownload hashes match.
after the line
   AUS:SVC Downloader:_verifyDownload downloaded size == expected size.

So why is this only for Win 32 bit set and shouldn't this be removed for it?
Robert Strong pointed me at https://bugzilla.mozilla.org/show_bug.cgi?id=1189843, where MAR signing was enabled for all platforms for Thunderbird. Sounds like it's safe to remove the pinning.
(In reply to Ben Hearsum (:bhearsum) from comment #12)
> Robert Strong pointed me at
> https://bugzilla.mozilla.org/show_bug.cgi?id=1189843, where MAR signing was
> enabled for all platforms for Thunderbird. Sounds like it's safe to remove
> the pinning.

Turns out I misread. MAR signing is not enabled for Linux or Mac, *and* cert pinning hasn't worked since roughly 2016-08-18. I'm duping this bug over because the one above has more detail and other people involved.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1189843
You need to log in before you can comment on or make changes to this bug.