[security-dev 01172]: Re: [PATCH FOR REVIEW]: 6763530: Fix breakage of NSS-based Elliptic Curve Cryptography in OpenJDK6

[Michael StJohns]

At 01:39 PM 9/3/2009, Andrew John Hughes wrote:
>2009/9/3 Michael StJohns <mstjohns at comcast.net>:
>>
>>
>>
>> At 03:14 PM 9/2/2009, Andrew John Hughes wrote:
>>>Ok here is a new webrev:
>>>
>>>http://cr.openjdk.java.net/~andrew/6763530/webrev.02/
>>>
>>>with a slightly revised version of your change (you can't throw a
>>>PKCS11Exception which only takes a long ID from the native code, so I
>>>changed this to an IllegalArgumentException).
>>
>> Yeah - when I realized this a while later (when I actually started building the JDK from source) I actually considered changing PKCS11Exception to implement constructors with just a message and with a message and a code. Â  If you throw with just a message the code would get set to CKR_GENERAL_ERROR. Â If you throw with message and a code, the message for the code would get prepended to the provided message. Â That's another topic though.
>>
>> This particular error comes under the heading of one that shouldn't happen - we did the explicit encoding so the "toByteArray()" shouldn't fail. Â That's pretty much the definition of a runtime error. Â Maybe use the little used PKCS11RuntimeError instead of the IllegalArgumentException?
>>
>
>IllegalArgumentException seems appropriate to me as it's a failure
>with one of the arguments to that method (the params).  Assuming
>PKCS11RuntimeError is in the same vein as PKCS11Exception, we don't
>want to use them as they pertain to the native NSS code which isn't
>the result of the fault here.  The main thing is that the IOException
>is available via the cause of the exception that actually gets thrown.
>
>>


Except that we're trapping errors from the DER encoding of the wrapped point, not from problems with the provided argument.  By the time we get to try-catch block, we've already turned the ECPoint into a byte array and wrapped it in a DerValue(OctetString,..). 

PKCS11RuntimeException does allow you to pass on the IOException and an informative message.  It has the normal 4 constructors - null, message, exception, message and exception.

Alternately, IOError may be appropriate.  If you actually have an encoding error, something really bad went wrong with the DerValue functions and user programs probably shouldn't attempt to handle it.

I can live with IllegalArgumentException - its Runtime so it doesn't change the requirements for calling the method. But it seems a little bit off as a choice here.

>> Mike
>>
>>
>>
>>
>
>
>
>-- 
>Andrew :-)
>
>Free Java Software Engineer
>Red Hat, Inc. (http://www.redhat.com)
>
>Support Free Java!
>Contribute to GNU Classpath and the OpenJDK
>http://www.gnu.org/software/classpath
>http://openjdk.java.net
>
>PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
>Fingerprint: F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8