RFR 8130648: JCK test api/java_security/AuthProvider/ProviderTests_login starts failing after JDK-7191662
Sean Mullan
sean.mullan at oracle.com
Fri Sep 25 15:44:38 UTC 2015
SecurityPermission:
Change "login, logout" to "login and logout"
All files:
Use "@since 9" instead of "1.9"
Looks good otherwise.
--Sean
On 9/24/15 10:28 PM, Valerie Peng wrote:
>
> Sure, the webrev is updated below with your comments and Joe's
> suggestion of adding a query API for provider configuration.
> I added a trivial regression test for the uninitialized PKCS11 provider
> as well.
>
> Webrev: http://cr.openjdk.java.net/~valeriep/8130648/webrev.01/
>
> Will update CCC once the webrev review is done.
> Thanks,
> Valerie
>
> On 9/24/2015 11:24 AM, Sean Mullan wrote:
>> Ok, I see now.
>>
>> I think now that it also makes sense to change setCallbackHandler to
>> throw IllegalStateException if a provider that requires configuration
>> has not been configured. This is because the permission depends on the
>> name of the provider, which (at least in the case of PKCS11) may not
>> be able to be determined until the provider is configured (since it is
>> part of the configuration data). Thus, then you can assume the handler
>> is null and then there is no need to copy it to the new provider object.
>>
>> Also, while you are fixing this, could you add the "authProvider.{
>> provider name}" permission to the permission table in the
>> SecurityPermission class summary? It looks like that was an oversight
>> and never got documented there.
>>
>> --Sean
>>
>> On 9/22/15 5:31 PM, Valerie Peng wrote:
>>> Hi, Sean,
>>>
>>> Thanks for the comments, I have changed to use the <> for line 110.
>>> For line 116-118, the difference is in the security check. With the
>>> current changes, the new provider will need to be granted
>>> |SecurityPermission("authProvider.name")| for the configuration call to
>>> complete and return the new provider.
>>>
>>> Or, one alternative is to not carrying over the callback handler if the
>>> particular permission is not granted for the new provider?
>>> Not sure if direct assignment/carryover would lead to anything bad,
>>> maybe I am being too paranoid...
>>>
>>> Thanks,
>>> Valerie
>>>
>>>
>>> On 9/22/2015 1:41 PM, Sean Mullan wrote:
>>>> On 09/18/2015 08:37 PM, Valerie Peng wrote:
>>>>> Sean,
>>>>>
>>>>> I have updated the webrev based on your suggestions and CCC has also
>>>>> been filed:
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~valeriep/8130648/webrev.00/
>>>>
>>>> SunPKCS11.java
>>>>
>>>> 110: can use diamond operator for anon classes now, ex:
>>>> PrivilegedExceptionAction<>
>>>
>>>>
>>>> 116-118: wouldn't it be easier to do the following:
>>>>
>>>> 116 if (this.pHandler != null) {
>>>> 117 newOne.pHandler = this.pHandler;
>>>> 118 }
>>>>
>>>> then you don't need the catch block on lines 124-128.
>>>>
>>>> --Sean
>>>>
>>>>
More information about the security-dev
mailing list