[9] RFR 8163503: PKCS12 keystore cannot store non-X.509 certificates

[Sean Mullan]

Looks fine, although I would probably avoid calling checkX509Certs on 
line 970 and just checking the cert right there to avoid creating the 
array which is not needed.

--Sean

On 08/12/2016 11:07 AM, Vincent Ryan wrote:
> I’ve moved the X.509 check to earlier in the code and reverted the changes to the validateChain method.
> Updated webrev is at:
>    http://cr.openjdk.java.net/~vinnie/8163503/webrev.02/
>
>
>
>> On 10 Aug 2016, at 21:15, Sean Mullan <sean.mullan at oracle.com> wrote:
>>
>> On 08/10/2016 12:39 PM, Vincent Ryan wrote:
>>> Yes they could be merged but the first loop iterates over all the certs and the second one iterates over all but the final cert.
>>> And the special case of a 1-cert chain also needs to be handled. I think it’s a little clearer to leave them separate.
>>
>> Agreed, but it's probably better to check these earlier and bail out if they are not all X509Certificates, for example, right at the beginning of engineSetKeyEntry. There is no need to proceed with decoding keys, etc since it is already a violation of the supported API parameters.
>>
>> --Sean
>>
>>>
>>> An updated webrev is at:
>>>  http://cr.openjdk.java.net/~vinnie/8163503/webrev.01/
>>>
>>> Thanks.
>>>
>>>> On 10 Aug 2016, at 02:04, Xuelei Fan <Xuelei.Fan at Oracle.Com> wrote:
>>>>
>>>> The for loop at line 1507 and 1520 may be merged together.
>>>>
>>>> Xuelei
>>>>
>>>> On 8/10/2016 8:38 AM, Weijun Wang wrote:
>>>>> I thought I've seen this webrev before.
>>>>>
>>>>> Why not just throw a KeyStoreException in validateChain()?
>>>>>
>>>>> --Max
>>>>>
>>>>> On 8/10/2016 2:14, Vincent Ryan wrote:
>>>>>> Please review this fix to improve the error handling for attempts to
>>>>>> store a Certificate object in PKCS12 keystore.
>>>>>> The PKCS12 keystore implementation supports storing only
>>>>>> X509Certificate objects but the KeyStore API allows Certificate objects.
>>>>>> This fix rejects attempts to store non-X.509 certificates and throws a
>>>>>> KeyStoreException.
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8163503
>>>>>> Webrev: http://cr.openjdk.java.net/~vinnie/8163503/webrev.00/
>>>>>>
>>>>>>
>>>>
>>>
>