8368984: Extra slashes in Cipher transformation leads to NSPE instead of NSAE

Reviewed-by: weijun
This commit is contained in:
Valerie Peng 2025-10-08 17:36:03 +00:00
parent 79bcc7b8ec
commit ac73e688b1
4 changed files with 111 additions and 119 deletions

View File

@ -286,7 +286,32 @@ public class Cipher {
this.lock = new Object();
}
private static final String SHA512TRUNCATED = "SHA512/2";
// for special handling SHA-512/224, SHA-512/256, SHA512/224, SHA512/256
static int indexOfRealSlash(String s, int fromIndex) {
while (true) {
int pos = s.indexOf('/', fromIndex);
// 512/2
if (pos > 3 && pos + 1 < s.length()
&& s.charAt(pos - 3) == '5'
&& s.charAt(pos - 2) == '1'
&& s.charAt(pos - 1) == '2'
&& s.charAt(pos + 1) == '2') {
fromIndex = pos + 1;
// see 512/2, find next
} else {
return pos;
}
}
}
static String reqNonEmpty(String in, String msg)
throws NoSuchAlgorithmException {
in = in.trim();
if (in.isEmpty()) {
throw new NoSuchAlgorithmException(msg);
}
return in;
}
// Parse the specified cipher transformation for algorithm and the
// optional mode and padding. If the transformation contains only
@ -305,42 +330,34 @@ public class Cipher {
* 2) feedback component (e.g., CFB) - optional
* 3) padding component (e.g., PKCS5Padding) - optional
*/
// check if the transformation contains algorithms with "/" in their
// name which can cause the parsing logic to go wrong
int sha512Idx = transformation.toUpperCase(Locale.ENGLISH)
.indexOf(SHA512TRUNCATED);
int startIdx = (sha512Idx == -1 ? 0 :
sha512Idx + SHA512TRUNCATED.length());
int endIdx = transformation.indexOf('/', startIdx);
boolean algorithmOnly = (endIdx == -1);
String algo = (algorithmOnly ? transformation.trim() :
transformation.substring(0, endIdx).trim());
if (algo.isEmpty()) {
throw new NoSuchAlgorithmException("Invalid transformation: " +
"algorithm not specified-"
+ transformation);
int endIdx = indexOfRealSlash(transformation, 0);
if (endIdx == -1) { // algo only, done
return new String[] { reqNonEmpty(transformation,
"Invalid transformation: algorithm not specified")
};
}
if (algorithmOnly) { // done
return new String[] { algo };
// must be algo/mode/padding
String algo = reqNonEmpty(transformation.substring(0, endIdx),
"Invalid transformation: algorithm not specified");
int startIdx = endIdx + 1;
endIdx = indexOfRealSlash(transformation, startIdx);
if (endIdx == -1) {
throw new NoSuchAlgorithmException(
"Invalid transformation format: " + transformation);
}
String mode = reqNonEmpty(transformation.substring(startIdx,
endIdx), "Invalid transformation: missing mode");
startIdx = endIdx + 1;
endIdx = indexOfRealSlash(transformation, startIdx);
if (endIdx == -1) {
return new String[] { algo, mode,
reqNonEmpty(transformation.substring(startIdx),
"Invalid transformation: missing padding") };
} else {
// continue parsing mode and padding
startIdx = endIdx+1;
endIdx = transformation.indexOf('/', startIdx);
if (endIdx == -1) {
throw new NoSuchAlgorithmException("Invalid transformation"
+ " format:" + transformation);
}
String mode = transformation.substring(startIdx, endIdx).trim();
String padding = transformation.substring(endIdx+1).trim();
// ensure mode and padding are specified
if (mode.isEmpty() || padding.isEmpty()) {
throw new NoSuchAlgorithmException("Invalid transformation: " +
"missing mode and/or padding-"
+ transformation);
}
return new String[] { algo, mode, padding };
throw new NoSuchAlgorithmException(
"Invalid transformation format: " + transformation);
}
}

View File

@ -23,7 +23,7 @@
/*
* @test
* @bug 8153029 8360463
* @bug 8153029 8360463 8368984
* @library /test/lib
* @run main ChaCha20CipherUnitTest
* @summary Unit test for com.sun.crypto.provider.ChaCha20Cipher.
@ -72,10 +72,12 @@ public class ChaCha20CipherUnitTest {
checkTransformation("ChaCha20", null);
checkTransformation("ChaCha20/None/NoPadding", null);
checkTransformation("ChaCha20-Poly1305", null);
checkTransformation("ChaCha20-Poly1305/None/NoPadding", null);
checkTransformation("ChaCha20/None//", NSAE);
checkTransformation("ChaCha20/ECB/NoPadding", NSAE);
checkTransformation("ChaCha20/None/PKCS5Padding", NSPE);
checkTransformation("ChaCha20-Poly1305", null);
checkTransformation("ChaCha20-Poly1305/None/NoPadding", null);
checkTransformation("ChaCha20-Poly1305/None//", NSAE);
checkTransformation("ChaCha20-Poly1305/ECB/NoPadding", NSAE);
checkTransformation("ChaCha20-Poly1305/None/PKCS5Padding", NSPE);
}

View File

@ -24,7 +24,7 @@
/*
* @test
* @bug 8359388
* @bug 8359388 8368984
* @summary test that the Cipher.getInstance() would reject improper
* transformations with empty mode and/or padding.
*/
@ -67,6 +67,9 @@ public class TestEmptyModePadding {
"PBEWithHmacSHA512/256AndAES_128/CBC/ ",
"PBEWithHmacSHA512/256AndAES_128//PKCS5Padding",
"PBEWithHmacSHA512/256AndAES_128/ /PKCS5Padding",
// 4 or more components transformations
"PBEWithHmacSHA512///PKCS5Padding",
"AES/GCM/NoPadding///",
};
for (String t : testTransformations) {

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2024, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 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
@ -23,9 +23,10 @@
/*
* @test
* @bug 4898428
* @bug 4898428 8368984
* @summary test that the new getInstance() implementation works correctly
* @author Andreas Sterbenz
* @library /test/lib
* @run main TestGetInstance DES PBEWithMD5AndTripleDES
* @run main TestGetInstance AES PBEWithHmacSHA1AndAES_128
*/
@ -35,6 +36,7 @@ import java.security.spec.*;
import java.util.Locale;
import javax.crypto.*;
import jdk.test.lib.Utils;
public class TestGetInstance {
@ -48,8 +50,8 @@ public class TestGetInstance {
String algo = args[0];
String algoLC = algo.toLowerCase(Locale.ROOT);
String pbeAlgo = args[1];
Provider p = Security.getProvider(
System.getProperty("test.provider.name", "SunJCE"));
String pName = System.getProperty("test.provider.name", "SunJCE");
Provider p = Security.getProvider(pName);
Cipher c;
@ -68,86 +70,54 @@ public class TestGetInstance {
c = Cipher.getInstance(algoLC + "/cbc/pkcs5padding", p);
same(p, c.getProvider());
try {
c = Cipher.getInstance(algo + "/XYZ/PKCS5Padding");
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance(algo + "/XYZ/PKCS5Padding",
System.getProperty("test.provider.name", "SunJCE"));
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance(algo + "/XYZ/PKCS5Padding", p);
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
// invalid transformations or transformations containing unsupported
// modes which should lead to NSAE
String[] nsaeTransformations = {
(algo + "/XYZ/PKCS5Padding"),
(algo + "/CBC/XYZWithSHA512/224Padding/"),
(algo + "/CBC/XYZWithSHA512/256Padding/"),
(pbeAlgo + "/CBC/XYZWithSHA512/224Padding/"),
(pbeAlgo + "/CBC/XYZWithSHA512/256Padding/"),
"foo",
};
for (String t : nsaeTransformations) {
System.out.println("Testing NSAE on " + t);
Utils.runAndCheckException(() -> Cipher.getInstance(t),
NoSuchAlgorithmException.class);
Utils.runAndCheckException(() -> Cipher.getInstance(t, pName),
NoSuchAlgorithmException.class);
Utils.runAndCheckException(() -> Cipher.getInstance(t, p),
NoSuchAlgorithmException.class);
}
try {
c = Cipher.getInstance(algo + "/CBC/XYZPadding");
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance(algo + "/CBC/XYZPadding",
System.getProperty("test.provider.name", "SunJCE"));
throw new AssertionError();
} catch (NoSuchPaddingException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance(algo + "/CBC/XYZPadding", p);
throw new AssertionError();
} catch (NoSuchPaddingException e) {
System.out.println(e);
// transformations containing unsupported paddings for SunJCE provider
// which should lead to NSPE
String[] nspeTransformations = {
(algo + "/CBC/XYZPadding"),
(algo + "/CBC/XYZWithSHA512/224Padding"),
(algo + "/CBC/XYZWithSHA512/256Padding"),
(pbeAlgo + "/CBC/XYZWithSHA512/224Padding"),
(pbeAlgo + "/CBC/XYZWithSHA512/256Padding"),
};
for (String t : nspeTransformations) {
System.out.println("Testing NSPE on " + t);
Utils.runAndCheckException(() -> Cipher.getInstance(t, pName),
NoSuchPaddingException.class);
Utils.runAndCheckException(() -> Cipher.getInstance(t, p),
NoSuchPaddingException.class);
}
try {
c = Cipher.getInstance("foo");
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance("foo",
System.getProperty("test.provider.name", "SunJCE"));
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance("foo", p);
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance("foo",
System.getProperty("test.provider.name", "SUN"));
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance("foo", Security.getProvider(
System.getProperty("test.provider.name", "SUN")));
throw new AssertionError();
} catch (NoSuchAlgorithmException e) {
System.out.println(e);
}
try {
c = Cipher.getInstance("foo", "bar");
throw new AssertionError();
} catch (NoSuchProviderException e) {
System.out.println(e);
}
// additional misc tests
Utils.runAndCheckException(() -> Cipher.getInstance("foo",
System.getProperty("test.provider.name", "SUN")),
NoSuchAlgorithmException.class);
Utils.runAndCheckException(() -> Cipher.getInstance("foo",
Security.getProvider(System.getProperty("test.provider.name",
"SUN"))), NoSuchAlgorithmException.class);
Utils.runAndCheckException(() -> Cipher.getInstance("foo", "bar"),
NoSuchProviderException.class);
System.out.println("All Tests ok");
}