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

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Andrew Overholt [:overholt]]

Once bug 792452 (having multiple signatures in MARs) lands, we'll need RelEng to sign these updates.

Comment 1

[Brian R. Bondy [:bbondy]]

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?

Comment 2

[Brian R. Bondy [:bbondy]]

I'll provide the commands they need to use for signmar once bug 792452 passes reviews and lands on m-c.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.)

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(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

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 6

[Brian R. Bondy [:bbondy]]

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.

Comment 7

[Andrew Overholt [:overholt]]

Chris, what's the status here?

Comment 8

[Chris AtLee [:catlee]]

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.

Comment 9

[Chris AtLee [:catlee]]

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

Comment 10

[Chris AtLee [:catlee]]

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

Comment 11

[Chris AtLee [:catlee]]

Attachment: test certs

Comment 12

[Dietrich Ayala (:dietrich)]

What needs to happen with these? Need review? Land somewhere?

Comment 13

[Brian R. Bondy [:bbondy]]

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.

Comment 14

[Chris AtLee [:catlee]]

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

Comment 15

[Chris AtLee [:catlee]]

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.

Comment 16

[Brian R. Bondy [:bbondy]]

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.

Comment 17

[Chris AtLee [:catlee]]

Yes, this bug will be for automation changes required for nightly signing of b2g mars.

Comment 18

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(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.

Comment 19

[Chris AtLee [:catlee]]

Attachment: add jar/b2gmar signing support to signtool and signing server

Comment 20

[Aki Sasaki (not active)]

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.

Comment 21

[Chris AtLee [:catlee]]

(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 22

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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.

Comment 23

[Chris AtLee [:catlee]]

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 24

[Rail Aliiev [:rail]]

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)

Comment 25

[Chris AtLee [:catlee]]

Attachment: enable signing for m-c/m-a unagi nightlies

Comment 26

[Chris AtLee [:catlee]]

Attachment: Do signing if MOZ_SIGNING_SERVERS is set

Comment 27

[Aki Sasaki (not active)]

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.

Comment 28

[Chris AtLee [:catlee]]

Attachment: Do signing if MOZ_SIGNING_SERVERS is set

Comment 29

[Chris AtLee [:catlee]]

(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

Comment 30

[Chris AtLee [:catlee]]

Attachment: add jar/b2gmar signing support to signtool and signing server

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.

Comment 31

[Rail Aliiev [:rail]]

Comment on attachment 681060 [details] [diff] [review]
add jar/b2gmar signing support to signtool and signing server

lgtm

Comment 32

[Aki Sasaki (not active)]

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.

Comment 33

[Chris AtLee [:catlee]]

Attachment: Use SigningScriptFactory if signing servers are present

we also set MOZ_SIGNING_SERVERS in the environment to make it easier for mozharness scripts to do signing.

Comment 34

[Chris AtLee [:catlee]]

Attachment: puppet manifests for signing server changes

Comment 35

[[e-mail] (:bhearsum)]

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.

Comment 36

[Chris AtLee [:catlee]]

Attachment: puppet manifests for signing server changes

Comment 37

[Chris AtLee [:catlee]]

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.

Comment 38

[Chris AtLee [:catlee]]

mac-signing{2..4} are now done.

Comment 39

[Aki Sasaki (not active)]

In production.

Comment 40

[Chris AtLee [:catlee]]

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.

Comment 41

[Brian R. Bondy [:bbondy]]

> 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?

Comment 42

[[e-mail] (:bhearsum)]

(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.