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