Mercurial > hg > icedtea7-forest > jdk
changeset 9231:d4e4af750774
8175251: Failed to load RSA private key from pkcs12
Summary: Enhanced DER library with extra arg to control leading-0 check
Reviewed-by: mullan
| author | igerasim |
|---|---|
| date | Tue, 18 Jul 2017 06:14:47 +0100 |
| parents | f6daf5edd956 |
| children | c54643d2a32b |
| files | src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java src/share/classes/sun/security/util/DerInputBuffer.java src/share/classes/sun/security/util/DerInputStream.java src/share/classes/sun/security/util/DerValue.java test/sun/security/pkcs/pkcs8/PKCS8Test.java test/sun/security/pkcs/pkcs8/TestLeadingZeros.java |
| diffstat | 7 files changed, 199 insertions(+), 91 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java Thu Apr 21 17:08:50 2016 +0300 +++ b/src/share/classes/sun/security/rsa/RSAPrivateCrtKeyImpl.java Tue Jul 18 06:14:47 2017 +0100 @@ -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/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java Thu Apr 21 17:08:50 2016 +0300 +++ b/src/share/classes/sun/security/rsa/RSAPublicKeyImpl.java Tue Jul 18 06:14:47 2017 +0100 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2008, 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 @@ -110,8 +110,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/src/share/classes/sun/security/util/DerInputBuffer.java Thu Apr 21 17:08:50 2016 +0300 +++ b/src/share/classes/sun/security/util/DerInputBuffer.java Tue Jul 18 06:14:47 2017 +0100 @@ -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/src/share/classes/sun/security/util/DerInputStream.java Thu Apr 21 17:08:50 2016 +0300 +++ b/src/share/classes/sun/security/util/DerInputStream.java Tue Jul 18 06:14:47 2017 +0100 @@ -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) { @@ -393,7 +392,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"); @@ -421,7 +421,7 @@ DerValue value; do { - value = new DerValue(newstr.buffer); + value = new DerValue(newstr.buffer, buffer.allowBER); vec.addElement(value); } while (newstr.available() > 0);
--- a/src/share/classes/sun/security/util/DerValue.java Thu Apr 21 17:08:50 2016 +0300 +++ b/src/share/classes/sun/security/util/DerValue.java Tue Jul 18 06:14:47 2017 +0100 @@ -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 @@ 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 @@ * @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 @@ 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 @@ } } + // 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 @@ * @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 @@ * @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 @@ * 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 @@ 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 @@ /* * 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 @@ byte[] bytes = IOUtils.readFully(in, length, true); - buffer = new DerInputBuffer(bytes); + buffer = new DerInputBuffer(bytes, allowBER); return new DerInputStream(buffer); } @@ -480,7 +506,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/test/sun/security/pkcs/pkcs8/PKCS8Test.java Thu Apr 21 17:08:50 2016 +0300 +++ b/test/sun/security/pkcs/pkcs8/PKCS8Test.java Tue Jul 18 06:14:47 2017 +0100 @@ -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 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/test/sun/security/pkcs/pkcs8/TestLeadingZeros.java Tue Jul 18 06:14:47 2017 +0100 @@ -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; + } +}