RFR 8215032: Support Kerberos cross-realm referrals (RFC 6806)

Weijun Wang weijun.wang at oracle.com
Tue Jun 4 03:28:28 UTC 2019


Tiny comments:

- java.security typos:

   492,497: ovewritten
   496: infite

- CredentialsUtils.java:

   36: unused import

- KDCRep.java:

   no need to move the position

- ReferralsCache.java:

   Red Hat has different copyright lines. For example, in java.base. Maybe any one is OK. IANAL. Same with ReferralsTest.java.

share/classes/sun/security/ssl/ExtendedMasterSecretExtension.java:2: * Copyright (c) 2017, Red Hat, Inc. and/or its affiliates.
share/classes/jdk/internal/misc/UnsafeConstants.java:2: * Copyright (c) 2019, Red Hat Inc. All rights reserved.
share/classes/com/sun/crypto/provider/GHASH.java:3: * Copyright (c) 2015 Red Hat, Inc.

BTW, is it worth adding some comments here?

- TicketFlags.java:

   55: enc-pa-rep (15)
   Remove the whitespace before "(15)" to be consistent with others.

Everything else looks fine. You are free to push the change now.

Thanks,
Max

> On May 29, 2019, at 2:20 AM, Martin Balao <mbalao at redhat.com> wrote:
> 
> Hi Max,
> 
> Thanks for your feedback.
> 
> Here it's Webrev.03:
> 
> * http://cr.openjdk.java.net/~mbalao/webrevs/8215032/8215032.webrev.03/
> 
> Changes:
> 
> * msgType is now private
> 
> * Check in CredentialsUtil.java removed (always true)
> 
> * "PrincipalName cSname = (PrincipalName) sname.clone();" not necessary
> 
> * In CredentialsUtils.java, I added comments to methods "serviceCreds",
> "serviceCredsReferrals" and "serviceCredsSingle". I'll keep the "single"
> suffix but if there is a better one, we can change it. Hopefully this is
> clearer with the comments now and removing the unnecessary "if".
> 
> * "new PrincipalName(oldName.getNameString().replace..., newRealm)"
> replaced
> 
> Regression testing in "jdk/sun/security/krb5" passed.
> 
> On 5/24/19 9:23 PM, Weijun Wang wrote:
>>> 1)
>>> src/java.security.jgss/share/classes/sun/security/krb5/KrbAsReqBuilder.java:
>>> 
>>> * When NT-ENTERPRISE names are used, a "@" char can be part of the name
>>> and we should not interpret it as a realm separator. If we don't escape,
>>> we may be missing part of the name when building a new PrincipalName.
>>> The exact place where this happens is in PrincipalName.parseName function.
>> 
>> Is the character already escaped in the oldName? How about just changing
>> 
>>  new PrincipalName(oldName.getNameString().replace..., newRealm)
>> 
>> to
>> 
>>  new PrincipalName(oldName.getNameStrings(), newRealm)
>> 
>> 
> 
> Yes, this should be fine. Addressed in Webrev.03.
> 
>> 
>> It is also called by getTGTforRealm(). Is this for RFC 6806 "9.  Cross-Realm Routing"?
>> 
> 
> No, getTGTforRealm is part of the cross-realm operation described by RFC
> 4120. If we need to get a TGS for a service which is in a different
> realm than the TGT we have, we first ask for a TGT on the service's
> realm (using the original TGT) and then we ask for the TGS. The service
> realm is determined by previous configurations and heuristics.
> 
> My understanding is that getTGTforRealm is never needed when following
> RFC 6806 cross-realm referrals because the referral TGT we use for the
> next query is always for the next realm we are going to ask (the
> "service's realm"). That's how I see both things working together. A
> comment was added to "serviceCredsSingle" method explaining this.
> 
> Kind regards,
> Martin.-




More information about the security-dev mailing list