On 4/15/2013 2:18 PM, Weijun Wang wrote: > > > On 4/15/13 1:38 PM, Xuelei Fan wrote: >>>> Do you want to file a simple enhancement request (CCC)? >>> >>> Why CCC? This is all internal. >>> >> Yes, it is optional. I think, now it can accept unbound principal in >> server side, it is an enhancement. It would be nice to have the >> community and SQE know the improvement. > > I consider it to be a natural benefit of > http://ccc.us.oracle.com/8001104. The release note will mention SASL, > GSS-API, JAAS, TLS all. > OK.
>> >>>> >>>> . KerberosClientKeyExchangeImpl.java >>>> ------------------------------------ >>>> Do you want to check the return value to make sure it is non-null or >>>> empty? Otherwise, it is possible to run into NPE when using the >>>> serverKeys. >>>> >>>> 188 KerberosKey[] serverKeys = AccessController.doPrivileged( >>>> >>>> An IOException will be thrown if the principal is not matched. I think >>>> we need to reserve the behavior. > > I see. > > Webrev updated > > http://cr.openjdk.java.net/~weijun/8005523/webrev.01/ > > 3 changes: > > 1. Pass AccessControlContext instead of Handshaker (so Handshaker can be > protected again). > > 2. New IOE when there is no keys for princ in ServiceCreds > > 3. Enhance findkey in KerberosClientKeyExchangeImpl to include the logic > of 7197159: accept different kvno if there no match. > > 4. Smaller change in SSL.java the test. Add a case instead of change to > all. > 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. I think it is OK to get the krb5 principal for bound krb5, right? 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? 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); 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 >>>> >>