New issue
Advanced search Search tips

Issue 847417 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Dec 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Handle corner case of bad TPM 2.0 NVRAM spaces correctly in mount-encrypted

Project Member Reported by mnissler@chromium.org, May 29 2018

Issue description

The NVRAM space for that contains the system key for stateful encryption doesn't behave properly for the following edge cases:

1. Space isn't defined with the correct attributes.
2. Space is defined but not yet written.

We should adjust mount-encrypted to detect and handle these cases gracefully. We can either try and fix things (attempt to undefine the space and redefine it) or bail out and surface the fatal error to the user (+ change the recovery tool to fix the bad space). The latter might be required regardless for cases where we can't fix the space in normal mode and this situation does pop up in practice.

Andrey, let me know what you think.
 
Correction: Space defined by not yet written is handled, but the error code we receive from the TPM on read might not be processed correctly. Need to test against an actual device and adjust the unit test accordingly.
Case #1 is actually already covered by the existing issue 840361, so no longer relevant here.

Case #2 is already working correctly, i.e. the NVRAM space read fails. That failure doesn't cause bad behavior though as we also check the NVRAM space attributes to test for space presence and do perform a write. I verified this behavior on bob. The only thing that was inconsistent is the existing test case which did incorrectly generate TPM_E_BADINDEX for unwritten spaces. This is fixed by https://chromium-review.googlesource.com/#/c/chromiumos/platform2/+/1080627
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 5 2018

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

commit 4c4c7f87fb734a9ec1fd2a1e3bc9c167c2844d3a
Author: Mattias Nissler <mnissler@chromium.org>
Date: Tue Jun 05 00:23:47 2018

cryptohome: Fix unwritten NVRAM space test in mount-encrypted

Tlcl actually doesn't return TPM_E_BADINDEX on unwritten TPM 2.0 NVRAM
spaces. Fix the fake implementation to not return that error code. It
should ideally return TPMA_RC_NV_UNINITIALIZED matching TPM behavior,
but since that code is not declared and illegal to use by code calling
into Tlcl, just return TPM_E_INTERNAL_ERROR as we already do for
similar situations.

BUG= chromium:847417 
TEST=Compiles and passes tests.

Change-Id: Id0cf7cc204e3d16c5e14541bb67a542f7a4d3213
Reviewed-on: https://chromium-review.googlesource.com/1080627
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/4c4c7f87fb734a9ec1fd2a1e3bc9c167c2844d3a/cryptohome/mount_encrypted/tlcl_stub.cc

Components: OS>Systems>Security
Status: Fixed (was: Available)
IIUC this is fixed - Mattias please re-open if I've misunderstood

Sign in to add a comment