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

Issue 719907 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Cert manager allows import of CA roots an messing with trust bits on Kiosk network config screen

Project Member Reported by mnissler@chromium.org, May 9 2017

Issue description

VERSION
Chrome Version: Tested on 60.0.3086.0, but older versions likely affected as well.
Operating System: 9517.0 developer build

REPRODUCTION CASE
1. Flip the device into Hotrod mode
2. Enroll to remora-test.mygbiz.com
3. When the kiosk app launches, hit Ctrl-Alt-N
4. Click "Manage certificates" in the network config screen
5. Mess with CA trust bits (Authorities -> select one -> Edit) or import additional server or CA certs. Observe the changes take effect in the Kiosk session. For example, if you revoke web trust for "GeoTrust Global CA", the CfM app will not start up due to HTTPS connections to Google failing.

This allows arbitrary Kiosk users to compromise HTTPS connections originating from the Kiosk device. For example, an attacker may install a malicious CA and will then be able to man-in-the-middle all HTTPS traffic that the Kiosk device makes. In the CfM use case, this probably allows tapping the video stream for an attacker that has access to a network the traffic flows through.

One mitigating factor is that we apparently have a server-side knob to disallow access to the kiosk network config screen. I'm not sure what it's default is though, on the test domain I was using the network screen was readily accessible.

We should get this fixed ASAP and merge the fix back to stable. Assigning to xiyuan@ to handle logistics.
 
FWIW, here is a page using an untrusted root that you can use for testing: https://cacert.org
Cc: rsleevi@chromium.org
Some additional considerations:

1. Can we somehow identify manually-imported certificates via this bug? It'd be prudent to remove them on devices out there. That might conflict with deployments thought that have accidentally come to rely on this (I hope there are none).

2. Even if you remove the imported certs, existing cache contents and profile state may remain compromised. Not much you can do about that than wiping all the Kiosk app profiles.

Also cc'ing rsleevi@ FYI cause broken HTTPS and he might have useful input.
Project Member

Comment 3 by sheriffbot@chromium.org, May 9 2017

Labels: M-59
Cc: mattm@chromium.org
We should be able to purge all NSS trust settings not in the read-only nssckbi for Kiosk profiles. The 'simplest' way is simply to purge the profile ~/.pki directory - that will delete all private keys, certificates, and trust settings imported into the Kiosk mode. If you do that before Chrome starts (or, ideally, with every launch of kiosk mode before Chrome starts), that will ensure it resorts to the 'known good' config (namely, the one in nssckbi.so on disk).

Note you also want to ensure the Chrome process itself is relaunched.
Cc: r...@chromium.org
+rkc

The policy to turn off network config is prompt_for_network_when_offline [1]. The default is "true". 

How does the following plan sound?
- Change the policy default to "false" so that admin has to explicitly
  turn it on.
- Remove the "Authority" tab in kiosk cert manager
  (what else should we disable there?)
- Following #4 to fix the existing compromised nss trust.

[1]: https://cs.chromium.org/chromium/src/chrome/browser/chromeos/policy/proto/chrome_device_policy.proto?rcl=1cd6a2975ed6d43005f31dcb84044a421628319f&l=388

Comment 6 by r...@chromium.org, May 10 2017

Cc: tbarzic@chromium.org

Comment 7 by r...@chromium.org, May 10 2017

sgtm - do you need Toni or I to help here Xiyuan?

Plan per #5 sounds good for the most part.

Note that when purging, you need to be careful to not accidentally purge client keys and the corresponding certificates. The keys are maintained by chapsd if I'm not mistaken, so should be fine. I'm not so sure about the certs though, so you'll want to test that.

rsleevi@: Do we also need to remove the "servers" tab? IIUC, this is only concerned with intermediate certs (but if that's correct, why do we have it at all?), so that should be fine?
Cc: dlt@google.com dconnolly@google.com
@Comment 8: "servers" tab is about direct connections to server certificates (e.g. bypassing verification). "authorities" tab is about certificate-issuing-certificates. Yes, you'd want to remove both.

Is there any reason you'd want to have _any_ of those tabs? It might be easier to avoid showing the whole UI?

I'm also not sure I understood what you meant about 'accidentally' purging client keys/certs. The removal of ~/.pki will purge everything from the profile - trust settings, yes, but also any client certs and (not bound to TPM) client keys.

Perhaps if you could explain what functionality, from the certificates side, is expected to work, and what is expected to be prevented, we can figure out how to get there, since it sounds like you're saying removing ~/.pki (which I naievely understood to be the desired end state) isn't right.
Re #10: The reason why the cert manager UI was exposed for Kiosk sessions in the first place was to serve use cases such as EAP-TLS where you need to import client certs. In an email conversation, Zel suggests that the party that brought up this use case actually didn't end up using the feature. It's hard to tell though in the absence of UMA (or do we have any?) to tell whether this is actively used.
I'm not sure of any UMA on the Chrome side, sadly. That does mean it'll be much more difficult to get this to a cleaned-up state (and makes me much more nervous about bugs/testing)
Stuff like #of installed CA roots, private keys, certs, some indication on whether they see any would probably be pretty useful, regardless of the Kiosk use case. Do we have a bug to add UMA stats on cert usage? Or should we file one?

Comment 14 by r...@chromium.org, May 10 2017

Cc: sduraisamy@chromium.org
Cc: abodenha@chromium.org
Cc: binzhao@chromium.org
If I understand this discussion correctly, purging the cert directory for kiosk mode (with out getting some stats) and removing the option to display'cert-manager' and import certs without providing alternatives may lead to customer support requests / complaints.

Please see our chrome deployment guide here - https://docs.google.com/document/d/17GV-4kYC2POQ2k5jpckj8izX0VbUxftpLNiiypSA4Rg/edit#heading=h.xd7zp9r53o60

I believe our deployment teams are recommending Ctrl+Alt+N to import certs for Kiosk mode and I think there may be some who are actively using imported certs in Kiosk mode (probably some in EDU as well)

I also think prompt_for_network_when_offline policy is not exposed in CPanel. Bin, can you confirm?

One option would be (to minimize any impact) -

* To enable a policy to push certs from CPanel in Kiosk mode - https://docs.google.com/document/d/1FbgdCh-F0pVq210OJfahU-GpHINmMBe2ls-cRGDb4nY/edit
* Expose UI for prompt_for_network_when_offline
* Remove the "servers" and "authorities" tab

I totally understand that this is something that we need to address quickly but I am trying to suggest ways to minimize the impact.

I checked and prompt_for_network_when_offline is not exposed in CPanel.
Let's keep the scope of this bug just wide enough to fix the security issue and avoid waiting on other features or CPanel changes.

Thus, the plan in comment #5 still seems the way to go - with the caveat that we need to figure out a way to clear out CA certs but keep client certificates and private keys.
Cc: atwilson@chromium.org
+ drew, fyi
Project Member

Comment 20 by sheriffbot@chromium.org, May 25 2017

xiyuan: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 21 by sheriffbot@chromium.org, Jun 8 2017

xiyuan: Uh oh! This issue still open and hasn't been updated in the last 28 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 22 by bugdroid1@chromium.org, Jun 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/208cf98ca5b1b2bdfdb620b68d20af6109a23c79

commit 208cf98ca5b1b2bdfdb620b68d20af6109a23c79
Author: xiyuan <xiyuan@chromium.org>
Date: Tue Jun 13 19:50:00 2017

kiosk: Tighten cert manager UI

- Make prompt_for_network_when_offline default to false;
- Hide "Servers" and "Authorities" tab in kiosk cert manager;

BUG= 719907 
TEST=CertificateManagerStandaloneWebUITest.testCertsDisplaying
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2920253003
Cr-Commit-Position: refs/heads/master@{#479108}

[modify] https://crrev.com/208cf98ca5b1b2bdfdb620b68d20af6109a23c79/chrome/browser/chromeos/login/app_launch_controller.cc
[modify] https://crrev.com/208cf98ca5b1b2bdfdb620b68d20af6109a23c79/chrome/browser/chromeos/policy/proto/chrome_device_policy.proto
[modify] https://crrev.com/208cf98ca5b1b2bdfdb620b68d20af6109a23c79/chrome/browser/resources/options/certificate_manager.js

Project Member

Comment 23 by sheriffbot@chromium.org, Jul 26 2017

Labels: -M-59 M-60
Cc: michae...@chromium.org
It turns out that network config UI is needed (e.g. configure wifi network in the field). We should handle the cert manager separately, maybe via a new policy.
Project Member

Comment 25 by bugdroid1@chromium.org, Aug 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4226f48ce099df93e1f3e4c05a985c2ac094cc38

commit 4226f48ce099df93e1f3e4c05a985c2ac094cc38
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Tue Aug 08 18:40:32 2017

kiosk: Restore prompt_for_network_when_offline default to true

Partial revert of https://codereview.chromium.org/2920253003
to restore the default of prompt_for_network_when_offline policy.

BUG= 752705 , 719907 

Change-Id: Idfa0b4f7df100b7569de9d32aa87fffd0dde5550
Reviewed-on: https://chromium-review.googlesource.com/606708
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492710}
[modify] https://crrev.com/4226f48ce099df93e1f3e4c05a985c2ac094cc38/chrome/browser/chromeos/login/app_launch_controller.cc
[modify] https://crrev.com/4226f48ce099df93e1f3e4c05a985c2ac094cc38/chrome/browser/chromeos/policy/proto/chrome_device_policy.proto

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 8 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d83a85e0407f159d68b4097da3cce4962a7b5527

commit d83a85e0407f159d68b4097da3cce4962a7b5527
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Tue Aug 08 19:25:12 2017

Merge "kiosk: Restore prompt_for_network_when_offline default to true"

> Partial revert of https://codereview.chromium.org/2920253003
> to restore the default of prompt_for_network_when_offline policy.
>
> BUG= 752705 , 719907 
>
> Change-Id: Idfa0b4f7df100b7569de9d32aa87fffd0dde5550
> Reviewed-on: https://chromium-review.googlesource.com/606708
> Reviewed-by: Toni Barzic <tbarzic@chromium.org>
> Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#492710}
> (cherry picked from commit 4226f48ce099df93e1f3e4c05a985c2ac094cc38)

TBR=xiyuan@chromium.org

Change-Id: I20a6f95eff273e36b587c4d8238352c297271d62
Reviewed-on: https://chromium-review.googlesource.com/606703
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#387}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/d83a85e0407f159d68b4097da3cce4962a7b5527/chrome/browser/chromeos/login/app_launch_controller.cc
[modify] https://crrev.com/d83a85e0407f159d68b4097da3cce4962a7b5527/chrome/browser/chromeos/policy/proto/chrome_device_policy.proto

Project Member

Comment 27 by sheriffbot@chromium.org, Aug 9 2017

Status: Fixed (was: Assigned)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 28 by sheriffbot@chromium.org, Aug 10 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-60 -merge-merged-3163
Status: Assigned (was: Fixed)
Reopen since the work is not done. We need to figure out the right policy and cpanel change to disable the cert manager UI. Using prompt_for_network_when_offline policy is an overkill because it is not just cert manager related. It is needed for configuring network in the field.

We would need a new policy to just control cert manager UI and corresponding cpanel change to set the policy.

Removing M60 tag since it would not be in for M60.
Project Member

Comment 30 by sheriffbot@chromium.org, Aug 16 2017

Labels: M-61
(Security sheriff rotation ping) Are there any updates here? Has the part of this bug that involved the security issue resolved and if so should we file a new bug instead?
Project Member

Comment 32 by sheriffbot@chromium.org, Oct 18 2017

Labels: -M-61 M-62
xiyuan: Is my understanding correct that the immediate risk is mitigated by hiding the Servers and Authorities tabs? If so, please close this security bug and file a bug for any follow-up work that is needed.
I need to create a CL to hide those in the new MD cert manager dialog webui.

Future work would include:
- Having a separate policy to control just the cert dialog
- Mitigate the nss db and remove bad certs
Project Member

Comment 35 by sheriffbot@chromium.org, Dec 7 2017

Labels: -M-62 M-63
Hi folks, this seems to have fallen through the cracks.

Xiyuan, has the CL mentioned in c#33 been uploaded?
Uploaded the CL to hide the two tabs in new MD cert manager UI:
https://chromium-review.googlesource.com/c/chromium/src/+/820638
Status: Started (was: Assigned)
Thanks Xiyuan!
Project Member

Comment 39 by bugdroid1@chromium.org, Dec 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0bb5b86a7061d895e90353d81392c2d73313b0d2

commit 0bb5b86a7061d895e90353d81392c2d73313b0d2
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Tue Dec 12 00:27:47 2017

kiosk: Tighten cert manager UI

- No "Servers" and "Authorities" tab;
- No "Import" as in previous non-MD impl;

Bug:  719907 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ie6fc8e7b257937b4e0607e426d9b0550dc2ddd4b
Reviewed-on: https://chromium-review.googlesource.com/820638
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523267}
[modify] https://crrev.com/0bb5b86a7061d895e90353d81392c2d73313b0d2/chrome/browser/ui/webui/chromeos/certificate_manager_dialog_ui.cc
[modify] https://crrev.com/0bb5b86a7061d895e90353d81392c2d73313b0d2/ui/webui/resources/cr_components/certificate_manager/certificate_list.js
[modify] https://crrev.com/0bb5b86a7061d895e90353d81392c2d73313b0d2/ui/webui/resources/cr_components/certificate_manager/certificate_manager.html
[modify] https://crrev.com/0bb5b86a7061d895e90353d81392c2d73313b0d2/ui/webui/resources/cr_components/certificate_manager/certificate_manager.js

Status: Fixed (was: Started)
Marking fixed as per c#33. Xiyuan, what would be the remaining work here?

Mattias, do you want to get this merged back?
The remaining work is listed in #34 and I filed issue 794188 for them.
Labels: -M-63 M-65
Project Member

Comment 43 by sheriffbot@chromium.org, Mar 21 2018

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Verified (was: Fixed)

Sign in to add a comment