Conflicting double TPM connections when creating a new passphrase protected key #353
Assignees
Comments
chrisccoulson
added a commit
to chrisccoulson/secboot
that referenced
this issue
Jan 7, 2025
Secboot currently opens a transport to the Linux character device at a hardcoded path (/dev/tpm0) and passes the opened transport to the deprecated tpm2.NewTPMContext API. The replacement API (tpm2.OpenTPMDevice) makes use of a new abstraction (tpm2.TPMDevice). The linux sub-package provides a way of enumerating TPM devices or providing access to the default one. It also provides access to the resource managed device, which we might want to use in secboot in the future, and it provides access to the physical presence interface for performing actions that are implemented in platform firmware. Implementing this now is required to write test cases for canonical#353, as the current test harness will permit the code under test to open as many TPM connections as it likes, when in reality, it can only have a single open connection if it is using the direct character device (/dev/tpm0).
chrisccoulson
added a commit
to chrisccoulson/secboot
that referenced
this issue
Jan 7, 2025
Secboot currently opens a transport to the Linux character device at a hardcoded path (/dev/tpm0) and passes the opened transport to the deprecated tpm2.NewTPMContext API. The replacement API (tpm2.OpenTPMDevice) makes use of a new abstraction (tpm2.TPMDevice). The linux sub-package provides a way of enumerating TPM devices or providing access to the default one. It also provides access to the resource managed device, which we might want to use in secboot in the future, and it provides access to the physical presence interface for performing actions that are implemented in platform firmware. Implementing this now is required to write test cases for canonical#353, as the current test harness will permit the code under test to open as many TPM connections as it likes, when in reality, it can only have a single open connection if it is using the direct character device (/dev/tpm0).
chrisccoulson
added a commit
to chrisccoulson/secboot
that referenced
this issue
Jan 7, 2025
Secboot currently opens a transport to the Linux character device at a hardcoded path (/dev/tpm0) and passes the opened transport to the deprecated tpm2.NewTPMContext API. The replacement API (tpm2.OpenTPMDevice) makes use of a new abstraction (tpm2.TPMDevice). The linux sub-package provides a way of enumerating TPM devices or providing access to the default one. It also provides access to the resource managed device, which we might want to use in secboot in the future, and it provides access to the physical presence interface for performing actions that are implemented in platform firmware. Implementing this now is required to write test cases for canonical#353, as the current test harness will permit the code under test to open as many TPM connections as it likes, when in reality, it can only have a single open connection if it is using the direct character device (/dev/tpm0).
chrisccoulson
added a commit
to chrisccoulson/secboot
that referenced
this issue
Jan 7, 2025
Secboot currently opens a transport to the Linux character device at a hardcoded path (/dev/tpm0) and passes the opened transport to the deprecated tpm2.NewTPMContext API. The replacement API (tpm2.OpenTPMDevice) makes use of a new abstraction (tpm2.TPMDevice). The linux sub-package provides a way of enumerating TPM devices or providing access to the default one. It also provides access to the resource managed device, which we might want to use in secboot in the future, and it provides access to the physical presence interface for performing actions that are implemented in platform firmware. Implementing this now is required to write test cases for canonical#353, as the current test harness will permit the code under test to open as many TPM connections as it likes, when in reality, it can only have a single open connection if it is using the direct character device (/dev/tpm0).
chrisccoulson
added a commit
to chrisccoulson/secboot
that referenced
this issue
Jan 7, 2025
Secboot currently opens a transport to the Linux character device at a hardcoded path (/dev/tpm0) and passes the opened transport to the deprecated tpm2.NewTPMContext API. The replacement API (tpm2.OpenTPMDevice) makes use of a new abstraction (tpm2.TPMDevice). The linux sub-package provides a way of enumerating TPM devices or providing access to the default one. It also provides access to the resource managed device, which we might want to use in secboot in the future, and it provides access to the physical presence interface for performing actions that are implemented in platform firmware. Implementing this now is required to write test cases for canonical#353, as the current test harness will permit the code under test to open as many TPM connections as it likes, when in reality, it can only have a single open connection if it is using the direct character device (/dev/tpm0).
chrisccoulson
added a commit
that referenced
this issue
Jan 8, 2025
tpm2: Use tpm2.TPMDevice abstraction. Secboot currently opens a transport to the Linux character device at a hardcoded path (/dev/tpm0) and passes the opened transport to the deprecated `tpm2.NewTPMContext` API. The replacement API (`tpm2.OpenTPMDevice`) makes use of a new abstraction (`tpm2.TPMDevice`). The `linux` sub-package provides a way of enumerating TPM devices or providing access to the default one. It also provides access to the resource managed device, which we might want to use in secboot in the future, and it provides access to the physical presence interface for performing actions that are implemented in platform firmware. Implementing this now is required to write test cases for #353, as the current test harness will permit the code under test to open as many TPM connections as it likes, when in reality, it can only have a single open connection if it is using the direct character device (/dev/tpm0).
|
I'll be working on this now #357 has landed, which was a pre-requisite for this. |
chrisccoulson
added a commit
that referenced
this issue
Jan 28, 2025
…ctions tpm2: Avoid attempting to open duplicate TPM connections. When using `NewTPMPassphraseProtectedKey`, it is passed an already open connection. But the core secboot package calls back into the platform handler to set the user auth value, which opens a second connection. When using the direct device (/dev/tpm0), this doesn't work. This updates the test suite to make sure that this scenario is caught and results in an error. It also fixes the problem by providing a way to pass the open TPM connection to the singleton tpm2 platform handler via a new field on the `secboot.KeyWithPassphraseParams` struct which can be set to any value and is passed to the `ChangeAuthKey` implementation when setting the initial auth value. The tpm2 platform handler implementation uses this connection if passed rather than trying to open a new connection. If the `ChangeAuthKey` implementation is called via `KeyData.ChangePassphrase`, then the new argument will be nil and the tpm2 implementation will open a new TPM connection as required. This also identified a bug that the core secboot package was not passing the role value to the platform handler, which only seemed to be caught by adding tests that created passphrase protected TPM keys with a role. This addresses issue #353
|
This is fixed since #360 landed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
NewTPMPassphraseProtectedKeytakes a tpm connection as input, but internally the constructor ends up callingChangeAuthKeyon the tpm platform handler which tries to create a new TPM connection causing an error due trying to open two connection for the same TPM device.My initial thoughts was to override
ConnectToTPMto reuse the opened tpm connection and restore it whenNewTPMPassphraseProtectedKeycompletes and it works ZeyadYasser@56aea41 as expected, but unfortunately this approach is not concurrent-safe and given that there could be multiple ongoing snapd changes at the same time, it doesn't look like a viable solution.The text was updated successfully, but these errors were encountered: