Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-12 Thread Vincent Ryan
+1 > On 12 Jul 2017, at 15:51, Adam Petcher wrote: > > I made a minor tweak to the test. I realized that the test will still pass if > the curve becomes supported in the future. I want the test to fail in this > case because it would no longer be testing an

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-12 Thread Adam Petcher
I made a minor tweak to the test. I realized that the test will still pass if the curve becomes supported in the future. I want the test to fail in this case because it would no longer be testing an unsupported curve. latest webrev: http://cr.openjdk.java.net/~apetcher/8182999/webrev.02/ On

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-12 Thread Vincent Ryan
Looks fine to me too. We should investigate how best to support similar behaviour for the SunPKCS11 provider. To track this issue I’ve filed a related bug 8184290 : SunPKCS11 throws ProviderException for unsupported curves > On 10 Jul 2017,

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Adam Petcher
I created JDK-8184122[1] to track this SunEC refactoring effort. [1] https://bugs.openjdk.java.net/browse/JDK-8184122 On 7/10/2017 2:51 PM, Michael StJohns wrote: (Hmm.. have you checked to make sure that the list of curves in ecdecode is a subset of the curves in CurveDB - if not you may

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Michael StJohns
On 7/10/2017 2:28 PM, Adam Petcher wrote: On 7/10/2017 1:55 PM, Michael StJohns wrote: What I'm mostly trying to get at here is to decouple or remove the list of curves in ecdecode.c in favor of the list in the java stuff (CurveDB.java) (or elsewhere). The C code should mostly only have

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Adam Petcher
On 7/10/2017 1:55 PM, Michael StJohns wrote: What I'm mostly trying to get at here is to decouple or remove the list of curves in ecdecode.c in favor of the list in the java stuff (CurveDB.java) (or elsewhere). The C code should mostly only have to deal with the math and not the

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Michael StJohns
On 7/10/2017 12:59 PM, Adam Petcher wrote: This fix addresses an issue in which the provider behaves incorrectly when initialized with parameters for a curve that is not supported by the provider. If I am interpreting your suggestion correctly, it sounds like you are requesting a change to the

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Adam Petcher
This fix addresses an issue in which the provider behaves incorrectly when initialized with parameters for a curve that is not supported by the provider. If I am interpreting your suggestion correctly, it sounds like you are requesting a change to the set of curves that is supported by the

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Michael StJohns
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

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Seán Coffey
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

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-10 Thread Adam Petcher
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

Re: RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-07 Thread Seán Coffey
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

RFR 8182999: SunEC throws ProviderException on invalid curves

2017-07-07 Thread Adam Petcher
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