jmod enhancements to support signed modules

[Vincent Ryan]

Thanks for your comments Sean.

I've made the changes you suggest except for removing PKCS7VerifierParameters
class because it is used by the ModuleFormatTest01 unit test.

An updated webrev is available at:

http://cr.openjdk.java.net/~vinnie/6957907/webrev.01/



On 03/06/2010 19:33, Sean Mullan wrote:
> Hi Vinnie,
> 
> Some comments -
> 
> * Library
> 
> - For the install method verifySignature parameter, it should state that
> the signature verification is performed only if a signature exists
> 
> - I think that we will need more than the verifySignature flag. There
> are many settings that one may want control over when validating
> certificate chains. I would suggest keeping the one-arg install methods
> (for no signature verification) and adding overloaded install methods
> that take additional parameters for verifying the signature, ex:
> 
>     public void install(Collection<File> mfs,
>                         ModuleFileVerifier.Parameters parameters)
> 
> However, I don't think this is critical right now. This is something we
> could do as a follow-on change once we understand the use cases a little
> better.
> 
> * Librarian
> 
> - When we document the -noverify option, we should make it clear that
> this means the signature is completely ignored and the module will be
> installed as an unsigned module.
> 
> * ModuleFileVerifier
> 
> - The description of verifySignature should also state that the digital
> signature is verified
> 
> - I think that we will need an extensible mechanism to allow the caller
> to decide if it wants to trust each CodeSigner. There also may be cases
> where the certificate chain validation fails (ex: certificate is
> expired) but the local policy allows the user to decide if they want to
> trust the signer.
> 
> We need some sort of callback mechanism to allow the caller (and/or
> policy) to decide if it wants to trust each code signer. But this needs
> more thought, because the callback needs to be supplied with the
> appropriate level of information to make that decision. Minimally the
> CodeSigner, but probably also information about the module and where it
> came from.
> 
> This is also something I don't think is critical right now, and I think
> the use cases will become clearer as we do more testing and work on
> other jigsaw tasks.
> 
> * ModuleFileFormat
> 
> - line 1624, 1728, you can just return ModuleFile.SignatureType.PKCS7,
> enums are constants already
> 
> - lines 1743-1748, needs to be inside a doPrivileged block, also
> System.getProperty could be called once, or is java.home cached anywhere
> else?
> 
> - line 1747, looks like you never close this file input stream
> 
> - line 1768-1770, I think a parsing exception should be treated as an
> error (also see my last comment below) ...
> 
> - It doesn't look like the PKCS7VerifierParameters class is used, should
> it be removed?
> 
> - lines 1810-1812, I think we should throw the CertificateException.
> Until we have a callback mechanism in place, I think it is better both
> for debugging and security to treat any certificate validation exception
> as fatal and by default do not install the module.
> 
> 
> --Sean
> 
> On 6/2/10 2:49 PM, Vincent Ryan wrote:
>> Hello,
>>
>> Please review these code changes to support the creation of signed
>> modules:
>>
>>    http://cr.openjdk.java.net/~vinnie/6957907/webrev.00/
>>
>> 'jmod install<module-file>' now performs certificate path validation of
>> each signer's cert chain when the module file carries a signature.
>>
>> Thanks.