[rfc][icedtea-web] PR1592 reproducers update

[Andrew Azores]

On 01/22/2014 07:25 AM, Jiri Vanek wrote:
> On 01/21/2014 03:43 PM, Andrew Azores wrote:
>> On 01/21/2014 06:16 AM, Jiri Vanek wrote:
>>> On 01/20/2014 09:49 PM, Andrew Azores wrote:
>>>> On 01/20/2014 11:07 AM, Jiri Vanek wrote:
>>>>> On 01/09/2014 08:06 PM, Andrew Azores wrote:
>>>>>> On 01/09/2014 11:17 AM, Andrew Azores wrote:
>>>>>>> On 01/03/2014 02:43 PM, Andrew Azores wrote:
>>>>>>>> Updated PR1592 tests, using a custom reproducer rather than 
>>>>>>>> split simple/signed. This allows
>>>>>>>> method calls to be made in the normal way as well as via 
>>>>>>>> reflection. JNLP includes both
>>>>>>>> applications and applets now, and they close properly as well.
>>>>>>>>
>>>>>>>> (snip)
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Andrew A
>>>>>>>
>>>>>>> Went back over this and realized one of the tests was written 
>>>>>>> wrong. The
>>>>>>> assertAccessControlException helper method in the testcase file 
>>>>>>> is now a little stricter about the
>>>>>>> type of AccessControlException (so that the exceptions due to 
>>>>>>> applets not being allowed to call
>>>>>>> System.exit don't falsely fulfill this assertion), and 
>>>>>>> MixedSigningAppletHelper.attackDoPrivileged
>>>>>>> now properly calls 
>>>>>>> MixedSigningAppletSigned#testSignedReadPropertiesDoPrivileged, 
>>>>>>> as it should
>>>>>>> have been doing. In this case, the Unsigned JAR actually *is* 
>>>>>>> meant to be able to retrieve data
>>>>>>> from the Signed JAR (as is the point of the 
>>>>>>> AccessController.doPrivileged call), so the testcases
>>>>>>> now expect this test to successfully read from 
>>>>>>> System.getProperty, rather than receive an
>>>>>>> AccessControlException. However, the tests still verify that in 
>>>>>>> situations where the Signed JAR
>>>>>>> has a method call that involves a privileged action *without* 
>>>>>>> being placed inside a doPrivileged
>>>>>>> call, an AccessControlException will be thrown if the Unsigned 
>>>>>>> code attempts to access it, as
>>>>>>> expected.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>
>>>>>> Sorry, please ignore the previous patch. The extra changes were 
>>>>>> not made based on the most recent
>>>>>> other changes. Attached are the properly rebased patches, also 
>>>>>> split into three as they were
>>>>>> originally.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>
>>>>> Thanx for ping.
>>>>> There is really many of jnlps which are nearly similar. Maybe 
>>>>> better idea can be to have one template, and generate all the rest 
>>>>> from it?
>>>>>
>>>>> I altready did this - and generated them ion BeforeClass.
>>>>>
>>>>> What do you think?
>>>>
>>>> I like this idea, but I didn't know we were okay with having 
>>>> reproducers do tricks like this ;)
>>>>
>>>> Thanks,
>>>>
>>>
>>>
>>> On seriosu flaw: you have "private static final ServerAccess server 
>>> = new ServerAccess();" declard. by this you are owerwritting the one 
>>> in BrowserTest, so no browser test will work, and wil fial 'can not 
>>> lunch unset browser".
>>> Just remove this line.
>>
>> Oops, right.
> [2]
>>
>>>
>>> Also - non of the jnlp have security element specified. It i s 
>>> intentional?? I thought it was an reason for this test to be redone.
>>>
>>> After fixed first, and explined second, ok to head.
>>>
>>> J.
>>
>> http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2013-December/025635.html 
>>
>>
>> The JNLP spec seems a little bit ambiguous here - it seems to say 
>> that "All JARs must be signed" if requesting All-Permission, but 
>> doesn't explicitly say if full signing is required to be able to 
>> request permissions at all. But as I explained in that other thread, 
>> I think it's correct to use the security tag when you have full 
>> signing, and not correct to use it when you have partial signing.
>
>
> Hmhm. And what should itw do with such an malicious (security tag) 
> file and contne (not all jars signed)t? It should die...
>
> And thats exactly what the reproducers should check.
>
> Or not?
>
> So from my side (and after reading the [1]) I'm +1 to add the tests.
>
> J.
>
> [2] dont forget on it in case of new round :)

It does die, which is correct IMO. Clearly it would be wrong if we 
actually respected that security tag, and I think it would be ambiguous 
of us and kind of useless to silently ignore the security tag - doing 
this would probably just end up annoying some app developer. Best to 
explicitly fail with the launch exception describing the exact problem. 
If you think it's worthy of a new patch/review round then I can also add 
that. But how in-depth do you want it to be... ? Just one test to ensure 
that the security tag causes a failed launch, or repeat all existing 
tests with an addition security tag, or somewhere in between?

Thanks,

-- 
Andrew A