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