Hi Sean,
Thank you for your feedback.
It was confusing to me that the impl supports indefinite-length encoding
for DER. According to [1], indefinite-length method shall be used for DER:
...
10.1
Length forms
The definite form of length encoding shall be used, encoded in the
minimum number of octets. [Contrast with 8.1.3.2 b).]
...
But then I found a couple of bugs for support of indefinite-length (for
example [2]). Probably it is needed for real applications.
I updated the diff:
- added getDefiniteLength() methods that throw IOException in case of
indefinite-length encoding
- getLength() method, which can return a negative value, is used to
decode sequences, sets in DerInputStream
- getLength() method is also used in constructor and init() method of
DerValue class that check for indefinite-length encoding
Tested with available regression, JCK and SQE tests.
Please take a look:
http://cr.openjdk.java.net/~asmotrak/8028591/webrev.01/
[1] Information technology – ASN.1 encoding rules: Specification of
Basic Encoding Rules (BER), Canonical Encoding Rules (CER) and
Distinguished Encoding Rules (DER),
http://www.itu.int/ITU-T/recommendations/rec.aspx?rec=x.690
[2] https://bugs.openjdk.java.net/browse/JDK-4119673: Need to support
indefinite length DER encodings
Artem
On 02/05/2014 06:37 PM, Sean Mullan wrote:
Hi Artem,
The specific fix looks fine, but there are many other calls to
getLength() in DerInputStream that subsequently initialize an array
with the return value, and could also cause the same issue. It seems
to me that a better fix would be to pass a flag to the getLength
method (or create a new method) and if the flag is true, throw an
IOException if an indefinite length encoding is used (instead of
returning -1). Then, for the encodings where it is illegal to use the
indefinite-length method, change the code to call the method with the
flag set to true.
--Sean
On 01/30/2014 03:47 AM, Artem Smotrakov wrote:
Please review this fix for 9:
https://bugs.openjdk.java.net/browse/JDK-8028591
http://cr.openjdk.java.net/~asmotrak/8028591/webrev.00/
<http://cr.openjdk.java.net/%7Easmotrak/8028591/webrev.00/>
getLength() method is used to get a length of bit string. The method can
return a negative value that means indefinite-length encoding that is
not allowed in DER. Currently a negative value is not checked. As a
result, NegativeArraySizeException can occur.
I added the following checks in
sun.security.util.DerInputStream.getUnalignedBitString() method:
1. IOException is thrown if getLength() method returns a negative value.
2. Empty BitArray is returned if getLength() method returns zero.
I think that an empty bit string should be encoded as "03 01 00" in DER.
I am not sure, but probably "03 00" is valid one as well. I tried both
ones with OpenSSL asn1parse, and both ones were parsed successfully:
hexdump -C emtpy_bit_string_1
00000000 03 01 00 |...|
00000003
openssl asn1parse -inform der -in emtpy_bit_string_1
0:d=0 hl=2 l= 1 prim: BIT STRING
hexdump -C emtpy_bit_string_2
00000000 03 00 |..|
00000002
openssl asn1parse -inform der -in emtpy_bit_string_2
0:d=0 hl=2 l= 0 prim: BIT STRING
3. IOException is thrown if number of calculated valid bits is negative.
Added a test case for
test/java/security/cert/X509Certificate/X509BadCertificate.java
(bad-cert-2.pem is corrupted self-signed certificate). Tested with
available regression, SQE and JCK tests.
Artem
