8302017: Allocate BadPaddingException only if it will be thrown

Reviewed-by: mbalao, andrew
Backport-of: 334b977259930368160db705c1f2feda0b0e8707

Author: Alexey Bakhtin

jdk/src/share/classes/com/sun/crypto/provider/RSACipher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -353,21 +353,38 @@ private byte[] doFinal() throws BadPaddingException,
             switch (mode) {
             case MODE_SIGN:
                 paddingCopy = padding.pad(buffer, 0, bufOfs);
-                result = RSACore.rsa(paddingCopy, privateKey, true);
+                if (paddingCopy != null) {
+                    result = RSACore.rsa(paddingCopy, privateKey, true);
+                } else {
+                    throw new BadPaddingException("Padding error in signing");
+                }
                 break;
             case MODE_VERIFY:
                 byte[] verifyBuffer = RSACore.convert(buffer, 0, bufOfs);
                 paddingCopy = RSACore.rsa(verifyBuffer, publicKey);
                 result = padding.unpad(paddingCopy);
+                if (result == null) {
+                    throw new BadPaddingException
+                            ("Padding error in verification");
+                }
                 break;
             case MODE_ENCRYPT:
                 paddingCopy = padding.pad(buffer, 0, bufOfs);
-                result = RSACore.rsa(paddingCopy, publicKey);
+                if (paddingCopy != null) {
+                    result = RSACore.rsa(paddingCopy, publicKey);
+                } else {
+                    throw new BadPaddingException
+                            ("Padding error in encryption");
+                }
                 break;
             case MODE_DECRYPT:
                 byte[] decryptBuffer = RSACore.convert(buffer, 0, bufOfs);
                 paddingCopy = RSACore.rsa(decryptBuffer, privateKey, false);
                 result = padding.unpad(paddingCopy);
+                if (result == null) {
+                    throw new BadPaddingException
+                            ("Padding error in decryption");
+                }
                 break;
             default:
                 throw new AssertionError("Internal error");
@@ -376,9 +393,9 @@ private byte[] doFinal() throws BadPaddingException,
         } finally {
             Arrays.fill(buffer, 0, bufOfs, (byte)0);
             bufOfs = 0;
-            if (paddingCopy != null             // will not happen
+            if (paddingCopy != null
                     && paddingCopy != buffer    // already cleaned
-                    && paddingCopy != result) { // DO NOT CLEAN, THIS IS RESULT!
+                    && paddingCopy != result) { // DO NOT CLEAN, THIS IS RESULT
                 Arrays.fill(paddingCopy, (byte)0);
             }
         }

jdk/src/share/classes/sun/security/pkcs11/P11Signature.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -728,9 +728,12 @@ private byte[] pkcs1Pad(byte[] data) {
             int len = (p11Key.length() + 7) >> 3;
             RSAPadding padding = RSAPadding.getInstance
                                         (RSAPadding.PAD_BLOCKTYPE_1, len);
-            byte[] padded = padding.pad(data);
-            return padded;
-        } catch (GeneralSecurityException e) {
+            byte[] result = padding.pad(data);
+            if (result == null) {
+                throw new ProviderException("Error padding data");
+            }
+            return result;
+        } catch (InvalidKeyException | InvalidAlgorithmParameterException e) {
             throw new ProviderException(e);
         }
     }

jdk/src/share/classes/sun/security/rsa/RSAPadding.java

@@ -30,7 +30,6 @@
 import java.security.*;
 import java.security.spec.*;
 
-import javax.crypto.BadPaddingException;
 import javax.crypto.spec.PSource;
 import javax.crypto.spec.OAEPParameterSpec;
 
@@ -236,24 +235,22 @@ public int getMaxDataSize() {
     }
 
     /**
-     * Pad the data and return the padded block.
+     * Pad the data and return the result or null if error occurred.
      */
-    public byte[] pad(byte[] data) throws BadPaddingException {
+    public byte[] pad(byte[] data) {
         return pad(data, 0, data.length);
     }
 
     /**
-     * Pad the data and return the padded block.
+     * Pad the data and return the result or null if error occurred.
      */
-    public byte[] pad(byte[] data, int ofs, int len)
-            throws BadPaddingException {
+    public byte[] pad(byte[] data, int ofs, int len) {
         if (len > maxDataSize) {
-            throw new BadPaddingException("Data must be shorter than "
-                + (maxDataSize + 1) + " bytes but received "
-                + len + " bytes.");
+            return null;
         }
         switch (type) {
         case PAD_NONE:
+            // assert len == paddedSize and data.length - ofs > len?
             return RSACore.convert(data, ofs, len);
         case PAD_BLOCKTYPE_1:
         case PAD_BLOCKTYPE_2:
@@ -266,31 +263,30 @@ public byte[] pad(byte[] data, int ofs, int len)
     }
 
     /**
-     * Unpad the padded block and return the data.
+     * Unpad the padded block and return the result or null if error occurred.
      */
-    public byte[] unpad(byte[] padded) throws BadPaddingException {
-        if (padded.length != paddedSize) {
-            throw new BadPaddingException("Decryption error." +
-                "The padded array length (" + padded.length +
-                ") is not the specified padded size (" + paddedSize + ")");
-        }
-        switch (type) {
-        case PAD_NONE:
-            return padded;
-        case PAD_BLOCKTYPE_1:
-        case PAD_BLOCKTYPE_2:
-            return unpadV15(padded);
-        case PAD_OAEP_MGF1:
-            return unpadOAEP(padded);
-        default:
-            throw new AssertionError();
+    public byte[] unpad(byte[] padded) {
+        if (padded.length == paddedSize) {
+            switch (type) {
+            case PAD_NONE:
+                return padded;
+            case PAD_BLOCKTYPE_1:
+            case PAD_BLOCKTYPE_2:
+                return unpadV15(padded);
+            case PAD_OAEP_MGF1:
+                return unpadOAEP(padded);
+            default:
+                throw new AssertionError();
+            }
+        } else {
+            return null;
         }
     }
 
     /**
      * PKCS#1 v1.5 padding (blocktype 1 and 2).
      */
-    private byte[] padV15(byte[] data, int ofs, int len) throws BadPaddingException {
+    private byte[] padV15(byte[] data, int ofs, int len) {
         byte[] padded = new byte[paddedSize];
         System.arraycopy(data, ofs, padded, paddedSize - len, len);
         int psSize = paddedSize - 3 - len;
@@ -328,10 +324,10 @@ private byte[] padV15(byte[] data, int ofs, int len) throws BadPaddingException
 
     /**
      * PKCS#1 v1.5 unpadding (blocktype 1 (signature) and 2 (encryption)).
-     *
+     * Return the result or null if error occurred.
      * Note that we want to make it a constant-time operation
      */
-    private byte[] unpadV15(byte[] padded) throws BadPaddingException {
+    private byte[] unpadV15(byte[] padded) {
         int k = 0;
         boolean bp = false;
 
@@ -367,10 +363,8 @@ private byte[] unpadV15(byte[] padded) throws BadPaddingException {
         byte[] data = new byte[n];
         System.arraycopy(padded, p, data, 0, n);
 
-        BadPaddingException bpe = new BadPaddingException("Decryption error");
-
         if (bp) {
-            throw bpe;
+            return null;
         } else {
             return data;
         }
@@ -379,8 +373,9 @@ private byte[] unpadV15(byte[] padded) throws BadPaddingException {
     /**
      * PKCS#1 v2.0 OAEP padding (MGF1).
      * Paragraph references refer to PKCS#1 v2.1 (June 14, 2002)
+     * Return the result or null if error occurred.
      */
-    private byte[] padOAEP(byte[] M, int ofs, int len) throws BadPaddingException {
+    private byte[] padOAEP(byte[] M, int ofs, int len) {
         if (random == null) {
             random = JCAUtil.getSecureRandom();
         }
@@ -429,8 +424,9 @@ private byte[] padOAEP(byte[] M, int ofs, int len) throws BadPaddingException {
 
     /**
      * PKCS#1 v2.1 OAEP unpadding (MGF1).
+     * Return the result or null if error occurred.
      */
-    private byte[] unpadOAEP(byte[] padded) throws BadPaddingException {
+    private byte[] unpadOAEP(byte[] padded) {
         byte[] EM = padded;
         boolean bp = false;
         int hLen = lHash.length;
@@ -486,12 +482,6 @@ private byte[] unpadOAEP(byte[] padded) throws BadPaddingException {
         byte [] m = new byte[EM.length - mStart];
         System.arraycopy(EM, mStart, m, 0, m.length);
 
-        BadPaddingException bpe = new BadPaddingException("Decryption error");
-
-        if (bp) {
-            throw bpe;
-        } else {
-            return m;
-        }
+        return (bp? null : m);
     }
 }

jdk/src/share/classes/sun/security/rsa/RSASignature.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -189,13 +189,15 @@ protected byte[] engineSign() throws SignatureException {
         try {
             byte[] encoded = encodeSignature(digestOID, digest);
             byte[] padded = padding.pad(encoded);
-            byte[] encrypted = RSACore.rsa(padded, privateKey, true);
-            return encrypted;
+            if (padded != null) {
+                return RSACore.rsa(padded, privateKey, true);
+            }
         } catch (GeneralSecurityException e) {
             throw new SignatureException("Could not sign data", e);
         } catch (IOException e) {
             throw new SignatureException("Could not encode data", e);
         }
+        throw new SignatureException("Could not sign data");
     }
 
     // verify the data and return the result. See JCA doc
@@ -206,21 +208,21 @@ protected boolean engineVerify(byte[] sigBytes) throws SignatureException {
         }
 
         if (sigBytes.length != RSACore.getByteLength(publicKey)) {
-            throw new SignatureException("Signature length not correct: got " +
+            throw new SignatureException("Bad signature length: got " +
                     sigBytes.length + " but was expecting " +
                     RSACore.getByteLength(publicKey));
         }
-        byte[] digest = getDigestValue();
+
         try {
+            // https://www.rfc-editor.org/rfc/rfc8017.html#section-8.2.2
+            // Step 4 suggests comparing the encoded message
             byte[] decrypted = RSACore.rsa(sigBytes, publicKey);
-            byte[] unpadded = padding.unpad(decrypted);
-            byte[] decodedDigest = decodeSignature(digestOID, unpadded);
-            return MessageDigest.isEqual(digest, decodedDigest);
+
+            byte[] digest = getDigestValue();
+            byte[] encoded = encodeSignature(digestOID, digest);
+            byte[] padded = padding.pad(encoded);
+            return MessageDigest.isEqual(padded, decrypted);
         } catch (javax.crypto.BadPaddingException e) {
-            // occurs if the app has used the wrong RSA public key
-            // or if sigBytes is invalid
-            // return false rather than propagating the exception for
-            // compatibility/ease of use
             return false;
         } catch (IOException e) {
             throw new SignatureException("Signature encoding error", e);

jdk/test/sun/security/rsa/RSAPaddingCheck.java

@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+/*
+ * @test
+ * @bug 8302017
+ * @summary Ensure that RSAPadding class works as expected after refactoring
+ * @modules java.base/sun.security.rsa
+ */
+import java.util.Arrays;
+import sun.security.rsa.RSAPadding;
+
+public class RSAPaddingCheck {
+
+    private static int[] PADDING_TYPES =  {
+        RSAPadding.PAD_BLOCKTYPE_1,
+        RSAPadding.PAD_BLOCKTYPE_2,
+        RSAPadding.PAD_NONE,
+        RSAPadding.PAD_OAEP_MGF1,
+    };
+
+    public static void main(String[] args) throws Exception {
+        int size = 2048 >> 3;
+        byte[] testData = "This is some random to-be-padded Data".getBytes();
+        for (int type : PADDING_TYPES) {
+            byte[] data = (type == RSAPadding.PAD_NONE?
+                    Arrays.copyOf(testData, size) : testData);
+            System.out.println("Testing PaddingType: " + type);
+            RSAPadding padding = RSAPadding.getInstance(type, size);
+            byte[] paddedData = padding.pad(data);
+            if (paddedData == null) {
+                throw new RuntimeException("Unexpected padding op failure!");
+            }
+
+            byte[] data2 = padding.unpad(paddedData);
+            if (data2 == null) {
+                throw new RuntimeException("Unexpected unpadding op failure!");
+            }
+            if (!Arrays.equals(data, data2)) {
+                throw new RuntimeException("diff check failure!");
+            }
+        }
+    }
+}