Closed Bug 906466 Opened 11 years ago Closed 10 years ago

Updates not properly signed on the nightly-ux branch: Certificate did not match issuer or name.

Categories

(Release Engineering :: General, defect, P2)

Product:
Release Engineering
Component:
General
Platform:
x86_64
Windows 7
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: steffen.wilberg, Assigned: bhearsum)

References

Details

Attachments

(3 files)

[buildbot-configs] Fix ux win32
2.27 KB, patch
catlee
: review+
nthomas
: checked-in+
Details | Diff | Splinter Review
[buildbot-configs] Fix other branches & cleanup
13.86 KB, patch
catlee
: review+
nthomas
: checked-in+
Details | Diff | Splinter Review
patch.diff
880 bytes, patch
robert.strong.bugs
: review+
bhearsum
: feedback+
bhearsum
: checked-in+
Details | Diff | Splinter Review 
The UX nightly is not properly signed. It therefore can't use the Maintenance Service to prevent the Windows UAC prompt.

This is from my C:\ProgramData\Mozilla\logs\maintenanceservice.log:

Executing service command software-update, ID: bab8956f-0c60-4b2f-bcd4-0f4c592852fa
Passed in path: 'C:\Users\Steffen\AppData\Local\Mozilla\updates\253E8F7686B240C5\updates\0\updater.exe'; Using this path for updating: 'C:\Program Files (x86)\Mozilla Maintenance Service\update\updater.exe'.
updater.exe was compared successfully to the installation directory updater.exe.
The updater.exe application contains the Mozilla updater identity.
*** Warning: Certificate did not match issuer or name.  (1168)***
*** Warning: Error on certificate check.  (1168)***
*** Warning: Could not start process due to certificate check error on updater.exe. Updating update.status.  (0)***
Service command software-update complete.
service command MozillaMaintenance complete with result: Failure.
This is something releng would have to take care of.
You could alternately compile without enabling the maintenance service, then it wouldn't even attempt to use the service. Or set the pref app.update.service.enabled = false.
Assignee: nobody → nthomas
Priority: -- → P2
This is a bunch of other tidying related to signing servers. It fixes signing for elm, where the latest tenant has asked for nightlies (bug 900212), and for oak where mac builds were signed with the wrong cert. The rest is assorted cleanup (inbound and profiling don't do nightlies any more (which is the default), remove definitions for debug platforms, reset birch now that it's not b2g-inbound).

re Android, there's a bunch of places we're not setting the nightly server, eg x86 on m-c, all nightly builds on aurora/elm/fig. I'm not sure what the impact of this is, particularly to existing installs, but happy to add them to the patch if they should be set.
Attachment #791994 - Flags: review?(catlee)
Attachment #791992 - Flags: review?(catlee) → review+
Attachment #791994 - Flags: review?(catlee) → review+
These will go live in the next reconfig, ~ Thursday unless it's more urgent. The rest of the birch work I moved out to bug 906946.
Will this change the mar private key too?  If it does we need to change the cert used to verify it. Also if it is, then people will have to download new nightly builds and install them manually to get updates moving forward.

If this only changes the signing on the exes, then ignore the above paragraph.
My understanding is that we're building in the nightly cert in the updater, but signing the mar with the cert from the dep builds. This change should change the signing of both the installer and mar file to the nightly cert.
Actually it looks like we're already using the nightly mar private key to sign the profiling branch:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/Makefile.in#109

Otherwise I'm not sure how any updates are working right now for the profiling branch.  Mar verification happens for both the updater and the service.

So we should be fine, and I think this change only signs updater.exe using the Nightly cert now.
Profiling has no nightlies any more, and hence no updates, but we're talking about ux here, right ? 

I checked the most recent nightly for ux on windows (https://tbpl.mozilla.org/php/getParsedLog.php?id=26719820&tree=UX&full=1). It talks to the dep signing server for both executables, mar, and installers (port 9110 on the signing servers, vs 9100 for nightly cert).  But I'm worried now that nightly-ux isn't in that channel list at http://hg.mozilla.org/projects/ux/file/default/toolkit/mozapps/update/updater/Makefile.in#l109

Unfortunately the RC_FLAGS aren't echoed during the log, but perhaps attachment 791992 [details] [diff] [review] is not the right fix ?
(In reply to Nick Thomas [:nthomas] from comment #10)
> Profiling has no nightlies any more, and hence no updates, but we're talking
> about ux here, right ? 

ugh sorry you're right on that.

> I checked the most recent nightly for ux on windows
> (https://tbpl.mozilla.org/php/getParsedLog.php?id=26719820&tree=UX&full=1).
> It talks to the dep signing server for both executables, mar, and installers
> (port 9110 on the signing servers, vs 9100 for nightly cert).  But I'm
> worried now that nightly-ux isn't in that channel list at
> http://hg.mozilla.org/projects/ux/file/default/toolkit/mozapps/update/
> updater/Makefile.in#l109
> 

Yes this is the point I was making before I got ux and profiling confused.

> Unfortunately the RC_FLAGS aren't echoed during the log, but perhaps
> attachment 791992 [details] [diff] [review] is not the right fix ?

Right, so what this bug is asking for is to have the windows binaries signed with the nightly cert. But the MAR private key used for signing should not change. I.e. it should still be dep.

A build before your change will be using the dep public key to verify the mar. If it tries to consume a MAR signed with a nightly key, then it will fail. 

If you do want to change it though, then we need to update that code location I referenced, and you need to force users through a mandatory update which is signed with the dep cert, and which upgrades the user with the code change that I referenced.  Or just accept that people will need to manually download and install a new build in order to update to the new MAR files.
Can you think of any reason it is broken now ? It seems to me that we're signing with the dep key, and also verifying against it, so I dunno why the update service is not working. 

Also can't reproduce using WinXP. In the Browser Console there are messages indicating the service is being used:
[19:01:40.493] AUS:SVC Downloader:onStopRequest - setting state to: pending-service
[19:01:40.499] AUS:SVC getCanStageUpdates - able to stage updates because we'll use the service
[19:01:40.499] AUS:SVC Downloader:onStopRequest - attempting to stage update: UX 26.0a1
[19:01:44.362] AUS:SVC readStatusFile - status: applied, path: C:\Documents and Settings\mozilla\Local Settings\Application Data\Mozilla\Firefox\UX\updates\0\update.status
[19:01:44.374] AUS:SVC UpdateManager:refreshUpdateStatus - Notifying observers that the update was staged. state: applied-service, status: applied
The update works fine, but I can't find a maintenanceservice.log file to check on.

Perhaps I should be backing out my changes until we get to the bottom of this, and design a strategy to transfer existing users. There are about 1.5K users which the UX folks probably don't want to lose right now.
On Windows 8 I reproduce, seeing this in maintenanceservice.log:

Passed in path: 'C:\Users\mozilla\AppData\Local\Mozilla\updates\E880E39E569BDD82\updates\0\updater.exe'; Using this path for updating: 'C:\Program Files (x86)\Mozilla Maintenance Service\update\updater.exe'.
updater.exe was compared successfully to the installation directory updater.exe.
The updater.exe application contains the Mozilla updater identity.
*** Warning: Certificate did not match issuer or name.  (1168)***
*** Warning: Error on certificate check.  (1168)***
*** Warning: Could not start process due to certificate check error on updater.exe. Updating update.status.  (0)***
Service command software-update complete.
service command MozillaMaintenance complete with result: Failure.
Component: Release Automation → General Automation
QA Contact: bhearsum → catlee
(In reply to Nick Thomas [:nthomas] from comment #12)
> Can you think of any reason it is broken now ? It seems to me that we're
> signing with the dep key, and also verifying against it, so I dunno why the
> update service is not working. 

The dep key for MARs is different and independent to the authenticode cert used for udpater.exe verification.  The authenticode check that the service does on updater.exe ensures that it is signed and from a trusted issuer. For dep builds it is not from a trusted issuer.
Ok, if I'm following this correctly the current state is that windows builds can't use the maintenance service because updater.exe has the wrong authenticode cert (dep rather than nightly). The updater falls back to a UAC prompt, so people are still getting updated (confirmed this by looking at blocklist pings).

This attachment would fix the signing of updater.exe, but also change the signing of the mar file. This breaks updates on windows completely, which I simulated by taking a beta build and changing the channel to nightly.

 --> backout: https://hg.mozilla.org/build/buildbot-configs/rev/92dac386a813

Need to figure out how to implement the last part of comment #11.
Comment on attachment 791992 [details] [diff] [review]
[buildbot-configs] Fix ux win32

Meant to make that comment on this attachment.
Attachment #791992 - Flags: checked-in+ → checked-in-
(In reply to Nick Thomas [:nthomas] from comment #15)
> Ok, if I'm following this correctly the current state is that windows builds
> can't use the maintenance service because updater.exe has the wrong
> authenticode cert (dep rather than nightly). 

Yep, more specifically it is probably signed by something issued from Mozilla Fake CA and not a trusted authority, so that's why the maintenance service won't accept it.
 
> The updater falls back to a UAC
> prompt, so people are still getting updated (confirmed this by looking at
> blocklist pings).

Right when there is a signature error on updater.exe we still allow the update without the service, because that way the user of the system can confirm if they trust the UAC prompt or not.

> This attachment would fix the signing of updater.exe, but also change the
> signing of the mar file. This breaks updates on windows completely, which I
> simulated by taking a beta build and changing the channel to nightly.

Yep that's what I mentioned would happen in the previous comment.  Thanks for confirming :) 

>  --> backout: https://hg.mozilla.org/build/buildbot-configs/rev/92dac386a813
> 
> Need to figure out how to implement the last part of comment #11.

The easiest way to fix without breaking updates is to not change how the MAR file is signed, but do nightly authenticode signing.
Live in production.
Nick and I talked about this the other day. I'm pretty sure that this is happening because "nightly-ux" isn't in the list of branches defined here:
https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/updater/moz.build#79
Attached patch patch.diff — — Splinter Review 
Will this work? This bug is really nagging me...(the workaround in comment 1 doesn't work for me).
Attachment #8402755 - Flags: review?(robert.strong.bugs)
Attachment #8402755 - Flags: feedback?(bhearsum)
Comment on attachment 8402755 [details] [diff] [review]
patch.diff

Review of attachment 8402755 [details] [diff] [review]:
-----------------------------------------------------------------

I'm pretty sure this will do it.
Attachment #8402755 - Flags: feedback?(bhearsum) → feedback+
Comment on attachment 8402755 [details] [diff] [review]
patch.diff

I checked the cert on nightly-ux and that should do it
Attachment #8402755 - Flags: review?(robert.strong.bugs) → review+
So, looks like I was wrong about the in-tree patch being the only thing that's needed. Nick pointed out to me that if we just do that, we'll embed the right fingerprints, but things will be signed wrong, because Buildbot will still use the self signed certs. I believe that's what Brian is talking about in comment #17, too.

Please don't land the latest patch yet - I'm going to look into this some more and see what the options are.
Assignee: nthomas → bhearsum
For reference, this is what I have for the firefox.exe cert on today's ux
CN = DigiCert Assured ID Code Signing CA-1
OU = www.digicert.com
O = DigiCert Inc
C = US
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #25)
> For reference, this is what I have for the firefox.exe cert on today's ux
> CN = DigiCert Assured ID Code Signing CA-1
> OU = www.digicert.com
> O = DigiCert Inc
> C = US

Which should be OK. I _think_ we're more concerned about the MAR certs here. Can you see what the current MARs are using, or tell me how to? I couldn't get mar or signmar to give me anything beyond a base64 dump of the signature...
Flags: needinfo?(robert.strong.bugs)
Verify a MAR file:
  mar [-C workingDir] -D DERFilePath -v signed_archive.mar
At most 8 signature certificate DER files are specified by -D0 DERFilePath1 -D1 DERFilePath2, ...
At most 8 verification certificate names are specified by -n0 certName -n1 certName2, ...

You can also see a usage of it in the following test
http://mxr.mozilla.org/mozilla-central/source/modules/libmar/tests/unit/test_sign_verify.js#132
Flags: needinfo?(robert.strong.bugs)
that is using signmar btw
Thanks Rob. I guess I have an older version, but I managed to make it work:
[cltsign@signing4.srv.releng.scl3.mozilla.com ~]$ /tools/signmar/bin/signmar -d /builds/signing/nightly-key-signing-server/secrets/mar/ -n nightly1 -v firefox-31.0a1.en-US.win32.complete.mar 
[cltsign@signing4.srv.releng.scl3.mozilla.com ~]$ echo $?
0
[cltsign@signing4.srv.releng.scl3.mozilla.com ~]$ /tools/signmar/bin/signmar -d /builds/signing/dep-key-signing-server/secrets/mar -n  dep1 -v firefox-31.0a1.en-US.win32.complete.mar 
ERROR: Error verifying signature.
ERROR: Not all signatures were verified.
[cltsign@signing4.srv.releng.scl3.mozilla.com ~]$ echo $?
255

So, AFAICT our current state is that all new UX nightlies' installers/exes/mars are being signed by the nightly certs, but embedding dep certs. This means that no updates for these users are working. Just landing Nochum's patch will fix this for new installs, but it won't help existing users. If we want to help them get up-to-date we can do the following:
1) Revert back to dep signing
2) Ship an update that embeds dep+nightly mar certs (ux only, _NOT_ mozilla-central)
3) wait some period of time for people to get the update
4) Switch back to nightly certs and revert to only embedding nightly certs

Or we can just land Nochum's patch and live the fact that people will need to re-install.
I'd leave that decision up to the maintainers of the ux branch who should have knowledge of the number of users, etc.
OK, so the UX branch averages about 1,500 users per day (over the last week). That seems low enough to just suck it up and live with stranding those users. It's a Nightly channel, anyways.

Gavin, can you make this call, or tell me who can?
Flags: needinfo?(gavin.sharp)
Can the fix here get checked in and later decide whether to fix old users automatically?
Comment on attachment 8402755 [details] [diff] [review]
patch.diff

(In reply to Nochum Sossonko [:Natch] from comment #33)
> Can the fix here get checked in and later decide whether to fix old users
> automatically?

Good idea. Now that you mention it, there's no downside to doing that. I landed this on inbound and ux. Anyone who installed after the buildbot-configs patches landed (roughly august last year) should get an update tomorrow.

https://hg.mozilla.org/integration/mozilla-inbound/rev/82187deb2938
https://hg.mozilla.org/projects/ux/rev/4cd1722578eb
Attachment #8402755 - Flags: checked-in+
https://hg.mozilla.org/mozilla-central/rev/82187deb2938
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Re-opening because we may do something with older users still.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 791992 [details] [diff] [review]
[buildbot-configs] Fix ux win32

I was confused, until I noticed that this got relanded in another bug
  http://hg.mozilla.org/build/buildbot-configs/rev/44dfa7abc482
March 17 2014.
Attachment #791992 - Flags: checked-in- → checked-in+
I assume that "Windows users that installed in the affected window" are a further subset of those 1500.

Sounds like we probably shouldn't do anything here.
Flags: needinfo?(gavin.sharp)
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #38)
> I assume that "Windows users that installed in the affected window" are a
> further subset of those 1500.
> 
> Sounds like we probably shouldn't do anything here.

Yeah, looks about 80% are Windows.

Thanks Gavin, we're all done here now.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Been getting updates for some time now. Yay!

Thank you very much.
Status: RESOLVED → VERIFIED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: