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

[Jiri Vanek]

On 07/30/2014 03:01 PM, Jiri Vanek wrote:
> ...snip...
>>>
>>> As you mentioned - the signed, but with permissions:sandbox app can not be run with all permissions.
>>> So skip all attrributes, except permissions one
>>>
>>> The sandbox one should be checked:
>>> signed: permissions:all-permissions - all permissions
>>> unsigned: permissions:all-permissions - sandbox
>>> unsigned: permissions:sandbox - sandbox
>>> signed: permissions:sandbox - sandbox
>>> signed: permissions missing - all permissions
>>> unsigned: permissions missing - sandbox
>>>
>>> On low permissions, no asking.
>>>
>>> For other then low permissions user should be asked for each except
>>> signed: permissions:all-permissions
>>> unsigned: permissions:sandbox
>>> cases.
>>>
>>> But the dialogue can be done as separate changeset (with custom-permissions button:)
>>>
>>>
>>>
>>>> New tests are also on the way and with the new tests will come the fixed version of the previously
>>>> sent test, which will make be a signed reproducer rather than custom.
>>>>
>>>
>>> J.
>>>
>>
>> Okay, I wasn't sure what you had meant. It didn't seem right to skip them all but I thought maybe
>> you had something else in mind to implement to fix it. So here's another attempt.
>>
>> Setting deployment.manifest.attributes.check to false causes all checks to be skipped, except for
>> the Permissions attribute, which is always checked regardless. Using Low Extended Applet Security
>> causes the ALACA and Permissions checks to not generate dialogs and assumes that it's OK to run the
>> applet(s). In the case of the Permissions attribute, it will still auto sandbox if necessary. Is
>> this what you wanted?
>>
>> Thanks,
>>
>> --
>> Andrew A
>>
>
> I have not applied and not tested the patch.
>
> It looks good in design and code.
>
>
> assuming the:
> +        // If the attribute is not specified in the manifest, prompt the user. Oracle's spec says
> that the
> +        // attribute is required, but this breaks a lot of existing applets. Therefore, when on the
> highest
> +        // security level, we refuse to run these applets. On the standard security level, we ask.
> And on the
> +        // lowest security level, we simply proceed without asking.
>
> really works, I'm ok with this to go to head . I also think it should go to 1.5 but I left final
> decision to you. (ok for 1.5 from my side)
>
>
>
>>
>> pr1769-6.patch
>>
>>
>> diff --git a/ChangeLog b/ChangeLog
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,43 @@
>> +2014-06-24  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
>> +    (initializePermissions): renamed (initializeReadJarPermissions). Redundant
>> +    null-check removed
>> +    (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. A new Collection (validJars) is used
>> +    to hold available and valid JARs between discovering the JARs and applying
>> +    any security settings to the them
>> +    (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
>> +    (isLowSecurity): new method
>> +    (checkAll): Extended Applet Security on Low disables all manifest checks
>> +    except for Permissions
>> +    (checkPermissionsAttribute): do not skip checking if Extended Applet
>> +    Security is Low. Remove try/catch on setRunInSandbox call as this is now
>> +    supported.
>> +    (checkApplicationLibraryAllowableCodebaseAttribute): do not display dialog
>> +    prompts on low security, allow the applet to run without prompt
>
> I think the tests are not included in this patch. Please adjust the changelog to current state.
>
>> +    *
>> 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
>> +
> ...snip...
>
> Thank you very much,
> J.
>

ouch it broken some unittests

Please double check and fix.