make update verify work in staging with dep signing
Categories
(Release Engineering :: Release Automation: Updates, enhancement, P1)
Tracking
(firefox-esr60 fixed)
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | fixed |
People
(Reporter: bhearsum, Unassigned)
References
Details
(Keywords: leave-open)
Attachments
(6 files, 5 obsolete files)
46 bytes,
text/x-phabricator-request
|
robert.strong.bugs
:
review+
|
Details | Review |
46 bytes,
text/x-phabricator-request
|
nthomas
:
review+
|
Details | Review |
10.02 KB,
patch
|
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
46 bytes,
text/x-phabricator-request
|
robert.strong.bugs
:
review+
mshal
:
review+
|
Details | Review |
46 bytes,
text/x-phabricator-request
|
tomprince
:
review+
|
Details | Review |
46 bytes,
text/x-phabricator-request
|
ted
:
review+
|
Details | Review |
We've had a long standing problem in staging releases where update verify fails unless the staging release is signed with the release keys (well documented in bug 997732 and 1401572). We don't want to do this anymore, both because it's bad practice and it makes it more difficult to do staging releases on Try. Nick had some ideas on how to fix this in https://bugzilla.mozilla.org/show_bug.cgi?id=1401572#c1. To fix it immediately, we're going to do a binary search and replace on the updater binary from old releases to replace the release certs they have embedded in them with dep certs. I've been working on this in https://github.com/mozilla/build-tools/compare/master...mozbhearsum:cr2?expand=1 and https://github.com/mozbhearsum/gecko-unified/compare/dep-updater?expand=1. For a more long term, and less hacky fix, we're going to start building a separate updater binary with releases that always embeds the dep certs. Once it's available for all releases ahead of the current watershed, we can start using it for update verify instead of the hacking the release updater. I've been working on this part in https://github.com/mozbhearsum/gecko-unified/compare/separate-updater?expand=1.
Reporter | ||
Comment 1•6 years ago
|
||
This patch gets us building an updater binary that always embeds the dep certificates (instead of release or nightly). It gets installed to its own directory, and a subsequent patch will get us creating a test package that includes it.
Reporter | ||
Comment 2•6 years ago
|
||
This patch creates a test package for the new dep updater from https://phabricator.services.mozilla.com/D5539.
Comment 3•6 years ago
|
||
Comment on attachment 9008074 [details] bug 1490119: Add build system bits for building dep updater. Robert Strong [:rstrong] (use needinfo to contact me) has approved the revision.
Pushed by bhearsum@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/59f5c546faa2 Add build system bits for building dep updater. r=rstrong
Reporter | ||
Updated•6 years ago
|
Backout by nbeleuzu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0ce61c4a2125 Backed out changeset 59f5c546faa2 for artifact build failures. CLOSED TREE
Updated•6 years ago
|
Reporter | ||
Comment 6•6 years ago
|
||
This patch adds support for mar cert replacement to the UpdateVerifyConfig class (which we use when we chunk the full update verify config) as well as to update verify itself. The most interesting parts are: - The replacement script itself, which does a binary replacement (as you suggested!), and assumes that the old and new certs will be the same length. I tried doing padding in this script, but it did not work out - the updater ended up not recognizing the certs. Tom changed our existing dep certs to be the right length by changing the subject with https://github.com/google/der-ascii. (Thankfully, nightly and release certs are exactly the same length already!) - The verify.sh changes. I took a very simple approach here and just made it possible to swap in any set of certs by looking for both other sets and trying to replace them. We'll never find nightly and release in the same updater binary (I hope), but this avoids the need for the caller to have knowledge of which certs the updater binary already has. This was tested on try in pushes like https://treeherder.mozilla.org/#/jobs?repo=try&revision=633b257b78953216e1e630fdfda1b8b04377edf1, as well as on maple in pushes like https://treeherder.mozilla.org/#/jobs?repo=maple&revision=68810f430192e2ae388a1e722b29c9e2c34da6b4. The original set of jobs on maple failed because I forgot to update the tools repo it was using -- the reruns from each platform continued to fail because of channel differences. I believe this is something that Tom is planning to address as part of his try staging work. There's an accompanying in-tree change coming via Phabricator.
Reporter | ||
Comment 7•6 years ago
|
||
Imports the changes to the UpdateVerifyConfig class, and sets --override-certs for staging releases.
Comment 8•6 years ago
|
||
Comment on attachment 9008529 [details] [diff] [review] support mar cert replacement in update verify >diff --git a/release/mar_certs/dep1.der b/release/mar_certs/dep1.der >new file mode 100644 Are we ever going to need to refresh these keys ? If so, some documentation on where they come from and how to pad would be valuable. >+++ b/release/replace-updater-certs.py >+for find_cert, replace_cert in zip(*[iter(cert_pairs)]*2): Well that's quite tricksy!
Comment 9•6 years ago
|
||
Comment on attachment 9008536 [details]
Set override_certs in update verify config creator.
Nick Thomas [:nthomas] (UTC+12) has approved the revision.
Reporter | ||
Comment 10•6 years ago
|
||
(In reply to Nick Thomas [:nthomas] (UTC+12) from comment #8) > Comment on attachment 9008529 [details] [diff] [review] > support mar cert replacement in update verify > > >diff --git a/release/mar_certs/dep1.der b/release/mar_certs/dep1.der > >new file mode 100644 > > Are we ever going to need to refresh these keys ? If so, some documentation > on where they come from and how to pad would be valuable. Good idea. I'll add a README to the directory. I know we have plans to rotate mar keys on a regular basis, so it may become important! > >+++ b/release/replace-updater-certs.py > >+for find_cert, replace_cert in zip(*[iter(cert_pairs)]*2): > > Well that's quite tricksy! :)
Reporter | ||
Comment 11•6 years ago
|
||
Here's the tools patch as landed (with a README).
Comment 12•6 years ago
|
||
Pushed by bhearsum@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/077a6422ec4a Set override_certs in update verify config creator. r=nthomas
Comment 13•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/077a6422ec4a
Reporter | ||
Comment 14•6 years ago
|
||
This patch gets us building an updater binary that always embeds the dep certificates (instead of release or nightly), and builds a new tests package that includes it. I had a lot of trouble getting the test package generated correctly due to the fact that things in "_tests" won't be included for test packages that aren't "common". My fix for that isn't ideal - I'm open to something better.
Comment 15•6 years ago
|
||
Comment on attachment 9009247 [details] bug 1490119: Add build system bits for building dep updater. Robert Strong [:rstrong] (use needinfo to contact me) has approved the revision.
Comment 16•6 years ago
|
||
Comment on attachment 9009247 [details] bug 1490119: Add build system bits for building dep updater. Michael Shal [:mshal] has approved the revision.
Comment 17•6 years ago
|
||
Pushed by bhearsum@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/96059b4f6d8a Add build system bits for building dep updater. r=firefox-build-system-reviewers,mshal,rstrong
Comment 18•6 years ago
|
||
Pushed by bhearsum@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1f4d6c868248 Bump timeout for automationrelevance. r=tomprince
Reporter | ||
Comment 19•6 years ago
|
||
Bump the timeout for pulling the automationrelevance json. For very large merges, this can take awhile. For example, https://hg.mozilla.org/projects/birch/json-automationrelevance/7ecef4cf1884534b0ba6271c9f94902b3ed644a6 takes ~22 seconds.
Comment 20•6 years ago
|
||
Comment on attachment 9010411 [details]
Bump timeout for automationrelevance.
Tom Prince [:tomprince] has approved the revision.
Reporter | ||
Comment 21•6 years ago
|
||
OK, so we're now at a state where staging releases will work fine when dep signed, because we do cert replacement as part of update verify. This is still only intended to be a short term fix until we watershed at a version that includes the new dep updater -- then we can download that instead of doing cert replacement in the real updater. To do that, I believe we'll need to: - Start beetmoving the dep updater into the candidates directory - Adjust the update verify config generator to point at it for the updater package (only for staging) - Ensure update verify can download/unpack/use it correctly It's worth pointing out that using the dep updater means that we won't use localized updater binaries to apply updates -- but I think that's an OK trade-off to make for staging (the alternative is to continue using the more fragile cert replacement indefinitely). If anybody thinks this is a bad idea, I'm open to reconsidering it.
Comment 22•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/96059b4f6d8a
Reporter | ||
Comment 23•6 years ago
|
||
chmanchester, can you help me sort out the OS X artifact build issues with https://phabricator.services.mozilla.com/D5900?
Comment 24•6 years ago
|
||
Backed out changeset 96059b4f6d8a (Bug 1490119) for breaking OSX builds. Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=59f5c546faa289cd0b7b713dc5ac9e4e53cb0c02 Backout link: https://hg.mozilla.org/mozilla-central/rev/ac9f1219d11bf1a56ec1ace8e3ba9ff113b5cacb
Comment 25•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1f4d6c868248
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Comment 26•6 years ago
|
||
This patch gets us building an updater binary that always embeds the dep certificates (instead of release or nightly), and builds a new tests package that includes it. This was originally D5900, but that was backed out due to busting artifact builds. I've fixed that by removing the Makefile that Ted pointed out is unnecessary.
Reporter | ||
Comment 27•6 years ago
|
||
(In reply to Ben Hearsum (:bhearsum) from comment #23) > chmanchester, can you help me sort out the OS X artifact build issues with > https://phabricator.services.mozilla.com/D5900? Ted actually got me unblocked today.
Comment 28•6 years ago
|
||
Comment on attachment 9010718 [details] bug 1490119: Build separate updater that always embeds dep certificates. Ted Mielczarek [:ted] [:ted.mielczarek] has approved the revision.
Comment 29•6 years ago
|
||
Pushed by bhearsum@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eeb4c8b90872 Build separate updater that always embeds dep certificates. r=ted,firefox-build-system-reviewers
Comment 30•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/eeb4c8b90872
Reporter | ||
Comment 31•6 years ago
|
||
After running some tests in one of the Docker containers our tests run in, I realized we need more libraries in this package - namely, NSS and friends. My original testing was done on my laptop, and system NSS libraries made me miss this requirement :(.
Reporter | ||
Comment 32•6 years ago
|
||
Beetmoves the new dep updater into the candidates directory, like the other test packages.
Reporter | ||
Comment 33•6 years ago
|
||
This patch changes the update verify config creator to use the updater-dep package instead of the previous full build. Depending what happens with https://phabricator.services.mozilla.com/D6698 we may end needing both =\.
Reporter | ||
Comment 34•6 years ago
|
||
I'm going to be away for a number of months after this week, so I wanted to drop a quick status update. As things stand now, cert replacement is on mozilla-central and fully working -- this means that staging releases should be able to run update verify correctly without doing release signing. What remains in this bug is finishing up the work on the dep updater. It's now being created, but there's some follow-ups before we can use it: * Beetmove it to the candidates dir (https://github.com/mozilla-releng/beetmoverscript/pull/186, https://bugzilla.mozilla.org/attachment.cgi?id=9012671) * Add the other required shared libraries to the package (https://bugzilla.mozilla.org/attachment.cgi?id=9011585) -- which may or may not happen * Finish up the update verify config creator work to point at the updater-dep package (https://bugzilla.mozilla.org/attachment.cgi?id=9012691). If we don't add the required shared libraries to that package, we'll need to add both the updater-dep and full package to the configs. * Tidy up and test the update verify changes needed (https://github.com/mozilla/build-tools/compare/master...mozbhearsum:update-verify-dep-updater?expand=1). This will also need a tweak if we end up not adding other shared libraries to the updater-dep package.
Reporter | ||
Updated•6 years ago
|
Comment 35•6 years ago
|
||
If someone else is going to pick up this work before bhearsum gets back, I had a comment on one of the patches in the series: https://phabricator.services.mozilla.com/D6698#193003
Comment 36•6 years ago
|
||
uplift |
https://hg.mozilla.org/releases/mozilla-release/rev/5ccec2d040a6d401261013979140b502b9dd4eff
Comment 37•6 years ago
|
||
(In reply to bhearsum@mozilla.com (back in 2019Q1) from comment #34) > What remains in this bug is finishing up the work on the dep updater. It's > now being created, but there's some follow-ups before we can use it: > * Beetmove it to the candidates dir > (https://github.com/mozilla-releng/beetmoverscript/pull/186, Beetmoverscript bump to 7.10.1 to encompass the updater in its templates lands today.
Comment 38•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-esr60/rev/f01536d9b9a7 https://hg.mozilla.org/releases/mozilla-esr60/rev/ff621ab0c6f6
Reporter | ||
Comment 39•5 years ago
|
||
Tom, it appears you got update verify working on try without the dep updater work. Do you think this part is still needed?
Comment 40•5 years ago
|
||
The binary hacking code seems to work well enough, that I'm not sure it worth spending more time getting working. If we do, we should consider publishing the dep-updater as a toolchain (so it can be accessed via the index), rather than publishing to archive.m.o.
Reporter | ||
Comment 41•5 years ago
|
||
(In reply to bhearsum@mozilla.com (:bhearsum) from comment #21)
OK, so we're now at a state where staging releases will work fine when dep
signed, because we do cert replacement as part of update verify. This is
still only intended to be a short term fix until we watershed at a version
that includes the new dep updater -- then we can download that instead of
doing cert replacement in the real updater.To do that, I believe we'll need to:
- Start beetmoving the dep updater into the candidates directory
- Adjust the update verify config generator to point at it for the updater
package (only for staging)- Ensure update verify can download/unpack/use it correctly
It's worth pointing out that using the dep updater means that we won't use
localized updater binaries to apply updates -- but I think that's an OK
trade-off to make for staging (the alternative is to continue using the more
fragile cert replacement indefinitely). If anybody thinks this is a bad
idea, I'm open to reconsidering it.
Given how long the cert replacement has been working for, I'm going to call this fixed. If we want to come back and finish up with the dep updater later, we can do that.
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Description
•