chore(agent-data-plane): switch to CNG for TLS backend on Windows#1887
chore(agent-data-plane): switch to CNG for TLS backend on Windows#1887tobz wants to merge 6 commits into
Conversation
|
Binary Size Analysis (Agent Data Plane)Baseline: 3ecd1e0 · Comparison: 95c806f · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (35)Experiments configured
Bounds Checks: ❌ Failed (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 ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4d366e06b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # see `saluki-tls`. Pinned to an upstream commit past the 0.1.0 crates.io release: 0.1.0 directly depends | ||
| # on rustls-webpki 0.102 (open RUSTSEC advisories), which upstream has since dropped (alg_id now comes from | ||
| # rustls-pki-types). Switch back to a crates.io version once a release newer than 0.1.0 is published. | ||
| rustls-cng-crypto = { git = "https://github.com/tofay/rustls-cng-crypto", rev = "955b52e27ac76041e04459122ab4652875ab39a8", default-features = false, features = ["tls12"] } |
There was a problem hiding this comment.
Patch the broken CNG AES-256 ECDSA suite
At the pinned rustls-cng-crypto rev 955b52e, its TLS 1.2 TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 suite is wired to AES_128_GCM; because this change installs that provider for Windows, any Windows TLS 1.2 connection to an ECDSA endpoint that selects that common AES-256 suite derives the wrong traffic keys and fails during the handshake/first record. Please filter that suite with a custom provider or patch/fork the provider before making it the Windows default.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We will fix this bug and others we are finding in rustls-cng-crypto in a fork of that repository
thieman
left a comment
There was a problem hiding this comment.
Looks good for now but we need to fix some of the issues in rustls-cng-crypto and switch to our fork before merging this, or at least before our next release
95c806f to
3ff25a1
Compare
|
Closing in favor of #1929 |

Summary
This PR switches the
rustlscrypto provider backend to the native Microsoft crypto stack (CNG) for Windows builds.Currently, we use AWS-LC for all platforms, which functionally is correct but has one big problem: FIPS validation on Windows. While AWS-LC FIPS 3.x is validated for Linux x86_64/aarch64, it is not validated for Windows at all. We don't want to spend time or energy trying to deal with auditors to argue about how it may or may not be fine to call it vendor-affirmed... we just want something that is unquestionably already validated.
This PR switches to using
rustls-cng-cryptoon Windows, which uses Cryptography API: Next Generation (CNG), a native cryptography library provided through Windows itself: nothing to bundle with ADP, it's just available for us. Additionally, Microsoft has already gone through the hard part of getting FIPS validation for CNG, which is the same approach that the Datadog Agent takes, actually, for delivering a FIPS compliant build on Windows.In order to keep things simple, we're using the CNG backend for all Windows builds, rather than just Windows FIPS builds.
Change Type
How did you test this PR?
References
DADP-2