envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.71k stars 4.75k forks source link

Update boringcrypto to enable TLS 1.3 without CHACHA stream ciphers in FIPS builds #31746

Open valerian-roche opened 8 months ago

valerian-roche commented 8 months ago

FIPS requirements require support of TLS 1.3 as of Jan 1st 2024, and current boringcrypto version and lack of cipher deactivation prevent its use in envoy FIPS builds

go has decided in 1.21.6 to migrate to the candidate FIPS version of boringcrypto and enable TLS 1.3, as FIPS regulation mandates TLS 1.3 as of Jan 1st 2024

Agencies shall support TLS 1.3 by January 1, 2024. After this date, servers shall support TLS 1.3 for both government-only and citizen or business-facing applications. In general, servers that support TLS 1.3 should be configured to use TLS 1.2 as well. However, TLS 1.2 may be disabled on servers that support TLS 1.3 if it has been determined that TLS 1.2 is not needed for interoperability.

In order to be compliant and use TLS 1.3, we’d need this version of boringcrypto and its ability to deactivate CHACHA cipher in TLS 1.3 bound in envoy to use it (either using the new policy context or explicitly setting ciphers) Go took the stance that support of TLS 1.3 takes precedence over certification of this boringcrypto version.

Is the position of envoy maintainers the same or to wait for the cerfication? Or will it wait for the certificate prior to progress on this?

kyessenov commented 8 months ago

FIPS compliant Envoy required TLS 1.2 as the maximum version.

Currently TLS 1.3 in Envoy FIPS mode allows using chacha20 ciphersuite because Envoy does not prevent boringcrypto from selecting it. I tested it as follows:

> openssl s_client -connect localhost:8081 -ciphersuites TLS_CHACHA20_POLY1305_SHA256 -CAfile testdata/certs/root.cert
New, TLSv1.3, Cipher is TLS_CHACHA20_POLY1305_SHA256
Server public key is 2048 bit
Secure Renegotiation IS NOT supported
Compression: NONE
Expansion: NONE
No ALPN negotiated
Early data was not sent
Verify return code: 0 (ok)
> curl -s localhost:15000/stats | grep fips
server.compilation_settings.fips_mode: 1
phlax commented 8 months ago

cc @ggreenway

kyessenov commented 8 months ago

I got some clarification on the situation from @agl. There is no requirement to support TLS 1.3 in FIPS-140 in general, which is understood to be a certification program for crypto modules via https://csrc.nist.gov/projects/cryptographic-module-validation-program. The policy to support TLS 1.3 applies to government agencies, separately from FIPS-140.

Additionally, FIPS-140 only affects boringssl-crypto and not the TLS stack. We are allowed to patch TLS stack and remove ChaCha20-Poly1305 ciphersuite from boringssl in FIPS mode when using TLS 1.3.

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

wadey commented 1 month ago

FYI the updated BoringCrypto FIPS module has now been published: https://csrc.nist.gov/projects/cryptographic-module-validation-program/certificate/4735

ggreenway commented 1 month ago

I started the work to upgrade, but I'm hitting a test failure in the BoringSSL tests. See #35534.

dio commented 1 month ago

Copy over my comment on the PR https://github.com/envoyproxy/envoy/pull/35534 here:

@ggreenway @kyessenov I think the certs in that particular test (e.g. https://github.com/google/boringssl/blob/0c6f40132b828e92ba365c6b7680e32820c63fa7/ssl/ssl_test.cc#L8014 are "expired" ("Not After" 2022-12-06).

Probably the following options are viable:

  1. Replace the certs
  2. Set X509_VERIFY_PARAM_set_flags with the X509_V_FLAG_NO_CHECK_TIME (this is added in later commits, I tested the following patch, and it works: https://gist.github.com/dio/f1de152a4cc2af5508a962ed43d10a98)

but yeah both options require patching the boringssl source (but only on test).

howardjohn commented 2 weeks ago

I think we need https://github.com/google/boringssl/blob/ec09a2dad2781e35bdb2a3b873ff395b4376e444/ssl/ssl_lib.cc#L3421 (SSL_CTX_set_compliance_policy) to be settable as well ?