Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: default cipher suite docs are misleading for server and client context #16469

Closed
desimone opened this issue May 12, 2021 · 1 comment · Fixed by #16474
Closed

doc: default cipher suite docs are misleading for server and client context #16469

desimone opened this issue May 12, 2021 · 1 comment · Fixed by #16474

Comments

@desimone
Copy link
Contributor

desimone commented May 12, 2021

Title: default cipher suite docs are misleading for server and client context

Description:

The default tls client config has changed since v1.16 to be distinct from the default server config.

From

const std::string ClientContextConfigImpl::DEFAULT_CIPHER_SUITES =
#ifndef BORINGSSL_FIPS
"[ECDHE-ECDSA-AES128-GCM-SHA256|ECDHE-ECDSA-CHACHA20-POLY1305]:"
"[ECDHE-RSA-AES128-GCM-SHA256|ECDHE-RSA-CHACHA20-POLY1305]:"
#else // BoringSSL FIPS
"ECDHE-ECDSA-AES128-GCM-SHA256:"
"ECDHE-RSA-AES128-GCM-SHA256:"
#endif
"ECDHE-ECDSA-AES128-SHA:"
"ECDHE-RSA-AES128-SHA:"
"AES128-GCM-SHA256:"
"AES128-SHA:"
"ECDHE-ECDSA-AES256-GCM-SHA384:"
"ECDHE-RSA-AES256-GCM-SHA384:"
"ECDHE-ECDSA-AES256-SHA:"
"ECDHE-RSA-AES256-SHA:"
"AES256-GCM-SHA384:"
"AES256-SHA";
const std::string ClientContextConfigImpl::DEFAULT_CURVES =
#ifndef BORINGSSL_FIPS
"X25519:"
#endif
"P-256";

To

const std::string ClientContextConfigImpl::DEFAULT_CIPHER_SUITES =
#ifndef BORINGSSL_FIPS
"[ECDHE-ECDSA-AES128-GCM-SHA256|ECDHE-ECDSA-CHACHA20-POLY1305]:"
"[ECDHE-RSA-AES128-GCM-SHA256|ECDHE-RSA-CHACHA20-POLY1305]:"
#else // BoringSSL FIPS
"ECDHE-ECDSA-AES128-GCM-SHA256:"
"ECDHE-RSA-AES128-GCM-SHA256:"
#endif
"ECDHE-ECDSA-AES256-GCM-SHA384:"
"ECDHE-RSA-AES256-GCM-SHA384:";
const std::string ClientContextConfigImpl::DEFAULT_CURVES =
#ifndef BORINGSSL_FIPS
"X25519:"
#endif
"P-256";

This is not clear in the docs.

For example, where tls_minimum_protocol_version / tls_maximum_protocol_version explicitly mention the difference in client and server configs, cipher_suites does not.

If you agree, I'd be happy to make this change / update the docs to be more clear that the new default client cipher suite is as follows.

[ECDHE-ECDSA-AES128-GCM-SHA256|ECDHE-ECDSA-CHACHA20-POLY1305]
[ECDHE-RSA-AES128-GCM-SHA256|ECDHE-RSA-CHACHA20-POLY1305]
ECDHE-ECDSA-AES256-GCM-SHA384
ECDHE-RSA-AES256-GCM-SHA384

Related context

@antoniovicente
Copy link
Contributor

@PiotrSikora @ggreenway @phlax

You're right, the client cipher list changed due to #13850

We'ld be happy to accept PRs to update the docs so they are more correct and clear.

@alyssawilk alyssawilk added area/docs and removed triage Issue requires triage labels May 20, 2021
htuch pushed a commit that referenced this issue May 23, 2021
These changes clarify that as of v1.16 the default cipher suite is different for client and servers.

Risk Level: Low
Testing: N/A
Docs Changes: Yes
Release Notes: N/A
Platform Specific Features: N/A

Fixes #16469

Signed-off-by: Bobby DeSimone <[email protected]>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this issue Sep 30, 2021
These changes clarify that as of v1.16 the default cipher suite is different for client and servers.

Risk Level: Low
Testing: N/A
Docs Changes: Yes
Release Notes: N/A
Platform Specific Features: N/A

Fixes envoyproxy#16469

Signed-off-by: Bobby DeSimone <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants