jdk
jdk copied to clipboard
8341964: Add mechanism to disable different parts of TLS cipher suite
The current syntax of the jdk.tls.disabledAlgorithms makes it difficult to disable algorithms that affect both the key exchange and authentication parts of a TLS cipher suite. For example, if you add "RSA" to the jdk.tls.disabledAlgorithms security property, it disables all cipher suites that use RSA, whether it is for key exchange or authentication. If you only want to disable cipher suites that use RSA for key exchange, the only workaround is to list the whole cipher suite name, so an exact match is done, but if there are many cipher suites that use that key exchange algorithm, this becomes cumbersome.
We should extend the syntax of the property to be able to distinguish between different cryptographic primitives used in the cipher suite. I think adding a new constraint something like:
TLSCipherConstraint: kx | authn
So when disabling TLS_RSA suites, you would add "RSA kx" to the property.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8341964: Add mechanism to disable different parts of TLS cipher suite (Enhancement - P3)
Reviewers
- @abdelhak-zaaim (no known openjdk.org user name / role) 🔄 Re-review required (review applies to 96ec04cf)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21577/head:pull/21577
$ git checkout pull/21577
Update a local copy of the PR:
$ git checkout pull/21577
$ git pull https://git.openjdk.org/jdk.git pull/21577/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 21577
View PR using the GUI difftool:
$ git pr show -t 21577
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21577.diff
Webrev
:wave: Welcome back abarashev! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
❗ This change is not yet ready to be integrated. See the Progress checklist in the description for automated requirements.
@artur-oracle The following label will be automatically applied to this pull request:
security
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.
Webrevs
- 03: Full - Incremental (6972f540)
- 02: Full - Incremental (ecf44e38)
- 01: Full - Incremental (fd3ae924)
- 00: Full (69855099)
Did you consider the option to support cipher suite regex Pattern, for example TLS_RSA_* for the restriction? The update is little bit complicated to me.
Did you consider the option to support cipher suite regex Pattern, for example TLS_RSA_* for the restriction? The update is little bit complicated to me.
Yes, I think we mentioned using regex during our internal discussions. We also consider a much simpler solution that would be fine for a one-off case but it's kind of hacky: use RSA-kx in the config file and also add it to SSLAlgorithmDecomposer#decomposes(CipherSuite.KeyExchange keyExchange). I approached this task not as a one-off case but as an expandable design to disable parts of TLS cipher suite. So we can easily expand it by adding new bulk or hash parameters to disable certain algorithms to be used as bulk ciphers or hash functions.
About using wildcards: that's definitely worth considering, I may put together an alternative PR that uses wild cards (true regex is not really needed here, not as user-friendly too). It would not be as user-friendly as RSA kx though, the end-users need to know exactly what they are doing, i.e. they need to know the exact format of cipher suite name, etc. Since these config values are used to disable both a simple algorithm and a complete cipher suite, the wild card would apply to both, so putting something like *ECDH* would disable whole bunch of algorithms and cipher suites.
Alternative wildcard draft: https://github.com/openjdk/jdk/pull/21841
It may be personal, but I think the wildcard's solution is much simple: simple to use, simple to implement and simple to extension, simple to back-port, and user friendly. It adds new learning curve if we adding something like "kx", which does not show elsewhere except here. I don't think we want it if wild-card solution works.
I will review the PR using wild-card next week.
It may be personal, but I think the wildcard's solution is much simple: simple to use, simple to implement and simple to extension, simple to back-port, and user friendly. It adds new learning curve if we adding something like "kx", which does not show elsewhere except here. I don't think we want it if wild-card solution works.
I will review the PR using wild-card next week.
Yes, it is much simpler indeed. We are having discussions to decide on the final design.
/issue remove JDK-8341964
@artur-oracle This PR does not contain any additional solved issues that can be removed. To remove the primary solved issue, simply edit the title of this PR.