8349583: Add mechanism to disable signature schemes based on their TLS scope#830
Open
jerboaa wants to merge 3 commits into
Open
8349583: Add mechanism to disable signature schemes based on their TLS scope#830jerboaa wants to merge 3 commits into
jerboaa wants to merge 3 commits into
Conversation
8349583: Add mechanism to disable signature schemes based on their TLS scope Since 'JDK-8226374: Restrict TLS signature schemes and named groups' is not in 8u, take minimal parts from it to update SignatureScheme: - use 'SigAlgParamSpec signAlgParams' over 'AlgorithmParameterSpec signAlgParameter' field. - Update usages of 'getPreferableAlgorithm()' and 'getSignerOfPreferableAlgorithm()' in: CertificateVerify, DHServerKeyExchange, ECDHServerKeyExchange, CertificateRequest and CertificateMessage passing in SSLAlgorithmConstraints as an extra parameter. - Add 'isPermitted()' without the extra bit added by JDK-8226374. - Update 'SignatureUtil.initVerifyWithParam()' with the change of the 'signAlgParams' field. The change to java.security-* is duplicated to all versions. Since '8341964: Add mechanism to disable different parts of TLS cipher suite' is not a dependency only include the relevant parts for JDK-8349583. Various test changes to adapt to 8u library code: - /test/lib => /lib/testlibrary - Utils.java is in jdk.testlibrary.Utils package in 8u. - Remove DTLS 1.2 test which isn't supported in 8u TLS 1.3 - Change usage of Map.ofEntries() which is only JDK 11+ API - Change usage of List.of() which is only in JDK 11+ API Other changes: - Set.of(x) => Collections.unmodifiableSet(EnumSet.of(x))
|
👋 Welcome back sgehwolf! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
This backport pull request has now been updated with issue from the original commit. |
When ct.sym is being generated by the CreateSymbols annotation processor, generation fails when trying to process DisabledAlgorithmConstraints class since SSLScope doesn't exist in the boot JDK (it's introduced in this patch). Fix the issue by adding a minimal extra boot class path for the boot JDK with just the added class. This will no longer be needed once an 8u512 build has been released with the class included in rt.jar
0aee5d0 to
91fcc83
Compare
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unclean backport of JDK-8349583 because some dependencies aren't in 8u. This is a needed backport for JDK-8340321 which we'd like to bring into 8u. The dep missing for this backport is
JDK-8226374: Restrict TLS signature schemes and named groups. Getting that in would also require8171279: Support X25519 and X448 in TLS. So I've omitted it and only added relevant parts of it to make the backport work:In
SignatureScheme:SigAlgParamSpec signAlgParamsoverAlgorithmParameterSpec signAlgParameterfield.getPreferableAlgorithm()and
getSignerOfPreferableAlgorithm()in:CertificateVerify,DHServerKeyExchange,ECDHServerKeyExchange,CertificateRequestandCertificateMessagepassing inSSLAlgorithmConstraintsas an extra parameter.
isPermitted()without the extra bit added by JDK-8226374.SignatureUtil.initVerifyWithParam()with the change of thesignAlgParamsfield.The change to
java.security-*is duplicated to all OS versions.Since
8341964: Add mechanism to disable different parts of TLS cipher suiteis nota dependency only include the relevant parts for JDK-8349583 (this patch).
Various test changes to adapt to 8u library code:
/test/lib => /lib/testlibraryUtils.javais injdk.testlibrary.Utilspackage in 8u.Map.ofEntries()which is JDK 11+ only APIList.of()which is JDK 11+ only APIOther changes:
Set.of(x)=>Collections.unmodifiableSet(EnumSet.of(x))sun.security.ssl.SSLScopeenum which is being used in signatures ofDisabledAlgorithmConstraintswhich is being processed during the build forct.symcreation. Since this tool - and annotation processor - runs on the boot JDK during the build with a modified boot classpath that originally didn't include the newSSLScopeclass, the processing failed when looking up the class bytes for this class. The fix is to create an extra limited boot classpath that adds this class so that the build also works with an older boot JDK. See second commit.Testing:
jdk/test/javax/net/ssljdk/test/sun/net/www/protocol/https/jdk/test/sun/security/ssl/with these results:Test results: passed: 270; failed: 2; error: 1Failures are the same as in master.jdk/security_infratests are known failures. JDK-8333788 and JDK-8385584Progress
Integration blocker
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/830/head:pull/830$ git checkout pull/830Update a local copy of the PR:
$ git checkout pull/830$ git pull https://git.openjdk.org/jdk8u-dev.git pull/830/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 830View PR using the GUI difftool:
$ git pr show -t 830Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/830.diff
Using Webrev
Link to Webrev Comment