8244565: Accept PKCS #8 with version number 1

Reviewed-by: valeriep

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2020, 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
@@ -26,29 +26,36 @@
 package sun.security.pkcs;
 
 import java.io.*;
-import java.util.Properties;
-import java.math.*;
 import java.security.Key;
 import java.security.KeyRep;
 import java.security.PrivateKey;
 import java.security.KeyFactory;
 import java.security.MessageDigest;
-import java.security.Security;
-import java.security.Provider;
 import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.PKCS8EncodedKeySpec;
+import java.util.Arrays;
 
-import sun.security.util.HexDumpEncoder;
 import sun.security.x509.*;
 import sun.security.util.*;
 
 /**
  * Holds a PKCS#8 key, for example a private key
  *
- * @author Dave Brownell
+ * According to https://tools.ietf.org/html/rfc5958:
- * @author Benjamin Renaud
+ *
+ *     OneAsymmetricKey ::= SEQUENCE {
+ *        version                   Version,
+ *        privateKeyAlgorithm       PrivateKeyAlgorithmIdentifier,
+ *        privateKey                PrivateKey,
+ *        attributes            [0] Attributes OPTIONAL,
+ *        ...,
+ *        [[2: publicKey        [1] PublicKey OPTIONAL ]],
+ *        ...
+ *      }
+ *
+ * We support this format but do not parse attributes and publicKey now.
  */
 public class PKCS8Key implements PrivateKey {
 
@@ -62,186 +69,100 @@ public class PKCS8Key implements PrivateKey {
     /* The key bytes, without the algorithm information */
     protected byte[] key;
 
-    /* The encoded for the key. */
+    /* The encoded for the key. Created on demand by encode(). */
     protected byte[] encodedKey;
 
     /* The version for this key */
-    public static final BigInteger version = BigInteger.ZERO;
+    private static final int V1 = 0;
+    private static final int V2 = 1;
 
     /**
-     * Default constructor.  The key constructed must have its key
+     * Default constructor. Constructors in sub-classes that create a new key
-     * and algorithm initialized before it may be used, for example
+     * from its components require this. These constructors must initialize
-     * by using <code>decode</code>.
+     * {@link #algid} and {@link #key}.
      */
-    public PKCS8Key() { }
+    protected PKCS8Key() { }
 
-    /*
+    /**
-     * Build and initialize as a "default" key.  All PKCS#8 key
+     * Another constructor. Constructors in sub-classes that create a new key
-     * data is stored and transmitted losslessly, but no knowledge
+     * from an encoded byte array require this. We do not assign this
-     * about this particular algorithm is available.
+     * encoding to {@link #encodedKey} directly.
+     *
+     * This method is also used by {@link #parseKey} to create a raw key.
      */
-    private PKCS8Key (AlgorithmId algid, byte[] key)
+    protected PKCS8Key(byte[] input) throws InvalidKeyException {
-    throws InvalidKeyException {
+        decode(new ByteArrayInputStream(input));
-        this.algid = algid;
-        this.key = key;
-        encode();
     }
 
-    /*
+    private void decode(InputStream is) throws InvalidKeyException {
-     * Binary backwards compatibility. New uses should call parseKey().
+        try {
-     */
+            DerValue val = new DerValue(is);
-    public static PKCS8Key parse (DerValue in) throws IOException {
+            if (val.tag != DerValue.tag_Sequence) {
-        PrivateKey key;
+                throw new InvalidKeyException("invalid key format");
+            }
 
-        key = parseKey(in);
+            int version = val.data.getInteger();
-        if (key instanceof PKCS8Key)
+            if (version != V1 && version != V2) {
-            return (PKCS8Key)key;
+                throw new InvalidKeyException("unknown version: " + version);
+            }
+            algid = AlgorithmId.parse (val.data.getDerValue ());
+            key = val.data.getOctetString ();
 
-        throw new IOException("Provider did not return PKCS8Key");
+            DerValue next;
+            if (val.data.available() == 0) {
+                return;
+            }
+            next = val.data.getDerValue();
+            if (next.isContextSpecific((byte)0)) {
+                if (val.data.available() == 0) {
+                    return;
+                }
+                next = val.data.getDerValue();
+            }
+
+            if (next.isContextSpecific((byte)1)) {
+                if (version == V1) {
+                    throw new InvalidKeyException("publicKey seen in v1");
+                }
+                if (val.data.available() == 0) {
+                    return;
+                }
+            }
+            throw new InvalidKeyException("Extra bytes");
+        } catch (IOException e) {
+            throw new InvalidKeyException("IOException : " + e.getMessage());
+        }
     }
 
     /**
-     * Construct PKCS#8 subject public key from a DER value.  If
+     * Construct PKCS#8 subject public key from a DER value.  If a
-     * the runtime environment is configured with a specific class for
+     * security provider supports the key algorithm with a specific class,
-     * this kind of key, a subclass is returned.  Otherwise, a generic
+     * a PrivateKey from the provider is returned.  Otherwise, a raw
      * PKCS8Key object is returned.
      *
-     * <P>This mechanism gurantees that keys (and algorithms) may be
+     * <P>This mechanism guarantees that keys (and algorithms) may be
      * freely manipulated and transferred, without risk of losing
      * information.  Also, when a key (or algorithm) needs some special
-     * handling, that specific need can be accomodated.
+     * handling, that specific need can be accommodated.
      *
      * @param in the DER-encoded SubjectPublicKeyInfo value
      * @exception IOException on data format errors
      */
-    public static PrivateKey parseKey (DerValue in) throws IOException
+    public static PrivateKey parseKey(DerValue in) throws IOException {
-    {
-        AlgorithmId algorithm;
-        PrivateKey privKey;
-
-        if (in.tag != DerValue.tag_Sequence)
-            throw new IOException ("corrupt private key");
-
-        BigInteger parsedVersion = in.data.getBigInteger();
-        if (!version.equals(parsedVersion)) {
-            throw new IOException("version mismatch: (supported: " +
-                                  Debug.toHexString(version) +
-                                  ", parsed: " +
-                                  Debug.toHexString(parsedVersion));
-        }
-
-        algorithm = AlgorithmId.parse (in.data.getDerValue ());
-
         try {
-            privKey = buildPKCS8Key (algorithm, in.data.getOctetString ());
+            PKCS8Key rawKey = new PKCS8Key(in.toByteArray());
-
+            PKCS8EncodedKeySpec pkcs8KeySpec
-        } catch (InvalidKeyException e) {
+                = new PKCS8EncodedKeySpec(rawKey.getEncoded());
-            throw new IOException("corrupt private key");
-        }
-
-        if (in.data.available () != 0)
-            throw new IOException ("excess private key");
-        return privKey;
-    }
-
-    /**
-     * Parse the key bits.  This may be redefined by subclasses to take
-     * advantage of structure within the key.  For example, RSA public
-     * keys encapsulate two unsigned integers (modulus and exponent) as
-     * DER values within the <code>key</code> bits; Diffie-Hellman and
-     * DSS/DSA keys encapsulate a single unsigned integer.
-     *
-     * <P>This function is called when creating PKCS#8 SubjectPublicKeyInfo
-     * values using the PKCS8Key member functions, such as <code>parse</code>
-     * and <code>decode</code>.
-     *
-     * @exception IOException if a parsing error occurs.
-     * @exception InvalidKeyException if the key encoding is invalid.
-     */
-    protected void parseKeyBits () throws IOException, InvalidKeyException {
-        encode();
-    }
-
-    /*
-     * Factory interface, building the kind of key associated with this
-     * specific algorithm ID or else returning this generic base class.
-     * See the description above.
-     */
-    static PrivateKey buildPKCS8Key (AlgorithmId algid, byte[] key)
-    throws IOException, InvalidKeyException
-    {
-        /*
-         * Use the algid and key parameters to produce the ASN.1 encoding
-         * of the key, which will then be used as the input to the
-         * key factory.
-         */
-        DerOutputStream pkcs8EncodedKeyStream = new DerOutputStream();
-        encode(pkcs8EncodedKeyStream, algid, key);
-        PKCS8EncodedKeySpec pkcs8KeySpec
-            = new PKCS8EncodedKeySpec(pkcs8EncodedKeyStream.toByteArray());
-
-        try {
-            // Instantiate the key factory of the appropriate algorithm
-            KeyFactory keyFac = KeyFactory.getInstance(algid.getName());
-
-            // Generate the private key
-            return keyFac.generatePrivate(pkcs8KeySpec);
-        } catch (NoSuchAlgorithmException e) {
-            // Return generic PKCS8Key with opaque key data (see below)
-        } catch (InvalidKeySpecException e) {
-            // Return generic PKCS8Key with opaque key data (see below)
-        }
-
-        /*
-         * Try again using JDK1.1-style for backwards compatibility.
-         */
-        String classname = "";
-        try {
-            Properties props;
-            String keytype;
-            Provider sunProvider;
-
-            sunProvider = Security.getProvider("SUN");
-            if (sunProvider == null)
-                throw new InstantiationException();
-            classname = sunProvider.getProperty("PrivateKey.PKCS#8." +
-              algid.getName());
-            if (classname == null) {
-                throw new InstantiationException();
-            }
-
-            Class<?> keyClass = null;
             try {
-                keyClass = Class.forName(classname);
+                return KeyFactory.getInstance(rawKey.algid.getName())
-            } catch (ClassNotFoundException e) {
+                        .generatePrivate(pkcs8KeySpec);
-                ClassLoader cl = ClassLoader.getSystemClassLoader();
+            } catch (NoSuchAlgorithmException | InvalidKeySpecException e) {
-                if (cl != null) {
+                // Ignore and return raw key
-                    keyClass = cl.loadClass(classname);
+                return rawKey;
-                }
             }
-
+        } catch (InvalidKeyException e) {
-            @SuppressWarnings("deprecation")
+            throw new IOException("corrupt private key", e);
-            Object      inst = (keyClass != null) ? keyClass.newInstance() : null;
-            PKCS8Key    result;
-
-            if (inst instanceof PKCS8Key) {
-                result = (PKCS8Key) inst;
-                result.algid = algid;
-                result.key = key;
-                result.parseKeyBits();
-                return result;
-            }
-        } catch (ClassNotFoundException e) {
-        } catch (InstantiationException e) {
-        } catch (IllegalAccessException e) {
-            // this should not happen.
-            throw new IOException (classname + " [internal error]");
         }
-
-        PKCS8Key result = new PKCS8Key();
-        result.algid = algid;
-        result.key = key;
-        return result;
     }
 
     /**
@@ -254,26 +175,22 @@ public class PKCS8Key implements PrivateKey {
     /**
      * Returns the algorithm ID to be used with this key.
      */
-    public AlgorithmId  getAlgorithmId () { return algid; }
+    public AlgorithmId  getAlgorithmId () {
-
+        return algid;
-    /**
-     * PKCS#8 sequence on the DER output stream.
-     */
-    public final void encode(DerOutputStream out) throws IOException
-    {
-        encode(out, this.algid, this.key);
     }
 
     /**
-     * Returns the DER-encoded form of the key as a byte array.
+     * Returns the DER-encoded form of the key as a byte array,
+     * or {@code null} if an encoding error occurs.
      */
     public synchronized byte[] getEncoded() {
-        byte[] result = null;
         try {
-            result = encode();
+            encode();
+            return encodedKey.clone();
         } catch (InvalidKeyException e) {
+            // ignored and return null
         }
-        return result;
+        return null;
     }
 
     /**
@@ -284,76 +201,26 @@ public class PKCS8Key implements PrivateKey {
     }
 
     /**
-     * Returns the DER-encoded form of the key as a byte array.
+     * DER-encodes this key as a byte array that can be retrieved
+     * by the {@link #getEncoded()} method.
      *
      * @exception InvalidKeyException if an encoding error occurs.
      */
-    public byte[] encode() throws InvalidKeyException {
+    private void encode() throws InvalidKeyException {
         if (encodedKey == null) {
             try {
-                DerOutputStream out;
+                DerOutputStream out = new DerOutputStream ();
-
+                DerOutputStream tmp = new DerOutputStream();
-                out = new DerOutputStream ();
+                tmp.putInteger(V1);
-                encode (out);
+                algid.encode(tmp);
+                tmp.putOctetString(key);
+                out.write(DerValue.tag_Sequence, tmp);
                 encodedKey = out.toByteArray();
-
             } catch (IOException e) {
                 throw new InvalidKeyException ("IOException : " +
                                                e.getMessage());
             }
         }
-        return encodedKey.clone();
-    }
-
-    /**
-     * Initialize an PKCS8Key object from an input stream.  The data
-     * on that input stream must be encoded using DER, obeying the
-     * PKCS#8 format: a sequence consisting of a version, an algorithm
-     * ID and a bit string which holds the key.  (That bit string is
-     * often used to encapsulate another DER encoded sequence.)
-     *
-     * <P>Subclasses should not normally redefine this method; they should
-     * instead provide a <code>parseKeyBits</code> method to parse any
-     * fields inside the <code>key</code> member.
-     *
-     * @param in an input stream with a DER-encoded PKCS#8
-     * SubjectPublicKeyInfo value
-     *
-     * @exception InvalidKeyException if a parsing error occurs.
-     */
-    public void decode(InputStream in) throws InvalidKeyException
-    {
-        DerValue        val;
-
-        try {
-            val = new DerValue (in);
-            if (val.tag != DerValue.tag_Sequence)
-                throw new InvalidKeyException ("invalid key format");
-
-
-            BigInteger version = val.data.getBigInteger();
-            if (!version.equals(PKCS8Key.version)) {
-                throw new IOException("version mismatch: (supported: " +
-                                      Debug.toHexString(PKCS8Key.version) +
-                                      ", parsed: " +
-                                      Debug.toHexString(version));
-            }
-            algid = AlgorithmId.parse (val.data.getDerValue ());
-            key = val.data.getOctetString ();
-            parseKeyBits ();
-
-            if (val.data.available () != 0)  {
-                // OPTIONAL attributes not supported yet
-            }
-
-        } catch (IOException e) {
-            throw new InvalidKeyException("IOException : " +
-                                          e.getMessage());
-        }
-    }
-
-    public void decode(byte[] encodedKey) throws InvalidKeyException {
-        decode(new ByteArrayInputStream(encodedKey));
     }
 
     @java.io.Serial
@@ -365,35 +232,18 @@ public class PKCS8Key implements PrivateKey {
     }
 
     /**
-     * Serialization read ... PKCS#8 keys serialize as
+     * We used to serialize a PKCS8Key as itself (instead of a KeyRep).
-     * themselves, and they're parsed when they get read back.
      */
     @java.io.Serial
-    private void readObject (ObjectInputStream stream)
+    private void readObject(ObjectInputStream stream) throws IOException {
-    throws IOException {
-
         try {
             decode(stream);
-
         } catch (InvalidKeyException e) {
-            e.printStackTrace();
             throw new IOException("deserialized key is invalid: " +
                                   e.getMessage());
         }
     }
 
-    /*
-     * Produce PKCS#8 encoding from algorithm id and key material.
-     */
-    static void encode(DerOutputStream out, AlgorithmId algid, byte[] key)
-        throws IOException {
-            DerOutputStream tmp = new DerOutputStream();
-            tmp.putInteger(version);
-            algid.encode(tmp);
-            tmp.putOctetString(key);
-            out.write(DerValue.tag_Sequence, tmp);
-    }
-
     /**
      * Compares two private keys. This returns false if the object with which
      * to compare is not of type <code>Key</code>.
@@ -401,29 +251,18 @@ public class PKCS8Key implements PrivateKey {
      * encoding of the given key object.
      *
      * @param object the object with which to compare
-     * @return <code>true</code> if this key has the same encoding as the
+     * @return {@code true} if this key has the same encoding as the
-     * object argument; <code>false</code> otherwise.
+     *          object argument; {@code false} otherwise.
      */
     public boolean equals(Object object) {
         if (this == object) {
             return true;
         }
-
         if (object instanceof Key) {
-
-            // this encoding
-            byte[] b1;
-            if (encodedKey != null) {
-                b1 = encodedKey;
-            } else {
-                b1 = getEncoded();
-            }
-
-            // that encoding
-            byte[] b2 = ((Key)object).getEncoded();
-
             // time-constant comparison
-            return MessageDigest.isEqual(b1, b2);
+            return MessageDigest.isEqual(
+                    getEncoded(),
+                    ((Key)object).getEncoded());
         }
         return false;
     }
@@ -433,12 +272,6 @@ public class PKCS8Key implements PrivateKey {
      * which are equal will also have the same hashcode.
      */
     public int hashCode() {
-        int retval = 0;
+        return Arrays.hashCode(getEncoded());
-        byte[] b1 = getEncoded();
-
-        for (int i = 1; i < b1.length; i++) {
-            retval += b1[i] * i;
-        }
-        return(retval);
     }
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1996, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2020, 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
@@ -25,11 +25,9 @@
 
 package sun.security.provider;
 
-import java.util.*;
 import java.io.*;
 import java.math.BigInteger;
 import java.security.InvalidKeyException;
-import java.security.ProviderException;
 import java.security.AlgorithmParameters;
 import java.security.spec.DSAParameterSpec;
 import java.security.spec.InvalidParameterSpecException;
@@ -37,10 +35,8 @@ import java.security.interfaces.DSAParams;
 
 import sun.security.x509.AlgIdDSA;
 import sun.security.pkcs.PKCS8Key;
-import sun.security.util.Debug;
 import sun.security.util.DerValue;
 import sun.security.util.DerInputStream;
-import sun.security.util.DerOutputStream;
 
 /**
  * A PKCS#8 private key for the Digital Signature Algorithm.
@@ -54,7 +50,7 @@ import sun.security.util.DerOutputStream;
  */
 
 public final class DSAPrivateKey extends PKCS8Key
-implements java.security.interfaces.DSAPrivateKey, Serializable {
+        implements java.security.interfaces.DSAPrivateKey, Serializable {
 
     /** use serialVersionUID from JDK 1.1. for interoperability */
     @java.io.Serial
@@ -63,30 +59,19 @@ implements java.security.interfaces.DSAPrivateKey, Serializable {
     /* the private key */
     private BigInteger x;
 
-    /*
-     * Keep this constructor for backwards compatibility with JDK1.1.
-     */
-    public DSAPrivateKey() {
-    }
-
     /**
      * Make a DSA private key out of a private key and three parameters.
      */
     public DSAPrivateKey(BigInteger x, BigInteger p,
-                         BigInteger q, BigInteger g)
+                         BigInteger q, BigInteger g) {
-    throws InvalidKeyException {
         this.x = x;
         algid = new AlgIdDSA(p, q, g);
 
         try {
             key = new DerValue(DerValue.tag_Integer,
                                x.toByteArray()).toByteArray();
-            encode();
         } catch (IOException e) {
-            InvalidKeyException ike = new InvalidKeyException(
+            throw new AssertionError("Should not happen", e);
-                "could not DER encode x: " + e.getMessage());
-            ike.initCause(e);
-            throw ike;
         }
     }
 
@@ -94,8 +79,13 @@ implements java.security.interfaces.DSAPrivateKey, Serializable {
      * Make a DSA private key from its DER encoding (PKCS #8).
      */
     public DSAPrivateKey(byte[] encoded) throws InvalidKeyException {
-        clearOldKey();
+        super(encoded);
-        decode(encoded);
+        try {
+            DerInputStream in = new DerInputStream(key);
+            x = in.getBigInteger();
+        } catch (IOException e) {
+            throw new InvalidKeyException(e.getMessage(), e);
+        }
     }
 
     /**
@@ -113,7 +103,7 @@ implements java.security.interfaces.DSAPrivateKey, Serializable {
                     return null;
                 }
                 paramSpec = algParams.getParameterSpec(DSAParameterSpec.class);
-                return (DSAParams)paramSpec;
+                return paramSpec;
             }
         } catch (InvalidParameterSpecException e) {
             return null;
@@ -122,35 +112,8 @@ implements java.security.interfaces.DSAPrivateKey, Serializable {
 
     /**
      * Get the raw private key, x, without the parameters.
-     *
-     * @see getParameters
      */
     public BigInteger getX() {
         return x;
     }
-
-    private void clearOldKey() {
-        int i;
-        if (this.encodedKey != null) {
-            for (i = 0; i < this.encodedKey.length; i++) {
-                this.encodedKey[i] = (byte)0x00;
-            }
-        }
-        if (this.key != null) {
-            for (i = 0; i < this.key.length; i++) {
-                this.key[i] = (byte)0x00;
-            }
-        }
-    }
-
-    protected void parseKeyBits() throws InvalidKeyException {
-        try {
-            DerInputStream in = new DerInputStream(key);
-            x = in.getBigInteger();
-        } catch (IOException e) {
-            InvalidKeyException ike = new InvalidKeyException(e.getMessage());
-            ike.initCause(e);
-            throw ike;
-        }
-    }
 }

src/java.base/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java

@@ -79,6 +79,9 @@ public final class RSAPrivateCrtKeyImpl
      */
     public static RSAPrivateKey newKey(byte[] encoded)
             throws InvalidKeyException {
+        if (encoded == null || encoded.length == 0) {
+            throw new InvalidKeyException("Missing key encoding");
+        }
         RSAPrivateCrtKeyImpl key = new RSAPrivateCrtKeyImpl(encoded);
         // check all CRT-specific components are available, if any one
         // missing, return a non-CRT key instead
@@ -124,11 +127,8 @@ public final class RSAPrivateCrtKeyImpl
      * Construct a key from its encoding. Called from newKey above.
      */
     RSAPrivateCrtKeyImpl(byte[] encoded) throws InvalidKeyException {
-        if (encoded == null || encoded.length == 0) {
+        super(encoded);
-            throw new InvalidKeyException("Missing key encoding");
+        parseKeyBits();
-        }
-
-        decode(encoded);
         RSAKeyFactory.checkRSAProviderKeyLengths(n.bitLength(), e);
         try {
             // check the validity of oid and params
@@ -258,10 +258,7 @@ public final class RSAPrivateCrtKeyImpl
                + "\n  modulus: " + n + "\n  private exponent: " + d;
     }
 
-    /**
+    private void parseKeyBits() throws InvalidKeyException {
-     * Parse the key. Called by PKCS8Key.
-     */
-    protected void parseKeyBits() throws InvalidKeyException {
         try {
             DerInputStream in = new DerInputStream(key);
             DerValue derValue = in.getDerValue();

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Key.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2020, 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
@@ -807,13 +807,9 @@ abstract class P11Key implements Key, Length {
             token.ensureValid();
             if (encoded == null) {
                 fetchValues();
-                try {
+                Key key = new sun.security.provider.DSAPrivateKey
-                    Key key = new sun.security.provider.DSAPrivateKey
+                        (x, params.getP(), params.getQ(), params.getG());
-                            (x, params.getP(), params.getQ(), params.getG());
+                encoded = key.getEncoded();
-                    encoded = key.getEncoded();
-                } catch (InvalidKeyException e) {
-                    throw new ProviderException(e);
-                }
             }
             return encoded;
         }

src/jdk.crypto.ec/share/classes/sun/security/ec/ECPrivateKeyImpl.java

@@ -71,7 +71,8 @@ public final class ECPrivateKeyImpl extends PKCS8Key implements ECPrivateKey {
      * Construct a key from its encoding. Called by the ECKeyFactory.
      */
     ECPrivateKeyImpl(byte[] encoded) throws InvalidKeyException {
-        decode(encoded);
+        super(encoded);
+        parseKeyBits();
     }
 
     /**
@@ -112,8 +113,8 @@ public final class ECPrivateKeyImpl extends PKCS8Key implements ECPrivateKey {
     }
 
     private void makeEncoding(BigInteger s) throws InvalidKeyException {
-        algid = new AlgorithmId
+        algid = new AlgorithmId(AlgorithmId.EC_oid,
-        (AlgorithmId.EC_oid, ECParameters.getAlgorithmParameters(params));
+                ECParameters.getAlgorithmParameters(params));
         try {
             byte[] sArr = s.toByteArray();
             // convert to fixed-length array
@@ -131,8 +132,7 @@ public final class ECPrivateKeyImpl extends PKCS8Key implements ECPrivateKey {
                 new DerValue(DerValue.tag_Sequence, out.toByteArray());
             key = val.toByteArray();
         } catch (IOException exc) {
-            // should never occur
+            throw new AssertionError("Should not happen", exc);
-            throw new InvalidKeyException(exc);
         }
     }
 
@@ -163,10 +163,7 @@ public final class ECPrivateKeyImpl extends PKCS8Key implements ECPrivateKey {
         return params;
     }
 
-    /**
+    private void parseKeyBits() throws InvalidKeyException {
-     * Parse the key. Called by PKCS8Key.
-     */
-    protected void parseKeyBits() throws InvalidKeyException {
         try {
             DerInputStream in = new DerInputStream(key);
             DerValue derValue = in.getDerValue();

src/jdk.crypto.ec/share/classes/sun/security/ec/XDHPrivateKeyImpl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2020, 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
@@ -43,45 +43,34 @@ public final class XDHPrivateKeyImpl extends PKCS8Key implements XECPrivateKey {
     private byte[] k;
 
     XDHPrivateKeyImpl(XECParameters params, byte[] k)
-        throws InvalidKeyException {
+            throws InvalidKeyException {
 
         this.paramSpec = new NamedParameterSpec(params.getName());
         this.k = k.clone();
 
         this.algid = new AlgorithmId(params.getOid());
-        encodeKey();
+        DerOutputStream derKey = new DerOutputStream();
-
+        try {
+            derKey.putOctetString(k);
+            this.key = derKey.toByteArray();
+        } catch (IOException ex) {
+            throw new AssertionError("Should not happen", ex);
+        }
         checkLength(params);
     }
 
     XDHPrivateKeyImpl(byte[] encoded) throws InvalidKeyException {
-
+        super(encoded);
-        decode(encoded);
         XECParameters params = XECParameters.get(
             InvalidKeyException::new, algid);
         paramSpec = new NamedParameterSpec(params.getName());
-        decodeKey();
-
-        checkLength(params);
-    }
-
-    private void decodeKey() throws InvalidKeyException {
         try {
             DerInputStream derStream = new DerInputStream(key);
             k = derStream.getOctetString();
         } catch (IOException ex) {
             throw new InvalidKeyException(ex);
         }
-    }
+        checkLength(params);
-
-    private void encodeKey() {
-        DerOutputStream derKey = new DerOutputStream();
-        try {
-            derKey.putOctetString(k);
-            this.key = derKey.toByteArray();
-        } catch (IOException ex) {
-            throw new ProviderException(ex);
-        }
     }
 
     void checkLength(XECParameters params) throws InvalidKeyException {

src/jdk.crypto.ec/share/classes/sun/security/ec/ed/EdDSAPrivateKeyImpl.java

@@ -37,7 +37,7 @@ import sun.security.x509.AlgorithmId;
 import sun.security.util.*;
 
 public final class EdDSAPrivateKeyImpl
-    extends PKCS8Key implements EdECPrivateKey {
+        extends PKCS8Key implements EdECPrivateKey {
 
     private static final long serialVersionUID = 1L;
 
@@ -45,46 +45,36 @@ public final class EdDSAPrivateKeyImpl
     private byte[] h;
 
     EdDSAPrivateKeyImpl(EdDSAParameters params, byte[] h)
-        throws InvalidKeyException {
+            throws InvalidKeyException {
 
         this.paramSpec = new NamedParameterSpec(params.getName());
         this.algid = new AlgorithmId(params.getOid());
         this.h = h.clone();
 
-        encodeKey();
+        DerOutputStream derKey = new DerOutputStream();
-
+        try {
+            derKey.putOctetString(h);
+            this.key = derKey.toByteArray();
+        } catch (IOException ex) {
+            throw new AssertionError("Should not happen", ex);
+        }
         checkLength(params);
     }
 
     EdDSAPrivateKeyImpl(byte[] encoded) throws InvalidKeyException {
 
-        decode(encoded);
+        super(encoded);
         EdDSAParameters params = EdDSAParameters.get(
             InvalidKeyException::new, algid);
         paramSpec = new NamedParameterSpec(params.getName());
 
-        decodeKey();
-
-        checkLength(params);
-    }
-
-    private void decodeKey() throws InvalidKeyException {
         try {
             DerInputStream derStream = new DerInputStream(key);
             h = derStream.getOctetString();
         } catch (IOException ex) {
             throw new InvalidKeyException(ex);
         }
-    }
+        checkLength(params);
-
-    private void encodeKey() {
-        DerOutputStream derKey = new DerOutputStream();
-        try {
-            derKey.putOctetString(h);
-            this.key = derKey.toByteArray();
-        } catch (IOException ex) {
-            throw new ProviderException(ex);
-        }
     }
 
     void checkLength(EdDSAParameters params) throws InvalidKeyException {

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

@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8048357
+ * @bug 8048357 8244565
  * @summary PKCS8 Standards Conformance Tests
  * @library /test/lib
  * @modules java.base/sun.security.pkcs
@@ -31,250 +31,89 @@
  *          java.base/sun.security.provider
  *          java.base/sun.security.x509
  * @compile -XDignore.symbol.file PKCS8Test.java
- * @run main PKCS8Test
+ * @run testng PKCS8Test
  */
 
 import java.io.IOException;
 import java.math.BigInteger;
-import java.security.InvalidKeyException;
 import java.util.Arrays;
+
+import jdk.test.lib.Utils;
+import org.testng.Assert;
+import org.testng.annotations.Test;
 import sun.security.pkcs.PKCS8Key;
 import sun.security.provider.DSAPrivateKey;
-import sun.security.util.DerOutputStream;
 import sun.security.util.DerValue;
-import sun.security.x509.AlgorithmId;
-import jdk.test.lib.hexdump.HexPrinter;
-import static java.lang.System.out;
 
 public class PKCS8Test {
 
-    static final DerOutputStream derOutput = new DerOutputStream();
-
     static final String FORMAT = "PKCS#8";
-    static final String EXPECTED_ALG_ID_CHRS = "DSA\n\tp:     02\n\tq:     03\n"
+    static final String EXPECTED_ALG_ID_CHRS = "DSA\n" +
-            + "\tg:     04\n";
+            "\tp:     02\n\tq:     03\n\tg:     04\n";
     static final String ALGORITHM = "DSA";
-    static final String EXCEPTION_MESSAGE = "version mismatch: (supported:     "
-            + "00, parsed:     01";
 
-    // test second branch in byte[] encode()
+    static final byte[] EXPECTED = Utils.toByteArray(
-    // DER encoding,include (empty) set of attributes
+            "301e" + // SEQUENCE
-    static final int[] NEW_ENCODED_KEY_INTS = { 0x30,
+                "020100" +  // Version int 0
-            // length 30 = 0x1e
+                "3014" +    // PrivateKeyAlgorithmIdentifier
-            0x1e,
+                    "06072a8648ce380401" +      // OID DSA 1.2.840.10040.4.1
-            // first element
+                    "3009020102020103020104" +  // p=2, q=3, g=4
-            // version Version (= INTEGER)
+                "0403020101");  // PrivateKey OCTET int x = 1
-            0x02,
-            // length 1
-            0x01,
-            // value 0
-            0x00,
-            // second element
-            // privateKeyAlgorithmIdentifier PrivateKeyAlgorithmIdentifier
-            // (sequence)
-            // (an object identifier?)
-            0x30,
-            // length 18
-            0x12,
-            // contents
-            // object identifier, 5 bytes
-            0x06, 0x05,
-            // { 1 3 14 3 2 12 }
-            0x2b, 0x0e, 0x03, 0x02, 0x0c,
-            // sequence, 9 bytes
-            0x30, 0x09,
-            // integer 2
-            0x02, 0x01, 0x02,
-            // integer 3
-            0x02, 0x01, 0x03,
-            // integer 4
-            0x02, 0x01, 0x04,
-            // third element
-            // privateKey PrivateKey (= OCTET STRING)
-            0x04,
-            // length
-            0x03,
-            // privateKey contents
-            0x02, 0x01, 0x01,
-            // 4th (optional) element -- attributes [0] IMPLICIT Attributes
-            // OPTIONAL
-            // (Attributes = SET OF Attribute) Here, it will be empty.
-            0xA0,
-            // length
-            0x00 };
 
-    // encoding originally created, but with the version changed
+    @Test
-    static final int[] NEW_ENCODED_KEY_INTS_2 = {
+    public void test() throws IOException {
-            // sequence
-            0x30,
-            // length 28 = 0x1c
-            0x1c,
-            // first element
-            // version Version (= INTEGER)
-            0x02,
-            // length 1
-            0x01,
-            // value 1 (illegal)
-            0x01,
-            // second element
-            // privateKeyAlgorithmIdentifier PrivateKeyAlgorithmIdentifier
-            // (sequence)
-            // (an object identifier?)
-            0x30,
-            // length 18
-            0x12,
-            // contents
-            // object identifier, 5 bytes
-            0x06, 0x05,
-            // { 1 3 14 3 2 12 }
-            0x2b, 0x0e, 0x03, 0x02, 0x0c,
-            // sequence, 9 bytes
-            0x30, 0x09,
-            // integer 2
-            0x02, 0x01, 0x02,
-            // integer 3
-            0x02, 0x01, 0x03,
-            // integer 4
-            0x02, 0x01, 0x04,
-            // third element
-            // privateKey PrivateKey (= OCTET STRING)
-            0x04,
-            // length
-            0x03,
-            // privateKey contents
-            0x02, 0x01, 0x01 };
 
-    // 0000: 30 1E 02 01 00 30 14 06 07 2A 86 48 CE 38 04 01 0....0...*.H.8..
+        byte[] encodedKey = new DSAPrivateKey(
-    // 0010: 30 09 02 01 02 02 01 03 02 01 04 04 03 02 01 01 0...............
+                BigInteger.valueOf(1),
-    static final int[] EXPECTED = { 0x30,
+                BigInteger.valueOf(2),
-            // length 30 = 0x1e
+                BigInteger.valueOf(3),
-            0x1e,
+                BigInteger.valueOf(4)).getEncoded();
-            // first element
-            // version Version (= INTEGER)
-            0x02,
-            // length 1
-            0x01,
-            // value 0
-            0x00,
-            // second element
-            // privateKeyAlgorithmIdentifier PrivateKeyAlgorithmIdentifier
-            // (sequence)
-            // (an object identifier?)
-            0x30, 0x14, 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x38, 0x04, 0x01,
-            // integer 2
-            0x30, 0x09, 0x02,
-            // integer 3
-            0x01, 0x02, 0x02,
-            // integer 4
-            0x01, 0x03, 0x02,
-            // third element
-            // privateKey PrivateKey (= OCTET STRING)
-            0x01,
-            // length
-            0x04,
-            // privateKey contents
-            0x04, 0x03, 0x02,
-            // 4th (optional) element -- attributes [0] IMPLICIT Attributes
-            // OPTIONAL
-            // (Attributes = SET OF Attribute) Here, it will be empty.
-            0x01,
-            // length
-            0x01 };
 
-    static void raiseException(String expected, String received) {
+        Assert.assertTrue(Arrays.equals(encodedKey, EXPECTED),
-        throw new RuntimeException(
+                Utils.toHexString(encodedKey));
-                "Expected " + expected + "; Received " + received);
+
+        PKCS8Key decodedKey = (PKCS8Key)PKCS8Key.parseKey(
+                new DerValue(encodedKey));
+
+        Assert.assertEquals(ALGORITHM, decodedKey.getAlgorithm());
+        Assert.assertEquals(FORMAT, decodedKey.getFormat());
+        Assert.assertEquals(EXPECTED_ALG_ID_CHRS,
+                decodedKey.getAlgorithmId().toString());
+
+        byte[] encodedOutput = decodedKey.getEncoded();
+        Assert.assertTrue(Arrays.equals(encodedOutput, EXPECTED),
+                Utils.toHexString(encodedOutput));
+
+        // Test additional fields
+        enlarge(0, "8000");    // attributes
+        enlarge(1, "810100");  // public key for v2
+        enlarge(1, "8000", "810100");  // both
+
+        Assert.assertThrows(() -> enlarge(2));  // bad ver
+        Assert.assertThrows(() -> enlarge(0, "8000", "8000")); // no dup
+        Assert.assertThrows(() -> enlarge(0, "810100")); // no public in v1
+        Assert.assertThrows(() -> enlarge(1, "810100", "8000")); // bad order
+        Assert.assertThrows(() -> enlarge(1, "820100")); // bad tag
     }
 
-    public static void main(String[] args)
+    /**
-            throws IOException, InvalidKeyException {
+     * Add more fields to EXPECTED and see if it's still valid PKCS8.
-
+     *
-        BigInteger x = BigInteger.valueOf(1);
+     * @param newVersion new version
-        BigInteger p = BigInteger.valueOf(2);
+     * @param fields extra fields to add, in hex
-        BigInteger q = BigInteger.valueOf(3);
+     */
-        BigInteger g = BigInteger.valueOf(4);
+    static void enlarge(int newVersion, String... fields) throws IOException {
-
+        byte[] original = EXPECTED.clone();
-        DSAPrivateKey priv = new DSAPrivateKey(x, p, q, g);
+        int length = original.length;
-
+        for (String field : fields) {   // append fields
-        byte[] encodedKey = priv.getEncoded();
+            byte[] add = Utils.toByteArray(field);
-        byte[] expectedBytes = new byte[EXPECTED.length];
+            original = Arrays.copyOf(original, length + add.length);
-        for (int i = 0; i < EXPECTED.length; i++) {
+            System.arraycopy(add, 0, original, length, add.length);
-            expectedBytes[i] = (byte) EXPECTED[i];
+            length += add.length;
         }
-
+        Assert.assertTrue(length < 127);
-        dumpByteArray("encodedKey :", encodedKey);
+        original[1] = (byte)(length - 2);   // the length field inside DER
-        if (!Arrays.equals(encodedKey, expectedBytes)) {
+        original[4] = (byte)newVersion;     // the version inside DER
-            raiseException(new String(expectedBytes), new String(encodedKey));
+        PKCS8Key.parseKey(new DerValue(original));
-        }
-
-        PKCS8Key decodedKey = PKCS8Key.parse(new DerValue(encodedKey));
-
-        String alg = decodedKey.getAlgorithm();
-        AlgorithmId algId = decodedKey.getAlgorithmId();
-        out.println("Algorithm :" + alg);
-        out.println("AlgorithmId: " + algId);
-
-        if (!ALGORITHM.equals(alg)) {
-            raiseException(ALGORITHM, alg);
-        }
-        if (!EXPECTED_ALG_ID_CHRS.equalsIgnoreCase(algId.toString())) {
-            raiseException(EXPECTED_ALG_ID_CHRS, algId.toString());
-        }
-
-        decodedKey.encode(derOutput);
-        dumpByteArray("Stream encode: ", derOutput.toByteArray());
-        if (!Arrays.equals(derOutput.toByteArray(), expectedBytes)) {
-            raiseException(new String(expectedBytes), derOutput.toString());
-        }
-
-        dumpByteArray("byte[] encoding: ", decodedKey.getEncoded());
-        if (!Arrays.equals(decodedKey.getEncoded(), expectedBytes)) {
-            raiseException(new String(expectedBytes),
-                    new String(decodedKey.getEncoded()));
-        }
-
-        if (!FORMAT.equals(decodedKey.getFormat())) {
-            raiseException(FORMAT, decodedKey.getFormat());
-        }
-
-        try {
-            byte[] newEncodedKey = new byte[NEW_ENCODED_KEY_INTS.length];
-            for (int i = 0; i < newEncodedKey.length; i++) {
-                newEncodedKey[i] = (byte) NEW_ENCODED_KEY_INTS[i];
-            }
-            PKCS8Key newDecodedKey = PKCS8Key
-                    .parse(new DerValue(newEncodedKey));
-
-            throw new RuntimeException(
-                    "key1: Expected an IOException during " + "parsing");
-        } catch (IOException e) {
-            System.out.println("newEncodedKey: should have excess data due to "
-                    + "attributes, which are not supported");
-        }
-
-        try {
-            byte[] newEncodedKey2 = new byte[NEW_ENCODED_KEY_INTS_2.length];
-            for (int i = 0; i < newEncodedKey2.length; i++) {
-                newEncodedKey2[i] = (byte) NEW_ENCODED_KEY_INTS_2[i];
-            }
-
-            PKCS8Key newDecodedKey2 = PKCS8Key
-                    .parse(new DerValue(newEncodedKey2));
-
-            throw new RuntimeException(
-                    "key2: Expected an IOException during " + "parsing");
-        } catch (IOException e) {
-            out.println("Key 2: should be illegal version");
-            out.println(e.getMessage());
-            if (!EXCEPTION_MESSAGE.equals(e.getMessage())) {
-                throw new RuntimeException("Key2: expected: "
-                        + EXCEPTION_MESSAGE + " get: " + e.getMessage());
-            }
-        }
-    }
-
-    static void dumpByteArray(String nm, byte[] bytes) throws IOException {
-        out.println(nm + " length: " + bytes.length);
-        HexPrinter.simple().dest(out).format(bytes);
     }
 }