Update security/sandbox/chromium/ to Chromium commit ba7a0041073a5e9928d277806bfe24c325d113e5.

RESOLVED FIXED in Firefox 59

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
Last month

People

(Reporter: bobowen, Assigned: bobowen)

Tracking

(Blocks 1 bug)

Trunk
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: sb+)

Attachments

(26 attachments)

575.38 KB, patch
aklotz
: review+
jld
: review+
Details | Diff | Splinter Review
109.80 KB, patch
aklotz
: review+
jld
: review+
Details | Diff | Splinter Review
250.07 KB, patch
aklotz
: review+
Details | Diff | Splinter Review
8.23 KB, patch
aklotz
: review+
Details | Diff | Splinter Review
6.00 KB, patch
aklotz
: review+
Details | Diff | Splinter Review
7.20 KB, patch
jld
: review+
Details | Diff | Splinter Review
6.25 KB, patch
jld
: review+
Details | Diff | Splinter Review
10.39 KB, patch
aklotz
: review+
Details | Diff | Splinter Review
2.01 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
10.61 KB, patch
aklotz
: review+
Details | Diff | Splinter Review
28.81 KB, patch
aklotz
: review+
Details | Diff | Splinter Review
1.78 KB, patch
aklotz
: review+
Details | Diff | Splinter Review
1.75 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
12.23 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
1.17 KB, patch
jld
: review+
Details | Diff | Splinter Review
16.05 KB, patch
jld
: review+
Details | Diff | Splinter Review
11.97 KB, patch
jld
: review+
Details | Diff | Splinter Review
1.77 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
6.12 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
1.54 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
27.25 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
6.39 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
1.46 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
6.39 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
6.15 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
7.10 KB, patch
bobowen
: review+
Details | Diff | Splinter Review
Update security/sandbox/chromium/ to latest Chromium stable channel version once Fx56 is on m-c.
This should be chromium 59.
The way the dates work here mean that we might need to take some minor release patches as well.

Dropping this into milestone sbwc3, because I don't think we should block on this for sbwc2, but we'll want to do it soon afterwards if it doesn't make it into 56 for some reason.
Working on updating to chromium commit: 9f4b44b898b326679817ee5a327256f8fac6ee75
Assignee: nobody → bobowencode
Status: NEW → ASSIGNED

Updated

2 years ago
Priority: -- → P2
Whiteboard: sbwc3 → sb+
Priority: P2 → P1
Summary: Update security/sandbox/chromium/ to latest Chromium stable channel version in Fx56. → Update security/sandbox/chromium/ to Chromium commit ba7a0041073a5e9928d277806bfe24c325d113e5.
I'm going to upload the patches in broken down form to hopefully aid review.

I'm not going to land until Fx59, so there isn't a massive rush for these.

Then I'll land as two rolled-up patches.
The first with the chromium update and any patches needed to get a compiling and mostly working Firefox.
The second with the rest of our changes.

I'm changing the way we track our own patches away from the modifications-to-chromium-to-reapply-after-upstream-merge.txt file.
Now there will be two directories in security/sandbox/chromium-shim/patches/.
* with_update/ - for patches that need to be applied to get a compiling and mostly working Firefox.
* after_update/ - for the rest.

Each dir will have a patch_order.txt file, as it matters in a couple of cases.

I won't upload all the separate patches that add in the patch files themselves as it will just create more noise.
These were fixes to our upstream patch that didn't make it into release Chromium.


From db4c64b63d6098294ed255e962700fd2d465575e Mon Sep 17 00:00:00 2001
When a parent has two alternate desktops, one on a local winstation, the
other on an alternate winstation:

1) Ensure the desktops have different names.
2) Ensure both desktops have their integrity level set correctly.

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1229829#c11
Cq-Include-Trybots: master.tryserver.chromium.win:win10_chromium_x64_rel_ng
Change-Id: I2d17779e389c9f74146a83d97d62babffa903184
Reviewed-on: https://chromium-review.googlesource.com/638872
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499280}
---
Attachment #8926375 - Flags: review?(aklotz)
Comment on attachment 8926381 [details] [diff] [review]
Update chromium's list of linux-x86-32 syscalls

Carrying r=jld from original landing.
Attachment #8926381 - Flags: review+
This is basically a revert of chromium commit 996b42db5296bd3d11b3d7fde1a4602bbcefed2c.

We still use AddTargetPeer, required when duplicating handles to non-sandboxed children.
Attachment #8926383 - Flags: review?(aklotz)
This basically reverts chromium commit 569193665184525ca366e65d0735f5c851106e43.

We still use BrokerDuplicateHandle, Chromium does this all explicitly from the parent now and so removed this.
Attachment #8926385 - Flags: review?(aklotz)
This brings in new dependencies via FilePath and we don't currently use it.
As far as I can tell Chromium doesn't use it either.
Attachment #8926386 - Flags: review?(aklotz)
This has a dependency on nspr, which causes issues.

Originally landed in changeset:
https://hg.mozilla.org/mozilla-central/rev/477b991bf6fa7b4511768649c9bf37c7275d30d9
Comment on attachment 8926388 [details] [diff] [review]
Don't compile base::Time::FromStringInternal

Carrying r=aklotz from original landing.
Attachment #8926388 - Flags: review+
Comment on attachment 8926389 [details] [diff] [review]
Add option to Windows chromium sandbox policy to not use restricting SIDs

Carrying r=jimm from original landing.
Attachment #8926389 - Flags: review+
We don't currently make use of it and it brings in many dependencies.
Attachment #8926391 - Flags: review?(jld)
This basically reverts chromium commit 35a31743fdc9006d5424fd2e1e15f6aba0a3c30c.
Attachment #8926400 - Flags: review?(jld)
This basically reverts chromium commit 84956fa86786f73fa41ebf99e2f6b8d549688c91.
Attachment #8926402 - Flags: review?(jld)
The build warning is for "possible misuse of comma operator".

The comma operator is a bit of a footgun becasue its first operand's result
just gets dropped on the floor (in this case, the result of the DCHECK
expression).  It appears that Chromium's use of the comma operator here is
intentional, though -- so we might as well accept clang's suggestion and "cast
expression to void to silence warning".

This is also filed upstream as:
 https://bugs.chromium.org/p/chromium/issues/detail?id=729123
Comment on attachment 8926404 [details] [diff] [review]
Add a void cast to silence clang Wcomma build warning, in sandbox's snapshot of chromium header

Carrying r=bobowen from original landing.
Attachment #8926404 - Flags: review+
This also changes the read only related status checks in filesystem_interception.cc to include STATUS_NETWORK_OPEN_RESTRICTION (0xC0000201), which gets returned in some cases and fails because we never ask the broker.
Comment on attachment 8926405 [details] [diff] [review]
Allow a special all paths rule in the Windows process sandbox when using semantics FILES_ALLOW_READONLY

Carrying r=jimm from original landing.
Attachment #8926405 - Flags: review+
Comment on attachment 8926407 [details] [diff] [review]
Revert commit f7540af7428f4b146136ec19b781886693f8c03f changes to policy_target.cc for causing issues with CoInitializeSecurity

Carrying r=aklotz from original landing.
Attachment #8926407 - Flags: review+
Comment on attachment 8926408 [details] [diff] [review]
Re-apply - Logging changes to the Chromium interception code

Carrying r=tabraldes from the original landing.
Attachment #8926408 - Flags: review+
Comment on attachment 8926409 [details] [diff] [review]
Change to allow network drives in sandbox rules with non-file device fix

Carrying r=aklotz from original landing.
Attachment #8926409 - Flags: review+
Comment on attachment 8926410 [details] [diff] [review]
Add KEY_WOW64_64Key and KEY_WOW64_32KEY to the Chromium sandbox allowed registry read flags

Carrying r=aklotz from original landing.
Attachment #8926410 - Flags: review+
Comment on attachment 8926412 [details] [diff] [review]
Change USER_NON_ADMIN access token level from whitelist to blacklist containing Admin SIDs

Carrying r=jimm from original landing.
Attachment #8926412 - Flags: review+
Hook this into the browser via the XREAppData. This patch contains only the changes to Chromium source code.

Originally landed in changeset:
https://hg.mozilla.org/mozilla-central/rev/6ecd19d25822
Comment on attachment 8926413 [details] [diff] [review]
Add mechanism to libsandbox_s to track names of files that have been given special sandbox access permissions (PermissionsService)

Carrying r=bobowen from original landing.
Attachment #8926413 - Flags: review+
Allows the creation/use of temp files when the user has already green-lit
the use of a file for write purposes in that folder.

Originally landed in changeset:
https://hg.mozilla.org/mozilla-central/rev/0f64b24c40c4
Comment on attachment 8926414 [details] [diff] [review]
Permit sandboxed processes to access Flash temporary files

Carrying r=bobowen from original landing.
Attachment #8926414 - Flags: review+
Depends on: 1415569
Comment on attachment 8926372 [details] [diff] [review]
Update existing security/sandbox/chromium/base files to chromium commit ba7a0041073a5e9928d277806bfe24c325d113e5

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

rs=me
Attachment #8926372 - Flags: review?(aklotz) → review+
Attachment #8926373 - Flags: review?(aklotz) → review+
Comment on attachment 8926374 [details] [diff] [review]
Update existing security/sandbox/chromium/sandbox/win files to chromium commit ba7a0041073a5e9928d277806bfe24c325d113e5

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

rs=me
Attachment #8926374 - Flags: review?(aklotz) → review+
Attachment #8926375 - Flags: review?(aklotz) → review+
Attachment #8926377 - Flags: review?(aklotz) → review+
Attachment #8926380 - Flags: review?(aklotz) → review+
Attachment #8926383 - Flags: review?(aklotz) → review+
Attachment #8926385 - Flags: review?(aklotz) → review+
Attachment #8926386 - Flags: review?(aklotz) → review+
Comment on attachment 8926372 [details] [diff] [review]
Update existing security/sandbox/chromium/base files to chromium commit ba7a0041073a5e9928d277806bfe24c325d113e5

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

I've either read or at least skimmed through everything here that isn't Windows-specific.
Attachment #8926372 - Flags: review?(jld) → review+
Attachment #8926373 - Flags: review?(jld) → review+
Attachment #8926378 - Flags: review?(jld) → review+
Attachment #8926379 - Flags: review?(jld) → review+
Attachment #8926391 - Flags: review?(jld) → review+
Attachment #8926400 - Flags: review?(jld) → review+
Attachment #8926402 - Flags: review?(jld) → review+

Comment 46

2 years ago
Pushed by bobowencode@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d87028e56a05
Part 1: Roll-up of chromium sandbox update and mozilla patches to get a running browser. r=jld,aklotz,jimm,bobowen
https://hg.mozilla.org/integration/mozilla-inbound/rev/387f44470903
Part 2: Roll-up patch to apply remaining mozilla changes to chromium sandbox. r=tabraldes,aklotz,jimm,bobowen

Comment 47

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d87028e56a05
https://hg.mozilla.org/mozilla-central/rev/387f44470903
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
https://hg.mozilla.org/projects/oak/rev/d87028e56a054954aabd4936d787095d79a7babc
Bug 1366701 Part 1: Roll-up of chromium sandbox update and mozilla patches to get a running browser. r=jld,aklotz,jimm,bobowen

https://hg.mozilla.org/projects/oak/rev/387f44470903a99137251849adf382b82e66c159
Bug 1366701 Part 2: Roll-up patch to apply remaining mozilla changes to chromium sandbox. r=tabraldes,aklotz,jimm,bobowen
Depends on: 1452090
You need to log in before you can comment on or make changes to this bug.