Bump sha2 from 0.9.9 to 0.10.9 - #254
Conversation
|
Vaultkeeper |
3cb024d to
ec35018
Compare
ec35018 to
2ccd635
Compare
Bumps [sha2](https://github.com/RustCrypto/hashes) from 0.9.9 to 0.10.9. - [Commits](RustCrypto/hashes@sha2-v0.9.9...sha2-v0.10.9) --- updated-dependencies: - dependency-name: sha2 dependency-version: 0.10.9 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
2ccd635 to
cb6e304
Compare
aws-nitro-enclaves-image-format 0.2.0 pins sha2 0.9, so its EifHasher would not accept a sha2 0.10 Sha384. Versions of that crate which take sha2 0.10 (0.6.0+) force aws-nitro-enclaves-cose/key_kms, pulling in the AWS KMS SDK and breaking attestation-doc-validation 0.7.4. EifHasher::new_without_cache + write_all + tpm_extend_finalize_reset is just SHA384(0u8 * 48 || SHA384(cert)), so compute that inline and drop the dependency. Verified byte-identical output against the old code path for empty, short, 256-byte and 100KB inputs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Review: sha2 0.9.9 → 0.10.9 + inline PCR8 computation
This is a Dependabot bump plus a manual commit (738b3bf) that drops aws-nitro-enclaves-image-format and replaces its EifHasher with an inline PCR8 computation. The extra commit is necessary because that crate pins sha2 0.9, and the versions that accept sha2 0.10 would pull in the AWS KMS SDK — good call unbundling it.
Correctness — verified byte-identical ✅
Because PCR8 is attestation-critical (it's uploaded to the API and used to lock enclaves to signing certs), any change to its value would silently invalidate every already-locked cert reference. I checked the equivalence two ways:
- Source trace: For
new_without_cache(block_size == 0),EifHasher::write_alljust forwards to the underlyingSha384,finalize_reset()returnsSHA384(cert), andtpm_extend_finalize_reset()then computesSHA384(0u8*48 || SHA384(cert))on a fresh hasher — exactly what the new inline code does. - Empirical: I re-implemented both paths faithfully and confirmed identical output for empty, short, 48-byte, 256-byte, 100KB and PEM-like inputs.
SHA-384 itself is unchanged across the sha2 bump, so hash values are stable. Existing locked PCR8 values remain valid.
Dependency / build
EifHasherwas the crate's only usage (confirmed by grep); removal also cleanly dropsopenssl,clap 3,num-derive,crc 1.x, etc. via feature unification.- Lockfile now resolves a single
sha2 0.10.9. - CI
clippy_check_cli(which runscargo build,cargo nextest run -p ev-cli -p ev-enclave, fmt and clippy) passes.
Non-blocking suggestions
- Add a regression test pinning a fixed certificate to its expected PCR8 string. The equivalence was manually verified but nothing in the repo guards against a future refactor silently changing this attestation-critical value.
CertError::HashErroris no longer constructed anywhere (only matched inexitcode). It could be removed for cleanliness.
LGTM — approving.
| .map_err(|err| CertError::HashError(err.to_string()))?; | ||
|
|
||
| let hash = hex::encode(hash_bytes); | ||
| let hash = hex::encode(hasher.finalize()); |
There was a problem hiding this comment.
Verified this is byte-identical to the previous EifHasher::new_without_cache(...).write_all(...).tpm_extend_finalize_reset() path — for block_size == 0 that reduces to exactly SHA384(0u8*48 || SHA384(cert)), confirmed for empty/short/48B/256B/100KB inputs.
Since PCR8 is attestation-critical (a silent change here would invalidate every already-locked cert reference), consider adding a regression test that pins a fixed cert to its known PCR8 value so any future refactor of this block is caught automatically.
| let (_, pem) = parse_x509_pem(&cert_contents).map_err(CertError::PEMError)?; | ||
|
|
||
| let mut hasher = EifHasher::new_without_cache(Sha384::new()).map_err(CertError::HashError)?; | ||
| // PCR8 is a TPM-style extend of the cert digest into a zeroed register: |
There was a problem hiding this comment.
Nice — the comment documents the TPM-extend semantics clearly and matches the implementation. Minor: CertError::HashError is now unused as a constructor (the old EifHasher path was its only producer); it's still referenced in error.rs's exitcode match so it won't warn, but it could be removed for cleanliness.
Bumps sha2 from 0.9.9 to 0.10.9.
Commits
82c36a4sha2: add soft-compact backend (backport of #686) (#687)c1e85aeci: remove pre-1.56 jobs and other fixes (#688)a667dd9skein: fix implementation for output sizes not multiple of 8 (#682)62b6ff1Fix blake2 and ascon-hash CI (#521)f3b657cExpose belt_compress to public (#520)9f66cb8md4: Optimize compress to improve hash performance (#519)70a2b62Add simpler code snippet to the documentation (#515)e3ab257Update Cargo.lockc640781ascon-hash: Add Zeroize feature (#480)cfda39fUpdate Cargo.lock