changeset 61070:142d090cab77

8239385: KerberosTicket client name refers wrongly to sAMAccountName in AD Reviewed-by: weijun
author mbalao
date Sat, 28 Mar 2020 19:41:10 -0300
parents cf9358cd555b
children d330734e1189
files src/java.security.jgss/share/classes/sun/security/krb5/Config.java src/java.security.jgss/share/classes/sun/security/krb5/KrbAsReqBuilder.java src/java.security.jgss/share/classes/sun/security/krb5/KrbKdcRep.java test/jdk/sun/security/krb5/auto/ReferralsTest.java
diffstat 4 files changed, 120 insertions(+), 34 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.security.jgss/share/classes/sun/security/krb5/Config.java	Tue May 05 18:34:39 2020 +0100
+++ b/src/java.security.jgss/share/classes/sun/security/krb5/Config.java	Sat Mar 28 19:41:10 2020 -0300
@@ -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 @@
         KdcComm.initStatic();
         EType.initStatic();
         Checksum.initStatic();
+        KrbAsReqBuilder.ReferralsState.initStatic();
     }
 
 
--- a/src/java.security.jgss/share/classes/sun/security/krb5/KrbAsReqBuilder.java	Tue May 05 18:34:39 2020 +0100
+++ b/src/java.security.jgss/share/classes/sun/security/krb5/KrbAsReqBuilder.java	Sat Mar 28 19:41:10 2020 -0300
@@ -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 @@
         }
         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 @@
         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 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;
 
-        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.");
+        static {
+            initStatic();
+        }
+
+        // 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 isEnabled() {
             return enabled;
         }
+
+        boolean sendCanonicalize() {
+            return sendCanonicalize;
+        }
     }
 
     /**
--- a/src/java.security.jgss/share/classes/sun/security/krb5/KrbKdcRep.java	Tue May 05 18:34:39 2020 +0100
+++ b/src/java.security.jgss/share/classes/sun/security/krb5/KrbKdcRep.java	Sat Mar 28 19:41:10 2020 -0300
@@ -44,11 +44,13 @@
                       ) 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 @@
         }
 
         // 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;
                 }
--- a/test/jdk/sun/security/krb5/auto/ReferralsTest.java	Tue May 05 18:34:39 2020 +0100
+++ b/test/jdk/sun/security/krb5/auto/ReferralsTest.java	Sat Mar 28 19:41:10 2020 -0300
@@ -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 @@
             testDelegation();
             testImpersonation();
             testDelegationWithReferrals();
+            testNoCanonicalize();
         } finally {
             cleanup();
         }
@@ -139,14 +144,20 @@
         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 @@
             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
+        }
+    }
 }