diff --git a/src/java.base/share/classes/javax/crypto/Cipher.java b/src/java.base/share/classes/javax/crypto/Cipher.java index e729a984811b6..22dc66127e2d6 100644 --- a/src/java.base/share/classes/javax/crypto/Cipher.java +++ b/src/java.base/share/classes/javax/crypto/Cipher.java @@ -316,19 +316,22 @@ private Cipher(CipherSpi firstSpi, Service firstService, private static final String SHA512TRUNCATED = "SHA512/2"; + // Parse the specified cipher transformation for algorithm and the + // optional mode and padding. If the transformation contains only + // algorithm, then only algorithm is returned. Otherwise, the + // transformation must contain all 3 and they must be non-empty. private static String[] tokenizeTransformation(String transformation) throws NoSuchAlgorithmException { if (transformation == null) { throw new NoSuchAlgorithmException("No transformation given"); } /* - * array containing the components of a cipher transformation: + * Components of a cipher transformation: * - * index 0: algorithm component (e.g., AES) - * index 1: feedback component (e.g., CFB) - * index 2: padding component (e.g., PKCS5Padding) + * 1) algorithm component (e.g., AES) + * 2) feedback component (e.g., CFB) - optional + * 3) padding component (e.g., PKCS5Padding) - optional */ - String[] parts = { "", "", "" }; // check if the transformation contains algorithms with "/" in their // name which can cause the parsing logic to go wrong @@ -337,27 +340,35 @@ private static String[] tokenizeTransformation(String transformation) int startIdx = (sha512Idx == -1 ? 0 : sha512Idx + SHA512TRUNCATED.length()); int endIdx = transformation.indexOf('/', startIdx); - if (endIdx == -1) { - // algorithm - parts[0] = transformation.trim(); + + 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); + } + if (algorithmOnly) { // done + return new String[] { algo }; } else { - // algorithm/mode/padding - parts[0] = transformation.substring(0, endIdx).trim(); + // continue parsing mode and padding startIdx = endIdx+1; endIdx = transformation.indexOf('/', startIdx); if (endIdx == -1) { throw new NoSuchAlgorithmException("Invalid transformation" + " format:" + transformation); } - parts[1] = transformation.substring(startIdx, endIdx).trim(); - parts[2] = transformation.substring(endIdx+1).trim(); - } - if (parts[0].isEmpty()) { - throw new NoSuchAlgorithmException("Invalid transformation: " + - "algorithm not specified-" + 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 }; } - return parts; } // Provider attribute name for supported chaining mode @@ -453,28 +464,17 @@ private static List getTransforms(String transformation) throws NoSuchAlgorithmException { String[] parts = tokenizeTransformation(transformation); - String alg = parts[0]; - String mode = (parts[1].length() == 0 ? null : parts[1]); - String pad = (parts[2].length() == 0 ? null : parts[2]); - - if ((mode == null) && (pad == null)) { + if (parts.length == 1) { // Algorithm only - Transform tr = new Transform(alg, "", null, null); - return Collections.singletonList(tr); + return List.of(new Transform(parts[0], "", null, null)); } else { - // Algorithm w/ at least mode or padding or both - List list = new ArrayList<>(4); - if ((mode != null) && (pad != null)) { - list.add(new Transform(alg, "/" + mode + "/" + pad, null, null)); - } - if (mode != null) { - list.add(new Transform(alg, "/" + mode, null, pad)); - } - if (pad != null) { - list.add(new Transform(alg, "//" + pad, mode, null)); - } - list.add(new Transform(alg, "", mode, pad)); - return list; + // Algorithm w/ both mode and padding + return List.of( + new Transform(parts[0], "/" + parts[1] + "/" + parts[2], + null, null), + new Transform(parts[0], "/" + parts[1], null, parts[2]), + new Transform(parts[0], "//" + parts[2], parts[1], null), + new Transform(parts[0], "", parts[1], parts[2])); } } diff --git a/test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java b/test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java index bd36e7f3ca708..56e35f2a72aec 100644 --- a/test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java +++ b/test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java @@ -24,33 +24,63 @@ /* * @test - * @bug 8358159 - * @summary test that the Cipher.getInstance() handles - * transformations with empty mode and/or padding - * @run main TestEmptyModePadding + * @bug 8359388 + * @summary test that the Cipher.getInstance() would reject improper + * transformations with empty mode and/or padding. */ - -import java.security.*; -import javax.crypto.*; +import java.security.NoSuchAlgorithmException; +import java.security.Provider; +import java.security.Security; +import javax.crypto.Cipher; public class TestEmptyModePadding { public static void main(String[] args) throws Exception { - Provider provider = Security.getProvider(System.getProperty("test.provider.name", "SunJCE")); + Provider provider = Security.getProvider( + System.getProperty("test.provider.name", "SunJCE")); + + System.out.println("Testing against " + provider.getName()); - test("AES", provider); - test("AES/ECB/PKCS5Padding", provider); - test("AES//PKCS5Padding", provider); // Empty mode - test("AES/CBC/", provider); // Empty padding - test("AES/ /NoPadding", provider); // Mode is a space - test("AES/CBC/ ", provider); // Padding is a space - test("AES/ / ", provider); // Both mode and padding are spaces - test("AES//", provider); // Both mode and padding are missing + String[] testTransformations = { + // transformations w/ only 1 component, i.e. algo + " ", + // transformations w/ only 2 components + "AES/", + "AES/ ", + "AES/CBC", + "PBEWithHmacSHA512/224AndAES_128/", + "PBEWithHmacSHA512/256AndAES_128/ ", + "PBEWithHmacSHA512/224AndAES_128/CBC", + // 3-component transformations w/ empty component(s) + "AES//", + "AES/ /", + "AES// ", + "AES/ / ", + "AES/CBC/", "AES/CBC/ ", + "AES//PKCS5Padding", "AES/ /NoPadding", + "PBEWithHmacSHA512/224AndAES_128//", + "PBEWithHmacSHA512/224AndAES_128/ /", + "PBEWithHmacSHA512/224AndAES_128// ", + "PBEWithHmacSHA512/224AndAES_128/ / ", + "PBEWithHmacSHA512/256AndAES_128/CBC/", + "PBEWithHmacSHA512/256AndAES_128/CBC/ ", + "PBEWithHmacSHA512/256AndAES_128//PKCS5Padding", + "PBEWithHmacSHA512/256AndAES_128/ /PKCS5Padding", + }; + for (String t : testTransformations) { + test(t, provider); + } } - private static void test(String transformation, Provider provider) throws Exception { - Cipher c = Cipher.getInstance(transformation, provider); + private static void test(String t, Provider p) throws Exception { + try { + Cipher c = Cipher.getInstance(t, p); + throw new RuntimeException("Should throw NSAE for \'" + t + "\'"); + } catch (NoSuchAlgorithmException nsae) { + // transformation info is already in the NSAE message + System.out.println("Expected NSAE: " + nsae.getMessage()); + } } }