Closed Bug 1339729 Opened 3 years ago Closed 3 years ago

Removing wow_helper from Windows process sandboxing.

Categories

(Core :: Security: Process Sandboxing, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: bobowen, Assigned: bobowen)

References

Details

(Whiteboard: sbwc2)

Attachments

(1 file)

wow_helper was only required on WinXP 64-bit and Vista 64-bit, which we no longer support.
MozReview-Commit-ID: Bx0mPTEKz3l
Attachment #8837477 - Flags: review?(mh+mozilla)
Comment on attachment 8837477 [details] [diff] [review]
Remove wow_helper from Windows process sandboxing

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

I guess the chromium sandbox code update removes all the Wow64 stuff under security/sandbox/chromium/sandbox/win (that invokes wow_helper). Either way, that code is essentially dead because it's all behind a runtime windows version check for <= vista.
Attachment #8837477 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #3)
> Comment on attachment 8837477 [details] [diff] [review]
> Remove wow_helper from Windows process sandboxing
> 
> Review of attachment 8837477 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I guess the chromium sandbox code update removes all the Wow64 stuff under
> security/sandbox/chromium/sandbox/win (that invokes wow_helper). Either way,
> that code is essentially dead because it's all behind a runtime windows
> version check for <= vista.

Yes that patch will remove it, thanks.
I'll land this when I get back.
I was just about to land this, when I remembered that I think we had to get wow_helper.exe signed and I'm not sure if whatever does that is going to break when I remove this.

glandium - do you know about that or if not know who does?
Flags: needinfo?(mh+mozilla)
I don't know, maybe catlee?
Flags: needinfo?(mh+mozilla) → needinfo?(catlee)
AFAIK we don't treat this specially. It's signed along with all the other exes and dlls that are generated as part of the build. If it's not produced, it won't be signed, and won't be missed.
Flags: needinfo?(catlee)
(In reply to Chris AtLee [:catlee] from comment #7)
> AFAIK we don't treat this specially. It's signed along with all the other
> exes and dlls that are generated as part of the build. If it's not produced,
> it won't be signed, and won't be missed.

Thanks.
I think I remember the history now.
This didn't get signed at first because it was a 64-bit binary in the 32-bit build and the signing tool couldn't cope with it.
We thought we would have to special case it, but in the end we switched to using the 64-bit signing tool for all the binaries.
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f73f900fab1c
Remove wow_helper from Windows process sandboxing. r=glandium
https://hg.mozilla.org/mozilla-central/rev/f73f900fab1c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.