parallaxsecond / rust-tss-esapi

TSS 2.0 Enhanced System API (ESAPI) Rust wrapper
https://docs.rs/tss-esapi/
Apache License 2.0
88 stars 52 forks source link

Bump bindgen and update bindings #439

Closed gowthamsk-arm closed 1 year ago

gowthamsk-arm commented 1 year ago

Signed-off-by: Gowtham Suresh Kumar gowtham.sureshkumar@arm.com

gowthamsk-arm commented 1 year ago

The x86_64-unknown-linux-gnu.rs already was having some differences compared to the other 3 binding files.

Was this done intentionally by any chance?

ionut-arm commented 1 year ago

@wiktor-k @Superhepper - any thoughts on moving to v2.4.6 of tpm2-tss?

Superhepper commented 1 year ago

None other then it would require a major version bump.

wiktor-k commented 1 year ago

@wiktor-k @Superhepper - any thoughts on moving to v2.4.6 of tpm2-tss?

In general I'm in favor of upgrading what is possible and in this case I don't see any downsides. Go head 👍

ionut-arm commented 1 year ago

it would require a major version bump.

Why? I'm hoping we'd only have additions to the bindings, nothing removed (or if it would be removed, for example to rename from TPMI_CAMELLIA_KEY_BITS to TPMI_TPM2_CAMELLIA_KEY_BITS we can probably keep both), which would be compatible with a minor version bump.

Superhepper commented 1 year ago

If it is only an addition and it still works with the old version even if we do not support it then I guess no major version change is required.

gowthamsk-arm commented 1 year ago

So after generating the new bindings with the 2.4.6 version of the library, I noticed that the x86 bindings were already generated for 2.4.6 and the rest of the bindings were not updated to 2.4.6 along with x86. This explains the discrepancies between the bindings.

The new patch updates the other bindings to 2.4.6. As discussed, I have manually edited the bindings file to not introduce any breaking change. But one thing that is a bit tricky is TPMI_TPM2_CAMELLIA_KEY_BITS. This has been changed to TPMI_CAMELLIA_KEY_BITS in the newer version. Currently I have just added the new definition and didn't change the struct which is using the old TPMI_TPM2_CAMELLIA_KEY_BITS.

pub union TPMU_SYM_KEY_BITS {
    pub aes: TPMI_AES_KEY_BITS,
    pub sm4: TPMI_SM4_KEY_BITS,
    pub camellia: TPMI_TPM2_CAMELLIA_KEY_BITS,
    pub sym: TPM2_KEY_BITS,
    pub exclusiveOr: TPMI_ALG_HASH,
}
gowthamsk-arm commented 1 year ago

Any idea why the ubuntu test is failing?

./configure: line 17494: JSON_C_LIBS: command not found
checking for JSON_C... no
/tmp/rust-tss-esapi/tss-esapi/tests/pkg-config: 4: /tmp/rust-tss-esapi/tss-esapi/tests/pkg-config: SYSROOT: not found
configure: error: Package requirements (json-c) were not met:

/tmp/rust-tss-esapi/tss-esapi/tests/pkg-config: 4: /tmp/rust-tss-esapi/tss-esapi/tests/pkg-config: SYSROOT: not found
No package 'json-c' found

This happens for tpm2-tss 2.4.6 cross compilation. Should I use another docker base image for ubuntucontainer?

tgonzalezorlandoarm commented 1 year ago

@Superhepper CI is passing now and I think all issues are resolved

gowthamsk-arm commented 1 year ago

@ionut-arm @wiktor-k @Superhepper can I get a review, please? :)