8302225: SunJCE Provider doesn't validate key sizes when using 'constrained' transforms for AES/KW and AES/KWP

Reviewed-by: xuelei

src/java.base/share/classes/com/sun/crypto/provider/KeyWrapCipher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 2023, 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
@@ -123,6 +123,25 @@ abstract class KeyWrapCipher extends CipherSpi {
         }
     }
 
+    // validate the key algorithm/encoding and then returns the key bytes
+    // which callers should erase after use
+    private static byte[] checkKey(Key key, int fixedKeySize)
+            throws InvalidKeyException {
+
+        byte[] keyBytes = key.getEncoded();
+        if (keyBytes == null) {
+            throw new InvalidKeyException("Null key");
+        }
+        int keyLen = keyBytes.length;
+        if (!key.getAlgorithm().equalsIgnoreCase("AES") ||
+            !AESCrypt.isKeySizeValid(keyLen) ||
+            (fixedKeySize != -1 && fixedKeySize != keyLen)) {
+                throw new InvalidKeyException("Invalid key length: " +
+                        keyLen + " bytes");
+        }
+        return keyBytes;
+    }
+
     // store the specified bytes, e.g. in[inOfs...(inOfs+inLen-1)] into
     // 'dataBuf' starting at 'dataIdx'.
     // NOTE: if 'in' is null, this method will ensure that 'dataBuf' has enough
@@ -292,10 +311,8 @@ abstract class KeyWrapCipher extends CipherSpi {
     // actual impl for various engineInit(...) methods
     private void implInit(int opmode, Key key, byte[] iv, SecureRandom random)
             throws InvalidKeyException, InvalidAlgorithmParameterException {
-        byte[] keyBytes = key.getEncoded();
+        byte[] keyBytes = checkKey(key, fixedKeySize);
-        if (keyBytes == null) {
+
-            throw new InvalidKeyException("Null key");
-        }
         this.opmode = opmode;
         boolean decrypting = (opmode == Cipher.DECRYPT_MODE ||
                 opmode == Cipher.UNWRAP_MODE);
@@ -656,21 +673,11 @@ abstract class KeyWrapCipher extends CipherSpi {
      * @exception InvalidKeyException if <code>key</code> is invalid.
      */
     protected int engineGetKeySize(Key key) throws InvalidKeyException {
-        byte[] encoded = key.getEncoded();
+        byte[] keyBytes = checkKey(key, fixedKeySize);
-        if (encoded == null)  {
+        // only need length; erase immediately
-            throw new InvalidKeyException("Cannot decide key length");
+        Arrays.fill(keyBytes, (byte) 0);
-        }
+        return Math.multiplyExact(keyBytes.length, 8);
 
-        // only need length
-        Arrays.fill(encoded, (byte) 0);
-        int keyLen = encoded.length;
-        if (!key.getAlgorithm().equalsIgnoreCase("AES") ||
-            !AESCrypt.isKeySizeValid(keyLen) ||
-            (fixedKeySize != -1 && fixedKeySize != keyLen)) {
-            throw new InvalidKeyException("Invalid key length: " +
-                    keyLen + " bytes");
-        }
-        return Math.multiplyExact(keyLen, 8);
     }
 
     /**

test/jdk/com/sun/crypto/provider/Cipher/KeyWrap/TestKeySizeCheck.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2023, 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,7 +23,7 @@
 
 /*
  * @test
- * @bug 8248268
+ * @bug 8248268 8302225
  * @summary Verify cipher key size restriction is enforced properly with IKE
  * @run main TestKeySizeCheck
  */
@@ -43,6 +43,8 @@ public class TestKeySizeCheck {
         }
     }
 
+    private static final int[] AES_KEYSIZES = { 128, 192, 256 };
+
     private static SecretKey getKey(int sizeInBytes) {
         if (sizeInBytes <= BYTES_32.length) {
             return new SecretKeySpec(BYTES_32, 0, sizeInBytes, "AES");
@@ -64,7 +66,7 @@ public class TestKeySizeCheck {
         int[] modes = { Cipher.ENCRYPT_MODE, Cipher.WRAP_MODE };
         for (int ks : invalidKeySizes) {
             System.out.println("keysize: " + ks);
-            SecretKey key = getKey(ks);
+            SecretKey key = getKey(ks >> 3);
 
             for (int m : modes) {
                 try {
@@ -72,11 +74,25 @@ public class TestKeySizeCheck {
                     throw new RuntimeException("Expected IKE not thrown for "
                             + getModeStr(m));
                 } catch (InvalidKeyException ike) {
-                    System.out.println(" => expected IKE thrown for "
+                    System.out.println(getModeStr(m) + " => got expected IKE");
-                            + getModeStr(m));
                 }
             }
         }
+
+        // now test against the valid key size(s) and make sure they work
+        int underscoreIdx = algo.indexOf("_");
+        int[] validKeySizes = (algo.indexOf("_") == -1 ?
+            AES_KEYSIZES : new int[] { Integer.parseInt(algo.substring
+                    (underscoreIdx + 1, underscoreIdx + 4)) });
+        for (int ks : validKeySizes) {
+            System.out.println("keysize: " + ks);
+            SecretKey key = getKey(ks >> 3);
+
+            for (int m : modes) {
+                c.init(m, key);
+                System.out.println(getModeStr(m) + " => ok");
+            }
+        }
     }
 
     public static void main(String[] argv) throws Exception {