8231139: Improved keystore support

Reviewed-by: mbalao

10 files changed, 63 insertions, 104 deletions

diff --git a/src/share/classes/com/sun/crypto/provider/JceKeyStore.java b/src/share/classes/com/sun/crypto/provider/JceKeyStore.java
index d47f5a9c9f..d05dcacac1 100644
--- a/src/share/classes/com/sun/crypto/provider/JceKeyStore.java
+++ b/src/share/classes/com/sun/crypto/provider/JceKeyStore.java
@@ -43,6 +43,7 @@ import java.security.cert.CertificateFactory;
 import java.security.cert.CertificateException;
 import javax.crypto.SealedObject;
 
+import sun.misc.IOUtils;
 import sun.misc.ObjectInputFilter;
 
 /**
@@ -70,7 +71,7 @@ public final class JceKeyStore extends KeyStoreSpi {
     private static final class PrivateKeyEntry {
         Date date; // the creation date of this entry
         byte[] protectedKey;
-        Certificate chain[];
+        Certificate[] chain;
     };
 
     // Secret key
@@ -738,23 +739,11 @@ public final class JceKeyStore extends KeyStoreSpi {
                         entry.date = new Date(dis.readLong());
 
                         // read the private key
-                        try {
-                            entry.protectedKey = new byte[dis.readInt()];
-                        } catch (OutOfMemoryError e) {
-                            throw new IOException("Keysize too big");
-                        }
-                        dis.readFully(entry.protectedKey);
+                        entry.protectedKey = IOUtils.readExactlyNBytes(dis, dis.readInt());
 
                         // read the certificate chain
                         int numOfCerts = dis.readInt();
-                        try {
-                            if (numOfCerts > 0) {
-                                entry.chain = new Certificate[numOfCerts];
-                            }
-                        } catch (OutOfMemoryError e) {
-                            throw new IOException("Too many certificates in "
-                                                  + "chain");
-                        }
+                        List<Certificate> tmpCerts = new ArrayList<>();
                         for (int j = 0; j < numOfCerts; j++) {
                             if (xVersion == 2) {
                                 // read the certificate type, and instantiate a
@@ -762,27 +751,24 @@ public final class JceKeyStore extends KeyStoreSpi {
                                 // existing factory if possible)
                                 String certType = dis.readUTF();
                                 if (cfs.containsKey(certType)) {
-                                // reuse certificate factory
+                                    // reuse certificate factory
                                     cf = cfs.get(certType);
                                 } else {
-                                // create new certificate factory
+                                    // create new certificate factory
                                     cf = CertificateFactory.getInstance(
                                         certType);
-                                // store the certificate factory so we can
-                                // reuse it later
+                                    // store the certificate factory so we can
+                                    // reuse it later
                                     cfs.put(certType, cf);
                                 }
                             }
                             // instantiate the certificate
-                            try {
-                                encoded = new byte[dis.readInt()];
-                            } catch (OutOfMemoryError e) {
-                                throw new IOException("Certificate too big");
-                            }
-                            dis.readFully(encoded);
+                            encoded = IOUtils.readExactlyNBytes(dis, dis.readInt());
                             bais = new ByteArrayInputStream(encoded);
-                            entry.chain[j] = cf.generateCertificate(bais);
+                            tmpCerts.add(cf.generateCertificate(bais));
                         }
+                        entry.chain = tmpCerts.toArray(
+                                new Certificate[numOfCerts]);
 
                         // Add the entry to the list
                         entries.put(alias, entry);
@@ -814,12 +800,7 @@ public final class JceKeyStore extends KeyStoreSpi {
                                 cfs.put(certType, cf);
                             }
                         }
-                        try {
-                            encoded = new byte[dis.readInt()];
-                        } catch (OutOfMemoryError e) {
-                            throw new IOException("Certificate too big");
-                        }
-                        dis.readFully(encoded);
+                        encoded = IOUtils.readExactlyNBytes(dis, dis.readInt());
                         bais = new ByteArrayInputStream(encoded);
                         entry.cert = cf.generateCertificate(bais);
 
@@ -870,18 +851,14 @@ public final class JceKeyStore extends KeyStoreSpi {
                  * with
                  */
                 if (password != null) {
-                    byte computed[], actual[];
-                    computed = md.digest();
-                    actual = new byte[computed.length];
-                    dis.readFully(actual);
-                    for (int i = 0; i < computed.length; i++) {
-                        if (computed[i] != actual[i]) {
-                            throw new IOException(
+                    byte[] computed = md.digest();
+                    byte[] actual = IOUtils.readExactlyNBytes(dis, computed.length);
+                    if (!MessageDigest.isEqual(computed, actual)) {
+                        throw new IOException(
                                 "Keystore was tampered with, or "
                                 + "password was incorrect",
-                                    new UnrecoverableKeyException(
-                                            "Password verification failed"));
-                        }
+                                new UnrecoverableKeyException(
+                                        "Password verification failed"));
                     }
                 }
             }  finally {
diff --git a/src/share/classes/java/security/CodeSource.java b/src/share/classes/java/security/CodeSource.java
index eb78c1d7a2..e2ca471360 100644
--- a/src/share/classes/java/security/CodeSource.java
+++ b/src/share/classes/java/security/CodeSource.java
@@ -570,7 +570,7 @@ public class CodeSource implements java.io.Serializable {
                 cfs.put(certType, cf);
             }
             // parse the certificate
-            byte[] encoded = IOUtils.readNBytes(ois, ois.readInt());
+            byte[] encoded = IOUtils.readExactlyNBytes(ois, ois.readInt());
             ByteArrayInputStream bais = new ByteArrayInputStream(encoded);
             try {
                 certList.add(cf.generateCertificate(bais));
diff --git a/src/share/classes/java/security/UnresolvedPermission.java b/src/share/classes/java/security/UnresolvedPermission.java
index 2e8f5c755a..c7eee55af7 100644
--- a/src/share/classes/java/security/UnresolvedPermission.java
+++ b/src/share/classes/java/security/UnresolvedPermission.java
@@ -590,7 +590,7 @@ implements java.io.Serializable
                 cfs.put(certType, cf);
             }
             // parse the certificate
-            byte[] encoded = IOUtils.readNBytes(ois, ois.readInt());
+            byte[] encoded = IOUtils.readExactlyNBytes(ois, ois.readInt());
             ByteArrayInputStream bais = new ByteArrayInputStream(encoded);
             try {
                 certList.add(cf.generateCertificate(bais));
diff --git a/src/share/classes/java/security/cert/CertificateRevokedException.java b/src/share/classes/java/security/cert/CertificateRevokedException.java
index d478a7901f..98e434607b 100644
--- a/src/share/classes/java/security/cert/CertificateRevokedException.java
+++ b/src/share/classes/java/security/cert/CertificateRevokedException.java
@@ -239,7 +239,7 @@ public class CertificateRevokedException extends CertificateException {
         for (int i = 0; i < size; i++) {
             String oid = (String) ois.readObject();
             boolean critical = ois.readBoolean();
-            byte[] extVal = IOUtils.readNBytes(ois, ois.readInt());
+            byte[] extVal = IOUtils.readExactlyNBytes(ois, ois.readInt());
             Extension ext = sun.security.x509.Extension.newExtension
                 (new ObjectIdentifier(oid), critical, extVal);
             extensions.put(oid, ext);
diff --git a/src/share/classes/sun/misc/IOUtils.java b/src/share/classes/sun/misc/IOUtils.java
index 3bc78f0ec8..327477d912 100644
--- a/src/share/classes/sun/misc/IOUtils.java
+++ b/src/share/classes/sun/misc/IOUtils.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 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
@@ -51,46 +51,31 @@ public class IOUtils {
     private static final int MAX_BUFFER_SIZE = Integer.MAX_VALUE - 8;
 
     /**
-     * Read up to {@code length} of bytes from {@code in}
-     * until EOF is detected.
+     * Read exactly {@code length} of bytes from {@code in}.
+     *
+     * <p> Note that this method is safe to be called with unknown large
+     * {@code length} argument. The memory used is proportional to the
+     * actual bytes available. An exception is thrown if there are not
+     * enough bytes in the stream.
+     *
      * @param is input stream, must not be null
      * @param length number of bytes to read
-     * @param readAll if true, an EOFException will be thrown if not enough
-     *        bytes are read.
      * @return bytes read
-     * @throws IOException Any IO error or a premature EOF is detected
+     * @throws EOFException if there are not enough bytes in the stream
+     * @throws IOException if an I/O error occurs or {@code length} is negative
+     * @throws OutOfMemoryError if an array of the required size cannot be
+     *         allocated.
      */
-    public static byte[] readFully(InputStream is, int length, boolean readAll)
+    public static byte[] readExactlyNBytes(InputStream is, int length)
             throws IOException {
         if (length < 0) {
-            throw new IOException("Invalid length");
+            throw new IOException("length cannot be negative: " + length);
         }
-        byte[] output = {};
-        int pos = 0;
-        while (pos < length) {
-            int bytesToRead;
-            if (pos >= output.length) { // Only expand when there's no room
-                bytesToRead = Math.min(length - pos, output.length + 1024);
-                if (output.length < pos + bytesToRead) {
-                    output = Arrays.copyOf(output, pos + bytesToRead);
-                }
-            } else {
-                bytesToRead = output.length - pos;
-            }
-            int cc = is.read(output, pos, bytesToRead);
-            if (cc < 0) {
-                if (readAll) {
-                    throw new EOFException("Detect premature EOF");
-                } else {
-                    if (output.length != pos) {
-                        output = Arrays.copyOf(output, pos);
-                    }
-                    break;
-                }
-            }
-            pos += cc;
+        byte[] data = readNBytes(is, length);
+        if (data.length < length) {
+            throw new EOFException();
         }
-        return output;
+        return data;
     }
 
     /**
diff --git a/src/share/classes/sun/security/krb5/internal/NetClient.java b/src/share/classes/sun/security/krb5/internal/NetClient.java
index 62243560ca..ab4fe40c3f 100644
--- a/src/share/classes/sun/security/krb5/internal/NetClient.java
+++ b/src/share/classes/sun/security/krb5/internal/NetClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 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
@@ -103,7 +103,7 @@ class TCPClient extends NetClient {
         }
 
         try {
-            return IOUtils.readFully(in, len, true);
+            return IOUtils.readExactlyNBytes(in, len);
         } catch (IOException ioe) {
             if (Krb5.DEBUG) {
                 System.out.println(
diff --git a/src/share/classes/sun/security/krb5/internal/ccache/CCacheInputStream.java b/src/share/classes/sun/security/krb5/internal/ccache/CCacheInputStream.java
index c7d9d2adef..a73af4614f 100644
--- a/src/share/classes/sun/security/krb5/internal/ccache/CCacheInputStream.java
+++ b/src/share/classes/sun/security/krb5/internal/ccache/CCacheInputStream.java
@@ -128,7 +128,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
             length--;
         for (int i = 0; i <= length; i++) {
             namelength = readLength4();
-            byte[] bytes = IOUtils.readFully(this, namelength, true);
+            byte[] bytes = IOUtils.readExactlyNBytes(this, namelength);
             result.add(new String(bytes));
         }
         if (result.isEmpty()) {
@@ -186,7 +186,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
         if (version == KRB5_FCC_FVNO_3)
             read(2); /* keytype recorded twice in fvno 3 */
         keyLen = readLength4();
-        byte[] bytes = IOUtils.readFully(this, keyLen, true);
+        byte[] bytes = IOUtils.readExactlyNBytes(this, keyLen);
         return new EncryptionKey(bytes, keyType, new Integer(version));
     }
 
@@ -239,7 +239,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
             for (int i = 0; i < num; i++) {
                 adtype = read(2);
                 adlength = readLength4();
-                data = IOUtils.readFully(this, adlength, true);
+                data = IOUtils.readExactlyNBytes(this, adlength);
                 auData.add(new AuthorizationDataEntry(adtype, data));
             }
             return auData.toArray(new AuthorizationDataEntry[auData.size()]);
@@ -253,7 +253,7 @@ public class CCacheInputStream extends KrbDataInputStream implements FileCCacheC
         if (length == 0) {
             return null;
         } else {
-            return IOUtils.readFully(this, length, true);
+            return IOUtils.readExactlyNBytes(this, length);
         }
     }
 
diff --git a/src/share/classes/sun/security/provider/JavaKeyStore.java b/src/share/classes/sun/security/provider/JavaKeyStore.java
index 409af47cdf..6befb54d68 100644
--- a/src/share/classes/sun/security/provider/JavaKeyStore.java
+++ b/src/share/classes/sun/security/provider/JavaKeyStore.java
@@ -691,7 +691,7 @@ abstract class JavaKeyStore extends KeyStoreSpi {
 
                     // Read the private key
                     entry.protectedPrivKey =
-                            IOUtils.readFully(dis, dis.readInt(), true);
+                            IOUtils.readExactlyNBytes(dis, dis.readInt());
 
                     // Read the certificate chain
                     int numOfCerts = dis.readInt();
@@ -716,7 +716,7 @@ abstract class JavaKeyStore extends KeyStoreSpi {
                                 }
                             }
                             // instantiate the certificate
-                            encoded = IOUtils.readFully(dis, dis.readInt(), true);
+                            encoded = IOUtils.readExactlyNBytes(dis, dis.readInt());
                             bais = new ByteArrayInputStream(encoded);
                             certs.add(cf.generateCertificate(bais));
                             bais.close();
@@ -755,7 +755,7 @@ abstract class JavaKeyStore extends KeyStoreSpi {
                             cfs.put(certType, cf);
                         }
                     }
-                    encoded = IOUtils.readFully(dis, dis.readInt(), true);
+                    encoded = IOUtils.readExactlyNBytes(dis, dis.readInt());
                     bais = new ByteArrayInputStream(encoded);
                     entry.cert = cf.generateCertificate(bais);
                     bais.close();
@@ -776,16 +776,13 @@ abstract class JavaKeyStore extends KeyStoreSpi {
             if (password != null) {
                 byte computed[], actual[];
                 computed = md.digest();
-                actual = new byte[computed.length];
-                dis.readFully(actual);
-                for (int i = 0; i < computed.length; i++) {
-                    if (computed[i] != actual[i]) {
-                        Throwable t = new UnrecoverableKeyException
+                actual = IOUtils.readExactlyNBytes(dis, computed.length);
+                if (!MessageDigest.isEqual(computed, actual)) {
+                    Throwable t = new UnrecoverableKeyException
                             ("Password verification failed");
-                        throw (IOException)new IOException
+                    throw (IOException) new IOException
                             ("Keystore was tampered with, or "
-                            + "password was incorrect").initCause(t);
-                    }
+                                    + "password was incorrect").initCause(t);
                 }
             }
         }
diff --git a/src/share/classes/sun/security/util/DerValue.java b/src/share/classes/sun/security/util/DerValue.java
index 46dcbd34f3..595c011693 100644
--- a/src/share/classes/sun/security/util/DerValue.java
+++ b/src/share/classes/sun/security/util/DerValue.java
@@ -409,7 +409,7 @@ public class DerValue {
         if (fullyBuffered && in.available() != length)
             throw new IOException("extra data given to DerValue constructor");
 
-        byte[] bytes = IOUtils.readFully(in, length, true);
+        byte[] bytes = IOUtils.readExactlyNBytes(in, length);
 
         buffer = new DerInputBuffer(bytes, allowBER);
         return new DerInputStream(buffer);
diff --git a/test/sun/security/util/DerValue/BadValue.java b/test/sun/security/util/DerValue/BadValue.java
index 772115506c..d02a47896e 100644
--- a/test/sun/security/util/DerValue/BadValue.java
+++ b/test/sun/security/util/DerValue/BadValue.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2017 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2009, 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
@@ -35,23 +35,23 @@ public class BadValue {
 
     public static void main(String[] args) throws Exception {
 
-        // Test IOUtils.readFully
+        // Test IOUtils.
 
         // We have 4 bytes
         InputStream in = new ByteArrayInputStream(new byte[10]);
-        byte[] bs = IOUtils.readFully(in, 4, true);
+        byte[] bs = IOUtils.readExactlyNBytes(in, 4);
         if (bs.length != 4 || in.available() != 6) {
             throw new Exception("First read error");
         }
         // But only 6 left
-        bs = IOUtils.readFully(in, 10, false);
+        bs = IOUtils.readNBytes(in, 10);
         if (bs.length != 6 || in.available() != 0) {
             throw new Exception("Second read error");
         }
         // MAX length results in exception
         in = new ByteArrayInputStream(new byte[10]);
         try {
-            bs = IOUtils.readFully(in, Integer.MAX_VALUE, true);
+            bs = IOUtils.readExactlyNBytes(in, Integer.MAX_VALUE);
             throw new Exception("No exception on MAX_VALUE length");
         } catch (EOFException ex) {
             // this is expected
@@ -59,7 +59,7 @@ public class BadValue {
         // -1 length results in exception
         in = new ByteArrayInputStream(new byte[10]);
         try {
-            bs = IOUtils.readFully(in, -1, true);
+            bs = IOUtils.readExactlyNBytes(in, -1);
             throw new Exception("No exception on -1 length");
         } catch (IOException ex) {
             // this is expected
@@ -68,13 +68,13 @@ public class BadValue {
         // 20>10, readAll means failure
         in = new ByteArrayInputStream(new byte[10]);
         try {
-            bs = IOUtils.readFully(in, 20, true);
+            bs = IOUtils.readExactlyNBytes(in, 20);
             throw new Exception("No exception on EOF");
         } catch (EOFException e) {
             // OK
         }
         int bignum = 10 * 1024 * 1024;
-        bs = IOUtils.readFully(new SuperSlowStream(bignum), bignum, true);
+        bs = IOUtils.readExactlyNBytes(new SuperSlowStream(bignum), bignum);
         if (bs.length != bignum) {
             throw new Exception("Read returned small array");
         }