From 1e7b93fa2b8e39e94a309474d1e0729340a88e87 Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 3 Jan 2020 18:09:11 +0000 Subject: 8231139: Improved keystore support Reviewed-by: mbalao --- .../com/sun/crypto/provider/JceKeyStore.java | 61 +++++++--------------- src/share/classes/java/security/CodeSource.java | 2 +- .../java/security/UnresolvedPermission.java | 2 +- .../security/cert/CertificateRevokedException.java | 2 +- src/share/classes/sun/misc/IOUtils.java | 51 +++++++----------- .../sun/security/krb5/internal/NetClient.java | 4 +- .../krb5/internal/ccache/CCacheInputStream.java | 8 +-- .../sun/security/provider/JavaKeyStore.java | 19 +++---- src/share/classes/sun/security/util/DerValue.java | 2 +- 9 files changed, 55 insertions(+), 96 deletions(-) (limited to 'src/share') 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 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}. + * + *

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); -- cgit v1.2.3