RFR 8182999: SunEC throws ProviderException on invalid curves

Michael StJohns mstjohns at comcast.net
Mon Jul 10 16:17:30 UTC 2017


Actually - wouldn't it make a lot more sense to generalize the provider 
so it can take ANY set of curve data?  Locking this to only what has an 
OID to parameters mapping doesn't seem to be actually meeting the 
contract for an EC key generator.

I understand a number of tools (e.g. PKIX related/keytool) can't be used 
without the OID, but this isn't at that level.

The webrev feels more like a bandaid than a solution.

Mike


On 7/10/2017 12:03 PM, Seán Coffey wrote:
> Thanks for the update! Looks fine to me.
>
> Regards,
> Sean.
>
> On 10/07/17 16:13, Adam Petcher wrote:
>> New webrev: http://cr.openjdk.java.net/~apetcher/8182999/webrev.01/
>>
>> Yes, this is a good idea. I made this work by printing out the value 
>> from AlgorithmParameters.toString(), so hopefully that means you 
>> should always get a useful string. At the moment (with SunEC 
>> AlgorithmParameters), the string prints the friendly name followed by 
>> the OID:
>>
>> Unsupported curve: brainpoolP256r1 (1.3.36.3.3.2.8.1.1.7)
>>
>> On 7/7/2017 4:12 PM, Seán Coffey wrote:
>>> Adam,
>>>
>>> would it be useful to get the curve name in the new exception ? I 
>>> think it would help with future debugging. Line 96 already gets the 
>>> curve name if we're dealing with ECGenParameterSpec instance. I 
>>> think the same approach could be applied to your new code.
>>>
>>> Regards,
>>> Sean.
>>>
>>>
>>> On 07/07/2017 19:59, Adam Petcher wrote:
>>>> This is a bug fix related to invalid curves in the SunEC provider. 
>>>> During ECKeyPairGenerator.initialize(), the provider only checks 
>>>> whether the curve is known, but it doesn't check whether the curve 
>>>> is actually supported by the native code. So the call to 
>>>> generateKeyPair() can fail in the native code and throw a 
>>>> ProviderException. This change adds a new native method to check 
>>>> whether the curve is supported. This method is called by 
>>>> initialize(), which will set the state to uninitialized and throw 
>>>> the expected exception when the curve is not supported.
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8182999
>>>> Webrev: http://cr.openjdk.java.net/~apetcher/8182999/webrev.00/
>>>>
>>>
>>
>



More information about the security-dev mailing list