From 4d8a8e20c5a5a46fec43d7e2c8bfb03a27e79425 Mon Sep 17 00:00:00 2001 From: Alexey Bakhtin Date: Fri, 15 Dec 2023 20:08:18 +0000 Subject: [PATCH] 8302017: Allocate BadPaddingException only if it will be thrown Reviewed-by: mbalao, andrew Backport-of: 334b977259930368160db705c1f2feda0b0e8707 --- .../com/sun/crypto/provider/RSACipher.java | 27 +++++-- .../sun/security/pkcs11/P11Signature.java | 11 +-- .../classes/sun/security/rsa/RSAPadding.java | 72 ++++++++----------- .../sun/security/rsa/RSASignature.java | 26 +++---- .../sun/security/rsa/RSAPaddingCheck.java | 63 ++++++++++++++++ 5 files changed, 137 insertions(+), 62 deletions(-) create mode 100644 jdk/test/sun/security/rsa/RSAPaddingCheck.java diff --git a/jdk/src/share/classes/com/sun/crypto/provider/RSACipher.java b/jdk/src/share/classes/com/sun/crypto/provider/RSACipher.java index 9c57bb0c06..e6921467b6 100644 --- a/jdk/src/share/classes/com/sun/crypto/provider/RSACipher.java +++ b/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 @@ public final class RSACipher extends CipherSpi { 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 @@ public final class RSACipher extends CipherSpi { } 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); } } diff --git a/jdk/src/share/classes/sun/security/pkcs11/P11Signature.java b/jdk/src/share/classes/sun/security/pkcs11/P11Signature.java index 2841ae82eb..7260c0bebf 100644 --- a/jdk/src/share/classes/sun/security/pkcs11/P11Signature.java +++ b/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 @@ final class P11Signature extends SignatureSpi { 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); } } diff --git a/jdk/src/share/classes/sun/security/rsa/RSAPadding.java b/jdk/src/share/classes/sun/security/rsa/RSAPadding.java index 908cefadec..d0a00c5a90 100644 --- a/jdk/src/share/classes/sun/security/rsa/RSAPadding.java +++ b/jdk/src/share/classes/sun/security/rsa/RSAPadding.java @@ -30,7 +30,6 @@ import java.util.*; 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 final class RSAPadding { } /** - * 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 final class RSAPadding { } /** - * 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 @@ public final class RSAPadding { /** * 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 @@ public final class RSAPadding { 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 @@ public final class RSAPadding { /** * 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 @@ public final class RSAPadding { /** * 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 @@ public final class RSAPadding { 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); } } diff --git a/jdk/src/share/classes/sun/security/rsa/RSASignature.java b/jdk/src/share/classes/sun/security/rsa/RSASignature.java index 0e719e806e..ccd0f458f5 100644 --- a/jdk/src/share/classes/sun/security/rsa/RSASignature.java +++ b/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 @@ public abstract class RSASignature extends SignatureSpi { 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 @@ public abstract class RSASignature extends SignatureSpi { } 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); diff --git a/jdk/test/sun/security/rsa/RSAPaddingCheck.java b/jdk/test/sun/security/rsa/RSAPaddingCheck.java new file mode 100644 index 0000000000..807e4e3bf4 --- /dev/null +++ b/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!"); + } + } + } +}