Last Comment Bug 509158 - Sign builds as part of the build process
: Sign builds as part of the build process
Status: RESOLVED FIXED
[signing][oldbugs]
:
Product: Release Engineering
Classification: Other
Component: Other (show other bugs)
: other
: x86 Windows Server 2003
: P2 normal (vote)
: ---
Assigned To: Chris AtLee [:catlee]
:
Mentors:
: 638896 663055 (view as bug list)
Depends on: 638896 692549 694300 700347 706660 706832 716431
Blocks: 400296 481815 558460 607389 615311 627271 663055 701087 708656
  Show dependency treegraph
 
Reported: 2009-08-07 18:33 PDT by Chris AtLee [:catlee]
Modified: 2013-08-12 21:54 PDT (History)
24 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Hooks for signing script in build process (5.15 KB, patch)
2011-10-05 07:57 PDT, Chris AtLee [:catlee]
no flags Details | Diff | Splinter Review
Hooks for signing script in build process (10.25 KB, patch)
2011-10-13 09:02 PDT, Chris AtLee [:catlee]
ted: feedback+
Details | Diff | Splinter Review
set signing formats, servers in configs (6.19 KB, patch)
2011-11-07 09:11 PST, Ben Hearsum (:bhearsum)
no flags Details | Diff | Splinter Review
factory/misc.py changes (3.37 KB, patch)
2011-11-07 09:12 PST, Ben Hearsum (:bhearsum)
no flags Details | Diff | Splinter Review
Sign MARs (1.74 KB, patch)
2011-11-08 07:54 PST, Rail Aliiev [:rail]
no flags Details | Diff | Splinter Review
support l10n signing, some bugfixes (12.81 KB, patch)
2011-11-09 07:54 PST, Ben Hearsum (:bhearsum)
no flags Details | Diff | Splinter Review
change signing format variable format (6.19 KB, patch)
2011-11-09 07:56 PST, Ben Hearsum (:bhearsum)
no flags Details | Diff | Splinter Review
Sign MARs (1.05 KB, patch)
2011-11-09 11:05 PST, Rail Aliiev [:rail]
no flags Details | Diff | Splinter Review
Hooks for signing script in build process (27.60 KB, patch)
2011-11-29 07:57 PST, Chris AtLee [:catlee]
no flags Details | Diff | Splinter Review
use signing servers in build factories (17.30 KB, patch)
2011-12-02 10:39 PST, Chris AtLee [:catlee]
bhearsum: review+
catlee: checked‑in+
Details | Diff | Splinter Review
buildbot configs to enable signing servers (16.00 KB, patch)
2011-12-02 11:28 PST, Ben Hearsum (:bhearsum)
no flags Details | Diff | Splinter Review
enable signing server for debug builds; disable pre-signed updater on elm (17.96 KB, patch)
2011-12-02 11:39 PST, Ben Hearsum (:bhearsum)
rail: review+
catlee: review+
bhearsum: checked‑in+
Details | Diff | Splinter Review
signing tools! (351.74 KB, patch)
2011-12-02 11:54 PST, Chris AtLee [:catlee]
bhearsum: review+
rail: review+
catlee: checked‑in+
Details | Diff | Splinter Review
look for signing servers at platform level instead of branch (3.33 KB, patch)
2011-12-02 11:57 PST, Ben Hearsum (:bhearsum)
catlee: review+
bhearsum: checked‑in+
Details | Diff | Splinter Review
turn on only on elm for now (3.73 KB, patch)
2011-12-02 22:17 PST, Chris AtLee [:catlee]
bear: review+
catlee: checked‑in+
Details | Diff | Splinter Review
puppet support for secrets in passwords.py (873 bytes, patch)
2011-12-02 22:40 PST, Chris AtLee [:catlee]
bear: review+
catlee: checked‑in+
Details | Diff | Splinter Review
disable signing for l10n check commands (3.51 KB, patch)
2011-12-04 10:16 PST, Chris AtLee [:catlee]
rail: review+
catlee: checked‑in+
Details | Diff | Splinter Review
turn on signing on m-c (2.03 KB, patch)
2011-12-05 22:16 PST, Chris AtLee [:catlee]
catlee: review+
catlee: checked‑in+
Details | Diff | Splinter Review
do sha1sum calculations out of process (3.89 KB, patch)
2011-12-06 09:59 PST, Chris AtLee [:catlee]
bear: review+
catlee: checked‑in+
Details | Diff | Splinter Review
enable signing on inbound (871 bytes, patch)
2011-12-07 12:04 PST, Chris AtLee [:catlee]
rail: review+
catlee: checked‑in+
Details | Diff | Splinter Review
reduce default token time to 2 hours (811 bytes, patch)
2011-12-07 12:06 PST, Chris AtLee [:catlee]
rail: review+
catlee: checked‑in+
Details | Diff | Splinter Review
signing server enhancements (241.50 KB, patch)
2011-12-07 13:51 PST, Chris AtLee [:catlee]
bear: review+
bhearsum: review+
catlee: checked‑in+
Details | Diff | Splinter Review
sign ALL the branches! (8.28 KB, patch)
2011-12-17 06:33 PST, Chris AtLee [:catlee]
rail: review+
catlee: checked‑in+
Details | Diff | Splinter Review

Description Chris AtLee [:catlee] 2009-08-07 18:33:07 PDT
One bottleneck in our current signing process is the amount of time it takes to download all of the builds to the signing machine.  We currently download them all from stage before starting any signing.

If we could sign the builds as they're finished, our signing process would add very little extra wall clock time to the release process.

The streaming process could look like:

* Watch for new builds to appear on stage
* Download new builds
* Make sure we wait until we have en-US before proceeding, it's the canonical version for dlls and exes.
** Do internal and installer signing of windows builds
** Do GPG signing for all platforms
** Verify results & push back to stage
* Create MD5SUMS and SHA1SUMS & push back to stage
Comment 1 Chris Cooper [:coop] 2010-02-15 07:52:42 PST
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Comment 2 Ben Hearsum (:bhearsum) 2010-10-26 11:31:05 PDT
Another possible implementation, assuming we have fully automated signing (bug 558464), is to have any builders which produce something that gets signed call out to some process (through some build in buildbot process, a web service, or whatever) to trigger signing of their things.
Comment 4 John Ford [:jhford] CET/CEST Berlin Time 2010-12-06 12:48:40 PST
Looks like bug 558464 has an initial implementation which would resolve this bug.  Marking this bug as a duplicate.

*** This bug has been marked as a duplicate of bug 558464 ***
Comment 5 Ben Hearsum (:bhearsum) 2011-01-11 08:08:09 PST
I don't see how this is a dupe of 607389. That bug is likely to be one thing that happens as part of signing on demand, but this encompasses a lot more than just that.
Comment 6 Chris Cooper [:coop] 2011-01-11 08:50:45 PST
Now we have an old bug with no owner.
Comment 7 Chris Cooper [:coop] 2011-01-20 13:01:15 PST
Reassigning to previous owner.
Comment 8 John Ford [:jhford] CET/CEST Berlin Time 2011-02-22 10:15:17 PST
(In reply to comment #0)
> * Watch for new builds to appear on stage
> * Download new builds
> * Make sure we wait until we have en-US before proceeding, it's the canonical
> version for dlls and exes.
> ** Do internal and installer signing of windows builds
> ** Do GPG signing for all platforms
> ** Verify results & push back to stage
> * Create MD5SUMS and SHA1SUMS & push back to stage

(In reply to comment #5)
> I don't see how this is a dupe of 607389. That bug is likely to be one thing
> that happens as part of signing on demand, but this encompasses a lot more than
> just that.

What is not satisfied by the current signing scripts?  From my experience on beta10 and beta11, it seemed like all of the requirements of comment 0 are done as part of the current signing process.
Comment 9 Ben Hearsum (:bhearsum) 2011-02-22 10:17:44 PST
The current signing scripts download builds as they become available, but they do not sign them as they become available.
Comment 10 Chris AtLee [:catlee] 2011-03-03 14:39:50 PST
The current plan we've been discussing is to create a "signing server" that runs on keymaster.  This signing server will listen for properly formatted and secured HTTP requests of files to be signed.

Once we have this, we can modify the build process so that signing is done as part of the build by having the build machine submit the files to be packaged to the signing server and pulling down the signed bits once they're ready.  Updates and snippets can also be generated immediately for builds and repacks.
Comment 11 Chris AtLee [:catlee] 2011-04-28 09:20:08 PDT
*** Bug 638896 has been marked as a duplicate of this bug. ***
Comment 12 Chris Cooper [:coop] 2011-09-06 13:53:38 PDT
cc-ing kev. He's very interested in this for BYOB where they have many GBs of builds that need to make it through signing in one pass currently.
Comment 13 Chris AtLee [:catlee] 2011-10-05 07:57:35 PDT
Created attachment 564855 [details] [diff] [review]
Hooks for signing script in build process

The release automation would set a couple of variables so that signing happens during 'make package':

MOZ_SIGN_CMD - command to run to do the actual signing

MOZ_EXTERNAL_SIGNING_FORMAT - for linux,osx this will be 'gpg' so we can generate the detached gpg signatures. for win32 this will be 'signcode,gpg' so we sign the installers and then the detached gpg signature

MOZ_INTERNAL_SIGNING_FORMAT - for linux,osx this will be unset. for win32 this will be 'signcode' so that all the .exe's and .dll's are signed.
Comment 14 Chris AtLee [:catlee] 2011-10-06 12:11:14 PDT
I believe that we can make use of the mono tools to do signing under linux. This requires a little tweaking to the certificates to work:

openssl x509 -in MozAuthenticode.spc -inform DER -text > MozAuthenticode.crt
makecert MozAuthenticode.crt MozAuthenticode.spc

Then signcode works as expected, and accepts a password for the private key on stdin.
Comment 15 Chris AtLee [:catlee] 2011-10-13 09:02:28 PDT
Created attachment 566851 [details] [diff] [review]
Hooks for signing script in build process
Comment 16 Ted Mielczarek [:ted.mielczarek] 2011-10-18 10:12:09 PDT
Comment on attachment 566851 [details] [diff] [review]
Hooks for signing script in build process

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

::: toolkit/mozapps/installer/signing.mk
@@ +39,5 @@
> +_PKG = $(_ABS_DIST)/$(PACKAGE)
> +endif
> +
> +ifeq (WINNT,$(OS_ARCH))
> +MOZ_SIGN_PREPARED_PACKAGE_CMD=$(MOZ_SIGN_CMD) -f $(MOZ_INTERNAL_SIGNING_FORMAT) -i '*.dll' -i '*.exe' -x 'D3DCompiler*.dll' -x 'd3dx9*.dll' $(_PKGDIR)

I'd like to see these include/exclude lists factored out into separate variables. Something like:
SIGN_INCLUDES := \
  '*.dll' \
  '*.exe' \
  $(NULL)

then you can use it like: $foreach(i, $(SIGN_INCLUDES),-i $(i))

It looks like if you did that, you might be able to make MOZ_SIGN_PREPARED_PACKAGE_CMD the same on every platform, and only have the includes/excludes ifdefed per-platform.
Comment 17 Ben Hearsum (:bhearsum) 2011-11-07 09:11:55 PST
Created attachment 572491 [details] [diff] [review]
set signing formats, servers in configs

We'll probably need to move signing_servers to {production,preproduction,staging}_config.py, but this is basically what we need.
Comment 18 Ben Hearsum (:bhearsum) 2011-11-07 09:12:26 PST
Created attachment 572492 [details] [diff] [review]
factory/misc.py changes
Comment 19 Ben Hearsum (:bhearsum) 2011-11-07 13:24:33 PST
I tested catlee's latest signing-server tools branch with my buildbotcustom and buildbot-configs patches. Here's the results:
* Linux: no problems
* Mac: main "make package" worked fine, "make package" with prettynames failed because of missing quotes somewhere
* Windows: non-pretty "make package" didn't work at all until I installed the 'ssl' module (see bug 700347). I also had to make some changes to signing.py for Python 2.5 specific things:
$ hg diff
diff --git a/release/signing/signing.py b/release/signing/signing.py
--- a/release/signing/signing.py
+++ b/release/signing/signing.py
@@ -154,17 +154,17 @@ def copyfile(src, dst, copymode=True):
     shutil.copyfile(src, dst)
     if copymode:
         shutil.copymode(src, dst)
         shutil.copystat(src, dst)

 def sha1sum(f):
     """Return the SHA-1 hash of the contents of file `f`, in hex format"""
     h = hashlib.sha1()
-    fp = open(f)
+    fp = open(f, 'rb')
     while True:
         block = fp.read(512*1024)
         if not block:
             break
         h.update(block)
     return h.hexdigest()

 def sums_are_equal(base_package, packages):
@@ -256,34 +256,34 @@ def packexe(exefile, srcdir):
     os.unlink(appbundle)

 def bunzip2(filename):
     """Uncompress `filename` in place"""
     log.debug("Uncompressing %s", filename)
     tmpfile = "%s.tmp" % filename
     os.rename(filename, tmpfile)
     b = bz2.BZ2File(tmpfile)
-    f = open(filename, "w")
+    f = open(filename, "wb")
     while True:
         block = b.read(512*1024)
         if not block:
             break
         f.write(block)
     f.close()
     b.close()
     shutil.copystat(tmpfile, filename)
     shutil.copymode(tmpfile, filename)
     os.unlink(tmpfile)

 def bzip2(filename):
     """Compress `filename` in place"""
     log.debug("Compressing %s", filename)
     tmpfile = "%s.tmp" % filename
     os.rename(filename, tmpfile)
-    b = bz2.BZ2File(filename, "w")
+    b = bz2.BZ2File(filename, "wb")
     f = open(tmpfile)
     while True:
         block = f.read(512*1024)
         if not block:
             break
         b.write(block)
     f.close()
     b.close()
@@ -553,21 +553,18 @@ def buildValidatingOpener(ca_certs):
     except ImportError:
         from httplib import HTTPSConnection
         from urllib2 import HTTPSHandler

     class VerifiedHTTPSConnection(HTTPSConnection):
         def connect(self):
             # overrides the version in httplib so that we do
             #    certificate verification
-            sock = socket.create_connection((self.host, self.port),
-                                            self.timeout)
-            if self._tunnel_host:
-                self.sock = sock
-                self._tunnel()
+            sock = socket.socket()
+            sock.connect((self.host, self.port))

             # wrap the socket using verification with the root
             #    certs in trusted_root_certs
             self.sock = ssl.wrap_socket(sock,
                                         self.key_file,
                                         self.cert_file,
                                         cert_reqs=ssl.CERT_REQUIRED,
                                         ca_certs=ca_certs,


After that, I got a new traceback:
WARNING: Found 311 duplicate files taking 968406 bytes
Stripped 317565 bytes
Deoptimized 0/2047 in ./omni.jar
2011-11-07 13:17:09,092 - ea2613b229ba571130d66d9375331a62fd03c2d7: processing e
:/builds/moz2_slave/m-cen-w32/build/obj-firefox/browser/installer/../../dist/fir
efox\AccessibleMarshal.dll on https://dev-master01.build.scl1.mozilla.com:8080
Traceback (most recent call last):
  File "e:/builds/moz2_slave/m-cen-w32/tools/release/signing/signtool.py", line
127, in <module>
    main()
  File "e:/builds/moz2_slave/m-cen-w32/tools/release/signing/signtool.py", line
111, in main
    errors.extend(remote_signdir(options, url, f, fmt))
  File "e:\builds\moz2_slave\m-cen-w32\tools\release\signing\signing.py", line 5
32, in remote_signdir
    if not remote_signfile(options, url, fullpath, fmt, dest=dest):
  File "e:\builds\moz2_slave\m-cen-w32\tools\release\signing\signing.py", line 4
80, in remote_signfile
    os.rename(tmpfile, dest)
WindowsError: [Error 17] Cannot create a file when that file already exists
make[2]: *** [make-package] Error 1
make[2]: Leaving directory `/e/builds/moz2_slave/m-cen-w32/build/obj-firefox/bro
wser/installer'
make[1]: *** [all] Error 2
make[1]: Leaving directory `/e/builds/moz2_slave/m-cen-w32/build/obj-firefox/bro
wser/installer'
make: *** [package] Error 2
Comment 20 Rail Aliiev [:rail] 2011-11-08 07:54:51 PST
Created attachment 572813 [details] [diff] [review]
Sign MARs

Not tested yet. Should work for external (GPG) singing of MAR files.
Comment 21 Ben Hearsum (:bhearsum) 2011-11-09 07:54:48 PST
Created attachment 573196 [details] [diff] [review]
support l10n signing, some bugfixes

Changes in this patch:
* Inherit en-US env for l10n builds
* Port MercurialBuildFactory signing server work to BaseRepackFactory
* Set win32_basedir, because we need it for PYTHONPATH to work on Windows
Comment 22 Ben Hearsum (:bhearsum) 2011-11-09 07:56:36 PST
Created attachment 573199 [details] [diff] [review]
change signing format variable format

This patch changes the separator in the signing format variables to a space, to make it easy to filter out things in the build system.
Comment 23 Rail Aliiev [:rail] 2011-11-09 11:05:11 PST
Created attachment 573255 [details] [diff] [review]
Sign MARs

Using MOZ_SIGN_PACKAGE_CMD
Comment 24 Ben Hearsum (:bhearsum) 2011-11-11 06:19:48 PST
Comment on attachment 573199 [details] [diff] [review]
change signing format variable format

I have an updated buildbot-configs patch on my github that supports different signing servers for dep/nightly builds, and different signing servers for production/preproduction/config: https://github.com/bhearsum/buildbot-configs/compare/master...signing-server
Comment 25 Ben Hearsum (:bhearsum) 2011-11-11 06:31:12 PST
Comment on attachment 573196 [details] [diff] [review]
support l10n signing, some bugfixes

And an updated buildbotcustom patch here: https://github.com/bhearsum/buildbotcustom/compare/master...signing-server
Comment 26 Chris AtLee [:catlee] 2011-11-29 07:57:29 PST
Created attachment 577613 [details] [diff] [review]
Hooks for signing script in build process

planning on landing this on elm first
Comment 27 Chris AtLee [:catlee] 2011-12-02 10:39:23 PST
Created attachment 578633 [details] [diff] [review]
use signing servers in build factories
Comment 28 Ben Hearsum (:bhearsum) 2011-12-02 11:28:43 PST
Created attachment 578664 [details] [diff] [review]
buildbot configs to enable signing servers
Comment 29 Ben Hearsum (:bhearsum) 2011-12-02 11:39:39 PST
Created attachment 578670 [details] [diff] [review]
enable signing server for debug builds; disable pre-signed updater on elm
Comment 30 Chris AtLee [:catlee] 2011-12-02 11:54:24 PST
Created attachment 578675 [details] [diff] [review]
signing tools!
Comment 31 Chris AtLee [:catlee] 2011-12-02 11:56:09 PST
Comment on attachment 578675 [details] [diff] [review]
signing tools!

https://github.com/catlee/tools/compare/master...signing-server
Comment 32 Ben Hearsum (:bhearsum) 2011-12-02 11:57:21 PST
Created attachment 578676 [details] [diff] [review]
look for signing servers at platform level instead of branch

This applies on top of catlee's buildbotcustom patch.
Comment 33 Rail Aliiev [:rail] 2011-12-02 12:12:07 PST
Comment on attachment 578670 [details] [diff] [review]
enable signing server for debug builds; disable pre-signed updater on elm

Almost there!
Comment 34 Rail Aliiev [:rail] 2011-12-02 12:43:25 PST
Comment on attachment 578675 [details] [diff] [review]
signing tools!

YAY!
Comment 35 Brian R. Bondy [:bbondy] 2011-12-02 12:48:57 PST
awesome!
Comment 36 Chris AtLee [:catlee] 2011-12-02 13:16:48 PST
(In reply to Chris AtLee [:catlee] from comment #14)
> I believe that we can make use of the mono tools to do signing under linux.
> This requires a little tweaking to the certificates to work:
> 
> openssl x509 -in MozAuthenticode.spc -inform DER -text > MozAuthenticode.crt
> makecert MozAuthenticode.crt MozAuthenticode.spc

sorry, this should be 'cert2spc MozAuthenticode.crt MozAuthenticode.spc'
Comment 37 Chris AtLee [:catlee] 2011-12-02 22:17:32 PST
Created attachment 578810 [details] [diff] [review]
turn on only on elm for now

let's limit possible damage to elm/oak for now
Comment 38 Chris AtLee [:catlee] 2011-12-02 22:40:08 PST
Created attachment 578811 [details] [diff] [review]
puppet support for secrets in passwords.py
Comment 39 Chris AtLee [:catlee] 2011-12-04 10:16:52 PST
Created attachment 578918 [details] [diff] [review]
disable signing for l10n check commands
Comment 40 Chris AtLee [:catlee] 2011-12-05 22:16:13 PST
Created attachment 579232 [details] [diff] [review]
turn on signing on m-c
Comment 41 Chris AtLee [:catlee] 2011-12-06 07:45:21 PST
Comment on attachment 579232 [details] [diff] [review]
turn on signing on m-c

r+ from rail over irc
Comment 42 Chris AtLee [:catlee] 2011-12-06 09:59:58 PST
Created attachment 579350 [details] [diff] [review]
do sha1sum calculations out of process

currently sha1sum calculations are done in the same process and greenlet as the web server. this effectively blocks the server until the sha1sum is complete, which can take a while for larger files.

this patch makes sha1sum() use a pool of workers to do the calculation in separate processes, also allowing the web server to continue serving requests.
Comment 43 Mike Taylor [:bear] 2011-12-06 12:00:16 PST
Comment on attachment 579350 [details] [diff] [review]
do sha1sum calculations out of process

this is a lot cleaner than I've had to do in the past with my own code - looks good
Comment 44 Chris AtLee [:catlee] 2011-12-07 12:04:50 PST
Created attachment 579781 [details] [diff] [review]
enable signing on inbound
Comment 45 Chris AtLee [:catlee] 2011-12-07 12:06:48 PST
Created attachment 579782 [details] [diff] [review]
reduce default token time to 2 hours
Comment 46 Chris AtLee [:catlee] 2011-12-07 13:51:18 PST
Created attachment 579828 [details] [diff] [review]
signing server enhancements

this patch adds a few nice-to-have things:

* -d option to daemonize the process (using python-daemon library, in vendor directory) 
* --stop/--restart/--reload commands. for restart we ask for the passphrases before stopping the old server so that downtime is minimal
* backdoor support
Comment 47 Mike Taylor [:bear] 2011-12-07 14:09:59 PST
Comment on attachment 579828 [details] [diff] [review]
signing server enhancements


1. I think you need to get the log object from multiprocessing.get_logger() if your calling log from inside a worker

2. wouldn't it be better if for the reload that you check for a valid pid and only proceed if one is found
Comment 48 Chris Cooper [:coop] 2011-12-14 09:32:38 PST
I reconfig-ed with the landed changes from this bug today.
Comment 49 Chris AtLee [:catlee] 2011-12-17 06:33:59 PST
Created attachment 582526 [details] [diff] [review]
sign ALL the branches!

This enables dep signing for all win32 builds.

m-c, m-a, m-i, elm and oak nightlies get to use nightly signing keys.
Comment 50 Chris AtLee [:catlee] 2011-12-19 09:29:03 PST
Calling this done.
Comment 51 Ben Hearsum (:bhearsum) 2012-03-05 17:03:09 PST
(In reply to Chris AtLee [:catlee] from comment #36)
> (In reply to Chris AtLee [:catlee] from comment #14)
> > I believe that we can make use of the mono tools to do signing under linux.
> > This requires a little tweaking to the certificates to work:
> > 
> > openssl x509 -in MozAuthenticode.spc -inform DER -text > MozAuthenticode.crt
> > makecert MozAuthenticode.crt MozAuthenticode.spc
> 
> sorry, this should be 'cert2spc MozAuthenticode.crt MozAuthenticode.spc'

and make the first one: openssl x509 -in MozAuthenticode.spc -inform DER -outform PEM > MozAuthenticode.crt
Comment 52 Steffen Wilberg 2012-03-17 15:48:02 PDT
*** Bug 663055 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.