8032573:...

8032573: CertificateFactory.getInstance("X.509").generateCertificates(InputStream) does not throw CertificateException for invalid input
Reviewed-by: weijun

src/share/classes/sun/security/provider/X509Factory.java

 /*
- * Copyright (c) 1998, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1998, 2014, 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
@@ -80,6 +80,7 @@ public class X509Factory extends CertificateFactorySpi {
      *
      * @exception CertificateException on parsing errors.
      */
+    @Override
     public Certificate engineGenerateCertificate(InputStream is)
         throws CertificateException
     {
@@ -103,8 +104,8 @@ public class X509Factory extends CertificateFactorySpi {
                 throw new IOException("Empty input");
             }
         } catch (IOException ioe) {
-            throw (CertificateException)new CertificateException
-            ("Could not parse certificate: " + ioe.toString()).initCause(ioe);
+            throw new CertificateException("Could not parse certificate: " +
+                    ioe.toString(), ioe);
         }
     }
 
@@ -140,6 +141,12 @@ public class X509Factory extends CertificateFactorySpi {
      * It is useful for certificates that cannot be created via
      * generateCertificate() and for converting other X509Certificate
      * implementations to an X509CertImpl.
+     *
+     * @param c The source X509Certificate
+     * @return An X509CertImpl object that is either a cached certificate or a
+     *      newly built X509CertImpl from the provided X509Certificate
+     * @throws CertificateException if failures occur while obtaining the DER
+     *      encoding for certificate data.
      */
     public static synchronized X509CertImpl intern(X509Certificate c)
             throws CertificateException {
@@ -170,6 +177,12 @@ public class X509Factory extends CertificateFactorySpi {
     /**
      * Return an interned X509CRLImpl for the given certificate.
      * For more information, see intern(X509Certificate).
+     *
+     * @param c The source X509CRL
+     * @return An X509CRLImpl object that is either a cached CRL or a
+     *      newly built X509CRLImpl from the provided X509CRL
+     * @throws CRLException if failures occur while obtaining the DER
+     *      encoding for CRL data.
      */
     public static synchronized X509CRLImpl intern(X509CRL c)
             throws CRLException {
@@ -229,6 +242,7 @@ public class X509Factory extends CertificateFactorySpi {
      * @exception CertificateException if an exception occurs while decoding
      * @since 1.4
      */
+    @Override
     public CertPath engineGenerateCertPath(InputStream inStream)
         throws CertificateException
     {
@@ -260,6 +274,7 @@ public class X509Factory extends CertificateFactorySpi {
      *   the encoding requested is not supported
      * @since 1.4
      */
+    @Override
     public CertPath engineGenerateCertPath(InputStream inStream,
         String encoding) throws CertificateException
     {
@@ -292,6 +307,7 @@ public class X509Factory extends CertificateFactorySpi {
      * @exception CertificateException if an exception occurs
      * @since 1.4
      */
+    @Override
     public CertPath
         engineGenerateCertPath(List<? extends Certificate> certificates)
         throws CertificateException
@@ -311,6 +327,7 @@ public class X509Factory extends CertificateFactorySpi {
      *         <code>CertPath</code> encodings (as <code>String</code>s)
      * @since 1.4
      */
+    @Override
     public Iterator<String> engineGetCertPathEncodings() {
         return(X509CertPath.getEncodingsStatic());
     }
@@ -326,6 +343,7 @@ public class X509Factory extends CertificateFactorySpi {
      *
      * @exception CertificateException on parsing errors.
      */
+    @Override
     public Collection<? extends java.security.cert.Certificate>
             engineGenerateCertificates(InputStream is)
             throws CertificateException {
@@ -351,6 +369,7 @@ public class X509Factory extends CertificateFactorySpi {
      *
      * @exception CRLException on parsing errors.
      */
+    @Override
     public CRL engineGenerateCRL(InputStream is)
         throws CRLException
     {
@@ -388,6 +407,7 @@ public class X509Factory extends CertificateFactorySpi {
      *
      * @exception CRLException on parsing errors.
      */
+    @Override
     public Collection<? extends java.security.cert.CRL> engineGenerateCRLs(
             InputStream is) throws CRLException
     {
@@ -410,11 +430,30 @@ public class X509Factory extends CertificateFactorySpi {
         parseX509orPKCS7Cert(InputStream is)
         throws CertificateException, IOException
     {
+        int peekByte;
+        byte[] data;
+        PushbackInputStream pbis = new PushbackInputStream(is);
         Collection<X509CertImpl> coll = new ArrayList<>();
-        byte[] data = readOneBlock(is);
-        if (data == null) {
+
+        // Test the InputStream for end-of-stream.  If the stream's
+        // initial state is already at end-of-stream then return
+        // an empty collection.  Otherwise, push the byte back into the
+        // stream and let readOneBlock look for the first certificate.
+        peekByte = pbis.read();
+        if (peekByte == -1) {
             return new ArrayList<>(0);
+        } else {
+            pbis.unread(peekByte);
+            data = readOneBlock(pbis);
         }
+
+        // If we end up with a null value after reading the first block
+        // then we know the end-of-stream has been reached and no certificate
+        // data has been found.
+        if (data == null) {
+            throw new CertificateException("No certificate data found");
+        }
+
         try {
             PKCS7 pkcs7 = new PKCS7(data);
             X509Certificate[] certs = pkcs7.getCertificates();
@@ -422,13 +461,13 @@ public class X509Factory extends CertificateFactorySpi {
             if (certs != null) {
                 return Arrays.asList(certs);
             } else {
-                // no crls provided
+                // no certificates provided
                 return new ArrayList<>(0);
             }
         } catch (ParsingException e) {
             while (data != null) {
                 coll.add(new X509CertImpl(data));
-                data = readOneBlock(is);
+                data = readOneBlock(pbis);
             }
         }
         return coll;
@@ -443,11 +482,30 @@ public class X509Factory extends CertificateFactorySpi {
         parseX509orPKCS7CRL(InputStream is)
         throws CRLException, IOException
     {
+        int peekByte;
+        byte[] data;
+        PushbackInputStream pbis = new PushbackInputStream(is);
         Collection<X509CRLImpl> coll = new ArrayList<>();
-        byte[] data = readOneBlock(is);
-        if (data == null) {
+
+        // Test the InputStream for end-of-stream.  If the stream's
+        // initial state is already at end-of-stream then return
+        // an empty collection.  Otherwise, push the byte back into the
+        // stream and let readOneBlock look for the first CRL.
+        peekByte = pbis.read();
+        if (peekByte == -1) {
             return new ArrayList<>(0);
+        } else {
+            pbis.unread(peekByte);
+            data = readOneBlock(pbis);
         }
+
+        // If we end up with a null value after reading the first block
+        // then we know the end-of-stream has been reached and no CRL
+        // data has been found.
+        if (data == null) {
+            throw new CRLException("No CRL data found");
+        }
+
         try {
             PKCS7 pkcs7 = new PKCS7(data);
             X509CRL[] crls = pkcs7.getCRLs();
@@ -461,7 +519,7 @@ public class X509Factory extends CertificateFactorySpi {
         } catch (ParsingException e) {
             while (data != null) {
                 coll.add(new X509CRLImpl(data));
-                data = readOneBlock(is);
+                data = readOneBlock(pbis);
             }
         }
         return coll;
@@ -519,7 +577,7 @@ public class X509Factory extends CertificateFactorySpi {
 
             // Step 2: Read the rest of header, determine the line end
             int end;
-            StringBuffer header = new StringBuffer("-----");
+            StringBuilder header = new StringBuilder("-----");
             while (true) {
                 int next = is.read();
                 if (next == -1) {
@@ -562,7 +620,7 @@ public class X509Factory extends CertificateFactorySpi {
             }
 
             // Step 4: Consume the footer
-            StringBuffer footer = new StringBuffer("-");
+            StringBuilder footer = new StringBuilder("-");
             while (true) {
                 int next = is.read();
                 // Add next == '\n' for maximum safety, in case endline
@@ -623,7 +681,7 @@ public class X509Factory extends CertificateFactorySpi {
 
         int n = is.read();
         if (n == -1) {
-            throw new IOException("BER/DER length info ansent");
+            throw new IOException("BER/DER length info absent");
         }
         bout.write(n);
 

test/java/security/cert/CertificateFactory/invalidEncodedCerts/invalidcert.pem

------BEGIN CERTIFICATE-----
-XIICJjCCAdCgAwIBAgIBITANBgkqhkiG9w0BAQQFADCBqTELMAkGA1UEBhMCVVMx
-EzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNhbiBGcmFuY2lzY28xFTAT
-BgNVBAoTDEJFQSBXZWJMb2dpYzERMA8GA1UECxMIU2VjdXJpdHkxIzAhBgNVBAMT
-GkRlbW8gQ2VydGlmaWNhdGUgQXV0aG9yaXR5MR4wHAYJKoZIhvcNAQkBFg9zdXBw
-b3J0QGJlYS5jb20wHhcNMDAwNTMwMjEzODAxWhcNMDQwNTEzMjEzODAxWjCBjDEL
-MAkGA1UEBhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExFjAUBgNVBAcTDVNhbiBG
-cmFuY2lzY28xFTATBgNVBAoTDEJFQSBXZWJMb2dpYzEZMBcGA1UEAxMQd2VibG9n
-aWMuYmVhLmNvbTEeMBwGCSqGSIb3DQEJARYPc3VwcG9ydEBiZWEuY29tMFwwDQYJ
-KoZIhvcNAQEBBQADSwAwSAJBALdsXEHqKHgs6zj0hU5sXMAUHzoT8kgWXmNkKHXH
-79qbPh6EfdlriW9G/AbRF/pKrCQu7hhllAxREbqTuSlf2EMCAwEAATANBgkqhkiG
-9w0BAQQFAANBACgmqflL5m5LNeJGpWx9aIoABCiuDcpw1fFyegsqGX7CBhffcruS
-1p8h5vkHVbMu1frD1UgGnPlOO/K7Ig/KrsU=
------END CERTIFICATE-----