Conversation
| @@ -270,7 +270,6 @@ hex = "0.4" | |||
| memchr = "2" | |||
| sonic-rs = { version = "0.5", optional = true } | |||
| sha2 = "0.10" | |||
There was a problem hiding this comment.
AWS-LC already has SHA-256 (digest::SHA256). This PR correctly drops the direct hmac crate for WIF, but sha2 stays as a first-party dep for leftover call sites that are mechanical to port:
crl/cache.rsurl_digest(cache filename)token_cachebuild_cache_key(lookup id)s3_transfer.rss3_creds_fingerprint(client-cache key)tls/x509_utils.rssubject_der_hash/issuer_der_hash(Name DER map keys for chain building, not CRL signature verify)
None of those needs a second hash implementation. Please switch them to aws_lc_rs::digest in this PR (or a fast follow) where it is that mechanical, and drop the direct sha2 dep if nothing in sf_core calls it afterwards.
Transitive sha2 via aws-sigv4 / pkcs8 is a separate inventory item — not asking you to fight those here. OAuth PKCE S256 is F8 if this is not stacked on #1349.
There was a problem hiding this comment.
Addressed in c6aa8ad. The CRL URL digest, token-cache key, S3 credential fingerprint, and X.509 Name DER map keys now use aws_lc_rs::digest::SHA256. I removed sf_core’s direct sha2 dependency from its manifest and all three lockfiles; transitive sha2 remains for SDK/PKCS#8 consumers. Existing token-cache golden vectors and new CRL/S3 vectors preserve the identifier bytes. The standard and FIPS sf_core library suites passed (2,359 and 2,360 tests respectively), and a public token-cache API smoke run produced the established MFA key. OAuth PKCE production behavior and the SDK-owned SigV4 signer remain separate work.
zeroshade
changed the title
|
Merged in private repo. |
…SigV4 signing through AWS-LC Imported from #1352. Original PR body: This PR routes the driver's own AWS workload-identity `GetCallerIdentity` SigV4 request hashing and HMAC-SHA256 through AWS-LC. It removes the now-unused direct `hmac` dependency and its obsolete initialization error; the AWS SDK's separate S3 signer is unchanged. The WIF attestation signer currently uses RustCrypto even in `fips-tls` builds. That leaves a security-sensitive signing operation outside the selected AWS-LC module despite the TLS and direct application-crypto ports in SNOW-3927213. The local signer retains its canonical request, signing-key derivation, and Authorization format, changing only the cryptographic backend to `aws_lc_rs::digest` and `aws_lc_rs::hmac`. Infallible AWS-LC key construction removes an error state that no longer exists. The SDK-owned signing path remains a separate upstream-dependent change. The primary risk is a changed signature for requests with or without session credentials. Two fixed, independently derived SigV4 Authorization values cover both cases and catch differences in canonical headers, scope, or HMAC chaining. This does not establish a whole-artifact FIPS compliance claim. Verification: `cargo test --locked -p sf_core --lib workload_identity::aws` passed 23 tests; the corresponding `--no-default-features --features fips-tls,protobuf` run passed 23 tests with the GCC 13 FIPS toolchain. Both reduced-workspace lockfile checks, `cargo fmt --all -- --check`, and `git diff --check` passed. `roborev refine --branch fips-wif-aws-lc --since cab14fc` passed its branch review. The AWS SDK's ordinary S3 SigV4 signer remains dependent on upstream smithy-rs provider support; that work is intentionally not stacked on this PR. --- Internal CI validates this change before merge. On merge to main the outbound mirror will push the commit back to the public repo; close the original mirror PR with a link to the mirrored commit. --------- Co-authored-by: Matt Topol <zotthewizard@gmail.com> Co-authored-by: Piotr Fus <piotr.fus@snowflake.com> GitOrigin-RevId: f5f1f7bbaf42de2c01f7b7f27cdd73f3feec4ac7
This PR routes the driver's AWS workload-identity
GetCallerIdentitySigV4 hashing and HMAC-SHA256 through AWS-LC. It also moves the remaining first-party SHA-256 calls for CRL cache filenames, token-cache keys, S3 credential fingerprints, and certificate-name map keys toaws_lc_rs::digest, removing the directhmacandsha2dependencies fromsf_core. The AWS SDK's separate S3 signer is unchanged.The WIF attestation signer previously used RustCrypto even in
fips-tlsbuilds. The other first-party hash calls usedsha2to derive identifiers and chain-selection map keys despite AWS-LC already providing SHA-256, as noted in the review of this PR. These operations should use the selected module without changing their persisted or cross-driver values. SNOW-3927213 tracks the broader FIPS work; this does not establish a whole-artifact compliance claim.The WIF signer retains its canonical request, signing-key derivation, and Authorization format, changing only the cryptographic backend to
aws_lc_rs::digestandaws_lc_rs::hmac. The remaining SHA-256 calls usedigest::digest(&digest::SHA256, ...)on the same input bytes and preserve their hexadecimal or raw-byte outputs. Infallible AWS-LC HMAC key construction removes an error state that no longer exists.sha2remains in the dependency graph through transitiveaws-sigv4/pkcs8paths; the SDK-owned signer and OAuth PKCE S256 are tracked separately.The main risk is changing a WIF signature or a persisted cache identifier. Fixed SigV4 Authorization vectors cover requests with and without session credentials; fixed CRL and S3 hash vectors plus existing token-cache golden vectors guard compatibility. The certificate-name digest remains a map key for chain construction, not a substitute for signature verification.
Verification: the
sf_corelibrary suite passed 2,359 tests (one ignored) in standard mode and 2,360 tests (one ignored) with--no-default-features --features fips-tls,protobuf. An executable using the publicbuild_cache_keyAPI produced the establishedSnowflakeTokenCache.v2.MfaTokenvector.cargo fmt --all -- --checkandgit diff --checkpassed. The original WIF-focused runs passed 23 tests in each mode, and both reduced-workspace lockfile checks passed before this review update.The SDK's ordinary S3 SigV4 signer remains dependent on upstream smithy-rs provider support; that work is intentionally not stacked on this PR.