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. >>>>> >>> >>> > >
