Sorry for the late comment here -
On 9/28/2015 7:34 PM, Weijun Wang wrote:
On 9/29/2015 3:27 AM, Sean Mullan wrote:
Looks good, just a couple of comments:
AlgorithmId: can you use braces around the conditional statements on
lines 1008-1017?
OK.
I had a few issues with how this was coded. The first and probably most
important is that you can't/shouldn't pick the default signature
algorithm solely based on the key type, but instead on the key type and
strength.
The second is that even if you don't fix the above, its probably better
to code this as a switch/case grouping rather than a series of
if/then/else items.
So changing this a bit:
public static String getDefaultSigAlg (PrivateKey k) {
switch (k.getAlgorithm.toUpperCase()) {
case "EC":
return
ecStrength(((ECKey)k).getParams().getCurve().getField().getFieldSize())
+ "withECDSA";
case "DSA":
return ifcFfcStrength
(((DSAKey)k).getParams().getP().bitLength()) + "withDSA";
case "RSA":
return ifcFfcStrength
(((RSAKey)k).getModulus().bitLength())+"withRSA";
default:
return null;
}
// Values from SP800-57 part 1 rev 3 tables 2 and three
static String ecStrength (int bitLength) {
if (bitLength > 384) { // 256 bits of strength
return "SHA512";
}else if (bitLength > 256) { // 192 bits of strength
return "SHA384";
} else { // 128 bits of strength and less
return "SHA256";
}
}
// same values for RSA and DSA
static String ifcFfcStrength (int bitLength) {
if (bitLength > 7680) { // 256 bits
return "SHA512";
} else if (bitLength > 3072) { // 192 bits
return "SHA384";
} else { // 128 bits and less
return "SHA256";
}
}
Function: are you missing an @modules tag for the jarsigner module?
Which one? I thought @modules is only used if you want to call
non-exported classes.
Options.java: why not use the JarSigner API here instead of the
jarsigner tool?
This test is to make sure jarsigner still behaves correctly after it
is modified to be based on the JarSigner API. As for the API itself, I
use Function.java to check its function and Spec.java to check it's spec.
I'll add some @summary.
Thanks
Max
--Sean
> http://cr.openjdk.java.net/~weijun/8056174/webrev.05/
