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

Include TLSv1.3 suites in some Tomcat configurations #253

Merged
merged 2 commits into from
Oct 17, 2024

Conversation

janbrasna
Copy link
Collaborator

@janbrasna janbrasna commented Oct 8, 2024

To support not only OpenSSL, but also JSSE fully, whenever ciphers are configured, TLSv1.3 suites must be included, otherwise TLSv1.3–only clients won't be able to connect if not using OpenSSL.

Fixes mozilla/server-side-tls#280
(similar to #226, but with more possible implementations treating the config different ways)

Rationale

The output template doesn't set any Connector details, so the config works for whatever implementation the consumers use (JSSE Java SSL or OpenSSL, depending on tcnative or APR/native lib):

https://tomcat.apache.org/tomcat-9.0-doc/config/http.html#SSL_Support

The NIO and NIO2 connectors use either the JSSE Java SSL implementation or an OpenSSL implementation, whereas the APR/native connector uses OpenSSL only. Prior to Tomcat 8.5, different configuration attributes were used for JSSE and OpenSSL. From Tomcat 8.5 onwards, and as far as possible, common configuration attributes are used for both JSSE and OpenSSL. Also if using the JSSE OpenSSL implementation, configuration can be set using either the JSSE or APR attributes (note: but not both types within the same configuration). This is to aid simpler switching between connector implementations for SSL connectors.

We kinda imply OpenSSL due to naming in the configs, but thanks to how the value is parsed JSSE will accept openssl naming instead of its iana and happily apply that — so we can use one cipherstring for both and not limit what library can be used.

Currently works over OpenSSL but fails for JSSE unless RFC 8446 suites are also defined with the rest, given the API differences when using different crypto connectors. (APR won't configure TLSv1.3 at all and apply defaults, while JSSE will adhere to the cipher list for all protocols, needing the TLSv1.3 defaults explicitly enumerated in the allowed cipherstring.)

Significant changes and points to review

It is slightly complicated as the cipherstring is parsed first, checking allowlists, env configs etc., comparing the suites set with some internal list first, and raising warnings for mismatches — so it has to first pass internal checks before being set by the connector/library.

The addition is necessary for JSSE that treats the list as exhaustive (applied to all TLS versions) and won't configure anything what's not on it on its own, but it won't bother OpenSSL as it won't configure what it doesn't have in allowlists (for ≤TLSv1.2, and using defaults for TLSv1.3); so this can be safely used with both implementations.

  • When using Java's own SSL implementation:
    Only the three explicit TLSv1.3 suites we have defined ["TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384", "TLS_CHACHA20_POLY1305_SHA256"] will be configured and allowed.
  • When used over any OpenSSL implementation:
    The defaults will be applied for TLSv1.3 connections, so e.g. any CCM suites enabled on the system will also be allowed (or any other disabled elsewhere not changed etc.), so the TLSv1.3–related part of the cipherstring will have no impact.

Testing

https://fix-tomcat-tls13--mozsslconf-dev.netlify.app/#server=tomcat

@janbrasna janbrasna added P2 Priority: 2 S2 Severity: 2 bug Something isn't working compatibility Warnings, deprecations or incompatibilities to tackle labels Oct 8, 2024
Copy link
Collaborator Author

@janbrasna janbrasna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notes:

src/js/configs.js Show resolved Hide resolved
src/templates/partials/tomcat.hbs Show resolved Hide resolved
@janbrasna janbrasna marked this pull request as ready for review October 12, 2024 16:40
@gstrauss

This comment was marked as resolved.

@gstrauss
Copy link
Collaborator

Besides splitting the linting commit to a separate PR, the Tomcat changes here LGTM.

@gstrauss
Copy link
Collaborator

Thanks. Please submit the removed commit with linting changes and removing defaults to another PR.

@gstrauss gstrauss merged commit f7a5e5b into mozilla:master Oct 17, 2024
3 checks passed
@janbrasna janbrasna deleted the fix/tomcat-tls13 branch October 17, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compatibility Warnings, deprecations or incompatibilities to tackle P2 Priority: 2 S2 Severity: 2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tomcat Intermediate configuration (TLS v1.2 + TLS v1.3) not working correctly
2 participants