diff --git a/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java b/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java index 7e68c427e52f91cb5fbb47122aff28a7c516ae86..cb5e53579bb744e07305d54b88e91276b1a700ec 100644 --- a/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java +++ b/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java @@ -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 @@ public final class RSAPrivateCrtKeyImpl 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 @@ public final class RSAPrivateCrtKeyImpl 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; - } } diff --git a/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java b/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java index 52c0d6718d7227ee2a479bf8b8e12196de84cf65..5f2afac4df9fde0a942904241072a5b9c1f72f40 100644 --- a/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java +++ b/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java @@ -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 @@ public final class RSAPublicKeyImpl extends X509Key implements RSAPublicKey { 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"); } diff --git a/src/share/classes/sun/security/util/DerInputBuffer.java b/src/share/classes/sun/security/util/DerInputBuffer.java index 7d3ad55401e4bb3895babb2a44e5d5f156d3ff1b..b09b7b3041108c2382e22320893882165aeef6b6 100644 --- a/src/share/classes/sun/security/util/DerInputBuffer.java +++ b/src/share/classes/sun/security/util/DerInputBuffer.java @@ -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 @@ import sun.util.calendar.CalendarSystem; */ class DerInputBuffer extends ByteArrayInputStream implements Cloneable { - DerInputBuffer(byte[] buf) { super(buf); } + boolean allowBER = true; - DerInputBuffer(byte[] buf, int offset, int len) { + // used by sun/security/util/DerInputBuffer/DerInputBufferEqualsHashCode.java + DerInputBuffer(byte[] buf) { + this(buf, true); + } + + 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 @@ class DerInputBuffer extends ByteArrayInputStream implements Cloneable { 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"); } diff --git a/src/share/classes/sun/security/util/DerInputStream.java b/src/share/classes/sun/security/util/DerInputStream.java index 5af3cf428c8e8b168532966ffc7d1029f5b0829c..f513c93c4a3c85670ef43a28e37de296fc6895a4 100644 --- a/src/share/classes/sun/security/util/DerInputStream.java +++ b/src/share/classes/sun/security/util/DerInputStream.java @@ -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,7 +81,8 @@ public class DerInputStream { } /** - * Create a DER input stream from part of a data buffer. + * 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. * @@ -90,15 +91,16 @@ public class DerInputStream { * 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) throws IOException { - init(data, offset, len, true); + 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 with - * additional arg to indicate whether to allow constructed - * indefinite-length encoding. + * 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. * @@ -107,35 +109,32 @@ public class DerInputStream { * 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); + public DerInputStream(byte[] data, int offset, int len) throws IOException { + init(data, offset, len, true); } /* * 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 class DerInputStream { */ public DerInputStream subStream(int len, boolean do_skip) throws IOException { - DerInputBuffer newbuf = buffer.dup(); + DerInputBuffer newbuf = buffer.dup(); newbuf.truncate(len); if (do_skip) { @@ -393,7 +392,8 @@ public class DerInputStream { 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"); @@ -421,7 +421,7 @@ public class DerInputStream { DerValue value; do { - value = new DerValue(newstr.buffer); + value = new DerValue(newstr.buffer, buffer.allowBER); vec.addElement(value); } while (newstr.available() > 0); diff --git a/src/share/classes/sun/security/util/DerValue.java b/src/share/classes/sun/security/util/DerValue.java index 8cd989b6b682f3fd76906f9545c40beb09670af6..7f4eb3ac053f9923cde8bb60da38b35555929b6f 100644 --- a/src/share/classes/sun/security/util/DerValue.java +++ b/src/share/classes/sun/security/util/DerValue.java @@ -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 @@ -226,6 +226,16 @@ public class DerValue { 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. * @@ -233,20 +243,16 @@ public class DerValue { * @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); @@ -261,7 +267,7 @@ public class DerValue { 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"); @@ -283,6 +289,12 @@ public class DerValue { } } + // 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 @@ -291,7 +303,14 @@ public class DerValue { * @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); } /** @@ -304,7 +323,13 @@ public class DerValue { * @param length 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); } /** @@ -317,10 +342,11 @@ public class DerValue { * 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; @@ -348,7 +374,7 @@ public class DerValue { 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; @@ -357,8 +383,8 @@ public class DerValue { /* * 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(); @@ -385,7 +411,7 @@ public class DerValue { byte[] bytes = IOUtils.readFully(in, length, true); - buffer = new DerInputBuffer(bytes); + buffer = new DerInputBuffer(bytes, allowBER); return new DerInputStream(buffer); } @@ -480,7 +506,8 @@ public class DerValue { 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()); diff --git a/test/sun/security/pkcs/pkcs8/PKCS8Test.java b/test/sun/security/pkcs/pkcs8/PKCS8Test.java index 6bd3af5b96c98ce2ee39166000a7ceec96a6b49d..af40667efa81a2d33a6fb53b5902fa39d21806d5 100644 --- a/test/sun/security/pkcs/pkcs8/PKCS8Test.java +++ b/test/sun/security/pkcs/pkcs8/PKCS8Test.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2016, 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 @@ -191,12 +191,12 @@ public class PKCS8Test { 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]; diff --git a/test/sun/security/pkcs/pkcs8/TestLeadingZeros.java b/test/sun/security/pkcs/pkcs8/TestLeadingZeros.java new file mode 100644 index 0000000000000000000000000000000000000000..92a204501571d7e8b2e943a74c55570be4269977 --- /dev/null +++ b/test/sun/security/pkcs/pkcs8/TestLeadingZeros.java @@ -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; + } +}