[8] Code review request for 8005408: KeyStore API enhancements

[Vincent Ryan]

Thanks for your comments Michael. Your suggestions involve API changes that I cannot accommodate
in time for Milestone 6. I will address these issues following M6 and have filed the following bug report
to track that task:

8006796: Address additional review comments for 8005408


BTW the current syntax for the trustedKeyUsage attribute is designed to enable certs to be marked as
trusted for limited purposes. Your suggestion delivers a simpler syntax but loses this feature.




On 23 Jan 2013, at 17:42, Michael StJohns wrote:

> In KeyStore.java -
> 
> Attribute should probably be abstract rather than interface - mainly because you need to define equals properly to honor the Set contract for an attribute.  E.g. the equals is only against the "name" - not the encoded name/value.  This can be overridden later.
> 
> In PKCS12Attribute -  line 194 - a simple compare against the OID value of type is probably where you want to go here.  Otherwise you can have multiple Attributes with the same type, but different values.
> 
> 
> There's something wrong with your definition of the trusted key attribute - I think.   See below.
> 
> 
> 
> At 10:18 AM 1/21/2013, Vincent Ryan wrote:
>> Hello,
>> 
>> Please review the fix for 8005408. It adds support for associating attributes with keystore entries.
>> It is yet another component of the JEP-166 delivery.
>> 
>> This new API permits several enhancements to the PKCS12 keystore implementation: the storage of
>> trusted certificates, storage of secret keys and support for entry metadata. Currently, only the
>> PKCS12 keystore takes advantage of these new KeyStore APIs.
>> 
>> Webrev: http://cr.openjdk.java.net/~vinnie/8005408/webrev.00/
>> 
>> 
>> For storing trusted certificates in PKCS12 a new SafeBag attribute (with a familiar syntax) is introduced
>> to indicate a trust usage:
>> 
>> trustedKeyUsage ATTRIBUTE ::= {
>>     WITH SYNTAX ExtKeyUsageSyntax
>>     ID id-at-trustedKeyUsage  -- object identifier from an Oracle arc
>> }
>>  
>> -- from RFC 5832, Section 4.2.1.12
>>     ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId 
>>     KeyPurposeId ::= OBJECT IDENTIFIER
>>     anyExtendedKeyUsage OBJECT IDENTIFIER ::= { id-ce-extKeyUsage 0 }
> 
> 
> What I think  you want as an encoding is 
> 
> SEQUENCE {
>   id-at-trustedCert,
>   SET of {
>         BOOLEAN DEFAULT TRUE
>   }
> }
> 
> Or basically the oid with an empty set under it.   Don't use ExtKeyUsage as the syntax.  Its probably incorrect for what you're trying to accomplish.
> 
> TrustedCert :: BOOLEAN DEFAULT TRUE
> 
> trustedCertAttribute ATTRIBUTE ::= {
>     ID id-at-trustedCert,
>     WITH SYNTAX TrustedCert
> }
> 
> Alternately, use a syntax of NULL.
> 
> Try to get a real OID allocation for id-at-trustedCert before this goes final.
> 
> 
> 
>> Note that this approach does not preclude the storage of a Trust Anchor List (as defined in RFC 5914)
>> which was proposed earlier on this list.
>> 
>> 
>> There is one omission from the webrev above: the java.security.PKCS12Attribute class needs some
>> additional changes and will be posted shortly.
>> 
>> Again, JEP-166 is on a tight schedule for M6 so your early comments are appreciated.
>> 
>> Thanks.
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20130123/146f891a/attachment.htm>