Issue metadata
Sign in to add a comment
|
Security: Interrupted TPM firmware update doesn't clear out weak SRK |
||||||||||||||||||||||
Issue description(Spin-off originally discovered per issue 854576 ) Hypothetical repro (to be confirmed): 0. Find device that doesn't have TPM firmware upgraded. 1. Trigger TPM firmware update via powerwash dialog from login screen. 2. At the installation progress screen, force EC reset using power+refresh 3. Observe the device hitting "Chrome OS is missing or damaged" 4. Supply recovery media and have the installer retry the TPM firmware update. Let it complete this time. 5. Reboot into the system to OOBE. 6. Verify that there's a log message from cryptohomed in /var/log/messages, saying "Configuring TPM, ownership taken: 0." This implies that the SRK which was created when ownership was taken by the firmware updater has not been cleared out yet. Thus, the system would be running with fixed firmware, but a weak SRK. Assigning to apronin@ in the hope he can confirm (I no longer have any non-updated devices at hand). As a result, we're might have a couple users out there that have updated, but are still running with weak SRKs. The number of devices affected is relatively small. Per UMA data at https://uma.googleplex.com/p/chrome/timeline_v2/?sid=5ed6f81cae80365c250a93f4429cb10e we're seeing in the order of device per day that comes back after interrupted firmware update, so we're talking about a few hundreds of devices. We should obviously fix this. The solution discussed in issue 854576 should solve this as well, i.e. checking that we always come back with a clear TPM after a firmware update. The question is what should happen to the few devices ending up in the unfortunate state. With a bit of work we can detect the situation. Should we notify users?
,
Jul 2
,
Jul 3
,
Jul 3
,
Jul 3
Confirmed 'ownership taken: 0' as a result of taking steps 1-6 on auron-yuna, 68.0.3440.40/10718.34.0
,
Jul 3
Thanks Andrey for confirming my suspicion. We'll need to fix this ASAP then. https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1123830 should do the trick, and I'll request merge to M68 once we've landed and verified it (Andrey, I'll need your help to burn another device on that). The remaining question is how we handle updated devices with weak keys. Let me start a mail thread with Dan and Zach to discuss the product side of this.
,
Jul 3
Wrong Zach, sorry...
,
Jul 5
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/ae52c33eeb4dfc2b04d9db62935fb19719629bb5 commit ae52c33eeb4dfc2b04d9db62935fb19719629bb5 Author: Mattias Nissler <mnissler@chromium.org> Date: Thu Jul 05 22:14:37 2018 infineon-firmware-updater: Force TPM clear after update. We need to make sure the TPM doesn't hold on to an SRK that originates from before the TPM firmware update because that SRK might be weak. This can happen in edge cases, for example when we start the TPM firmware update using owner authorization but then get interrupted and perform a retry via recovery mode. This change adds post-update logic in the tpm-firmware-update.sh script which runs during boot when a TPM firmware update has been requested. It makes sure that we only continue booting if the TPM is clear and requests the TPM to be cleared if necessary. BUG= chromium:854576 , chromium:778332, chromium:859511 TEST=See bug. Change-Id: I091faf438e34c7c05944df1d26840367bc560fb7 Reviewed-on: https://chromium-review.googlesource.com/1123830 Commit-Ready: Mattias Nissler <mnissler@chromium.org> Tested-by: Mattias Nissler <mnissler@chromium.org> Reviewed-by: Mattias Nissler <mnissler@chromium.org> [rename] https://crrev.com/ae52c33eeb4dfc2b04d9db62935fb19719629bb5/chromeos-base/infineon-firmware-updater/infineon-firmware-updater-1.1.2459.0-r27.ebuild [modify] https://crrev.com/ae52c33eeb4dfc2b04d9db62935fb19719629bb5/chromeos-base/infineon-firmware-updater/files/tpm-firmware-update.sh
,
Jul 6
Passing ownership of this to apronin@ who kindly agreed to champion this while I'm on vacation for the next two weeks. What's left to do here: 1. Verify a build that has https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/ae52c33eeb4dfc2b04d9db62935fb19719629bb5 to not exhibit the bug, i.e. go through an interrupted life update per the repro steps. 2. Merge https://chromium-review.googlesource.com/1123830 to M68. Putting a ReleaseBlock-Stable label just in case (even though this is technically not a regression, I think it's important that we prevent further devices in the field to get into the bad state which requires a powerwash to fix).
,
Jul 11
CL from #8 landed in 10850.0.0. Verified that M69-10860.0.0 image does not exhibit the bug on auron-yuna and asuka: "ownership taken: 1" in both cases. (If you will be repeating the test, note that I filed issue 862429 , issue 862703 , issue 862677 while going through the repro steps on that build.)
,
Jul 11
This bug requires manual review: We are only 12 days from stable. Please contact the 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
,
Jul 12
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
,
Jul 13
,
Jul 16
,
Jul 16
,
Jul 18
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/1fe15bb74804db1a26905f213cd96cec27278e53 commit 1fe15bb74804db1a26905f213cd96cec27278e53 Author: Mattias Nissler <mnissler@chromium.org> Date: Wed Jul 18 01:30:26 2018 infineon-firmware-updater: Force TPM clear after update. We need to make sure the TPM doesn't hold on to an SRK that originates from before the TPM firmware update because that SRK might be weak. This can happen in edge cases, for example when we start the TPM firmware update using owner authorization but then get interrupted and perform a retry via recovery mode. This change adds post-update logic in the tpm-firmware-update.sh script which runs during boot when a TPM firmware update has been requested. It makes sure that we only continue booting if the TPM is clear and requests the TPM to be cleared if necessary. BUG= chromium:854576 , chromium:778332, chromium:859511 TEST=See bug. Change-Id: I091faf438e34c7c05944df1d26840367bc560fb7 Reviewed-on: https://chromium-review.googlesource.com/1123830 Commit-Ready: Mattias Nissler <mnissler@chromium.org> Tested-by: Mattias Nissler <mnissler@chromium.org> Reviewed-by: Mattias Nissler <mnissler@chromium.org> (cherry picked from commit ae52c33eeb4dfc2b04d9db62935fb19719629bb5) Reviewed-on: https://chromium-review.googlesource.com/1134158 Reviewed-by: Andrey Pronin <apronin@chromium.org> Tested-by: Andrey Pronin <apronin@chromium.org> Commit-Queue: Andrey Pronin <apronin@chromium.org> [modify] https://crrev.com/1fe15bb74804db1a26905f213cd96cec27278e53/chromeos-base/infineon-firmware-updater/files/tpm-firmware-update.sh [rename] https://crrev.com/1fe15bb74804db1a26905f213cd96cec27278e53/chromeos-base/infineon-firmware-updater/infineon-firmware-updater-1.1.2459.0-r26.ebuild
,
Jul 18
,
Oct 19
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 |
|||||||||||||||||||||||
Comment 1 by sheriffbot@chromium.org
, Jul 2