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