Closed
Bug 400296
Opened 17 years ago
Closed 13 years ago
Have release automation support signing OSX builds
Categories
(Release Engineering :: Release Automation, enhancement, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: u49640, Assigned: edransch)
References
()
Details
(Keywords: sec-want, Whiteboard: [hg-automation][sg:want][signing][oldbug])
Attachments
(4 files, 22 obsolete files)
10.26 KB,
patch
|
bhearsum
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
567 bytes,
text/plain
|
Details | |
15.07 KB,
patch
|
bhearsum
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
60.00 KB,
application/x-tar
|
Details |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.7) Gecko/20070914 Firefox/2.0.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.7) Gecko/20070914 Firefox/2.0.0.7
According to the link, Mac OS X understands signed applications. Since the Windows firefox.exe is already signed, it would make sense to sing firefox.app too (and thunderbird.app, but I'm to lazy to file another bug)
From the URL:
"Signed Applications
Feel safe with your applications. A digital signature on an application verifies its identity and ensures its integrity. All applications shipped with Leopard are signed by Apple, and third-party software developers can also sign their applications."
Reproducible: Always
Steps to Reproduce:
1.
2.
3.
Updated•17 years ago
|
Assignee: nobody → build
Component: OS Integration → Build & Release
Priority: -- → P3
Product: Firefox → mozilla.org
QA Contact: os.integration → mozpreed
Version: unspecified → other
Comment 1•17 years ago
|
||
Let me know if a separate bugs is required for Tbird.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•17 years ago
|
Assignee: build → nobody
QA Contact: mozpreed → build
Comment 2•17 years ago
|
||
Some documentation from Apple:
http://developer.apple.com/documentation/Security/Conceptual/CodeSigningGuide/index.html#//apple_ref/doc/uid/TP40005929
Updated•17 years ago
|
Summary: Sign the Mac-Binary for OS X10.5 → Sign the Mac-Binary for OSX 10.5
Comment 3•17 years ago
|
||
You need to be on 10.5 to do this in the supported way, and all of the release tinderbuilders are 10.4.
I don't know if there's any unsupported way to do this on 10.4, I haven't looked closely.
Comment 4•17 years ago
|
||
This would be a nice polish bug to get for final. I can't raise the visibility since the flags don't pertain to this component.
Comment 5•17 years ago
|
||
Do you want this to be done automatically from within tinderbox, or do you want for there to be a separate procedure to do it? From what I understand, signing the Windows builds is done separately, outside of the ordinary tinderbox build.
Comment 6•17 years ago
|
||
This could be supported using something similar to what I crafted in bug 409459. The logistical (on-tinderbox-or-elsewhere) and political (get-the-certificate) questions still need to be answered.
Comment 7•17 years ago
|
||
Mark: I guess the answer to comment 5 would be up the build team. If there were an easy way to do it so we could get the signed bits without waiting for a separate build, I am all for it. Thanks for helping out with this.
Comment 8•17 years ago
|
||
If you wanted to do this on a tinderbox, the tinderbox would need to be running Leopard. Currently, there are no Leopard tinderboxes anywhere in all of Mozilla-land, except for the one we set up for Camino on the community build network.
Given a Leopard tinderbox, someone would still need to make a policy decision about whether you want to sign all builds always (meaning that the tinderbox doing the signed builds would need the key installed on it) or only releases that have been through some QA (meaning that the key could be guarded more closely). I'm not sure if there are existing policy reasons for signing the Windows builds the way you do them, so barking up that tree might be a good first step. If nobody's opposed to doing them automated, then we can start thinking about tinderboxen or other things, and we can get the script polished up for use by other apps than Camino.
We'll also want to talk about certificates as they relate to Camino, so if you're interested, drop on by bug 409459.
Comment 9•17 years ago
|
||
(In reply to comment #3)
> You need to be on 10.5 to do this in the supported way, and all of the release
> tinderbuilders are 10.4.
> I don't know if there's any unsupported way to do this on 10.4, I haven't
> looked closely.
For the FF3.0 releases, we're still on 10.4, so looks like earliest we can consider doing this is in moz2. Reassigning to "future".
Component: Release Engineering → Release Engineering: Future
QA Contact: build → release
Updated•17 years ago
|
Whiteboard: [hg-automation]
Comment 10•16 years ago
|
||
Now there are 10.5 tinderbuilders, for Firefox 3.1 and Firefox 3.2. Are there any planes now to sign the Mac-Binary for OSX 10.5?
Comment 11•16 years ago
|
||
Not at this time. I can't see this happening until at least the summer, and probably not until later than that.
Comment 12•16 years ago
|
||
I don't think this should be done on the tinderbuilders, the insecurity of that setup would defeat the purpose. Or we could sign those with a cheap cert for nightlies, but I don't see that buying us much. The releases need to be signed on a restricted build signing machine with controlled access to the key.
Whiteboard: [hg-automation] → [hg-automation][sg:want]
Updated•16 years ago
|
Comment 13•16 years ago
|
||
We need to raise the priority of this, I think. There is currently no way for a user to know if they got an intact Mac Firefox from us, since we deliver over HTTP and there's no signature. (Like, security experts at Black Hat couldn't tell and were nervous about it, and if *they* can't tell...)
We don't want to do it on the tinderbuilders, we should do it like we do it for Windows (in which case we don't have to upgrade the tinderboxes either, if that were still needed, since we could just sign on a 10.5 machine I think).
Can we get an updated ETA here, or an idea of how someone else could get it working?
Comment 14•16 years ago
|
||
(In reply to comment #13)
> We need to raise the priority of this, I think. There is currently no way for
> a user to know if they got an intact Mac Firefox from us, since we deliver over
> HTTP and there's no signature. (Like, security experts at Black Hat couldn't
> tell and were nervous about it, and if *they* can't tell...)
We do GPG sign all our installers, and the detached signatures are available on our ftp site. I know, terrible user experience, but it's _possible_ to verify that you downloaded an intact build.
Comment 15•16 years ago
|
||
From the apple docs it looks like we may be able to re-use our existing signing key, which would be nice.
I'd want a separate signing machine to do this on, like we have for windows.
Comment 16•15 years ago
|
||
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
Priority: P3 → P5
Updated•15 years ago
|
Whiteboard: [hg-automation][sg:want] → [hg-automation][sg:want][signing]
Comment 17•15 years ago
|
||
As long as the Mac binaries are not code signed, addons like mozilla-keychain cannot work 100% satisfactorily.
Because the Firefox binary is not code signed, access to each password of Keychain has to be confirmed everytime a new version of Firefox is installed. The same applies to Thunderbird (I found no bug entry) and (I guess) Camino (Bug 400296).
I already voted for the bug but wanted to point out that it would be a real gain for Mac users.
Comment 18•15 years ago
|
||
We should be able to use the codesign tool that Apple provides to use any normal signing certificate (which would be imported into the Keystore) to sign the binary.
This would need to be verified (i'm working from older notes and memory) and it also requires an OS X signing computer. Another glitch that I'm remembering is that the codesign binary seemed to require being run from a windowed session, i.e. you couldn't run it from a headless ssh session.
Comment 19•15 years ago
|
||
Morphing this bug to be a generic OS X signing issue as the solution is the same for 10.5 and 10.6
Status: NEW → ASSIGNED
Hardware: PowerPC → All
Summary: Sign the Mac-Binary for OSX 10.5 → Sign the Mac-Binary for OS X
Updated•15 years ago
|
Comment 20•15 years ago
|
||
I doubt it requires a windowed session, as Apple's own RelEng signing system is
headless. Perhaps they use a private framework though.
I am more concerned about breaking the signature with updates (manual and
auto), changing files, localization, etc. Once we sign it the assumption is a
broken signature is "bad". It would also be arguably worse to have something
like mozilla-keychain work well and then after an update have it start to ask
for permission to access the keychain.
Some considerations:
* We'd have to ignore any dynamically changing files (blocklists and such, if
stored in the app bundle) in the code signature. This is fairly trivial to do
once we identify the files
* We can have "optionally" signed files. That is, if the file isn't signed it
is a failure, but if it is signed it needs to match the signature provided. Not
sure which files would fall in this category for Firefox, perhaps
localizations?
* If files removed between versions will be left over in the app bundle, it
will cause codesign -v to fail with "seal broken / extra files" messages and
the prompts will start
Comment 21•15 years ago
|
||
(In reply to comment #20)
> I doubt it requires a windowed session, as Apple's own RelEng signing system is
> headless. Perhaps they use a private framework though.
>
> I am more concerned about breaking the signature with updates (manual and
> auto), changing files, localization, etc. Once we sign it the assumption is a
> broken signature is "bad". It would also be arguably worse to have something
> like mozilla-keychain work well and then after an update have it start to ask
> for permission to access the keychain.
>
> Some considerations:
> * We'd have to ignore any dynamically changing files (blocklists and such, if
> stored in the app bundle) in the code signature. This is fairly trivial to do
> once we identify the files
> * We can have "optionally" signed files. That is, if the file isn't signed it
> is a failure, but if it is signed it needs to match the signature provided. Not
> sure which files would fall in this category for Firefox, perhaps
> localizations?
> * If files removed between versions will be left over in the app bundle, it
> will cause codesign -v to fail with "seal broken / extra files" messages and
> the prompts will start
How is any of this different than on windows?
Comment 22•15 years ago
|
||
Not sure if it is any different. I'm just saying these were considerations when shipping updates to signed bundles on Mac OS X previously and we should at least be aware.
Comment 23•15 years ago
|
||
(In reply to comment #20)
> I doubt it requires a windowed session, as Apple's own RelEng signing system is
> headless. Perhaps they use a private framework though.
It was something I remembered from setting up Seesmic's build/release scripts and wanted to mark it for my research :)
Comment 24•15 years ago
|
||
(In reply to comment #23)
> (In reply to comment #20)
> > I doubt it requires a windowed session, as Apple's own RelEng signing system is
> > headless. Perhaps they use a private framework though.
>
> It was something I remembered from setting up Seesmic's build/release scripts
> and wanted to mark it for my research :)
For completeness - here is how Apple probably solved it:
http://lists.apple.com/archives/apple-cdsa/2008/Jan/msg00027.html
Comment 25•14 years ago
|
||
rough patch to add support for mac signing, tested on a mac slave with a manually unlocked keychain and self-signed certificate set up for testing. A few things here are obviously going to need work/testing (hardcoding of app bundle paths and single-file mac signing for updates are the main ones that come to mind).
Assignee: bear → salbiz
Attachment #488135 -
Flags: feedback?(catlee)
Attachment #488135 -
Flags: feedback?(bhearsum)
Comment 26•14 years ago
|
||
Comment on attachment 488135 [details] [diff] [review]
integrate mac signing into signing scripts
A great start!
>diff --git a/release/signing/sign-release.py b/release/signing/sign-release.py
>--- a/release/signing/sign-release.py
>+++ b/release/signing/sign-release.py
>@@ -1,28 +1,68 @@
> #!/usr/bin/python
> import tempfile, os, shutil, time, bz2, sys, re
> import logging
> from subprocess import *
>
> from signing import *
>
> log = logging.getLogger()
>+def signbundle(dirname, keydir, fake=False):
>+ """Sign a packaged app bundle with the keychain in keydir"""
>+ if fake:
>+ time.sleep(1)
>+ return
>+ #get top level app bundles in the package dir
>+ bundles = os.walk(dirname).next()
>+ for bundle in bundles[1]:
This is a little hard to understand. bundles = [d for d in os.listdir(dirname) if os.path.isdir(dirname)] might be better?
>-def signfile(filename, keydir, fake=False):
>+def signmacfile(filename, keydir, platform):
>+ basename = os.path.basename(filename)
>+ dirname = os.path.dirname(filename)
>+ stdout = tempfile.TemporaryFile()
>+ command = ['codesign',
>+ '-s', 'cltsign',
>+ '--keychain', '%s/cltsign.keychain' % keydir,
>+ '-i', 'org.mozilla.firefox',
>+ basename]
>+ try:
>+ check_call(command, cwd=dirname, stdout=stdout, stderr=STDOUT)
>+ except:
>+ stdout.seek(0)
>+ data = stdout.read()
>+ log.exception(data)
>+ raise
>+
>+def signfile(filename, keydir, platform='win32', fake=False):
> """Sign the given file with keys in keydir.
>
> If fake is True, then don't actually sign anything, just sleep for a
> second to simulate signing time."""
> if fake:
> time.sleep(1)
> return
>+ if platform == 'mac':
>+ return signmacfile(filename, keydir, platform)
Can you split off the rest into a 'signwinfile' function?
>@@ -152,69 +192,72 @@ class Signer:
> nFiles = 0
> cacheHits = 0
> nSigned = 0
> tmpdir = tempfile.mkdtemp()
> try:
> # Unpack it
> logs.append("Unpacking %s to %s" % (pkgfile, tmpdir))
> unpackfile(pkgfile, tmpdir)
>- # Swap in files we have already signed
>- for f in findfiles(tmpdir):
>- # We don't need to do anything to files we're not going to sign
>- if not shouldSign(f):
>- continue
>+ if pkgfile.endswith('.dmg'):
>+ signbundle(tmpdir, self.keydir, self.fake)
Putting a 'continue' or 'return' here would save having to have this giant 'else' clause below.
>+ else:
>+ # Swap in files we have already signed
>+ for f in findfiles(tmpdir):
>+ # We don't need to do anything to files we're not going to sign
>+ if not shouldSign(f):
>+ continue
>
>- h = sha1sum(f)
>- basename = os.path.basename(f)
>- nFiles += 1
>+ h = sha1sum(f)
>+ basename = os.path.basename(f)
>+ nFiles += 1
>
>- # Look in the cache for another file with the same original hash
>- cachedFile = self.getFile(h, f)
>- if cachedFile:
>- cacheHits += 1
>- assert os.path.basename(cachedFile) == basename
>- logs.append("Copying %s from %s" % (basename, cachedFile))
>- # Preserve the original file's mode; don't use the cached mode
>- # We usually process installer .exe's first, and 7z doesn't
>- # preserve the file mode, so the cached copies of the files
>- # are mode 0666. In the mar files, executables have mode
>- # 0777, so we want to preserve that.
>- copyfile(cachedFile, f, copymode=False)
>- else:
>- # We need to sign this file
>- # If this file is compressed, check if we have a cached copy that
>- # is uncompressed
>- if compressed:
>- bunzip2(f)
>- h2 = sha1sum(f)
>- cachedFile = self.getFile(h2, f)
>- if cachedFile:
>- # We have a cached copy of this file that is uncompressed.
>- # So copy it into our dstdir, and recompress it, and
>- # save it for future use.
>- cacheHits += 1
>- assert os.path.basename(cachedFile) == basename
>- logs.append("Copying %s from uncompressed %s" % (basename, cachedFile))
>- # See note above about not copying the file's mode
>- copyfile(cachedFile, f, copymode=False)
>+ # Look in the cache for another file with the same original hash
>+ cachedFile = self.getFile(h, f)
>+ if cachedFile:
>+ cacheHits += 1
>+ assert os.path.basename(cachedFile) == basename
>+ logs.append("Copying %s from %s" % (basename, cachedFile))
>+ # Preserve the original file's mode; don't use the cached mode
>+ # We usually process installer .exe's first, and 7z doesn't
>+ # preserve the file mode, so the cached copies of the files
>+ # are mode 0666. In the mar files, executables have mode
>+ # 0777, so we want to preserve that.
>+ copyfile(cachedFile, f, copymode=False)
>+ else:
>+ # We need to sign this file
>+ # If this file is compressed, check if we have a cached copy that
>+ # is uncompressed
>+ if compressed:
>+ bunzip2(f)
>+ h2 = sha1sum(f)
>+ cachedFile = self.getFile(h2, f)
>+ if cachedFile:
>+ # We have a cached copy of this file that is uncompressed.
>+ # So copy it into our dstdir, and recompress it, and
>+ # save it for future use.
>+ cacheHits += 1
>+ assert os.path.basename(cachedFile) == basename
>+ logs.append("Copying %s from uncompressed %s" % (basename, cachedFile))
>+ # See note above about not copying the file's mode
>+ copyfile(cachedFile, f, copymode=False)
>+ bzip2(f)
>+ if remember:
>+ logs.append("Caching compressed %s as %s" % (f, h))
>+ self.rememberFile(h, f)
>+ continue
>+
>+ nSigned += 1
>+ logs.append("Signing %s" % f)
>+ signfile(f, self.keydir, self.fake)
Do you need to pass platform in here?
>+def unpackdmg(dmgfile, destdir):
>+ """Unpack the given dmgfile into destdir, using hdiutil"""
>+ nullfd = open(os.devnull, "w")
>+ dmgfile = os.path.abspath(dmgfile)
>+ unpack_dir = tempfile.mkdtemp()
>+ try:
>+ check_call(['hdiutil', 'attach', dmgfile,
>+ '-readonly', '-mountroot', unpack_dir], stdout=nullfd)
>+ check_call(['rsync', '-av', unpack_dir, destdir], stdout=nullfd)
>+ except:
>+ log.exception("Error unpacking dmg %s to %s", dmgfile, destdir)
>+ raise
>+ nullfd.close()
>+ try:
>+ check_call(['hdiutil', 'detach', os.path.join(unpack_dir, 'Firefox')])
>+ except:
>+ log.exception("Error cleaning up %s", unpack_dir)
>+ raise
Take a look at our installdmg.sh script for some extra error checking you may want to do here. In fact, we may want to use installdmg.sh. The problem is that hdiutil attach can return before the disk is fully attached.
Attachment #488135 -
Flags: feedback?(catlee) → feedback+
Comment 27•14 years ago
|
||
Comment on attachment 488135 [details] [diff] [review]
integrate mac signing into signing scripts
>diff --git a/release/signing/sign-release.py b/release/signing/sign-release.py
>+def signbundle(dirname, keydir, fake=False):
>+ """Sign a packaged app bundle with the keychain in keydir"""
>+ if fake:
>+ time.sleep(1)
>+ return
>+ #get top level app bundles in the package dir
>+ bundles = os.walk(dirname).next()
>+ for bundle in bundles[1]:
>+ stdout = tempfile.TemporaryFile()
>+ command = ['codesign',
>+ '-s', 'cltsign',
Can you define this at the top of a file somewhere? Bonus points if it's overridable.
>+ '--keychain', '%s/cltsign.keychain' % keydir,
Same for the cltsign.keychain portion of this.
>+ bundle]
>+ try:
>+ check_call(command, cwd=dirname, stdout=stdout, stderr=STDOUT)
>+ except:
>+ stdout.seek(0)
>+ data = stdout.read()
>+ log.exception(data)
>+ raise
>
>-def signfile(filename, keydir, fake=False):
>+def signmacfile(filename, keydir, platform):
>+ basename = os.path.basename(filename)
>+ dirname = os.path.dirname(filename)
>+ stdout = tempfile.TemporaryFile()
>+ command = ['codesign',
>+ '-s', 'cltsign',
>+ '--keychain', '%s/cltsign.keychain' % keydir,
>+ '-i', 'org.mozilla.firefox',
This too, hardcoding 'firefox' makes it hard to re-use this work for other projects.
>+def unpackdmg(dmgfile, destdir):
>+ """Unpack the given dmgfile into destdir, using hdiutil"""
>+ nullfd = open(os.devnull, "w")
>+ dmgfile = os.path.abspath(dmgfile)
>+ unpack_dir = tempfile.mkdtemp()
>+ try:
>+ check_call(['hdiutil', 'attach', dmgfile,
>+ '-readonly', '-mountroot', unpack_dir], stdout=nullfd)
>+ check_call(['rsync', '-av', unpack_dir, destdir], stdout=nullfd)
>+ except:
>+ log.exception("Error unpacking dmg %s to %s", dmgfile, destdir)
>+ raise
>+ nullfd.close()
>+ try:
>+ check_call(['hdiutil', 'detach', os.path.join(unpack_dir, 'Firefox')])
>+ except:
>+ log.exception("Error cleaning up %s", unpack_dir)
>+ raise
I agree with Catlee's suggestion of using the existing unpackdmg script.
>+def packdmg(dmgfile, srcdir):
>+ """Recreate the files in srcdir into the given dmgfile"""
>+ command = ['hdiutil',
>+ 'makehybrid',
>+ '-hfs',
>+ '-hfs-volume-name', dmgfile.rstrip(".dmg"),
>+ '-o', dmgfile,
>+ srcdir]
>+ try:
>+ check_call(command)
>+ except:
>+ log.exception("Error packing %s into %s", os.path.abspath(srcdir), os.path.abspath(dmgfile))
>+ raise
This part concerns me a little bit, we do a lot of extra when we package the original DMG. The wrapper script that the build system uses sets the volume name, a background, a disk image icon, inserts a symlink, and possibly other things. Have you compared the unsigned vs. signed to see if that information is retained?
At the very least we'll need to make sure we duplicate that. I'm a bit worried about the invocation in the build system changing, and us missing that, or different invocations between branches. I'm not quite sure how to solve that last part without requiring a full sourceRepo clone. We can live with just duplicating the existing logic for now, I guess.
Attachment #488135 -
Flags: feedback?(bhearsum) → feedback+
Comment 28•14 years ago
|
||
Should take care of most of the issues brought up. I have verified this script on the en-US dmg, but will obviously need to do a full run-through comparing the repacked dmg to the original unsigned dmg.
In addition to changing the dmg unpacking, I refactored how bundles are signed, so that we don't miss sub-bundles and individual binaries that live under MacOS/, which don't get automatically signed by signing the bundle.
Attachment #488135 -
Attachment is obsolete: true
Attachment #488989 -
Flags: feedback?(catlee)
Attachment #488989 -
Flags: feedback?(bhearsum)
Comment 29•14 years ago
|
||
Comment on attachment 488989 [details] [diff] [review]
refine dmg packing/unpacking
>diff --git a/buildfarm/utils/installdmg.sh b/buildfarm/utils/installdmg.sh
> # Now we can copy everything out of the mnt directory into the current directory
>-rsync -av ./mnt/* .
>+rsync -av ./mnt/ .
Any particular reason you're changing this?
>diff --git a/release/signing/Makefile b/release/signing/Makefile
Hmm, you're using the same Makefile to do signing on Mac? That seems a little square-peg/roundhole-ish. You don't need most of the tools here to do Mac signing, so I'd suggest setting up a different set of targets or a different Makefile altogether for Mac signing. Unless/until we get to a point where we sign both on the same machine I think it's only asking for trouble to have them run through the same targets.
This seems good overall to me, but feedback- because of the Makefile stuff.
Attachment #488989 -
Flags: feedback?(bhearsum) → feedback-
Comment 30•14 years ago
|
||
Comment on attachment 488989 [details] [diff] [review]
refine dmg packing/unpacking
(In reply to comment #29)
> Comment on attachment 488989 [details] [diff] [review]
> refine dmg packing/unpacking
>
> >diff --git a/buildfarm/utils/installdmg.sh b/buildfarm/utils/installdmg.sh
>
> > # Now we can copy everything out of the mnt directory into the current directory
> >-rsync -av ./mnt/* .
> >+rsync -av ./mnt/ .
>
> Any particular reason you're changing this?
This line failed when I tested it with File not Found errors due to the way the shell interpreted the asterisk as a special character (or rather, how it didn't). Removing the asterisk still captures the intended functionality (grab everything inside mnt/, without creating mnt itself (which AFAIK is what the slash does), without the special character to mess things up.
Removing feedback until I get the Makefile issues sorted
Attachment #488989 -
Flags: feedback?(catlee)
Comment 31•14 years ago
|
||
be careful:
>-rsync -av ./mnt/* .
this one doesn't copy hidden files
>+rsync -av ./mnt/ .
this one does.
Comment 32•14 years ago
|
||
In addition to the Makefile changes, I've also made a couple of changes to the verification scripts to address a few issues that came up during testing. Right now, the plan to integrate mac signing into regular releases is to have a dedicated mac signing box with the GPG keys and code signing cert in a keychain.
After repacks are done, the windows and mac signing can both be kicked off in parallel, with the windows signing box codesigning the win32 binaries and generating the detached sigs/checksums for both win32/linux/linux64, and mac singing box codesigning the mac/mac64 binaries and generating the detached sigs and checksums for mac/mac64.
In order to integrate this with regular releases, I will also need to add a builder to merge the MD5SUMS/SHA1SUMS files generated by the two signing processes, and modify the FtpPoller to look for both the win32 and mac signing logs before proceeding.
Attachment #488989 -
Attachment is obsolete: true
Attachment #489964 -
Flags: feedback?(catlee)
Attachment #489964 -
Flags: feedback?(bhearsum)
Comment 33•14 years ago
|
||
accidentally included older patch.
Attachment #489964 -
Attachment is obsolete: true
Attachment #489969 -
Flags: feedback?(catlee)
Attachment #489969 -
Flags: feedback?(bhearsum)
Attachment #489964 -
Flags: feedback?(catlee)
Attachment #489964 -
Flags: feedback?(bhearsum)
Comment 34•14 years ago
|
||
Comment on attachment 489969 [details] [diff] [review]
refresh patch
Makefile stuff:
Can you do autodetection of the OS? Testing for /Users is probably sufficient.
There's a few rules where the only difference is how mail is sent. Can you factor that part out and make the rules work on both OS'? Specifically: create-mac-sigs, verify-mac-sigs, and fake-upload-mac.
We're going to need a different place to upload the existing *SUMS files, too, otherwise we'll have to overwrite them later, which is bad form.
Please rename download-exclude.list to download-win32-exclude.list.
Signing script stuff:
Can you add a comment on why XUL is a special case on Mac?
I think you can make _sign_special_case non-Mac specific, too. If it has to Mac-specific, name the variable to reflect that.
If CodeResources are expected artifacts we should test that they exist rather than removing them. Are they created in any predictable manner?
Nit: Put --no-check-certificate before the file you're downloading
feedback+ in general, but the Makefile still needs some reworking.
Attachment #489969 -
Flags: feedback?(bhearsum) → feedback+
Comment 35•14 years ago
|
||
Comment on attachment 489969 [details] [diff] [review]
refresh patch
> # Now we can copy everything out of the mnt directory into the current directory
>-rsync -av ./mnt/* .
>+rsync -av ./mnt/ .
Have you checked that this doesn't break any of our existing uses of installdmg.sh?
>+setup-mac:
>+ cvs -d:ext:cltbld@cvs.mozilla.org:/mofo co -d signing release/signing/tools
>+ cvs -d:ext:cltbld@cvs.mozilla.org:/mofo co -d stage release/stage
>+ cvs -d:ext:cltbld@cvs.mozilla.org:/mofo co -d key-checkout release/keys/pgp
>+ wget -O pkg-dmg $(HGROOT)$(REPO)/raw-file/$(TAG)/build/package/mac_osx/pkg-dmg --no-check-certificate
>+ cp ../../buildfarm/utils/installdmg.sh .
I think we're going to have to have the tools directory defined as a variable
that defaults to $(HOME)/tools.
>+++ b/release/signing/download-mac-exclude.list
>@@ -0,0 +1,17 @@
>+- *.zip
>+- *.partial.mar
>+- *.asc
>+- *.tests.tar.bz2
>+- *.exe
>+- *.tar.bz2
>+- /contrib/
>+- /contrib-localized/
>+# Skip the root update directory
>+- /update/
>+# Skip everything that's not a mac unsigned build
>+- /win32/
>+- /mac/
>+- /linux-i686/
>+- /unsigned/win32/
>+- /partner-repacks/
>+- /unsigned/update/win32/
How about '+ /unsigned/mac', and '- *'?
>+def signmacfile(filename, keydir):
> basename = os.path.basename(filename)
> dirname = os.path.dirname(filename)
> stdout = tempfile.TemporaryFile()
>+ command = ['codesign',
>+ '-s', MAC_SIGNING_IDENTITY,
>+ '--keychain', '%s' % keydir,
'%s' % keydir is unnecessary, you can use '--keychain', keydir I think.
>@@ -491,10 +526,13 @@ if __name__ == "__main__":
> if not os.path.exists('checkouts/stubs'):
> parser.error("No checkouts/stubs directory found")
>
> if options.concurrency == 1:
> logging.basicConfig(level=logging.INFO, format="%(message)s")
> else:
> logging.basicConfig(level=logging.INFO, format="[%(process)d] %(message)s")
>
>+ MAC_SIGNING_IDENTITY = options.mac_identity
>+ MAC_SIGNING_IDENTIFIER = options.mac_identifier
Having these global variables used in signmacfile makes me nervous. Can we
pass these to Signer instead, which can pass it down to signmacfile?
Attachment #489969 -
Flags: feedback?(catlee) → feedback+
Comment 36•14 years ago
|
||
I'm going to put these comments here, feel free to break out into individual bugs:
1) When generated on 10.5 (like http://people.mozilla.com/~salbiz/en-US/), we get the "old" style signature on disk. 10.5-style looks like:
[whatever].app/
CodeResouces
Contents/
...
While 10.6 looks like:
[whatever].app/
CodeResouces -> Contents/_CodeSignature/CodeResources
Contents/
_CodeSignature/
CodeResources
Not sure if it makes much of a difference but thought I would mention it.
2) We should have a way to manually specify rules in the CodeResources file. We'll need to use those to omit files from the codesignature checking. I think we need at the very minimum the blocklist xml and anything in updates/. Essentially, anything that may be written into the signed app bundle after signing needs to be ignored, unless it delivers an updated CodeResources file.
As an example, here's a rule from Safari:
<key>^Resources/.*\.lproj/SafariHelp/</key>
<dict>
<key>omit</key>
<true/>
<key>weight</key>
<real>50</real>
</dict>
Here they are making the signature verification ignore any help content. They need to do this as help content is updated periodically in the background. They can't deliver a newer CodeResources with the new help content's checksums because users may be running multiple versions of safari with different checksums...there is no way to specify multiple valid checksums for a given path in CodeResources.
Comment 37•14 years ago
|
||
This addresses the issues brought up above, The signing code places the CodeResources artifacts in 10.6 form regardless of where it's run, with the symlink for back compatibility. A custom resource rules file is used to specify what to sign, and so far I've only included the default stuff in Resources and all binaries (dylibs, XUL and firefox-bin) in the seal.
This patch also addresses the tools side of integrating this with release automation, buildbotcustom and eventually configs will also need an update.
Attachment #489969 -
Attachment is obsolete: true
Attachment #495161 -
Flags: feedback?(catlee)
Attachment #495161 -
Flags: feedback?(bhearsum)
Comment 38•14 years ago
|
||
Comment on attachment 495161 [details] [diff] [review]
integrate with release automation, use custom resource rules
Do you need to be checking signed_platforms in makeReleaseRepackUrls ?
It would be nice if the platform specific makefile targets had either win32 or mac in them, e.g. 'sign-win32' instead of just 'sign'
Attachment #495161 -
Flags: feedback?(catlee) → feedback+
Comment 39•14 years ago
|
||
Comment on attachment 495161 [details] [diff] [review]
integrate with release automation, use custom resource rules
I echo both of Catlee's comments, particularly about checking signed_platforms
Can you generalize the rsync stuff in util.commands? You should be able to do do the ssh key handling there, too. Also, use ssh -oIdentityFile= instead of -i -- the latter doesn't expand ~/ properly.
Can you factor out the merge+sort part of merge_files to a different function? Doing so should make it easier to change/extend this to merge large swaths of checksums files down the road (eg, bug 607399).
The docstring of combine_sums.py looks out of date.
Why do we need an exclude and an include rsync file for Mac? Seems like we should just use an exclude one, like Windows. Let me know if you're having trouble getting it to exclude everything you need it to.
You should use rsync --delete when uploading merged checksums -- I'd also recommend --include=*SUMS* --exclude=* to make sure you don't modify anything but sums files.
By and large this looks ok. I'd like to have a run through of it myself after the above is addressed.
Attachment #495161 -
Flags: feedback?(bhearsum) → feedback+
Comment 40•14 years ago
|
||
buildbotcustom integration. post_signing FtpPoller needs to look for artifacts for all signed platforms, and builds and repacks have a configurable way of uploading to the unsigned/ sub-dir.
This is dependant on the tools-side integration landing, as that affects the l10n repacks and post_upload.py.
Attachment #495161 -
Attachment is obsolete: true
Attachment #496640 -
Flags: review?(catlee)
Attachment #496640 -
Flags: review?(bhearsum)
Comment 41•14 years ago
|
||
This separates out the tools side automation integration from changes to the signing scripts. These should be good to land in advance of the changes of the signing scripts, since they default to only treating win32 as a signed platform as usual. Once signing script changes are OK to land and we have a mac signing box set up, the switch can be flipped in the release configs.
Attachment #496645 -
Flags: review?(catlee)
Attachment #496645 -
Flags: review?(bhearsum)
Comment 42•14 years ago
|
||
What should be visible signs the binaries are signed? For example, if you compare the copyright information, between Firefox and Chrome, which appears in the application information (cmnd-i) you could argue that the one for Firefox should say something about Mozilla Corp., but it doesn't say anything.
Do you have a list of things I should be checking for with a human eye?
Comment 43•14 years ago
|
||
(In reply to comment #42)
> What should be visible signs the binaries are signed? For example, if you
> compare the copyright information, between Firefox and Chrome, which appears in
> the application information (cmnd-i) you could argue that the one for Firefox
> should say something about Mozilla Corp., but it doesn't say anything.
>
> Do you have a list of things I should be checking for with a human eye?
As discussed on IRC, the way we've been verifying these internally is running codesign -v <filename> on the app + sealed app internals. Looking at the CodeResources file tells you what files are sealed into the signature. I believe the info string is set in the Info.plist key CFBundleGetInfoString, but this is unrelated to the signing process.
Comment 44•14 years ago
|
||
Comment on attachment 496640 [details] [diff] [review]
integrate multiple signed platforms in buildbotcustom
>diff --git a/changes/ftppoller.py b/changes/ftppoller.py
>--- a/changes/ftppoller.py
>+++ b/changes/ftppoller.py
>@@ -113,16 +113,55 @@ class FtpPoller(FtpPollerBase):
> self.gotFile = 0
> # scenario 2:
> # gotFile is 0, we are waiting for a match to trigger build
> if self.gotFile == 0:
> if re.search(self.searchString, pageContents):
> self.gotFile = 1
> return True
>
>+class MultiFtpPoller(FtpPollerBase):
>+ """Look for multiple files under the same ftpURLs
>+ Fire a sendchange when all files appear under at least one of the URLs"""
I don't think this actually fires a sendchange.
>+ gotFiles = True
Defaulting to True when you end up setting it to 0/1 below. Can we use True/False, or True/False/None?
Looks good otherwise.
Attachment #496640 -
Flags: review?(catlee) → review-
Updated•14 years ago
|
Attachment #496645 -
Flags: review?(catlee) → review+
Comment 45•14 years ago
|
||
Comment on attachment 496645 [details] [diff] [review]
tools side integration
create-release-repacks.py already has access to the release config, so don't add a new flag for this. Check for the platform in releaseConfig['signed_platforms'] instead.
Seems fine other than that, I'd like to see the results of l10n in a staging run once you've got the above fixed.
Attachment #496645 -
Flags: review?(bhearsum) → review-
Comment 46•14 years ago
|
||
Comment on attachment 496640 [details] [diff] [review]
integrate multiple signed platforms in buildbotcustom
What's the point in adding signedPlatformsFiles the release config? Because the signing Makefile doesn't pull the log file names from the release config I think it's better to keep them in release.py. We're hardcoding them either way, and doing it in release.py is more centralized.
l10n stuff needs updating after changing create-release-repacks.py per my previous comment.
Looks fine otherwise.
Attachment #496640 -
Flags: review?(bhearsum) → review-
Comment 47•14 years ago
|
||
Done, will run it through staging to make sure it works as expected.
Attachment #496645 -
Attachment is obsolete: true
Attachment #497982 -
Flags: review?(bhearsum)
Comment 48•14 years ago
|
||
Should address all the comments brought up above.
releaseConfig['signedPlatforms'] will need to be set in the releaseConfigs before this can be deployed, unless we're ok with adding some additional logic to create-release-repacks.py to special case signedPlatforms to a default if it isn't set in the configs.
Attachment #496640 -
Attachment is obsolete: true
Attachment #497984 -
Flags: review?(bhearsum)
Comment 49•14 years ago
|
||
Comment on attachment 497982 [details] [diff] [review]
fix tools-side automation integration
(In reply to comment #48)
> Created attachment 497984 [details] [diff] [review]
> fix buildbotcustom integration
>
> Should address all the comments brought up above.
> releaseConfig['signedPlatforms'] will need to be set in the releaseConfigs
> before this can be deployed, unless we're ok with adding some additional logic
> to create-release-repacks.py to special case signedPlatforms to a default if it
> isn't set in the configs.
I think it's OK to assume it's set.
This patch seems fine now!
Attachment #497982 -
Flags: review?(bhearsum) → review+
Updated•14 years ago
|
Attachment #497984 -
Flags: review?(bhearsum) → review+
Comment 50•14 years ago
|
||
Syed, is there anything left to do here besides land it?
Comment 51•14 years ago
|
||
I dropped the ball here. I needed to:
1 - Check with the security team that excluding everything by default and only including the files we know about is ok
2 - Check with rs to get confirmation about what files can change in the bundle / where ISVs/3rd parties can potentially stick extensions, etc.
CCing some folks.
![]() |
||
Comment 52•14 years ago
|
||
cc'ing Kev in case he has input
We support adding files in the distribution directory though I am not familiar with the structure. I'm not sure if we support adding files in the app's extensions directory for distributions.
Comment 53•14 years ago
|
||
Only question I have around this centers around pkg-dmg, which we use for creating repacked .dmg's from the original installers. I'm going to assume that, since we unpack and repack everything for OSX DMGs already, nothing will change with regards to creating the repack .dmg. We'll have to integrate signing into both the partner repacks and BYOB, and we'll need to acquire a code-signing cert for BYOB as well (which is what we've done for the win32 builds).
Repacks modify the dmg in two places. As rstrong states, we add a distribution directory into Contents/MacOS/, and can add unpacked extensions into Contents/MacOS/extensions as well (but moving fwd we're going to be bundling them in the distribution directory).
Structure of the distribution directory will hopefully not matter, but the nickel tour is:
distribution/distribution.ini primary config file
distribution/bundle - bundled extensions, only addon installer currently (see bug 392251)
distribution/extensions - bundled extension xpis
distribution/searchplugins - bundled search plugins
Don't think there's huge impact, but would ask that RelEng review current partner-repacks scripts, and ensure they'll take this change up as well.
Comment 54•14 years ago
|
||
Ok, so it sounds like the most secure way is likely to specifically exclude Contents/MacOS/extensions and Contents/MacOS/distribution and require everything else we generate. This is slightly different than the previous plan, which was ignoring * and then requiring a signature for every file we know about. The new plan will make files dropped in the app bundle in places we don't expect to break the seal. The old plan allowed them to exist and not break the seal (signature would still prevent people tampering with our binaries and resources though).
(In reply to comment #53)
> Only question I have around this centers around pkg-dmg, which we use for
> creating repacked .dmg's from the original installers. I'm going to assume
> that, since we unpack and repack everything for OSX DMGs already, nothing will
> change with regards to creating the repack .dmg. We'll have to integrate
> signing into both the partner repacks and BYOB, and we'll need to acquire a
> code-signing cert for BYOB as well (which is what we've done for the win32
> builds).
Would we need a different key? I see two options:
a) Sign Firefox resources with Mozilla's key, but not include anything in extensions or distribution in the signature
b) Resign the entire bundle with a different key
I think arguments can be made for both.
Rob, does Firefox edit the app bundle in any other way (blocklist, updates, unpacking extensions, etc)? Or is that all in the profile?
![]() |
||
Comment 55•14 years ago
|
||
App update on Mac adds active-update.xml and updates.xml along with an updates directory all in Contents/MacOS. I will likely be changing that after Firefox 4.
The add-ons manager does not install into the Contents/MacOS/extensions/ directory but I suspect there have been third parties that do so as well as corporate distributions.
The blocklist.xml in Contents/MacOS is not updated by the blocklist service... instead, the blocklist.xml in the profile is created / updated.
I wouldn't be surprised if corporate distributions added / deleted the search plugins under searchplugins.
The same goes for dictionaries in the dictionaries directory.
Corporate distributions often lock prefs creating a mozilla.cfg (typically added to Contents/MacOS) and adding a general.config.filename pref (typically added to one of our pref files or to a new pref file) that points to the mozilla.cfg... this is currently broken :( - see bug 595522.
That is a quick brain dump and there may be others.
![]() |
||
Comment 56•14 years ago
|
||
(In reply to comment #55)
> App update on Mac adds active-update.xml and updates.xml along with an updates
> directory all in Contents/MacOS. I will likely be changing that after Firefox
> 4.
Should have mentioned there are several files that get added to the updates directory while performing an update. If you need more info on the files in the updates directory let me know.
Comment 57•14 years ago
|
||
(In reply to comment #56)
> (In reply to comment #55)
> > App update on Mac adds active-update.xml and updates.xml along with an updates
> > directory all in Contents/MacOS. I will likely be changing that after Firefox
> > 4.
> Should have mentioned there are several files that get added to the updates
> directory while performing an update. If you need more info on the files in the
> updates directory let me know.
I've compiled a list of files (so far) to omit/optionally allow:
Contents/MacOS/active-update.xml
Contents/MacOS/updates.xml
Contents/MacOS/updates/*
Contents/MacOS/extensions/*
Contents/MacOS/distribution/*
Contents/MacOS/mozilla.cfg -> and possibly altering other pref files to add a general.config.filename pref?
If arbritrary pref files may be legitimately modified, perhaps those should all be excluded from the seal as well? Otherwise, is this list of exceptions complete, or could there be more?
Christian, going forward, do you think our signing scripts should be generating the seal based on your comment #54 (include everything in the package under the seal with just the above exceptions, no new additions without a new seal), or fall back to something more like originally planned (include files explicitly, omit some stuff, allow any new files to be added)?
Comment 58•14 years ago
|
||
(In reply to comment #57)
> Christian, going forward, do you think our signing scripts should be generating
> the seal based on your comment #54 (include everything in the package under the
> seal with just the above exceptions, no new additions without a new seal), or
> fall back to something more like originally planned (include files explicitly,
> omit some stuff, allow any new files to be added)?
I think the original plan is less brittle (and thus better) but I'd like the security team to weigh in as it is arguably more permissive in that case. A fair amount of the security team is out on vacation until the new year though.
Updated•14 years ago
|
Attachment #497982 -
Attachment is obsolete: true
Updated•14 years ago
|
Attachment #497984 -
Attachment is obsolete: true
Comment 59•14 years ago
|
||
mostly bitrot refresh, with some changes to account for automated signing.
Attachment #500364 -
Flags: review?(bhearsum)
Comment 60•14 years ago
|
||
Attachment #500365 -
Flags: review?(bhearsum)
Comment 61•14 years ago
|
||
At this point, the only further changes that should be necessary to the signing scripts are tweaking the generateCodeResourcesFile() function in signing.py or scrapping it entirely for a static CodeResources file, based on what the verdict is. The rest of the functionality has been run through on staging and should not change based on that decision.
Attachment #500403 -
Flags: feedback?(rail)
Attachment #500403 -
Flags: feedback?(bhearsum)
Comment 62•14 years ago
|
||
Once the blocking items for this bug are done, this patch turns on mac signing in the release automation. If we want to stagger the landing of the integration, we can also do that by removing macosx64 from the signedPlatforms entry.
Comment 63•14 years ago
|
||
At Catlee's urging, I pinged release-drivers about whether or not we want this for Firefox 4 and the answer was a solid "no". We'll come back to this at some point, but for now, I'm going to removing the feedback/review requests since this isn't a priority.
Updated•14 years ago
|
Attachment #500364 -
Flags: review?(bhearsum)
Updated•14 years ago
|
Attachment #500365 -
Flags: review?(bhearsum)
Updated•14 years ago
|
Attachment #500403 -
Flags: feedback?(rail)
Attachment #500403 -
Flags: feedback?(bhearsum)
Updated•14 years ago
|
Assignee: s.s.albiz → nobody
Updated•14 years ago
|
Whiteboard: [hg-automation][sg:want][signing] → [hg-automation][sg:want][signing][triage]
Comment 64•14 years ago
|
||
FWIW, 10.7 codesign has slightly different output:
107-11A390-LaCie250:Sandbox doug$ codesign -vv Firefox.app/
Firefox.app/: code object is not signed at all
In architecture: i386
than 10.6:
…Firefox 4.0b12/Firefox.app: code object is not signed
Comment 65•14 years ago
|
||
(In reply to comment #63)
> At Catlee's urging, I pinged release-drivers about whether or not we want this
> for Firefox 4 and the answer was a solid "no". We'll come back to this at some
> point, but for now, I'm going to removing the feedback/review requests since
> this isn't a priority.
bhearsum: could we land everything and pref it off for now?
Comment 66•14 years ago
|
||
These patches aren't ready to land. They need unbitroting and at least another tag -> signing test before we could even land them prefed off. I don't think landing them prefed off would satisfy this bug, anyways.
Updated•14 years ago
|
Assignee: nobody → catlee
Priority: P3 → P2
Whiteboard: [hg-automation][sg:want][signing][triage] → [hg-automation][sg:want][signing][oldbug]
Comment 67•14 years ago
|
||
From triage, with coop, catlee, rail, joduinn
This bug is about having release automation capable of signing OSX universal builds in a similar manner to how we sign win32 builds. We will initially support the intel32/64 universal builds from Firefox 4 and greater.
Once this work is completed and verified by RelEng, we will land this in production, pref'd off for now. Tweaking summary to match.
Before RelEng can pref-on this for a live shipping release, there are many product decisions to be worked through. Some include:
* what release do we want to first ship signed OSX builds (be mindful of other features in that release; upcoming osx10.7 release, etc, etc)
* what info support needs in advance?
* QA signoff
** does this work on all versions of OSX?
** can user on unsigned build update to a signed build?
** are there any sideeffects when installing plugins/addons - especially if the addon/plugin adds files into the Firefox directory?
* anything else?
Summary: Sign the Mac-Binary for OS X → Have release automation support signing OSX builds
Updated•13 years ago
|
Blocks: hg-automation
Comment 69•13 years ago
|
||
Comment on attachment 500365 [details] [diff] [review]
refresh tools integration
I think all of this got implemented in a different bug.
Attachment #500365 -
Attachment is obsolete: true
Comment 70•13 years ago
|
||
Comment on attachment 500364 [details] [diff] [review]
refresh buildbotcustom integration
This won't be needed anymore.
Attachment #500364 -
Attachment is obsolete: true
Comment 71•13 years ago
|
||
Comment on attachment 500406 [details] [diff] [review]
turn on signing in the release configs
Implemented elsewhere.
Attachment #500406 -
Attachment is obsolete: true
Comment 72•13 years ago
|
||
Comment on attachment 500403 [details] [diff] [review]
signing script changes
Review of attachment 500403 [details] [diff] [review]:
-----------------------------------------------------------------
::: release/signing/Makefile
@@ -52,1 @@
> KEYDIR ?= d:/2010-keys
This file isn't needed anymore.
::: release/signing/combine_sums.py
@@ +1,1 @@
> +#!/usr/bin/env python
This file isn't needed anymore.
::: release/signing/download-mac-exclude.list
@@ +1,1 @@
> +- *.zip
This file isn't needed anymore.
::: release/signing/download-mac-include.list
@@ +1,1 @@
> ++ *.dmg
This file isn't needed anymore.
::: release/signing/download-exclude.list
@@ -1,1 @@
> - *.zip
This file isn't needed anymore.
::: release/signing/verify-signature.py
@@ -2,1 @@
> # Verifies that a directory of signed files matches a corresponding directory
This file isn't needed anymore.
Assignee | ||
Comment 73•13 years ago
|
||
Description
•