Code Review Request: running signed modules with SecurityManager
Sean Mullan
sean.mullan at oracle.com
Fri Jun 4 12:22:46 PDT 2010
On 6/4/10 2:31 PM, mark.reinhold at oracle.com wrote:
>> Date: Tue, 01 Jun 2010 09:50:21 -0400
>> From: sean.mullan at oracle.com
>
>> I posted a new webrev addressing the comments I have received so far:
>>
>> http://cr.openjdk.java.net/~mullan/jigsaw/webrevs/SecurityManager2/webrev.01/
>
> Mostly looks good, but I'd like to see a few changes before you push.
>
> SimpleLibrary.java
>
> [55] Is there a reason to put signer.ser in its own subdirectory?
Just trying to think ahead - we'll likely want to store additional security
information such as the module's granted permissions, and potentially other
certificate related information such as CRLs.
> If not, please change that. Also, please name it "signer", without the
> extension.
Ok.
> [522] Serialization is a pretty heavyweight (and brittle) solution
> here. I can live with it for now, but in a future changeset please
> modify this code to store and load CodeSigners using the existing
> conventions for metadata files.
Ok, I will address this in a subsequent changeset.
>
> [540] Casts should not be separated from the expression being cast,
> i.e., (CodeSigner)ois.readObject(), not (CodeSigner) ois.readObject() .
Ok.
> Loader.java
>
> The codeSourceForName map is awkward. Please add a codeSource property
> to java.lang.reflect.Module instead. That will obviate the need for
> this map and also make the CodeSource available for other uses.
Sounds good.
> [196] If there's no CodeSigner then shouldn't you pass null as the last
> argument to defineModule, rather than new CodeSource(null, null)?
No, I don't believe so. There is a subtle difference. A CodeSource of (null,
null) will still be granted permissions where the URL/certs don't matter, ex the
permissions of the sandbox policy. But a null CodeSource won't be granted any
permissions.
> ModuleFileFormat.java
>
> [831] Knowledge of the format of the signer file should be encapsulated
> in one place. When installing a module file the SimpleLibrary.install
> method already gets the signers, by invoking Reader.verifySignature,
> so it'd be better to store the signers at that point rather than in the
> module-file reader. Moving this code now will also make it possible to
> eliminate serialization later.
Yes, good point.
I'll try to post an updated webrev later today.
Thanks,
Sean
More information about the jigsaw-dev
mailing list