Hi Max, Thanks for your review.
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. 2) src/java.security.jgss/share/classes/sun/security/krb5/internal/EncKDCRepPart.java: * My understanding is that there won't be confusion between caddr and padata because the tag for each one is different (0x0B and 0x0C). When the tag does not match, "parse" functions return null and execution continues. See for example HostAddress.parse or PAData.parseSequence methods. All the previous "optionals" work the same way. 3) src/java.security.jgss/share/classes/sun/security/krb5/internal/EncKDCRepPart.java: * This was a trivial renaming because I found more clear to have "out" for the final output and "temp / bytes" for intermediate outputs. I can revert this change and use a different name scheme that does not modify the original "bytes" variable if you wish. 4) src/java.security.jgss/share/classes/sun/security/krb5/internal/KDCRep.java: * msgType being public is no longer needed (it was needed in a previous changeset). I'll fix it for Webrev.03. 5) KrbTgsReqBuilder * Yes, probably. 6) src/java.security.jgss/share/classes/sun/security/krb5/internal/CredentialsUtil.java * Perhaps "serviceCredsSingle" is not the best name. What I meant with single was in respect to cross-realm referrals (which are not handled in that method). getTGTforRealm won't be called when we get a cross-realm referral because there will be a match between the sname realm and the realm we will ask for a ticket (both elements are part of the cross-realm TGT). * In regards to the check being possibly false, you are probably correct. Let me do some checking and I'll fix it for Webrev.03. * In regards to cross-realm referrals for S4U2self and S4U2proxy, this will be my next step when proposing a patch for resource-based constrained delegation. * "PrincipalName cSname = (PrincipalName) sname.clone();" Right, I'll fix this for Webrev.03. Kind regards, Martin.-
