Adding Rust support to TSS.MSR - #209
Open
Roee Kasher (Kasher) wants to merge 16 commits into
Open
Conversation
Author
|
@microsoft-github-policy-service agree company="Microsoft" |
Dennis Mattoon (DMattoon)
self-requested a review
February 24, 2026 18:20
Added support for unowned TBS context
Add bounds/underflow checking to TpmBuffer reads so truncated or malformed responses fail instead of silently returning zeros, fix ValidateSignature to use the signature's own hash algorithm, and move fromTpm/fromBytes to TpmStructure trait defaults (removing ~3.6k lines of generated code). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Harden TSS.Rust unmarshaling and clean up clippy warnings
Add checked TPM serialization and exact deserialization
* Consolidate RSA primitives into the Crypto facade
tpm_type_extensions.rs reached directly for the `rsa` and `rand` crates,
making it the only file bypassing `Crypto` and leaving two separate
RSA-OAEP implementations in the tree. Move all of it behind `Crypto` so
there is a single place where a crypto backend is selected.
- Add `Crypto::rsa_oaep_encrypt`, `rsa_pkcs1v15_verify`,
`rsa_generate_keypair` and `rsa_pkcs1v15_sign`, plus the `RsaKeyParts`
carrier and the shared `RSA_DEFAULT_EXPONENT` constant.
- Collapse the duplicate OAEP paths in `create_activation`, `encrypt` and
`encrypt_session_salt` onto one implementation. Labels are now passed
as bytes, which is how the TPM specification defines them.
- Route `validate_signature` and `TSS_KEY::{create_key, sign}` through the
facade. `tpm_type_extensions.rs` no longer imports `rsa` or `rand`.
Two panics become errors on the way: `create_key` no longer indexes into
an empty prime list, and `sign` no longer divides by a zero prime.
Error text converges at four sites that previously had their own wording
for the same failure. Behavior is otherwise unchanged.
* Size the activation seed from the key's name algorithm
`create_activation` generated a fixed 16 byte seed. TPM 2.0 Part 1
"Credential Protection" sizes the seed from the key's nameAlg, and Part 4
`CryptSecretDecrypt` rejects any other size with TPM_RC_VALUE, so
`TPM2_ActivateCredential` could never recover a credential built this way.
A SHA-256 key needs 32 bytes and a SHA-1 key 20.
`tpm_samples::activate_credentials` has been tolerating the resulting
mismatch with a "known create_activation issue" message.
TSS.NET already gets this right in `TpmKey.CreateActivationCredentials`,
which carries a comment stating the rule. TSS.CPP has the same defect in
`TPMT_PUBLIC::CreateActivation` and is the likely origin of this one.
Add a test that plays the part of TPM2_ActivateCredential in software: it
recovers the seed, enforces the same seed size check the TPM makes,
rederives both keys, verifies the integrity HMAC and decrypts the
credential. Modelling the TPM's size check is what gives the test its
value, because the seed travels inside the blob and would otherwise round
trip at any length.
* Consolidate RSA primitives into the Crypto facade
tpm_type_extensions.rs reached directly for the `rsa` and `rand` crates,
making it the only file bypassing `Crypto` and leaving two separate
RSA-OAEP implementations in the tree. Move all of it behind `Crypto` so
there is a single place where a crypto backend is selected.
- Add `Crypto::rsa_oaep_encrypt`, `rsa_pkcs1v15_verify`,
`rsa_generate_keypair` and `rsa_pkcs1v15_sign`, plus the `RsaKeyParts`
carrier and the shared `RSA_DEFAULT_EXPONENT` constant.
- Collapse the duplicate OAEP paths in `create_activation`, `encrypt` and
`encrypt_session_salt` onto one implementation. Labels are now passed
as bytes, which is how the TPM specification defines them.
- Route `validate_signature` and `TSS_KEY::{create_key, sign}` through the
facade. `tpm_type_extensions.rs` no longer imports `rsa` or `rand`.
Two panics become errors on the way: `create_key` no longer indexes into
an empty prime list, and `sign` no longer divides by a zero prime.
Error text converges at four sites that previously had their own wording
for the same failure. Behavior is otherwise unchanged.
* Size the activation seed from the key's name algorithm
`create_activation` generated a fixed 16 byte seed. TPM 2.0 Part 1
"Credential Protection" sizes the seed from the key's nameAlg, and Part 4
`CryptSecretDecrypt` rejects any other size with TPM_RC_VALUE, so
`TPM2_ActivateCredential` could never recover a credential built this way.
A SHA-256 key needs 32 bytes and a SHA-1 key 20.
`tpm_samples::activate_credentials` has been tolerating the resulting
mismatch with a "known create_activation issue" message.
TSS.NET already gets this right in `TpmKey.CreateActivationCredentials`,
which carries a comment stating the rule. TSS.CPP has the same defect in
`TPMT_PUBLIC::CreateActivation` and is the likely origin of this one.
Add a test that plays the part of TPM2_ActivateCredential in software: it
recovers the seed, enforces the same seed size check the TPM makes,
rederives both keys, verifies the integrity HMAC and decrypts the
credential. Modelling the TPM's size check is what gives the test its
value, because the seed travels inside the blob and would otherwise round
trip at any length.
* Split the crypto layer behind a CryptoProvider
Move crypto.rs into a crypto/ directory module. crypto::provider defines the
primitives TSS.Rust needs from a backend as a struct of function pointers;
crypto::software_provider supplies one built on the RustCrypto crates. Crypto
keeps the logic the TPM specification defines on top of those primitives --
digest sizes, KDFa, signature validation -- and delegates the rest.
Crypto's public signatures are unchanged except get_random, which now returns
Result so an RNG failure is reported rather than swallowed. Tpm2::roll_nonces
becomes fallible as a consequence.
The provider is still selected internally by a temporary Crypto::provider().
Callers will pass one explicitly in a later change.
* Make the software crypto backend optional
Put the RustCrypto-backed provider behind a software-crypto feature, on by
default, and mark the eight crates it needs optional. Building with
--no-default-features drops them entirely, so a host that would rather not link
a second implementation of primitives its operating system already provides can
supply its own provider instead.
With no backend compiled in, Crypto routes to a provider whose every primitive
reports NotSupported. The tests that verify against the rsa crate are gated on
the feature; the remaining 12 still run. Every sample opens an HMAC session and
so derives a session key on the host, so tpm_samples now requires the feature
rather than building into a binary that cannot work.
* Thread the crypto provider through every call path
The provider added in the previous commit was not reachable: every public
Crypto operation went through a private selector that always picked either
the software backend or an unimplemented one. Building without the
software-crypto feature therefore produced a library where all
crypto-dependent operations failed at runtime.
Take the provider explicitly on each Crypto primitive and on the library
call paths that use them, and drop the hidden selector along with the
unimplemented placeholder provider.
The generated command methods dispatch through `&mut self` and have no
parameter to carry a provider, so Tpm2 holds one. Tpm2::new now takes it,
Tpm2::with_software_crypto supplies the built-in backend, and Tpm2::crypto
exposes it so callers computing a policy digest or a key name alongside a
live TPM agree on one backend.
Split create_tpm into create_tpm_with_crypto, which keeps the platform
selection logic, and a feature-gated create_tpm on top of it, so a build
without the software backend keeps that logic instead of reimplementing it.
Breaking: PolicyAssertion::update_policy_digest, the TPMT_PUBLIC and
TSS_KEY helpers, Tpm2::new and all Crypto primitives take a provider;
create_tpm and create_tpm_with_device now require the software-crypto
feature.
None of these are regressions from the CryptoProvider work; the split simply made them visible by gathering the primitives into one place. AES-192 and AES-256 panicked. The CFB routine handed every key to Aes128::new regardless of length, and that constructor asserts on a key it cannot take, so a 24 or 32 byte key aborted the process. Dispatch on key length instead, and build the cipher with KeyInit::new_from_slice, which reports a bad length rather than panicking. All three variants share a 128 bit block, so the CFB loop itself is generic over the cipher and unchanged. Hashing an empty input returned zeros. `hash` short circuited on empty data and handed back a buffer of digestSize zero bytes, so hash(SHA256, &[]) gave zeros instead of e3b0c442..., and an unsupported algorithm returned Ok. The special case exists because three callers in policy.rs only wanted the digest length, which they obtained by hashing nothing and measuring the result. Give them Crypto::digest_size_checked and delete the special case. PolicyPcr with an empty pcrValues was reaching the wrong path for real, not just in theory. The RSA public exponent was ignored. Every operation assumed 65537, so a key declaring any other exponent was encrypted to, and verified against, the wrong key. Add TPMS_RSA_PARMS::exponent_bytes, which resolves the specification's zero encoding to 65537 at the point of use, and thread it through the five sites that needed it. Generation had the same defect from the other side: create_key ignored params.exponent, so requesting e=3 produced a 65537 key whose stored public area still claimed 3. RsaGenerateKeypairFn therefore takes the exponent and returns the one actually used. The resolution deliberately happens at each call site rather than by normalising the stored field. A key's Name is a digest over its public area, so rewriting exponent 0 to 65537 would change the Name of every default key and silently break every policy digest, ActivateCredential and Name comparison. exponent_bytes carries that reasoning in its doc comment. Signing did not check that the supplied prime divides the modulus. q was computed as n / p by truncating division, which yields a plausible looking but wrong q for any p that is not a factor, and the resulting signature fails to verify with no indication why. Reject a non-zero remainder. KDFa returned an empty key when asked for zero bits. The loop is driven by the output length, so bits = 0 exits immediately with Ok(vec![]), and callers used that empty vector as key material. Callers reached that state by sizing their request as digestSize(alg) * 8, which is zero for anything that is not a hash. Reject the request in KDFa, and size those requests with digest_size_checked so the failure names the actual problem. Crypto::digestSize keeps returning zero, because TPMT_HA unmarshalling depends on TPM_ALG_NULL having a zero length digest. Eleven tests cover the fixes. Nothing in the existing suite would have caught any of them.
* Support ECC storage keys in create_activation TPMT_PUBLIC::create_activation rejected anything that was not an RSA key, so a TPM whose endorsement key is ECC could not be issued a credential at all. This adds the ECC path. The two algorithms differ only in how the seed reaches the TPM, which is now isolated in produce_seed. With RSA the seed is random and the secret is that seed under OAEP. With ECC nothing is transported: both sides derive the seed from an ECDH agreement, and the secret is the ephemeral public point the TPM needs to repeat it. Everything after the seed is defined on the seed alone and is shared unchanged. The crypto layer gains KDFe, the SP800-56A concatenation KDF that TPM 2.0 Part 1 section 11.4.10.3 defines for this derivation. It is easy to confuse with KDFa: it hashes rather than HMACs, and it does not hash the requested length. Getting either wrong yields output of the right shape and the wrong value. Only the ephemeral agreement is delegated to the provider, and it returns the raw agreed value. Deriving from it is specification behaviour rather than backend behaviour, so it stays in Crypto. Some platform APIs offer to perform the concatenation themselves, but at least one silently ignores the requested hash algorithm and always uses SHA-256, which no interoperability failure would attribute to the KDF. Coordinate width is answered by Crypto rather than by a provider, because a TPM may drop leading zero bytes from a coordinate it marshals into a TPM2B while KDFe hashes coordinates at full width. A curve outside the TCG registry is an error rather than a guess, since guessing produces a plausible key that simply does not match the peer's. The software provider implements the agreement over P-256, P-384 and P-521. Barreto-Naehrig and SM2 curves are rejected rather than approximated with a NIST curve. Tested by a round trip against a stand-in that repeats the agreement, on three curves across three nameAlgs. Mutation testing confirms the round trip fails if partyU and partyV are swapped, or if the KDFe label is changed, on one side only. It cannot catch a mistake made identically on both sides, since both ends call the same KDFe, so KDFe is additionally pinned to an independently computed vector. * Address review of the ECC create_activation change Four defects found in review of the previous commit, two of them behavioural and two in what the code claims about itself. KDFa and KDFe are specified in bits but truncated to whole octets, so a request that is not a multiple of eight returned too many significant bits instead of the leftmost bits right aligned. TSS.NET and TSS.CPP shift the stream to correct this. Both KDFs now share one helper rather than being fixed apart, since the two disagreeing about what a bit count means is worse than the original bug. KDFa was affected identically and is fixed here even though the previous commit did not touch it. This is unreachable through TPM 2.0 itself, where every KDF request is a digest or symmetric key size and therefore already octet aligned. It is fixed so that a caller using the library directly agrees with the other stacks. The shift is applied to the retained octets rather than to the whole stream, which is equivalent because the octets a full shift would produce beyond the requested length are discarded in either order. Where the request is octet aligned the operation is byte for byte the previous truncation, confirmed by forcing the shift to zero and observing that only the new test fails. The agreed ECDH value was held in an ordinary Vec and printed by a derived Debug, while the seed derived from it was being zeroized. Wrapping it in Zeroizing alone would have left the printing, since Zeroizing implements Debug whenever its contents do, so Debug is now written by hand and shows the public coordinates in full while withholding the secret. That keeps the value useful for diagnosing an encoding mismatch, which is what it is for. EccEphemeralAgreement documented all three of its fields as KDFe inputs. Only z and the ephemeral X reach the KDF; the ephemeral Y travels to the peer inside the marshalled point. The correction says why Y is still padded, since a reader told only that it is unhashed could conclude it needs no padding, and a TPM parsing the point is entitled to a full width coordinate. The same claim appeared in a comment in produce_seed and is corrected there too. Coordinate restoration had no coverage. Points built from a SEC1 encoding carry full width coordinates, so the padding was exercised only as a no op and removing it entirely passed the whole suite. A round trip is added against a key whose public X carries a leading zero, stripped from the fixture the way a TPM marshalling a TPM2B may strip it, along with a rejection case for a coordinate wider than its curve: trimming an oversized coordinate to fit would agree with a point the peer never held and fail only at the TPM. That fixture uses a fixed scalar. One key in 256 has a leading zero octet, and generating that many in an unoptimised test build cost more than the rest of the suite together. The test asserts the coordinate is short before relying on it, so a fixture that stopped being short would fail rather than quietly revert to a no op. The KDF test pins both functions at 250 bits against independently computed vectors. The two are unrelated to each other because KDFa hashes the requested length into every iteration and KDFe does not, so KDFa at 250 bits is a different stream rather than a shortened one. * Address the second review pass on the ECC activation change Wipe the KDF intermediates. Both KDFs accumulated the derived key in a plain Vec, and KDFe additionally copied the agreed value into a plain per-iteration buffer, so several copies of key material were freed unwiped. All are now Zeroizing, and both are reserved to their final size up front because a Vec that grows frees its old allocation without wiping it, which would have left copies behind despite the wrapper. KDFa's hashed input is deliberately left plain: everything in it is public and the key travels separately as the HMAC key. Give TEST_P192 its coordinate width. It was the only registry curve other than NONE falling through to the unknown arm. This function is a registry lookup, so it answers for any curve the registry names; whether a backend will agree over that curve is answered separately. The new sweep test enumerates the registry through the generated try_from rather than a hand-written list, so a curve added upstream fails the test instead of silently falling through. Stop pulling default features from the curve crates. They brought in the ECDSA and RFC6979 crates for a provider that only ever agrees. The graph goes from 70 crates to 68; the PKCS#8 and PEM crates stay, since the rsa crate needs them independently. * Qualify size_of_val to match the rest of the crate Every other use of this family is written out in full: std::mem::size_of in device.rs, tpm2_helpers.rs and twice in tpm_buffer.rs. This one line was the only unqualified use, so it now matches the other four. This is a consistency change rather than a fix. size_of_val has been in the prelude since Rust 1.80, the crate declares no MSRV below that, and the unqualified form built and passed the suite; a review comment claiming otherwise was mistaken. The _val form is kept rather than size_of::<u32>() so that the reservation follows the counter's type if it ever changes, which is what makes the reservation correct.
* Support ECC storage keys in create_activation TPMT_PUBLIC::create_activation rejected anything that was not an RSA key, so a TPM whose endorsement key is ECC could not be issued a credential at all. This adds the ECC path. The two algorithms differ only in how the seed reaches the TPM, which is now isolated in produce_seed. With RSA the seed is random and the secret is that seed under OAEP. With ECC nothing is transported: both sides derive the seed from an ECDH agreement, and the secret is the ephemeral public point the TPM needs to repeat it. Everything after the seed is defined on the seed alone and is shared unchanged. The crypto layer gains KDFe, the SP800-56A concatenation KDF that TPM 2.0 Part 1 section 11.4.10.3 defines for this derivation. It is easy to confuse with KDFa: it hashes rather than HMACs, and it does not hash the requested length. Getting either wrong yields output of the right shape and the wrong value. Only the ephemeral agreement is delegated to the provider, and it returns the raw agreed value. Deriving from it is specification behaviour rather than backend behaviour, so it stays in Crypto. Some platform APIs offer to perform the concatenation themselves, but at least one silently ignores the requested hash algorithm and always uses SHA-256, which no interoperability failure would attribute to the KDF. Coordinate width is answered by Crypto rather than by a provider, because a TPM may drop leading zero bytes from a coordinate it marshals into a TPM2B while KDFe hashes coordinates at full width. A curve outside the TCG registry is an error rather than a guess, since guessing produces a plausible key that simply does not match the peer's. The software provider implements the agreement over P-256, P-384 and P-521. Barreto-Naehrig and SM2 curves are rejected rather than approximated with a NIST curve. Tested by a round trip against a stand-in that repeats the agreement, on three curves across three nameAlgs. Mutation testing confirms the round trip fails if partyU and partyV are swapped, or if the KDFe label is changed, on one side only. It cannot catch a mistake made identically on both sides, since both ends call the same KDFe, so KDFe is additionally pinned to an independently computed vector. * Address review of the ECC create_activation change Four defects found in review of the previous commit, two of them behavioural and two in what the code claims about itself. KDFa and KDFe are specified in bits but truncated to whole octets, so a request that is not a multiple of eight returned too many significant bits instead of the leftmost bits right aligned. TSS.NET and TSS.CPP shift the stream to correct this. Both KDFs now share one helper rather than being fixed apart, since the two disagreeing about what a bit count means is worse than the original bug. KDFa was affected identically and is fixed here even though the previous commit did not touch it. This is unreachable through TPM 2.0 itself, where every KDF request is a digest or symmetric key size and therefore already octet aligned. It is fixed so that a caller using the library directly agrees with the other stacks. The shift is applied to the retained octets rather than to the whole stream, which is equivalent because the octets a full shift would produce beyond the requested length are discarded in either order. Where the request is octet aligned the operation is byte for byte the previous truncation, confirmed by forcing the shift to zero and observing that only the new test fails. The agreed ECDH value was held in an ordinary Vec and printed by a derived Debug, while the seed derived from it was being zeroized. Wrapping it in Zeroizing alone would have left the printing, since Zeroizing implements Debug whenever its contents do, so Debug is now written by hand and shows the public coordinates in full while withholding the secret. That keeps the value useful for diagnosing an encoding mismatch, which is what it is for. EccEphemeralAgreement documented all three of its fields as KDFe inputs. Only z and the ephemeral X reach the KDF; the ephemeral Y travels to the peer inside the marshalled point. The correction says why Y is still padded, since a reader told only that it is unhashed could conclude it needs no padding, and a TPM parsing the point is entitled to a full width coordinate. The same claim appeared in a comment in produce_seed and is corrected there too. Coordinate restoration had no coverage. Points built from a SEC1 encoding carry full width coordinates, so the padding was exercised only as a no op and removing it entirely passed the whole suite. A round trip is added against a key whose public X carries a leading zero, stripped from the fixture the way a TPM marshalling a TPM2B may strip it, along with a rejection case for a coordinate wider than its curve: trimming an oversized coordinate to fit would agree with a point the peer never held and fail only at the TPM. That fixture uses a fixed scalar. One key in 256 has a leading zero octet, and generating that many in an unoptimised test build cost more than the rest of the suite together. The test asserts the coordinate is short before relying on it, so a fixture that stopped being short would fail rather than quietly revert to a no op. The KDF test pins both functions at 250 bits against independently computed vectors. The two are unrelated to each other because KDFa hashes the requested length into every iteration and KDFe does not, so KDFa at 250 bits is a different stream rather than a shortened one. * Address the second review pass on the ECC activation change Wipe the KDF intermediates. Both KDFs accumulated the derived key in a plain Vec, and KDFe additionally copied the agreed value into a plain per-iteration buffer, so several copies of key material were freed unwiped. All are now Zeroizing, and both are reserved to their final size up front because a Vec that grows frees its old allocation without wiping it, which would have left copies behind despite the wrapper. KDFa's hashed input is deliberately left plain: everything in it is public and the key travels separately as the HMAC key. Give TEST_P192 its coordinate width. It was the only registry curve other than NONE falling through to the unknown arm. This function is a registry lookup, so it answers for any curve the registry names; whether a backend will agree over that curve is answered separately. The new sweep test enumerates the registry through the generated try_from rather than a hand-written list, so a curve added upstream fails the test instead of silently falling through. Stop pulling default features from the curve crates. They brought in the ECDSA and RFC6979 crates for a provider that only ever agrees. The graph goes from 70 crates to 68; the PKCS#8 and PEM crates stay, since the rsa crate needs them independently. * Qualify size_of_val to match the rest of the crate Every other use of this family is written out in full: std::mem::size_of in device.rs, tpm2_helpers.rs and twice in tpm_buffer.rs. This one line was the only unqualified use, so it now matches the other four. This is a consistency change rather than a fix. size_of_val has been in the prelude since Rust 1.80, the crate declares no MSRV below that, and the unqualified form built and passed the suite; a review comment claiming otherwise was mistaken. The _val form is kept rather than size_of::<u32>() so that the reservation follows the counter's type if it ever changes, which is what makes the reservation correct. * Add a CryptoProvider built on Windows CNG The software provider brings in fifty crates to implement primitives Windows already ships. This adds a second backend behind a cng-crypto feature that calls bcrypt.dll through the windows bindings the crate already depends on, so it adds no dependencies of its own. Building with --no-default-features --features cng-crypto takes the graph from 68 crates to 18. Seven of the nine primitives are implemented. generate_keypair and pkcs1v15_sign are not, for the reason already recorded on RsaOps: signing recovers the second prime by dividing the modulus by the first, and CNG exposes no big-integer division. They are left together because a key this provider generated but could not sign with would be a trap. Three places where the obvious CNG call is the wrong one, each documented where it is made: No algorithm provider is ever opened. Pseudo-handles cover every algorithm needed here, including AES-ECB, which also removes the chaining-mode property that would otherwise have to be set. Only key and secret handles need owning. AES-CFB is built on the ECB primitive rather than BCRYPT_CHAIN_MODE_CFB. That mode defaults to an eight bit segment rather than a full block, and rejects input that is not block aligned; a TPM credential is a digest behind a two byte size, so it never is. The agreed ECDH value comes back least significant byte first, unlike every other value CNG exports, and is reversed and left padded to the curve width here. The raw secret is exported rather than letting CNG run the SP800-56A concatenation, both because the derivation belongs to Crypto::kdfe and because that KDF ignores the hash algorithm it is handed. Thirteen tests compare the two backends primitive by primitive, and run KDFa and KDFe over both so the stack above the provider is covered too. ECDH cannot be compared that way, since each backend generates its own ephemeral key, so that test holds the peer's private half and recomputes the agreed value from the ephemeral point CNG returned; without it the byte reversal would be invisible, because a wrongly ordered value is still the right length.
* Declare the crate's licence in its manifest The repository's LICENSE file has said MIT since the beginning, but the Rust manifest named no licence at all. Tooling that audits dependency licences reads the manifest rather than the file, so the crate is reported as unlicensed, and a consumer whose policy refuses unlicensed dependencies cannot take it at all. cargo-deny fails outright on this. A description and repository are added alongside. The repository points at this fork rather than microsoft/TSS.MSR, because upstream carries C, C++, Java, JavaScript, .NET and Python bindings but no Rust one, so there is as yet no upstream home to name. * Report an unusable RSA signature as invalid rather than as a failure BCryptVerifySignature answers STATUS_INVALID_SIGNATURE for a signature that does not verify, but STATUS_INVALID_PARAMETER for one it will not attempt at all, which for an attacker supplied value mostly means a signature numerically greater than the modulus or of the wrong length. Only the first was being turned into Ok(false), so a malformed signature came back as an error. That disagreed with the RustCrypto provider, which answers Ok(false) for exactly those inputs, so the two backends differed on which signatures are acceptable. A caller checking a signature off the wire wants to hear "invalid" rather than an error it then has to classify. The test that caught this was itself defective: it corrupted the signature's high byte, which pushes the value above the modulus only for some keys, and the key is generated afresh on every run. It therefore passed or failed by luck, and it passed the first time. It now covers a flipped low byte, a flipped high byte, a truncated signature and an empty one, and asserts that both backends agree on each, so the outcome no longer depends on which key was drawn. * Rename the crate to tss-msr-rs The package was named tss-rust, which is already taken on crates.io by an unrelated project, so that name is unavailable if this crate is ever published to a registry. tss-msr-rs names the repository this lives in and follows the ecosystem's -rs convention. The sibling bindings cannot be copied directly: they are Microsoft.TSS on NuGet and com.microsoft.azure:TSS.Java on Maven, both of which carry the vendor in a namespace that crates.io does not have. Claiming a microsoft- prefixed name on a flat namespace would assert an ownership this repository is not currently in a position to assert. Only the manifest and one example needed changing: the library refers to itself as `crate` throughout, so nothing internal depended on the old name.
…safety (#10) * Report absent union fields as an error instead of panicking TargetLang.UnionMember emitted `.unwrap().` for Rust, so serializing a struct whose union selector is not its first marshaled field panicked instead of returning an error. TPMS_ATTEST derives Default and TPMS_ATTEST::new(..., &None) is public, so TPMS_ATTEST::default().toBytes() was a panic reachable from safe, documented public API. Emit `.ok_or(TpmError::InvalidUnion)?.` instead. Both serialize and deserialize already return Result<(), TpmError>, so no signatures change. The first-field guard is retained: it is a load-bearing "empty object" convention that lets a default-constructed TPMT_SENSITIVE marshal as a zero-size sized object, which LoadExternal and PolicySigned rely on. It is also emitted for the other five target languages. Verified: TPMS_ATTEST::default().toBytes() now returns Err; the TPMS_CAPABILITY_DATA, TPMT_SENSITIVE and TPMT_PUBLIC defaults still return Ok(0 bytes). Regeneration is byte-for-byte reproducible and the generated diff contains only this substitution. Non-Rust generator branches are unchanged. * Fix the non-Windows build and close a TBS context leak The crate did not compile off Windows at all. Three independent causes: - `windows` was declared in the platform-agnostic [dependencies], and it unconditionally pulls windows-future, which does not build off Windows. This is the blocking failure; no amount of cfg-gating in device.rs alone would have fixed it. The cfg(windows) block already declares the crate with a superset of features. - device.rs imported windows::Win32::System::TpmBaseServices with no cfg, unlike the imports bracketing it. - The non-Windows branch of create_tpm_with_crypto referenced TpmTcpDevice, which is entirely commented out, and passed a device by value where Box<dyn TpmDevice> is required. The Linux path also used TcpStream, Read and Write without importing them. macOS and the BSDs now get a documented NotSupported stub rather than silently having no device. Separately, TpmTbsDevice acquired a TBS context but had no Drop, so the context leaked on every drop and on every early return; TSS.CPP closes it in its destructor. The handle now lives in an owning TbsContext type whose Drop closes it, which also collapses the scattered null checks into an Option. Borrowed contexts are still not closed, and that invariant is now asserted rather than merely documented. The Linux socket read path bounded its allocation by a peer-supplied length prefix; it is now capped by MAX_RESPONSE_SIZE, and read and write timeouts are set on connect. Verified: cargo check and clippy --all-targets --all-features -D warnings both pass for x86_64-unknown-linux-gnu and aarch64-apple-darwin, and fail on the parent commit. * Fix out-of-bounds panic selecting PCR 24 or above TPMS_PCR_SELECTION::new_from_pcr_u32 divided pcr_bytes by 8 a second time in its growth check, comparing the wrong quantity. The selection vector therefore never grew below PCR 192, so indexing it for PCR 24 -- the first PCR outside the standard 0-23 range -- panicked. Compare pcr_bytes + 1 against the current size instead. PCRs 0-23 keep their historical three-byte pcrSelect exactly; this is a wire format, so a change there would silently invalidate existing PCR policies. A test pins that. new_from_pcrs_vec was audited for the same defect and does not have it: it sizes the vector with a single division and clamps to the standard range. * Say where an unusable signature falls in the verify contract RsaOps::pkcs1v15_verify already says that `false` means the signature did not verify and that `Err` means the verification could not be performed, "for instance because the key or the hash algorithm was rejected". It did not say where a signature that is not a candidate at all falls: one whose length differs from the modulus, or whose value is at or above it. Neither the key nor the hash algorithm is at fault in that case, and those are what `Err` is reserved for, so both shapes are `false`. That follows from the existing sentence rather than adding to it, but it is worth stating, because both are chosen by whoever supplied the signature: a backend that reported them as failures would divert a caller such as validate_certify out of its `false` branch on input a remote party controls, and would disagree with the other backend about which signatures are acceptable. The software provider gets those shapes as a test. It answers them with `Ok(false)` because the `rsa` crate folds every verification error into one; the CNG provider, which sits on an API that distinguishes them, folds them in itself and is covered by its own tests. * Withhold and zeroize secret material in Debug output Session derived Debug over session_key -- the key that authorizes every command on the session and keys parameter encryption -- and is cloned on essentially every command. For a password session sess_in.hmac holds the raw authValue. RsaKeyParts derived Debug over prime, a private RSA factor sitting next to its own modulus. Both now have hand-written Debug impls that withhold the secret, and hand-written Drop impls that zeroize it. This follows the pattern EccEphemeralAgreement already uses in crypto/provider.rs. session_key is left as Vec<u8> rather than Zeroizing<Vec<u8>> deliberately: Zeroizing does not deref-coerce on assignment, so it would force edits inside Session::new, from_tpm_response and calc_session_key. The hand-written Drop wipes the same material and additionally covers sess_in.hmac, which Zeroizing could not reach because TPMS_AUTH_COMMAND is generated. TSS_KEY::create_key moved fields out of RsaKeyParts, which Drop forbids; it now uses std::mem::take. * Fix policy digest computation and session authorization Policy digests -------------- Three defects each produced a digest no TPM session could satisfy, so objects sealed against them were permanently unusable. All three are checked against TSS.CPP TpmPolicy.cpp and TSS.NET PolicyAces.cs, and the new tests pin externally derived vectors rather than our own output. - policy_update skipped its second extend when arg3 was empty, despite its own doc comment saying the extend is unconditional. This hit PolicySigned, PolicySecret and PolicyAuthorize with the common empty policyRef. - PolicyNv delegated to policy_update and so performed two extends where TPM2_PolicyNV defines exactly one. - PolicyPcr::execute sent the first raw PCR value as pcrDigest instead of the digest computed a few lines above, and indexed [0] unchecked. An empty pcrDigest makes the TPM skip the comparison entirely, so this could silently produce a session that did not check PCRs. Both paths now share one pcr_digest() and empty input is rejected. Session authorization --------------------- The response HMAC was verified only for HMAC sessions and policy sessions running PolicyAuthValue, leaving every other policy session's response parameters unauthenticated. The rule is now the TPM's own: authorization is expected unless the session is PWAP or has run TPM2_PolicyPassword. Both a missing and an unexpected authorization are errors, and the tag is compared in constant time. The command side needed the same rule. Probing a real TPM showed a policy session carrying only PolicyCommandCode returning an empty response authorization, because this client sent an empty command auth and triggered the TPM's "key empty and input empty" shortcut. Sending a computed HMAC for every non-PWAP, non-password session -- mirroring includeAuth in the reference implementation -- also fixes a pre-existing bug: salted and bound policy sessions were unusable, failing with TPM_RC_BAD_AUTH. The next command's session attributes were taken from the response, so an adversary could clear encrypt/decrypt and silently downgrade the following command to cleartext. The caller's attributes are now authoritative and a differing echo is an error. Salted sessions and parameter encryption ---------------------------------------- start_auth_session_ex read the salt key's public area over the very channel the salt protects, so a man in the middle could supply its own key, learn the salt and forge both HMACs undetectably. It now takes a caller-pinned TrustedPublic and the internal ReadPublic is removed. The salt is generated internally at the digest size. TrustedPublic pairs a public area with its locally derived name and makes the trust decision explicit at the call site. Its documentation states that this is caller-asserted trust: it is not channel authentication and not proof of TPM residency. Parameter encryption on an unsalted, unbound session derived its key from nothing but the two cleartext nonces. It is now refused at construction and again in param_xcrypt. The gate is a secret_key_material flag rather than an emptiness check, because a session bound to an entity with an empty authValue has a non-empty session key that is still a pure function of public data. Tests ----- Adds a mock TpmDevice, so tpm2_impl.rs has executed coverage for the first time, and six hardware round trips behind #[ignore]. * Use a single extend for the assertions that take one Making policy_update's second extend unconditional was right for the TPM's PolicyUpdate helper, which PolicySigned, PolicySecret and PolicyAuthorize use. But eight assertions were routing through that helper with an empty reference and relying on the removed guard to get a single extend, so they began producing a digest no TPM session could satisfy. PolicyCommandCode, PolicyPcr, PolicyAuthValue, PolicyPassword, PolicyCpHash, PolicyNameHash, PolicyCounterTimer and PolicyDuplicationSelect each extend once, as their counterparts in TSS.CPP TpmPolicy.cpp do. They now go through a single-extend helper, and policy_update is defined in terms of it. PolicyLocality, PolicyNv and PolicyOr already extended once by hand and are routed through the same helper without changing their bytes. The PolicyPcr test vector was itself the two-extend value, which is why it agreed with the defect instead of catching it. It is re-derived from PolicyPcr::UpdatePolicyDigest, the old value is kept as a negative assertion, and every other assertion gains a vector taken from its own matching C++ function rather than from the shared helper. Separately, the generated session salt was sized to the digest of the session's hash algorithm; the TPM sizes it to the digest of the salt key's nameAlg, so a SHA-1 salt key was sent a 32-byte salt and answered TPM_RC_VALUE. Two hardware tests added alongside it were also wrong: a restricted decryption key was given a signing scheme, and a PCR selection was a single all-zero byte. Verified on a physical TPM: the policy tree sample reports every digest matching its trial session, and all six hardware round trips pass. * Decode format-1 response codes, fix retry, check returned Names Response codes -------------- process_response ran the raw code through the generated exhaustive TPM_RC match, whose fallback is InvalidEnumValue. Format-1 codes embed a handle, parameter or session index in bits 8-11, so they are not bare enum members: TPM_RC_SIZE against parameter 1 arrives as 0x1D5 and was absent from the match, as were 0x1C5, 0x18B, 0x98E and others. Ordinary TPM failures therefore surfaced as "Invalid enum value" with the real code discarded and last_response_code never set. The code is now read as a raw u32 and split into its base TPM_RC and an index. The masks match TSS.NET Tpm2.cs and TSS.CPP Tpm2.cpp. The two private helpers written for this and left unreachable behind the `?` are folded into the new decoder. TpmCommandError was declared, given Display, Error and From, stored in last_error and exposed by last_error() -- but never constructed, so last_error() always returned None. It now carries the raw code and index and is actually built. Retry ----- TPM_RC::RETRY returned without clearing current_cmd_code, so the resend always failed with "Pending async command must be completed" after an unconditional one second sleep. The invocation state is now cleared, resends are bounded, and the stall is replaced by a short doubling backoff. Returned Names -------------- Object Names were taken verbatim from the response and never recomputed. They are now checked against the public area. This is documented strictly as a consistency check. It catches a malfunctioning or inconsistent TPM or resource manager. It is not authentication, does not defeat an adversary who supplies both a public area and a matching Name, and is not proof of TPM residency; callers who need that pin an expected Name out of band via TrustedPublic::from_pinned_name. Only CreatePrimary and CreateLoaded return the public area. Load and LoadExternal take it as a command input -- LoadExternalResponse carries only a handle and a name -- so for those the Name is checked against the caller's own public area, which is the stronger comparison. * Make validate_certify state its precondition and fail closed validate_certify moves onto TrustedPublic, so the trust decision in the signing key is present at every call site rather than implied. The signing key's provenance is a precondition, not a result. The attestation, the signature and the signing key's own objectAttributes all reach this function from the same untrusted source, so an adversary who fabricates a public area and an attestation to match satisfies every check at once. The documentation says so plainly: this authenticates no channel and shows nothing about where the private half lives. Callers establish provenance out of band, by credential activation against an endorsement key or by an attestation key certificate, and the new from_activated_credential constructor covers the first of those -- with a real check rather than a relabelling, verifying both that the credential came back intact and that the public area belongs to the object the credential was bound to. Given that provenance, the added restricted, sign, fixedTPM and fixedParent checks are defense in depth. restricted is the load-bearing one: it is what gives the magic == TPM_GENERATED_VALUE check content, because an unrestricted key will sign a hand-written TPMS_ATTEST and reduce that check to a test of whether the attacker remembered a constant. A non-RSASSA signature was delegated to validate_signature with an empty digest, skipping the magic, extraData and Name checks entirely; it was unexploitable only because validate_signature independently rejects such keys. Those checks now run before the algorithm dispatch, so no future branch can bypass them, and an unsupported algorithm returns NotSupported. The verdict was a Result<bool> that callers could and did drop with `?`. It is now Result<()> with a distinct VerificationFailed error. The sample had the signing key certify itself against a public area read from the TPM being attested, which is circular trust demonstrated as usage; it now certifies a second key. * Close leaks, correct overclaims, and make the tests pin their fixes A re-review of the preceding commits found resource leaks, one real authorization defect, and several claims that were stronger than the code behind them. Leaks ----- start_auth_session_ex flushed nothing when session construction failed after the TPM had already loaded the session, and a sample triggered that path deliberately on every run. The Name consistency check leaked a loaded object when it rejected one. TbsContext skipped its close under cfg(test) while the hardware tests opened real contexts, so a test run leaked one per test; the close is now reached through a function pointer that only the dangling-handle unit test replaces, and a probe over 1000 connect/drop cycles shows no handle growth where it previously showed 1000. Authorization ------------- Every PolicyAssertion::execute returned tpm.last_session(). For PolicySecret and PolicyNV the TPM auto-creates a password session for their auth handle, so that is what came back, and the next assertion in the tree then sent TPM_RS_PW as its policySession. Against an object with userWithAuth and an empty authValue the command would succeed by password while the caller believed a policy was enforced. All fourteen sites now select the session by handle. Overclaims corrected -------------------- expects_response_auth said skipping verification left responses unauthenticated "even though the session had the key material". An unsalted, unbound policy session has an empty session key, so the tag is forgeable by anyone on the wire: verification there detects corruption, not tampering. The second TPM behaviour it cites was observed on hardware, and now says so instead of implying the reference implementation. The samples reached for TrustedPublic::assume_trusted where a real check was free, teaching the weak form of an API whose documentation is careful; they now pin the Name the surrounding code already verified. A sample still described the session salt as sized to the session hash, which an earlier commit had corrected, and claimed no channel existed between the process and a locally created key -- the resource manager is that channel. TSS_KEY now zeroizes its private prime on drop. Its derived Debug still prints it, and TPM_HANDLE::auth_value has the same problem; both need a generator change and are documented as such rather than described as fixed. Tests ----- Ten tests asserted outcomes that also held before the fix they named. The salt-sizing regression, which reached hardware, still had no test at all. Session HMAC assertions keyed on the session key the code under test derived, so a KDFa label or nonce-order change stayed green. Added: an external KDFa vector, a salt-length test, digest vectors for the two assertions that legitimately extend twice, and rewrites of the tests that could not fail. Each added or repaired test was checked against a revert of the fix it covers. Also: consistent session-to-handle association between command and response, a bounds check on provider-supplied key material, checked arithmetic on a response length, two broken intra-doc links, and the removal of ~440 lines of commented-out transport code together with the cdylib output and dependencies that only it used. * Pin the rejection shapes over bytes that do not change between runs pkcs1v15_verification_agrees_with_the_software_provider draws a fresh key on every run, so it can only ever say that today's key was fine. Those shapes are the regression test for a provider that answered some of them with `Err`, which is worth stating over bytes that are the same every time: this test either always passes or always fails, and a failure is reproducible by running it again. It needs no private half, because a value of the right width below the modulus stands in for a signature perfectly well when every shape derived from it is meant to be rejected. Two of the shapes are not covered elsewhere. A signature with every byte set is at or above any modulus of the same width, which is how CNG comes to answer STATUS_INVALID_PARAMETER for a reason other than length; flipping the leading byte of a generated signature reaches that only for some keys, and how often depends on the modulus drawn. A signature one byte too long is the other side of the truncated one. verification_still_fails_rather_than_rejects_when_it_cannot_be_performed pins what is left of `Err` now that both statuses meaning "no" are folded into `Ok(false)`: a key CNG cannot import, and a hash algorithm it does not offer, both fail ahead of the status map rather than through it, so the order of the three steps in rsa_pkcs1v15_verify is what keeps them errors. It is read against a call differing in neither respect that comes back as a verdict, so it cannot be satisfied by a provider that simply errored on everything. * Stop the Rust generator deriving Debug over secret key material TSS_KEY::privatePart (an RSA prime) and TPM_HANDLE::auth_value (the caller's authorization value) were wiped or not, but either way printable: CGenRust emitted #[derive(Debug, Clone, Derivative)] for every struct, so any log line or error path that formatted one rendered the secret in full. CGenRust now carries a named list of structs it withholds Debug from, and the redacting implementations are hand-written in tpm_type_extensions.rs. The list is not inferred from the AST on purpose: whether a field is secret is not in the specification tables, and TPM_HANDLE::auth_value is injected from tpm_extensions.rs.snips so the generator never sees it. To keep a missing implementation from being a silent gap, the generator also emits a compile-time assertion requiring Debug for every listed type. A type with no implementation and no other use fails the build and is named, so the list and the implementations cannot drift apart. Beyond the two reported types, the survey added the two structures that carry a sensitive area in the clear (TPMT_SENSITIVE, TPMS_SENSITIVE_CREATE) and the five TPMU_SENSITIVE_COMPOSITE members, none of which is used anywhere else in the binding. Left alone deliberately: TPM2B_AUTH, which is a type alias for TPM2B_DIGEST and could not be redacted without redacting every public digest; the TPM-encrypted blobs TPM2B_PRIVATE, TPM2B_ENCRYPTED_SECRET and TPM2B_ID_OBJECT; and TPM2B_SENSITIVE, whose only field redacts itself. TPM_HANDLE still has no zeroizing Drop, so its auth value is no longer printable but is still left in freed heap. That is stated in the type's documentation rather than implied. * Narrow the empty-object marshaling exception to TPMT_SENSITIVE CodeGenBase emitted an early `return Ok(())` for every structure whose union selector is its first marshaled field, so TPMS_CAPABILITY_DATA, TPMT_SIG_SCHEME, TPMT_PUBLIC and eleven others serialized to an empty buffer when their required union was absent, bypassing the `ok_or(TpmError::InvalidUnion)` introduced in c789038. Only TPMT_SENSITIVE needs the exception: TPM2_LoadExternal loads a public area alone by sending a zero-size inPrivate sized object. Gate the early return on a new EmptyMarshalingStructs list, matched on the structure's spec name, which CGenRust threads in through a lambda over the outermost struct so base-class fields are attributed to the type being serialized. The condition is `!TargetLang.Rust || <in list>`, so C++, Java, JS, Python and .NET keep the old behavior; regenerating all six languages touches only tpm_types.rs, removing 14 guards and keeping TPMT_SENSITIVE's. absent_union_marshaling in tpm_structure.rs pins the contract: TPMT_SENSITIVE still marshals to nothing, all fourteen others and TPMS_ATTEST now return InvalidUnion, and a public-key-only TPM2_LoadExternal request still puts a zero-size inPrivate ahead of the public area. On the unmodified generated output the leading-union test fails with "TPMS_CAPABILITY_DATA with an absent union marshaled as Ok([])". * Wipe secrets on overwrite, bound PCR selections, and name the check that failed Three review comments, all correct. Wipe a secret before overwriting it, not only when its owner dies. `TSS_KEY::create_key` assigned the newly generated prime straight over `privatePart`. That drops the previous `Vec`, and `Vec`'s `Drop` only frees; `Drop for TSS_KEY` runs when the whole key goes out of scope, so a second `create_key` left the first prime in freed heap. Every write to the field now goes through `TSS_KEY::set_private_part`, which zeroizes first. The same shape existed on `Session::set_auth_value`, whose `sess_in.hmac` is the caller's auth value verbatim, and defensively on `Session::calc_session_key`. `publicPart.unique` is left alone: the modulus is public. The wipe is observable, and is observed. A `GlobalAlloc` is handed a still-valid pointer to a block on its way out, so a test-only allocator wrapper inspects the one block whose address it was armed with, reads only the bytes that block had initialized, and records whether they were zero. Removing either `zeroize` makes both new tests fail with the prime still in the freed block. Report a Name mismatch as a failed verification, not as a generic error. `TPMT_PUBLIC::verify_name` returned `GenericError`, so a caller could not tell an untrusted public area from an operational failure without matching on the message -- unlike the activation and certification checks beside it. It now returns `VerificationFailed`, and so does the Name-consistency check in `Tpm2::update_resp_handle`, which is that same error re-wrapped. The response authorization checks in `dispatch` are deliberately left as they are: they are transport authentication rather than public-area trust, and reclassifying some of them and not others would be worse than leaving all of them, since an adversary chooses which path they trip. Bound a PCR selection instead of letting it size an allocation. Fixing the out-of-bounds panic on PCR 24 replaced it with an unbounded allocation: `u32::MAX` asked for half a gigabyte. `pcrSelect` is marshalled behind a one-byte size prefix, so it holds at most 255 bytes and PCR 2039 is the largest index any selection can name. `new_from_pcr_u32` and `new_from_pcrs_vec` now check the index before allocating and return `Result`; `get_selection_array` carries the same bound. PCRs 0-23 still produce the identical three-byte selection, which is a wire format and is pinned by test. * Decide the next command's session attributes bit by bit Making the caller's requested TPMA_SESSION authoritative fixed a real downgrade -- a response could clear encrypt/decrypt and have the next command's first parameter go out in the clear -- but it was applied to the whole byte, including bits that are not the caller's to keep. auditReset and auditExclusive are conditions on the one command that carries them. Resending auditReset re-initialises the session's audit digest before every later command, so a session audit only ever covered the most recent one instead of accumulating; resending auditExclusive turns a one-off precondition into a standing one. They are consumed here rather than by trusting the response, because the reference implementation echoes auditReset back and reports auditExclusive SET for an exclusive session. continueSession is the one bit the TPM answers rather than echoes: CLEAR means it flushed the session when the command completed. Such a session is no longer offered through last_session()/last_sessions(), which is what the comment there always claimed and the code never did. encrypt, decrypt and audit stay exactly as the caller asked, and the strict encrypt/decrypt echo check is untouched. * Assert the session-attribute rule on the wire, not the accessor The test read the retained session's attributes back through last_session(), which shows what the client stored rather than what it would send. It now takes the reuse path a caller would take and asserts against the device's command log, so the assertion lands on the bytes the TPM would see.
derivative is unmaintained (RUSTSEC-2024-0388). It is a proc macro, so it contributes no code to the built artifact and this is supply-chain hygiene rather than a vulnerability, but the only thing the crate used it for was a Default with per-field values, and the generator can emit that itself. smart-default and educe were the obvious replacements and both were rejected: smart-default was last published in 2023 and is a future advisory waiting to happen, and educe would trade one proc macro dependency for another. The sibling repository this code is developed alongside hand-writes every Default impl and derives none, so emitting the implementation is also the local convention. A struct whose fields all start at Default::default() now derives Default as before. The rest get an emitted impl. Fields a .snips block injects are part of the struct and so have to be initialized too, which the generator could not see; CodeGenBase now exposes the snippet lines for that. TPM_HANDLE is the only type affected today, and its two injected fields would otherwise have made the emitted impl fail to compile. The 214 fields that carried an explicit default keep the same expression, except 162 that said TPM_HANDLE::default() for a TPM_HANDLE-typed field and now say Default::default(), which resolves to the same value because TPM_HANDLE's own default is unchanged at TPM_RH::NULL. Regeneration is idempotent and no other language's output moves.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a complete Rust implementation of the TSS, following the same architecture as the existing C++ support.
What's included: