[rfc][icedtea-web] Fix support for signed applets with sandbox permissions in manifest

[Jiri Vanek]

On 07/31/2014 03:37 PM, Andrew Azores wrote:
> On 07/31/2014 05:32 AM, Jiri Vanek wrote:
>>>
>>> But verifyJarEntryCerts is called from verifyJar, from verifyJars, from add, and add is still done
>>> before isFullySigned is called, same as before...
>>>
>>> Like I said, even without this patch applied, isTriviallySigned is still returning true for dummy
>>> JNLPs (and so the classloader thinks they're signed). So it isn't this patch that's making the dummy
>>> JNLP show up as signed - it already is doing that in HEAD right now. AFAICT this is just "something
>>> rotten already inside" :(
>>>
>>
>>
>> How does this behave in 1.5? I bet it is same.
>
> Yes, it's the same in 1.5, I just tested it now.
>
>>
>> How important is to backport this to 1.5? I would be hesitating with it now.
>
> 1.5 still has the "Sandbox button" workaround anyway. I think this is a big enough patch that it
> warrants being a 1.6 feature ;)
>
>>
>> I'm now ok with your patch for ehad, but pelase keep original lines from discussed hunk only
>> commented out instead of deleted, with comment why. Something like thjis, although not the best shot:
>>
>
> Sure thing.
>
>>
>> -        manifest6.getMainAttributes().put(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS), "sandbox");
>> +//[1]   manifest6.getMainAttributes().put(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS), "sandbox");
>> +        manifest6.getMainAttributes().put(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS), "all-permissions");
>>          manifest6.getMainAttributes().put(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.TRUSTED_LIBRARY), "false");
>>          manifest6.getMainAttributes().put(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.TRUSTED_ONLY), "false");
>>
>> @@ -180,7 +180,7 @@ public class JNLPFileTest extends NoStdO
>>          Assert.assertEquals("*.com  https://*.cz",
>> jnlpFile.getManifestsAttributes().getAttribute(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.APP_LIBRARY_ALLOWABLE)));
>>          Assert.assertEquals("*.net  ftp://*uu.co.uk",
>> jnlpFile.getManifestsAttributes().getAttribute(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.CALLER_ALLOWABLE)));
>>          Assert.assertEquals("*.com *.net *.cz *.co.uk",
>> jnlpFile.getManifestsAttributes().getAttribute(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.CODEBASE)));
>> - Assert.assertEquals(SecurityDesc.RequestedPermissionLevel.SANDBOX.toHtmlString(),
>> jnlpFile.getManifestsAttributes().getAttribute(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS)));
>> +//[1] Assert.assertEquals(SecurityDesc.RequestedPermissionLevel.SANDBOX.toHtmlString(),
>> jnlpFile.getManifestsAttributes().getAttribute(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS)));
>> + Assert.assertEquals(SecurityDesc.RequestedPermissionLevel.ALL.toHtmlString(),
>> jnlpFile.getManifestsAttributes().getAttribute(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS)));
>>          Assert.assertEquals("false", jnlpFile.getManifestsAttributes().getAttribute(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.TRUSTED_LIBRARY)));
>>          Assert.assertEquals("false", jnlpFile.getManifestsAttributes().getAttribute(new
>> Attributes.Name(JNLPFile.ManifestsAttributes.TRUSTED_ONLY)));
>>
>> @@ -206,7 +206,7 @@ public class JNLPFileTest extends NoStdO
>>          Assert.assertEquals(true, jnlpFile.getManifestsAttributes().getCodebase().matches(new
>> URL("ftp://aa.bb.net")));
>>          Assert.assertEquals(true, jnlpFile.getManifestsAttributes().getCodebase().matches(new
>> URL("https://x.net")));
>>          Assert.assertEquals(false, jnlpFile.getManifestsAttributes().getCodebase().matches(new
>> URL("http://aa.bb/com")));
>> -        Assert.assertEquals(JNLPFile.ManifestBoolean.TRUE,
>> jnlpFile.getManifestsAttributes().isSandboxForced());
>> +//[1]   Assert.assertEquals(JNLPFile.ManifestBoolean.TRUE,
>> jnlpFile.getManifestsAttributes().isSandboxForced());
>> +        Assert.assertEquals(JNLPFile.ManifestBoolean.FALSE,
>> jnlpFile.getManifestsAttributes().isSandboxForced());
>>          Assert.assertEquals(JNLPFile.ManifestBoolean.FALSE,
>> jnlpFile.getManifestsAttributes().isTrustedLibrary());
>>          Assert.assertEquals(JNLPFile.ManifestBoolean.FALSE,
>> jnlpFile.getManifestsAttributes().isTrustedOnly());
>> + //[1]:TODO: the commented out liens are causing failures after patch blahblahurl was added. IT
>> is probbaly issue in dummyjnlp file, but the  isTriviallySigned is returning true, although it
>> have only unsigned jars inisde.
>>
>>
>> J.
>
> At first glance I thought this said "aliens are causing failures" :)
>
> OK to push (after I verify reproducers one last time)?
>

yup.