-
Notifications
You must be signed in to change notification settings - Fork 43
Description
Highlevel problem description
I discovered a problem, that TPM2-TSS keys which have been created with the tpm2-openssl provider with a version lower than 1.2.0 or with the tpm2-tss-engine with a version lower than 1.2.0-rc0 can no longer be loaded with the current provider implementation.
Code that triggers the problem
/* Set TPM-based key */
OSSL_PROVIDER *prov_tpm2 = NULL;
OSSL_PROVIDER *prov_default = NULL;
OSSL_STORE_CTX *ctx = NULL;
OSSL_STORE_INFO *info = NULL;
EVP_PKEY *pKey = NULL;
UI_METHOD *ui_method = NULL;
/* Load default provider */
if ((prov_default = OSSL_PROVIDER_load(NULL, "default")) == NULL) {
perror("Unable to load OpenSSL provider: default.");
exit(EXIT_FAILURE);
}
/* Self-test */
if (!OSSL_PROVIDER_self_test(prov_default)) {
perror("OpenSSL provider (default) self test failed.");
exit(EXIT_FAILURE);
}
/* Load TPM2 provider */
if ((prov_tpm2 = OSSL_PROVIDER_load(NULL, "tpm2")) == NULL) {
perror("Unable to load OpenSSL provider: tpm2.");
exit(EXIT_FAILURE);
}
/* Self-test */
if (!OSSL_PROVIDER_self_test(prov_tpm2)) {
perror("OpenSSL provider (tpm2) self test failed.");
exit(EXIT_FAILURE);
}
if (!(ui_method = UI_UTIL_wrap_read_pem_callback(provide_password, 0))) {
perror("setting ui_method failed");
exit(EXIT_FAILURE);
}
const char *password = "mysupersecretpassword";
if ((ctx = OSSL_STORE_open_ex("mytpm2tsskey.pem", NULL, "?provider=tpm2", ui_method,
(void *)password, NULL, NULL, NULL)) ==
NULL ||
!OSSL_STORE_expect(ctx, OSSL_STORE_INFO_PKEY) ||
(info = OSSL_STORE_load(ctx)) == NULL ||
(pKey = OSSL_STORE_INFO_get1_PKEY(info)) == NULL) {
perror("OpenSSL provider (tpm2) key loading failed.");
exit(EXIT_FAILURE);
}
OSSL_STORE_close(ctx);
I'm always getting:
OpenSSL provider (tpm2) key loading failed.
Because OSSL_STORE_load(ctx) returns NULL.
Analysis
The problem actually arises when reading the parent key handle from the ASN1 structure
tpm2-openssl/src/tpm2-provider-pkey.c
Line 138 in 8a55726
| if (!ASN1_INTEGER_get_uint64(&parent, tpk->parent)) |
The error has been introduced when fixing #74.
Prior to the fix, the parent handle has been saved using ASN1_INTEGER_set().
tpm2-openssl/src/tpm2-provider-pkey.c
Line 79 in 611facf
| ASN1_INTEGER_set(tpk->parent, keydata->parent); |
keydata->parent is of type TPM2_HANDLE, which is an uint32_t
https://github.com/tpm2-software/tpm2-tss/blob/bd67f321440cabfe1a8b7bbef416e4d4d0c54655/include/tss2/tss2_tpm2_types.h#L1049
ASN1_INTEGER_set() treats the parameter as (signed!) long: https://docs.openssl.org/3.6/man3/ASN1_INTEGER_get_int64. This resulted in the problems described in #74: On a 32-bit system the MSB was set and therefore the value was considered to be negative.
This problem was fixed by calling ASN1_INTEGER_set_uint64 and ASN1_INTEGER_get_uint64: ff40b6f
This works perfectly well when writing and reading new keys. When a key generated using the previous implementation should be loaded, it fails, due to the set bit V_ASN1_NEG in the type
https://github.com/openssl/openssl/blob/134f17d526a5d0a9fbd66adf85e53df8a764a2ff/crypto/asn1/a_int.c#L388
The initial problem was also discovered in the tpm2-tss-engine and has been fixed in version 1.2.0-rc0
tpm2-software/tpm2-tss-engine#222
If you read through the threads you'll see that @AndreasFuchsTPM was making sure backwards compatibility was given.
Additionally the problem has also been addressed in the tpm2-tools:
tpm2-software/tpm2-tools@5e68544
Fix
I created a MR with a potential fix. I don't have the environment to test all possible cases and it would definitely be helpful to have tests which cover all the cases. This seems to have been done for the engine. I see this task in the hands of the maintainers.
I decided to go with the same principle used by the engine and the tpm2-tools by using BIGNUM for serialization and deserialization - for the sake of harmonization and maintainability (do the same thing the same way in all of the projects).
It would also be possible to solve the issue by keeping the serialization using uint64. This would result in a manual check in the provider is V_ASN1_NEG is set. And then conditionally call ASN1_INTEGER_get_uint64 or ASN1_INTEGER_get.