chore(tls): use CNG rustls provider on Windows#1929
chore(tls): use CNG rustls provider on Windows#1929gh-worker-dd-mergequeue-cf854d[bot] merged 10 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
Binary Size Analysis (Agent Data Plane)Baseline: 6860990 · Comparison: fc79c87 · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
| { | ||
| let mut provider = provider; | ||
| provider.cipher_suites.retain(|suite| suite.fips()); | ||
| provider.kx_groups.retain(|group| group.fips()); |
There was a problem hiding this comment.
Is this replacing a fips feature provided by rustls or AWS LC themselves? Can you verify this is correct?
There was a problem hiding this comment.
[GPT-5.5] Verified and adjusted the approach after this comment.
This no longer uses/replaces rustls/fips directly. rustls/fips still implies rustls/aws_lc_rs, which would make Windows FIPS builds pull AWS-LC again. Instead, Saluki’s single fips feature now enables the platform provider’s FIPS mode:
- non-Windows:
aws-lc-rs/fips, with Saluki filtering the AWS-LC provider to FIPS-approved cipher suites and key-exchange groups before installing it. - Windows:
rustls-cng-crypto/fips.
I verified the graph behavior:
cargo tree --locked -p saluki-tls --features fips -i aws-lc-fips-sysshows AWS-LC FIPS on the host/non-Windows path.cargo tree --locked -p agent-data-plane --target x86_64-pc-windows-msvc --features fips -i aws-lc-rsprints nothing.cargo check --locked -p saluki-tls --target x86_64-pc-windows-msvc --features fipspasses.
There was a problem hiding this comment.
This is still necessary with aws-lc-rs/fips enabled?
There was a problem hiding this comment.
[GPT-5.5] Yes, this filtering is still necessary with aws-lc-rs/fips enabled.
The reason is that we intentionally do not enable rustls/fips, because rustls/fips implies rustls/aws_lc_rs and would make Windows FIPS pull AWS-LC again. Enabling aws-lc-rs/fips switches the AWS-LC backend to FIPS, but Rustls’ AWS-LC provider only filters its default cipher suites/KX groups behind Rustls’ own cfg(feature = "fips").
I verified this in the Rustls source: rustls::crypto::aws_lc_rs::default_provider() always starts from DEFAULT_CIPHER_SUITES, and the CHACHA suites are excluded only under Rustls’ own feature = "fips". Since this PR avoids rustls/fips, Saluki still needs to retain only suite.fips() and group.fips() before installing the provider.
The other FIPS pieces are handled separately: aws-lc-rs/fips enables the AWS-LC FIPS backend and rustls-webpki/aws-lc-rs-fips makes the WebPKI signature algorithms report FIPS. The local Linux FIPS integration run (30 passed, 0 failed) confirms the resulting ClientConfig::fips() path works at startup/runtime.
|
|
||
| #[cfg(windows)] | ||
| fn default_crypto_provider() -> CryptoProvider { | ||
| rustls_cng_crypto::default_provider() |
There was a problem hiding this comment.
And for this one, how is FIPS propagated?
There was a problem hiding this comment.
[GPT-5.5] FIPS is propagated through the existing Saluki fips feature now:
fips = ["aws-lc-rs/fips", "rustls-cng-crypto/fips"]The dependencies are target-scoped, so only the relevant provider exists for the target:
aws-lc-rsis a non-Windows dependency and gets itsfipsfeature there.rustls-cng-cryptois a Windows dependency and gets itsfipsfeature there.
On Windows, the pinned rustls-cng-crypto crate uses its fips feature to make default_provider() return its FIPS provider. Verified with:
cargo tree --locked -p agent-data-plane --target x86_64-pc-windows-msvc --features fips -i rustls-cng-cryptoshows the CNG provider.cargo tree --locked -p agent-data-plane --target x86_64-pc-windows-msvc --features fips -i aws-lc-rsprints nothing.
| "tls12", | ||
| ] } | ||
| rustls-cng-crypto = { version = "0.2.0", git = "https://github.com/DataDog/rustls-cng-crypto", rev = "2498c870c949335cb70dc79ed558917b96e4ff72", default-features = false, features = [ | ||
| "tls12", |
There was a problem hiding this comment.
Explain inclusion of this feature and omission of any rustls-cng-crypto features you aren't including
There was a problem hiding this comment.
[GPT-5.5] The workspace dependency enables only tls12 intentionally.
rustls-cng-crypto is declared with default-features = false, so we explicitly opt into tls12 because Saluki supports TLS 1.2 and the existing Rustls workspace dependency also enables tls12. Without this, the CNG provider would only expose TLS 1.3 suites.
The omitted features are intentional:
default: not used because it currently only maps totls12; we keep that explicit for consistency with the rest of the workspace.fips: not enabled globally because FIPS must be controlled by Saluki’s existingfipsfeature.lib/saluki-tls/Cargo.tomlnow enablesrustls-cng-crypto/fipsonly when Saluki’sfipsfeature is enabled.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03051b6f4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [features] | ||
| default = [] | ||
| fips = ["rustls/fips"] | ||
| fips = ["aws-lc-rs/fips", "rustls-cng-crypto/fips"] |
There was a problem hiding this comment.
Restore rustls FIPS policy feature
On non-Windows --features fips builds this no longer enables rustls/fips, so rustls keeps the non-FIPS webpki signature algorithm set while ClientTLSConfigBuilder::build() still rejects any config for which config.fips() is false. Filtering cipher suites and KX groups later does not update signature_verification_algorithms, so FIPS Linux ADP clients fail to construct TLS configs at runtime with “Client TLS configuration is not FIPS compliant.” Keep rustls/fips on non-Windows or explicitly enable/filter the webpki FIPS algorithms.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[GPT-5.5] Fixed in 889ba3a0d7.
This was valid: CryptoProvider::fips() also checks signature_verification_algorithms.fips(), so filtering only cipher suites and KX groups was incomplete.
The single Saluki fips feature now additionally enables rustls-webpki/aws-lc-rs-fips on the non-Windows path:
fips = ["aws-lc-rs/fips", "rustls-cng-crypto/fips", "rustls-webpki/aws-lc-rs-fips"]rustls-webpki is a non-Windows target dependency, so Windows still does not pull AWS-LC. Verified with:
cargo tree --locked -p saluki-tls --features fips -e features -i rustls-webpkishowingrustls-webpki feature "aws-lc-rs-fips".cargo tree --locked -p agent-data-plane --target x86_64-pc-windows-msvc --features fips -i aws-lc-rsprinting nothing.cargo check --locked -p saluki-tls --target x86_64-pc-windows-msvc --features fipspassing.
There was a problem hiding this comment.
[GPT-5.5] Additional local verification after fixing this:
- Built the Linux FIPS ADP image with
make build-adp-image-fips. - Built a bundled Datadog Agent image using that FIPS ADP image and tagged it for the Linux integration runtime.
- Ran
basic-startupunder the Linux runtime against that FIPS image:PASSED: 1 passed, 0 failed. - Ran the full Linux integration suite under the Linux runtime against that FIPS image:
PASSED: 30 passed, 0 failed, 30 total.
So the non-Windows FIPS path no longer just compiles by dependency graph; it also boots and passes the Linux integration suite locally. I also rechecked the Windows FIPS graph/compile path separately: Windows --features fips uses rustls-cng-crypto, has no aws-lc-rs edge, and cargo check --locked -p saluki-tls --target x86_64-pc-windows-msvc --features fips passes.
| New-VsBuildToolsJunction | ||
| } | ||
|
|
||
| # saluki-metadata reads these at build time. Must match the values the Makefile passes through |
There was a problem hiding this comment.
Update Windows FIPS packaging for CNG
With this block removed, the build-release-zip-windows-amd64-fips workflow still sets BUILD_FEATURES=fips and this script still calls package-adp-zip.ps1; that packaging script currently treats any FIPS Windows build as aws-lc-backed and throws when no aws_lc_fips_*.dll is found (ci/tooling/package-adp-zip.ps1:79-87). Since the new CNG-backed FIPS build intentionally produces no AWS-LC DLL, the release zip job will fail after cargo succeeds unless the packaging condition is updated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[GPT-5.5] Fixed in 889ba3a0d7.
Windows FIPS now uses the CNG-backed Rustls provider and intentionally does not produce aws_lc_fips_*.dll, so the packaging script should not require or bundle AWS-LC FIPS DLLs anymore. I removed that block from ci/tooling/package-adp-zip.ps1 and left a short comment explaining that Windows FIPS uses CNG.
There was a problem hiding this comment.
[GPT-5.5] Additional verification after removing the AWS-LC FIPS DLL packaging requirement:
- Windows
--features fipsdependency graph has noaws-lc-rs/aws-lc-sysedge and does includerustls-cng-crypto. cargo check --locked -p saluki-tls --target x86_64-pc-windows-msvc --features fipspasses locally.
I did not run a Windows runtime boot locally, but the packaging change matches the verified dependency graph: Windows FIPS no longer builds or links AWS-LC FIPS, so no aws_lc_fips_*.dll should be required in the zip. For runtime startup coverage, I did run the Linux FIPS image locally through basic-startup and the full Linux integration suite; both passed.
| Copy-Item -Force $dll.FullName (Join-Path $StageRoot "bin\$($dll.Name)") | ||
| } | ||
| } | ||
| # Windows FIPS builds use the CNG-backed Rustls provider, so no AWS-LC FIPS runtime DLLs are produced or bundled. |
There was a problem hiding this comment.
Bad comment, remove
There was a problem hiding this comment.
[GPT-5.5] Removed in fc79c87af7.
## Summary Switch Windows Rustls builds to Datadog's CNG-backed Rustls provider. ## Key changes - Add `rustls-cng-crypto` from `DataDog/rustls-cng-crypto` at version `0.2.0`, rev `2498c870c949335cb70dc79ed558917b96e4ff72`. - Install the CNG provider on Windows and keep AWS-LC as the provider on non-Windows platforms. - Keep a single `fips` feature: on non-Windows it enables AWS-LC FIPS mode and Rustls WebPKI's AWS-LC FIPS verifier algorithms, then filters the AWS-LC provider to FIPS-approved suites/groups; on Windows it enables the CNG provider's FIPS mode. - Stop enabling Rustls AWS-LC provider features globally so Windows dependency graphs do not pull `aws-lc-rs`, `aws-lc-sys`, or `aws-lc-fips-sys`. - Use provider-neutral `reqwest` Rustls features in `panoramic` and target-specific `rcgen` crypto backends. - Add a Windows-only `saluki-tls` provider-construction smoke test. - Allow-list the `rustls-cng-crypto` Git source in `deny.toml` and regenerate `LICENSE-3rdparty.csv` for the new dependency graph. - Stop packaging AWS-LC FIPS runtime DLLs in Windows FIPS zips because Windows FIPS now uses CNG. ## Test plan - `make fmt` - `cargo check --locked -p saluki-tls --target x86_64-pc-windows-msvc --features fips` - `cargo tree --workspace --target x86_64-pc-windows-msvc --edges normal -i aws-lc-sys` - `cargo tree --locked -p agent-data-plane --target x86_64-pc-windows-msvc --features fips -i aws-lc-rs` - `cargo tree --locked -p agent-data-plane --target x86_64-pc-windows-msvc --features fips -i rustls-cng-crypto` - `cargo tree --locked -p saluki-tls --features fips -i aws-lc-fips-sys` *(shows non-Windows AWS-LC FIPS graph; local compile requires cmake)* - `cargo tree --locked -p saluki-tls --features fips -e features -i rustls-webpki` *(shows `rustls-webpki/aws-lc-rs-fips`)* - `cargo check --workspace` - `cargo check --workspace --tests` - `cargo nextest run -p saluki-tls` - `cargo nextest run -p saluki-io net::client::http::conn::tests::` - `cargo deny check advisories sources` - `make check-licenses` - Pre-commit checks Co-authored-by: travis.thieman <travis.thieman@datadoghq.com> 13f34fa
Summary
Switch Windows Rustls builds to Datadog's CNG-backed Rustls provider.
Key changes
rustls-cng-cryptofromDataDog/rustls-cng-cryptoat version0.2.0, rev2498c870c949335cb70dc79ed558917b96e4ff72.fipsfeature: on non-Windows it enables AWS-LC FIPS mode and Rustls WebPKI's AWS-LC FIPS verifier algorithms, then filters the AWS-LC provider to FIPS-approved suites/groups; on Windows it enables the CNG provider's FIPS mode.aws-lc-rs,aws-lc-sys, oraws-lc-fips-sys.reqwestRustls features inpanoramicand target-specificrcgencrypto backends.saluki-tlsprovider-construction smoke test.rustls-cng-cryptoGit source indeny.tomland regenerateLICENSE-3rdparty.csvfor the new dependency graph.Test plan
make fmtcargo check --locked -p saluki-tls --target x86_64-pc-windows-msvc --features fipscargo tree --workspace --target x86_64-pc-windows-msvc --edges normal -i aws-lc-syscargo tree --locked -p agent-data-plane --target x86_64-pc-windows-msvc --features fips -i aws-lc-rscargo tree --locked -p agent-data-plane --target x86_64-pc-windows-msvc --features fips -i rustls-cng-cryptocargo tree --locked -p saluki-tls --features fips -i aws-lc-fips-sys(shows non-Windows AWS-LC FIPS graph; local compile requires cmake)cargo tree --locked -p saluki-tls --features fips -e features -i rustls-webpki(showsrustls-webpki/aws-lc-rs-fips)cargo check --workspacecargo check --workspace --testscargo nextest run -p saluki-tlscargo nextest run -p saluki-io net::client::http::conn::tests::cargo deny check advisories sourcesmake check-licenses