Sorry for the late reply. Comments below:
1. ReferralsState::handleError
cname = new PrincipalName(cname.getNameString().replaceAll(
PrincipalName.NAME_REALM_SEPARATOR + "", "\\\\" +
PrincipalName.NAME_REALM_SEPARATOR), cname.getNameType(),
referredRealm.toString());
Why is the escape necessary? Wouldn't the getNameString() result already
contain the backslash? Is it OK to use getNameStrings()?
2. EncKDCRepPart::init
if (der.getData().available() > 0) {
caddr = HostAddresses.parse(der.getData(), (byte) 0x0B, true);
}
if (der.getData().available() > 0) {
pAData = PAData.parseSequence(der.getData(), (byte) 0x0C, true);
}
Is this safe? What if caddr is missing but paData is there? Then the first
getData() returns paData and cannot be read correctly.
3. EncKDCRepPart::asn1Encode
What is the benefit of renaming bytes to out? I have to review all the lines to
make sure there is no problem.
4. KDCRep::msgType
Is this field used elsewhere? Why make it public?
5. Looks like we will soon need a KrbTgsReqBuilder.
6. CredentialsUtil.java:
- serviceCredsSingle. Why use this name? It could still call getTGTforRealm()
which triggers more TGS-REQ (and might call into serviceCredsReferrals). Also,
is the following check possibly false?
String[] serverAsCredsNames = asCreds.getServer().getNameStrings();
if (serverAsCredsNames.length == 2 &&
serverAsCredsNames[0].equals(
PrincipalName.TGS_DEFAULT_SRV_NAME)) {
Shouldn't asCreds's server always be some krbtgt?
- serviceCredsReferrals. This method can be called by every other TGS related
call. Is it necessary to try referral for S4U2self, S4U2proxy, etc?
PrincipalName cSname = (PrincipalName) sname.clone();
Not necessary. PrincipalName is immutable.
I'll look more into this file. These serviceCreds* methods are little
complicated.
Thanks,
Max
> On May 11, 2019, at 8:05 AM, Martin Balao <mba...@redhat.com> wrote:
>
> Hi,
>
> I'd like to propose Webrev.02:
>
> * http://cr.openjdk.java.net/~mbalao/webrevs/8215032/8215032.webrev.02/
>
> Security properties were introduced mirroring System properties. See CSR
> [1].
>
> Thanks,
> Martin.-
>
> --
> [1] - https://bugs.openjdk.java.net/browse/JDK-8223172
