New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment
link

Issue 490654: Renderers not running as AppContainers in Chrome 43

Reported by chromegu...@gmail.com, May 21 2015

Issue description

Chrome Version       : 43.0.2357.65 (Official Build) m (32-bit)
Operating system version: 
>systeminfo | findstr /B /C:"OS Name" /C:"OS Version"
OS Name:                   Microsoft Windows 8.1 Pro
OS Version:                6.3.9600 N/A Build 9600

What steps will reproduce the problem?
1. Start Chrome
2. Go to a website
3. Start Chrome's task manager
4. Get the process id of the renderer associated with the viewed website
5. Start SysInternals Process Explorer
6. Add "Integrity Level" as a column in Process Explorer
7. Find the renderer of interest in Process Explorer. Note that its Integrity Level is "Untrusted", when "AppContainer" is expected
8. Double click on the renderer of interest and look at its token from the Security Tab. Note that the AppContainer SID is not present, and there's nothing that indicates it's an AppContainer

What is the expected result?
Process Explorer indicates that renderers run as AppContainers

What happens instead?
Process Explorer indicates that renderers have similar tokens to those in Chrome 42, when AppContainer tokens were not supported

According to https://twitter.com/justinschuh/status/601242024654737409, Chrome renderers should run with an AppContainer token on Chrome 43, but that does not appear to be happening.
 

Comment 1 by jsc...@chromium.org, May 21 2015

Cc: jsc...@chromium.org
Labels: Cr-Security
Owner: wfh@chromium.org
Status: Assigned
Looks like this might not be getting enabled for all user like it was supposed to be.

wfh@ - Can you look into this? It's fine for me on Canary, where it's unconditinoally enabled, but I'm not seeing the appcontainer either on beta or stable, where it's behind a finch gate.

Comment 2 by wfh@chromium.org, May 21 2015

Cc: cpu@chromium.org shrikant@chromium.org
This is strange. It was my understanding that app containers were not behind a field trial after concerns raised by cpu.

Comment 3 by wfh@chromium.org, May 21 2015

app containers were disabled in r323345 for debugging purposes.

M43 branched in r323860

app containers were re-enabled in r324348

I did do a sanity check of src/sandbox/win to check if there were any un-merged app container related CLs to M43, but did not see the one in content/browser/renderer_host/render_process_host_impl.cc.

This is unfortunate. We should merge these CLs into M43 for hopefully the next point release, I will track them down and handle the merge.

Comment 4 by wfh@chromium.org, May 21 2015

Labels: M-43 Merge-Request-43
The CLs to merge are 9203ef8157d4d47970dd0255d7de009697618bb2 and 55c2f2365e41a170f37ed56d13ced4ef054abffa and the sum of those two CLs is https://codereview.chromium.org/1149143002

This code has been on Dev/Canary for a long time (since Apr 9) so I believe it will be safe to merge to M43, but I will scour crash logs and do some testing to back this up.

Comment 5 by pennymac@google.com, May 21 2015

Labels: -Merge-Request-43 Merge-Review-43 Hotlist-Merge-Review
[Automated comment] No bugdroid (commit) comments found, couldn't auto-approve, needs manual review.

Comment 6 by shrikant@chromium.org, May 21 2015

Thanks wfh@, let me know if anything is required from my side.

Comment 7 by wfh@chromium.org, May 21 2015

laforge approved this on the CL - https://codereview.chromium.org/1149143002 so will merge now.

Comment 8 by laforge@google.com, May 21 2015

Labels: -Merge-Review-43 Merge-Approved-43

Comment 9 by wfh@chromium.org, May 21 2015

Labels: -Merge-Approved-43 -Hotlist-Merge-Review merge-merged-2357
Merged in 638956dafee998e7fd761afdd052959b01ad3bc7

Comment 10 by shrikant@chromium.org, May 21 2015

Cc: lafo...@chromium.org penny...@chromium.org ananta@chromium.org
I forgot to mention earlier, we also need to integrate DirectWrite font cache related CL (https://codereview.chromium.org/1122163002) to enable Font Cache on Windows 8/8.1. Otherwise users will see loading delays (pre-font cache) close to what was in v40. Permission to integrate? (+pennymac/+laforge)

Comment 11 by penny...@chromium.org, May 21 2015

Labels: M-44 Merge-Approved-44
You need to make a merge request and merge for m44 branch.  Go ahead with the merge to 2403.

Comment 12 by laforge@google.com, May 22 2015

Labels: Merge-Approved-43
I may yet regret this, but approved for M43 (2357).

Comment 13 by cpu@chromium.org, Jun 5 2015

Cc: -cpu@chromium.org

Comment 14 by wfh@chromium.org, Jul 30 2015

Status: Fixed

Comment 15 by laforge@google.com, Sep 11 2015

Labels: -Merge-Approved-44 -Merge-Approved-43
Clearing approvals older than 60 days

Sign in to add a comment