Validate manual run of omni.jar signing
Categories
(Cloud Services :: Operations: Autograph, enhancement)
Tracking
(firefox-esr68 fixed, firefox69 fixed)
People
(Reporter: Callek, Unassigned)
References
Details
Attachments
(5 files, 1 obsolete file)
Over in Bug 1515173 we are looking at signing omni.jar, using COSE signing with a fallback of PKCS#7. Likely using the same cert as System Addons
We have a few things to validate.
- That Firefox can validate the omni.jar signing itself when passed back from autograph
- To know if autograph changes the ordering of the omni.jar that we do in order to speed up file loads.
- If it does change the ordering, to know if the signing gets invalidated by fixing it post-signing.
once we know these answer we can get a better idea of timelines around these parts.
I'll provide this to g-k by sometime on Wed next week.
Got jars from :Callek yesterday and signed them (just w/ PK7 for now since COSE is busted).
autograph is compressing entries which might be an issue, but file order is preserved:
Downloads/omnijar-signing - [master●] » zipinfo unsigned/omni.ja | head
warning [unsigned/omni.ja]: 17104713 extra bytes at beginning or within zipfile
(attempting to process anyway)
Archive: unsigned/omni.ja
error [unsigned/omni.ja]: reported length of central directory is
-17104713 bytes too long (Atari STZip zipfile? J.H.Holm ZIPSPLIT 1.1
zipfile?). Compensating...
Zip file size: 17219536 bytes, number of entries: 1302
warning [unsigned/omni.ja]: 17104713 extra bytes at beginning or within zipfile
(attempting to process anyway)
error [unsigned/omni.ja]: reported length of central directory is
-17104713 bytes too long (Atari STZip zipfile? J.H.Holm ZIPSPLIT 1.1
zipfile?). Compensating...
-rw---- 2.0 fat 247740 b- stor 10-Jan-01 00:00 greprefs.js
-rw---- 2.0 fat 5109 b- stor 10-Jan-01 00:00 defaults/pref/services-sync.js
-rw---- 2.0 fat 1392 b- stor 10-Jan-01 00:00 defaults/pref/marionette.js
Downloads/omnijar-signing - [master●] » zipinfo signed/omni.ja | head
Archive: signed/omni.ja
Zip file size: 5549581 bytes, number of entries: 1305
-rw---- 2.0 fat 247740 bl defN 80-000-00 00:00 greprefs.js
-rw---- 2.0 fat 5109 bl defN 80-000-00 00:00 defaults/pref/services-sync.js
-rw---- 2.0 fat 1392 bl defN 80-000-00 00:00 defaults/pref/marionette.js
-rw---- 2.0 fat 6 bl defN 80-000-00 00:00 update.locale
-rw---- 2.0 fat 6 bl defN 80-000-00 00:00 res/multilocale.txt
-rw---- 2.0 fat 72 bl defN 80-000-00 00:00 chrome.manifest
-rw---- 2.0 fat 2716 bl defN 80-000-00 00:00 chrome/chrome.manifest
-rw---- 2.0 fat 2950 bl defN 80-000-00 00:00 components/components.manifest
OK the other omni.ja is too large to upload, but I put everything in: https://drive.google.com/drive/folders/1yqKitxgD2OydARrpemByut2Jkdp9xPUx
omni* files are signed versions of unsigned/omni.ja
browser-omni* files are signed versions of unsigned/browser/omni.ja
The *-pk7-sha2.ja files just have PKCS7 signatures with SHA256 digests and the *-pk7-sha2-cose-es256.ja files also have a COSE ES256 signature.
They're signed with autograph 3.0.2 (with the COSE packaging fix) using the prod extension intermediate cert (that we should swap for the HSM one once we settle on a signature format).
|
||
Comment 4•5 years ago
|
||
OK so I've tested these with the verifyJar function from bug 1515712 and the good news is that they all seem to work with the existing code, which uses https://searchfox.org/mozilla-central/rev/9ee63566281365f26e7a4b06c9d4e2219e64c3e8/security/manager/ssl/nsIX509CertDB.idl#264
The only thing that seems odd to me is that these use the "Mozilla Extensions" OU, which is a bit confusing since the omni JARs are not extensions :)
Would it make more sense to use a different OU, we use "Mozilla Components" for system add-on updates for instance and using this for bundled XPIs and JARs might make more sense.
|
Reporter | |
Comment 5•5 years ago
|
||
I have little opinion as releng, as to what the OU should be; it probably does make sense to either be "Mozilla Components" or even something entirely new for this, rather than "Mozilla Extensions".
|
||
Comment 6•5 years ago
|
||
I think it's either "Mozilla Components", aka a system addon, or something new entirely. I'm not very familiar with the permissions granted to system addon (is it different from what omni.ja can do?) so I'll leave that decision you, :rhelmer.
OK great! It sounds like we can start working on automating the signing.
I can resign with the "Mozilla Components" OU or something new (requires a trivial ~2 line change in autograph but that would still need to be deployed).
I also wasn't sure what CN to use and picked omnijar@mozilla.org as a placeholder.
What CN and OU do we want to use? Let me know and I can make those changes.
|
|
||
Comment 8•5 years ago
|
||
(In reply to Julien Vehent [:ulfr] from comment #6)
I think it's either "Mozilla Components", aka a system addon, or something new entirely. I'm not very familiar with the permissions granted to system addon (is it different from what omni.ja can do?) so I'll leave that decision you, :rhelmer.
"Mozilla Components" sgtm. System add-ons and omni.ja both run with the same privilege level.
(In reply to Greg Guthe [:g-k] [:gguthe] from comment #7)
OK great! It sounds like we can start working on automating the signing.
I can resign with the "Mozilla Components" OU or something new (requires a trivial ~2 line change in autograph but that would still need to be deployed).
I also wasn't sure what CN to use and picked
omnijar@mozilla.orgas a placeholder.What CN and OU do we want to use? Let me know and I can make those changes.
I don't have an opinion on the CN, :ulfr do you have thoughts on this?
OK!
OU=Mozilla Components i.e. system addon https://github.com/mozilla-services/autograph/blob/db863e066a4001d438e7e2af53396d04d11435e5/signer/xpi/xpi.go#L148
and per IRC we'll use CN=omni.ja@mozilla.org and the existing AMO intermediates:
g-k> ulfr: do you have an opinion on which CN to use for the omni.ja PK7 sig?
ulfr> omni.ja@mozilla.org ?
Finally, if I recall correctly we want to sign using signingscript from taskcluster.
|
||
Comment 10•5 years ago
|
||
Added a signer for system addons and creds for signingscript to the autograph configs to go out with the next deploy.
|
|
Reporter | |
Comment 11•5 years ago
|
||
I did a new run of omni.ja piece-together from some code I am writing to support the theory of how this will work.
This is still the older omni.ja files and the most recent run of signing.
Files are at https://drive.google.com/drive/folders/1epdXgUex4ZVZlKZDL30BXh3McOCtVtoh?usp=sharing in py2-sign and py3-sign (py3 is what I'll use, but I included both so i can validate if I broke in py3 migration of code)
:glandium
Can you please validate that these omni.ja files are properly organized and aligned?
:rhelmer
Can you please re validate these based on the test you did in https://bugzilla.mozilla.org/show_bug.cgi?id=1533818#c4 -- the OU will not be different but this should be testable.
|
|
Reporter | |
Comment 12•5 years ago
|
||
(In reply to Justin Wood (:Callek) from comment #11)
I did a new run of omni.ja piece-together from some code I am writing to support the theory of how this will work.
This is still the older omni.ja files and the most recent run of signing.
Apologies, I didn't double check my work, this isn't ready yet.
|
|
Reporter | |
Comment 13•5 years ago
|
||
(In reply to Justin Wood (:Callek) from comment #11)
I did a new run of omni.ja piece-together from some code I am writing to support the theory of how this will work.
This is still the older omni.ja files and the most recent run of signing.
Files are at https://drive.google.com/drive/folders/1epdXgUex4ZVZlKZDL30BXh3McOCtVtoh?usp=sharing in py2-sign and py3-sign (py3 is what I'll use, but I included both so i can validate if I broke in py3 migration of code)
:glandium
Can you please validate that these omni.ja files are properly organized and aligned?
:rhelmer
Can you please re validate these based on the test you did in https://bugzilla.mozilla.org/show_bug.cgi?id=1533818#c4 -- the OU will not be different but this should be testable.
These are now ready for testing, py2 and py3 variants have the same md5sum so it is unlikely to need separate tests.
Thank you in advance
|
|
Reporter | |
Comment 14•5 years ago
|
||
This attachment exists in order to have the code I used stored somewhere :-)
|
|
||
Comment 15•5 years ago
•
|
||
lgtm!
Here's the JS test code I used, for posterity. This can be run in the Browser Console.
|
||
Comment 16•5 years ago
|
||
Comment on attachment 9060596 [details] python script to 'repack' the old jar with the signed metadata > signed_zip = zipfile.ZipFile(signed, 'r') > for fname in signed_zip.namelist(): > if fname.startswith('META-INF'): > dest_writer.add( > fname, > signed_zip.open(fname, 'r')) You could use a JarReader instead: ``` for f in JarReader(signed): if f.filename.startswith('META-INF/'): dest_writer.add(f.filename, f) ``` or even better, with a JarFinder. ``` for p, f in JarFinder(signed, JarReader(signed)).find('META-INF/**'): dest_writer.add(p, f) ``` The signed omni.ja look good (and I was expecting as much considering the code) ; note I only checked the py2 one. But I think you said the py3 was byte-identical, so...
|
|
||
Comment 17•5 years ago
|
||
Generally speaking, I think it would be better to handle this with a repack, so that you don't hardcode anything about where the omni jars are, that there are two, etc. Because that's subject to change, and it would be better if your code didn't have to change when that happens.
|
|
Reporter | |
Comment 18•5 years ago
|
||
|
|
Reporter | |
Comment 19•5 years ago
|
||
|
|
Reporter | |
Comment 20•5 years ago
|
||
|
||
Updated•5 years ago
|
|
||
Comment 21•5 years ago
|
||
Pushed by jwood@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/69128ae35322 Start signing omni.ja files. r=aki https://hg.mozilla.org/integration/autoland/rev/2511377ca37f Don't try to expand from a 'cose.manifest' file when repacking the l10n copy of omni.ja. r=aki
|
||
Comment 22•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/69128ae35322
https://hg.mozilla.org/mozilla-central/rev/2511377ca37f
|
||
Comment 23•5 years ago
|
||
This bug may have contributed to a sudden change in the Telemetry probe corroborate.omnijar_corrupted 1 which seems to have occurred in Nightly 20190613 2 3.
There was a change from 100% true to 99% false... which is a heckuva swing : )
Is this an improvement? A regression?
Is this intentional? Is this expected?
Is this probe still measuring something useful?
|
|
||
Comment 24•5 years ago
|
||
(In reply to Chris H-C :chutten from comment #23)
This bug may have contributed to a sudden change in the Telemetry probe
corroborate.omnijar_corrupted1 which seems to have occurred in Nightly 20190613 2 3.There was a change from 100% true to 99% false... which is a heckuva swing : )
Is this an improvement? A regression?
Is this intentional? Is this expected?
This is unexpected to me at least :) I wonder if we stopped signing the omni jar?
Is this probe still measuring something useful?
Yes, though it will be more useful once it's shipped to Release channel. On Nightly it's useful for diagnostic testing, like this!
|
|
||
Comment 25•5 years ago
|
||
(In reply to Robert Helmer [:rhelmer] from comment #24)
(In reply to Chris H-C :chutten from comment #23)
This bug may have contributed to a sudden change in the Telemetry probe
corroborate.omnijar_corrupted1 which seems to have occurred in Nightly 20190613 2 3.There was a change from 100% true to 99% false... which is a heckuva swing : )
Oh actually I apologize, I misread this bit ^ I've been watching it on redash and it was hovering in the mid-to-high 90s for false last I checked... let me double-check my query.
Is this an improvement? A regression?
Is this intentional? Is this expected?
This is unexpected to me at least :) I wonder if we stopped signing the omni jar?
Is this probe still measuring something useful?
Yes, though it will be more useful once it's shipped to Release channel. On Nightly it's useful for diagnostic testing, like this!
|
|
||
Comment 26•5 years ago
|
||
OK here's what I've been watching:
https://sql.telemetry.mozilla.org/queries/63758#163057
|
|
||
Comment 27•5 years ago
|
||
Which agrees with the information on tmo. This change was from June 13, so your query starts a little late.
|
|
||
Comment 28•5 years ago
|
||
(In reply to Chris H-C :chutten from comment #27)
Which agrees with the information on tmo. This change was from June 13, so your query starts a little late.
Oh, OK. Sorry for my lack of reading comprehension, yes the omni jar started being signed and therefore the built-in check (corroborator) started working on or around June 13th: https://hg.mozilla.org/mozilla-central/rev/69128ae35322
So I would say this is all expected, and on Nightly I'd expect that we'd see very little corruption (that we see it at all is a bit worrying), my hypothesis is that Beta and Release will have much more.
|
|
Reporter | |
Comment 29•5 years ago
|
||
Chutten, before this bug landed, omni.ja wasn't actually signed... I wonder if the sudden fact that it is signed is allowing us to identify it is invalid...
The current code seems to suggest its required, https://searchfox.org/mozilla-central/source/security/apps/AppSignatureVerification.cpp#1274 (which is coupled with the policy we pass in which is int(2) which means COSE required pkcs valid if present)
I can look deeper here if useful though.
|
||
Comment 30•5 years ago
|
||
| uplift | ||
https://hg.mozilla.org/releases/mozilla-esr68/rev/995b67ef37a9f67687d786e5ce1bb244d2e76b99
FIREFOX_ESR_68_2_X_RELBRANCH https://hg.mozilla.org/releases/mozilla-esr68/rev/bee37ce71ba5025366206c031f82f559c2d02382
Description
•