NO-SNOW: FIPS phases 1-2: make fips reachable, consolidate TLS on aws-lc

[zeroshade]

What

Phases 1 and 2 of the internal FIPS compliance plan.

Phase 1 — make the FIPS TLS backend reachable. The feature existed on sf_core but no binding crate forwarded it, so it was physically unreachable from every shipped artifact. This adds fips-tls = ["sf_core/fips-tls"] passthroughs to odbc, jdbc_bridge, python_bridge, and nodejs_bridge, makes sf_core's feature explicit (["rustls/fips", "aws-lc-rs/fips"] — x509_utils calls aws_lc_rs directly and shouldn't rely on feature unification), removes the dead rustls-webpki 0.102 dependency, and adds runtime FIPS assertion tests (try_fips_mode(), provider.fips(), ClientConfig::fips()).

The feature is named fips-tls, not fips, because it swaps the TLS backend and nothing else: JWT signing, stage file encryption, DPoP and key parsing still run on OpenSSL, which has no FIPS-validated provider — and under vendored-openssl (which our release builds pass) that unvalidated copy is statically linked where a customer cannot replace it. A build combining the two is not a FIPS artifact, and the narrow name is what keeps it from being shipped as one; fips stays unclaimed for the complete thing after Phase 3. A compile_error! on the fips + vendored-openssl pair was the other candidate and was rejected for a mechanical reason: --all-features enables both by definition, so it would break the repo's own pre-commit clippy/check hooks and the full_put_get_tests lane.

Phase 2 — eliminate non-FIPS crypto reachability in the TLS stack.

• AWS Workload Identity and platform detection's STS probe now route through the driver's reqwest transport instead of the AWS SDK's bundled TLS stack. Independent of FIPS, this was a live TLS-policy hole: those STS calls honored none of the connection's TlsConfig (no CRL checking, no version window, no custom roots, no proxy). The SDK-backing client is the new AwsSdkReqwestClient newtype (formerly build_s3_reqwest_client), which applies the three SDK-owned transport adjustments (no redirects, no gzip auto-decompression, HTTP/1.1) and is the only type the SDK adapter accepts -- an unadjusted general-purpose client cannot back SigV4-signed SDK calls. In the login path it is built only when the configured WIF provider is AWS (the other providers never touch it), and it reuses the connection's shared CRL worker, threaded through snowflake_login_with_client / auth_request_data as a new SharedCrlWorker parameter, instead of spawning a per-login worker.
• New tls::ensure_crypto_provider() — Once-guarded aws-lc provider install at the driver-constructor chokepoint and every bare client-construction site.
• tls::fips_mode_active() is always compiled rather than feature-gated, and reports on the installed provider (false in a standard build). Phase 4 surfaces it through the wrappers as a customer-facing accessor, and an accessor that is absent without the feature would make "you installed the wrong artifact" indistinguishable from "your driver is too old to have it".
• WIF attestation fails closed in fips-tls builds: workload_identity::create_attestation runs ensure_crypto_provider() + require_fips_provider() at its entry, before any provider dispatch, surfacing a non-FIPS process-global provider as the new AttestationError::CryptoProvider — this also covers the wif_create_attestation RPC's plain client, which is not built through the TLS factories. The function is now pub(crate) (no external callers; keeping a public wrapper would preserve exactly the ungated path this closes).
• reqwest moved to the rustls-tls-*-no-provider feature variants and oauth2 to default-features = false (its default rustls-tls re-armed reqwest's silent ring fallback via feature unification). rustls 0.23 is now aws-lc-only; a client built before a provider install now panics loudly instead of silently degrading to ring. The trust-store composition (webpki + OS native roots) is unchanged.

What this deliberately does NOT do

No release-workflow FIPS lanes are added: shipping a "FIPS" artifact before the non-TLS crypto (JWT signing, stage encryption, DPoP, key parsing) is ported off vendored OpenSSL would invite a compliance claim the code does not support.

Verification

All numbers are current as of the rebase onto main @ 7c0ce98:

• cargo test -p sf_core --lib: 2084 passed / 0 failed
• cargo test -p sf_core --test integration_tests: 283 passed / 67 failed / 6 ignored — byte-identical failing-test list on clean main (re-verified post-rebase); the 67 are pre-existing env-dependent tests (dead-proxy sims, credential-requiring auth)
• cargo test -p sf_core --features fips-tls --lib: 2087 passed / 0 failed (the +3 are the FIPS-gated assertion tests; requires a GCC 13 toolchain for aws-lc-fips-sys 0.13.x — GCC 14+/clang 20 cannot build it, aws-lc-rs#569)
• All four binding crates cargo check clean with and without the feature; --all-features still compiles; fmt clean; clippy at main's own warning baseline
• The two secondary lockfiles that pin sf_core (python/Cargo.lock.sdist, tests/performance/drivers/core/app/Cargo.lock) are regenerated and pass cargo metadata --locked

Review notes

• The -no-provider switch means any test binary building raw reqwest::Clients must install a provider first; #[ctor] initializers in src/lib.rs (unit tests) and tests/common/crypto_provider.rs handle this -- the latter is picked up by integration_tests via tests/common/mod.rs and included directly by logging_query_tests. New dev-deps: rustls, ctor.
• In fips-tls builds, TLS client construction now fails closed: tls::require_fips_provider() is called from build_tls_client_and_rustls_config, configure_tls_builder, AwsSdkReqwestClient::build, and the WIF attestation entry point, so a fips-tls build whose process-global provider is not FIPS returns TlsError::FipsModeUnavailable through the normal FFI error path instead of serving traffic on a non-approved module. ensure_crypto_provider() itself still only logs, because it runs from constructors and paths with no error channel beneath an FFI boundary where unwinding is UB. Auxiliary raw clients (telemetry, CRL, IMDS) have no error channel and still rely on the logged mismatch.

[Copilot]

Pull request overview

This pull request advances the FIPS compliance plan by making the fips feature selectable from shipped binding crates and by consolidating the TLS stack onto rustls + aws-lc (with explicit provider installation) to prevent accidental fallback to non-FIPS crypto backends.

Changes:

• Forward sf_core’s fips feature through odbc, jdbc_bridge, python_bridge, and nodejs_bridge, and document the broader multi-phase compliance plan.
• Switch reqwest usage to rustls-tls-*-no-provider and add tls::ensure_crypto_provider() call sites so reqwest clients can’t silently resolve to ring.
• Route AWS STS calls (platform detection + WIF) through the driver’s shared reqwest transport to ensure connection TLS policy (roots/proxy/version/CRL) is consistently applied.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sf_core/tests/integration/mod.rs	Installs a default rustls provider early for integration tests that build ad-hoc reqwest clients.
sf_core/src/tls/mod.rs	Adds ensure_crypto_provider() and FIPS-mode runtime assertions/tests.
sf_core/src/tls/client.rs	Ensures provider installation occurs before any reqwest client/builder is constructed (including insecure paths).
sf_core/src/telemetry/platform_detection/mod.rs	Pins crypto provider before early platform-probe HTTP client creation; threads shared client into AWS identity probe.
sf_core/src/telemetry/platform_detection/aws.rs	Routes STS GetCallerIdentity through the shared reqwest transport and updates provider trait signature.
sf_core/src/rest/snowflake/workload_identity/aws.rs	Routes AWS SDK STS usage through shared reqwest transport; ensures provider before IMDS probe; threads client through credential resolution/assume-role.
sf_core/src/lib.rs	Installs default provider in unit-test builds to avoid order-dependent “No provider set” panics.
sf_core/src/crl/cache.rs	Pins provider before CRL HTTP client construction (including fallback minimal cache).
sf_core/src/apis/database_driver_v1/global_state.rs	Installs crypto provider at driver construction chokepoint (with_providers).
sf_core/Cargo.toml	Makes fips feature explicit (rustls/fips + aws-lc-rs/fips), switches reqwest features to -no-provider, disables oauth2 defaults, removes rustls-webpki 0.102, adds dev-deps for test harness provider installs.
python_bridge/Cargo.toml	Adds fips = ["sf_core/fips"] passthrough feature.
odbc/Cargo.toml	Adds fips = ["sf_core/fips"] passthrough feature.
nodejs_bridge/Cargo.toml	Adds fips = ["sf_core/fips"] passthrough feature.
jdbc_bridge/Cargo.toml	Adds fips = ["sf_core/fips"] passthrough feature.
docs/design/fips-compliance.md	Adds the FIPS gap analysis and phased remediation plan/design note.
Cargo.lock	Updates lockfile for new/removed deps (notably adding ctor 0.2.x and removing rustls-webpki 0.102.x / quinn-related entries).
.github/workflows/test-rust-core.yml	Updates Windows ARM64 non-FIPS lane notes about fips-gated test coverage.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

sf_core/src/tls/mod.rs:60

• fips_mode_active is currently pub, which makes it part of sf_core's public API (since tls is a public module). It doesn’t appear to be used outside tls, so consider reducing visibility to avoid committing to a new stable API surface.

pub fn fips_mode_active() -> bool {

sf_core/Cargo.toml:195

• sf_core adds a direct dev-dependency on ctor = "0.2", but the workspace already pulls in ctor 1.x (e.g., via napi), which results in multiple ctor versions in Cargo.lock. If the macro usage is compatible, consider using ctor = "1" here to reduce duplicate dependencies.

ctor = "0.2"

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

sf_core/Cargo.toml:197

• rustls is already a normal dependency of sf_core (with the required aws_lc_rs feature), so adding it again as a dev-dependency is redundant and can unintentionally enable additional default features for test builds. Keeping only ctor here should be sufficient for the test-only provider ctors.

rustls = "0.23"
ctor = "1"

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 4 comments.

Suppressed comments (1)

sf_core/src/rest/snowflake/mod.rs:793

• Using a fresh CrlWorker::new_lazy() here defeats the shared worker already used to build the connection client. With CRL checking enabled, CrlServerCertVerifier initializes this handle during the STS handshake, so each AWS WIF login/retry can spawn a separate crl-worker thread and then tear it down, creating thread churn under concurrent authentication. Thread the existing SharedCrlWorker through the login/auth-data path and pass it here instead.

                crate::crl::worker::CrlWorker::new_lazy(),

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

sf_core/src/rest/snowflake/mod.rs:794

• This client is built for every WIF provider, although only the AWS branch consumes aws_sdk_http. Azure/GCP/OIDC therefore pay for a second TLS/CRL client on each login, and a failure in this AWS-only setup can abort non-AWS attestation before its provider runs. Construct it only when cfg.provider == WifProvider::Aws and pass the resulting option through.

            let aws_sdk_http = crate::tls::aws_http_client::AwsSdkReqwestClient::build(
                &login_parameters.client_info.tls_config,
                Some(&login_parameters.client_info.proxy_config),
                crate::crl::worker::CrlWorker::new_lazy(),
            )

[Copilot]

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

sf_core/src/tls/aws_http_client.rs:10

• The module-level claim that every AWS SDK consumer inherits the connection TLS policy is inaccurate: with_default_tls() is intentionally used by platform detection and the no-connection WIF RPC, so those calls have no connection TlsConfig, CRL worker, or explicit proxy. Limit this claim to connection-scoped consumers or document the default-TLS exception.

//! stack that Azure and GCS transfers build via [`configure_tls_builder`], so every
//! AWS SDK consumer (S3 transfers, the WIF STS calls, platform detection's STS
//! probe) inherits one implementation of the connection's TLS policy (version
//! window, CRL, custom root store) and proxy handling (`proxy_host`/

[Copilot]

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 29 changed files in this pull request and generated no new comments.

Suppressed comments (1)

sf_core/src/tls/mod.rs:92

• The new FIPS tests cover only the successful aws-lc/provider/config path; they do not exercise this fail-closed branch when an embedding process has already installed a non-FIPS provider. Because the rustls provider is process-global, please add an isolated test binary/subprocess that installs the non-FIPS provider first and verifies TLS client construction returns FipsModeUnavailable instead of serving traffic.

pub(crate) fn require_fips_provider() -> Result<(), error::TlsError> {
    #[cfg(feature = "fips")]
    if !fips_mode_active() {
        return Err(error::FipsModeUnavailableSnafu.build());

[sfc-gh-pfus]

Strong PR. Two things stand out as genuinely subtle catches: hoisting ensure_crypto_provider() above the insecure early-returns closes a path where a verify_certificates=false client built first in a process would have silently resolved to ring, and spotting that oauth2's default features forward to reqwest/rustls-tls — re-arming __rustls-ring for the whole build through feature unification — is not obvious from reading either manifest.

The require_fips_provider() / ensure_crypto_provider() split is also the right shape: fail closed where there's an error channel, log where unwinding beneath FFI would be undefined behaviour.

One substantive request (the fips feature guard, inline on sf_core/Cargo.toml) — happy for that to be a follow-up PR rather than here, as long as it lands before we start building FIPS release artifacts. Everything else is a note or a nit.

[sfc-gh-pfus]

Just so this isn't confusing to read: nothing in this repo currently executes tls::fips_tests — lines 125 and 136 are cargo build and cargo test --no-run, so the tests compile but never assert. That's expected given how CI is split here, and adding the FIPS build and test lanes is on our side after this merges.

No action needed from you; flagging only so a future reader doesn't assume the assertions are already gating anything.

[zeroshade]

Understood, and I have made it explicit in the file rather than leaving it to be re-derived: the comment above the compile step now records that these two lanes are the only places fips-tls is turned on, that neither executes a test, and that the assertions therefore compile but never assert until the FIPS lanes exist on your side.

Flagging one consequence for whoever writes those lanes: tls::fips_tests asserts on the installed process-default provider, so it has to run in a binary where ensure_crypto_provider() has run — the test calls it, so cargo test --features fips-tls --lib is sufficient, but a lane that only builds proves nothing.

[zeroshade]

Follow-up: an automated reviewer independently flagged this same gap on the rebased commit (the --no-run lane not executing tls::fips_tests). I closed it pointing at your comment, since the lane needs the pinned GCC 13 toolchain for aws-lc-fips-sys and is yours to add — recording it here so the agreement is visible on the thread rather than only in my tooling.

[sfc-gh-pfus]

The scope note is exactly right, and thanks for making it explicit rather than leaving it implicit in the call sites. Noting for our side: we'll carry this into the compliance exception list, since telemetry / CRL fetch / IMDS proceeding on a non-FIPS provider with only a log line is the kind of thing that needs a written position rather than living in a code comment.

For what it's worth I think it's defensible as written — the exposure only materialises when an embedding application wins the provider race, and CRL fetch retrieves publicly signed data rather than protecting secrets.

[zeroshade]

Thanks — and agreed the compliance exception list is the right home for it rather than a code comment. The scope note is written to be the thing you can lift into that list: the three clients are named, and the reason they cannot fail closed (no error channel beneath an FFI boundary where unwinding is UB) is stated rather than implied, so the exception can be justified rather than just recorded.

One correction to carry over, since it changed after you read this: WIF attestation is no longer in that set. It now fails closed at the create_attestation entry point, which covers the wif_create_attestation RPC path that does not go through the TLS factories. The remaining exceptions are exactly telemetry, CRL fetch and IMDS. Your read on the residual exposure matches mine — it needs an embedding application to win the provider race, and CRL fetch retrieves publicly signed data.

[sfc-gh-pfus]

Agreed this preserves the previous composition exactly, and preserving it is the right call for a PR scoped like this one. Flagging rather than requesting a change: the effect is that we trust the bundled webpki roots union the OS trust store, so an enterprise that removes a CA from its OS store still has us trusting it via the Mozilla bundle.

That's a behaviour question rather than a FIPS one, and it needs a decision on our side before anyone changes it — but I'd rather it be an open question than settle quietly as documented-intent, since FIPS-conscious customers are exactly the population that curates its trust store.

[zeroshade]

Agreed on both counts — preserved deliberately here, and worth a real decision rather than quiet inheritance. To be precise about what is inherited: rustls-tls-webpki-roots-no-provider + rustls-tls-native-roots-no-provider is the same union the previous rustls-tls + rustls-tls-native-roots selection produced, so this PR changes the provider underneath, not the trust set.

Your framing of the risk is the sharp one: an enterprise that removes a CA from its OS store does not thereby stop us trusting it, because the Mozilla bundle still carries it — and removal, not addition, is how a curated store expresses distrust. The narrowing change would be dropping the webpki bundle and going OS-only, which is a behaviour change with real blast radius (the bundle is what makes us work on hosts with a thin or stale store). I have not touched it, and I would rather it be decided on its own merits than as a side effect of a FIPS PR. Happy to file it as a separate issue if you want it tracked somewhere more durable than this thread.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 29 changed files in this pull request and generated 2 comments.

Suppressed comments (5)

sf_core/src/protobuf/apis/database_driver_v1/mod.rs:1282

• The feature is intentionally named fips-tls, while fips is reserved for the future complete artifact. This new comment should use the actual feature name so it does not document an unavailable configuration.

        // in a `fips` build.

sf_core/src/rest/snowflake/workload_identity/mod.rs:80

• The feature is intentionally named fips-tls, while fips is reserved for the future complete artifact. This new error documentation also says every provider exchanges material over the supplied clients, but the OIDC provider makes no request; please use the actual feature name and qualify this as applying to networked providers.

    /// Raised before any provider is dispatched, in `fips` builds whose
    /// process-global rustls provider is not FIPS. All providers exchange
    /// authentication material over the caller-supplied clients, so the same
    /// fail-closed gate the TLS factories apply belongs here too -- notably

sf_core/src/rest/snowflake/workload_identity/mod.rs:175

• OIDC does not make a network request (the RPC path documents this below), so the statement that every provider exchanges authentication material over the supplied clients is inaccurate. Please qualify this explanation as applying to networked providers while retaining the separate process-wide gate.

    // Every provider exchanges authentication material over the supplied
    // clients. Pinning and gating the crypto backend at this single entry
    // point gives a caller-built plain client (the `wif_create_attestation`
    // RPC) the same fail-closed FIPS behaviour as clients built by the TLS
    // factories, which run both calls in `configure_tls_builder`. Redundant

sf_core/src/tls/client.rs:66

• The feature is intentionally named fips-tls, while fips is reserved for the future complete artifact. This comment therefore refers to a nonexistent or ambiguous feature; please use the actual feature name here.

    // Fail closed rather than serve traffic on a non-approved module: in `fips`
    // builds this refuses to build a client when the provider that won the
    // process-global slot is not FIPS. Compiles away without the feature.

sf_core/src/tls/mod.rs:27

• With the -no-provider reqwest features selected above, a client built before ensure_crypto_provider() panics with No provider set; it cannot silently resolve to another backend. This new documentation should describe the fail-fast behavior so future call sites are not audited against a fallback that no longer exists.

/// than ad hoc next to each client, is what stops a client built early in the
/// process (telemetry, CRL prefetch, cloud transfers) from silently resolving
/// to a different backend than the one carrying the session's own traffic --
/// under `--features fips-tls` that difference is the whole compliance claim.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 29 changed files in this pull request and generated 4 comments.

Suppressed comments (4)

.github/workflows/test-rust-core.yml:142

• The only runtime FIPS assertions added by this PR are compiled here but never executed: this step uses --no-run, and the remaining test matrix is non-FIPS. A regression in provider installation or ClientConfig::fips() can therefore pass CI; run the focused tls::fips_tests (or add an equivalent executing FIPS lane) instead of compile-only coverage.

        run: cargo test --locked --no-run --lib --package sf_core --no-default-features --features fips-tls

.github/workflows/test-rust-core.yml:399

• These newly added CI comments still describe the comparison as being 'with and without fips', although the feature renamed in this PR is fips-tls and fips is intentionally unclaimed. Use the actual feature name so the ARM64 rationale remains searchable and accurate.

  # and without fips; only the linked TLS crypto backend differs

sf_core/Cargo.toml:27

• The feature rename leaves docs/woa64/ADR-woa64.md:136 describing the workaround as dropping the old fips feature. After this manifest change that troubleshooting guidance points at a flag that is no longer declared; update the ADR (and related wording) to refer to fips-tls or FIPS TLS.

fips-tls = ["rustls/fips", "aws-lc-rs/fips"]

sf_core/src/tls/mod.rs:72

• This helper is exposed as pub fn from the already-public tls module, creating a new sf_core API before the Phase 4 wrapper exists. The function is only an internal provider check in this phase; keep it pub(crate) and introduce the supported wrapper surface when that phase is implemented, rather than committing this low-level helper as API.

pub fn fips_mode_active() -> bool {