secboot: update tpm connection error handling #8978
Conversation
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
When the TPM device is not available, snapcore/secboot now returns ErrNoTPM2Device instead of a path error, and the new IsEnabled() call allows us to check if the TPM device is functional (there are cases when the storage and endorsement hierarchies are disabled in the platform but the TPM device is still visible to the OS). Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
cmatsuoka
force-pushed
the
uc20-secboot-update-tpm-connection
branch
from
342e5f7
to
4a27782
Compare
There was a problem hiding this comment.
I'm not following TPM stuff closely but this looks good.
My only comment is that the error string that mentions a visible but not-enabled TPM should be curated more, so that it is clear what is going on. The user will not be familiar with enabled or disabled TPM most likely so I would use language that explains that the TPM was found but is unsable for a specific reason.
| // TPM connection error | ||
| {tpmErr: tpmErr, sbData: sbEnabled, err: "cannot connect to TPM device: TPM error"}, | ||
| // TPM was detected but it's not enabled | ||
| {tpmErr: nil, tpmEnabled: false, sbData: sbEnabled, err: "TPM device is not enabled"}, |
There was a problem hiding this comment.
Should we have a test case with sb.ErrNoTPM2Device to see if it's surfaced up?
| mockModel := &asserts.Model{} | ||
|
|
||
| mockTpm, restore := mockSbTPMConnection(c, tc.tpmErr) | ||
| defer restore() | ||
|
|
||
| restore = secboot.MockIsTPMEnabled(func(tpm *sb.TPMConnection) bool { | ||
| return tc.tpmEnabled |
There was a problem hiding this comment.
Wondering whether it makes sense to verify that this was never called when ErrNoTPM2Device was returned.
There was a problem hiding this comment.
This case would be a programming error where we call tpm.IsEnabled() for tpm == nil and we would see many tests crashing.
When verifying if key sealing is supported, also check for the case where we don't have a TPM2 device. Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
The current error message states that it's "detected but not enabled", which is indeed simplistic, however the full reason "is visible to the operating system but has the storage and endorsement hierarchies disabled by the platform" may add too much information, I'm not sure if both hierarchies are necessarily disabled in this case, or if another feature disabled in the platform could also cause IsEnabled() to fail. |
When the TPM device is not available, snapcore/secboot now returns
ErrNoTPM2Device instead of a path error. When the TPM device is
available, the new IsEnabled() call allows us to check if the TPM device
is fully functional (there are cases when the storage and endorsement
hierarchies are disabled in the platform but the TPM device is still
visible to the OS).
Signed-off-by: Claudio Matsuoka claudio.matsuoka@canonical.com