8175251: Failed to load RSA private key from pkcs12
Summary: Enhanced DER library with extra arg to control leading-0 check
Reviewed-by: mullan
--- a/jdk/src/java.base/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java Wed Mar 15 23:09:18 2017 +0100
+++ b/jdk/src/java.base/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java Wed Mar 15 22:57:48 2017 +0000
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2017, 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
@@ -191,14 +191,22 @@
if (version != 0) {
throw new IOException("Version must be 0");
}
- n = getBigInteger(data);
- e = getBigInteger(data);
- d = getBigInteger(data);
- p = getBigInteger(data);
- q = getBigInteger(data);
- pe = getBigInteger(data);
- qe = getBigInteger(data);
- coeff = getBigInteger(data);
+
+ /*
+ * Some implementations do not correctly encode ASN.1 INTEGER values
+ * in 2's complement format, resulting in a negative integer when
+ * decoded. Correct the error by converting it to a positive integer.
+ *
+ * See CR 6255949
+ */
+ n = data.getPositiveBigInteger();
+ e = data.getPositiveBigInteger();
+ d = data.getPositiveBigInteger();
+ p = data.getPositiveBigInteger();
+ q = data.getPositiveBigInteger();
+ pe = data.getPositiveBigInteger();
+ qe = data.getPositiveBigInteger();
+ coeff = data.getPositiveBigInteger();
if (derValue.data.available() != 0) {
throw new IOException("Extra data available");
}
@@ -206,23 +214,4 @@
throw new InvalidKeyException("Invalid RSA private key", e);
}
}
-
- /**
- * Read a BigInteger from the DerInputStream.
- */
- static BigInteger getBigInteger(DerInputStream data) throws IOException {
- BigInteger b = data.getBigInteger();
-
- /*
- * Some implementations do not correctly encode ASN.1 INTEGER values
- * in 2's complement format, resulting in a negative integer when
- * decoded. Correct the error by converting it to a positive integer.
- *
- * See CR 6255949
- */
- if (b.signum() < 0) {
- b = new BigInteger(1, b.toByteArray());
- }
- return b;
- }
}
--- a/jdk/src/java.base/share/classes/sun/security/rsa/RSAPublicKeyImpl.java Wed Mar 15 23:09:18 2017 +0100
+++ b/jdk/src/java.base/share/classes/sun/security/rsa/RSAPublicKeyImpl.java Wed Mar 15 22:57:48 2017 +0000
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2003, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2017, 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
@@ -111,8 +111,8 @@
throw new IOException("Not a SEQUENCE");
}
DerInputStream data = derValue.data;
- n = RSAPrivateCrtKeyImpl.getBigInteger(data);
- e = RSAPrivateCrtKeyImpl.getBigInteger(data);
+ n = data.getPositiveBigInteger();
+ e = data.getPositiveBigInteger();
if (derValue.data.available() != 0) {
throw new IOException("Extra data available");
}
--- a/jdk/src/java.base/share/classes/sun/security/util/DerInputBuffer.java Wed Mar 15 23:09:18 2017 +0100
+++ b/jdk/src/java.base/share/classes/sun/security/util/DerInputBuffer.java Wed Mar 15 22:57:48 2017 +0000
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1996, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2017, 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
@@ -44,16 +44,26 @@
*/
class DerInputBuffer extends ByteArrayInputStream implements Cloneable {
- DerInputBuffer(byte[] buf) { super(buf); }
+ boolean allowBER = true;
+
+ // used by sun/security/util/DerInputBuffer/DerInputBufferEqualsHashCode.java
+ DerInputBuffer(byte[] buf) {
+ this(buf, true);
+ }
- DerInputBuffer(byte[] buf, int offset, int len) {
+ DerInputBuffer(byte[] buf, boolean allowBER) {
+ super(buf);
+ this.allowBER = allowBER;
+ }
+
+ DerInputBuffer(byte[] buf, int offset, int len, boolean allowBER) {
super(buf, offset, len);
+ this.allowBER = allowBER;
}
DerInputBuffer dup() {
try {
DerInputBuffer retval = (DerInputBuffer)clone();
-
retval.mark(Integer.MAX_VALUE);
return retval;
} catch (CloneNotSupportedException e) {
@@ -147,8 +157,8 @@
System.arraycopy(buf, pos, bytes, 0, len);
skip(len);
- // check to make sure no extra leading 0s for DER
- if (len >= 2 && (bytes[0] == 0) && (bytes[1] >= 0)) {
+ // BER allows leading 0s but DER does not
+ if (!allowBER && (len >= 2 && (bytes[0] == 0) && (bytes[1] >= 0))) {
throw new IOException("Invalid encoding: redundant leading 0s");
}
--- a/jdk/src/java.base/share/classes/sun/security/util/DerInputStream.java Wed Mar 15 23:09:18 2017 +0100
+++ b/jdk/src/java.base/share/classes/sun/security/util/DerInputStream.java Wed Mar 15 22:57:48 2017 +0000
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1996, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 2017, 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
@@ -81,6 +81,25 @@
}
/**
+ * Create a DER input stream from part of a data buffer with
+ * additional arg to control whether DER checks are enforced.
+ * The buffer is not copied, it is shared. Accordingly, the
+ * buffer should be treated as read-only.
+ *
+ * @param data the buffer from which to create the string (CONSUMED)
+ * @param offset the first index of <em>data</em> which will
+ * be read as DER input in the new stream
+ * @param len how long a chunk of the buffer to use,
+ * starting at "offset"
+ * @param allowBER whether to allow constructed indefinite-length
+ * encoding as well as tolerate leading 0s
+ */
+ public DerInputStream(byte[] data, int offset, int len,
+ boolean allowBER) throws IOException {
+ init(data, offset, len, allowBER);
+ }
+
+ /**
* Create a DER input stream from part of a data buffer.
* The buffer is not copied, it is shared. Accordingly, the
* buffer should be treated as read-only.
@@ -95,47 +114,27 @@
init(data, offset, len, true);
}
- /**
- * Create a DER input stream from part of a data buffer with
- * additional arg to indicate whether to allow constructed
- * indefinite-length encoding.
- * The buffer is not copied, it is shared. Accordingly, the
- * buffer should be treated as read-only.
- *
- * @param data the buffer from which to create the string (CONSUMED)
- * @param offset the first index of <em>data</em> which will
- * be read as DER input in the new stream
- * @param len how long a chunk of the buffer to use,
- * starting at "offset"
- * @param allowIndefiniteLength whether to allow constructed
- * indefinite-length encoding
- */
- public DerInputStream(byte[] data, int offset, int len,
- boolean allowIndefiniteLength) throws IOException {
- init(data, offset, len, allowIndefiniteLength);
- }
-
/*
* private helper routine
*/
- private void init(byte[] data, int offset, int len,
- boolean allowIndefiniteLength) throws IOException {
+ private void init(byte[] data, int offset, int len, boolean allowBER) throws IOException {
if ((offset+2 > data.length) || (offset+len > data.length)) {
throw new IOException("Encoding bytes too short");
}
// check for indefinite length encoding
if (DerIndefLenConverter.isIndefinite(data[offset+1])) {
- if (!allowIndefiniteLength) {
+ if (!allowBER) {
throw new IOException("Indefinite length BER encoding found");
} else {
byte[] inData = new byte[len];
System.arraycopy(data, offset, inData, 0, len);
DerIndefLenConverter derIn = new DerIndefLenConverter();
- buffer = new DerInputBuffer(derIn.convert(inData));
+ buffer = new DerInputBuffer(derIn.convert(inData), allowBER);
}
- } else
- buffer = new DerInputBuffer(data, offset, len);
+ } else {
+ buffer = new DerInputBuffer(data, offset, len, allowBER);
+ }
buffer.mark(Integer.MAX_VALUE);
}
@@ -156,7 +155,7 @@
*/
public DerInputStream subStream(int len, boolean do_skip)
throws IOException {
- DerInputBuffer newbuf = buffer.dup();
+ DerInputBuffer newbuf = buffer.dup();
newbuf.truncate(len);
if (do_skip) {
@@ -399,7 +398,8 @@
dis.readFully(indefData, offset, readLen);
dis.close();
DerIndefLenConverter derIn = new DerIndefLenConverter();
- buffer = new DerInputBuffer(derIn.convert(indefData));
+ buffer = new DerInputBuffer(derIn.convert(indefData), buffer.allowBER);
+
if (tag != buffer.read())
throw new IOException("Indefinite length encoding" +
" not supported");
@@ -427,7 +427,7 @@
DerValue value;
do {
- value = new DerValue(newstr.buffer);
+ value = new DerValue(newstr.buffer, buffer.allowBER);
vec.addElement(value);
} while (newstr.available() > 0);
--- a/jdk/src/java.base/share/classes/sun/security/util/DerValue.java Wed Mar 15 23:09:18 2017 +0100
+++ b/jdk/src/java.base/share/classes/sun/security/util/DerValue.java Wed Mar 15 22:57:48 2017 +0000
@@ -1,5 +1,5 @@
-/*
- * Copyright (c) 1996, 2016, Oracle and/or its affiliates. All rights reserved.
+/**
+ * Copyright (c) 1996, 2017, 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
@@ -225,6 +225,16 @@
data = init(stringTag, value);
}
+ // Creates a DerValue from a tag and some DER-encoded data w/ additional
+ // arg to control whether DER checks are enforced.
+ DerValue(byte tag, byte[] data, boolean allowBER) {
+ this.tag = tag;
+ buffer = new DerInputBuffer(data.clone(), allowBER);
+ length = data.length;
+ this.data = new DerInputStream(buffer);
+ this.data.mark(Integer.MAX_VALUE);
+ }
+
/**
* Creates a DerValue from a tag and some DER-encoded data.
*
@@ -232,20 +242,16 @@
* @param data the DER-encoded data
*/
public DerValue(byte tag, byte[] data) {
- this.tag = tag;
- buffer = new DerInputBuffer(data.clone());
- length = data.length;
- this.data = new DerInputStream(buffer);
- this.data.mark(Integer.MAX_VALUE);
+ this(tag, data, true);
}
/*
* package private
*/
DerValue(DerInputBuffer in) throws IOException {
+
// XXX must also parse BER-encoded constructed
// values such as sequences, sets...
-
tag = (byte)in.read();
byte lenByte = (byte)in.read();
length = DerInputStream.getLength(lenByte, in);
@@ -260,7 +266,7 @@
dis.readFully(indefData, offset, readLen);
dis.close();
DerIndefLenConverter derIn = new DerIndefLenConverter();
- inbuf = new DerInputBuffer(derIn.convert(indefData));
+ inbuf = new DerInputBuffer(derIn.convert(indefData), in.allowBER);
if (tag != inbuf.read())
throw new IOException
("Indefinite length encoding not supported");
@@ -282,6 +288,12 @@
}
}
+ // Get an ASN.1/DER encoded datum from a buffer w/ additional
+ // arg to control whether DER checks are enforced.
+ DerValue(byte[] buf, boolean allowBER) throws IOException {
+ data = init(true, new ByteArrayInputStream(buf), allowBER);
+ }
+
/**
* Get an ASN.1/DER encoded datum from a buffer. The
* entire buffer must hold exactly one datum, including
@@ -290,7 +302,14 @@
* @param buf buffer holding a single DER-encoded datum.
*/
public DerValue(byte[] buf) throws IOException {
- data = init(true, new ByteArrayInputStream(buf));
+ this(buf, true);
+ }
+
+ // Get an ASN.1/DER encoded datum from part of a buffer w/ additional
+ // arg to control whether DER checks are enforced.
+ DerValue(byte[] buf, int offset, int len, boolean allowBER)
+ throws IOException {
+ data = init(true, new ByteArrayInputStream(buf, offset, len), allowBER);
}
/**
@@ -303,7 +322,13 @@
* @param len how many bytes are in the encoded datum
*/
public DerValue(byte[] buf, int offset, int len) throws IOException {
- data = init(true, new ByteArrayInputStream(buf, offset, len));
+ this(buf, offset, len, true);
+ }
+
+ // Get an ASN1/DER encoded datum from an input stream w/ additional
+ // arg to control whether DER checks are enforced.
+ DerValue(InputStream in, boolean allowBER) throws IOException {
+ data = init(false, in, allowBER);
}
/**
@@ -316,10 +341,11 @@
* which may be followed by additional data
*/
public DerValue(InputStream in) throws IOException {
- data = init(false, in);
+ this(in, true);
}
- private DerInputStream init(byte stringTag, String value) throws IOException {
+ private DerInputStream init(byte stringTag, String value)
+ throws IOException {
String enc = null;
tag = stringTag;
@@ -347,7 +373,7 @@
byte[] buf = value.getBytes(enc);
length = buf.length;
- buffer = new DerInputBuffer(buf);
+ buffer = new DerInputBuffer(buf, true);
DerInputStream result = new DerInputStream(buffer);
result.mark(Integer.MAX_VALUE);
return result;
@@ -356,8 +382,8 @@
/*
* helper routine
*/
- private DerInputStream init(boolean fullyBuffered, InputStream in)
- throws IOException {
+ private DerInputStream init(boolean fullyBuffered, InputStream in,
+ boolean allowBER) throws IOException {
tag = (byte)in.read();
byte lenByte = (byte)in.read();
@@ -384,7 +410,7 @@
byte[] bytes = IOUtils.readFully(in, length, true);
- buffer = new DerInputBuffer(bytes);
+ buffer = new DerInputBuffer(bytes, allowBER);
return new DerInputStream(buffer);
}
@@ -479,7 +505,8 @@
if (buffer.read(bytes) != length)
throw new IOException("short read on DerValue buffer");
if (isConstructed()) {
- DerInputStream in = new DerInputStream(bytes);
+ DerInputStream in = new DerInputStream(bytes, 0, bytes.length,
+ buffer.allowBER);
bytes = null;
while (in.available() != 0) {
bytes = append(bytes, in.getOctetString());
--- a/jdk/test/sun/security/pkcs/pkcs8/PKCS8Test.java Wed Mar 15 23:09:18 2017 +0100
+++ b/jdk/test/sun/security/pkcs/pkcs8/PKCS8Test.java Wed Mar 15 22:57:48 2017 +0000
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2017, 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
@@ -195,12 +195,12 @@
public static void main(String[] args)
throws IOException, InvalidKeyException {
- BigInteger p = BigInteger.valueOf(1);
- BigInteger q = BigInteger.valueOf(2);
- BigInteger g = BigInteger.valueOf(3);
- BigInteger x = BigInteger.valueOf(4);
+ BigInteger x = BigInteger.valueOf(1);
+ BigInteger p = BigInteger.valueOf(2);
+ BigInteger q = BigInteger.valueOf(3);
+ BigInteger g = BigInteger.valueOf(4);
- DSAPrivateKey priv = new DSAPrivateKey(p, q, g, x);
+ DSAPrivateKey priv = new DSAPrivateKey(x, p, q, g);
byte[] encodedKey = priv.getEncoded();
byte[] expectedBytes = new byte[EXPECTED.length];
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/sun/security/pkcs/pkcs8/TestLeadingZeros.java Wed Mar 15 22:57:48 2017 +0000
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2017, 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 8175251
+ * @summary ensure that PKCS8-encoded private key with leading 0s
+ * can be loaded.
+ * @run main TestLeadingZeros
+ */
+
+import java.io.*;
+import java.security.*;
+import java.security.spec.PKCS8EncodedKeySpec;
+import java.security.interfaces.*;
+import java.util.*;
+
+public class TestLeadingZeros {
+
+ // The following test vectors are various BER encoded PKCS8 bytes
+ static final String[] PKCS8_ENCODINGS = {
+ // first is the original one from PKCS8Test
+ "301e020100301206052b0e03020c30090201020201030201040403020101A000",
+ // changed original to version w/ 1 leading 0
+ "301f02020000301206052b0e03020c30090201020201030201040403020101A000",
+ // changed original to P w/ 1 leading 0
+ "301f020100301306052b0e03020c300a020200020201030201040403020101A000",
+ // changed original to X w/ 2 leading 0s
+ "3020020100301206052b0e03020c300902010202010302010404050203000001A000"
+ };
+
+ public static void main(String[] argv) throws Exception {
+ KeyFactory factory = KeyFactory.getInstance("DSA", "SUN");
+
+ for (String encodings : PKCS8_ENCODINGS) {
+ byte[] encodingBytes = hexToBytes(encodings);
+ PKCS8EncodedKeySpec encodedKeySpec =
+ new PKCS8EncodedKeySpec(encodingBytes);
+ DSAPrivateKey privKey2 = (DSAPrivateKey)
+ factory.generatePrivate(encodedKeySpec);
+ System.out.println("key: " + privKey2);
+ }
+ System.out.println("Test Passed");
+ }
+
+ private static byte[] hexToBytes(String hex) {
+ if (hex.length() % 2 != 0) {
+ throw new RuntimeException("Input should be even length");
+ }
+ int size = hex.length() / 2;
+ byte[] result = new byte[size];
+ for (int i = 0; i < size; i++) {
+ int hi = Character.digit(hex.charAt(2 * i), 16);
+ int lo = Character.digit(hex.charAt(2 * i + 1), 16);
+ if ((hi == -1) || (lo == -1)) {
+ throw new RuntimeException("Input should be hexadecimal");
+ }
+ result[i] = (byte) (16 * hi + lo);
+ }
+ return result;
+ }
+}