From f147743e01ddc6fa652c91c07e879c93958bb37d Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Sat, 8 Feb 2025 20:31:21 -0800 Subject: [PATCH 01/15] passing two tests --- .../provider/certpath/AlgorithmChecker.java | 7 +- .../util/AbstractAlgorithmConstraints.java | 29 +++- .../security/util/AlgorithmDecomposer.java | 23 ++- .../util/DisabledAlgorithmConstraints.java | 159 ++++++++++++++---- .../classes/sun/security/util/KeyUtil.java | 13 ++ .../share/conf/security/java.security | 5 +- 6 files changed, 187 insertions(+), 49 deletions(-) diff --git a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java index 7ce31492b4b41..8e5ba42ee1bf0 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java +++ b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java @@ -30,13 +30,13 @@ import java.security.cert.*; import java.security.cert.Certificate; import java.security.cert.CertPathValidatorException.BasicReason; -import java.security.interfaces.DSAParams; -import java.security.interfaces.DSAPublicKey; +import java.security.interfaces.*; import java.security.spec.DSAPublicKeySpec; import java.util.*; import sun.security.util.Debug; import sun.security.util.DisabledAlgorithmConstraints; +import sun.security.util.KeyUtil; import sun.security.validator.Validator; import sun.security.x509.AlgorithmId; import sun.security.x509.X509CertImpl; @@ -206,7 +206,8 @@ public void check(Certificate cert, CertPathConstraintsParameters cp = new CertPathConstraintsParameters(trustedPubKey, variant, anchor, date); - dac.permits(trustedPubKey.getAlgorithm(), cp, true); + dac.permits(KeyUtil.getAlgorithm(trustedPubKey), + cp, true); } // Check the signature algorithm and parameters against constraints CertPathConstraintsParameters cp = diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index dc5b1aafb2017..5d4afccf4f865 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -27,10 +27,7 @@ import java.security.AlgorithmConstraints; import java.security.Security; -import java.util.Arrays; -import java.util.Collections; -import java.util.Set; -import java.util.TreeSet; +import java.util.*; /** * The class contains common functionality for algorithm constraints classes. @@ -70,14 +67,33 @@ static Set getAlgorithms(String propertyName) { return algorithmsInPropertySet; } + + private static final String[] aliasEdDSA = new String[]{"EdDSA", "Ed25519", "Ed448"}; + private static final String[] aliasEd25519 = new String[]{"EdDSA", "Ed25519"}; + private static final String[] aliasEd448 = new String[]{"EdDSA", "Ed448"}; + public static List getAliases(String algorithm) { + return switch (algorithm) { + case "EdDSA" -> Arrays.asList(aliasEdDSA); + case "Ed25519" -> Arrays.asList(aliasEd25519); + case "Ed448" -> Arrays.asList(aliasEd448); + default -> Collections.emptyList(); + }; + } + static boolean checkAlgorithm(Set algorithms, String algorithm, AlgorithmDecomposer decomposer) { if (algorithm == null || algorithm.isEmpty()) { throw new IllegalArgumentException("No algorithm name specified"); } + System.err.println("checkAlgorithm: looking for " + algorithm); + algorithms.stream().forEach(s -> System.err.println("entry -> " + s.toString())); - if (algorithms.contains(algorithm)) { - return false; + // Check `algorithm` against disabled algorithms and their aliases + for (String a : algorithms) { + if (algorithm.equalsIgnoreCase(a) || + getAliases(a).contains(algorithm)) { + return false; + } } // decompose the algorithm into sub-elements @@ -85,6 +101,7 @@ static boolean checkAlgorithm(Set algorithms, String algorithm, // check the element of the elements for (String element : elements) { + System.err.println("checkAlgorithm elements: " + element); if (algorithms.contains(element)) { return false; } diff --git a/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java b/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java index 2b38bc09733a1..644f782dba49a 100644 --- a/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java +++ b/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java @@ -118,19 +118,29 @@ public Set decompose(String algorithm) { /** * Get aliases of the specified algorithm. - * - * May support more algorithms in the future. */ + + private static final String[] aliasDH = new String[]{"DH", "DiffieHellman"}; + private static final String[] aliasEdDSA = new String[]{"EdDSA", "Ed25519", "Ed448"}; + private static final String[] aliasEd25519 = new String[]{"EdDSA", "Ed25519"}; + private static final String[] aliasEd448 = new String[]{"EdDSA", "Ed448"}; public static Collection getAliases(String algorithm) { String[] aliases; if (algorithm.equalsIgnoreCase("DH") || algorithm.equalsIgnoreCase("DiffieHellman")) { - aliases = new String[] {"DH", "DiffieHellman"}; - } else { - aliases = new String[] {algorithm}; + return Arrays.asList(aliasDH); + /* + } else if (algorithm.equalsIgnoreCase("EdDSA")) { + return Arrays.asList(aliasEdDSA); + } else if (algorithm.equalsIgnoreCase("Ed25519")) { + return Arrays.asList(aliasEd25519); + } else if (algorithm.equalsIgnoreCase("Ed448")) { + return Arrays.asList(aliasEd448); + */ } - return Arrays.asList(aliases); + return Arrays.asList(algorithm); + } /** @@ -143,6 +153,7 @@ static Set decomposeName(String algorithm) { return new HashSet<>(); } + System.err.println("algo 2: " + algorithm); Set elements = decomposeImpl(algorithm); // no need to check further if algorithm doesn't contain "SHA" diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index 7612242733eeb..c54446c7b0acb 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -29,11 +29,14 @@ import java.lang.ref.SoftReference; import java.security.AlgorithmParameters; +import java.security.AsymmetricKey; import java.security.CryptoPrimitive; import java.security.Key; import java.security.cert.CertPathValidatorException; import java.security.cert.CertPathValidatorException.BasicReason; +import java.security.cert.CertificateRevokedException; import java.security.interfaces.ECKey; +import java.security.interfaces.EdECKey; import java.security.interfaces.XECKey; import java.security.spec.AlgorithmParameterSpec; import java.security.spec.InvalidParameterSpecException; @@ -44,16 +47,7 @@ import java.time.Instant; import java.time.ZoneId; import java.time.ZonedDateTime; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; -import java.util.StringTokenizer; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -154,7 +148,7 @@ public DisabledAlgorithmConstraints(String propertyName, break; } } - algorithmConstraints = new Constraints(propertyName, disabledAlgorithms); + algorithmConstraints = new Constraints(disabledAlgorithms); } /* @@ -164,6 +158,7 @@ public DisabledAlgorithmConstraints(String propertyName, @Override public final boolean permits(Set primitives, String algorithm, AlgorithmParameters parameters) { + System.err.println("algo: " + algorithm); if (primitives == null || primitives.isEmpty()) { throw new IllegalArgumentException("The primitives cannot be null" + " or empty."); @@ -172,15 +167,17 @@ public final boolean permits(Set primitives, throw new IllegalArgumentException("No algorithm name specified"); } + System.err.println("cachedCheckAlgorithm(" + algorithm + ")"); if (!cachedCheckAlgorithm(algorithm)) { + System.err.println("bounced"); return false; } if (parameters != null) { return algorithmConstraints.permits(algorithm, parameters); + } else { + return algorithmConstraints.permits(algorithm); } - - return true; } /* @@ -268,16 +265,22 @@ public final void permits(String algorithm, ConstraintsParameters cp, } private static List getNamedCurveFromKey(Key key) { - if (key instanceof ECKey) { - NamedCurve nc = CurveDB.lookup(((ECKey)key).getParams()); - return (nc == null ? List.of() - : Arrays.asList(nc.getNameAndAliases())); - } else if (key instanceof XECKey) { - return List.of( - ((NamedParameterSpec)((XECKey)key).getParams()).getName()); - } else { - return List.of(); - } + List x = switch (key) { + case ECKey ecKey -> { + NamedCurve nc = CurveDB.lookup(ecKey.getParams()); + if (nc == null) { + yield List.of(); + } + yield Arrays.asList(nc.getNameAndAliases()); + } + case XECKey xecKey -> List.of( + ((NamedParameterSpec) xecKey.getParams()).getName()); + case EdECKey edecKey -> List.of((edecKey.getParams()).getName()); + default -> List.of(); + }; + + x.stream().forEach(s -> System.err.println("NamedCurve: " + s)); + return x; } // Check algorithm constraints with key and algorithm @@ -338,7 +341,8 @@ private static class Holder { "denyAfter\\s+(\\d{4})-(\\d{2})-(\\d{2})"); } - public Constraints(String propertyName, Set constraintSet) { + public Constraints(Set constraintSet) { + System.err.println("constructed"); for (String constraintEntry : constraintSet) { if (constraintEntry == null || constraintEntry.isEmpty()) { continue; @@ -361,15 +365,54 @@ public Constraints(String propertyName, Set constraintSet) { new ArrayList<>(1)); // Consider the impact of algorithm aliases. - for (String alias : AlgorithmDecomposer.getAliases(algorithm)) { + /* + Collection aliasList = + AlgorithmDecomposer.getAliases(algorithm); + aliasList.forEach(a -> { + System.err.println("alias: " + a); constraintsMap.putIfAbsent( - alias.toUpperCase(Locale.ENGLISH), constraintList); + a.toUpperCase(Locale.ENGLISH), constraintList); + }); + + /* + Group algorithm mappings: + Some algorithms have an umbrella name with and more than one + implementation. One of those is EdDSA. Unfortunately there + may not be uniformity in these naming conventions. For EdDSA, + KeyPairGenerator.getInstance("EdDSA") will returns Ed25519. + But when disabling EdDSA, the user would assume "EdDSA" would + mean Ed25519 and Ed448. The below code defines these group + algorithms a possible name variants for + DisabledAlgorithmConstraints. + */ + //XXX wrong place + /* + switch (algorithm) { + case "EdDSA" -> { + constraintList.add(new DisabledConstraint("Ed25519")); + constraintList.add(new DisabledConstraint("Ed448")); + } + case "Ed25519" -> + constraintList.add(new DisabledConstraint("EdDSA")); + case "XDH" -> { + constraintList.add(new DisabledConstraint("X25519")); + constraintList.add(new DisabledConstraint("X448")); + } + case "X25519" -> + constraintList.add(new DisabledConstraint("XDH")); } + */ + + System.err.println("map"); + constraintsMap.keySet().forEach(s -> System.err.println(s + " constraintsMap: " + constraintsMap.get(s))); + System.err.println("list"); + constraintList.forEach(s -> System.err.println("constraintList: " + s)); // If there is no whitespace, it is an algorithm name; however, // if there is a whitespace, could be a multi-word EC curve too. if (space <= 0 || CurveDB.lookup(constraintEntry) != null) { constraintList.add(new DisabledConstraint(algorithm)); + System.err.println("Continued?"); continue; } @@ -453,6 +496,17 @@ public Constraints(String propertyName, Set constraintSet) { } } + public String toString() { + StringBuilder sb = new StringBuilder(100); + var s = constraintsMap.keySet(); + s.stream().forEach(k -> { + sb.append(k); + sb.append(" : "); + sb.append(constraintsMap.get(k)); + }); + return sb.toString(); + } + // Get applicable constraints based off the algorithm private List getConstraints(String algorithm) { return constraintsMap.get(algorithm.toUpperCase(Locale.ENGLISH)); @@ -460,17 +514,22 @@ private List getConstraints(String algorithm) { // Check if KeySizeConstraints permit the specified key public boolean permits(Key key) { - List list = getConstraints(key.getAlgorithm()); + String algorithm = KeyUtil.getAlgorithm(key); + List list = getConstraints(algorithm); + System.err.println(list); if (list == null) { return true; } for (Constraint constraint : list) { - if (!constraint.permits(key)) { - if (debug != null) { - debug.println("Constraints: failed key size " + + for (String a : AlgorithmDecomposer.decomposeName(algorithm)) { + System.err.println("constraints.permits()" + constraint + a); + if (!constraint.permits(key)) { + if (debug != null) { + debug.println("Constraints: failed key size " + "constraint check " + KeyUtil.getKeySize(key)); + } + return false; } - return false; } } return true; @@ -497,6 +556,22 @@ public boolean permits(String algorithm, AlgorithmParameters aps) { return true; } + public boolean permits(String algorithm) { + List list = getConstraints(algorithm); + if (list == null) { + return true; + } + + for (Constraint c : list) { + for (String a : AlgorithmDecomposer.getAliases(algorithm)) { + if (a.equalsIgnoreCase(c.algorithm)) { + return false; + } + } + } + return true; + } + public void permits(String algorithm, ConstraintsParameters cp, boolean checkKey) throws CertPathValidatorException { @@ -514,7 +589,7 @@ public void permits(String algorithm, ConstraintsParameters cp, if (checkKey) { for (Key key : cp.getKeys()) { - algorithms.add(key.getAlgorithm()); + algorithms.add(KeyUtil.getAlgorithm(key)); } } @@ -660,6 +735,10 @@ boolean next(ConstraintsParameters cp) boolean next(Key key) { return nextConstraint != null && nextConstraint.permits(key); } + + public String toString() { + return this.getClass().getName() + ": algorithm: " + algorithm; + } } /* @@ -971,14 +1050,28 @@ private boolean cachedCheckAlgorithm(String algorithm) { } } } + Boolean result = cache.get(algorithm); if (result != null) { + System.err.println("result 1: " + result.toString()); return result; } + // We won't check patterns if algorithm check fails. result = checkAlgorithm(disabledAlgorithms, algorithm, decomposer) - && checkDisabledPatterns(algorithm); + && checkDisabledPatterns(algorithm); + System.err.println("result 2: " + algorithm + ": " + (result ? "allowed" : "denied")); cache.put(algorithm, result); + +/* var algoList = AlgorithmDecomposer.getAliases(algorithm); + for (String algo : algoList) { + // We won't check patterns if algorithm check fails. + result = checkAlgorithm(disabledAlgorithms, algo, decomposer) + && checkDisabledPatterns(algo); + System.err.println("result 2: " + algo + ": " + (result ? "allowed" : "denied")); + cache.put(algo, result); + } +*/ return result; } diff --git a/src/java.base/share/classes/sun/security/util/KeyUtil.java b/src/java.base/share/classes/sun/security/util/KeyUtil.java index 6eb5acfade3fd..1ea8235addd3c 100644 --- a/src/java.base/share/classes/sun/security/util/KeyUtil.java +++ b/src/java.base/share/classes/sun/security/util/KeyUtil.java @@ -424,5 +424,18 @@ public static String hashAlgFromHSS(PublicKey publicKey) throw new NoSuchAlgorithmException("Cannot decode public key", e); } } + + /* + If the key is a sub-algorithm of a larger group of algorithms, this method + will return that sub-algorithm. For example, key.getAlgorithm() returns + "EdDSA", but the underlying key maybe "Ed448". For + DisabledAlgorithmConstraints, this distinction is important. + */ + public static String getAlgorithm(Key key) { + return switch (key) { + case EdECKey ed -> ed.getParams().getName(); + default -> key.getAlgorithm(); + }; + } } diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security index 0e24dee6ac27a..423a28c1f66f7 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -735,6 +735,9 @@ http.auth.digest.disabledAlgorithms = MD5, SHA-1 # "TLS_RSA_". Only cipher suites starting with "TLS_" are allowed to have # wildcard characters. # +# UsageConstraint: +# usage [TLSHandshake] +# # Note: The algorithm restrictions do not apply to trust anchors or # self-signed certificates. # @@ -746,7 +749,7 @@ http.auth.digest.disabledAlgorithms = MD5, SHA-1 # rsa_pkcs1_sha1, secp224r1, TLS_RSA_* jdk.tls.disabledAlgorithms=SSLv3, TLSv1, TLSv1.1, DTLSv1.0, RC4, DES, \ MD5withRSA, DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL, \ - ECDH, TLS_RSA_* + ECDH, TLS_RSA_*, rsa_pkcs1_sha1 usage TLSHandshake # # Legacy algorithms for Secure Socket Layer/Transport Layer Security (SSL/TLS) From 2b9a2f0a65c01dbf8428ef99afdfcc77d2e86ae9 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Sat, 8 Feb 2025 21:23:55 -0800 Subject: [PATCH 02/15] cleanup, both tests still pass --- .../provider/certpath/AlgorithmChecker.java | 5 +- .../util/AbstractAlgorithmConstraints.java | 11 +- .../security/util/AlgorithmDecomposer.java | 22 +--- .../util/DisabledAlgorithmConstraints.java | 114 ++++-------------- .../share/conf/security/java.security | 5 +- 5 files changed, 37 insertions(+), 120 deletions(-) diff --git a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java index 8e5ba42ee1bf0..f127ac65cc7b4 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java +++ b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -30,7 +30,8 @@ import java.security.cert.*; import java.security.cert.Certificate; import java.security.cert.CertPathValidatorException.BasicReason; -import java.security.interfaces.*; +import java.security.interfaces.DSAParams; +import java.security.interfaces.DSAPublicKey; import java.security.spec.DSAPublicKeySpec; import java.util.*; diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index 5d4afccf4f865..7f062d3cf76a6 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -27,7 +27,11 @@ import java.security.AlgorithmConstraints; import java.security.Security; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; /** * The class contains common functionality for algorithm constraints classes. @@ -85,8 +89,6 @@ static boolean checkAlgorithm(Set algorithms, String algorithm, if (algorithm == null || algorithm.isEmpty()) { throw new IllegalArgumentException("No algorithm name specified"); } - System.err.println("checkAlgorithm: looking for " + algorithm); - algorithms.stream().forEach(s -> System.err.println("entry -> " + s.toString())); // Check `algorithm` against disabled algorithms and their aliases for (String a : algorithms) { @@ -101,7 +103,6 @@ static boolean checkAlgorithm(Set algorithms, String algorithm, // check the element of the elements for (String element : elements) { - System.err.println("checkAlgorithm elements: " + element); if (algorithms.contains(element)) { return false; } diff --git a/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java b/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java index 644f782dba49a..21e30b911e598 100644 --- a/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java +++ b/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java @@ -118,29 +118,19 @@ public Set decompose(String algorithm) { /** * Get aliases of the specified algorithm. + * + * May support more algorithms in the future. */ - - private static final String[] aliasDH = new String[]{"DH", "DiffieHellman"}; - private static final String[] aliasEdDSA = new String[]{"EdDSA", "Ed25519", "Ed448"}; - private static final String[] aliasEd25519 = new String[]{"EdDSA", "Ed25519"}; - private static final String[] aliasEd448 = new String[]{"EdDSA", "Ed448"}; public static Collection getAliases(String algorithm) { String[] aliases; if (algorithm.equalsIgnoreCase("DH") || algorithm.equalsIgnoreCase("DiffieHellman")) { - return Arrays.asList(aliasDH); - /* - } else if (algorithm.equalsIgnoreCase("EdDSA")) { - return Arrays.asList(aliasEdDSA); - } else if (algorithm.equalsIgnoreCase("Ed25519")) { - return Arrays.asList(aliasEd25519); - } else if (algorithm.equalsIgnoreCase("Ed448")) { - return Arrays.asList(aliasEd448); - */ + aliases = new String[] {"DH", "DiffieHellman"}; + } else { + aliases = new String[] {algorithm}; } - return Arrays.asList(algorithm); - + return Arrays.asList(aliases); } /** diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index c54446c7b0acb..f7edd1df76e06 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,12 +29,10 @@ import java.lang.ref.SoftReference; import java.security.AlgorithmParameters; -import java.security.AsymmetricKey; import java.security.CryptoPrimitive; import java.security.Key; import java.security.cert.CertPathValidatorException; import java.security.cert.CertPathValidatorException.BasicReason; -import java.security.cert.CertificateRevokedException; import java.security.interfaces.ECKey; import java.security.interfaces.EdECKey; import java.security.interfaces.XECKey; @@ -47,7 +45,16 @@ import java.time.Instant; import java.time.ZoneId; import java.time.ZonedDateTime; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; +import java.util.StringTokenizer; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -158,7 +165,6 @@ public DisabledAlgorithmConstraints(String propertyName, @Override public final boolean permits(Set primitives, String algorithm, AlgorithmParameters parameters) { - System.err.println("algo: " + algorithm); if (primitives == null || primitives.isEmpty()) { throw new IllegalArgumentException("The primitives cannot be null" + " or empty."); @@ -167,9 +173,7 @@ public final boolean permits(Set primitives, throw new IllegalArgumentException("No algorithm name specified"); } - System.err.println("cachedCheckAlgorithm(" + algorithm + ")"); if (!cachedCheckAlgorithm(algorithm)) { - System.err.println("bounced"); return false; } @@ -265,7 +269,7 @@ public final void permits(String algorithm, ConstraintsParameters cp, } private static List getNamedCurveFromKey(Key key) { - List x = switch (key) { + return switch (key) { case ECKey ecKey -> { NamedCurve nc = CurveDB.lookup(ecKey.getParams()); if (nc == null) { @@ -278,9 +282,6 @@ private static List getNamedCurveFromKey(Key key) { case EdECKey edecKey -> List.of((edecKey.getParams()).getName()); default -> List.of(); }; - - x.stream().forEach(s -> System.err.println("NamedCurve: " + s)); - return x; } // Check algorithm constraints with key and algorithm @@ -342,7 +343,6 @@ private static class Holder { } public Constraints(Set constraintSet) { - System.err.println("constructed"); for (String constraintEntry : constraintSet) { if (constraintEntry == null || constraintEntry.isEmpty()) { continue; @@ -365,54 +365,15 @@ public Constraints(Set constraintSet) { new ArrayList<>(1)); // Consider the impact of algorithm aliases. - /* - Collection aliasList = - AlgorithmDecomposer.getAliases(algorithm); - aliasList.forEach(a -> { - System.err.println("alias: " + a); + for (String alias : AlgorithmDecomposer.getAliases(algorithm)) { constraintsMap.putIfAbsent( - a.toUpperCase(Locale.ENGLISH), constraintList); - }); - - /* - Group algorithm mappings: - Some algorithms have an umbrella name with and more than one - implementation. One of those is EdDSA. Unfortunately there - may not be uniformity in these naming conventions. For EdDSA, - KeyPairGenerator.getInstance("EdDSA") will returns Ed25519. - But when disabling EdDSA, the user would assume "EdDSA" would - mean Ed25519 and Ed448. The below code defines these group - algorithms a possible name variants for - DisabledAlgorithmConstraints. - */ - //XXX wrong place - /* - switch (algorithm) { - case "EdDSA" -> { - constraintList.add(new DisabledConstraint("Ed25519")); - constraintList.add(new DisabledConstraint("Ed448")); - } - case "Ed25519" -> - constraintList.add(new DisabledConstraint("EdDSA")); - case "XDH" -> { - constraintList.add(new DisabledConstraint("X25519")); - constraintList.add(new DisabledConstraint("X448")); - } - case "X25519" -> - constraintList.add(new DisabledConstraint("XDH")); + alias.toUpperCase(Locale.ENGLISH), constraintList); } - */ - - System.err.println("map"); - constraintsMap.keySet().forEach(s -> System.err.println(s + " constraintsMap: " + constraintsMap.get(s))); - System.err.println("list"); - constraintList.forEach(s -> System.err.println("constraintList: " + s)); // If there is no whitespace, it is an algorithm name; however, // if there is a whitespace, could be a multi-word EC curve too. if (space <= 0 || CurveDB.lookup(constraintEntry) != null) { constraintList.add(new DisabledConstraint(algorithm)); - System.err.println("Continued?"); continue; } @@ -496,17 +457,6 @@ public Constraints(Set constraintSet) { } } - public String toString() { - StringBuilder sb = new StringBuilder(100); - var s = constraintsMap.keySet(); - s.stream().forEach(k -> { - sb.append(k); - sb.append(" : "); - sb.append(constraintsMap.get(k)); - }); - return sb.toString(); - } - // Get applicable constraints based off the algorithm private List getConstraints(String algorithm) { return constraintsMap.get(algorithm.toUpperCase(Locale.ENGLISH)); @@ -514,22 +464,17 @@ private List getConstraints(String algorithm) { // Check if KeySizeConstraints permit the specified key public boolean permits(Key key) { - String algorithm = KeyUtil.getAlgorithm(key); - List list = getConstraints(algorithm); - System.err.println(list); + List list = getConstraints(KeyUtil.getAlgorithm(key)); if (list == null) { return true; } for (Constraint constraint : list) { - for (String a : AlgorithmDecomposer.decomposeName(algorithm)) { - System.err.println("constraints.permits()" + constraint + a); - if (!constraint.permits(key)) { - if (debug != null) { - debug.println("Constraints: failed key size " + - "constraint check " + KeyUtil.getKeySize(key)); - } - return false; + if (!constraint.permits(key)) { + if (debug != null) { + debug.println("Constraints: failed key size " + + "constraint check " + KeyUtil.getKeySize(key)); } + return false; } } return true; @@ -735,10 +680,6 @@ boolean next(ConstraintsParameters cp) boolean next(Key key) { return nextConstraint != null && nextConstraint.permits(key); } - - public String toString() { - return this.getClass().getName() + ": algorithm: " + algorithm; - } } /* @@ -1050,28 +991,15 @@ private boolean cachedCheckAlgorithm(String algorithm) { } } } - Boolean result = cache.get(algorithm); if (result != null) { - System.err.println("result 1: " + result.toString()); return result; } - // We won't check patterns if algorithm check fails. result = checkAlgorithm(disabledAlgorithms, algorithm, decomposer) - && checkDisabledPatterns(algorithm); - System.err.println("result 2: " + algorithm + ": " + (result ? "allowed" : "denied")); + && checkDisabledPatterns(algorithm); cache.put(algorithm, result); -/* var algoList = AlgorithmDecomposer.getAliases(algorithm); - for (String algo : algoList) { - // We won't check patterns if algorithm check fails. - result = checkAlgorithm(disabledAlgorithms, algo, decomposer) - && checkDisabledPatterns(algo); - System.err.println("result 2: " + algo + ": " + (result ? "allowed" : "denied")); - cache.put(algo, result); - } -*/ return result; } diff --git a/src/java.base/share/conf/security/java.security b/src/java.base/share/conf/security/java.security index 423a28c1f66f7..0e24dee6ac27a 100644 --- a/src/java.base/share/conf/security/java.security +++ b/src/java.base/share/conf/security/java.security @@ -735,9 +735,6 @@ http.auth.digest.disabledAlgorithms = MD5, SHA-1 # "TLS_RSA_". Only cipher suites starting with "TLS_" are allowed to have # wildcard characters. # -# UsageConstraint: -# usage [TLSHandshake] -# # Note: The algorithm restrictions do not apply to trust anchors or # self-signed certificates. # @@ -749,7 +746,7 @@ http.auth.digest.disabledAlgorithms = MD5, SHA-1 # rsa_pkcs1_sha1, secp224r1, TLS_RSA_* jdk.tls.disabledAlgorithms=SSLv3, TLSv1, TLSv1.1, DTLSv1.0, RC4, DES, \ MD5withRSA, DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL, \ - ECDH, TLS_RSA_*, rsa_pkcs1_sha1 usage TLSHandshake + ECDH, TLS_RSA_* # # Legacy algorithms for Secure Socket Layer/Transport Layer Security (SSL/TLS) From 08da228ed53dd7fb7ce9d9503c68f1db228cdaf7 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Sat, 8 Feb 2025 21:54:24 -0800 Subject: [PATCH 03/15] more cleanup --- .../util/AbstractAlgorithmConstraints.java | 10 +++++--- .../security/util/AlgorithmDecomposer.java | 1 - .../util/DisabledAlgorithmConstraints.java | 25 +++---------------- 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index 7f062d3cf76a6..ba237f4d5bd76 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -72,9 +72,13 @@ static Set getAlgorithms(String propertyName) { } - private static final String[] aliasEdDSA = new String[]{"EdDSA", "Ed25519", "Ed448"}; - private static final String[] aliasEd25519 = new String[]{"EdDSA", "Ed25519"}; - private static final String[] aliasEd448 = new String[]{"EdDSA", "Ed448"}; + private static final String[] aliasEdDSA = + new String[]{"EdDSA", "Ed25519", "Ed448"}; + private static final String[] aliasEd25519 = + new String[]{"EdDSA", "Ed25519"}; + private static final String[] aliasEd448 = + new String[]{"EdDSA", "Ed448"}; + public static List getAliases(String algorithm) { return switch (algorithm) { case "EdDSA" -> Arrays.asList(aliasEdDSA); diff --git a/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java b/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java index 21e30b911e598..2b38bc09733a1 100644 --- a/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java +++ b/src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java @@ -143,7 +143,6 @@ static Set decomposeName(String algorithm) { return new HashSet<>(); } - System.err.println("algo 2: " + algorithm); Set elements = decomposeImpl(algorithm); // no need to check further if algorithm doesn't contain "SHA" diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index f7edd1df76e06..707d8f8d058db 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -179,9 +179,9 @@ public final boolean permits(Set primitives, if (parameters != null) { return algorithmConstraints.permits(algorithm, parameters); - } else { - return algorithmConstraints.permits(algorithm); } + + return true; } /* @@ -367,7 +367,7 @@ public Constraints(Set constraintSet) { // Consider the impact of algorithm aliases. for (String alias : AlgorithmDecomposer.getAliases(algorithm)) { constraintsMap.putIfAbsent( - alias.toUpperCase(Locale.ENGLISH), constraintList); + alias.toUpperCase(Locale.ENGLISH), constraintList); } // If there is no whitespace, it is an algorithm name; however, @@ -472,7 +472,7 @@ public boolean permits(Key key) { if (!constraint.permits(key)) { if (debug != null) { debug.println("Constraints: failed key size " + - "constraint check " + KeyUtil.getKeySize(key)); + "constraint check " + KeyUtil.getKeySize(key)); } return false; } @@ -501,22 +501,6 @@ public boolean permits(String algorithm, AlgorithmParameters aps) { return true; } - public boolean permits(String algorithm) { - List list = getConstraints(algorithm); - if (list == null) { - return true; - } - - for (Constraint c : list) { - for (String a : AlgorithmDecomposer.getAliases(algorithm)) { - if (a.equalsIgnoreCase(c.algorithm)) { - return false; - } - } - } - return true; - } - public void permits(String algorithm, ConstraintsParameters cp, boolean checkKey) throws CertPathValidatorException { @@ -999,7 +983,6 @@ private boolean cachedCheckAlgorithm(String algorithm) { result = checkAlgorithm(disabledAlgorithms, algorithm, decomposer) && checkDisabledPatterns(algorithm); cache.put(algorithm, result); - return result; } From a56e19842b1433d78149a1d5ae60c9818a7b268f Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Thu, 13 Feb 2025 14:39:13 -0800 Subject: [PATCH 04/15] cleanup --- .../util/AbstractAlgorithmConstraints.java | 9 +- .../util/DisabledAlgorithmConstraints.java | 10 +- .../classes/sun/security/util/KeyUtil.java | 29 ++-- .../DisabledAlgorithmPermits.java | 143 ++++++++++++++++++ 4 files changed, 167 insertions(+), 24 deletions(-) create mode 100644 test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index ba237f4d5bd76..d00d79bb76064 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -76,14 +76,17 @@ static Set getAlgorithms(String propertyName) { new String[]{"EdDSA", "Ed25519", "Ed448"}; private static final String[] aliasEd25519 = new String[]{"EdDSA", "Ed25519"}; - private static final String[] aliasEd448 = - new String[]{"EdDSA", "Ed448"}; + private static final String[] aliasXDH = + new String[]{"XDH", "X25519", "X448"}; + private static final String[] aliasX25519 = + new String[]{"XDH", "X25519"}; public static List getAliases(String algorithm) { return switch (algorithm) { case "EdDSA" -> Arrays.asList(aliasEdDSA); case "Ed25519" -> Arrays.asList(aliasEd25519); - case "Ed448" -> Arrays.asList(aliasEd448); + case "XDH" -> Arrays.asList(aliasXDH); + case "X25519" -> Arrays.asList(aliasX25519); default -> Collections.emptyList(); }; } diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index 707d8f8d058db..ac22419434c92 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -34,12 +34,9 @@ import java.security.cert.CertPathValidatorException; import java.security.cert.CertPathValidatorException.BasicReason; import java.security.interfaces.ECKey; -import java.security.interfaces.EdECKey; -import java.security.interfaces.XECKey; import java.security.spec.AlgorithmParameterSpec; import java.security.spec.InvalidParameterSpecException; import java.security.spec.MGF1ParameterSpec; -import java.security.spec.NamedParameterSpec; import java.security.spec.PSSParameterSpec; import java.time.DateTimeException; import java.time.Instant; @@ -277,10 +274,7 @@ private static List getNamedCurveFromKey(Key key) { } yield Arrays.asList(nc.getNameAndAliases()); } - case XECKey xecKey -> List.of( - ((NamedParameterSpec) xecKey.getParams()).getName()); - case EdECKey edecKey -> List.of((edecKey.getParams()).getName()); - default -> List.of(); + default -> List.of(KeyUtil.getAlgorithm(key)); }; } @@ -305,7 +299,7 @@ private boolean checkConstraints(Set primitives, } // check the key algorithm - if (!permits(primitives, key.getAlgorithm(), null)) { + if (!permits(primitives, KeyUtil.getAlgorithm(key), null)) { return false; } diff --git a/src/java.base/share/classes/sun/security/util/KeyUtil.java b/src/java.base/share/classes/sun/security/util/KeyUtil.java index 1ea8235addd3c..8b570f04cf3c0 100644 --- a/src/java.base/share/classes/sun/security/util/KeyUtil.java +++ b/src/java.base/share/classes/sun/security/util/KeyUtil.java @@ -174,6 +174,22 @@ public static final int getKeySize(AlgorithmParameters parameters) { return -1; } + /* + If the key is a sub-algorithm of a larger group of algorithms, this method + will return that sub-algorithm. For example, key.getAlgorithm() returns + "EdDSA", but the underlying key maybe "Ed448". For + DisabledAlgorithmConstraints (DAC), this distinction is important. + "EdDSA" means all Ed algorithms for DAC, but when using it with + KeyPairGenerator, EdDSA means Ed25519. + */ + public static String getAlgorithm(Key key) { + return switch (key) { + case EdECKey ed -> ed.getParams().getName(); + case XECKey xe -> ((NamedParameterSpec) xe.getParams()).getName(); + default -> key.getAlgorithm(); + }; + } + /** * Returns whether the key is valid or not. *

@@ -424,18 +440,5 @@ public static String hashAlgFromHSS(PublicKey publicKey) throw new NoSuchAlgorithmException("Cannot decode public key", e); } } - - /* - If the key is a sub-algorithm of a larger group of algorithms, this method - will return that sub-algorithm. For example, key.getAlgorithm() returns - "EdDSA", but the underlying key maybe "Ed448". For - DisabledAlgorithmConstraints, this distinction is important. - */ - public static String getAlgorithm(Key key) { - return switch (key) { - case EdECKey ed -> ed.getParams().getName(); - default -> key.getAlgorithm(); - }; - } } diff --git a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java new file mode 100644 index 0000000000000..2e5efe43e6ae1 --- /dev/null +++ b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java @@ -0,0 +1,143 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import sun.security.util.DisabledAlgorithmConstraints; + +import java.security.CryptoPrimitive; +import java.security.KeyPairGenerator; +import java.security.PrivateKey; +import java.security.Security; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; +import java.util.Set; +/* + * @test + * @bug 8346129 + * @modules java.base/sun.security.util + * @summary Check DisabledAlgorithmConstraints methods permit(Key) and + * permit(Set, String, APS) with EdDSA & XDH keys + * + * @run main/othervm DisabledAlgorithmPermits Ed25519 + * @run main/othervm DisabledAlgorithmPermits Ed448 + * @run main/othervm DisabledAlgorithmPermits EdDSA + * @run main/othervm DisabledAlgorithmPermits X25519 + * @run main/othervm DisabledAlgorithmPermits X448 + * @run main/othervm DisabledAlgorithmPermits XDH + */ +public class DisabledAlgorithmPermits { + public static void main(String[] args) throws Exception { + String algorithm = args[0]; + Security.setProperty("x", algorithm); + + // Expected table are the expected results from the test + List expected = switch (algorithm) { + case "Ed25519" -> + Arrays.asList( + new TestCase("EdDSA", false), + new TestCase("Ed25519", false), + new TestCase("Ed448", true), + new TestCase("X448", true)); + case "Ed448" -> + Arrays.asList( + new TestCase("EdDSA", true), + new TestCase("Ed25519", true), + new TestCase("Ed448", false), + new TestCase("X448", true)); + case "EdDSA" -> + Arrays.asList( + new TestCase("EdDSA", false), + new TestCase("Ed25519", false), + new TestCase("Ed448", false), + new TestCase("X448", true)); + case "X25519" -> + Arrays.asList( + new TestCase("XDH", false), + new TestCase("X25519", false), + new TestCase("X448", true), + new TestCase("Ed448", true)); + case "X448" -> + Arrays.asList( + new TestCase("XDH", true), + new TestCase("X25519", true), + new TestCase("X448", false), + new TestCase("Ed448", true)); + case "XDH" -> + Arrays.asList( + new TestCase("XDH", false), + new TestCase("X25519", false), + new TestCase("X448", false), + new TestCase("Ed448", true)); + default -> null; + }; + + Objects.requireNonNull(expected, "algorithm being tested " + + algorithm + " not in expected table"); + System.out.println("---"); + var dac = new DisabledAlgorithmConstraints("x"); + System.out.println("disabled algorithms = " + Security.getProperty("x")); + expected.stream().forEach(tc -> { + boolean r; + System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), \"" + + tc.testAlgo + "\", null): " + + (r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), + tc.testAlgo, null)) + " : " ); + if (r != tc.expected) { + System.out.println("failed."); + throw new AssertionError("failed. Expected " + + tc.expected); + } + System.out.println("pass"); + }); + expected.stream().forEach(tc -> { + PrivateKey k; + try { + k = KeyPairGenerator.getInstance(tc.testAlgo).generateKeyPair(). + getPrivate(); + } catch (Exception e) { + throw new RuntimeException(e); + } + boolean r; + System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), " + + tc.testAlgo + " privkey): " + + (r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), k)) + + " : " ); + if (r != tc.expected) { + System.out.println("failed."); + throw new AssertionError("failed. Expected " + + tc.expected); + } + System.out.println("pass"); + }); + System.out.println("---"); + } + + record TestCase(int testType, String testAlgo, boolean expected) { + TestCase(String testAlgo, boolean expected) { + this( 0, testAlgo, expected); + } + } +} + From 42a329a577ab0e528e2c5dacc268d4bef179aef0 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Sat, 8 Feb 2025 20:31:21 -0800 Subject: [PATCH 05/15] cleanup --- .../provider/certpath/AlgorithmChecker.java | 6 +- .../util/AbstractAlgorithmConstraints.java | 31 +++- .../util/DisabledAlgorithmConstraints.java | 34 ++--- .../classes/sun/security/util/KeyUtil.java | 16 ++ .../DisabledAlgorithmPermits.java | 143 ++++++++++++++++++ 5 files changed, 207 insertions(+), 23 deletions(-) create mode 100644 test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java diff --git a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java index 7ce31492b4b41..f127ac65cc7b4 100644 --- a/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java +++ b/src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -37,6 +37,7 @@ import sun.security.util.Debug; import sun.security.util.DisabledAlgorithmConstraints; +import sun.security.util.KeyUtil; import sun.security.validator.Validator; import sun.security.x509.AlgorithmId; import sun.security.x509.X509CertImpl; @@ -206,7 +207,8 @@ public void check(Certificate cert, CertPathConstraintsParameters cp = new CertPathConstraintsParameters(trustedPubKey, variant, anchor, date); - dac.permits(trustedPubKey.getAlgorithm(), cp, true); + dac.permits(KeyUtil.getAlgorithm(trustedPubKey), + cp, true); } // Check the signature algorithm and parameters against constraints CertPathConstraintsParameters cp = diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index dc5b1aafb2017..d00d79bb76064 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,6 +29,7 @@ import java.security.Security; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.Set; import java.util.TreeSet; @@ -70,14 +71,38 @@ static Set getAlgorithms(String propertyName) { return algorithmsInPropertySet; } + + private static final String[] aliasEdDSA = + new String[]{"EdDSA", "Ed25519", "Ed448"}; + private static final String[] aliasEd25519 = + new String[]{"EdDSA", "Ed25519"}; + private static final String[] aliasXDH = + new String[]{"XDH", "X25519", "X448"}; + private static final String[] aliasX25519 = + new String[]{"XDH", "X25519"}; + + public static List getAliases(String algorithm) { + return switch (algorithm) { + case "EdDSA" -> Arrays.asList(aliasEdDSA); + case "Ed25519" -> Arrays.asList(aliasEd25519); + case "XDH" -> Arrays.asList(aliasXDH); + case "X25519" -> Arrays.asList(aliasX25519); + default -> Collections.emptyList(); + }; + } + static boolean checkAlgorithm(Set algorithms, String algorithm, AlgorithmDecomposer decomposer) { if (algorithm == null || algorithm.isEmpty()) { throw new IllegalArgumentException("No algorithm name specified"); } - if (algorithms.contains(algorithm)) { - return false; + // Check `algorithm` against disabled algorithms and their aliases + for (String a : algorithms) { + if (algorithm.equalsIgnoreCase(a) || + getAliases(a).contains(algorithm)) { + return false; + } } // decompose the algorithm into sub-elements diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index 7612242733eeb..ac22419434c92 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -34,11 +34,9 @@ import java.security.cert.CertPathValidatorException; import java.security.cert.CertPathValidatorException.BasicReason; import java.security.interfaces.ECKey; -import java.security.interfaces.XECKey; import java.security.spec.AlgorithmParameterSpec; import java.security.spec.InvalidParameterSpecException; import java.security.spec.MGF1ParameterSpec; -import java.security.spec.NamedParameterSpec; import java.security.spec.PSSParameterSpec; import java.time.DateTimeException; import java.time.Instant; @@ -154,7 +152,7 @@ public DisabledAlgorithmConstraints(String propertyName, break; } } - algorithmConstraints = new Constraints(propertyName, disabledAlgorithms); + algorithmConstraints = new Constraints(disabledAlgorithms); } /* @@ -268,16 +266,16 @@ public final void permits(String algorithm, ConstraintsParameters cp, } private static List getNamedCurveFromKey(Key key) { - if (key instanceof ECKey) { - NamedCurve nc = CurveDB.lookup(((ECKey)key).getParams()); - return (nc == null ? List.of() - : Arrays.asList(nc.getNameAndAliases())); - } else if (key instanceof XECKey) { - return List.of( - ((NamedParameterSpec)((XECKey)key).getParams()).getName()); - } else { - return List.of(); - } + return switch (key) { + case ECKey ecKey -> { + NamedCurve nc = CurveDB.lookup(ecKey.getParams()); + if (nc == null) { + yield List.of(); + } + yield Arrays.asList(nc.getNameAndAliases()); + } + default -> List.of(KeyUtil.getAlgorithm(key)); + }; } // Check algorithm constraints with key and algorithm @@ -301,7 +299,7 @@ private boolean checkConstraints(Set primitives, } // check the key algorithm - if (!permits(primitives, key.getAlgorithm(), null)) { + if (!permits(primitives, KeyUtil.getAlgorithm(key), null)) { return false; } @@ -338,7 +336,7 @@ private static class Holder { "denyAfter\\s+(\\d{4})-(\\d{2})-(\\d{2})"); } - public Constraints(String propertyName, Set constraintSet) { + public Constraints(Set constraintSet) { for (String constraintEntry : constraintSet) { if (constraintEntry == null || constraintEntry.isEmpty()) { continue; @@ -460,7 +458,7 @@ private List getConstraints(String algorithm) { // Check if KeySizeConstraints permit the specified key public boolean permits(Key key) { - List list = getConstraints(key.getAlgorithm()); + List list = getConstraints(KeyUtil.getAlgorithm(key)); if (list == null) { return true; } @@ -514,7 +512,7 @@ public void permits(String algorithm, ConstraintsParameters cp, if (checkKey) { for (Key key : cp.getKeys()) { - algorithms.add(key.getAlgorithm()); + algorithms.add(KeyUtil.getAlgorithm(key)); } } diff --git a/src/java.base/share/classes/sun/security/util/KeyUtil.java b/src/java.base/share/classes/sun/security/util/KeyUtil.java index 6eb5acfade3fd..8b570f04cf3c0 100644 --- a/src/java.base/share/classes/sun/security/util/KeyUtil.java +++ b/src/java.base/share/classes/sun/security/util/KeyUtil.java @@ -174,6 +174,22 @@ public static final int getKeySize(AlgorithmParameters parameters) { return -1; } + /* + If the key is a sub-algorithm of a larger group of algorithms, this method + will return that sub-algorithm. For example, key.getAlgorithm() returns + "EdDSA", but the underlying key maybe "Ed448". For + DisabledAlgorithmConstraints (DAC), this distinction is important. + "EdDSA" means all Ed algorithms for DAC, but when using it with + KeyPairGenerator, EdDSA means Ed25519. + */ + public static String getAlgorithm(Key key) { + return switch (key) { + case EdECKey ed -> ed.getParams().getName(); + case XECKey xe -> ((NamedParameterSpec) xe.getParams()).getName(); + default -> key.getAlgorithm(); + }; + } + /** * Returns whether the key is valid or not. *

diff --git a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java new file mode 100644 index 0000000000000..2e5efe43e6ae1 --- /dev/null +++ b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java @@ -0,0 +1,143 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import sun.security.util.DisabledAlgorithmConstraints; + +import java.security.CryptoPrimitive; +import java.security.KeyPairGenerator; +import java.security.PrivateKey; +import java.security.Security; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; +import java.util.Set; +/* + * @test + * @bug 8346129 + * @modules java.base/sun.security.util + * @summary Check DisabledAlgorithmConstraints methods permit(Key) and + * permit(Set, String, APS) with EdDSA & XDH keys + * + * @run main/othervm DisabledAlgorithmPermits Ed25519 + * @run main/othervm DisabledAlgorithmPermits Ed448 + * @run main/othervm DisabledAlgorithmPermits EdDSA + * @run main/othervm DisabledAlgorithmPermits X25519 + * @run main/othervm DisabledAlgorithmPermits X448 + * @run main/othervm DisabledAlgorithmPermits XDH + */ +public class DisabledAlgorithmPermits { + public static void main(String[] args) throws Exception { + String algorithm = args[0]; + Security.setProperty("x", algorithm); + + // Expected table are the expected results from the test + List expected = switch (algorithm) { + case "Ed25519" -> + Arrays.asList( + new TestCase("EdDSA", false), + new TestCase("Ed25519", false), + new TestCase("Ed448", true), + new TestCase("X448", true)); + case "Ed448" -> + Arrays.asList( + new TestCase("EdDSA", true), + new TestCase("Ed25519", true), + new TestCase("Ed448", false), + new TestCase("X448", true)); + case "EdDSA" -> + Arrays.asList( + new TestCase("EdDSA", false), + new TestCase("Ed25519", false), + new TestCase("Ed448", false), + new TestCase("X448", true)); + case "X25519" -> + Arrays.asList( + new TestCase("XDH", false), + new TestCase("X25519", false), + new TestCase("X448", true), + new TestCase("Ed448", true)); + case "X448" -> + Arrays.asList( + new TestCase("XDH", true), + new TestCase("X25519", true), + new TestCase("X448", false), + new TestCase("Ed448", true)); + case "XDH" -> + Arrays.asList( + new TestCase("XDH", false), + new TestCase("X25519", false), + new TestCase("X448", false), + new TestCase("Ed448", true)); + default -> null; + }; + + Objects.requireNonNull(expected, "algorithm being tested " + + algorithm + " not in expected table"); + System.out.println("---"); + var dac = new DisabledAlgorithmConstraints("x"); + System.out.println("disabled algorithms = " + Security.getProperty("x")); + expected.stream().forEach(tc -> { + boolean r; + System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), \"" + + tc.testAlgo + "\", null): " + + (r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), + tc.testAlgo, null)) + " : " ); + if (r != tc.expected) { + System.out.println("failed."); + throw new AssertionError("failed. Expected " + + tc.expected); + } + System.out.println("pass"); + }); + expected.stream().forEach(tc -> { + PrivateKey k; + try { + k = KeyPairGenerator.getInstance(tc.testAlgo).generateKeyPair(). + getPrivate(); + } catch (Exception e) { + throw new RuntimeException(e); + } + boolean r; + System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), " + + tc.testAlgo + " privkey): " + + (r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), k)) + + " : " ); + if (r != tc.expected) { + System.out.println("failed."); + throw new AssertionError("failed. Expected " + + tc.expected); + } + System.out.println("pass"); + }); + System.out.println("---"); + } + + record TestCase(int testType, String testAlgo, boolean expected) { + TestCase(String testAlgo, boolean expected) { + this( 0, testAlgo, expected); + } + } +} + From de2a57ac38be119891e14d23acf55fd19f46f969 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Fri, 14 Feb 2025 10:36:32 -0800 Subject: [PATCH 06/15] comments --- .../util/AbstractAlgorithmConstraints.java | 15 ++++++++++++++- .../share/classes/sun/security/util/KeyUtil.java | 2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index d00d79bb76064..44a93e7f83676 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -71,7 +71,6 @@ static Set getAlgorithms(String propertyName) { return algorithmsInPropertySet; } - private static final String[] aliasEdDSA = new String[]{"EdDSA", "Ed25519", "Ed448"}; private static final String[] aliasEd25519 = @@ -81,6 +80,11 @@ static Set getAlgorithms(String propertyName) { private static final String[] aliasX25519 = new String[]{"XDH", "X25519"}; + /** + * getAlias() adds extra algorithm names to the String if matched. Used by + * checkAlgorithm(), it returns additional names that may be on the + * DisabledAlgorithmConstraints list. + */ public static List getAliases(String algorithm) { return switch (algorithm) { case "EdDSA" -> Arrays.asList(aliasEdDSA); @@ -91,6 +95,15 @@ public static List getAliases(String algorithm) { }; } + /** + * This checks a given `algorithm' against the list of 'algorithms' from + * the DisabledAlgorithmConstraints or LegacyAlgorithmConstraints. + * + * @param algorithms List of algorithms from the constraints list + * @param algorithm algorithm being checked against list + * @param decomposer class the decomposing names into sub-elements + * @return + */ static boolean checkAlgorithm(Set algorithms, String algorithm, AlgorithmDecomposer decomposer) { if (algorithm == null || algorithm.isEmpty()) { diff --git a/src/java.base/share/classes/sun/security/util/KeyUtil.java b/src/java.base/share/classes/sun/security/util/KeyUtil.java index 8b570f04cf3c0..b7a3e563850ce 100644 --- a/src/java.base/share/classes/sun/security/util/KeyUtil.java +++ b/src/java.base/share/classes/sun/security/util/KeyUtil.java @@ -179,7 +179,7 @@ public static final int getKeySize(AlgorithmParameters parameters) { will return that sub-algorithm. For example, key.getAlgorithm() returns "EdDSA", but the underlying key maybe "Ed448". For DisabledAlgorithmConstraints (DAC), this distinction is important. - "EdDSA" means all Ed algorithms for DAC, but when using it with + "EdDSA" means all curves for DAC, but when using it with KeyPairGenerator, EdDSA means Ed25519. */ public static String getAlgorithm(Key key) { From d2a4f0bfff672eb5b640ef38c042ce0cfc7f5b1a Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Fri, 14 Feb 2025 13:18:43 -0800 Subject: [PATCH 07/15] immutable --- .../util/AbstractAlgorithmConstraints.java | 43 +++++++++++++------ .../util/DisabledAlgorithmConstraints.java | 3 +- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index 44a93e7f83676..127506304b4a4 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -71,14 +71,11 @@ static Set getAlgorithms(String propertyName) { return algorithmsInPropertySet; } - private static final String[] aliasEdDSA = - new String[]{"EdDSA", "Ed25519", "Ed448"}; - private static final String[] aliasEd25519 = - new String[]{"EdDSA", "Ed25519"}; - private static final String[] aliasXDH = - new String[]{"XDH", "X25519", "X448"}; - private static final String[] aliasX25519 = - new String[]{"XDH", "X25519"}; + // Allocate the immutable lists as needed. Overwriting is not a concern. + private static List aliasEdDSA = null; + private static List aliasEd25519 = null; + private static List aliasXDH = null; + private static List aliasX25519 = null; /** * getAlias() adds extra algorithm names to the String if matched. Used by @@ -87,11 +84,31 @@ static Set getAlgorithms(String propertyName) { */ public static List getAliases(String algorithm) { return switch (algorithm) { - case "EdDSA" -> Arrays.asList(aliasEdDSA); - case "Ed25519" -> Arrays.asList(aliasEd25519); - case "XDH" -> Arrays.asList(aliasXDH); - case "X25519" -> Arrays.asList(aliasX25519); - default -> Collections.emptyList(); + case "EdDSA" -> { + if (aliasEdDSA == null) { + aliasEdDSA = List.of("EdDSA", "Ed25519", "Ed448"); + } + yield aliasEdDSA; + } + case "Ed25519" -> { + if (aliasEd25519 == null) { + aliasEd25519 = List.of("EdDSA", "Ed25519"); + } + yield aliasEd25519; + } + case "XDH" -> { + if (aliasXDH == null) { + aliasXDH = List.of("XDH", "X25519", "X448"); + } + yield aliasXDH; + } + case "X25519" -> { + if (aliasX25519 == null) { + aliasX25519 = List.of("XDH", "X25519"); + } + yield aliasX25519; + } + default -> List.of(); }; } diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index ac22419434c92..4eb9d7e9cabf6 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -43,7 +43,6 @@ import java.time.ZoneId; import java.time.ZonedDateTime; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -272,7 +271,7 @@ private static List getNamedCurveFromKey(Key key) { if (nc == null) { yield List.of(); } - yield Arrays.asList(nc.getNameAndAliases()); + yield List.of(nc.getNameAndAliases()); } default -> List.of(KeyUtil.getAlgorithm(key)); }; From 9fbddcc2a5ede7eec19c8c8e6a1208e086a57705 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Fri, 14 Feb 2025 13:23:03 -0800 Subject: [PATCH 08/15] spacing --- .../util/AlgorithmConstraints/DisabledAlgorithmPermits.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java index 2e5efe43e6ae1..a45551e7c4964 100644 --- a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java +++ b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java @@ -33,6 +33,7 @@ import java.util.List; import java.util.Objects; import java.util.Set; + /* * @test * @bug 8346129 @@ -47,6 +48,7 @@ * @run main/othervm DisabledAlgorithmPermits X448 * @run main/othervm DisabledAlgorithmPermits XDH */ + public class DisabledAlgorithmPermits { public static void main(String[] args) throws Exception { String algorithm = args[0]; From 58aa6d634565be05f61beda7c862bd2b946f72b1 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Fri, 21 Feb 2025 17:51:49 -0800 Subject: [PATCH 09/15] review comments and remove aliases --- .../util/AbstractAlgorithmConstraints.java | 47 +------------ .../util/DisabledAlgorithmConstraints.java | 5 +- .../classes/sun/security/util/KeyUtil.java | 16 +++-- .../DisabledAlgorithmPermits.java | 67 ++++++++++++++----- 4 files changed, 65 insertions(+), 70 deletions(-) diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index 127506304b4a4..83c02b9e53482 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -29,7 +29,6 @@ import java.security.Security; import java.util.Arrays; import java.util.Collections; -import java.util.List; import java.util.Set; import java.util.TreeSet; @@ -71,47 +70,6 @@ static Set getAlgorithms(String propertyName) { return algorithmsInPropertySet; } - // Allocate the immutable lists as needed. Overwriting is not a concern. - private static List aliasEdDSA = null; - private static List aliasEd25519 = null; - private static List aliasXDH = null; - private static List aliasX25519 = null; - - /** - * getAlias() adds extra algorithm names to the String if matched. Used by - * checkAlgorithm(), it returns additional names that may be on the - * DisabledAlgorithmConstraints list. - */ - public static List getAliases(String algorithm) { - return switch (algorithm) { - case "EdDSA" -> { - if (aliasEdDSA == null) { - aliasEdDSA = List.of("EdDSA", "Ed25519", "Ed448"); - } - yield aliasEdDSA; - } - case "Ed25519" -> { - if (aliasEd25519 == null) { - aliasEd25519 = List.of("EdDSA", "Ed25519"); - } - yield aliasEd25519; - } - case "XDH" -> { - if (aliasXDH == null) { - aliasXDH = List.of("XDH", "X25519", "X448"); - } - yield aliasXDH; - } - case "X25519" -> { - if (aliasX25519 == null) { - aliasX25519 = List.of("XDH", "X25519"); - } - yield aliasX25519; - } - default -> List.of(); - }; - } - /** * This checks a given `algorithm' against the list of 'algorithms' from * the DisabledAlgorithmConstraints or LegacyAlgorithmConstraints. @@ -127,10 +85,9 @@ static boolean checkAlgorithm(Set algorithms, String algorithm, throw new IllegalArgumentException("No algorithm name specified"); } - // Check `algorithm` against disabled algorithms and their aliases + // Check `algorithm` against disabled algorithms for (String a : algorithms) { - if (algorithm.equalsIgnoreCase(a) || - getAliases(a).contains(algorithm)) { + if (algorithm.equalsIgnoreCase(a)) { return false; } } diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index 4eb9d7e9cabf6..670d4bad5323a 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -156,7 +156,8 @@ public DisabledAlgorithmConstraints(String propertyName, /* * This only checks if the algorithm has been completely disabled. If - * there are keysize or other limit, this method allow the algorithm. + * there are keysize, naming complexities (Ed & XDH), or other limit, this + * method will allow the algorithm. */ @Override public final boolean permits(Set primitives, @@ -273,7 +274,7 @@ private static List getNamedCurveFromKey(Key key) { } yield List.of(nc.getNameAndAliases()); } - default -> List.of(KeyUtil.getAlgorithm(key)); + default -> List.of(key.getAlgorithm(), KeyUtil.getAlgorithm(key)); }; } diff --git a/src/java.base/share/classes/sun/security/util/KeyUtil.java b/src/java.base/share/classes/sun/security/util/KeyUtil.java index b7a3e563850ce..ed1bd5140aad6 100644 --- a/src/java.base/share/classes/sun/security/util/KeyUtil.java +++ b/src/java.base/share/classes/sun/security/util/KeyUtil.java @@ -38,6 +38,8 @@ import javax.crypto.spec.DHPublicKeySpec; import sun.security.jca.JCAUtil; +import sun.security.pkcs.NamedPKCS8Key; +import sun.security.provider.ML_DSA; /** * A utility class to get key length, validate keys, etc. @@ -174,13 +176,13 @@ public static final int getKeySize(AlgorithmParameters parameters) { return -1; } - /* - If the key is a sub-algorithm of a larger group of algorithms, this method - will return that sub-algorithm. For example, key.getAlgorithm() returns - "EdDSA", but the underlying key maybe "Ed448". For - DisabledAlgorithmConstraints (DAC), this distinction is important. - "EdDSA" means all curves for DAC, but when using it with - KeyPairGenerator, EdDSA means Ed25519. + /** + * If the key is a sub-algorithm of a larger group of algorithms, this + * method will return that sub-algorithm. For example, key.getAlgorithm() + * returns "EdDSA", but the underlying key may be "Ed448". For + * DisabledAlgorithmConstraints (DAC), this distinction is important. + * "EdDSA" means all curves for DAC, but when using it with + * KeyPairGenerator, EdDSA means Ed25519. */ public static String getAlgorithm(Key key) { return switch (key) { diff --git a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java index a45551e7c4964..b1c0e8db59b41 100644 --- a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java +++ b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java @@ -38,8 +38,12 @@ * @test * @bug 8346129 * @modules java.base/sun.security.util - * @summary Check DisabledAlgorithmConstraints methods permit(Key) and - * permit(Set, String, APS) with EdDSA & XDH keys + * @summary Check DisabledAlgorithmConstraints using EdDSA & XDH against + * permit(Set, String, AP) and + * permit(Set, Key). Results will differ based + * on method used. The first method only can compare the String with the + * algorithms. The second can check the algorithm and NamedParameterSpec + * while results in more 'false' cases. * * @run main/othervm DisabledAlgorithmPermits Ed25519 * @run main/othervm DisabledAlgorithmPermits Ed448 @@ -58,40 +62,64 @@ public static void main(String[] args) throws Exception { List expected = switch (algorithm) { case "Ed25519" -> Arrays.asList( - new TestCase("EdDSA", false), + new TestCase("EdDSA", true), new TestCase("Ed25519", false), new TestCase("Ed448", true), - new TestCase("X448", true)); + new TestCase("X448", true), + new TestCase(1,"EdDSA", false), + new TestCase(1,"Ed25519", false), + new TestCase(1,"Ed448", true), + new TestCase(1,"X448", true)); case "Ed448" -> Arrays.asList( new TestCase("EdDSA", true), new TestCase("Ed25519", true), new TestCase("Ed448", false), - new TestCase("X448", true)); + new TestCase("X448", true), + new TestCase(1,"EdDSA", true), + new TestCase(1,"Ed25519", true), + new TestCase(1,"Ed448", false), + new TestCase(1,"X448", true)); case "EdDSA" -> Arrays.asList( new TestCase("EdDSA", false), - new TestCase("Ed25519", false), - new TestCase("Ed448", false), - new TestCase("X448", true)); + new TestCase("Ed25519", true), + new TestCase("Ed448", true), + new TestCase("X448", true), + new TestCase(1,"EdDSA", false), + new TestCase(1,"Ed25519", false), + new TestCase(1,"Ed448", false), + new TestCase(1,"X448", true)); case "X25519" -> Arrays.asList( - new TestCase("XDH", false), + new TestCase("XDH", true), new TestCase("X25519", false), new TestCase("X448", true), - new TestCase("Ed448", true)); + new TestCase("Ed448", true), + new TestCase(1, "XDH", false), + new TestCase(1, "X25519", false), + new TestCase(1, "X448", true), + new TestCase(1, "Ed448", true)); case "X448" -> Arrays.asList( new TestCase("XDH", true), new TestCase("X25519", true), new TestCase("X448", false), - new TestCase("Ed448", true)); + new TestCase("Ed448", true), + new TestCase(1, "XDH", true), + new TestCase(1, "X25519", true), + new TestCase(1, "X448", false), + new TestCase(1, "Ed448", true)); case "XDH" -> Arrays.asList( new TestCase("XDH", false), - new TestCase("X25519", false), - new TestCase("X448", false), - new TestCase("Ed448", true)); + new TestCase("X25519", true), + new TestCase("X448", true), + new TestCase("Ed448", true), + new TestCase(1, "XDH", false), + new TestCase(1, "X25519", false), + new TestCase(1, "X448", false), + new TestCase(1, "Ed448", true)); default -> null; }; @@ -100,7 +128,10 @@ public static void main(String[] args) throws Exception { System.out.println("---"); var dac = new DisabledAlgorithmConstraints("x"); System.out.println("disabled algorithms = " + Security.getProperty("x")); - expected.stream().forEach(tc -> { + + // Using only testType 0, this tests that permit(Set<>, String, null) + // will check only the algorithm against the disabled list + expected.stream().filter(n->n.testType == 0).forEach(tc -> { boolean r; System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), \"" + tc.testAlgo + "\", null): " + @@ -113,7 +144,11 @@ public static void main(String[] args) throws Exception { } System.out.println("pass"); }); - expected.stream().forEach(tc -> { + + // Using only testType 1, this tests permit(Set<>, Key) that will look + // at both the key.getAlgorithm() and the key.getParams().getName() + // against the disabled list + expected.stream().filter(n->n.testType == 1).forEach(tc -> { PrivateKey k; try { k = KeyPairGenerator.getInstance(tc.testAlgo).generateKeyPair(). From 203833b3decc94d9e2494f39a8e38465de6925a6 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Mon, 24 Feb 2025 13:21:36 -0800 Subject: [PATCH 10/15] undo comment --- .../sun/security/util/DisabledAlgorithmConstraints.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index 670d4bad5323a..e04f92ce4c24b 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -156,8 +156,7 @@ public DisabledAlgorithmConstraints(String propertyName, /* * This only checks if the algorithm has been completely disabled. If - * there are keysize, naming complexities (Ed & XDH), or other limit, this - * method will allow the algorithm. + * there are keysize or other limit, this method allow the algorithm. */ @Override public final boolean permits(Set primitives, From 3cbee6a2a238df4a8cd85c42b5bedd2e8f727b1c Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Tue, 25 Feb 2025 12:20:54 -0800 Subject: [PATCH 11/15] Undo AbtractAlgorithConstraint change & use AsymmetricKey --- .../util/AbstractAlgorithmConstraints.java | 18 +++--------------- .../util/DisabledAlgorithmConstraints.java | 4 ++-- .../classes/sun/security/util/KeyUtil.java | 14 ++++++-------- 3 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java index 83c02b9e53482..dc5b1aafb2017 100644 --- a/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -70,26 +70,14 @@ static Set getAlgorithms(String propertyName) { return algorithmsInPropertySet; } - /** - * This checks a given `algorithm' against the list of 'algorithms' from - * the DisabledAlgorithmConstraints or LegacyAlgorithmConstraints. - * - * @param algorithms List of algorithms from the constraints list - * @param algorithm algorithm being checked against list - * @param decomposer class the decomposing names into sub-elements - * @return - */ static boolean checkAlgorithm(Set algorithms, String algorithm, AlgorithmDecomposer decomposer) { if (algorithm == null || algorithm.isEmpty()) { throw new IllegalArgumentException("No algorithm name specified"); } - // Check `algorithm` against disabled algorithms - for (String a : algorithms) { - if (algorithm.equalsIgnoreCase(a)) { - return false; - } + if (algorithms.contains(algorithm)) { + return false; } // decompose the algorithm into sub-elements diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index e04f92ce4c24b..609b787860a5c 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -151,7 +151,7 @@ public DisabledAlgorithmConstraints(String propertyName, break; } } - algorithmConstraints = new Constraints(disabledAlgorithms); + algorithmConstraints = new Constraints(propertyName, disabledAlgorithms); } /* @@ -335,7 +335,7 @@ private static class Holder { "denyAfter\\s+(\\d{4})-(\\d{2})-(\\d{2})"); } - public Constraints(Set constraintSet) { + public Constraints(String propertyName, Set constraintSet) { for (String constraintEntry : constraintSet) { if (constraintEntry == null || constraintEntry.isEmpty()) { continue; diff --git a/src/java.base/share/classes/sun/security/util/KeyUtil.java b/src/java.base/share/classes/sun/security/util/KeyUtil.java index ed1bd5140aad6..77c09001724d3 100644 --- a/src/java.base/share/classes/sun/security/util/KeyUtil.java +++ b/src/java.base/share/classes/sun/security/util/KeyUtil.java @@ -38,8 +38,6 @@ import javax.crypto.spec.DHPublicKeySpec; import sun.security.jca.JCAUtil; -import sun.security.pkcs.NamedPKCS8Key; -import sun.security.provider.ML_DSA; /** * A utility class to get key length, validate keys, etc. @@ -182,14 +180,14 @@ public static final int getKeySize(AlgorithmParameters parameters) { * returns "EdDSA", but the underlying key may be "Ed448". For * DisabledAlgorithmConstraints (DAC), this distinction is important. * "EdDSA" means all curves for DAC, but when using it with - * KeyPairGenerator, EdDSA means Ed25519. + * KeyPairGenerator, "EdDSA" means "Ed25519". */ public static String getAlgorithm(Key key) { - return switch (key) { - case EdECKey ed -> ed.getParams().getName(); - case XECKey xe -> ((NamedParameterSpec) xe.getParams()).getName(); - default -> key.getAlgorithm(); - }; + if (key instanceof AsymmetricKey ak && + ak.getParams() instanceof NamedParameterSpec nps) { + return nps.getName(); + } + return key.getAlgorithm(); } /** From b26118a5516d592c38bf0d543fb85870fd411fa7 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Wed, 5 Mar 2025 11:01:06 -0800 Subject: [PATCH 12/15] rename getNamedCurveFromKey --- .../sun/security/util/DisabledAlgorithmConstraints.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index 609b787860a5c..e6ef6c5ca17c1 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -251,7 +251,7 @@ public final void permits(String algorithm, ConstraintsParameters cp, if (checkKey) { // Check if named curves in the key are disabled. for (Key key : cp.getKeys()) { - for (String curve : getNamedCurveFromKey(key)) { + for (String curve : getNamedParametersFromKey(key)) { if (!cachedCheckAlgorithm(curve)) { throw new CertPathValidatorException( "Algorithm constraints check failed on disabled " + @@ -264,7 +264,7 @@ public final void permits(String algorithm, ConstraintsParameters cp, algorithmConstraints.permits(algorithm, cp, checkKey); } - private static List getNamedCurveFromKey(Key key) { + private static List getNamedParametersFromKey(Key key) { return switch (key) { case ECKey ecKey -> { NamedCurve nc = CurveDB.lookup(ecKey.getParams()); @@ -303,7 +303,7 @@ private boolean checkConstraints(Set primitives, } // If this is an elliptic curve, check if it is disabled - for (String curve : getNamedCurveFromKey(key)) { + for (String curve : getNamedParametersFromKey(key)) { if (!permits(primitives, curve, null)) { return false; } From a0fc101f3e6b8b057a1b19b5c5558dff8ec0086f Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Tue, 11 Mar 2025 09:45:43 -0700 Subject: [PATCH 13/15] check for dup --- .../sun/security/util/DisabledAlgorithmConstraints.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java index e6ef6c5ca17c1..be5239f9fca53 100644 --- a/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java +++ b/src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java @@ -273,7 +273,13 @@ private static List getNamedParametersFromKey(Key key) { } yield List.of(nc.getNameAndAliases()); } - default -> List.of(key.getAlgorithm(), KeyUtil.getAlgorithm(key)); + default -> { + String n = KeyUtil.getAlgorithm(key); + if (n.equalsIgnoreCase(key.getAlgorithm())) { + yield List.of(n); + } + yield List.of(key.getAlgorithm(), n); + } }; } From 799c2acc4f05a668d262e1a0723e55401bf8e804 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Fri, 21 Mar 2025 13:32:55 -0700 Subject: [PATCH 14/15] comments on test --- .../DisabledAlgorithmPermits.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java index b1c0e8db59b41..e34a1d3e2fec5 100644 --- a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java +++ b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java @@ -132,11 +132,10 @@ public static void main(String[] args) throws Exception { // Using only testType 0, this tests that permit(Set<>, String, null) // will check only the algorithm against the disabled list expected.stream().filter(n->n.testType == 0).forEach(tc -> { - boolean r; + boolean r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), + tc.testAlgo, null); System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), \"" + - tc.testAlgo + "\", null): " + - (r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), - tc.testAlgo, null)) + " : " ); + tc.testAlgo + "\", null): " + r + " : " ); if (r != tc.expected) { System.out.println("failed."); throw new AssertionError("failed. Expected " + @@ -156,11 +155,9 @@ public static void main(String[] args) throws Exception { } catch (Exception e) { throw new RuntimeException(e); } - boolean r; + boolean r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), k); System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), " + - tc.testAlgo + " privkey): " + - (r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), k)) + - " : " ); + tc.testAlgo + " privkey): " + r + " : " ); if (r != tc.expected) { System.out.println("failed."); throw new AssertionError("failed. Expected " + From 6ff9f8f05a0370b4044b8ae6ac0253eec58e4779 Mon Sep 17 00:00:00 2001 From: Anthony Scarpino Date: Tue, 25 Mar 2025 15:28:13 -0700 Subject: [PATCH 15/15] code review comments --- .../util/AlgorithmConstraints/DisabledAlgorithmPermits.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java index e34a1d3e2fec5..913aa1266f46c 100644 --- a/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java +++ b/test/jdk/sun/security/util/AlgorithmConstraints/DisabledAlgorithmPermits.java @@ -4,9 +4,7 @@ * * This code is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. + * published by the Free Software Foundation. * * This code is distributed in the hope that it will be useful, but WITHOUT * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or @@ -170,7 +168,7 @@ public static void main(String[] args) throws Exception { record TestCase(int testType, String testAlgo, boolean expected) { TestCase(String testAlgo, boolean expected) { - this( 0, testAlgo, expected); + this(0, testAlgo, expected); } } }