Code review request: 8005523: Unbound krb5 for TLS

[Weijun Wang]

I've found something strange. The test has tried two SSL connections. 
When server is unbound, the client always requests for an initiate 
ServicePermission. When server is bound, only the first connection 
requests for the permission. Is it possible the server invalidate the 
SSLSession when it's unbound?

How can I trace it?

Thanks
Max


On 4/15/13 9:16 PM, Xuelei Fan wrote:
> On 4/15/2013 8:57 PM, Weijun Wang wrote:
>>>>       http://cr.openjdk.java.net/~weijun/8005523/webrev.01/
>>>>
>>>
>>> setupKerberosKeys() @ServerHandshaker.java:
>>> -------------------------------------------
>>> I would like to reserve the permission checking for bound krb5 here.
>>> The checking is done while parse ClientHello, and is useful to select
>>> the right one from a list of cipher suites.  It is too later to check it
>>> during KerberosClientKeyExchange.
>>
>> So it can fail early? That's OK.
>>
> It's more about to select the right cipher suite.  If it is not failed
> during cipher suite selection, the server may select a cipher suite that
> it cannot support, and does not have the chance to select a right cipher
> suite (e.g. non krb5 cipher suites). It's too bad.
>
>>>
>>> I think it is OK to get the krb5 principal for bound krb5, right?
>>
>> Yes.
>>
>>>
>>> BTW, how to set "accept" service policy for unbound krb5 in server side?
>>> We used to have a particular server principal. Is the "*" acceptable in
>>> policy configuration?
>>
>> "*" is acceptable in ServicePermission even before unbound krb5 is
>> introduced. It works nicely now.
>>
> I asked this question because I think we might not need to double check
> the permission in ClientKeyExchange.  Do we really need to double check
> the permissions in clientHello and ClientKeyExchange?
>
> Xuelei
>
>>>
>>>
>>> findKey() @KerberosClientKeyExchangeImpl.java
>>> --------------------------------------------------
>>> Thanks for the version matching update.
>>>
>>> I think it might be OK to remove this line:
>>>    449             //throw new KrbException(Krb5.KRB_AP_ERR_BADKEYVER);
>>
>> Haha. I'll remove it.
>>
>> I'm refining the test to make sure permissions are granted correctly.
>> Also, it seems there are useless requests made to server. Will double
>> check.
>>
>> Thanks
>> Max
>>
>>>
>>> Otherwise, looks fine to me.
>>>
>>> Xuelei
>>>
>>>> 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
>>>>>>>
>>>>>
>>>
>