Skip to content

Commit e4e6278

Browse files
author
Anthony Scarpino
committed
8346129: Simplify EdDSA & XDH curve name usage
Reviewed-by: weijun, abarashev
1 parent 7a2e198 commit e4e6278

File tree

4 files changed

+218
-22
lines changed

4 files changed

+218
-22
lines changed

src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2009, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -37,6 +37,7 @@
3737

3838
import sun.security.util.Debug;
3939
import sun.security.util.DisabledAlgorithmConstraints;
40+
import sun.security.util.KeyUtil;
4041
import sun.security.validator.Validator;
4142
import sun.security.x509.AlgorithmId;
4243
import sun.security.x509.X509CertImpl;
@@ -206,7 +207,8 @@ public void check(Certificate cert,
206207
CertPathConstraintsParameters cp =
207208
new CertPathConstraintsParameters(trustedPubKey, variant,
208209
anchor, date);
209-
dac.permits(trustedPubKey.getAlgorithm(), cp, true);
210+
dac.permits(KeyUtil.getAlgorithm(trustedPubKey),
211+
cp, true);
210212
}
211213
// Check the signature algorithm and parameters against constraints
212214
CertPathConstraintsParameters cp =

src/java.base/share/classes/sun/security/util/DisabledAlgorithmConstraints.java

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2010, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -34,18 +34,15 @@
3434
import java.security.cert.CertPathValidatorException;
3535
import java.security.cert.CertPathValidatorException.BasicReason;
3636
import java.security.interfaces.ECKey;
37-
import java.security.interfaces.XECKey;
3837
import java.security.spec.AlgorithmParameterSpec;
3938
import java.security.spec.InvalidParameterSpecException;
4039
import java.security.spec.MGF1ParameterSpec;
41-
import java.security.spec.NamedParameterSpec;
4240
import java.security.spec.PSSParameterSpec;
4341
import java.time.DateTimeException;
4442
import java.time.Instant;
4543
import java.time.ZoneId;
4644
import java.time.ZonedDateTime;
4745
import java.util.ArrayList;
48-
import java.util.Arrays;
4946
import java.util.Collection;
5047
import java.util.HashMap;
5148
import java.util.HashSet;
@@ -254,7 +251,7 @@ public final void permits(String algorithm, ConstraintsParameters cp,
254251
if (checkKey) {
255252
// Check if named curves in the key are disabled.
256253
for (Key key : cp.getKeys()) {
257-
for (String curve : getNamedCurveFromKey(key)) {
254+
for (String curve : getNamedParametersFromKey(key)) {
258255
if (!cachedCheckAlgorithm(curve)) {
259256
throw new CertPathValidatorException(
260257
"Algorithm constraints check failed on disabled " +
@@ -267,17 +264,23 @@ public final void permits(String algorithm, ConstraintsParameters cp,
267264
algorithmConstraints.permits(algorithm, cp, checkKey);
268265
}
269266

270-
private static List<String> getNamedCurveFromKey(Key key) {
271-
if (key instanceof ECKey) {
272-
NamedCurve nc = CurveDB.lookup(((ECKey)key).getParams());
273-
return (nc == null ? List.of()
274-
: Arrays.asList(nc.getNameAndAliases()));
275-
} else if (key instanceof XECKey) {
276-
return List.of(
277-
((NamedParameterSpec)((XECKey)key).getParams()).getName());
278-
} else {
279-
return List.of();
280-
}
267+
private static List<String> getNamedParametersFromKey(Key key) {
268+
return switch (key) {
269+
case ECKey ecKey -> {
270+
NamedCurve nc = CurveDB.lookup(ecKey.getParams());
271+
if (nc == null) {
272+
yield List.of();
273+
}
274+
yield List.of(nc.getNameAndAliases());
275+
}
276+
default -> {
277+
String n = KeyUtil.getAlgorithm(key);
278+
if (n.equalsIgnoreCase(key.getAlgorithm())) {
279+
yield List.of(n);
280+
}
281+
yield List.of(key.getAlgorithm(), n);
282+
}
283+
};
281284
}
282285

283286
// Check algorithm constraints with key and algorithm
@@ -301,12 +304,12 @@ private boolean checkConstraints(Set<CryptoPrimitive> primitives,
301304
}
302305

303306
// check the key algorithm
304-
if (!permits(primitives, key.getAlgorithm(), null)) {
307+
if (!permits(primitives, KeyUtil.getAlgorithm(key), null)) {
305308
return false;
306309
}
307310

308311
// If this is an elliptic curve, check if it is disabled
309-
for (String curve : getNamedCurveFromKey(key)) {
312+
for (String curve : getNamedParametersFromKey(key)) {
310313
if (!permits(primitives, curve, null)) {
311314
return false;
312315
}
@@ -460,7 +463,7 @@ private List<Constraint> getConstraints(String algorithm) {
460463

461464
// Check if KeySizeConstraints permit the specified key
462465
public boolean permits(Key key) {
463-
List<Constraint> list = getConstraints(key.getAlgorithm());
466+
List<Constraint> list = getConstraints(KeyUtil.getAlgorithm(key));
464467
if (list == null) {
465468
return true;
466469
}
@@ -514,7 +517,7 @@ public void permits(String algorithm, ConstraintsParameters cp,
514517

515518
if (checkKey) {
516519
for (Key key : cp.getKeys()) {
517-
algorithms.add(key.getAlgorithm());
520+
algorithms.add(KeyUtil.getAlgorithm(key));
518521
}
519522
}
520523

src/java.base/share/classes/sun/security/util/KeyUtil.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,22 @@ public static final int getKeySize(AlgorithmParameters parameters) {
174174
return -1;
175175
}
176176

177+
/**
178+
* If the key is a sub-algorithm of a larger group of algorithms, this
179+
* method will return that sub-algorithm. For example, key.getAlgorithm()
180+
* returns "EdDSA", but the underlying key may be "Ed448". For
181+
* DisabledAlgorithmConstraints (DAC), this distinction is important.
182+
* "EdDSA" means all curves for DAC, but when using it with
183+
* KeyPairGenerator, "EdDSA" means "Ed25519".
184+
*/
185+
public static String getAlgorithm(Key key) {
186+
if (key instanceof AsymmetricKey ak &&
187+
ak.getParams() instanceof NamedParameterSpec nps) {
188+
return nps.getName();
189+
}
190+
return key.getAlgorithm();
191+
}
192+
177193
/**
178194
* Returns whether the key is valid or not.
179195
* <P>
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
import sun.security.util.DisabledAlgorithmConstraints;
25+
26+
import java.security.CryptoPrimitive;
27+
import java.security.KeyPairGenerator;
28+
import java.security.PrivateKey;
29+
import java.security.Security;
30+
import java.util.Arrays;
31+
import java.util.List;
32+
import java.util.Objects;
33+
import java.util.Set;
34+
35+
/*
36+
* @test
37+
* @bug 8346129
38+
* @modules java.base/sun.security.util
39+
* @summary Check DisabledAlgorithmConstraints using EdDSA & XDH against
40+
* permit(Set<CryptoPrimitive>, String, AP) and
41+
* permit(Set<CryptoPrimitive>, Key). Results will differ based
42+
* on method used. The first method only can compare the String with the
43+
* algorithms. The second can check the algorithm and NamedParameterSpec
44+
* while results in more 'false' cases.
45+
*
46+
* @run main/othervm DisabledAlgorithmPermits Ed25519
47+
* @run main/othervm DisabledAlgorithmPermits Ed448
48+
* @run main/othervm DisabledAlgorithmPermits EdDSA
49+
* @run main/othervm DisabledAlgorithmPermits X25519
50+
* @run main/othervm DisabledAlgorithmPermits X448
51+
* @run main/othervm DisabledAlgorithmPermits XDH
52+
*/
53+
54+
public class DisabledAlgorithmPermits {
55+
public static void main(String[] args) throws Exception {
56+
String algorithm = args[0];
57+
Security.setProperty("x", algorithm);
58+
59+
// Expected table are the expected results from the test
60+
List<TestCase> expected = switch (algorithm) {
61+
case "Ed25519" ->
62+
Arrays.asList(
63+
new TestCase("EdDSA", true),
64+
new TestCase("Ed25519", false),
65+
new TestCase("Ed448", true),
66+
new TestCase("X448", true),
67+
new TestCase(1,"EdDSA", false),
68+
new TestCase(1,"Ed25519", false),
69+
new TestCase(1,"Ed448", true),
70+
new TestCase(1,"X448", true));
71+
case "Ed448" ->
72+
Arrays.asList(
73+
new TestCase("EdDSA", true),
74+
new TestCase("Ed25519", true),
75+
new TestCase("Ed448", false),
76+
new TestCase("X448", true),
77+
new TestCase(1,"EdDSA", true),
78+
new TestCase(1,"Ed25519", true),
79+
new TestCase(1,"Ed448", false),
80+
new TestCase(1,"X448", true));
81+
case "EdDSA" ->
82+
Arrays.asList(
83+
new TestCase("EdDSA", false),
84+
new TestCase("Ed25519", true),
85+
new TestCase("Ed448", true),
86+
new TestCase("X448", true),
87+
new TestCase(1,"EdDSA", false),
88+
new TestCase(1,"Ed25519", false),
89+
new TestCase(1,"Ed448", false),
90+
new TestCase(1,"X448", true));
91+
case "X25519" ->
92+
Arrays.asList(
93+
new TestCase("XDH", true),
94+
new TestCase("X25519", false),
95+
new TestCase("X448", true),
96+
new TestCase("Ed448", true),
97+
new TestCase(1, "XDH", false),
98+
new TestCase(1, "X25519", false),
99+
new TestCase(1, "X448", true),
100+
new TestCase(1, "Ed448", true));
101+
case "X448" ->
102+
Arrays.asList(
103+
new TestCase("XDH", true),
104+
new TestCase("X25519", true),
105+
new TestCase("X448", false),
106+
new TestCase("Ed448", true),
107+
new TestCase(1, "XDH", true),
108+
new TestCase(1, "X25519", true),
109+
new TestCase(1, "X448", false),
110+
new TestCase(1, "Ed448", true));
111+
case "XDH" ->
112+
Arrays.asList(
113+
new TestCase("XDH", false),
114+
new TestCase("X25519", true),
115+
new TestCase("X448", true),
116+
new TestCase("Ed448", true),
117+
new TestCase(1, "XDH", false),
118+
new TestCase(1, "X25519", false),
119+
new TestCase(1, "X448", false),
120+
new TestCase(1, "Ed448", true));
121+
default -> null;
122+
};
123+
124+
Objects.requireNonNull(expected, "algorithm being tested " +
125+
algorithm + " not in expected table");
126+
System.out.println("---");
127+
var dac = new DisabledAlgorithmConstraints("x");
128+
System.out.println("disabled algorithms = " + Security.getProperty("x"));
129+
130+
// Using only testType 0, this tests that permit(Set<>, String, null)
131+
// will check only the algorithm against the disabled list
132+
expected.stream().filter(n->n.testType == 0).forEach(tc -> {
133+
boolean r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE),
134+
tc.testAlgo, null);
135+
System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), \"" +
136+
tc.testAlgo + "\", null): " + r + " : " );
137+
if (r != tc.expected) {
138+
System.out.println("failed.");
139+
throw new AssertionError("failed. Expected " +
140+
tc.expected);
141+
}
142+
System.out.println("pass");
143+
});
144+
145+
// Using only testType 1, this tests permit(Set<>, Key) that will look
146+
// at both the key.getAlgorithm() and the key.getParams().getName()
147+
// against the disabled list
148+
expected.stream().filter(n->n.testType == 1).forEach(tc -> {
149+
PrivateKey k;
150+
try {
151+
k = KeyPairGenerator.getInstance(tc.testAlgo).generateKeyPair().
152+
getPrivate();
153+
} catch (Exception e) {
154+
throw new RuntimeException(e);
155+
}
156+
boolean r = dac.permits(Set.of(CryptoPrimitive.SIGNATURE), k);
157+
System.out.print("\tpermits(Set.of(CryptoPrimitive.SIGNATURE), " +
158+
tc.testAlgo + " privkey): " + r + " : " );
159+
if (r != tc.expected) {
160+
System.out.println("failed.");
161+
throw new AssertionError("failed. Expected " +
162+
tc.expected);
163+
}
164+
System.out.println("pass");
165+
});
166+
System.out.println("---");
167+
}
168+
169+
record TestCase(int testType, String testAlgo, boolean expected) {
170+
TestCase(String testAlgo, boolean expected) {
171+
this(0, testAlgo, expected);
172+
}
173+
}
174+
}
175+

0 commit comments

Comments
 (0)