Conversation
|
This supersedes the #153. |
gotthardp
force-pushed
the
tssfix
branch
3 times, most recently
from
58ad012 to
2615cd1
Compare
|
|
||
| if (parent == 0) | ||
| keydata->parent = TPM2_RH_OWNER; | ||
| else if (parent <= UINT32_MAX) |
There was a problem hiding this comment.
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.
|
I would have loved to see a comment in the final code, regarding the reason for this change. This is due to compatiblity with older provider/engine versions. I took quite an effort to analyze and document everything here: That might be helpful in the future. |
2 participants
Fixes: #152