From 2883bccf48f7a63c3635a0792138c5481050966f Mon Sep 17 00:00:00 2001 From: Martin Balao Date: Sat, 28 Mar 2020 19:41:10 -0300 Subject: [PATCH] 8239385: KerberosTicket client name refers wrongly to sAMAccountName in AD Reviewed-by: weijun --- .../classes/sun/security/krb5/Config.java | 3 +- .../sun/security/krb5/KrbAsReqBuilder.java | 93 ++++++++++++++----- .../classes/sun/security/krb5/KrbKdcRep.java | 18 ++-- .../sun/security/krb5/auto/ReferralsTest.java | 40 +++++++- 4 files changed, 120 insertions(+), 34 deletions(-) diff --git a/src/java.security.jgss/share/classes/sun/security/krb5/Config.java b/src/java.security.jgss/share/classes/sun/security/krb5/Config.java index 8c34be94db7..62abc719ad3 100644 --- a/src/java.security.jgss/share/classes/sun/security/krb5/Config.java +++ b/src/java.security.jgss/share/classes/sun/security/krb5/Config.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -154,6 +154,7 @@ public static void refresh() throws KrbException { KdcComm.initStatic(); EType.initStatic(); Checksum.initStatic(); + KrbAsReqBuilder.ReferralsState.initStatic(); } diff --git a/src/java.security.jgss/share/classes/sun/security/krb5/KrbAsReqBuilder.java b/src/java.security.jgss/share/classes/sun/security/krb5/KrbAsReqBuilder.java index c626cfda8e2..bdd70d64911 100644 --- a/src/java.security.jgss/share/classes/sun/security/krb5/KrbAsReqBuilder.java +++ b/src/java.security.jgss/share/classes/sun/security/krb5/KrbAsReqBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2010, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -278,7 +278,9 @@ private KrbAsReq build(EncryptionKey key, ReferralsState referralsState) } options = (options == null) ? new KDCOptions() : options; if (referralsState.isEnabled()) { - options.set(KDCOptions.CANONICALIZE, true); + if (referralsState.sendCanonicalize()) { + options.set(KDCOptions.CANONICALIZE, true); + } extraPAs = new PAData[]{ new PAData(Krb5.PA_REQ_ENC_PA_REP, new byte[]{}) }; } else { @@ -333,7 +335,7 @@ private KrbAsReqBuilder send() throws KrbException, IOException { boolean preAuthFailedOnce = false; KdcComm comm = null; EncryptionKey pakey = null; - ReferralsState referralsState = new ReferralsState(); + ReferralsState referralsState = new ReferralsState(this); while (true) { if (referralsState.refreshComm()) { comm = new KdcComm(refCname.getRealmAsString()); @@ -379,43 +381,88 @@ private KrbAsReqBuilder send() throws KrbException, IOException { } } - private final class ReferralsState { + static final class ReferralsState { + private static boolean canonicalizeConfig; private boolean enabled; + private boolean sendCanonicalize; + private boolean isEnterpriseCname; private int count; private boolean refreshComm; + private KrbAsReqBuilder reqBuilder; + + static { + initStatic(); + } - ReferralsState() throws KrbException { - if (Config.DISABLE_REFERRALS) { - if (refCname.getNameType() == PrincipalName.KRB_NT_ENTERPRISE) { - throw new KrbException("NT-ENTERPRISE principals only allowed" + - " when referrals are enabled."); + // Config may be refreshed while running so the setting + // value may need to be updated. See Config::refresh. + static void initStatic() { + canonicalizeConfig = false; + try { + canonicalizeConfig = Config.getInstance() + .getBooleanObject("libdefaults", "canonicalize") == + Boolean.TRUE; + } catch (KrbException e) { + if (Krb5.DEBUG) { + System.out.println("Exception in getting canonicalize," + + " using default value " + + Boolean.valueOf(canonicalizeConfig) + ": " + + e.getMessage()); } - enabled = false; - } else { - enabled = true; + } + } + + ReferralsState(KrbAsReqBuilder reqBuilder) throws KrbException { + this.reqBuilder = reqBuilder; + sendCanonicalize = canonicalizeConfig; + isEnterpriseCname = reqBuilder.refCname.getNameType() == + PrincipalName.KRB_NT_ENTERPRISE; + updateStatus(); + if (!enabled && isEnterpriseCname) { + throw new KrbException("NT-ENTERPRISE principals only" + + " allowed when referrals are enabled."); } refreshComm = true; } + private void updateStatus() { + enabled = !Config.DISABLE_REFERRALS && + (isEnterpriseCname || sendCanonicalize); + } + boolean handleError(KrbException ke) throws RealmException { if (enabled) { if (ke.returnCode() == Krb5.KRB_ERR_WRONG_REALM) { Realm referredRealm = ke.getError().getClientRealm(); - if (req.getMessage().reqBody.kdcOptions.get(KDCOptions.CANONICALIZE) && - referredRealm != null && referredRealm.toString().length() > 0 && + if (referredRealm != null && + !referredRealm.toString().isEmpty() && count < Config.MAX_REFERRALS) { - refCname = new PrincipalName(refCname.getNameType(), - refCname.getNameStrings(), referredRealm); + // A valid referral was received while referrals + // were enabled. Change the cname realm to the referred + // realm and set refreshComm to send a new request. + reqBuilder.refCname = new PrincipalName( + reqBuilder.refCname.getNameType(), + reqBuilder.refCname.getNameStrings(), + referredRealm); refreshComm = true; count++; return true; } } - if (count < Config.MAX_REFERRALS && - refCname.getNameType() != PrincipalName.KRB_NT_ENTERPRISE) { - // Server may raise an error if CANONICALIZE is true. - // Try CANONICALIZE false. - enabled = false; + if (count < Config.MAX_REFERRALS && sendCanonicalize) { + if (Krb5.DEBUG) { + System.out.println("KrbAsReqBuilder: AS-REQ failed." + + " Retrying with CANONICALIZE false."); + } + + // Server returned an unexpected error with + // CANONICALIZE true. Retry with false. + sendCanonicalize = false; + + // Setting CANONICALIZE to false may imply that referrals + // are now disabled (if cname is not of NT-ENTERPRISE type). + updateStatus(); + return true; } } @@ -431,6 +478,10 @@ boolean refreshComm() { boolean isEnabled() { return enabled; } + + boolean sendCanonicalize() { + return sendCanonicalize; + } } /** diff --git a/src/java.security.jgss/share/classes/sun/security/krb5/KrbKdcRep.java b/src/java.security.jgss/share/classes/sun/security/krb5/KrbKdcRep.java index 79ee135b55d..762d9d3ba51 100644 --- a/src/java.security.jgss/share/classes/sun/security/krb5/KrbKdcRep.java +++ b/src/java.security.jgss/share/classes/sun/security/krb5/KrbKdcRep.java @@ -44,11 +44,13 @@ static void check( ) throws KrbApErrException { // cname change in AS-REP is allowed only if the client - // sent CANONICALIZE and the server supports RFC 6806 - Section 11 - // FAST scheme (ENC-PA-REP flag). + // sent CANONICALIZE or an NT-ENTERPRISE cname in the request, and the + // server supports RFC 6806 - Section 11 FAST scheme (ENC-PA-REP flag). if (isAsReq && !req.reqBody.cname.equals(rep.cname) && - (!req.reqBody.kdcOptions.get(KDCOptions.CANONICALIZE) || - !rep.encKDCRepPart.flags.get(Krb5.TKT_OPTS_ENC_PA_REP))) { + ((!req.reqBody.kdcOptions.get(KDCOptions.CANONICALIZE) && + req.reqBody.cname.getNameType() != + PrincipalName.KRB_NT_ENTERPRISE) || + !rep.encKDCRepPart.flags.get(Krb5.TKT_OPTS_ENC_PA_REP))) { rep.encKDCRepPart.key.destroy(); throw new KrbApErrException(Krb5.KRB_AP_ERR_MODIFIED); } @@ -138,14 +140,16 @@ static void check( } // RFC 6806 - Section 11 mechanism check - if (rep.encKDCRepPart.flags.get(Krb5.TKT_OPTS_ENC_PA_REP) && - req.reqBody.kdcOptions.get(KDCOptions.CANONICALIZE)) { + // The availability of the ENC-PA-REP flag in the KDC response is + // mandatory on some cases (see Krb5.TKT_OPTS_ENC_PA_REP check above). + if (rep.encKDCRepPart.flags.get(Krb5.TKT_OPTS_ENC_PA_REP)) { boolean reqPaReqEncPaRep = false; boolean repPaReqEncPaRepValid = false; - // PA_REQ_ENC_PA_REP only required for AS requests for (PAData pa : req.pAData) { if (pa.getType() == Krb5.PA_REQ_ENC_PA_REP) { + // The KDC supports RFC 6806 and ENC-PA-REP was sent in + // the request (AS-REQ). A valid checksum is now required. reqPaReqEncPaRep = true; break; } diff --git a/test/jdk/sun/security/krb5/auto/ReferralsTest.java b/test/jdk/sun/security/krb5/auto/ReferralsTest.java index a6e81268f05..eb29f8eb80a 100644 --- a/test/jdk/sun/security/krb5/auto/ReferralsTest.java +++ b/test/jdk/sun/security/krb5/auto/ReferralsTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, Red Hat, Inc. + * Copyright (c) 2019, 2020, Red Hat, Inc. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -38,15 +38,19 @@ import java.util.Set; import javax.security.auth.kerberos.KerberosTicket; import javax.security.auth.Subject; +import javax.security.auth.login.LoginException; import org.ietf.jgss.GSSName; import sun.security.jgss.GSSUtil; +import sun.security.krb5.Config; import sun.security.krb5.PrincipalName; public class ReferralsTest { private static final boolean DEBUG = true; private static final String krbConfigName = "krb5-localkdc.conf"; + private static final String krbConfigNameNoCanonicalize = + "krb5-localkdc-nocanonicalize.conf"; private static final String realmKDC1 = "RABBIT.HOLE"; private static final String realmKDC2 = "DEV.RABBIT.HOLE"; private static final char[] password = "123qwe@Z".toCharArray(); @@ -100,6 +104,7 @@ public static void main(String[] args) throws Exception { testDelegation(); testImpersonation(); testDelegationWithReferrals(); + testNoCanonicalize(); } finally { cleanup(); } @@ -139,14 +144,20 @@ private static void initializeKDCs() throws Exception { kdc2.setOption(KDC.Option.ALLOW_S4U2PROXY, mapKDC2); KDC.saveConfig(krbConfigName, kdc1, kdc2, - "forwardable=true"); + "forwardable=true", "canonicalize=true"); + KDC.saveConfig(krbConfigNameNoCanonicalize, kdc1, kdc2, + "forwardable=true"); System.setProperty("java.security.krb5.conf", krbConfigName); } private static void cleanup() { - File f = new File(krbConfigName); - if (f.exists()) { - f.delete(); + String[] configFiles = new String[]{krbConfigName, + krbConfigNameNoCanonicalize}; + for (String configFile : configFiles) { + File f = new File(configFile); + if (f.exists()) { + f.delete(); + } } } @@ -325,4 +336,23 @@ private static void testDelegationWithReferrals() throws Exception { throw new Exception("Unexpected initiator or acceptor names"); } } + + /* + * The client tries to get a TGT (AS protocol) as in testSubjectCredentials + * but without the canonicalize setting in krb5.conf. The KDC + * must not return a referral but a failure because the client + * is not in the local database. + */ + private static void testNoCanonicalize() throws Exception { + System.setProperty("java.security.krb5.conf", + krbConfigNameNoCanonicalize); + Config.refresh(); + try { + Context.fromUserPass(new Subject(), + clientKDC1Name, password, false); + throw new Exception("should not succeed"); + } catch (LoginException e) { + // expected + } + } }