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