jpkg enhancements to create signed modules

Mark Reinhold mr at sun.com
Tue May 11 11:53:32 PDT 2010


> 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:
> 
>   http://cr.openjdk.java.net/~vinnie/6951048/webrev.00/webrev/
> 
> 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:

FileConstants.java

  [156] 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.

ModuleFileFormat.java

  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)
    {

  [545] 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?

Packager.java

  Why is the --nosign option needed?  Not signing a module should be the
  default; signing should be done only when requested.

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

  [200] 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.

- Mark



More information about the jigsaw-dev mailing list