8237219: Disabling the native SunEC implementation
Sean Mullan
sean.mullan at oracle.com
Tue Mar 3 19:18:53 UTC 2020
On 3/3/20 1:42 PM, Anthony Scarpino wrote:
> On 3/3/20 8:34 AM, Sean Mullan wrote:
>> Wouldn't it be better to throw an Exception when you call
>> Signature.initSign/Verify() and KeyAgreement.init() rather than
>> waiting until you sign/verify or generateSecret? This way you bail out
>> early before you start processing data.
>>
>> Also, throwing a ProviderException (a RuntimeException) could be a
>> behavioral change that an application may not be prepared for. We have
>> never done a very good job of documenting when ProviderException can
>> be thrown by the JCE APIs, however. But we should think about this and
>> whether maybe you want to throw InvalidKeyException instead which is
>> already specified in the throws clause of the init methods. In any
>> case it should be documented as a potential compatibility issue in the
>> CSR.
>
>
> Unfortunately curve support decisions are not made until the library is
> accessed. You will notice the checks that call native are not until the
> operation is starting, like engineSign(). If an unsupported curve is
> used in the existing code, this is where the error is generated from.
> The native code does generate an InvalidAlgorithmParameterException. I
> had chose ProviderException because it was a provider support change. I
> had thought about IAPE, but choose a bit more drastic action. That said,
> I'm fine with using IAPE.
Hmm, but engineSign, engineVerify, engineGenerateSecret don't throw IAPE.
I think it should probably be the same behavior as if the library did
not exist. What do we throw now if the library has been removed?
> If one was to remove libsunec, the existing code disables algorithms
> like SHA256withECDSA which makes it easier to separate native from java
> implementations. But that is not true anymore with the java
> implementation supporting the NIST curves. Some of these same decisions
> affect both property disabling and library removal.
>
> In the past we have had similar Signature discussions before about when
> Signature.setParameter() being called before or after init(). I have to
> wonder if the original provider design followed the same logic where
> errors are generated later in the process. Also there are some
> decisions in this provider that look similar to SunPKCS11 as both wait
> until the last moment to ask the native library.
>
>>
>> Did you consider documenting the system property in the API javadocs
>> for Signature, KeyPairGenerator, KeyAgreement? I realize this is
>> specific to the SunEC provider, but this would help users know how to
>> enable the system property (on the hopefully rare case) they get an
>> exception and still want to use one of these curves. It could be
>> something like:
>>
>> @implNote By default, the SunEC provider throws ...Exception if the
>> key is using a legacy curve. Set the {@systemProperty
>> jdk.sunec.disableNative} to {@code false} to disable this behavior.
>>
>
> I don't think putting in the API about one particular provider
> implementation detail is a proper place. I think this belongs in
> release notes, and maybe the provider doc.
Yeah, I am kind of on the fence about that myself as I don't necessarily
want to clutter the APIs with provider-specific stuff, but OTOH, most of
the security-related system properties *are* provider-specific, so we
lose the nice feature of seeing how different system properties affect
existing APIs while you are reading the javadoc. But, I think we
probably need to decide on a more general policy for documenting
provider-specific system properties first. So I guess we can hold off on
this for now.
--Sean
>> --Sean
>>
>> On 3/2/20 7:40 PM, Anthony Scarpino wrote:
>>> Hi
>>>
>>> I need a review of the CSR and webrev for disabling by default the
>>> native SunEC curves from the API. With the recent verification
>>> changes in JDK-8237218, SunJCE is long dependent on the native code
>>> for verifying the constant-time curves. This disabling can be undone
>>> with setting a system property, jdk.sunec.disableNative. I'm doing
>>> a simultaneous review as changes for one will likely affect the other.
>>>
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8238911
>>> webrev: https://cr.openjdk.java.net/~ascarpino/8237219/
>>>
>>> The curves affected are:
>>> secp112r1, secp112r2, secp128r1, secp128r2, secp160k1, secp160r1,
>>> secp160r2, secp192k1, secp192r1, secp224k1, secp224r1, secp256k1,
>>> sect113r1, sect113r2, sect131r1, sect131r2, sect163k1, sect163r1,
>>> sect163r2, sect193r1, sect193r2, sect233k1, sect233r1, sect239k1,
>>> sect283k1, sect283r1, sect409k1, sect409r1, sect571k1, sect571r1,
>>> X9.62 c2tnb191v1, X9.62 c2tnb191v2, X9.62 c2tnb191v3, X9.62
>>> c2tnb239v1, X9.62 c2tnb239v2, X9.62 c2tnb239v3, X9.62 c2tnb359v1,
>>> X9.62 c2tnb431r1, X9.62 prime192v2, X9.62 prime192v3, X9.62
>>> prime239v1, X9.62 prime239v2, X9.62 prime239v3, brainpoolP256r1
>>> brainpoolP320r1, brainpoolP384r1, brainpoolP512r1
>>>
>>> Tony
>
More information about the security-dev
mailing list