8231139: Improved keystore support

Reviewed-by: mullan, ahgross

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

@@ -26,6 +26,7 @@
 package com.sun.crypto.provider;
 
 import sun.security.util.Debug;
+import sun.security.util.IOUtils;
 
 import java.io.*;
 import java.util.*;
@@ -73,7 +74,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
@@ -742,23 +743,11 @@ public final class JceKeyStore extends KeyStoreSpi {
                         entry.date = new Date(dis.readLong());
 
                         // read the private key
-                        try {
+                        entry.protectedKey = IOUtils.readExactlyNBytes(dis, dis.readInt());
-                            entry.protectedKey = new byte[dis.readInt()];
-                        } catch (OutOfMemoryError e) {
-                            throw new IOException("Keysize too big");
-                        }
-                        dis.readFully(entry.protectedKey);
 
                         // read the certificate chain
                         int numOfCerts = dis.readInt();
-                        try {
+                        List<Certificate> tmpCerts = new ArrayList<>();
-                            if (numOfCerts > 0) {
-                                entry.chain = new Certificate[numOfCerts];
-                            }
-                        } catch (OutOfMemoryError e) {
-                            throw new IOException("Too many certificates in "
-                                                  + "chain");
-                        }
                         for (int j = 0; j < numOfCerts; j++) {
                             if (xVersion == 2) {
                                 // read the certificate type, and instantiate a
@@ -766,27 +755,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
+                                    // store the certificate factory so we can
-                                // reuse it later
+                                    // reuse it later
                                     cfs.put(certType, cf);
                                 }
                             }
                             // instantiate the certificate
-                            try {
+                            encoded = IOUtils.readExactlyNBytes(dis, dis.readInt());
-                                encoded = new byte[dis.readInt()];
-                            } catch (OutOfMemoryError e) {
-                                throw new IOException("Certificate too big");
-                            }
-                            dis.readFully(encoded);
                             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);
@@ -818,12 +804,7 @@ public final class JceKeyStore extends KeyStoreSpi {
                                 cfs.put(certType, cf);
                             }
                         }
-                        try {
+                        encoded = IOUtils.readExactlyNBytes(dis, dis.readInt());
-                            encoded = new byte[dis.readInt()];
-                        } catch (OutOfMemoryError e) {
-                            throw new IOException("Certificate too big");
-                        }
-                        dis.readFully(encoded);
                         bais = new ByteArrayInputStream(encoded);
                         entry.cert = cf.generateCertificate(bais);
 
@@ -882,18 +863,14 @@ public final class JceKeyStore extends KeyStoreSpi {
                  * with
                  */
                 if (password != null) {
-                    byte computed[], actual[];
+                    byte[] computed = md.digest();
-                    computed = md.digest();
+                    byte[] actual = IOUtils.readExactlyNBytes(dis, computed.length);
-                    actual = new byte[computed.length];
+                    if (!MessageDigest.isEqual(computed, actual)) {
-                    dis.readFully(actual);
+                        throw new IOException(
-                    for (int i = 0; i < computed.length; i++) {
-                        if (computed[i] != actual[i]) {
-                            throw new IOException(
                                 "Keystore was tampered with, or "
                                         + "password was incorrect",
-                                    new UnrecoverableKeyException(
+                                new UnrecoverableKeyException(
-                                            "Password verification failed"));
+                                        "Password verification failed"));
-                        }
                     }
                 }
             }  finally {

src/java.base/share/classes/java/security/CodeSource.java

@@ -598,7 +598,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));

src/java.base/share/classes/java/security/UnresolvedPermission.java

@@ -594,7 +594,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));

src/java.base/share/classes/java/security/cert/CertificateRevokedException.java

@@ -244,7 +244,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);

src/java.base/share/classes/sun/security/provider/JavaKeyStore.java

@@ -697,7 +697,7 @@ public 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();
@@ -722,7 +722,7 @@ public 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();
@@ -761,7 +761,7 @@ public 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();
@@ -787,16 +787,13 @@ public abstract class JavaKeyStore extends KeyStoreSpi {
              */
             if (password != null) {
                 byte[] computed = md.digest();
-                byte[] actual = new byte[computed.length];
+                byte[] actual = IOUtils.readExactlyNBytes(dis, computed.length);
-                dis.readFully(actual);
+                if (!MessageDigest.isEqual(computed, actual)) {
-                for (int i = 0; i < computed.length; i++) {
+                    Throwable t = new UnrecoverableKeyException
-                    if (computed[i] != actual[i]) {
-                        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);
-                    }
                 }
             }
         }

src/java.base/share/classes/sun/security/util/DerValue.java

@@ -395,7 +395,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);

src/java.base/share/classes/sun/security/util/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
@@ -32,68 +32,34 @@ package sun.security.util;
 import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.Arrays;
 
 public class IOUtils {
 
     /**
-     * Read up to {@code length} of bytes from {@code in}
+     * Read exactly {@code length} of bytes from {@code in}.
-     * until EOF is detected.
+     *
-     * @param is input stream, must not be null
+     * <p> Note that this method is safe to be called with unknown large
-     * @param length number of bytes to read
+     * {@code length} argument. The memory used is proportional to the
-     * @param readAll if true, an EOFException will be thrown if not enough
+     * actual bytes available. An exception is thrown if there are not
-     *        bytes are read.
+     * enough bytes in the stream.
-     * @return bytes read
-     * @throws IOException Any IO error or a premature EOF is detected
-     */
-    public static byte[] readFully(InputStream is, int length, boolean readAll)
-            throws IOException {
-        if (length < 0) {
-            throw new IOException("Invalid 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;
-        }
-        return output;
-    }
-
-    /**
-     * Read {@code length} of bytes from {@code in}. 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, must not be negative
+     * @param length number of bytes to read
      * @return bytes read
-     * @throws IOException if any IO error or a premature EOF is detected, or
+     * @throws EOFException if there are not enough bytes in the stream
-     *      if {@code length} is negative since this length is usually also
+     * @throws IOException if an I/O error occurs or {@code length} is negative
-     *      read from {@code is}.
+     * @throws OutOfMemoryError if an array of the required size cannot be
+     *         allocated.
      */
-    public static byte[] readNBytes(InputStream is, int length) throws IOException {
+    public static byte[] readExactlyNBytes(InputStream is, int length)
+            throws IOException {
         if (length < 0) {
             throw new IOException("length cannot be negative: " + length);
         }
-        return readFully(is, length, true);
+        byte[] data = is.readNBytes(length);
+        if (data.length < length) {
+            throw new EOFException();
+        }
+        return data;
     }
 }

src/java.security.jgss/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
@@ -102,7 +102,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(

src/java.security.jgss/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, 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);
         }
     }
 

test/jdk/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 = in.readNBytes(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
@@ -61,7 +61,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
@@ -70,13 +70,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");
         }