Code review request: 8005523: Unbound krb5 for TLS

[Weijun Wang]

>>      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.

>
> 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.

>
>
> 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
>>>>>
>>>
>