jpkg enhancements to create signed modules
vincent.x.ryan at oracle.com
Wed May 12 03:19:48 PDT 2010
Thanks for your review.
On 05/11/10 19:53, Mark Reinhold wrote:
>> Date: Mon, 10 May 2010 17:47:53 +0100
>> From: vincent.x.ryan at oracle.com
>> Please review these code changes to support the creation of signed modules:
>> It adds the following new options to the jpkg tool:
>> -S, --signer <ID> : module signer's identifier
>> -k, --keystore <location> : module signer's keystore location
>> -t, --storetype <type> : module signer's keystore type
>> --nosign : do not sign the module
>> --nopassword : do not prompt for a keystore password
>> Appropriate default values are supported and keystore passwords may be
>> supplied to jpkg by redirecting standard input.
> Thanks; this is a good start. Some comments:
>  We might support XML_DSIG or PGP someday, but we have no
> immediate plans for those, so please remove these elements from the
> SignatureType enum.
> Please use "//" rather than "/* .. */" for all non-javadoc comments.
> [486, and elsewhere] Indent non-initial lines of a long argument list,
> and put the following brace on its own line:
> public void setSignatureMechanism(ModuleFileSigner mechanism,
> ModuleFileSigner.Parameters parameters)
>  Having to shift the whole file here seems unfortunate. Is there
> no way to predict the size of the signature in advance? Does it depend
> upon anything other than the number of section hashes?
The default format for the module signature is PKCS #7 SignedData type ,
which is an ASN.1 BER encoding of nested tag-length-value data structures.
Unfortunately it is difficult to predict its exact size in advance without
performing the actual encoding.
The design choices are:
1) write the signature twice - first with a dummy signature to determine
the exact size and then later with the true signature. No shift.
2) write the signature once, at the end of the process. Shift required.
The current implementation involves 1) _plus_ the shift. I can certainly
improve on that.
> Why is the --nosign option needed? Not signing a module should be the
> default; signing should be done only when requested.
I'll reverse that.
> [137, 140] Variables named "nofoo" are confusing, and lead to clunky
> double-negative constructions such as "!nofoo". Please change these
> variables to their positive forms.
>  The new signing code makes this method way too long to be
> readable. Please refactor it into one or more subsidiary methods.
> Finally, you'll need a unit test for all this new functionality.
I've extended an existing unit test. I'll include that in my next webrev.
> - Mark
More information about the jigsaw-dev