On 05/01/2014 10:12 AM, Xuelei Fan wrote:
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
Yes, I agree and will change that.
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.
Ok, I moved those checks to the start of the method.
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?
I do not know but I am reluctant to remove it in case I break something.
The code is not commented very well.
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.
I'm not quite following you here. System.arrayCopy doesn't resize the
array, it will throw an exception if the dest array is not big enough.
Can you clarify what you mean or show me the corrected code?
Thanks,
Sean
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
