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

[Jiri Vanek]

On 07/30/2014 06:24 PM, Jiri Vanek wrote:
> On 07/30/2014 05:29 PM, Andrew Azores wrote:
>> On 07/30/2014 11:20 AM, Jiri Vanek wrote:
>>>
>>>>>> +++ b/tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPFileTest.java
>>>>>> @@ -129,7 +129,7 @@ public class JNLPFileTest extends NoStdO
>>>>>>           /*
>>>>>>            *  "sandbox" or "all-permissions"
>>>>>>            */
>>>>>> -        manifest6.getMainAttributes().put(new
>>>>>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS), "sandbox");
>>>>>> +        manifest6.getMainAttributes().put(new
>>>>>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS), "all-permissions");
>>>>>
>>>>> Why this change??? It should not be here...
>>>>>
>>>>>> 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("*.comhttps://*.cz",
>>>>>> jnlpFile.getManifestsAttributes().getAttribute(new
>>>>>> Attributes.Name(JNLPFile.ManifestsAttributes.APP_LIBRARY_ALLOWABLE)));
>>>>>>           Assert.assertEquals("*.netftp://*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)));
>>>>>> + Assert.assertEquals(SecurityDesc.RequestedPermissionLevel.ALL.toHtmlString(),
>>>>>> jnlpFile.getManifestsAttributes().getAttribute(new
>>>>>> Attributes.Name(JNLPFile.ManifestsAttributes.PERMISSIONS)));
>>>>>
>>>>> So this one,
>>>>>>           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());
>>>>>> +        Assert.assertEquals(JNLPFile.ManifestBoolean.FALSE,
>>>>>> jnlpFile.getManifestsAttributes().isSandboxForced());
>>>>>
>>>>> and this one. Why so?
>>>>>> Assert.assertEquals(JNLPFile.ManifestBoolean.FALSE,
>>>>>> jnlpFile.getManifestsAttributes().isTrustedLibrary());
>>>>>> Assert.assertEquals(JNLPFile.ManifestBoolean.FALSE,
>>>>>> jnlpFile.getManifestsAttributes().isTrustedOnly());
>>>>>>
>>>>>
>>>>>
>>>>
>>>> The JNLPClassLoader is determining the applet to be signed, and the ManifestAttributesChecker does
>>>> not allow for JNLP applets to have a mismatch between their signing state and the permissions level
>>>> they request (I believe this is in the spec for the Permissions attribute). Is the applet not
>>>> actually signed? I looked into how the JARs are being created and couldn't see if they're signed
>>>> one
>>>> way or the other.
>>>>
>>>
>>> Well thats the issue. The testing "applet" (its dummyjnlp) is not signed. So this change sholdnot
>>> be needed. Something different msut be wrong.
>>>
>>>
>>> J.
>>>
>>
>> The classloader's JarCertVerifier 'jcv' is returning true for 'jcv.isFullySigned()' on all of these
>> tests, even without the patch applied. It falls into the 'isTriviallySigned' case. If this needs to
>> be fixed then I think it's perhaps out of scope of this patch, no?
>>
>> Thanks,
>>
>
>
> ufff.. not so sure.
>
> Well the issue is here:
>
> public boolean isTriviallySigned() {
>          return getTotalJarEntries(jarSignableEntries) <= 0
>                  && certs.size() <= 0;
>      }
>
>
>
> I'm not sure if it is case of DummyJnlp only or if it hides something more serious.
>
>
>
> It seems to me that once there are no jars, and no certificates, then it is considered signed (like
> class only applet?/??) If I'm correct that this return is terribly wrong. Not sure if your current
> patch made it more visible, made new hole, or just pointed to something rotten already inside...
>

Looking deeper, into

VerifyResult verifyJarEntryCerts


I really think your patch disturbed something.  The loop here is counting the entries, and is 
distinguishing among  maniefested, not manifested jars.  So I guess this is an cause.


J.