8074935: jdk8 keytool doesn't validate pem files for RFC 1421 correctness, as jdk7 did

Reviewed-by: mullan

src/share/classes/sun/security/pkcs10/PKCS10.java

@@ -290,8 +290,9 @@ public class PKCS10 {
             throw new SignatureException("Cert request was not signed");
 
 
+        byte[] CRLF = new byte[] {'\r', '\n'};
         out.println("-----BEGIN NEW CERTIFICATE REQUEST-----");
-        out.println(Base64.getMimeEncoder().encodeToString(encoded));
+        out.println(Base64.getMimeEncoder(64, CRLF).encodeToString(encoded));
         out.println("-----END NEW CERTIFICATE REQUEST-----");
     }
 

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

@@ -28,6 +28,8 @@ package sun.security.provider;
 import java.io.*;
 import java.util.*;
 import java.security.cert.*;
+
+import sun.security.util.Pem;
 import sun.security.x509.X509CertImpl;
 import sun.security.x509.X509CRLImpl;
 import sun.security.pkcs.PKCS7;
@@ -633,7 +635,7 @@ public class X509Factory extends CertificateFactorySpi {
 
             checkHeaderFooter(header.toString(), footer.toString());
 
-            return Base64.getMimeDecoder().decode(new String(data, 0, pos));
+            return Pem.decode(new String(data, 0, pos));
         }
     }
 

src/share/classes/sun/security/tools/keytool/Main.java

@@ -79,6 +79,7 @@ import sun.security.pkcs.PKCS9Attribute;
 import sun.security.tools.KeyStoreUtil;
 import sun.security.tools.PathList;
 import sun.security.util.DerValue;
+import sun.security.util.Pem;
 import sun.security.x509.*;
 
 import static java.security.KeyStore.*;
@@ -99,6 +100,8 @@ import static sun.security.tools.keytool.Main.Option.*;
  */
 public final class Main {
 
+    private static final byte[] CRLF = new byte[] {'\r', '\n'};
+
     private boolean debug = false;
     private Command command = null;
     private String sigAlgName = null;
@@ -1205,7 +1208,7 @@ public final class Main {
                 sb.append(s);
             }
         }
-        byte[] rawReq = Base64.getMimeDecoder().decode(new String(sb));
+        byte[] rawReq = Pem.decode(new String(sb));
         PKCS10 req = new PKCS10(rawReq);
 
         info.set(X509CertInfo.KEY, new CertificateX509Key(req.getSubjectPublicKeyInfo()));
@@ -1282,7 +1285,7 @@ public final class Main {
         crl.sign(privateKey, sigAlgName);
         if (rfc) {
             out.println("-----BEGIN X509 CRL-----");
-            out.println(Base64.getMimeEncoder().encodeToString(crl.getEncodedInternal()));
+            out.println(Base64.getMimeEncoder(64, CRLF).encodeToString(crl.getEncodedInternal()));
             out.println("-----END X509 CRL-----");
         } else {
             out.write(crl.getEncodedInternal());
@@ -2251,7 +2254,7 @@ public final class Main {
         if (rfc) {
             X509CRL xcrl = (X509CRL)crl;
             out.println("-----BEGIN X509 CRL-----");
-            out.println(Base64.getMimeEncoder().encodeToString(xcrl.getEncoded()));
+            out.println(Base64.getMimeEncoder(64, CRLF).encodeToString(xcrl.getEncoded()));
             out.println("-----END X509 CRL-----");
         } else {
             out.println(crl.toString());
@@ -2278,7 +2281,7 @@ public final class Main {
                 sb.append(s);
             }
         }
-        PKCS10 req = new PKCS10(Base64.getMimeDecoder().decode(new String(sb)));
+        PKCS10 req = new PKCS10(Pem.decode(new String(sb)));
 
         PublicKey pkey = req.getSubjectPublicKeyInfo();
         out.printf(rb.getString("PKCS.10.Certificate.Request.Version.1.0.Subject.s.Public.Key.s.format.s.key."),
@@ -3059,7 +3062,7 @@ public final class Main {
     {
         if (rfc) {
             out.println(X509Factory.BEGIN_CERT);
-            out.println(Base64.getMimeEncoder().encodeToString(cert.getEncoded()));
+            out.println(Base64.getMimeEncoder(64, CRLF).encodeToString(cert.getEncoded()));
             out.println(X509Factory.END_CERT);
         } else {
             out.write(cert.getEncoded()); // binary

src/share/classes/sun/security/util/Pem.java

+/*
+ * Copyright (c) 2015, 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.
+ */
+
+package sun.security.util;
+
+import java.io.IOException;
+import java.util.Base64;
+
+/**
+ * The Length interface defines the length of an object
+ */
+public class Pem {
+
+    /**
+     * Decodes a PEM-encoded block.
+     *
+     * @param input the input string, according to RFC 1421, can only contain
+     *              characters in the base-64 alphabet and whitespaces.
+     * @return the decoded bytes
+     * @throws java.io.IOException if input is invalid
+     */
+    public static byte[] decode(String input) throws IOException {
+        byte[] src = input.replaceAll("\\s+", "").getBytes();
+        try {
+            return Base64.getDecoder().decode(src);
+        } catch (IllegalArgumentException e) {
+            throw new IOException(e);
+        }
+    }
+}

src/share/classes/sun/security/x509/X509CertImpl.java

@@ -271,7 +271,7 @@ public class X509CertImpl extends X509Certificate implements DerEncoder {
                         der = new DerValue(decstream.toByteArray());
                         break;
                     } else {
-                        decstream.write(Base64.getMimeDecoder().decode(line));
+                        decstream.write(Pem.decode(line));
                     }
                 }
             } catch (IOException ioe2) {

test/sun/security/provider/X509Factory/BadPem.java

+/*
+ * Copyright (c) 2015, 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.
+ *
+ * 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.
+ */
+
+/*
+ * @test
+ * @bug 8074935
+ * @summary jdk8 keytool doesn't validate pem files for RFC 1421 correctness, as jdk7 did
+ */
+
+import java.io.ByteArrayOutputStream;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.PrintStream;
+import java.security.KeyStore;
+import java.security.cert.CertificateException;
+import java.util.Arrays;
+import java.util.Base64;
+
+import sun.security.provider.X509Factory;
+import java.security.cert.CertificateFactory;
+import java.io.ByteArrayInputStream;
+
+public class BadPem {
+
+    public static void main(String[] args) throws Exception {
+        String ks = System.getProperty("test.src", ".")
+                + "/../../ssl/etc/keystore";
+        String pass = "passphrase";
+        String alias = "dummy";
+
+        KeyStore keyStore = KeyStore.getInstance("JKS");
+        keyStore.load(new FileInputStream(ks), pass.toCharArray());
+        byte[] cert = keyStore.getCertificate(alias).getEncoded();
+
+        ByteArrayOutputStream bout = new ByteArrayOutputStream();
+        PrintStream pout = new PrintStream(bout);
+        byte[] CRLF = new byte[] {'\r', '\n'};
+        pout.println(X509Factory.BEGIN_CERT);
+        for (int i=0; i<cert.length; i += 48) {
+            int blockLen = (cert.length > i + 48) ? 48 : (cert.length - i);
+            pout.println("!" + Base64.getEncoder()
+                    .encodeToString(Arrays.copyOfRange(cert, i, i + blockLen)));
+        }
+        pout.println(X509Factory.END_CERT);
+
+        CertificateFactory cf = CertificateFactory.getInstance("X.509");
+
+        try {
+            cf.generateCertificate(new ByteArrayInputStream(bout.toByteArray()));
+            throw new Exception("Should fail");
+        } catch (CertificateException e) {
+            // Good
+        }
+    }
+}
+

test/sun/security/tools/keytool/KeyToolTest.java

@@ -55,6 +55,8 @@
  * NSS PKCS11 config file are changed, DSA not supported now.
  */
 
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.security.KeyStore;
 import sun.security.x509.*;
 import java.io.*;
@@ -844,6 +846,24 @@ public class KeyToolTest {
         remove("mykey.cert");
     }
 
+    // 8074935: jdk8 keytool doesn't validate pem files for RFC 1421 correctness
+    static void checkPem(String file) throws Exception {
+        boolean maybeLast = false;
+        for (String s: Files.readAllLines(Paths.get(file))) {
+            if (s.isEmpty()) continue;
+            if (s.startsWith("---")) continue;
+            if (maybeLast) {
+                throw new Exception("Last line already seen");
+            }
+            if (s.length() > 64) {
+                throw new Exception(s);
+            }
+            if (s.length() < 64) {
+                maybeLast = true;
+            }
+        }
+    }
+
     void v3extTest(String keyAlg) throws Exception {
         KeyStore ks;
         remove("x.jks");
@@ -1153,11 +1173,14 @@ public class KeyToolTest {
                 "-rfc -file test.req");
         // printcertreq
         testOK("", "-printcertreq -file test.req");
-        // issue: deny KU, change criticality of 1.2.3 and 1.2.4, change content of BC, add 2.3.4
+        checkPem("test.req");
+        // issue: deny KU, change criticality of 1.2.3 and 1.2.4,
+        // change content of BC, add 2.3.4
         testOK("", simple+"-gencert -alias ca -infile test.req -ext " +
                 "honored=all,-KU,1.2.3:critical,1.2.4:non-critical " +
                 "-ext BC=2 -ext 2.3.4=01020304 " +
                 "-debug -rfc -outfile test.cert");
+        checkPem("test.cert");
         testOK("", simple+"-importcert -file test.cert -alias a");
         ks = loadStore("x.jks", "changeit", "JKS");
         X509CertImpl a = (X509CertImpl)ks.getCertificate("a");