Enrollment lost after state-preserving TPM firmware update |
||||||
Issue descriptionRepro: 0. Get an image that includes the device state preserving TPM firmware update flow installed on a device that runs old TPM firmware, preferably one that has tpm_firmware_update_params=dryrun:1 set in VPD 1. Enterprise-enroll the device 2. Go through the state-preserving update flow (needs policy to be set, then trigger via chrome://chrome) 3. TPM firmware update flow completes successfully. 4. Observe that enrollment is lost (for example, no managed device string in system menu) The underlying problem is that mount-encrypted and cryptohomed decide tha the lockbox is invalid. This didn't happen in previous (uncommitted) iterations of the code since mount-encrypted always took TPM ownership on state preservation. Now that it's smarter, various if-tpm-not-owned-lockbox-must-be-invalid conditions trigger. These must be fixed up.
,
May 29 2018
Proposed CLs for fixing this (under review): https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1071528 https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1075487
,
May 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/ee62fccb826f843f9348d9af741ed1d87f8434a2 commit ee62fccb826f843f9348d9af741ed1d87f8434a2 Author: Mattias Nissler <mnissler@chromium.org> Date: Wed May 30 08:02:34 2018 cryptohome: Fix lockbox validity check in mount-encrypted mount-encrypted only considered the lockbox space valid when the TPM is owned. For stateful preservation, this will not necessarily be the case as we only take ownership if the encstateful NVRAM space must be defined. To decide lockbox validity properly, add a CheckLockbox function in SystemKeyLoader, so the TPM 1.2 implementation can take into account presence of a lockbox MAC in the encstateful space and declare the lockbox valid in case the MAC validates, even for the case where the TPM isn't owned yet. BUG= chromium:845885 TEST=Updated unit tests. Change-Id: Ib75a9d765e3dc50685851c96cdacb0380583fb69 Reviewed-on: https://chromium-review.googlesource.com/1071528 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/ee62fccb826f843f9348d9af741ed1d87f8434a2/cryptohome/mount_encrypted/encryption_key_unittest.cc [modify] https://crrev.com/ee62fccb826f843f9348d9af741ed1d87f8434a2/cryptohome/mount_encrypted.cc [modify] https://crrev.com/ee62fccb826f843f9348d9af741ed1d87f8434a2/cryptohome/mount_encrypted/tpm.h [modify] https://crrev.com/ee62fccb826f843f9348d9af741ed1d87f8434a2/cryptohome/mount_encrypted/tpm1.cc [modify] https://crrev.com/ee62fccb826f843f9348d9af741ed1d87f8434a2/cryptohome/mount_encrypted/tpm2.cc
,
May 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/473967ec1b04effde06908050ce614539e0f6dd1 commit 473967ec1b04effde06908050ce614539e0f6dd1 Author: Mattias Nissler <mnissler@chromium.org> Date: Wed May 30 08:02:34 2018 cryptohome: Check install attributes validity before reset. cryptohomed needs to clear out previous install attributes state after TPM reset. This happens after taking TPM ownership. However, when mount-encrypted preserves a previous encrytped stateful file system across TPM reset, install attributes must be carried forward as well. To handle this situation correctly, check for presence of the install attributes cache file, which will only be present if lockbox-cache successfully verified the install attributes against the lockbox. BUG= chromium:845885 TEST=See bug. Change-Id: I6e68e0764ee0163bab8e6aabff57457e628e1f23 Reviewed-on: https://chromium-review.googlesource.com/1075487 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/473967ec1b04effde06908050ce614539e0f6dd1/cryptohome/install_attributes.cc [modify] https://crrev.com/473967ec1b04effde06908050ce614539e0f6dd1/cryptohome/init/lockbox-cache.conf
,
May 31 2018
Verified fixed on 10738.0.0. cyan build. Filing merge request.
,
Jun 1 2018
Your change meets the bar and is auto-approved for M68. Please go ahead and merge the CL to branch 3440 manually. Please contact milestone owner if you have questions. Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/294d419cb4871b8611cc5c31aa317cd80b96a59d commit 294d419cb4871b8611cc5c31aa317cd80b96a59d Author: Mattias Nissler <mnissler@chromium.org> Date: Mon Jun 04 10:10:48 2018 cryptohome: Fix lockbox validity check in mount-encrypted mount-encrypted only considered the lockbox space valid when the TPM is owned. For stateful preservation, this will not necessarily be the case as we only take ownership if the encstateful NVRAM space must be defined. To decide lockbox validity properly, add a CheckLockbox function in SystemKeyLoader, so the TPM 1.2 implementation can take into account presence of a lockbox MAC in the encstateful space and declare the lockbox valid in case the MAC validates, even for the case where the TPM isn't owned yet. BUG= chromium:845885 TEST=Updated unit tests. Change-Id: Ib75a9d765e3dc50685851c96cdacb0380583fb69 Reviewed-on: https://chromium-review.googlesource.com/1071528 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> (cherry picked from commit ee62fccb826f843f9348d9af741ed1d87f8434a2) Reviewed-on: https://chromium-review.googlesource.com/1084827 Reviewed-by: Mattias Nissler <mnissler@chromium.org> Trybot-Ready: Mattias Nissler <mnissler@chromium.org> [modify] https://crrev.com/294d419cb4871b8611cc5c31aa317cd80b96a59d/cryptohome/mount_encrypted/encryption_key_unittest.cc [modify] https://crrev.com/294d419cb4871b8611cc5c31aa317cd80b96a59d/cryptohome/mount_encrypted.cc [modify] https://crrev.com/294d419cb4871b8611cc5c31aa317cd80b96a59d/cryptohome/mount_encrypted/tpm.h [modify] https://crrev.com/294d419cb4871b8611cc5c31aa317cd80b96a59d/cryptohome/mount_encrypted/tpm1.cc [modify] https://crrev.com/294d419cb4871b8611cc5c31aa317cd80b96a59d/cryptohome/mount_encrypted/tpm2.cc
,
Jun 4 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/45a515dba37dc06ed33a1794daff652674b21f90 commit 45a515dba37dc06ed33a1794daff652674b21f90 Author: Mattias Nissler <mnissler@chromium.org> Date: Mon Jun 04 10:11:08 2018 cryptohome: Check install attributes validity before reset. cryptohomed needs to clear out previous install attributes state after TPM reset. This happens after taking TPM ownership. However, when mount-encrypted preserves a previous encrytped stateful file system across TPM reset, install attributes must be carried forward as well. To handle this situation correctly, check for presence of the install attributes cache file, which will only be present if lockbox-cache successfully verified the install attributes against the lockbox. BUG= chromium:845885 TEST=See bug. Change-Id: I6e68e0764ee0163bab8e6aabff57457e628e1f23 Reviewed-on: https://chromium-review.googlesource.com/1075487 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> (cherry picked from commit 473967ec1b04effde06908050ce614539e0f6dd1) Reviewed-on: https://chromium-review.googlesource.com/1084828 Reviewed-by: Mattias Nissler <mnissler@chromium.org> Trybot-Ready: Mattias Nissler <mnissler@chromium.org> [modify] https://crrev.com/45a515dba37dc06ed33a1794daff652674b21f90/cryptohome/install_attributes.cc [modify] https://crrev.com/45a515dba37dc06ed33a1794daff652674b21f90/cryptohome/init/lockbox-cache.conf
,
Jun 4 2018
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. 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
,
Jun 8 2018
Merged to 68, verification pending.
,
Jun 11 2018
Verified on 10718.18.0-cyan to work as expected. Setting Verified since test team already did redundant verification as part of regular release testing per https://bugs.chromium.org/p/chromium/issues/detail?id=849222#c4 |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by mnissler@chromium.org
, May 23 2018