diff --git a/src/share/classes/sun/security/ec/ECDSASignature.java b/src/share/classes/sun/security/ec/ECDSASignature.java index 8de129b85d358c246d395bfb7536ebb7aa257449..2abc262809aa2dca01fae5b4d02ddb5b6bb4afa7 100644 --- a/src/share/classes/sun/security/ec/ECDSASignature.java +++ b/src/share/classes/sun/security/ec/ECDSASignature.java @@ -364,18 +364,22 @@ abstract class ECDSASignature extends SignatureSpi { } // Convert the DER encoding of R and S into a concatenation of R and S - private byte[] decodeSignature(byte[] signature) throws SignatureException { + private byte[] decodeSignature(byte[] sig) throws SignatureException { try { - DerInputStream in = new DerInputStream(signature); + // Enforce strict DER checking for signatures + DerInputStream in = new DerInputStream(sig, 0, sig.length, false); DerValue[] values = in.getSequence(2); + + // check number of components in the read sequence + // and trailing data + if ((values.length != 2) || (in.available() != 0)) { + throw new IOException("Invalid encoding for signature"); + } + BigInteger r = values[0].getPositiveBigInteger(); BigInteger s = values[1].getPositiveBigInteger(); - // Check for trailing signature data - if (in.available() != 0) { - throw new IOException("Incorrect signature length"); - } // trim leading zeroes byte[] rBytes = trimZeroes(r.toByteArray()); byte[] sBytes = trimZeroes(s.toByteArray()); @@ -389,7 +393,7 @@ abstract class ECDSASignature extends SignatureSpi { return result; } catch (Exception e) { - throw new SignatureException("Could not decode signature", e); + throw new SignatureException("Invalid encoding for signature", e); } } diff --git a/src/share/classes/sun/security/pkcs11/P11Signature.java b/src/share/classes/sun/security/pkcs11/P11Signature.java index 8007027ce0f574d1731f2e74249efdb8f5f2f338..0809ee45f01a99980632471ea2817a1a15cfa745 100644 --- a/src/share/classes/sun/security/pkcs11/P11Signature.java +++ b/src/share/classes/sun/security/pkcs11/P11Signature.java @@ -705,17 +705,21 @@ final class P11Signature extends SignatureSpi { } } - private static byte[] asn1ToDSA(byte[] signature) throws SignatureException { + private static byte[] asn1ToDSA(byte[] sig) throws SignatureException { try { - DerInputStream in = new DerInputStream(signature); + // Enforce strict DER checking for signatures + DerInputStream in = new DerInputStream(sig, 0, sig.length, false); DerValue[] values = in.getSequence(2); + + // check number of components in the read sequence + // and trailing data + if ((values.length != 2) || (in.available() != 0)) { + throw new IOException("Invalid encoding for signature"); + } + BigInteger r = values[0].getPositiveBigInteger(); BigInteger s = values[1].getPositiveBigInteger(); - // Check for trailing signature data - if (in.available() != 0) { - throw new IOException("Incorrect signature length"); - } byte[] br = toByteArray(r, 20); byte[] bs = toByteArray(s, 20); if ((br == null) || (bs == null)) { @@ -725,21 +729,25 @@ final class P11Signature extends SignatureSpi { } catch (SignatureException e) { throw e; } catch (Exception e) { - throw new SignatureException("invalid encoding for signature", e); + throw new SignatureException("Invalid encoding for signature", e); } } - private byte[] asn1ToECDSA(byte[] signature) throws SignatureException { + private byte[] asn1ToECDSA(byte[] sig) throws SignatureException { try { - DerInputStream in = new DerInputStream(signature); + // Enforce strict DER checking for signatures + DerInputStream in = new DerInputStream(sig, 0, sig.length, false); DerValue[] values = in.getSequence(2); + + // check number of components in the read sequence + // and trailing data + if ((values.length != 2) || (in.available() != 0)) { + throw new IOException("Invalid encoding for signature"); + } + BigInteger r = values[0].getPositiveBigInteger(); BigInteger s = values[1].getPositiveBigInteger(); - // Check for trailing signature data - if (in.available() != 0) { - throw new IOException("Incorrect signature length"); - } // trim leading zeroes byte[] br = KeyUtil.trimZeroes(r.toByteArray()); byte[] bs = KeyUtil.trimZeroes(s.toByteArray()); @@ -750,7 +758,7 @@ final class P11Signature extends SignatureSpi { System.arraycopy(bs, 0, res, res.length - bs.length, bs.length); return res; } catch (Exception e) { - throw new SignatureException("invalid encoding for signature", e); + throw new SignatureException("Invalid encoding for signature", e); } } diff --git a/src/share/classes/sun/security/provider/DSA.java b/src/share/classes/sun/security/provider/DSA.java index 2b42286e968c2b84d01cf49d0b1b8147704d6c66..3a13e910d60baff6e5e56186ef47d8b332ebc4dd 100644 --- a/src/share/classes/sun/security/provider/DSA.java +++ b/src/share/classes/sun/security/provider/DSA.java @@ -267,18 +267,20 @@ abstract class DSA extends SignatureSpi { BigInteger s = null; // first decode the signature. try { - DerInputStream in = new DerInputStream(signature, offset, length); + // Enforce strict DER checking for signatures + DerInputStream in = + new DerInputStream(signature, offset, length, false); DerValue[] values = in.getSequence(2); + // check number of components in the read sequence + // and trailing data + if ((values.length != 2) || (in.available() != 0)) { + throw new IOException("Invalid encoding for signature"); + } r = values[0].getBigInteger(); s = values[1].getBigInteger(); - - // Check for trailing signature data - if (in.available() != 0) { - throw new IOException("Incorrect signature length"); - } } catch (IOException e) { - throw new SignatureException("invalid encoding for signature"); + throw new SignatureException("Invalid encoding for signature", e); } // some implementations do not correctly encode values in the ASN.1 diff --git a/src/share/classes/sun/security/rsa/RSASignature.java b/src/share/classes/sun/security/rsa/RSASignature.java index f1572f7a57d8865c3055c5ed05cd58fe7f7bc565..379d4019322d1f36508debbe391100c07a1a3ea0 100644 --- a/src/share/classes/sun/security/rsa/RSASignature.java +++ b/src/share/classes/sun/security/rsa/RSASignature.java @@ -223,9 +223,10 @@ public abstract class RSASignature extends SignatureSpi { * Decode the signature data. Verify that the object identifier matches * and return the message digest. */ - public static byte[] decodeSignature(ObjectIdentifier oid, byte[] signature) + public static byte[] decodeSignature(ObjectIdentifier oid, byte[] sig) throws IOException { - DerInputStream in = new DerInputStream(signature); + // Enforce strict DER checking for signatures + DerInputStream in = new DerInputStream(sig, 0, sig.length, false); DerValue[] values = in.getSequence(2); if ((values.length != 2) || (in.available() != 0)) { throw new IOException("SEQUENCE length error"); diff --git a/src/share/classes/sun/security/util/DerInputBuffer.java b/src/share/classes/sun/security/util/DerInputBuffer.java index 786e7914bbfe6e440f5e5769e07bfd83e061f543..7d3ad55401e4bb3895babb2a44e5d5f156d3ff1b 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, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2016, 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 @@ -147,6 +147,11 @@ 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)) { + throw new IOException("Invalid encoding: redundant leading 0s"); + } + if (makePositive) { return new BigInteger(1, bytes); } else { diff --git a/src/share/classes/sun/security/util/DerInputStream.java b/src/share/classes/sun/security/util/DerInputStream.java index fc4aee847fae679548c14272aac917472a0ea4cc..5af3cf428c8e8b168532966ffc7d1029f5b0829c 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, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2016, 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 @@ -77,7 +77,7 @@ public class DerInputStream { * @param data the buffer from which to create the string (CONSUMED) */ public DerInputStream(byte[] data) throws IOException { - init(data, 0, data.length); + init(data, 0, data.length, true); } /** @@ -92,23 +92,48 @@ public class DerInputStream { * starting at "offset" */ public DerInputStream(byte[] data, int offset, int len) throws IOException { - init(data, offset, len); + 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 data 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) throws IOException { + private void init(byte[] data, int offset, int len, + boolean allowIndefiniteLength) 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])) { - byte[] inData = new byte[len]; - System.arraycopy(data, offset, inData, 0, len); - - DerIndefLenConverter derIn = new DerIndefLenConverter(); - buffer = new DerInputBuffer(derIn.convert(inData)); + if (!allowIndefiniteLength) { + 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)); + } } else buffer = new DerInputBuffer(data, offset, len); buffer.mark(Integer.MAX_VALUE); @@ -233,12 +258,21 @@ public class DerInputStream { * First byte = number of excess bits in the last octet of the * representation. */ - int validBits = length*8 - buffer.read(); + int excessBits = buffer.read(); + if (excessBits < 0) { + throw new IOException("Unused bits of bit string invalid"); + } + int validBits = length*8 - excessBits; + if (validBits < 0) { + throw new IOException("Valid bits of bit string invalid"); + } byte[] repn = new byte[length]; - if ((length != 0) && (buffer.read(repn) != length)) - throw new IOException("short read of DER bit string"); + if ((length != 0) && (buffer.read(repn) != length)) { + throw new IOException("Short read of DER bit string"); + } + return new BitArray(validBits, repn); } @@ -252,7 +286,7 @@ public class DerInputStream { int length = getLength(buffer); byte[] retval = new byte[length]; if ((length != 0) && (buffer.read(retval) != length)) - throw new IOException("short read of DER octet string"); + throw new IOException("Short read of DER octet string"); return retval; } @@ -262,7 +296,7 @@ public class DerInputStream { */ public void getBytes(byte[] val) throws IOException { if ((val.length != 0) && (buffer.read(val) != val.length)) { - throw new IOException("short read of DER octet string"); + throw new IOException("Short read of DER octet string"); } } @@ -346,7 +380,7 @@ public class DerInputStream { DerInputStream newstr; byte lenByte = (byte)buffer.read(); - int len = getLength((lenByte & 0xff), buffer); + int len = getLength(lenByte, buffer); if (len == -1) { // indefinite length encoding found @@ -392,7 +426,7 @@ public class DerInputStream { } while (newstr.available() > 0); if (newstr.available() != 0) - throw new IOException("extra data at end of vector"); + throw new IOException("Extra data at end of vector"); /* * Now stick them into the array we're returning. @@ -483,7 +517,7 @@ public class DerInputStream { int length = getLength(buffer); byte[] retval = new byte[length]; if ((length != 0) && (buffer.read(retval) != length)) - throw new IOException("short read of DER " + + throw new IOException("Short read of DER " + stringName + " string"); return new String(retval, enc); @@ -544,7 +578,11 @@ public class DerInputStream { */ static int getLength(int lenByte, InputStream in) throws IOException { int value, tmp; + if (lenByte == -1) { + throw new IOException("Short read of DER length"); + } + String mdName = "DerInputStream.getLength(): "; tmp = lenByte; if ((tmp & 0x080) == 0x00) { // short form, 1 byte datum value = tmp; @@ -558,17 +596,23 @@ public class DerInputStream { if (tmp == 0) return -1; if (tmp < 0 || tmp > 4) - throw new IOException("DerInputStream.getLength(): lengthTag=" - + tmp + ", " + throw new IOException(mdName + "lengthTag=" + tmp + ", " + ((tmp < 0) ? "incorrect DER encoding." : "too big.")); - for (value = 0; tmp > 0; tmp --) { + value = 0x0ff & in.read(); + tmp--; + if (value == 0) { + // DER requires length value be encoded in minimum number of bytes + throw new IOException(mdName + "Redundant length bytes found"); + } + while (tmp-- > 0) { value <<= 8; value += 0x0ff & in.read(); } if (value < 0) { - throw new IOException("DerInputStream.getLength(): " - + "Invalid length bytes"); + throw new IOException(mdName + "Invalid length bytes"); + } else if (value <= 127) { + throw new IOException(mdName + "Should use short form for length"); } } return value; diff --git a/src/share/classes/sun/security/util/DerValue.java b/src/share/classes/sun/security/util/DerValue.java index beb51c19ae3feb11c2ec3d4478c2c856ad5573fc..8cd989b6b682f3fd76906f9545c40beb09670af6 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, 2009, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2016, 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 @@ -249,7 +249,7 @@ public class DerValue { tag = (byte)in.read(); byte lenByte = (byte)in.read(); - length = DerInputStream.getLength((lenByte & 0xff), in); + length = DerInputStream.getLength(lenByte, in); if (length == -1) { // indefinite length encoding found DerInputBuffer inbuf = in.dup(); int readLen = inbuf.available(); @@ -362,7 +362,7 @@ public class DerValue { tag = (byte)in.read(); byte lenByte = (byte)in.read(); - length = DerInputStream.getLength((lenByte & 0xff), in); + length = DerInputStream.getLength(lenByte, in); if (length == -1) { // indefinite length encoding found int readLen = in.available(); int offset = 2; // for tag and length bytes