xaptum / ecdaa

A C implementation of elliptic-curve-based Direct Anonymous Attestation (DAA) signatures. Created to support the Xaptum Edge Network Fabric, an IoT Network Solution.
https://www.xaptum.com
Apache License 2.0
45 stars 8 forks source link

test: Zero-initialize all structs passed to TSS lib. #138

Closed zanebeckwith closed 3 years ago

zanebeckwith commented 3 years ago

This PR ensures that all structs used in the create_tpm_key-util utility program are zero-initialized before use.

This utility is only used when running tests against physical TPMs, when it's run manually to create the necessary key.

The same changes were already made to the test files used when testing against a software TPM simulator. I just had forgotten to make these changes here.

cf. Issue #137

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.1%) to 80.029% when pulling 60b78c746a57c1501a955ff4c448e61fe181a6b8 on zanebeckwith/zero-init-createkey into f50c400befcc5c0de3af1dfb819d04c162ea005a on master.

zanebeckwith commented 3 years ago

Regarding the three instances of this PR setting to 0 a size field that previously was non-zero:

Yes, this is fine. The unmarshalling logic in the TSS only checks the size of a TPM2B_ struct given to it to make sure it is 0, and even then it only makes this check for structs that aren't actually sized buffers but rather are actually sub-objects (and the size could actually be calculated beforehand).

Other than the "check that size is 0" for "subobject" TPM2B_ structs, no validation of the provided size is done, and the size field is just set to the size value in the raw values getting unmarshalled.

So, none of these changed sizes is actually examined by the TSS (note that the size in the creationData, which is a "sub-object" type, hasn't changed, because it was 0 before, too).

Whew, so, changing the sizes of these structs doesn't have any impact (they just get overwritten during unmarshalling).

I had tested these changes to make sure they worked, but hadn't followed up with an examination of the TSS source to make sure it made sense. Sorry about that.