There is still a kerberosTicketSetClientAlias() call in Krb5LoginModule.java. Otherwise looks fine.
I'll run some internal test now. Thanks, Max > On Jul 17, 2019, at 2:58 PM, Martin Balao <mba...@redhat.com> wrote: > > Hi Max, > > Webrev.02 is ready: > http://cr.openjdk.java.net/~mbalao/webrevs/8227437/8227437.webrev.02/ > > On 7/17/19 12:23 AM, Weijun Wang wrote: >> Krb5Context.java and Krb5LoginModule.java: >> >> There is no need to set the aliases, already done inside >> Krb5Util.credsToTicket(serviceCreds). The case in Krb5LoginModule.java is >> more complicated, due to your change in webrev.01. See below. >> > > Duplicated alias set removed from Krb5Context.java. Thanks. > > In regards to Krb5LoginModule.java, I made a mistake when generating > Webrev.01 (my idea was to set a client alias equal to "principal"). > However, I decided to remove it for Webrev.02. When we retrieve the > ticket from the cache, we use either "principal" or null (but then we > assign "principal" to whatever cname the ticket has). As a result, the > ticket cname will match "principal" and setting it as an alias adds > nothing. Sorry for the noise here. > >>> >>> * Fixed a bug in referral TGTs Credentials (no server alias should be >>> there) >>> * See this change in KrbTgsRep.java >> >> But this is not harmful, right? The referral TGTs are not stored in a >> Subject and its alias is not used. > > Right. Not harmful but not correct. Found it a bit confusing when > debugging. As you said, referral TGTs are not stored. > >> >>> * Fixed a bug in Krb5LoginModule >>> * If we get a TGT from a ccache, that TGT will not have client alias >>> and its cname may be different than the subject principal. If we commit >>> this ticket in the subject private credentials, we will not find it. >>> * My proposal is to force the subject principal as an alias if: cname >>> differs from the subject principal AND there is no client alias. >>> * See this change in Krb5LoginModule.java >> >> KerberosPrincipal kClient = new KerberosPrincipal( >> cred.getClient().getName()); >> if (!princSet.contains(kClient)) { >> kClientAlias = kClient; >> } >> >> Looks like this ticket's name and alias will be the same? Is this useful? >> > > Removed. See above. > >> >> Or, if CANONICALIZE is not set (who knows why), then there will be no alias >> and the alias is the name. > > Yes, if we send no CANONICALIZE (i.e.: because of the fallback scheme), > we will have no client alias but the ticket cname will then be equal to > the subject (kerberos) principal. > > Regression testing in jdk/sun/security/krb5 category passed. > > Are we good to go with Webrev.02? > > Thanks, > Martin.-