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

[Jiri Vanek]

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...

J.