Code review request: 8005523: Unbound krb5 for TLS

[Weijun Wang]

On 4/15/13 1:38 PM, Xuelei Fan wrote:
>>> Do you want to file a simple enhancement request (CCC)?
>>
>> Why CCC? This is all internal.
>>
> Yes, it is optional. I think, now it can accept unbound principal in
> server side, it is an enhancement. It would be nice to have the
> community and SQE know the improvement.

I consider it to be a natural benefit of 
http://ccc.us.oracle.com/8001104. The release note will mention SASL, 
GSS-API, JAAS, TLS all.

>
>>>
>>> . KerberosClientKeyExchangeImpl.java
>>> ------------------------------------
>>> Do you want to check the return value to make sure it is non-null or
>>> empty? Otherwise, it is possible to run into NPE when using the
>>> serverKeys.
>>>
>>> 188  KerberosKey[] serverKeys = AccessController.doPrivileged(
>>>
>>> An IOException will be thrown if the principal is not matched. I think
>>> we need to reserve the behavior.

I see.

Webrev updated

     http://cr.openjdk.java.net/~weijun/8005523/webrev.01/

3 changes:

1. Pass AccessControlContext instead of Handshaker (so Handshaker can be 
protected again).

2. New IOE when there is no keys for princ in ServiceCreds

3. Enhance findkey in KerberosClientKeyExchangeImpl to include the logic 
of 7197159: accept different kvno if there no match.

4. Smaller change in SSL.java the test. Add a case instead of change to all.

Thanks
Max

>>
>> If the returned serverKeys is empty (it won't be null), line 208 will
>> return a null and line 213 will throw the IOE. Is that enough?
> The exception message will be confusing if the check is done in line 208
> and 213.  I like to show principal mismatch message when using bound
> principals.
>
> Xuelei
>
>>>
>>>
>>> Is it possible to add a new test for the unbound krb5 in TLS?
>>
>> It's already there. Note the "principal=*" in the updated SSL.java test.
>> Maybe I can provide 2 test cases, one bound, one unbound.
>>
>> Thanks
>> Max
>>
>>>
>>> Thanks,
>>> Xuelei
>>>
>>> On 4/1/2013 9:16 PM, Weijun Wang wrote:
>>>> Ping again.
>>>>
>>>> On 3/14/13 4:42 PM, Weijun Wang wrote:
>>>>> Hi Xuelei
>>>>>
>>>>> You might know that krb5 now supports unbound acceptor, which means if
>>>>> you set "principal=*" in an acceptor's JAAS login config file, it can
>>>>> serve as any service. The acceptor would read initiator's request, find
>>>>> out what the intended service name is, and then find a key for it from
>>>>> its keytab file.
>>>>>
>>>>> Currently TLS's krb5 ciphersuites must know the service principal at
>>>>> the
>>>>> beginning, it uses the info to read keys and then wait for incoming
>>>>> requests. This must be changed if it also want to be "unbound".
>>>>>
>>>>> I have a primitive patch here
>>>>>
>>>>>       http://cr.openjdk.java.net/~weijun/8005523/webrev.00
>>>>>
>>>>> You can see it gets a ServiceCreds instead of KerberosKey at the
>>>>> beginning. This ServiceCreds encapsulates keytabs and JAAS settings,
>>>>> and
>>>>> it can be used to find keys for any service name later.
>>>>>
>>>>> The fix is quite ugly. Especially, I make Handshaker public and pass it
>>>>> to KerberosClientKeyExchangeImpl so that its context can be used to
>>>>> check permissions. Is this necessary? I mean, is the context any
>>>>> different from the one inside KerberosClientKeyExchangeImpl?
>>>>>
>>>>> Thanks
>>>>> Max
>>>
>