RFR 8056174: New APIs for jar signing

Mike StJohns mstjohns at comcast.net
Tue Sep 29 18:36:22 UTC 2015


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/
>>>
>
>



More information about the security-dev mailing list