RFR 8171319: keytool should print out warnings when reading or generating cert/cert req using weak algorithms

[Seán Coffey]

Looks good to me.

regards,
Sean.


On 03/03/2017 01:17, Weijun Wang wrote:
> Updated again at
>
>    http://cr.openjdk.java.net/~weijun/8171319/webrev.03/
>
> Changes:
>
> 1. No more prompt when algorithm/key is weak in keytool -importcert. 
> This newly introduced prompt might hang a user script. Since we are 
> thinking of backporting this change to previous releases, this is a 
> little too dangerous.
>
> 2. WeakAlg.java test updated.
>
> 3. Other changes (that add a -noprompt) reverted.
>
> Thanks
> Max
>
> On 02/23/2017 11:42 AM, Weijun Wang wrote:
>> Updated again at
>>
>>    http://cr.openjdk.java.net/~weijun/8171319/webrev.02/
>>
>> Changes:
>>
>> 1. Combined printWeakWarningsWithoutNewLine() and printWeakWarnings() to
>> one method.
>>
>> 2. New X509CRLImpl.toStringWithAlgName(String name).
>>
>> Nothing more.
>>
>> Thanks
>> Max
>>
>> On 02/15/2017 11:04 PM, Seán Coffey wrote:
>>> Hi Weijun,
>>>
>>> That's looks good to me and will be a big help for keytool usability.
>>>
>>> some thoughts :
>>>
>>> Main.java : in your printCRL method, would you consider editing the
>>> X509CRLImpl class to print with a customized string ? It'll make the
>>> code more resilient to future changes in this area
>>>
>>> i.e. something like this in X509CRLImpl :
>>>
>>>  public String toString() { printCRL(null); }
>>>  public String printCRL(String custom) {
>>>    // transfer the toString() code to here
>>>    // and allow for 'custom' string to be injected if non-null
>>>    ..
>>>   }
>>>
>>> in Main.java, I'd suggest an instanceof check for X509CRLImpl before
>>> calling printCRL(..). Could X509CRL.getSigAlgName() then be used for
>>> passing into the withWeak method call ?
>>>
>>> ===
>>>
>>> Also in Main.java, maybe you could reduce
>>> printWeakWarningsWithoutNewLine and printWeakWarnings() to one method -
>>> e.g. printWeakWarnings(boolean newline)
>>>
>>> ===
>>>
>>> Regards,
>>> Sean.
>>>
>>> On 15/02/17 01:16, Weijun Wang wrote:
>>>> Ping again.
>>>>
>>>> Also, must we resolve this one before ZBB?
>>>>
>>>> --Max
>>>>
>>>> On 02/09/2017 10:26 AM, Weijun Wang wrote:
>>>>> An update webrev is at
>>>>>
>>>>>    http://cr.openjdk.java.net/~weijun/8171319/webrev.01/
>>>>>
>>>>> The major change is that every risk warning has a owner now, i.e.
>>>>> instead of just saying "MD5withRSA is weak", it prints out whose
>>>>> algorithm is weak. For example:
>>>>>
>>>>>    The generated CRL uses the MD5withRSA signature algorithm which is
>>>>> considered a security risk.
>>>>>
>>>>> Please take a look at the output of the newly added regression 
>>>>> test at
>>>>>
>>>>> http://cr.openjdk.java.net/~weijun/8171319/webrev.01/examples.txt
>>>>>
>>>>> Thanks
>>>>> Max
>>>>>
>>>>> On 01/23/2017 06:02 PM, Weijun Wang wrote:
>>>>>> Hi All
>>>>>>
>>>>>> Please take a review at
>>>>>>
>>>>>>    http://cr.openjdk.java.net/~weijun/8171319/webrev.00/
>>>>>>
>>>>>> Warnings are printed to System.err when weak algorithms/keysizes are
>>>>>> detected during the execution, this includes input, output, and any
>>>>>> certs used.
>>>>>>
>>>>>> The detection applies to many keytool functions:
>>>>>>
>>>>>> - generation of certificate, certificate request, CRL
>>>>>> - reading (printing, listing, exporting) of above
>>>>>> - importing of certificate or certificates reply
>>>>>>
>>>>>> The behavior of most functions remains unchanged. The only
>>>>>> exception is
>>>>>> "keytool -importcert", where the user must reply to a prompt if weak
>>>>>> algorithms/keysizes are detected, unless -noprompt is specified 
>>>>>> on the
>>>>>> command line.
>>>>>>
>>>>>> Warnings are either printed at the end, or before a prompt.
>>>>>>
>>>>>> If there are multiple weak points, multiple warnings will be 
>>>>>> printed.
>>>>>>
>>>>>> The detection is based on the security property
>>>>>> jdk.certpath.disabledAlgorithms.
>>>>>>
>>>>>> For example:
>>>>>>
>>>>>> $ keytool -genkeypair -alias a -dname CN=a -keyalg RSA -sigalg
>>>>>> MD5withRSA
>>>>>>
>>>>>> Warning:
>>>>>> The MD5withRSA signature algorithm is considered a security risk.
>>>>>>
>>>>>> $ keytool -keystore ks -storepass changeit -keypass changeit -list
>>>>>>
>>>>>> Keystore type: JKS
>>>>>> Keystore provider: SUN
>>>>>>
>>>>>> Your keystore contains 3 entries
>>>>>>
>>>>>> b, Jan 23, 2017, PrivateKeyEntry,
>>>>>> Certificate fingerprint (SHA-256):
>>>>>> D8:46:B7:0B:8B:97:C2:DE:A2:17:62:01:27:82:2B:CE:B1:9B:12:0B:24:D5:47:BF:BD:54:EE:8A:71:29:2B:CE 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> a, Jan 23, 2017, PrivateKeyEntry,
>>>>>> Certificate fingerprint (SHA-256):
>>>>>> 66:70:DF:11:14:A1:96:58:92:F5:6A:10:09:B1:2F:CC:1C:CC:2D:55:47:1D:EE:74:75:AA:26:63:E4:9D:EA:83 
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Warning:
>>>>>> <b>'s 512-bit RSA key is considered a security risk.
>>>>>> <a>'s MD5withRSA signature algorithm is considered a security risk.
>>>>>>
>>>>>> $ keytool -importcert -alias a -file b+a.certs
>>>>>>
>>>>>> Warning:
>>>>>> Reply #2 of 2's 512-bit RSA key is considered a security risk.
>>>>>>
>>>>>> Install reply anyway? [no]:no
>>>>>> Certificate reply was not installed in keystore
>>>>>>
>>>>>> Thanks
>>>>>> Max
>>>