Closed Bug 1083907 Opened 10 years ago Closed 9 years ago

Thunderbird still doesn't have a valid OS X V2 signature

Categories

(Thunderbird :: Build Config, defect)

Product:
Thunderbird
Component:
Build Config
Platform:
All
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

(thunderbird38+ fixed)

Status:
RESOLVED DUPLICATE of bug 1134867
Tracking Flags:
Tracking Status
thunderbird38 + fixed

People

(Reporter: jsbruner, Unassigned)

References

Details

Attachments

(1 file)

Like this? [backed out]
567 bytes, patch
rkent
: review+
Details | Diff | Splinter Review 
Currently all known patches have been ported to Thunderbird:

Bug 1074002 - Modify file structure of Thunderbird.app to allow for OSX v2 signing
Bug 1074006 - Get Thunderbird to launch with the new .app bundle structure
Bug 1074011 - Thunderbird's preprocessed channel-prefs.js file needs to be the same for each build
Bug 1072652 - Update removed-files for the move from Contents/MacOS to Contents/Resources

However, I tested the signature on a freshly installed OS X VM, and the signature still isn't valid. (One can download the finished TB builds from here: https://tbpl.mozilla.org/?tree=Thunderbird-Trunk&rev=67a8eb70e294)

I don't really understand what else is needed, or how to diagnose this any farther. Perhaps something in our builders needs updating?
You're using the wrong signing cert!

From the results of codesign -d -vvvv for the build from comment #0:

Authority=Mozilla Fake DMG Cert

From the results of codesign -d -vvvv for today's m-c nightly:

Authority=Developer ID Application: Mozilla Corporation
Authority=Developer ID Certification Authority
Authority=Apple Root CA
Interesting, thanks.

Mark, this seems like it has to do with the builders, so could you take a look at this?
Flags: needinfo?(standard8)
It's normal for this cert to be used for on-change and try builds. You only get a real cert for Nightly and Release builds.
Okay, good to know. I'll make sure to test tomorrow's build then.
Yeah, lets try tomorrow. The last nightly (which didn't have the patches) looked like it should be signed with the right cert.
Note that the patches I submitted for Thunderbird and SeaMonkey were to just keep them working and I didn't check if everything that would be needed for signing was completed. So, there might still be additional work to get a valid signature.
Flags: needinfo?(standard8)
Well, I tried the Nightly build today and we're still not signing correctly, however the signature is the Mozilla one.

Authority=Developer ID Application: Mozilla Corporation
Authority=Developer ID Certification Authority
Authority=Apple Root CA

Unfortunately I'm not really sure what else to look at, as our file structure seems correct.
> Well, I tried the Nightly build today and we're still not signing correctly

What, exactly, do you mean?  What's going wrong?

What's the output of spctl -a -v Daily.app?

Also please attach the entire output of codesign -d -vvvv Daily.app.
"What, exactly, do you mean?  What's going wrong?"

Attempting to launch the application still gets blocked by Gatekeeper.

"What's the output of spctl -a -v Daily.app?"

Daily.app: rejected
source=no usable signature

"Also please attach the entire output of codesign -d -vvvv Daily.app."

Identifier=org.mozilla.daily
Format=bundle with Mach-O universal (i386 x86_64)
CodeDirectory v=20200 size=281 flags=0x0(none) hashes=7+3 location=embedded
Hash type=sha1 size=20
CDHash=040bec045cfa26040cfb6b8577ca9dfb08987146
Signature size=8524
Authority=Developer ID Application: Mozilla Corporation
Authority=Developer ID Certification Authority
Authority=Apple Root CA
Timestamp=Oct 17, 2014, 7:35:42 AM
Info.plist entries=20
TeamIdentifier=43AQ936H96
Sealed Resources version=2 rules=12 files=85
Internal requirements count=1 size=188
I've just tried myself (with today's comm-central nightly on OS X 10.9.5) and got the same results.

The output of codesign -d -vvvv looks completely valid (when compared with the output for yesterday's m-c nightly, with which spctl -a -v has no problems).

I'm stumped.  Ben, do you have any ideas?

Stephen Pohl will be back next week.  We may have to wait for him.
Flags: needinfo?(bhearsum)
I spoke too soon.  I've figured out that there's a problem with the Contents/Library directory, or possibly with the stuff in it.

When I try to force resign today's comm-central nightly with my own Apple developer cert, I get the following output (and error):

codesign -f -s "Steven Michaud" Daily\ 2014-10-17.app
Daily 2014-10-17.app: replacing existing signature
Daily 2014-10-17.app: code object is not signed at all
In subcomponent: /Users/smichaud/Desktop/Daily 2014-10-17.app/Contents/Library/Spotlight/thunderbird.mdimporter

But if I delete the Library directory (and its contents) and resign again, I see no errors.  I also get a clean bill of health from spctl -a -v.
I notice that none of the Apple apps in /Applications (on 10.9.5) has a Library directory.  I expect that's the problem.  You probably need to move thunderbird.mdimporter to some other location.  I don't know *which* location.  But whoever implemented thunderbird.mdimporter should re-examine the relevant Apple docs and see what might have changed.
Flags: needinfo?(bhearsum)
Oops again.  The true solution is much simpler:

You need to sign thunderbird.mdimporter and Daily.app seperately -- first the one and then the other.

If I do that with my Apple developer cert, spctl -a -v has no problems with Daily.app.
Alright, fascinating. Thanks!

I guess the question now is how do we sign both in the builders?
> I guess the question now is how do we sign both in the builders?

I have no idea.
Flags: needinfo?(standard8)
I think bhearsum would know what needs to be done
Flags: needinfo?(bhearsum)
I had a brief look at the build log and it appears to be signing things. I'm guessing that the package isn't laid out correctly, but I'm not certain. comment #9 is confusing to me, spctl says it can't find a usable signature, but codesign clearly shows one. The "codesign -vvvv" output may be interesting here, -d seems to give different output than I would expect.
Flags: needinfo?(bhearsum)
Without the -d option I get:

Daily.app/: code object is not signed at all
In subcomponent: /Users/josiah/Desktop/Daily.app/Contents/Library/Spotlight/thunderbird.mdimporter

Which makes sense considering comment 13.
I'm pretty sure we just need to change the build system to sign first Contents/Library/Spotlight/thunderbird.mdimporter and the whole of Daily.app.
I'm pretty sure we just need to change the build system to sign first Contents/Library/Spotlight/thunderbird.mdimporter and then the whole of Daily.app.
Robert, Ben, any idea how we could get the mdimporter signed? I'm not quite sure where to look in the code.
Flags: needinfo?(standard8)
Flags: needinfo?(robert.strong.bugs)
Flags: needinfo?(bhearsum)
(In reply to Mark Banner (:standard8) (away until 23rd Oct) from comment #21)
> Robert, Ben, any idea how we could get the mdimporter signed? I'm not quite
> sure where to look in the code.

You probably need code similar to http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#531. It should "just work" if you do it during packaging...
Flags: needinfo?(bhearsum)
I think it needs to be included in package-manifest.in and packaged like everything else.
Flags: needinfo?(robert.strong.bugs)
Blocks: 1055326
Is this a critical issue for Thunderbird 38?
I would say yes. Although we *could* release without, it would reflect quite poorly on us from a security stance.
Severity: normal → critical
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #23)
> I think it needs to be included in package-manifest.in and packaged like
> everything else.

Something like this??
Depends on when thunderbird.mdimporter is built. To find out it will need to be built, signed, etc.
Comment on attachment 8567211 [details] [diff] [review]
Like this? [backed out]

Is this something we need to land and test, or how else can we make progress on this?
Could we land on comm-central, wait for the nightly built, and then check the results (Backing the patch out at that point if necessary?. That's pretty simple. We don't do the signing on our other branches, so I guess it's comm-central or nothing. Alternatively, we could try to get signing going on another branch.

Of course we'd run a Try Build first to make sure builds function properly.

Joshua, thoughts/approval?
Flags: needinfo?(Pidgeot18)
I think the m-c crowd has a dedicated branch to testing releng stuff (ash?), so that's usually how this sort of stuff is integrated. I don't know much about OS X build systems, and even less about signing, so I cannot give adequate review on any related patch.

Trying to set up a separate releng branch for testing would take far too long, so it makes sense to me to test it on comm-central, even if it is suboptimal.
Flags: needinfo?(Pidgeot18)
So it seems like really the only way forward is to land it on c-c, and test in the nightly.

Somebody has to take the fall for this if it fails, so I'll volunteer. I'll r+ and land after I've done what I can to test it locally (which is not much).
Comment on attachment 8567211 [details] [diff] [review]
Like this? [backed out]

Let's leave the bug open while we test the nightlies.

https://hg.mozilla.org/comm-central/rev/b5da9a76aca5
Attachment #8567211 - Flags: review+
Backed out https://hg.mozilla.org/comm-central/rev/816f22bf646f

	package-manifest ../../dist ../../dist/thunderbird \
--non-resource defaults/messenger/mailViews.dat
Error: /builds/slave/tb-c-cen-osx64-d-0000000000000/build/objdir-tb/mail/installer/package-manifest:38: DailyDebug.app/Contents/Library/Spotlight/thunderbird.mdimporter/Contents/Info.plist already added
Error: /builds/slave/tb-c-cen-osx64-d-0000000000000/build/objdir-tb/mail/installer/package-manifest:38: DailyDebug.app/Contents/Library/Spotlight/thunderbird.mdimporter/Contents/Resources/English.lproj/InfoPlist.strings already added
Error: /builds/slave/tb-c-cen-osx64-d-0000000000000/build/objdir-tb/mail/installer/package-manifest:38: DailyDebug.app/Contents/Library/Spotlight/thunderbird.mdimporter/Contents/Resources/English.lproj/schema.strings already added
Error: /builds/slave/tb-c-cen-osx64-d-0000000000000/build/objdir-tb/mail/installer/package-manifest:38: DailyDebug.app/Contents/Library/Spotlight/thunderbird.mdimporter/Contents/Resources/schema.xml already added
Traceback (most recent call last):
File "/builds/slave/tb-c-cen-osx64-d-0000000000000/build/mozilla/toolkit/mozapps/installer/packager.py", line 399, in <module>
main()
File "/builds/slave/tb-c-cen-osx64-d-0000000000000/build/mozilla/toolkit/mozapps/installer/packager.py", line 352, in main
copier.add(mozpack.path.join(respath, 'removed-files'), removals)
File "/tools/python/lib/python2.7/contextlib.py", line 24, in __exit__
self.gen.next()
File "/builds/slave/tb-c-cen-osx64-d-0000000000000/build/mozilla/python/mozbuild/mozpack/errors.py", line 129, in accumulate
raise AccumulatedErrors()
mozpack.errors.AccumulatedErrors
make[3]: *** [stage-package] Error 1
make[2]: *** [make-package] Error 2
make[1]: *** [default] Error 2
make: *** [package] Error 2
Could it be, that the mdimporter doesn't get signed, because it is missing its folder /Contents/MacOS since version 36 (Bug 1134867), and so the signing machinery does not detect it as an executable?
Actually, it looks like mdimporter's executable is completely gone! The executable was (and should be) in MacOS/, but you are right that it's gone.

Evidently we are packaging mdimporter, so I'll fix Bug 1134867 and see what happens here.
Okay, that bug's patch landed. I'll verify the results of the signing tomorrow.
Fantastic. My two outputs now:

codesign -d -vvvv Daily.app/
Executable=/Users/mozilla/Desktop/Daily.app/Contents/MacOS/thunderbird
Identifier=org.mozilla.daily
Format=bundle with Mach-O universal (i386 x86_64)
CodeDirectory v=20200 size=281 flags=0x0(none) hashes=7+3 location=embedded
Hash type=sha1 size=20
CDHash=f58b7535dd3d18bbb1c2be1c2b442c2185fb7ef0
Signature size=8523
Authority=Developer ID Application: Mozilla Corporation
Authority=Developer ID Certification Authority
Authority=Apple Root CA
Timestamp=Mar 7, 2015, 8:03:00 AM
Info.plist entries=20
TeamIdentifier=43AQ936H96
Sealed Resources version=2 rules=12 files=85
Internal requirements count=1 size=188

and

spctl -a -v Daily.app
Daily.app: accepted
source=Developer ID

Looks like that bug fixed it!
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 1134867
Since the patch in bug 1134867 resolved this with no other required code, this bug is a duplicate of that one.
No longer depends on: 1134867
Resolution: FIXED → DUPLICATE
Attachment #8567211 - Attachment description: Like this? → Like this? [backed out]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: