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

[Michael StJohns]

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/>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/c0211acb/attachment.htm>