8308010: X509Key and PKCS8Key allows garbage bytes at the end

Reviewed-by: mullan

src/java.base/share/classes/sun/security/pkcs/PKCS8Key.java

@@ -92,13 +92,15 @@ public class PKCS8Key implements PrivateKey, InternalPrivateKey {
      * This method is also used by {@link #parseKey} to create a raw key.
      */
     protected PKCS8Key(byte[] input) throws InvalidKeyException {
-        decode(new ByteArrayInputStream(input));
+        try {
+            decode(new DerValue(input));
+        } catch (IOException e) {
+            throw new InvalidKeyException("Unable to decode key", e);
+        }
     }
 
-    private void decode(InputStream is) throws InvalidKeyException {
+    private void decode(DerValue val) throws InvalidKeyException {
-        DerValue val = null;
         try {
-            val = new DerValue(is);
             if (val.tag != DerValue.tag_Sequence) {
                 throw new InvalidKeyException("invalid key format");
             }
@@ -132,7 +134,7 @@ public class PKCS8Key implements PrivateKey, InternalPrivateKey {
             }
             throw new InvalidKeyException("Extra bytes");
         } catch (IOException e) {
-            throw new InvalidKeyException("IOException : " + e.getMessage());
+            throw new InvalidKeyException("Unable to decode key", e);
         } finally {
             if (val != null) {
                 val.clear();
@@ -241,10 +243,9 @@ public class PKCS8Key implements PrivateKey, InternalPrivateKey {
     @java.io.Serial
     private void readObject(ObjectInputStream stream) throws IOException {
         try {
-            decode(stream);
+            decode(new DerValue(stream));
         } catch (InvalidKeyException e) {
-            throw new IOException("deserialized key is invalid: " +
+            throw new IOException("deserialized key is invalid", e);
-                                  e.getMessage());
         }
     }
 

src/java.base/share/classes/sun/security/x509/X509Key.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2023, 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
@@ -334,8 +334,7 @@ public class X509Key implements PublicKey, DerEncoder {
     }
 
     /**
-     * Initialize an X509Key object from an input stream.  The data on that
+     * Initialize an X509Key object from a DerValue, obeying the X.509
-     * input stream must be encoded using DER, obeying the X.509
      * <code>SubjectPublicKeyInfo</code> format.  That is, the data is a
      * sequence consisting of an algorithm ID and a bit string which holds
      * the key.  (That bit string is often used to encapsulate another DER
@@ -350,17 +349,11 @@ public class X509Key implements PublicKey, DerEncoder {
      * private keys may override this method, <code>encode</code>, and
      * of course <code>getFormat</code>.
      *
-     * @param in an input stream with a DER-encoded X.509
+     * @param val a DER-encoded X.509 SubjectPublicKeyInfo value
-     *          SubjectPublicKeyInfo value
      * @exception InvalidKeyException on parsing errors.
      */
-    public void decode(InputStream in)
+    void decode(DerValue val) throws InvalidKeyException {
-    throws InvalidKeyException
-    {
-        DerValue        val;
-
         try {
-            val = new DerValue(in);
             if (val.tag != DerValue.tag_Sequence)
                 throw new InvalidKeyException("invalid key format");
 
@@ -371,13 +364,16 @@ public class X509Key implements PublicKey, DerEncoder {
                 throw new InvalidKeyException ("excess key data");
 
         } catch (IOException e) {
-            throw new InvalidKeyException("IOException: " +
+            throw new InvalidKeyException("Unable to decode key", e);
-                                          e.getMessage());
         }
     }
 
     public void decode(byte[] encodedKey) throws InvalidKeyException {
-        decode(new ByteArrayInputStream(encodedKey));
+        try {
+            decode(new DerValue(encodedKey));
+        } catch (IOException e) {
+            throw new InvalidKeyException("Unable to decode key", e);
+        }
     }
 
     /**
@@ -396,11 +392,9 @@ public class X509Key implements PublicKey, DerEncoder {
     @java.io.Serial
     private void readObject(ObjectInputStream stream) throws IOException {
         try {
-            decode(stream);
+            decode(new DerValue(stream));
         } catch (InvalidKeyException e) {
-            e.printStackTrace();
+            throw new IOException("deserialized key is invalid", e);
-            throw new IOException("deserialized key is invalid: " +
-                                  e.getMessage());
         }
     }
 

test/jdk/sun/security/pkcs/pkcs8/LongPKCS8orX509KeySpec.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2023, 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 8308010
+ * @summary X509Key and PKCS8Key allows garbage bytes at the end
+ * @library /test/lib
+ */
+
+import jdk.test.lib.Utils;
+
+import java.security.KeyFactory;
+import java.security.KeyPairGenerator;
+import java.security.spec.InvalidKeySpecException;
+import java.security.spec.PKCS8EncodedKeySpec;
+import java.security.spec.X509EncodedKeySpec;
+import java.util.Arrays;
+
+public class LongPKCS8orX509KeySpec {
+
+    public static void main(String[] argv) throws Exception {
+        var g = KeyPairGenerator.getInstance("EC");
+        var f = KeyFactory.getInstance("EC");
+        Utils.runAndCheckException(() -> f.generatePublic(new X509EncodedKeySpec(
+                Arrays.copyOf(g.generateKeyPair().getPublic().getEncoded(), 1000))),
+                InvalidKeySpecException.class);
+        Utils.runAndCheckException(() -> f.generatePrivate(new PKCS8EncodedKeySpec(
+                Arrays.copyOf(g.generateKeyPair().getPrivate().getEncoded(), 1000))),
+                InvalidKeySpecException.class);
+    }
+}