Split the crypto layer behind a CryptoProvider - #5
Merged
Conversation
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.
`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.
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.
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.
# Conflicts: # TSS.Rust/src/crypto.rs # TSS.Rust/src/tpm_type_extensions.rs
There was a problem hiding this comment.
Pull request overview
Splits TSS.Rust cryptography into a provider abstraction and a default RustCrypto backend.
Changes:
- Adds pluggable crypto-provider interfaces and software implementation.
- Makes cryptographic randomness fallible.
- Adds feature-gating for software crypto dependencies and related tests/examples.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
TSS.Rust/Cargo.toml |
Adds the software-crypto feature. |
TSS.Rust/src/crypto.rs |
Removes the former monolithic module. |
TSS.Rust/src/crypto/mod.rs |
Adds provider routing and shared crypto logic. |
TSS.Rust/src/crypto/provider.rs |
Defines provider function interfaces. |
TSS.Rust/src/crypto/software_provider.rs |
Implements the RustCrypto backend. |
TSS.Rust/src/tpm2_impl.rs |
Propagates random-generation errors. |
TSS.Rust/src/tpm_type_extensions.rs |
Propagates randomness errors and gates tests. |
TSS.Rust/examples/tpm_samples.rs |
Handles fallible random generation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (2)
TSS.Rust/src/crypto/software_provider.rs:258
aes_cfbaccepts 24- and 32-byte keys, andSession::param_xcryptexplicitly permits AES-256, but this always constructsAes128.GenericArray::from_slice(key)therefore panics for a nonempty AES-256 session instead of encrypting it. Dispatch to the matching AES-128/192/256 cipher (or consistently reject unsupported sizes before this provider is called).
let cipher = Aes128::new(GenericArray::from_slice(key));
TSS.Rust/src/crypto/software_provider.rs:40
- This special case does not hash the empty message: it returns an all-zero buffer. It also lets an unsupported algorithm succeed with an empty result because
digestSizereturns zero, bypassing the algorithm match. Let each hasher process empty input normally so callers receive the standardized digest (and unsupported algorithms still returnNotSupported).
if data.is_empty() {
return Ok(vec![0; Crypto::digestSize(alg)]);
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.
No description provided.