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

Issue 722261 link

Starred by 3 users

Issue metadata

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

Blocked on:
issue 728130


Show other hotlists

Hotlists containing this issue:
Hotlist-1
Hotlist-1


Sign in to add a comment

Security: RSA key generation weakness in certain TPM models

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

Issue description

We have received a report of a weakness in RSA key generation taking place within certain TPM models. Details are still somewhat unclear, however the weakness likely allows an attack to recovery the private half of the RSA key under somewhat realistic resource budgets.

In terms of technical steps to recover from this, we'll need to design a process to update TPM firmware. There are a number of complications with this:
 1. Assuming the firmware upgrade path relies on TPM_FieldUpgrade, we'll require owner authorization, which we don't have in steady state. This implies we need to go through a TPM reset.
 2. Resetting the TPM will destroy all secrets that a TPM currently holds. This means that we'll lose all user data encryption keys, but also other keys, such as attestation identity keys. We'll have to figure out a comprehensive list.

Thus, at first look it sounds like we need to go through a TPM reset. We need to figure out whether and how we can mitigate the UX implications of doing so. The naive approach would looks like what you get with crossystem clear_tpm_owner_request=1, which means that we'll lose all state, including encrypted stateful, encrypted user data, etc. We may be able to smoothen this for certain classes of devices (e.g. devices used only by a single user, kiosk devices, etc.), but we'll have to work through the cases in detail to see what's possible.
 
Project Member

Comment 1 by sheriffbot@chromium.org, May 15 2017

Labels: M-58
Components: OS>Hardware>Firmware

Comment 3 by aarya@google.com, May 15 2017

Cc: -apronin@chromium.org
Owner: apronin@chromium.org
Status: Assigned (was: Unconfirmed)
apronin@, can you please help to find an owner.
Yes this is unpleasant.  This choice came up in the past when daisies started bricking at a relatively high rate, and we had these alternatives: 1. try to slow down the bricking rate with a couple of mitigations, and 2. essentially eliminate bricking with a change that required a TPM owner clear (and thus lost all local data).

We ended up doing just #1 and in the end it seemed to work OK.  Many people were very vocal against #2 (IIRC Cyrus was among them) so if we go that way we'll need some clear UI that explains what goes on and why.  There is also the issue of whether users should have a say in choosing inconvenience vs. insecurity (i.e. we think we understand the danger better than users do, but at the same time we don't want to be fascist).  So this is a juicy bug.

Cc: rbdebeer@google.com
Cc: janschej...@google.com
Project Member

Comment 7 by sheriffbot@chromium.org, May 29 2017

apronin: 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
Status: Started (was: Assigned)
Setting status to Started - all the action is happening on other bugs/documents ;-)
Blockedon: 728130
Project Member

Comment 10 by sheriffbot@chromium.org, Jun 6 2017

Labels: -M-58 M-59
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/cd01872c8126e350c94f13bfed896f49b075e573

commit cd01872c8126e350c94f13bfed896f49b075e573
Author: Andrey Pronin <apronin@chromium.org>
Date: Fri Jun 09 16:16:28 2017

cryptohome: use scrypt for key derivation

To address the scenarios outlined in chromium:454638

1) Use scrypt instead of EVP_BytesToKey to derive AES key used in
EVKK <-> IEVKK transition for tpm-wrapped keys.

2) Use scrypt instead of EVP_BytesToKey to derive VKK key and IV.

BUG= chromium:454638 
BUG= chromium:722261 
TEST=1) On a machine with existing users with tpm-wrapped keys, deploy and
        restart new cryptohomed, verify that the new derivation approach is
        used and login still succeeds.
     2) Create new users on a machine with the new derivation approach.
     2) Unit tests.

Change-Id: I46ae4e0a94e07ad7ed35af45c0652734cb8e58aa
Reviewed-on: https://chromium-review.googlesource.com/516805
Commit-Ready: Mattias Nissler <mnissler@chromium.org>
Tested-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/cd01872c8126e350c94f13bfed896f49b075e573/cryptohome/mount_unittest.cc
[modify] https://crrev.com/cd01872c8126e350c94f13bfed896f49b075e573/cryptohome/mount.cc
[modify] https://crrev.com/cd01872c8126e350c94f13bfed896f49b075e573/cryptohome/crypto_unittest.cc
[modify] https://crrev.com/cd01872c8126e350c94f13bfed896f49b075e573/cryptohome/vault_keyset.proto
[modify] https://crrev.com/cd01872c8126e350c94f13bfed896f49b075e573/cryptohome/crypto.h
[modify] https://crrev.com/cd01872c8126e350c94f13bfed896f49b075e573/cryptohome/crypto.cc

Cc: mxt@chromium.org
Project Member

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

Labels: merge-merged-release-R60-9592.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/fcc713a899f7165b1bbc3bfdf21efeb4d51a4be2

commit fcc713a899f7165b1bbc3bfdf21efeb4d51a4be2
Author: Andrey Pronin <apronin@chromium.org>
Date: Thu Jun 15 16:59:05 2017

cryptohome: decrypt tpm+scrypt VKKs

Backport the ability to decrypt VKKs that use scrypt for keys derivation
from CL:516805. Unlike that CL, don't migrate VKKs to the new
scrypt-based method, but support decrypting VKKs encrypted with
that method.

Having the decrypt support for such keys allows the rollback path after
unsuccessful upgrades to revisions that implement the new key derivation
method, even after a user has logged in and migrated the key.

BUG= chromium:454638 
BUG= chromium:722261 
TEST=1) On a machine with existing users with tpm+scrypt VKKs, deploy and
        restart cryptohomed, verify that the tpm+scrypt keys are decrypted
        and login still succeeds.
     2) Unit tests.

Change-Id: I4c4bea519e5973c0e3c174583d09d6eb1f412fd1
Reviewed-on: https://chromium-review.googlesource.com/534599
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Tested-by: Mattias Nissler <mnissler@chromium.org>

[modify] https://crrev.com/fcc713a899f7165b1bbc3bfdf21efeb4d51a4be2/cryptohome/vault_keyset.proto
[modify] https://crrev.com/fcc713a899f7165b1bbc3bfdf21efeb4d51a4be2/cryptohome/crypto.h
[modify] https://crrev.com/fcc713a899f7165b1bbc3bfdf21efeb4d51a4be2/cryptohome/crypto.cc

Project Member

Comment 14 by sheriffbot@chromium.org, Jun 16 2017

Status: Fixed (was: Started)
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 15 by sheriffbot@chromium.org, Jun 17 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Cc: jarmour@chromium.org
Cc: c...@chromium.org
Cc: jpm@google.com
Cc: naveenv@chromium.org zalcorn@chromium.org
Tracking the merge of https://chromium-review.googlesource.com/c/516805 to M60 in issue 755159.
Project Member

Comment 21 by bugdroid1@chromium.org, Aug 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ee8ce6daa32c98dc098408671312c0896d1c4e08

commit ee8ce6daa32c98dc098408671312c0896d1c4e08
Author: Andrey Pronin <apronin@chromium.org>
Date: Tue Aug 15 17:03:37 2017

cryptohome: use scrypt for key derivation

To address the scenarios outlined in chromium:454638

1) Use scrypt instead of EVP_BytesToKey to derive AES key used in
EVKK <-> IEVKK transition for tpm-wrapped keys.

2) Use scrypt instead of EVP_BytesToKey to derive VKK key and IV.

BUG=chromium:755159
BUG= chromium:454638 
BUG= chromium:722261 
TEST=1) On a machine with existing users with tpm-wrapped keys, deploy and
        restart new cryptohomed, verify that the new derivation approach is
        used and login still succeeds.
     2) Create new users on a machine with the new derivation approach.
     2) Unit tests.

Change-Id: I46ae4e0a94e07ad7ed35af45c0652734cb8e58aa
Reviewed-on: https://chromium-review.googlesource.com/516805
Commit-Ready: Mattias Nissler <mnissler@chromium.org>
Tested-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
(cherry picked from commit cd01872c8126e350c94f13bfed896f49b075e573)
Reviewed-on: https://chromium-review.googlesource.com/615018
Reviewed-by: Andrey Pronin <apronin@chromium.org>
Commit-Queue: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/ee8ce6daa32c98dc098408671312c0896d1c4e08/cryptohome/mount.cc
[modify] https://crrev.com/ee8ce6daa32c98dc098408671312c0896d1c4e08/cryptohome/crypto_unittest.cc
[modify] https://crrev.com/ee8ce6daa32c98dc098408671312c0896d1c4e08/cryptohome/crypto.cc
[modify] https://crrev.com/ee8ce6daa32c98dc098408671312c0896d1c4e08/cryptohome/mount_unittest.cc

Project Member

Comment 22 by sheriffbot@chromium.org, Sep 23 2017

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
Labels: Restrict-View-SecurityTeam
Labels: -Restrict-View-SecurityTeam -allpublic Restrict-View-Google
Status: Assigned (was: Fixed)
Labels: -Restrict-View-Google Restrict-View-SecurityTeam
Cc: mruhstaller@google.com
Labels: Restrict-View-Google
Cc: kberdan@google.com
Project Member

Comment 29 by sheriffbot@chromium.org, Sep 24 2017

Labels: -M-59 M-61
apronin: Uh oh! This issue still open and hasn't been updated in the last 132 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 30 by sheriffbot@chromium.org, Sep 24 2017

Labels: Deadline-Exceeded
We commit ourselves to a 60 day deadline for fixing for high severity vulnerabilities, and have exceeded it here. If you're unable to look into this soon, could you please find another owner or remove yourself so that this gets back into the security triage queue?

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: ewettstein@google.com
Labels: Restrict-View-SecurityEmbargo
Project Member

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

Labels: -M-61 M-62
Cc: shurst@google.com
Labels: -Restrict-View-Google -Restrict-View-SecurityEmbargo
Status: Fixed (was: Assigned)
I'm going to mark this fixed and public since this is no longer sensitive and we're tracking the remaining follow-up work elsewhere.
Cc: puneetster@google.com

Comment 37 by evn@google.com, Jan 29 2018

Hi

Did you mean to remove the Restrict-View-SecurityTeam label?
Project Member

Comment 38 by sheriffbot@chromium.org, Feb 19 2018

Labels: -Restrict-View-SecurityTeam 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

Sign in to add a comment