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

src/java.base/share/classes/javax/crypto/Cipher.java

@@ -286,7 +286,32 @@ private Cipher(CipherSpi firstSpi, Service firstService,
         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 @@ private static String[] tokenizeTransformation(String transformation)
          * 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);
-        }
-        if (algorithmOnly) { // done
-            return new String[] { algo };
+        int endIdx = indexOfRealSlash(transformation, 0);
+        if (endIdx == -1) { // algo only, done
+            return new String[] { reqNonEmpty(transformation,
+                        "Invalid transformation: algorithm not specified")
+            };
+        }
+        // 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);
         }
     }
 

test/jdk/com/sun/crypto/provider/Cipher/ChaCha20/unittest/ChaCha20CipherUnitTest.java

@@ -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 @@ private static void testTransformations() throws Exception {
 
         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);
     }

test/jdk/javax/crypto/Cipher/TestEmptyModePadding.java

@@ -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 static void main(String[] args) throws Exception {
             "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) {

test/jdk/javax/crypto/Cipher/TestGetInstance.java

@@ -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.util.Locale;
 
 import javax.crypto.*;
+import jdk.test.lib.Utils;
 
 public class TestGetInstance {
 
@@ -48,8 +50,8 @@ public static void main(String[] args) throws Exception {
         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,87 +70,55 @@ public static void main(String[] args) throws Exception {
         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",
+        };
 
-        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);
+        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("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);
-        }
+        // 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"),
+        };
 
-        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);
+        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);
         }
 
+        // 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");
     }
 }