JDK 9 Review Request for 8038349: Signing XML with DSA throws Exception when key is larger than 1024 bits

[Xuelei Fan]

> security/utils/JavaUtils.java
> =============================
The updated code is a straightforward move from other classes in order
to remove the duplicated code.  Looks fine but we might want to make a
few little enhancements within this fix.

162  public static byte[] convertASN1toXMLDSIG(byte asn1Bytes[], ..
201  public static byte[] convertXMLDSIGtoASN1(byte xmldsigBytes[], ..

Minor comment about code conversions.  I may prefer to use consistent
style to declare the array parameters.
    byte asn1Bytes[] -> byte[] asn1Bytes
    byte xmldsigBytes[] -> byte[] xmldsigBytes

convertASN1toXMLDSIG(byte asn1Bytes[], int size):
-------------------------------------------------
I may check the asn1Bytes.length against/and the "size" parameter, and
check first 3 bytes before getting the 4rd byte.  Similarly, I may check
the first byte and the rest length of the array before try to parse the
s (the 2nd fragment of the array) value of the signature.

convertXMLDSIGtoASN1(byte xmldsigBytes[], int size):
----------------------------------------------------
I did not get the idea behind lines 212-215 and lines 220-223.  Why try
to check and ignore the negative value?

225         byte asn1Bytes[] = new byte[6 + j + l];
By the line, the length of the target byte array is known. I might
prefer to allocate the exactly size for byte array here.  Otherwise, the
2nd System.arraycopy() would need re-allocate the array for more space.

Xuelei

> On 4/30/2014 4:48 AM, Sean Mullan wrote:
>> Please review the following change which adds support for 2048-bit DSA
>> keys and the DSA-SHA256 algorithm to the XML Signature implementation:
>>
>> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8038349/webrev.00/
>>
>> JDK 8 already includes the underlying support for both of these in the
>> Sun provider. For 2048 bit keys, the ASN.1 parsing code in the XML
>> Signature layer needed to be adapted to handle 2048 bit keys, and for
>> SHA-256 it was just a matter of registering the algorithm URI and
>> instantiating a Signature object with the SHA256WithDSA algorithm.
>>
>> Thanks,
>> Sean
>