Looks good to me.
At 08:40 AM 10/11/2011, Vincent Ryan wrote:
>Hello Michael,
>
>I agree with your preference for symmetric handling of the EC point
>encoding/decoding. So I've modified the decoder method in P11Key.java
>to use the security provider config attribute 'useEcX963Encoding' just
>like the encoder method in P11ECKeyFactory.java
>
>The updated webrev is at:
> http://cr.openjdk.java.net/~vinnie/7099228/webrev.00/
>
>(NOTE: I've used a new bug ID for this change)
>
>Thanks.
>
>
>On 09/24/11 01:24 AM, Michael StJohns wrote:
>> At 04:13 PM 9/23/2011, Vincent Ryan wrote:
>>> On 09/23/11 05:41 PM, Michael StJohns wrote:
>>>> Heh - I thought this looked familiar.
>>>>
>>>> Please take a look at my comments in bug 6763530, especially the fix for
>>>> P11Key around line 1017.
>>>>
>>>
>>> I didn't see any comments from you in bug 6763530?
>>
>> The 29 November comments are from me.
>>
>>
>>
>>
>>>> Would it make sense to back out the fix that was put into P11Key to fix
>>>> that bug in favor of a provider based solution (e.g. same boolean check)?
>>>>
>>>>
>>>
>>> I think the flexibility to decode either format regardless of the setting of
>>> the new configuration attribute is a useful benefit. However I'm not
>>> convinced
>>> that the decoder will ever need to parse an encoding that was not generated
>>> by
>>> it.
>>
>> Yeah - I was thinking that having the encode/decode make sure they're doing
>> the same thing was a good idea. But I think as written it will do the right
>> thing, albeit slightly less efficiently.
>>
>>
>>>> Otherwise I'm fine with this fix.
>>>>
>>>
>>> Great. Thanks.
>>>
>>>
>>>> Mike
>>>>
>>>>
>>>> At 11:41 AM 9/23/2011, Vincent Ryan wrote:
>>>>> Thanks for your feedback Michael. I agree that your provider-based
>>>>> solution is
>>>>> preferable. Here's a revised webrev:
>>>>>
>>>>> http://cr.openjdk.java.net/~vinnie/7054637/webrev.01/
>>>>>
>>>>>
>>>>>
>>>>> On 09/22/11 11:18 PM, Michael StJohns wrote:
>>>>>> Hi Vincent -
>>>>>>
>>>>>> Sorry - it took me a few days to look at this. I can't support it.
>>>>>>
>>>>>> I think this the wrong way to do things - specifically the dependence on
>>>>>> the
>>>>>> presence of a PKCS11 attribute in the Vendor space.
>>>>>>
>>>>>> You've got a value in PKCS11Constants
>>>>>>
>>>>>>
>>>>>> + /* Only the raw encoding for an EC point is
>>>>>> supported */
>>>>>> + public static final long CKA_ENABLE_RAW_EC_POINT =
>>>>>> (CKA_VENDOR_DEFINED | 1);
>>>>>> +
>>>>>>
>>>>>>
>>>>>> The problem is that this has a pretty good chance of colliding with an
>>>>>> actual
>>>>>> vendor defined value. For example, Utimaco has CKA_ATTRIBUTE_LIST
>>>>>> 0x80000001
>>>>>> (same as CKA_VENDOR_DEFINED | 1).
>>>>>>
>>>>>> Instead, use the attribute list of the provider configuration. You'll
>>>>>> have to
>>>>>> mod sun.security.pkcs11.Config.java - but you won't have the changes in
>>>>>> either
>>>>>> PKCS11Constants or Functions that are currently there.
>>>>>>
>>>>>>
>>>>>> The problem should be provider wide and not need a per-token config item.
>>>>>>
>>>>>> So the config file gets instead:
>>>>>>
>>>>>> useEcX963Encoding=true around line 13 of the .cfg file.
>>>>>>
>>>>>> Config.java sets the boolean useEcX963Encoding.
>>>>>>
>>>>>> P11ECKeyFactory.java checks "token.config.useEcX963Encoding".
>>>>>>
>>>>>> Avoid the use of the term "raw" please - that got us into a number of
>>>>>> problems
>>>>>> when revising PKCS11. It actually tends to mean just the concatenation
>>>>>> of the X
>>>>>> and Y points without a format identifier which is not what you mean here
>>>>>> according to PKCS11. X9.63 encoding is 1 octet of format identifier -
>>>>>> generally
>>>>>> 04 for uncompressed - and 2N bytes of X and Y. ECPoint (as defined in
>>>>>> PKCS11)
>>>>>> is the X9.63 encoding wrapped in an ASN1 OctetString.
>>>>>>
>>>>>>
>>>>>> Thanks! Mike
>>>>>>
>>>>>>
>>>>>> At 02:17 PM 9/14/2011, Vincent Ryan wrote:
>>>>>>> Please review the following fix to the SunPKCS11 JCE provider:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~vinnie/7054637/webrev.00/
>>>>>>>
>>>>>>> The problem is that some older PKCS11 tokens support only the raw
>>>>>>> encoding for
>>>>>>> EC point in Elliptic Curve public keys. This fix introduces a
>>>>>>> configuration
>>>>>>> attribute that controls whether the raw-encoding or DER-encoding shall
>>>>>>> be used.
>>>>>>>
>>>>>>> It aids interoperability between older and newer PKCS11 tokens.
>>>>>>>
>>>>>>> Thanks.
>>>>>>
>>>>
>>>>
>>
>>
