RFR 8130648: JCK test api/java_security/AuthProvider/ProviderTests_login starts failing after JDK-7191662

Valerie Peng valerie.peng at oracle.com
Fri Sep 25 02:28:43 UTC 2015


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