Support for timestamped signed modules
mandy.chung at oracle.com
Thu Apr 7 14:13:15 PDT 2011
I looked at all files and will count on Vinnie or some other who is
familiar with the security implementation to review it.
There are some leftover commented lines to be removed (L614, L671, etc)
Would it be better to rename this to a different class name
(PKCSSupport?) to avoid name conflict with sun.security.pkcs.PKCS7?
L256-260: should it only call signerCert.getNotAfter() if there is a
timestamp? And also tsaValidator is only needed when there is a timestamp.
L120: looks like a debugging code to be removed?
L150: is the loadCACertsKeyStore method only called by the jmod
tool? If so, there is no security manager enabled and do you still need
doPrivileged? Otherwise, System.getProperty also needs to be wrapped by
doPrivileged? I'm thinking that you can use try-with-resource resulting
in cleaner code.
At some point, we need to look at the interface of these new
command-line tools. I was reading the Solaris guideline for the
command-line utilities some time ago and it states that every
short-option should have exactly one corresponding long-option and every
long-option should have exactly one corresponding short-option. I don't
know if these tools should follow this guideline. Given that not all
long options have its corresponding short option in the current
implementation, it's fine that you remove a few short-option in this
should be in the same line. Perhaps you can break tsaURI in another line.
L470: maybe assert tsa.length == 1, or it is guaranteed to be true??
L285: Is failureInfo of varying length (returned from
statusInfo.data.getUnalignedBitString())? Is there a better way to
check this rather than having the isSet() method to throw
ArrayIndexOutOfBoundsException? How about:
return position >= 0 && position < failureInfo.length &&
L67, 68: debugging code - if you want to leave them there, maybe
better to include the debugging capability in the test such as adding
these 2 options before adding signingArgs when invoking Packager.run
(L164) if debugging is enabled (e.g. by an input argument to the main
method) - rather than uncommenting them and recompile.
Both timestamp-test.sh and TimestampTest.java hardcodes the name
of the module library, module name, and location. Would it be better to
pass these hardcoded names from the shell test to TimestampTest as
system properties or input arguments? These values might unlikely be
changed but I would rather avoid having a hardcoded name in multiple
files for the single test.
On 04/06/11 12:44, Sean Mullan wrote:
> Please review the following webrev which adds support for timestamped
> signed modules.
> A new option (tsa) has been added to jpkg to specify the timestamping
> authority to use. I have also renamed some of the signing options to
> be more descriptive. Here is an example of creating a timestamped
> signed module :
> jpkg --sign --alias duke --keystore keystore \
> --tsa https://timestamp.geotrust.com/tsa \
> -m lib/test.security jmod test.security
> A lot of the changes involved restructuring so that jars and modules
> could share the same timestamping code.
More information about the jigsaw-dev