8183107: PKCS11 regression regarding checkKeySize

Changed key size check in PKCS11 provider to only enforce positive return values

Reviewed-by: phh, andrew
Backport-of: 67ca52873f088a08705baeef3e66319de1600576

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2008, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2019, 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
@@ -119,11 +119,13 @@ static int checkKeySize(long keyGenMech, int keySize, Token token)
                 // RC4 which is in bits. However, some PKCS#11 impls still use
                 // bytes for all mechs, e.g. NSS. We try to detect this
                 // inconsistency if the minKeySize seems unreasonably small.
-                int minKeySize = (int)info.ulMinKeySize;
-                int maxKeySize = (int)info.ulMaxKeySize;
+                int minKeySize = info.iMinKeySize;
+                int maxKeySize = info.iMaxKeySize;
                 if (keyGenMech != CKM_RC4_KEY_GEN || minKeySize < 8) {
-                    minKeySize = (int)info.ulMinKeySize << 3;
-                    maxKeySize = (int)info.ulMaxKeySize << 3;
+                    minKeySize = Math.multiplyExact(minKeySize, 8);
+                    if (maxKeySize != Integer.MAX_VALUE) {
+                        maxKeySize = Math.multiplyExact(maxKeySize, 8);
+                    }
                 }
                 // Explicitly disallow keys shorter than 40-bits for security
                 if (minKeySize < 40) minKeySize = 40;

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2019, 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
@@ -73,7 +73,7 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi {
     private BigInteger rsaPublicExponent = RSAKeyGenParameterSpec.F4;
 
     // the supported keysize range of the native PKCS11 library
-    // if the value cannot be retrieved or unspecified, -1 is used.
+    // if mechanism info is unavailable, 0/Integer.MAX_VALUE is used
     private final int minKeySize;
     private final int maxKeySize;
 
@@ -83,13 +83,13 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi {
     P11KeyPairGenerator(Token token, String algorithm, long mechanism)
             throws PKCS11Exception {
         super();
-        int minKeyLen = -1;
-        int maxKeyLen = -1;
+        int minKeyLen = 0;
+        int maxKeyLen = Integer.MAX_VALUE;
         try {
             CK_MECHANISM_INFO mechInfo = token.getMechanismInfo(mechanism);
             if (mechInfo != null) {
-                minKeyLen = (int) mechInfo.ulMinKeySize;
-                maxKeyLen = (int) mechInfo.ulMaxKeySize;
+                minKeyLen = mechInfo.iMinKeySize;
+                maxKeyLen = mechInfo.iMaxKeySize;
             }
         } catch (PKCS11Exception p11e) {
             // Should never happen
@@ -101,35 +101,33 @@ final class P11KeyPairGenerator extends KeyPairGeneratorSpi {
         // override upper limit to deter DOS attack
         if (algorithm.equals("EC")) {
             keySize = DEF_EC_KEY_SIZE;
-            if ((minKeyLen == -1) || (minKeyLen < 112)) {
+            if (minKeyLen < 112) {
                 minKeyLen = 112;
             }
-            if ((maxKeyLen == -1) || (maxKeyLen > 2048)) {
+            if (maxKeyLen > 2048) {
                 maxKeyLen = 2048;
             }
         } else {
             if (algorithm.equals("DSA")) {
                 keySize = DEF_DSA_KEY_SIZE;
             } else if (algorithm.equals("RSA")) {
                 keySize = DEF_RSA_KEY_SIZE;
+                if (maxKeyLen > 64 * 1024) {
+                    maxKeyLen = 64 * 1024;
+                }
             } else {
                 keySize = DEF_DH_KEY_SIZE;
             }
-            if ((minKeyLen == -1) || (minKeyLen < 512)) {
+            if (minKeyLen < 512) {
                 minKeyLen = 512;
             }
-            if (algorithm.equals("RSA")) {
-                if ((maxKeyLen == -1) || (maxKeyLen > 64 * 1024)) {
-                    maxKeyLen = 64 * 1024;
-                }
-            }
         }
 
         // auto-adjust default keysize in case it's out-of-range
-        if ((minKeyLen != -1) && (keySize < minKeyLen)) {
+        if (keySize < minKeyLen) {
             keySize = minKeyLen;
         }
-        if ((maxKeyLen != -1) && (keySize > maxKeyLen)) {
+        if (keySize > maxKeyLen) {
             keySize = maxKeyLen;
         }
         this.token = token;
@@ -233,13 +231,17 @@ public void initialize(AlgorithmParameterSpec params, SecureRandom random)
 
     private void checkKeySize(int keySize, AlgorithmParameterSpec params)
         throws InvalidAlgorithmParameterException {
+        if (keySize <= 0) {
+            throw new InvalidAlgorithmParameterException
+                    ("key size must be positive, got " + keySize);
+        }
         // check native range first
-        if ((minKeySize != -1) && (keySize < minKeySize)) {
+        if (keySize < minKeySize) {
             throw new InvalidAlgorithmParameterException(algorithm +
                 " key must be at least " + minKeySize + " bits. " +
                 "The specific key size " + keySize + " is not supported");
         }
-        if ((maxKeySize != -1) && (keySize > maxKeySize)) {
+        if (keySize > maxKeySize) {
             throw new InvalidAlgorithmParameterException(algorithm +
                 " key must be at most " + maxKeySize + " bits. " +
                 "The specific key size " + keySize + " is not supported");
@@ -272,12 +274,8 @@ private void checkKeySize(int keySize, AlgorithmParameterSpec params)
                         ((RSAKeyGenParameterSpec)params).getPublicExponent();
                 }
                 try {
-                    // Reuse the checking in SunRsaSign provider.
-                    // If maxKeySize is -1, then replace it with
-                    // Integer.MAX_VALUE to indicate no limit.
                     RSAKeyFactory.checkKeyLengths(keySize, tmpExponent,
-                        minKeySize,
-                        (maxKeySize==-1? Integer.MAX_VALUE:maxKeySize));
+                        minKeySize, maxKeySize);
                 } catch (InvalidKeyException e) {
                     throw new InvalidAlgorithmParameterException(e);
                 }

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

@@ -370,8 +370,9 @@ private void checkKeySize(String keyAlgo, Key key)
             // skip the check if no native info available
             return;
         }
-        int minKeySize = (int) mechInfo.ulMinKeySize;
-        int maxKeySize = (int) mechInfo.ulMaxKeySize;
+        int minKeySize = mechInfo.iMinKeySize;
+        int maxKeySize = mechInfo.iMaxKeySize;
+
         // need to override the MAX keysize for SHA1withDSA
         if (md != null && mechanism == CKM_DSA && maxKeySize > 1024) {
                maxKeySize = 1024;
@@ -395,11 +396,11 @@ private void checkKeySize(String keyAlgo, Key key)
                     " key must be the right type", cce);
             }
         }
-        if ((minKeySize != -1) && (keySize < minKeySize)) {
+        if (keySize < minKeySize) {
             throw new InvalidKeyException(keyAlgo +
                 " key must be at least " + minKeySize + " bits");
         }
-        if ((maxKeySize != -1) && (keySize > maxKeySize)) {
+        if (keySize > maxKeySize) {
             throw new InvalidKeyException(keyAlgo +
                 " key must be at most " + maxKeySize + " bits");
         }

jdk/src/share/classes/sun/security/pkcs11/wrapper/CK_MECHANISM_INFO.java

@@ -1,3 +1,27 @@
+/*
+ * Copyright (c) 2019, 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
 /*
  * reserved comment block
  * DO NOT REMOVE OR ALTER!
@@ -47,7 +71,7 @@
 
 package sun.security.pkcs11.wrapper;
 
-
+import java.security.ProviderException;
 
 /**
  * class CK_MECHANISM_INFO provides information about a particular mechanism.
@@ -74,6 +98,10 @@ public class CK_MECHANISM_INFO {
      */
     public long ulMinKeySize;
 
+    // the integer version of ulMinKeySize for doing the actual range
+    // check in SunPKCS11 provider, defaults to 0
+    public final int iMinKeySize;
+
     /**
      * <B>PKCS#11:</B>
      * <PRE>
@@ -82,6 +110,10 @@ public class CK_MECHANISM_INFO {
      */
     public long ulMaxKeySize;
 
+    // the integer version of ulMaxKeySize for doing the actual range
+    // check in SunPKCS11 provider, defaults to Integer.MAX_VALUE
+    public final int iMaxKeySize;
+
     /**
      * <B>PKCS#11:</B>
      * <PRE>
@@ -94,6 +126,10 @@ public CK_MECHANISM_INFO(long minKeySize, long maxKeySize,
                              long flags) {
         this.ulMinKeySize = minKeySize;
         this.ulMaxKeySize = maxKeySize;
+        this.iMinKeySize = ((minKeySize < Integer.MAX_VALUE && minKeySize > 0)?
+                (int)minKeySize : 0);
+        this.iMaxKeySize = ((maxKeySize < Integer.MAX_VALUE && maxKeySize > 0)?
+                (int)maxKeySize : Integer.MAX_VALUE);
         this.flags = flags;
     }