Sign B2G MAR files, provide 3 certs to use for testing

RESOLVED FIXED

Status

defect
P3
normal
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: overholt, Assigned: catlee)

Tracking

other
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:+, firefox18 fixed)

Details

Attachments

(6 attachments, 3 obsolete attachments)

Reporter

Description

7 years ago
Once bug 792452 (having multiple signatures in MARs) lands, we'll need RelEng to sign these updates.
Depends on: 795921
No longer depends on: 795921
bsmith, could you provide RelEng with the suggested certutil commands to generate 3 public/private key pairs, how to export a cert with the public keys, and then also provide commands for how I can import those certs into an existing NSS config dir?
I'll provide the commands they need to use for signmar once bug 792452 passes reviews and lands on m-c.
Summary: Sign B2G updates → Sign B2G MAR files, provide 3 certs to use for testing
Assignee

Updated

7 years ago
Depends on: 800364
Blocks: 797477
Depends on: 795921
certutil -S -d . -s "CN=test-oem-1"     -n test-oem-1     -x -t ",," -g 2048 -y 3
certutil -S -d . -s "CN=test-carrier-1" -n test-carrier-1 -x -t ",," -g 2048 -y 3
certutil -S -d . -s "CN=test-mozilla-1" -n test-oem-1     -x -t ",," -g 2048 -y 3

"-g 2048 -y 3" means 2048-bit (RSA) certificate with exponent 3.

-n XXX is the name you use to refer to the certificate in signmar and other utilities
-s "CN=XXX" is the subject distinguished name that will appear in the certificate.

Notice that these commands are the same for Firefox update MAR certificates, B2G update MAR certificates, and appstore certificates, except for the names of the certificates. (I added "-y 3" just as a performance optimization.)
(In reply to Brian Smith (:bsmith) from comment #3)
> certutil -S -d . -s "CN=test-oem-1"     -n test-oem-1     -x -t ",," -g 2048
> -y 3
> certutil -S -d . -s "CN=test-carrier-1" -n test-carrier-1 -x -t ",," -g 2048
> -y 3
> certutil -S -d . -s "CN=test-mozilla-1" -n test-oem-1     -x -t ",," -g 2048
> -y 3

certutil -S -d . -s "CN=test-oem-1"     -n test-oem-1     -x -t ",," -g 2048 -y 3
certutil -S -d . -s "CN=test-carrier-1" -n test-carrier-1 -x -t ",," -g 2048 -y 3
certutil -S -d . -s "CN=test-mozilla-1" -n test-mozilla-1 -x -t ",," -g 2048 -y 3
also, for completeness, to create a new certificate database in the current directory:

certutil -d . -N

You will be prompted twice for a password.

Also, in all of these commands, you can replace "-d ." with "-d <path>" where path is the path to some directory where you want the cert?.db and key?.db files to live.
I talked to bsmith and he said the command lines likely won't change after reviewing, so I think RelEng has everything they need to get started as soon as they are ready.

Here's what RelEng needs to do:
1. checkout mozilla-central on the platform you need signmar for on the signing servers
2. qpush the patch from bug 795921
3. qpush the patches from bug 792452 in any order
4. Add this to the bottom of your .mozconfig file: ac_add_options --enable-signmar$ 
5. make -f client.mk >out.txt 2>&1
6. copy out the objdir/dist/bin/signmar file
7. To sign a MAR file, it's the same as before except it accepts multiple certname parameters:
mar [-C workingDir] -d NSSConfigDir -n0 certname -n1 certname2 -n2 certname3 -s archive.mar out_signed_archive.mar
8. For b2g we can either use a single private key 3 times for now, or 3 different ones.   0 is returned on success.
9. To verify a MAR file, it's the same as before except it accepts multiple certname parameters:
mar [-C workingDir] -d NSSConfigDir -n0 certname -n1 certname2 -n2 certname3 -v signed_archive.mar
10. I'm providing these verify commands because it would be good after signing to make sure it verifies.  0 is returned on success.
11. Use the certutil commands that bsmith provided on the signing server
12. Sign the b2g builds with 3 signatures

Please only enable this new signmar first on oak for testing, that way we are sure we aren't breaking anything with the existing MAR signing.  And we are sure we won't break current b2g nightly updates.
Assignee

Updated

7 years ago
Priority: -- → P3
Reporter

Comment 7

7 years ago
Chris, what's the status here?
Flags: needinfo?(catlee)
Assignee

Comment 8

7 years ago
Just spoke with Brian and Marshall, and I think I'm unblocked here.

I'll create these certs tomorrow, and then start work on integrating signing into the build process.
Flags: needinfo?(catlee)
Assignee

Comment 9

7 years ago
I think signmar is working, but I can't verify:

catlee@aglon [11:40:33] [/mnt/ssd0/catlee/tools/release/signing] [signing *]
-> % ./b2gmar -t signing.1JB.mar
SIZE    MODE    NAME
5120000 0600    /tmp/signing.1JB
catlee@aglon [11:40:41] [/mnt/ssd0/catlee/tools/release/signing] [signing *]
-> % ./b2gmar -t -d secrets/b2gmar -v signing.1JB.mar
catlee@aglon [11:40:50] [/mnt/ssd0/catlee/tools/release/signing] [signing *]
-> % ./b2gmar -t -d secrets/b2gmar -v signing.1JB.mar ; echo $?
255
catlee@aglon [11:40:53] [/mnt/ssd0/catlee/tools/release/signing] [signing *]
-> % ./b2gmar -t -d secrets/b2gmar -v -n0 test-oem-1 -n1 test-carrier-1 -n2 test-mozilla-1 signing.1JB.mar ; echo $?
255
catlee@aglon [11:41:13] [/mnt/ssd0/catlee/tools/release/signing] [signing *]
-> % certutil -d secrets/b2gmar -L

Certificate Nickname                                         Trust Attributes
                                                             SSL,S/MIME,JAR/XPI

test-mozilla-1                                               u,u,u
test-oem-1                                                   u,u,u
test-carrier-1                                               u,u,u
Assignee

Comment 10

7 years ago
sorted this out with bbondy on irc. need to use the 'signmar' executable to verify signatures and not the 'mar' binary.
Assignee

Comment 11

7 years ago
Posted file test certs
What needs to happen with these? Need review? Land somewhere?
bsmith needs to take them and put them on the readonly nss db of the device and then give me the path of that nss db and cert names.
Flags: needinfo?(bsmith)
Assignee

Comment 14

7 years ago
And then releng needs to stand up machinery to sign update mars for nightlies, or do manual signing.
Assignee

Comment 15

7 years ago
And then releng needs to stand up machinery to sign update mars for nightlies, or do manual signing. We can't enable client-side signature checking until the updates are being signed properly.
Is there a bug for setting up signing for nightly builds yet catlee? I don't think this would work manually. I thought that was being done in this bug as well.
Assignee

Comment 17

7 years ago
Yes, this bug will be for automation changes required for nightly signing of b2g mars.
(In reply to Brian R. Bondy [:bbondy] from comment #13)
> bsmith needs to take them and put them on the readonly nss db of the device
> and then give me the path of that nss db and cert names.

I will create a patch that adds them, and I will put that patch in bug 797477.
Flags: needinfo?(bsmith)
Assignee

Comment 19

7 years ago
Attachment #680303 - Flags: review?(rail)
Attachment #680303 - Flags: review?(aki)
Comment on attachment 680303 [details] [diff] [review]
add jar/b2gmar signing support to signtool and signing server

>+        if passphrase:
>+            passphrases = passphrase.split(' ')
>+            for p in passphrases:
>+                proc.stdin.write(p)
>+                proc.stdin.write("\n")

Clever.

You could also split via null \0 or tab \t or something if you wanted to allow spaces in a future passphrase; or we could make the store/key passphrases the same, in which case we only need one passphrase.

But this works, and I'm fine with splitting on spaces.
Attachment #680303 - Flags: review?(aki) → review+
Assignee

Comment 21

7 years ago
(In reply to Aki Sasaki [:aki] from comment #20)
> Comment on attachment 680303 [details] [diff] [review]
> add jar/b2gmar signing support to signtool and signing server
> 
> >+        if passphrase:
> >+            passphrases = passphrase.split(' ')
> >+            for p in passphrases:
> >+                proc.stdin.write(p)
> >+                proc.stdin.write("\n")
> 
> Clever.
> 
> You could also split via null \0 or tab \t or something if you wanted to
> allow spaces in a future passphrase; or we could make the store/key
> passphrases the same, in which case we only need one passphrase.
> 
> But this works, and I'm fine with splitting on spaces.

Also we can change the key passphrase to match the keystore passphrase, in which case you only need to enter the one passphrase once. jarsigner tries to use the keystore passphrase to unlock the key, and if that fails, it asks for the key passphrase.
Comment on attachment 680303 [details] [diff] [review]
add jar/b2gmar signing support to signtool and signing server

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

::: lib/python/signing/utils.py
@@ +142,5 @@
>          log.exception(data)
>          raise
>  
> +def jar_unsignfile(filename):
> +    command = ["zip", filename, '-d', 'META-INF/*']

Technically, META-INF/* is too broad because there could be other things in META-INF/ that aren't signature-related.

Are we going to prohibit app developers from putting anything in META-INF/*? I think that is a good idea, but we should make sure it is in the spec.

Also, I am not sure if the zip command is case-insensitive; the matching of "meta-inf" needs to be case-insensitive.
Assignee

Comment 23

7 years ago
The zip option for case-insensitive matching is '-ic'. Probably a good idea to put that in the zip invocation.

Which files in META-INF are required for signatures?
Comment on attachment 680303 [details] [diff] [review]
add jar/b2gmar signing support to signtool and signing server

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

::: lib/python/signing/utils.py
@@ +142,5 @@
>          log.exception(data)
>          raise
>  
> +def jar_unsignfile(filename):
> +    command = ["zip", filename, '-d', 'META-INF/*']

I second :bsmith. Would be better to have more strict glob here. Not a stopper.

@@ +146,5 @@
> +    command = ["zip", filename, '-d', 'META-INF/*']
> +    stdout = tempfile.TemporaryFile()
> +    log.debug("running %s", command)
> +    proc = Popen(command, stdout=stdout, stderr=STDOUT, stdin=PIPE)
> +    if proc.wait() not in (0, 12):

Can you add some comments here why you ignore exit status 12? BTW, shouldn't it be 11? At least my man page states this:

11     no matching files were found. (what happens for not signed JARs)
Attachment #680303 - Flags: review?(rail) → review+
Assignee

Comment 26

7 years ago
Attachment #680710 - Flags: review?(aki)
Comment on attachment 680710 [details] [diff] [review]
Do signing if MOZ_SIGNING_SERVERS is set

Hm.
I was thinking this weekend that maybe I should have put the retry logic in vcs_checkout() instead of vcs_checkout_repos().  Or we could return a list of revisions from vcs_checkout_repos(), either way.
Attachment #680710 - Flags: review?(aki) → review+
Assignee

Comment 28

7 years ago
Attachment #680710 - Attachment is obsolete: true
Attachment #680742 - Flags: review?(aki)

Updated

7 years ago
Attachment #680742 - Flags: review?(aki) → review+
Assignee

Updated

7 years ago
Attachment #680742 - Flags: checked-in+
Assignee

Comment 29

7 years ago
(In reply to Rail Aliiev [:rail] from comment #24)
> Comment on attachment 680303 [details] [diff] [review]
> add jar/b2gmar signing support to signtool and signing server
> 
> Review of attachment 680303 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: lib/python/signing/utils.py
> @@ +142,5 @@
> >          log.exception(data)
> >          raise
> >  
> > +def jar_unsignfile(filename):
> > +    command = ["zip", filename, '-d', 'META-INF/*']
> 
> I second :bsmith. Would be better to have more strict glob here. Not a
> stopper.

I'm not sure what the correct list of files here should be. I've seen these in various signed files:
MANIFEST.MF
MYKEY.DSA
MYKEY.SF
CERT.SF
CERT.RSA
Assignee

Comment 30

7 years ago
turns out the zip we use doesn't support '-ic'. I've changed jar_unsignfile to run "unzip -l" to get a list of files, and then use case insensitive regex to find META-INF/* files.
Attachment #680303 - Attachment is obsolete: true
Attachment #681060 - Flags: review?(rail)
Attachment #681060 - Flags: review?(aki)
Comment on attachment 681060 [details] [diff] [review]
add jar/b2gmar signing support to signtool and signing server

lgtm
Attachment #681060 - Flags: review?(rail) → review+
Comment on attachment 681060 [details] [diff] [review]
add jar/b2gmar signing support to signtool and signing server

We probably don't have to check for exit code 12 anymore, since we're checking to make sure there's something to do beforehand.  But most likely it's harmless.
Attachment #681060 - Flags: review?(aki) → review+
Assignee

Comment 33

7 years ago
we also set MOZ_SIGNING_SERVERS in the environment to make it easier for mozharness scripts to do signing.
Attachment #681121 - Flags: review?(bhearsum)
Assignee

Updated

7 years ago
Attachment #680652 - Flags: review?(bhearsum)
Assignee

Comment 34

7 years ago
Attachment #681162 - Flags: review?(bhearsum)
Attachment #680652 - Flags: review?(bhearsum) → review+
Attachment #681121 - Flags: review?(bhearsum) → review+
Comment on attachment 681162 [details] [diff] [review]
puppet manifests for signing server changes

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

::: modules/signingserver/manifests/init.pp
@@ +38,5 @@
>                      }
> +                   file {
> +                        # Make sure jarsigner is in $PATH for signing
> +                        "/usr/local/bin/jarsigner":
> +                            ensure => "/tools/jdk-1.6.0_17/bin/jarsigner";

Should this depend on the jdk1.6 package? Probably doesn't matter...

::: modules/signingserver/manifests/instance.pp
@@ +130,5 @@
>                  "$basedir/bin/signmar":
>                      source => "puppet:///modules/signingserver/signmar",
>                      mode => 755,
>                      require => Exec["$basedir-virtualenv"];
> +                "$basedir/bin/b2gsignmar":

Per IRC, it seems like we should be able to use the same signmar as long as it's backwards compatible.
Assignee

Updated

7 years ago
Attachment #681060 - Flags: checked-in+
Assignee

Updated

7 years ago
Attachment #681121 - Flags: checked-in+
Assignee

Comment 36

7 years ago
Attachment #681162 - Attachment is obsolete: true
Attachment #681162 - Flags: review?(bhearsum)
Attachment #681228 - Flags: review?(bhearsum)
Attachment #681228 - Flags: review?(bhearsum) → review+
Assignee

Updated

7 years ago
Attachment #681228 - Flags: checked-in+
Assignee

Updated

7 years ago
Attachment #680652 - Flags: checked-in+
Assignee

Comment 37

7 years ago
Status today:

signing{1..3} have been updated
mac-signing1 has been updated
mac-signing{2..4} still need to be done. This is ok since the mac signing servers aren't used for b2g/jar signing.

The buildbot-configs patch has landed. The next unagi nightlies that happen after the next reconfig should be signed.
Assignee

Comment 38

7 years ago
mac-signing{2..4} are now done.
In production.
Assignee

Comment 40

7 years ago
unagi nightly mars are now being signed

NB - these aren't the builds used for dogfooding yet, so client-side checks for signatures shouldn't be enabled.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
> NB - these aren't the builds used for dogfooding yet, so client-side checks for 
> signatures shouldn't be enabled.

Is there another bug tracking that? When should it be enabled?
(In reply to Brian R. Bondy [:bbondy] from comment #41)
> > NB - these aren't the builds used for dogfooding yet, so client-side checks for 
> > signatures shouldn't be enabled.
> 
> Is there another bug tracking that? When should it be enabled?

I assume that Chris meant that until the RelEng produced unagi builds are the ones being used on the dogfooding devices, that we can't enable client side checks on them. I thought there was a bug on that somewhere, but I'm having trouble finding it.
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.