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

[Andrew Azores]

On 06/11/2014 09:46 AM, Andrew Azores wrote:
> On 06/03/2014 01:48 PM, Andrew Azores wrote:
>> On 05/27/2014 12:23 PM, Andrew Azores wrote:
>>> Hi,
>>>
>>> This patch allows signed applets with sandbox permissions specified 
>>> in their manifests to actually be run sandboxed. This is in contrast 
>>> to the current behaviour where such applets will fail to launch, and 
>>> the failure message presented asks the user to try launching via the 
>>> Sandbox button next time. This was because the dialog which 
>>> presented the Sandbox button appeared very early in the 
>>> JNLPClassLoader's life cycle - early enough that no security 
>>> settings had yet been set for the classloader or any of the applet's 
>>> JAR locations - whereas the manifest checks were done later, after 
>>> these settings would have already been initialized. Fixing the issue 
>>> was not as simple as doing the manifest checks before presenting the 
>>> security dialog because the dialog was presented part way through 
>>> the initialization process, where JARs are being downloaded and 
>>> checked for signing, so that the appropriate security dialog could 
>>> be shown to the user. Putting the manifest checks first would 
>>> therefore not work properly because the JARs were not yet available. 
>>> This patch resolves the issue by moving the manifest checks inside 
>>> the method which initializes the relevant security settings, such 
>>> that the required resources are available, it is known what type of 
>>> applet is about to be run, but the security settings for the JAR 
>>> locations have not yet been initialized and the applet can thus 
>>> still be set to run sandboxed safely.
>>>
>>> Additionally, the ManifestAttributesChecker check for the 
>>> Permissions attribute is no longer skipped when Extended Applet 
>>> Security is set to the Low level, since this allows for signed 
>>> applets with Sandbox permissions specified in their manifests to run 
>>> with full permissions when Low security is set.
>>>
>>> All existing reproducers have been run with this patch applied and 
>>> there appears to be no effect. Manual tests from the wiki have also 
>>> been run and no failures noted. Two new applets, reported in PR1767 
>>> [0], have been added as test cases to the wiki. These fail without 
>>> the patch and pass with it. A reproducer is also included, however, 
>>> the ALACA dialog will appear, which means manual intervention is 
>>> required to run this test as well. The test is marked KnownToFail 
>>> and this issue documented with the test case. This cannot be worked 
>>> around by disabling manifest attributes checking since this would 
>>> render the test meaningless as the Permissions manifest attribute 
>>> would then not be run to force sandboxing.
>>>
>>> ChangeLog:
>>>
>>> 2014-05-27  Andrew Azores  <aazores at redhat.com>
>>>
>>>     Fixed support for signed applets which specify the Permissions 
>>> attribute
>>>     as "sandbox" in their manifests. These applets are now properly run
>>>     sandboxed automatically, rather than requiring the user to click 
>>> the
>>>     "Sandbox" run button.
>>>     * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>>     (JNLPClassLoader): manifest attributes checking and security 
>>> settings
>>>     moved inside initializeResources
>>>     (initializeResources): do not set entries in 
>>> jarLocationSecurityMap until
>>>     after prompting the user on whether to run the applet as well as
>>>     performing manifest attribute checks
>>>     (initializeManifestAttributesChecker): new method
>>>     (getJnlpFileCodebase): new method, extracted from 
>>> initializeResources
>>>     (SecurityDelegateImpl.setRunInSandbox): throw exception if 
>>> already forced
>>>     to run in sandbox, rather than if already prompted
>>>     * netx/net/sourceforge/jnlp/runtime/ManifestAttributesChecker.java
>>>     (checkPermissionsAttribute): do not skip checking if Extended 
>>> Applet
>>>     Security is Low. Remove try/catch on setRunInSandbox call as 
>>> this is now
>>>     supported.
>>>     * 
>>> tests/reproducers/custom/SignedAppletManifestSpecifiesSandbox/testcases/SignedAppletManifestSpecifiesSandboxTests.java:
>>>     new test case
>>>     * 
>>> tests/reproducers/custom/SignedAppletManifestSpecifiesSandbox/resources/SignedAppletManifestSpecifiesSandbox.html:
>>>     same
>>>     * 
>>> tests/reproducers/custom/SignedAppletManifestSpecifiesSandbox/srcs/MANIFEST.MF:
>>>     same
>>>     * 
>>> tests/reproducers/custom/SignedAppletManifestSpecifiesSandbox/srcs/Makefile:
>>>     same
>>>     * 
>>> tests/reproducers/custom/SignedAppletManifestSpecifiesSandbox/srcs/SignedAppletManifestSpecifiesSandbox.java:
>>>     same
>>>
>>>
>>> [0] http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=1767
>>>
>>> Thanks,
>>>
>>
>> Ping. I know this is a big one, and there's currently a decent 
>> workaround in place, so no rush on review. Just need to make sure it 
>> doesn't get lost.
>>
>> Thanks,
>>
>
> Ping again. I think this is probably going to have to wait for Jiri to 
> come back though.
>
> Thanks,
>

Ping.

Thanks,

-- 
Andrew A