Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)
> Date: Mon, 10 May 2010 17:47:53 +0100
> From:

> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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
)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)
> Date: Mon, 10 May 2010 17:47:53 +0100
> From:

> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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
) > Date: Mon, 10 May 2010 14:40:03 -0400

> Vinnie discovered a small issue with the signed module file format [1] while
> implementing.
>
> The module header in the module file format [2] includes a field named csize
> which holds the size of the module file. The value of this field is not known
> until the contents of the module file has been written. The csize also includes
> the size of the signature section, but the signature cannot be written until
> all of the hashes have been generated. Two of those hashes (the module header
> hash and the whole file hash) are over content containing the csize field. Thus
> there is a circular dependency issue.
>
> The current implementation works around this by first writing a dummy signature
> and then later replacing it with the real signature once the overall size of
> the signature is known.

Won't it still be necessary to write a dummy signature even if the csize
field is omitted?

> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.

Yet another solution is simply to remove the csize field from module-file
headers.

It's read by the module-repository code when publishing a module, but the
compressed size of a module can just as easily be determined by asking
the filesystem.

Unless someone sees a need for this field, let's remove it.

- Mark
)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)
> Date: Mon, 10 May 2010 17:47:53 +0100
> From:

> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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
) > Date: Mon, 10 May 2010 14:40:03 -0400

> Vinnie discovered a small issue with the signed module file format [1] while
> implementing.
>
> The module header in the module file format [2] includes a field named csize
> which holds the size of the module file. The value of this field is not known
> until the contents of the module file has been written. The csize also includes
> the size of the signature section, but the signature cannot be written until
> all of the hashes have been generated. Two of those hashes (the module header
> hash and the whole file hash) are over content containing the csize field. Thus
> there is a circular dependency issue.
>
> The current implementation works around this by first writing a dummy signature
> and then later replacing it with the real signature once the overall size of
> the signature is known.

Won't it still be necessary to write a dummy signature even if the csize
field is omitted?

> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.

Yet another solution is simply to remove the csize field from module-file
headers.

It's read by the module-repository code when publishing a module, but the
compressed size of a module can just as easily be determined by asking
the filesystem.

Unless someone sees a need for this field, let's remove it.

- Mark
) On 05/10/10 09:47, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week
Hi Vinnie,

Just a couple of questions for now and will do a detailed review
in your new version.

Packager.java
line 224, 267:
- Does this message ("Enter password") need to be localized? If so,
it should get from a resource bundle. The jigsaw tools have yet to
support L10N. Perhaps we can start with this message.

Does this change introduce new dependency to the jdk.boot module?
You can check the module-info.java from the build directory:
build/linux-i586/tmp/modules/src/jdk.boot/module-info.java

jdk.boot is the minimal module that doesn't depend on any
other module.

jpkg is preinstalled in the jdk-base-module in the modules build.
You can run build/linux-i586/jdk-base-module/bin/jpkg
to verify if it causes unexpected dependencies (not found by
the static analysis done by the class analyzer).

Mandy
)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)
> Date: Mon, 10 May 2010 17:47:53 +0100
> From:

> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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
) > Date: Mon, 10 May 2010 14:40:03 -0400

> Vinnie discovered a small issue with the signed module file format [1] while
> implementing.
>
> The module header in the module file format [2] includes a field named csize
> which holds the size of the module file. The value of this field is not known
> until the contents of the module file has been written. The csize also includes
> the size of the signature section, but the signature cannot be written until
> all of the hashes have been generated. Two of those hashes (the module header
> hash and the whole file hash) are over content containing the csize field. Thus
> there is a circular dependency issue.
>
> The current implementation works around this by first writing a dummy signature
> and then later replacing it with the real signature once the overall size of
> the signature is known.

Won't it still be necessary to write a dummy signature even if the csize
field is omitted?

> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.

Yet another solution is simply to remove the csize field from module-file
headers.

It's read by the module-repository code when publishing a module, but the
compressed size of a module can just as easily be determined by asking
the filesystem.

Unless someone sees a need for this field, let's remove it.

- Mark
) On 05/10/10 09:47, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week
Hi Vinnie,

Just a couple of questions for now and will do a detailed review
in your new version.

Packager.java
line 224, 267:
- Does this message ("Enter password") need to be localized? If so,
it should get from a resource bundle. The jigsaw tools have yet to
support L10N. Perhaps we can start with this message.

Does this change introduce new dependency to the jdk.boot module?
You can check the module-info.java from the build directory:
build/linux-i586/tmp/modules/src/jdk.boot/module-info.java

jdk.boot is the minimal module that doesn't depend on any
other module.

jpkg is preinstalled in the jdk-base-module in the modules build.
You can run build/linux-i586/jdk-base-module/bin/jpkg
to verify if it causes unexpected dependencies (not found by
the static analysis done by the class analyzer).

Mandy
) Vincent Ryan wrote:
> Hello,
>
> Please review these code changes to support the creation of signed modules:
>
> http://cr.openjdk.java.net/~vinnie/6951048/webrev.00/webrev/
>

Nice! Thank you for the cleanups, too.

I've only got one small nit, in ModuleFileFormat.java:
SECTION_HEADER_LENGTH - doesn't seem to be used anywhere.

cheers,
dalibor topic
--
*******************************************************************

Java F/OSS Ambassador AIM: robiladonaim

Nagelsweg 55 http://openjdk.java.net
D-20097 Hamburg mailto:
Sitz der Gesellschaft: Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Amtsgericht München: HRB 161028
Geschäftsführer: Jürgen Kunz


)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)
> Date: Mon, 10 May 2010 17:47:53 +0100
> From:

> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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
) > Date: Mon, 10 May 2010 14:40:03 -0400

> Vinnie discovered a small issue with the signed module file format [1] while
> implementing.
>
> The module header in the module file format [2] includes a field named csize
> which holds the size of the module file. The value of this field is not known
> until the contents of the module file has been written. The csize also includes
> the size of the signature section, but the signature cannot be written until
> all of the hashes have been generated. Two of those hashes (the module header
> hash and the whole file hash) are over content containing the csize field. Thus
> there is a circular dependency issue.
>
> The current implementation works around this by first writing a dummy signature
> and then later replacing it with the real signature once the overall size of
> the signature is known.

Won't it still be necessary to write a dummy signature even if the csize
field is omitted?

> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.

Yet another solution is simply to remove the csize field from module-file
headers.

It's read by the module-repository code when publishing a module, but the
compressed size of a module can just as easily be determined by asking
the filesystem.

Unless someone sees a need for this field, let's remove it.

- Mark
) On 05/10/10 09:47, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week
Hi Vinnie,

Just a couple of questions for now and will do a detailed review
in your new version.

Packager.java
line 224, 267:
- Does this message ("Enter password") need to be localized? If so,
it should get from a resource bundle. The jigsaw tools have yet to
support L10N. Perhaps we can start with this message.

Does this change introduce new dependency to the jdk.boot module?
You can check the module-info.java from the build directory:
build/linux-i586/tmp/modules/src/jdk.boot/module-info.java

jdk.boot is the minimal module that doesn't depend on any
other module.

jpkg is preinstalled in the jdk-base-module in the modules build.
You can run build/linux-i586/jdk-base-module/bin/jpkg
to verify if it causes unexpected dependencies (not found by
the static analysis done by the class analyzer).

Mandy
) Vincent Ryan wrote:
> Hello,
>
> Please review these code changes to support the creation of signed modules:
>
> http://cr.openjdk.java.net/~vinnie/6951048/webrev.00/webrev/
>

Nice! Thank you for the cleanups, too.

I've only got one small nit, in ModuleFileFormat.java:
SECTION_HEADER_LENGTH - doesn't seem to be used anywhere.

cheers,
dalibor topic
--
*******************************************************************

Java F/OSS Ambassador AIM: robiladonaim

Nagelsweg 55 http://openjdk.java.net
D-20097 Hamburg mailto:
Sitz der Gesellschaft: Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Amtsgericht München: HRB 161028
Geschäftsführer: Jürgen Kunz


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

I thought signing with the single privateKeyEntry using JKS type
~/.keystore is the default when no signature-related option is provided. If
default action is no-sign, it looks a --sign option is needed, since all
other options are optional.

Max

)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)
> Date: Mon, 10 May 2010 17:47:53 +0100
> From:

> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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
) > Date: Mon, 10 May 2010 14:40:03 -0400

> Vinnie discovered a small issue with the signed module file format [1] while
> implementing.
>
> The module header in the module file format [2] includes a field named csize
> which holds the size of the module file. The value of this field is not known
> until the contents of the module file has been written. The csize also includes
> the size of the signature section, but the signature cannot be written until
> all of the hashes have been generated. Two of those hashes (the module header
> hash and the whole file hash) are over content containing the csize field. Thus
> there is a circular dependency issue.
>
> The current implementation works around this by first writing a dummy signature
> and then later replacing it with the real signature once the overall size of
> the signature is known.

Won't it still be necessary to write a dummy signature even if the csize
field is omitted?

> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.

Yet another solution is simply to remove the csize field from module-file
headers.

It's read by the module-repository code when publishing a module, but the
compressed size of a module can just as easily be determined by asking
the filesystem.

Unless someone sees a need for this field, let's remove it.

- Mark
) On 05/10/10 09:47, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week
Hi Vinnie,

Just a couple of questions for now and will do a detailed review
in your new version.

Packager.java
line 224, 267:
- Does this message ("Enter password") need to be localized? If so,
it should get from a resource bundle. The jigsaw tools have yet to
support L10N. Perhaps we can start with this message.

Does this change introduce new dependency to the jdk.boot module?
You can check the module-info.java from the build directory:
build/linux-i586/tmp/modules/src/jdk.boot/module-info.java

jdk.boot is the minimal module that doesn't depend on any
other module.

jpkg is preinstalled in the jdk-base-module in the modules build.
You can run build/linux-i586/jdk-base-module/bin/jpkg
to verify if it causes unexpected dependencies (not found by
the static analysis done by the class analyzer).

Mandy
) Vincent Ryan wrote:
> Hello,
>
> Please review these code changes to support the creation of signed modules:
>
> http://cr.openjdk.java.net/~vinnie/6951048/webrev.00/webrev/
>

Nice! Thank you for the cleanups, too.

I've only got one small nit, in ModuleFileFormat.java:
SECTION_HEADER_LENGTH - doesn't seem to be used anywhere.

cheers,
dalibor topic
--
*******************************************************************

Java F/OSS Ambassador AIM: robiladonaim

Nagelsweg 55 http://openjdk.java.net
D-20097 Hamburg mailto:
Sitz der Gesellschaft: Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Amtsgericht München: HRB 161028
Geschäftsführer: Jürgen Kunz


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

I thought signing with the single privateKeyEntry using JKS type
~/.keystore is the default when no signature-related option is provided. If
default action is no-sign, it looks a --sign option is needed, since all
other options are optional.

Max

) Thanks for your review.


On 05/11/10 19:53, Mark Reinhold wrote:
>> Date: Mon, 10 May 2010 17:47:53 +0100
>> From:
>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.

OK.


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

OK.


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

The default format for the module signature is PKCS #7 SignedData type [1],
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.
-or-
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.



[1] http://tools.ietf.org/html/rfc2315#section-9.1


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

OK.


>
> [200] The new signing code makes this method way too long to be
> readable. Please refactor it into one or more subsidiary methods.

OK.

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

)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)
> Date: Mon, 10 May 2010 17:47:53 +0100
> From:

> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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
) > Date: Mon, 10 May 2010 14:40:03 -0400

> Vinnie discovered a small issue with the signed module file format [1] while
> implementing.
>
> The module header in the module file format [2] includes a field named csize
> which holds the size of the module file. The value of this field is not known
> until the contents of the module file has been written. The csize also includes
> the size of the signature section, but the signature cannot be written until
> all of the hashes have been generated. Two of those hashes (the module header
> hash and the whole file hash) are over content containing the csize field. Thus
> there is a circular dependency issue.
>
> The current implementation works around this by first writing a dummy signature
> and then later replacing it with the real signature once the overall size of
> the signature is known.

Won't it still be necessary to write a dummy signature even if the csize
field is omitted?

> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.

Yet another solution is simply to remove the csize field from module-file
headers.

It's read by the module-repository code when publishing a module, but the
compressed size of a module can just as easily be determined by asking
the filesystem.

Unless someone sees a need for this field, let's remove it.

- Mark
) On 05/10/10 09:47, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week
Hi Vinnie,

Just a couple of questions for now and will do a detailed review
in your new version.

Packager.java
line 224, 267:
- Does this message ("Enter password") need to be localized? If so,
it should get from a resource bundle. The jigsaw tools have yet to
support L10N. Perhaps we can start with this message.

Does this change introduce new dependency to the jdk.boot module?
You can check the module-info.java from the build directory:
build/linux-i586/tmp/modules/src/jdk.boot/module-info.java

jdk.boot is the minimal module that doesn't depend on any
other module.

jpkg is preinstalled in the jdk-base-module in the modules build.
You can run build/linux-i586/jdk-base-module/bin/jpkg
to verify if it causes unexpected dependencies (not found by
the static analysis done by the class analyzer).

Mandy
) Vincent Ryan wrote:
> Hello,
>
> Please review these code changes to support the creation of signed modules:
>
> http://cr.openjdk.java.net/~vinnie/6951048/webrev.00/webrev/
>

Nice! Thank you for the cleanups, too.

I've only got one small nit, in ModuleFileFormat.java:
SECTION_HEADER_LENGTH - doesn't seem to be used anywhere.

cheers,
dalibor topic
--
*******************************************************************

Java F/OSS Ambassador AIM: robiladonaim

Nagelsweg 55 http://openjdk.java.net
D-20097 Hamburg mailto:
Sitz der Gesellschaft: Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Amtsgericht München: HRB 161028
Geschäftsführer: Jürgen Kunz


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

I thought signing with the single privateKeyEntry using JKS type
~/.keystore is the default when no signature-related option is provided. If
default action is no-sign, it looks a --sign option is needed, since all
other options are optional.

Max

) Thanks for your review.


On 05/11/10 19:53, Mark Reinhold wrote:
>> Date: Mon, 10 May 2010 17:47:53 +0100
>> From:
>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.

OK.


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

OK.


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

The default format for the module signature is PKCS #7 SignedData type [1],
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.
-or-
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.



[1] http://tools.ietf.org/html/rfc2315#section-9.1


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

OK.


>
> [200] The new signing code makes this method way too long to be
> readable. Please refactor it into one or more subsidiary methods.

OK.

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

) On 5/11/10 10:09 AM, Vincent Ryan wrote:

>> 5. Do we have SHA256withDSA now?
>
> That's a problem. Will investigate.

I don't think this is necessarily a problem. We haven't added support for this
algorithm in JDK 7 yet [1] but someone else could add support for it. As long as
jpkg returns a meaningful error indicating the algorithm is not available, I
think it is ok.

Thanks,
Sean

[1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6560751
)

Hello,

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 : module signer's identifier
-k, --keystore : module signer's keystore location
-t, --storetype : 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.


This is just one of a number of changes to support signed modules throughout
jigsaw.

Please send me your comments as I'm hoping to address any issues and integrate
these changes by the end of this week.

Thanks.
) Vinnie discovered a small issue with the signed module file format [1] while
implementing.

The module header in the module file format [2] includes a field named csize
which holds the size of the module file. The value of this field is not known
until the contents of the module file has been written. The csize also includes
the size of the signature section, but the signature cannot be written until all
of the hashes have been generated. Two of those hashes (the module header hash
and the whole file hash) are over content containing the csize field. Thus there
is a circular dependency issue.

The current implementation works around this by first writing a dummy signature
and then later replacing it with the real signature once the overall size of the
signature is known.

However, this is not an optimal solution.

The simplest fix would be to not include csize in the hashes. Another
possibility is to not sign the module header, as most of the information
contained in the header is part of initializing the processing of the file
itself, and any modifications would likely lead to an error before the signature
is validated anyway.

Comments?

[1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
[2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/


On 5/10/10 12:47 PM, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week.
>
> Thanks.
) What's critical inside the module header? The version numbers might be used
for conpatibility check. Can usize be altered to exhaust precious memory?

Maybe the header can be divided into 2 parts?

Thanks
Max

------- Original message -------
> From: Sean Mullan <>
> To: jigsaw-
> Sent: 11.5.'10, 2:39
>
> Vinnie discovered a small issue with the signed module file format [1]
> while implementing.
>
> The module header in the module file format [2] includes a field named
> csize which holds the size of the module file. The value of this field is
> not known until the contents of the module file has been written. The
> csize also includes the size of the signature section, but the signature
> cannot be written until all of the hashes have been generated. Two of
> those hashes (the module header hash and the whole file hash) are over
> content containing the csize field. Thus there is a circular dependency
> issue.
>
> The current implementation works around this by first writing a dummy
> signature and then later replacing it with the real signature once the
> overall size of the signature is known.
>
> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.
>
> Comments?
>
> [1]: http://cr.openjdk.java.net/~mullan/jigsaw/signed-module-file-format
> [2]: http://cr.openjdk.java.net/~mr/jigsaw/notes/module-file-format/
>
>
> On 5/10/10 12:47 PM, Vincent Ryan wrote:
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.

) Simple ones first.

ContentInfo:
HexDumpEncoder.encodeBuffer(*) is preferred.

Packager:
1. --nosign and --signer etc can be combinated quite freely. Is there an
illegal combination?
2. We used to specify NONE for PKCS #11 keystore name. Although it's not
recommended now, is it allowed?
3. What if user specifies a non-JKS type but hasn't provided keystore name?
4. The char[] returned by Password.readPassword() is not zeroed.
5. Do we have SHA256withDSA now?
6. We've added CRL into signed jars recently. Any plan for it in signed
modules? You might embed it in PKCS #7 block or create a supplementary
module section (Is this possible?)

ModuleFileFormat:
hashtype is hardcoded to SHA256?

Thanks
Max


------- Original message -------
> From: Vincent Ryan <>
> To: jigsaw-
> Sent: 11.5.'10, 0:46
>
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules
> throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and
> integrate
> these changes by the end of this week.
>
> Thanks.

) Thanks for your comments Max.

On 11/05/2010 13:15, **** Weijun wrote:
> Simple ones first.
>
> ContentInfo:
> HexDumpEncoder.encodeBuffer(*) is preferred.

Done.


>
> Packager:
> 1. --nosign and --signer etc can be combinated quite freely. Is there an
> illegal combination?

Now it throws an exception when --nosign is supplied with any of --signer
--keystore --storetype.


> 2. We used to specify NONE for PKCS #11 keystore name. Although it's not
> recommended now, is it allowed?

Now supported.


> 3. What if user specifies a non-JKS type but hasn't provided keystore name?

That's allowed, for example, PKCS11 and Windows-MY


> 4. The char[] returned by Password.readPassword() is not zeroed.

Done.


> 5. Do we have SHA256withDSA now?

That's a problem. Will investigate.


> 6. We've added CRL into signed jars recently. Any plan for it in signed
> modules? You might embed it in PKCS #7 block or create a supplementary
> module section (Is this possible?)

No plans to support this yet.


>
> ModuleFileFormat:
> hashtype is hardcoded to SHA256?

Yes.


>
> Thanks
> Max
>
>
> ------- Original message -------
>> From: Vincent Ryan <>
>> To: jigsaw-
>> Sent: 11.5.'10, 0:46
>>
>> Hello,
>>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.
>>
>>
>> This is just one of a number of changes to support signed modules
>> throughout
>> jigsaw.
>>
>> Please send me your comments as I'm hoping to address any issues and
>> integrate
>> these changes by the end of this week.
>>
>> Thanks.
>

) >> 3. What if user specifies a non-JKS type but hasn't provided keystore
>> name?
>
> That's allowed, for example, PKCS11 and Windows-MY
>
>

What about pkcs12 and jceks?
) On 11/05/2010 15:56, **** Weijun wrote:
>>> 3. What if user specifies a non-JKS type but hasn't provided keystore
>>> name?
>>
>> That's allowed, for example, PKCS11 and Windows-MY
>>
>>
>
> What about pkcs12 and jceks?

An exception will be thrown. For example,

% jpkg --storetype PKCS12 --signer mykey jmod mymodule
Enter password for PKCS12 keystore:
java.security.KeyStoreException: Signer key not found in PKCS12 keystore



)
> Date: Mon, 10 May 2010 17:47:53 +0100
> From:

> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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
) > Date: Mon, 10 May 2010 14:40:03 -0400

> Vinnie discovered a small issue with the signed module file format [1] while
> implementing.
>
> The module header in the module file format [2] includes a field named csize
> which holds the size of the module file. The value of this field is not known
> until the contents of the module file has been written. The csize also includes
> the size of the signature section, but the signature cannot be written until
> all of the hashes have been generated. Two of those hashes (the module header
> hash and the whole file hash) are over content containing the csize field. Thus
> there is a circular dependency issue.
>
> The current implementation works around this by first writing a dummy signature
> and then later replacing it with the real signature once the overall size of
> the signature is known.

Won't it still be necessary to write a dummy signature even if the csize
field is omitted?

> However, this is not an optimal solution.
>
> The simplest fix would be to not include csize in the hashes. Another
> possibility is to not sign the module header, as most of the information
> contained in the header is part of initializing the processing of the file
> itself, and any modifications would likely lead to an error before the
> signature is validated anyway.

Yet another solution is simply to remove the csize field from module-file
headers.

It's read by the module-repository code when publishing a module, but the
compressed size of a module can just as easily be determined by asking
the filesystem.

Unless someone sees a need for this field, let's remove it.

- Mark
) On 05/10/10 09:47, Vincent Ryan wrote:
> Hello,
>
> 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 : module signer's identifier
> -k, --keystore : module signer's keystore location
> -t, --storetype : 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.
>
>
> This is just one of a number of changes to support signed modules throughout
> jigsaw.
>
> Please send me your comments as I'm hoping to address any issues and integrate
> these changes by the end of this week
Hi Vinnie,

Just a couple of questions for now and will do a detailed review
in your new version.

Packager.java
line 224, 267:
- Does this message ("Enter password") need to be localized? If so,
it should get from a resource bundle. The jigsaw tools have yet to
support L10N. Perhaps we can start with this message.

Does this change introduce new dependency to the jdk.boot module?
You can check the module-info.java from the build directory:
build/linux-i586/tmp/modules/src/jdk.boot/module-info.java

jdk.boot is the minimal module that doesn't depend on any
other module.

jpkg is preinstalled in the jdk-base-module in the modules build.
You can run build/linux-i586/jdk-base-module/bin/jpkg
to verify if it causes unexpected dependencies (not found by
the static analysis done by the class analyzer).

Mandy
) Vincent Ryan wrote:
> Hello,
>
> Please review these code changes to support the creation of signed modules:
>
> http://cr.openjdk.java.net/~vinnie/6951048/webrev.00/webrev/
>

Nice! Thank you for the cleanups, too.

I've only got one small nit, in ModuleFileFormat.java:
SECTION_HEADER_LENGTH - doesn't seem to be used anywhere.

cheers,
dalibor topic
--
*******************************************************************

Java F/OSS Ambassador AIM: robiladonaim

Nagelsweg 55 http://openjdk.java.net
D-20097 Hamburg mailto:
Sitz der Gesellschaft: Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Amtsgericht München: HRB 161028
Geschäftsführer: Jürgen Kunz


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

I thought signing with the single privateKeyEntry using JKS type
~/.keystore is the default when no signature-related option is provided. If
default action is no-sign, it looks a --sign option is needed, since all
other options are optional.

Max

) Thanks for your review.


On 05/11/10 19:53, Mark Reinhold wrote:
>> Date: Mon, 10 May 2010 17:47:53 +0100
>> From:
>
>> 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 : module signer's identifier
>> -k, --keystore : module signer's keystore location
>> -t, --storetype : 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.

OK.


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

OK.


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

The default format for the module signature is PKCS #7 SignedData type [1],
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.
-or-
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.



[1] http://tools.ietf.org/html/rfc2315#section-9.1


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

OK.


>
> [200] The new signing code makes this method way too long to be
> readable. Please refactor it into one or more subsidiary methods.

OK.

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

) On 5/11/10 10:09 AM, Vincent Ryan wrote:

>> 5. Do we have SHA256withDSA now?
>
> That's a problem. Will investigate.

I don't think this is necessarily a problem. We haven't added support for this
algorithm in JDK 7 yet [1] but someone else could add support for it. As long as
jpkg returns a meaningful error indicating the algorithm is not available, I
think it is ok.

Thanks,
Sean

[1] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6560751
) On 5/12/10 6:19 AM, Vincent Ryan wrote:

>> [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?
>
> The default format for the module signature is PKCS #7 SignedData type [1],
> 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.
> -or-
> 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.
>
>
>
> [1] http://tools.ietf.org/html/rfc2315#section-9.1

I think another option would be to process the data in two passes. The first
pass generates all the hash values, and the second pass writes out the file.

--Sean
)