Comments:
* JceKeyStore
[906] add @Override to engineProbe method
* KeyStore
[1615] use @throws instead of @exception. Should probably also add an
@throws NullPointerException if keystore is null.
You need to define what happens if the File does not exist. You could
either throw a KeyStoreException, or you could instantiate a KeyStore
using the default keystore type. Each have their pros/cons. The latter
seems a little better as this method could then be used to open existing
keystores or for creating new keystores.
[1638] This is a provider configuration error, but there still may be
other KeyStoreSpis that can parse the file. I would probably just log
this and continue to check other providers.
[1654] Same here. I would log this and continue to check other
providers. Method currently say it throws KeyStoreExc only if no
Provider supports the file, but we have not necessarily checked all
providers yet.
* KeyStoreSpi
[606] s/@exception/@throws/. Should probably also add an @throws
NullPointerException if stream is null.
You need to define that the method returns false by default.
* PKCS12KeyStore
[2340] @Override
[2342-51] indentation should be 4 spaces
[2353,2364] these should be static fields
* JavaKeyStore
[54] why did you make this public?
[816] @Override
* keytool/Main
[814, 1919] I'm not sure if it is correct to ignore the storetype. I
think getInstance(File) should only be called if the storetype is not
specified.
* KeyStoreDelegator
overridden methods should have @Override tag
Can you make this class final with a private ctor, and instead have a
static method to return instances?
The engine methods throw NPE if the keystore has not been loaded. This
is not consistent with the current JKS Keystore and PKCS12 Keystore
impls and looks like it is not compliant with the KeyStoreSpi spec. I
think you need to modify the methods to return null, etc instead if the
keystore has not been loaded yet.
--Sean
On 12/02/2014 06:23 AM, Vincent Ryan wrote:
Please review the following enhancement to improve keystore security by
creating PKCS12 keystores by default.
Previously, JKS keystores were created by default. PKCS12 has the advantage of
supporting stronger crypto
and hashing algorithms. It is also an open, extensible format and supports
associating arbitrary attributes with
keystore entries.
Webrev: http://cr.openjdk.java.net/~vinnie/8044445/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8044445
To assist with compatibility across JDK releases, both JKS and PKCS12 keystore
implementations have been
extended to support both file formats. Applications that access keystores
created by earlier releases should
require no code changes.
This changeset also includes a new convenience method for instantiating a
file-based keystore: KeyStore.getInstance
- it takes a File argument. The specified file is probed by each supported
keystore implementation to determine its
keystore type. KeyStoreSpi has been enhanced with a boolean engineProbe method
to perform the actual probing.
Finally, to improve performance, the PKCS12 keystore implementation has been
moved from the SunJSSE provider
to the SUN provider (as it appears earlier in the default list of installed JCE
providers).
