Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ TESTS_SHELL = test/list.sh \
test/rsa_genpkey_sign.sh \
test/rsa_genpkey_sign_rawin.sh \
test/rsa_genpkey_auth_parent.sh \
test/rsa_encodeobject_check.sh \
test/rsa_encodeobject_check_parent.sh \
test/rsa_createak_auth.sh \
test/rsa_createak_sign_object.sh \
test/rsa_createak_sign_handle.sh \
Expand Down
38 changes: 26 additions & 12 deletions src/tpm2-provider-pkey.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ int
tpm2_keydata_write(const TPM2_KEYDATA *keydata, BIO *bout, TPM2_PKEY_FORMAT format)
{
TSSPRIVKEY *tpk = NULL;
BIGNUM *bn_parent = NULL;
int res = 0;

uint8_t privbuf[sizeof(keydata->priv)];
uint8_t pubbuf[sizeof(keydata->pub)];
Expand All @@ -74,13 +76,18 @@ tpm2_keydata_write(const TPM2_KEYDATA *keydata, BIO *bout, TPM2_PKEY_FORMAT form
if (!tpk->type)
goto error;

// note the ASN1_INTEGER_set is not reliable for uin32_t on 32-bit machines
tpk->emptyAuth = ! !keydata->emptyAuth;

// note the ASN1_INTEGER_set is not reliable for uin32_t on 32-bit machines
if (!(bn_parent = BN_new()))
goto error;

if (keydata->parent != 0)
ASN1_INTEGER_set_uint64(tpk->parent, keydata->parent);
BN_set_word(bn_parent, keydata->parent);
else
ASN1_INTEGER_set_uint64(tpk->parent, TPM2_RH_OWNER);
BN_set_word(bn_parent, TPM2_RH_OWNER);

BN_to_ASN1_INTEGER(bn_parent, tpk->parent);
ASN1_STRING_set(tpk->privkey, &privbuf[0], privbuf_len);
ASN1_STRING_set(tpk->pubkey, &pubbuf[0], pubbuf_len);

Expand All @@ -95,11 +102,11 @@ tpm2_keydata_write(const TPM2_KEYDATA *keydata, BIO *bout, TPM2_PKEY_FORMAT form
goto error;
}

TSSPRIVKEY_free(tpk);
return 1;
res = 1;
error:
BN_free(bn_parent);
TSSPRIVKEY_free(tpk);
return 0;
return res;
}

/** Deserialize TPM2_KEYDATA from disk
Expand All @@ -114,9 +121,11 @@ tpm2_keydata_write(const TPM2_KEYDATA *keydata, BIO *bout, TPM2_PKEY_FORMAT form
int
tpm2_keydata_read(BIO *bin, TPM2_KEYDATA *keydata, TPM2_PKEY_FORMAT format)
{
uint64_t parent;
BIGNUM *bn_parent = NULL;
TPM2_HANDLE parent;
TSSPRIVKEY *tpk = NULL;
char type_oid[64];
int res = 0;

switch (format) {
case KEY_FORMAT_PEM:
Expand All @@ -135,9 +144,14 @@ tpm2_keydata_read(BIO *bin, TPM2_KEYDATA *keydata, TPM2_PKEY_FORMAT format)
keydata->emptyAuth = (tpk->emptyAuth != V_ASN1_UNDEF && tpk->emptyAuth);

// the ASN1_INTEGER_get on a 32-bit machine will fail for numbers of UINT32_MAX
if (!ASN1_INTEGER_get_uint64(&parent, tpk->parent))
if (!(bn_parent = ASN1_INTEGER_to_BN(tpk->parent, NULL)))
goto error;

if (BN_is_negative(bn_parent))
parent = ASN1_INTEGER_get(tpk->parent);
else
parent = BN_get_word(bn_parent);

if (parent == 0)
keydata->parent = TPM2_RH_OWNER;
else if (parent <= UINT32_MAX)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By changing parent from type uint64_t to TPM2_HANDLE (which according to tpm2-tss is an uint32_t see: https://github.com/tpm2-software/tpm2-tss/blob/a54a27df0732cde2c47b1d4d0e7bef489dc8a7d5/include/tss2/tss2_tpm2_types.h#L1050 and https://github.com/tpm2-software/tpm2-tss/blob/a54a27df0732cde2c47b1d4d0e7bef489dc8a7d5/include/tss2/tss2_common.h#L24) this line will always be true.

Have a look at line 153:
BN_get_word returns an BN_ULONG, which is 32-bits on 32-bit systems and 64-bits on 64-bit systems.
BN_get_word set's all bits in case of an error.

parent is of type TPM2_HANDLE which is of type uint32_t.

On a 64 bit system, we're performing an implicit conversion from uint64 to uint32. This shouldn't be a problem in case of a valid parent-handle as they're in the range 0x81000000 - 0x81FFFFFF. I think we should handle the case where BN_get_word() returns something bigger than a 32bit value on 64bits systems and on 32bit systems we should treat the return value UINT32_MAX as error.

Expand All @@ -159,11 +173,11 @@ tpm2_keydata_read(BIO *bin, TPM2_KEYDATA *keydata, TPM2_PKEY_FORMAT format)
&keydata->pub))
goto error;

res = 1;
error:
BN_free(bn_parent);
TSSPRIVKEY_free(tpk);
return 1;
error:
TSSPRIVKEY_free(tpk);
return 0;
return res;
}

static const TPM2B_PUBLIC primaryRsaTemplate = {
Expand Down
27 changes: 27 additions & 0 deletions test/rsa_encodeobject_check.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#!/usr/bin/env bash
# SPDX-License-Identifier: BSD-3-Clause
set -eufx

# create primary using the same ECC template that tpm2-openssl uses internally
tpm2_createprimary \
-G ecc256:aes128cfb \
-g sha256 \
-a 'fixedtpm|fixedparent|sensitivedataorigin|userwithauth|noda|restricted|decrypt' \
-c primary.ctx

# generate a private key and store its public portions to PEM
tpm2_create -C primary.ctx -u key.pub -r key.priv -f pem -o pub.pem

# encode private key to PEM
tpm2_encodeobject -C primary.ctx -u key.pub -r key.priv -o priv.pem

# print public key modulus
openssl rsa -pubin -in pub.pem -modulus -noout

# validate the private key
openssl rsa -provider tpm2 -provider base -in priv.pem -passin pass: -check -noout

# print private key modulus
openssl rsa -provider tpm2 -provider base -in priv.pem -passin pass: -modulus -noout

rm primary.ctx key.pub key.priv pub.pem priv.pem
29 changes: 29 additions & 0 deletions test/rsa_encodeobject_check_parent.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/usr/bin/env bash
# SPDX-License-Identifier: BSD-3-Clause
set -eufx

# create primary key
tpm2_createprimary -c primary.ctx

# make the primary persistent
HANDLE=$(tpm2_evictcontrol -c primary.ctx | cut -d ' ' -f 2 | head -n 1)

# generate a private key and store its public portions to PEM
tpm2_create -C primary.ctx -u key.pub -r key.priv -f pem -o pub.pem

# encode private key to PEM using the persistent parent handle
tpm2_encodeobject -C ${HANDLE} -u key.pub -r key.priv -o priv.pem

# print public key modulus
openssl rsa -pubin -in pub.pem -modulus -noout

# validate the private key
openssl rsa -provider tpm2 -provider base -in priv.pem -passin pass: -check -noout

# print private key modulus
openssl rsa -provider tpm2 -provider base -in priv.pem -passin pass: -modulus -noout

# release the persistent primary
tpm2_evictcontrol -c ${HANDLE}

rm primary.ctx key.pub key.priv pub.pem priv.pem
Loading