Code review request: 8005523: Unbound krb5 for TLS

[Weijun Wang]

Sorry, seems localPrincipal is the last requested service name set in 
clientKeyExchange.

On 4/16/13 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.
>
> I do have a way to check if the Subject contains unbound keytab objects.
> Is it worth trying?
>
> 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
>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>