Closed Bug 1160514 Opened 4 years ago Closed 4 years ago

wow_helper.exe should be signed

Categories

(Core :: Security: Process Sandboxing, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla41
Tracking Status
firefox39 + verified
firefox40 + verified
firefox41 --- verified
firefox-esr38 39+ verified

People

(Reporter: dmajor, Assigned: catlee)

References

Details

Attachments

(1 file, 1 obsolete file)

(In reply to Bob Owen (:bobowen) from bug 1139497 comment #14)
> I also noticed that wow_helper.exe is not signed, whereas the other EXEs seem to be.

Glandium, who is the right owner for this? You? Releng?
Flags: needinfo?(mh+mozilla)
Releng. Maybe bhearsum?
Flags: needinfo?(mh+mozilla)
From yesterday's nightly build log it sure looks like we're trying to sign it:
06:52:03     INFO -  2015-04-30 06:50:57,069 - 0d8c10378535a4bbd02f743b53515c48029ce712: processing ../../installer-stage\core\wow_helper.exe on https://signing5.srv.releng.scl3.mozilla.com:9100
06:52:03     INFO -  2015-04-30 06:50:57,944 - 0d8c10378535a4bbd02f743b53515c48029ce712: uploading for signing
06:52:03     INFO -  2015-04-30 06:50:59,016 - 0d8c10378535a4bbd02f743b53515c48029ce712: processing ../../installer-stage\core\wow_helper.exe on https://signing5.srv.releng.scl3.mozilla.com:9100
06:52:03     INFO -  2015-04-30 06:50:59,301 - 0d8c10378535a4bbd02f743b53515c48029ce712: OK
06:52:03     INFO -  2015-04-30 06:50:59,301 - Copying ../../installer-stage\core\wow_helper.exe to cache c:\\builds\\moz2_slave\\m-cen-w32-ntly-000000000000000\\build\\signing_cache\signcode\0d8c10378535a4bbd02f743b53515c48029ce712
Oh, I see, wow_helper.exe is a 64-bit binary in our 32-bit builds, so signcode is probably just failing to produce a valid signature. We use osslsigncode instead of signcode for 64-bit builds:
https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/signing.mk#11

(I don't think this bug needs to be security sensitive, it's just a build screwup.)
Group: core-security
Attached file wow_helper.exe (obsolete) —
If I could get a signed version, that'd be helpful for an OEM partner requirement.
Comment on attachment 8606570 [details]
wow_helper.exe

nm, moving to a separate bug.
Attachment #8606570 - Attachment is obsolete: true
yeah, we just need to switch the signing format for this to osslsigncode. Arguably, we could do that for all 32-bit signing.

The 'signcode' format uses mono's signcode implementation, which silently fails on 64-bit binaries.
(In reply to Chris AtLee [:catlee] from comment #6)

Is this something we want to do asap to catch the 38.0.5 RC builds ?
Tracking enabled for 39 and 40.
we should fix this on nightly first before beta/release to make sure there's no fallout
I agree with catlee and we can still probably take this for 39 if it lands in the next few days. Ted is this something you can take on, or help find an owner for it?
Flags: needinfo?(ted)
This is a RelEng bug.
Flags: needinfo?(ted)
Looks good. Let's land on nightly today, then uplift after a few days.
Attachment #8616716 - Flags: review?(ted) → review+
Attachment #8616716 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/e82614b263f7
Assignee: nobody → nthomas
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment on attachment 8616716 [details] [diff] [review]
use osslsigncode for all windows signing

Approval Request Comment
[Feature/regressing bug #]: bug 1160514
[User impact if declined]: wow_helper.exe may be flagged as malware
[Describe test coverage new/current, TreeHerder]: no new testing
[Risks and why]: fixes incorrect signing for wow_helper.exe
[String/UUID change made/needed]: none
Attachment #8616716 - Flags: approval-mozilla-beta?
Attachment #8616716 - Flags: approval-mozilla-aurora?
Assignee: nthomas → catlee
Comment on attachment 8616716 [details] [diff] [review]
use osslsigncode for all windows signing

Signed exec is better. Taking it for aurora. Liz will make the call for beta.
Attachment #8616716 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
FWIW this won't impact release where we don't have win64 builds, so putting this on beta right now would only gain us a couple of beta builds.
(In reply to David Major [:dmajor] from comment #19)
> FWIW this won't impact release where we don't have win64 builds, so putting
> this on beta right now would only gain us a couple of beta builds.

Confusingly, this is for win32 builds, so it does affect beta and release.
At least for Vista 64-bit and WinXP 64-bit (64-bit OS and 32-bit Firefox that is).

The 64-bit wow-helper.exe binary is needed for some of the process memory patching that happens during the sandbox launch, when the 32-bit plugin-container.exe is running under WOW64.
Oops; you're right Bob. I was confusing this with another bug. This is what I get for commenting before the coffee kicks in!
Comment on attachment 8616716 [details] [diff] [review]
use osslsigncode for all windows signing

Yes, please uplift to beta.
Attachment #8616716 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Chris suggested we uplift this to ESR39. Adding a tracking flag for ESR39. Once this fix is in beta for a few days, we can review it again.
dmajor, could you check all is well on Nightly ?
Flags: needinfo?(dmajor)
I opened the Properties page for wow_helper.exe from ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/06/2015-06-16-03-02-01-mozilla-central/firefox-41.0a1.en-US.win32.zip, and it now has a Digital Signatures tab with reasonable-looking information.
Flags: needinfo?(dmajor)
I checked wow-helper.exe from the Aurora and Beta builds and they were both signed with the "Mozilla Fake SPC" certificate.
Have we spun a new beta since then? The latest Aurora at ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/06/2015-06-17-00-40-06-mozilla-aurora/ looks fine.
(In reply to David Major [:dmajor] from comment #29)
> Have we spun a new beta since then? The latest Aurora at
> ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2015/06/2015-06-17-00-
> 40-06-mozilla-aurora/ looks fine.

I think that's tomorrow, I was just looking at the uplift builds.
(In reply to Bob Owen (:bobowen) from comment #28)
> I checked wow-helper.exe from the Aurora and Beta builds and they were both
> signed with the "Mozilla Fake SPC" certificate.

which URLs did you check?
(In reply to Chris AtLee [:catlee] from comment #31)
> (In reply to Bob Owen (:bobowen) from comment #28)
> > I checked wow-helper.exe from the Aurora and Beta builds and they were both
> > signed with the "Mozilla Fake SPC" certificate.
> 
> which URLs did you check?

Aurora:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-win32/1434483407/firefox-40.0a2.en-US.win32.zip

Beta:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-win32/1434484011/firefox-39.0.en-US.win32.zip
Hi Chris, please request uplift to ESR 38 once this fix has baked long enough on Beta and Aurora release channels.
Flags: needinfo?(catlee)
(In reply to Bob Owen (:bobowen) from comment #32)
> (In reply to Chris AtLee [:catlee] from comment #31)
> > (In reply to Bob Owen (:bobowen) from comment #28)
> > > I checked wow-helper.exe from the Aurora and Beta builds and they were both
> > > signed with the "Mozilla Fake SPC" certificate.
> > 
> > which URLs did you check?
> 
> Aurora:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-
> aurora-win32/1434483407/firefox-40.0a2.en-US.win32.zip
> 
> Beta:
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-
> win32/1434484011/firefox-39.0.en-US.win32.zip

Generally anything in tinderbox-builds will not have valid signatures. We only sign nightly builds and release builds with real keys. Nightly builds are in .../firefox/nightly/...; and we don't generate nightly builds for beta, but you can find recent candidate builds in .../firefox/candidates/...
Flags: needinfo?(catlee)
(In reply to Chris AtLee [:catlee] from comment #34)
> (In reply to Bob Owen (:bobowen) from comment #32)

> Generally anything in tinderbox-builds will not have valid signatures. We
> only sign nightly builds and release builds with real keys. Nightly builds
> are in .../firefox/nightly/...; and we don't generate nightly builds for
> beta, but you can find recent candidate builds in .../firefox/candidates/...

Sorry, my comment was not very clear, I was suggesting that the fix seemed to have worked as the exe was getting signed (albeit by a non-real key).
Whereas before it wasn't getting signed at all.
Flags: qe-verify+
Verified on Windows 7 64-bit, Windows 8 64-bit and Windows 10 32-bit that Firefox 39 beta 7`s wow_helper.exe does have a Digital Signatures tab in properties.
Blocks: 1177771
Comment on attachment 8616716 [details] [diff] [review]
use osslsigncode for all windows signing

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: ESR38 is the last place we're using the old signing (signcode); switching to the new format will allow us to simplify our signing infrastructure
User impact if declined: wow_helper.exe may be flagged as malware
Fix Landed on Version: 39.0
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch: none
Attachment #8616716 - Flags: approval-mozilla-esr38?
Also verified on Windows 7 64-bit, Windows 8.1 64-bit and Windows 10 32-bit that Firefox 40 beta 4 and Developer Edition`s wow_helper.exe does have a Digital Signatures tab in properties.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Comment on attachment 8616716 [details] [diff] [review]
use osslsigncode for all windows signing

verified in all the other builds, taking it to simplify your life.
Attachment #8616716 - Flags: approval-mozilla-esr38? → approval-mozilla-esr38+
Flags: qe-verify+
Also verified with the latest Firefox 38.2.0 ESR build 1 (BuildID=20150803192212) on Windows 7 x64. wow_helper.exe is signed same as in Firefox 39, and same as Firefox.exe.
You need to log in before you can comment on or make changes to this bug.