[rfc][icedtea-web] Trusted-only manifest attribute

[Andrew Azores]

On 03/20/2014 12:23 PM, Jiri Vanek wrote:
> On 03/20/2014 04:33 PM, Andrew Azores wrote:
>> Hi,
>>
>> This implements the Trusted-only manifest attribute [0]. It's quite a 
>> simple check compared to some of the other new ones. A test case is 
>> included that verifies that specifying the attribute in the manifest, 
>> for a signed applet, without specifying the applet security in the 
>> applet HTML tag, is not allowed. More test cases will come later on.
>>
>> [0] 
>> http://docs.oracle.com/javase/7/docs/technotes/guides/jweb/security/manifest.html#trusted_only
>
> Thanx for prompt work!
>
> Few nits:
>
> + ... 
> file.getManifestsAttributes().getAttribute(JNLPFile.ManifestsAttributes.TRUSTED_ONLY);
>
> You do not need to bother with this. There is method for you in 
> file.getManifestsAttributes() which returns  ManifestBoolean (TRUE, 
> FALSE, UNDEFINED) for you.
>
> It will affect also
>
> +        if (!trustedOnly.equalsIgnoreCase("true")) {
> + OutputController.getLogger().log(OutputController.Level.WARNING_ALL, 
> "Trusted Only manifest attribute is not recognized. Continuing anyway.");
> +            return;
> +        }
>
> Instead of this your code will recieve IllegalArgumentException, And I 
> 'm in favour to let it be thrown.
>
> Instead of MESSAGE_ALL please go on with MESSAGE_DEBUG for those two 
> messages

All above fixed.

>
> +        if (!(isFullySigned && 
> SecurityDesc.ALL_PERMISSIONS.equals(desc))) {
> + OutputController.getLogger().log(OutputController.Level.ERROR_ALL,
> +                    "Trusted Only manifest attribute is \"true\". 
> Applet is fully signed? " + isFullySigned
> +                            + ". Applet is requesting permission 
> level: " + securityType + ". This is fatal.");
> +            throw new 
> LaunchException(Translator.R("STrustedOnlyAttributeFailure", 
> Boolean.toString(isFullySigned), securityType));
> +        }
>
>
> I would like to have the
> OutputController.getLogger().log(OutputController.Level.ERROR_ALL, as 
> MESSAFE_DEBUG and *before* the if itself.

Sure.

>
> Also the essafe is bit wierd :)

Yea... ;)

>
> "Trusted Only manifest attribute is " + trustedOnly +  ". Applet's 
> signing is " + signing + ". Applet is requesting permission level: " + 
> securityType + ".");
>
> Or similar.. moreover everything iexcept the "Applet is fully signed?" :)

Better now?

>
> Also the LaunchException itself -  Ithink the 
> Boolean.toString(isFullySigned)  nor securityType is necessary. All is 
> known in this state.
> So just throw exception with explanation without variables (thay may 
> be in only one state or not? isFullySigned==true and 
> desc!=SecurityDesc.ALL_PERMISSIONS) )

If Trusted-only is true, having either desc != ALL_PERMISSIONS *or* 
signing != SigningState.FULL will cause a failure. I'd like to be able 
to have it distinguish between these two cases, rather than just saying 
"one of these things went wrong", especially because both of those have 
several possible values.

>
>
> Also maybe different value for null and for "rest" in case of 
> securityType string.

Done.

>
>
> The reproducer is missing javaws parts and is missing correct case. 
> Minimalistical  reproducer should be
> - applet signed  trusted-only=false
> - applet signed  trusted-only=true
> - applet signed  trusted-only=illegal
> - applet mixed signatures  trusted-only=false
> - applet mixed signatures  trusted-only=false
> - applet mixed signatures  trusted-only=illegal
> - applet not signed  trusted-only=false
> - applet not signed  trusted-only=true
> - applet not signed  trusted-only=illegal
> - javaws  signed  trusted-only=false
> - javaws  signed  trusted-only=true
> - javaws  signed  trusted-only=illegal
> - javaws  not signed  trusted-only=false
> - javaws  not signed  trusted-only=true
> - javaws  not signed  trusted-only=illegal
>
> However it is to much work. In long-term it would be nice to have 
> them. For now, just extends your:
> applet not signed  trusted-only=true
> by:
> javaws not signed  trusted-only=true

It's:
applet signed trusted-only=true
right now. But yes. JNLP version of the same added.

>
>
> Also youu do not need custom reproducer for this!  Unsigned applet 
> with manifest is classical simple reproducer :)
>
> thanx!
>   J.

It will need to be custom eventually for the mixed signatures, though. I 
was intending to use just one custom reproducer to build and test all of 
the different cases (eventually).

Thanks,

-- 
Andrew A

-------------- next part --------------
A non-text attachment was scrubbed...
Name: trusted-only-attribute.patch
Type: text/x-patch
Size: 4013 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140320/382f8c97/trusted-only-attribute.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: trusted-only-attribute-tests.patch
Type: text/x-patch
Size: 12125 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20140320/382f8c97/trusted-only-attribute-tests.patch>