Code review request: 8005523: Unbound krb5 for TLS

[Weijun Wang]

On 4/16/13 5:34 PM, Xuelei Fan wrote:
> On 4/16/2013 5:30 PM, Weijun Wang wrote:
>> The problem is inside ServerHandshaker::clientHello:
>>
>>      if (subject != null) {
>>          // Eliminate dependency on KerberosPrincipal
>>          Set<Principal> principals =
>>              subject.getPrincipals(Principal.class);
>>          if (!principals.contains(localPrincipal)) {
>>              resumingSession = false;
>>              if (debug != null && Debug.isOn("session")) {
>>                  System.out.println("Subject identity" +
>>                                      " is not the same");
>>              }
>>          } else {
>>              if (debug != null && Debug.isOn("session"))
>>                  System.out.println("Subject identity" +
>>                                      " is same");
>>          }
>>      } else {
>>
>> Here, localPrincipal is the requested service name, and principals is
>> empty (unbound). Therefore resuming never succeeds.
>>
> Good catch!
>
>> I do have a way to check if the Subject contains unbound keytab objects.
>> Is it worth trying?
>>
> I think we should allow session resume for unbound principal. It is
> essential for performance consideration.

I am not sure how to do that. Can I just skip this check and whenever 
subject != null always set resumingSession to true? This is not very 
correct but is it possible to detect the mismatch later and "resume" the 
full negotiation?

-Max

>
> Thanks,
> Xuelei
>
>> Thanks
>> Max
>>
>>
>> On 4/16/13 1:38 PM, Xuelei Fan wrote:
>>> Using JSSE debug option, system property "javax.net.debug=all", and
>>> check the log about whether the session is resumed or not.  If you want
>>> the check in test code, it may be ok to check the session ID
>>> (SSLSession.getId()).
>>>
>>> Xuelei
>>>
>>> On 4/16/2013 1:11 PM, Weijun Wang wrote:
>>>> 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
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
>