[rfc][icedtea-web] Mixed-signing applet permissions (PR1592)

[Jiri Vanek]

On 11/28/2013 05:19 PM, Andrew Azores wrote:
> On 11/20/2013 07:42 AM, Jiri Vanek wrote:
>> On 11/04/2013 09:19 PM, Andrew Azores wrote:
>>> Hi,
>>>
>>> This patch allows signed JARs within mixed-signing applets to be granted full permissions, while
>>> unsigned JARs in the same applets retain sandbox permissions only. The user is warned/prompted for
>>> the okay to proceed when this occurs.
>>>
>>> I was not able to create a working reproducer test for this due to the following error:
>>>
>>> java.lang.SecurityException: class "MixedSigningApplet"'s signer information does not match signer
>>> information of other classes in the same package
>>>
>>> I'd need to have two different packages in use to get around this, but AFAIK we don't have a way to
>>> support this with our reproducer system. Also, even if I had that working, the
>>> SecurityDialogs.showNotAllSignedWarningDialog still doesn't really respect the Extended Applet
>>> Security settings and will appear to prompt the user even if security is set to lowest. This would
>>> break the automation of the reproducer test and make it fairly useless anyway.
>>>
>>> The patch is split in two. The first one does the actual work. The second patch just removes an old
>>> unused local variable and an associated enclosing try/catch. This indentation change creates hard to
>>> read diff output, so I included this separately for ease of review. They need to be applied in order.
>>>
>>> Changelog:
>>> * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: (initializeResources) grant full
>>> permissions to signed JARs of mixed-signing applets
>>>
>>> Thanks,
>>>
>>
>> Hm.. It took time to get through all possible consequences of this patch.
>> Tbh I'm surpised with simplicity of it.
>>
>> the "jarSingleton" hack is good enough, but wel. should be more readableas on first glance it is not sure what it dies, - it is not singleton in its true meaning.
>
> Yea, I'll rename it. It definitely is not a "singleton" :)
>
>>
>> "IMO, the right solution would just be moving the logic into the verifier itself" worth to do then?"
>
> I'm thinking of adding it as a static method, seems to me to make the most sense that way, and it will be the easiest to implement too.
>
>>
>> "signing" variable is now misleading - it should be in three states. Eg. some dialogues can have changed context then.
>
> I'll change the type of "signing" to a three-valued enum. "Full", "Partial", and "None" sound good for its values? And call the enum something like SigningState?
>
>>
>> I must insist on set of reproducers (not automated, but placed in reproducers direcotries - if they will be custom , there can be script to lunch them or something like that)) where you will try to hack your own approach. Most basic signed and uunsigned together, unsigned code doing something wrong. Or unsigned called signed, which is then doing soemthing wrong.. or similar. No limits to imagination :))
>>
>> Good speed,
>> j.
>
> I have an ugly "external reproducer" working now. It just consists of one unsigned JAR with one class, and one signed JAR with one class. I've got it testing a few scenarios:
> - Non-privileged actions (printing out some string constant) - always works
> - Privileged action of reading System.getProperty values from:
> - Unsigned JAR - not allowed
> - Signed JAR - works for html applet, not for JNLP application *
> - Unsigned JAR calling a public method in signed JAR - not allowed
> - Signed JAR calling a public method in unsigned that calls back into signed - not allowed
> - Unsigned calling into signed by reflection - not allowed
>
> * this is simply because the new logic is within a block that checks if the JNLPFile is an instance of PluginBridge, so it's obviously only being allowed for plugins. Do we also want to allow this for JNLP? I remember someone saying that the webstart side intentionally had stricter security. IMO we should not just be failing to launch applets like this though - but at the same time we certainly shouldn't run them without asking the user first, since they are inherently risky.
>
> The testing is done from both HTML and JNLP so it's tested on both sides. So it seems to me that it's working as intended, mostly (see above). Do you want there to also be more tests added that test the same things with different JAR setups or something? I'll add a test with an external main-class and a signed JAR, but are there any other scenarios?
>
> Thanks,
>

ok, it sounds  excelent.
   For changes you mention, I'm looking forward to new patch
   for reproducer, please remake you dirty one reproducer as regullar reproducer
     - have one jar created via simple, and one via siged.
     - in one of them have  jnlp and html files under sources
     - you can adapt the calls between jars via args/params
     - in one of them have testcase, which will lunch all the mentioned cases (but commetn out @Test untill issue with dialogue is fixed)
            - rember that test must have at least on @test method
    - have comment both in both srcs, and in testcase, that those two jars are connected
    - adapt above to your feelings, this is just group of advices of ill men :)

Sorry for insisting :(
   J