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