CVE-2017-6594: Improper Certificate Validation (CWE-295)
value: The transit path validation code in Heimdal before 7.3 might allow attackers to bypass the capath policy protection mechanism by leveraging failure to add the previous hop realm to the transit path of issued tickets.
CWE description: The product does not validate, or incorrectly validates, a certificate.
CVSS v3 base score: 7.5
Published: 2017-08-28T19:29Z
Repository: https://github.com/heimdal/heimdal
Language: Other
Files changed: NEWS, kdc/krb5tgs.c, tests/kdc/check-kdc.in, tests/kdc/krb5.conf.in
Commit date: 2017-04-13 18:06:39 -0500
Commit message: Fix transit path validation CVE-2017-6594

Commit f469fc6 (2010-10-02) inadvertently caused the previous hop realm
to not be added to the transit path of issued tickets.  This may, in
some cases, enable bypass of capath policy in Heimdal versions 1.5
through 7.2.

Note, this may break sites that rely on the bug.  With the bug some
incomplete [capaths] worked, that should not have.  These may now break
authentication in some cross-realm configurations.

Diff: diff --git a/NEWS b/NEWS
index ccc06ffc8..79efe803a 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,19 @@
+Release Notes - Heimdal - Version Heimdal 7.3
+
+ Security
+
+ - Fix transit path validation.  Commit f469fc6 (2010-10-02) inadvertently
+   caused the previous hop realm to not be added to the transit path
+   of issued tickets.  This may, in some cases, enable bypass of capath
+   policy in Heimdal versions 1.5 through 7.2.
+
+   Note, this may break sites that rely on the bug.  With the bug some
+   incomplete [capaths] worked, that should not have.  These may now break
+   authentication in some cross-realm configurations.
+   (CVE-2017-6594)
+
 Release Notes - Heimdal - Version Heimdal 7.2
 
  Bug fixes
  - Portability improvements
  - More strict parsing of encoded URI components in HTTP KDC
diff --git a/kdc/krb5tgs.c b/kdc/krb5tgs.c
index 6048b9c55..98503812f 100644
--- a/kdc/krb5tgs.c
+++ b/kdc/krb5tgs.c
@@ -653,12 +653,16 @@ fix_transited_encoding(krb5_context context,
     if(ret){
 	krb5_warn(context, ret,
 		  "Decoding transited encoding");
 	return ret;
     }
+
+    /*
+     * If the realm of the presented tgt is neither the client nor the server
+     * realm, it is a transit realm and must be added to transited set.
+     */
     if(strcmp(client_realm, tgt_realm) && strcmp(server_realm, tgt_realm)) {
-	/* not us, so add the previous realm to transited set */
 	if (num_realms + 1 > UINT_MAX/sizeof(*realms)) {
 	    ret = ERANGE;
 	    goto free_realms;
 	}
 	tmp = realloc(realms, (num_realms + 1) * sizeof(*realms));
@@ -735,10 +739,11 @@ tgs_make_reply(krb5_context context,
 	       hdb_entry_ex *server,
 	       krb5_principal server_principal,
 	       const char *server_name,
 	       hdb_entry_ex *client,
 	       krb5_principal client_principal,
+               const char *tgt_realm,
 	       hdb_entry_ex *krbtgt,
 	       krb5_enctype krbtgt_etype,
 	       krb5_principals spp,
 	       const krb5_data *rspac,
 	       const METHOD_DATA *enc_pa_data,
@@ -796,11 +801,11 @@ tgs_make_reply(krb5_context context,
 				    PRINCIPAL_ALLOW_DISABLE_TRANSITED_CHECK(server)) ||
 				   GLOBAL_ALLOW_DISABLE_TRANSITED_CHECK),
 				 &tgt->transited, &et,
 				 krb5_principal_get_realm(context, client_principal),
 				 krb5_principal_get_realm(context, server->entry.principal),
-				 krb5_principal_get_realm(context, krbtgt->entry.principal));
+				 tgt_realm);
     if(ret)
 	goto out;
 
     copy_Realm(&server_principal->realm, &rep.ticket.realm);
     _krb5_principal2principalname(&rep.ticket.sname, server_principal);
@@ -1517,10 +1522,12 @@ tgs_build_reply(krb5_context context,
     krb5_principals spp = NULL;
     const EncryptionKey *ekey;
     krb5_keyblock sessionkey;
     krb5_kvno kvno;
     krb5_data rspac;
+    const char *tgt_realm = /* Realm of TGT issuer */
+        krb5_principal_get_realm(context, krbtgt->entry.principal);
     const char *our_realm = /* Realm of this KDC */
         krb5_principal_get_comp_string(context, krbtgt->entry.principal, 1);
     char **capath = NULL;
     size_t num_capath = 0;
 
@@ -2322,10 +2329,11 @@ server_lookup:
 			 server,
 			 rsp,
 			 spn,
 			 client,
 			 cp,
+                         tgt_realm,
 			 krbtgt_out,
 			 tkey_sign->key.keytype,
 			 spp,
 			 &rspac,
 			 &enc_pa_data,
diff --git a/tests/kdc/check-kdc.in b/tests/kdc/check-kdc.in
index 962d5468a..b9174e4db 100644
--- a/tests/kdc/check-kdc.in
+++ b/tests/kdc/check-kdc.in
@@ -51,10 +51,11 @@ R2=TEST2.H5L.SE
 R3=TEST3.H5L.SE
 R4=TEST4.H5L.SE
 R5=SOME-REALM5.FR
 R6=SOME-REALM6.US
 R7=SOME-REALM7.UK
+R8=SOME-REALM8.UK
 
 H1=H1.$R
 H2=H2.$R
 H3=H3.$H2
 H4=H4.$H2
@@ -146,10 +147,16 @@ ${kadmin} \
     init \
     --realm-max-ticket-life=1day \
     --realm-max-renewable-life=1month \
     ${R7} || exit 1
 
+${kadmin} \
+    init \
+    --realm-max-ticket-life=1day \
+    --realm-max-renewable-life=1month \
+    ${R8} || exit 1
+
 ${kadmin} \
     init \
     --realm-max-ticket-life=1day \
     --realm-max-renewable-life=1month \
     ${H1} || exit 1
@@ -189,10 +196,11 @@ ${kadmin} add -p foo --use-defaults foo@${R2} || exit 1
 ${kadmin} add -p foo --use-defaults foo@${R3} || exit 1
 ${kadmin} add -p foo --use-defaults foo@${R4} || exit 1
 ${kadmin5} add -p foo --use-defaults foo@${R5} || exit 1
 ${kadmin} add -p foo --use-defaults foo@${R6} || exit 1
 ${kadmin} add -p foo --use-defaults foo@${R7} || exit 1
+${kadmin} add -p foo --use-defaults foo@${R8} || exit 1
 ${kadmin} add -p foo --use-defaults foo@${H1} || exit 1
 ${kadmin} add -p foo --use-defaults foo/host.${h1}@${H1} || exit 1
 ${kadmin} add -p foo --use-defaults foo@${H2} || exit 1
 ${kadmin} add -p foo --use-defaults foo/host.${h2}@${H2} || exit 1
 ${kadmin} add -p foo --use-defaults foo@${H3} || exit 1
@@ -247,10 +255,13 @@ ${kadmin5} add -p cross1 --use-defaults krbtgt/${R6}@${R5} || exit 1
 ${kadmin} add -p cross2 --use-defaults krbtgt/${R5}@${R6} || exit 1
 
 ${kadmin} add -p cross1 --use-defaults krbtgt/${R7}@${R6} || exit 1
 ${kadmin} add -p cross2 --use-defaults krbtgt/${R6}@${R7} || exit 1
 
+${kadmin} add -p cross1 --use-defaults krbtgt/${R8}@${R6} || exit 1
+${kadmin} add -p cross2 --use-defaults krbtgt/${R6}@${R8} || exit 1
+
 ${kadmin} add -p cross1 --use-defaults krbtgt/${H1}@${R} || exit 1
 ${kadmin} add -p cross2 --use-defaults krbtgt/${R}@${H1} || exit 1
 
 ${kadmin} add -p cross1 --use-defaults krbtgt/${H2}@${R} || exit 1
 ${kadmin} add -p cross2 --use-defaults krbtgt/${R}@${H2} || exit 1
@@ -282,10 +293,11 @@ ${kadmin} check ${R2} || exit 1
 ${kadmin} check ${R3} || exit 1
 ${kadmin} check ${R4} || exit 1
 ${kadmin5} check ${R5} || exit 1
 ${kadmin} check ${R6} || exit 1
 ${kadmin} check ${R7} || exit 1
+${kadmin} check ${R8} || exit 1
 ${kadmin} check ${H1} || exit 1
 ${kadmin} check ${H2} || exit 1
 ${kadmin} check ${H3} || exit 1
 ${kadmin} check ${H4} || exit 1
 
@@ -386,10 +398,12 @@ echo "Getting x-realm tickets with capaths for $R -> $R5"
 ${kgetcred} foo@${R5} || { ec=1 ; eval "${testfailed}"; }
 echo "Getting x-realm tickets with capaths for $R -> $R6"
 ${kgetcred} foo@${R6} || { ec=1 ; eval "${testfailed}"; }
 echo "Getting x-realm tickets with capaths for $R -> $R7"
 ${kgetcred} foo@${R7} || { ec=1 ; eval "${testfailed}"; }
+echo "Should not get x-realm tickets with capaths for $R -> $R8"
+${kgetcred} foo@${R8} && { ec=1 ; eval "${testfailed}"; }
 ${kdestroy}
 
 echo "Testing capaths logic (reverse order)"
 ${kinit} --password-file=${objdir}/foopassword \
     -e ${aesenctype} -e ${aesenctype} \
@@ -416,14 +430,17 @@ ${kinit} --password-file=${objdir}/foopassword \
     foo@${H3} || \
 	{ ec=1 ; eval "${testfailed}"; }
 
 echo "Getting x-realm tickets with hierarchical referrals for $H3 -> $H1"
 ${kgetcred} --hostbased --canonicalize foo host.${h1} || { ec=1 ; eval "${testfailed}"; }
+fgrep "cross-realm ${H3} -> ${H1} via [${H2}, ${R}]" messages.log > /dev/null || { ec=1 ; eval "${testfailed}"; }
 echo "Getting x-realm tickets with hierarchical referrals for $H3 -> $R"
 ${kgetcred} --hostbased --canonicalize foo host.${r} || { ec=1 ; eval "${testfailed}"; }
+fgrep "cross-realm ${H3} -> ${R} via [${H2}]" messages.log > /dev/null || { ec=1 ; eval "${testfailed}"; }
 echo "Getting x-realm tickets with hierarchical referrals for $H3 -> $H2"
 ${kgetcred} --hostbased --canonicalize foo host.${h2} || { ec=1 ; eval "${testfailed}"; }
+fgrep "cross-realm ${H3} -> ${H2}" messages.log > /dev/null || { ec=1 ; eval "${testfailed}"; }
 ${kdestroy}
 
 echo "Testing multi-hop [capaths] referral logic"
 ${kinit} --password-file=${objdir}/foopassword \
     -e ${aesenctype} -e ${aesenctype} \
diff --git a/tests/kdc/krb5.conf.in b/tests/kdc/krb5.conf.in
index cc2dedb2d..849e773d0 100644
--- a/tests/kdc/krb5.conf.in
+++ b/tests/kdc/krb5.conf.in
@@ -38,10 +38,13 @@
 		kdc = localhost:@port@
 	}
 	SOME-REALM7.UK = {
 		kdc = localhost:@port@
 	}
+	SOME-REALM8.UK = {
+		kdc = localhost:@port@
+	}
 	TEST-HTTP.H5L.SE = {
 		kdc = http/localhost:@port@
 	}
 	H1.TEST.H5L.SE = {
 		kdc = localhost:@port@
@@ -145,10 +148,11 @@
 		TEST4.H5L.SE = TEST2.H5L.SE
 		TEST4.H5L.SE = TEST3.H5L.SE
 		SOME-REALM6.US = SOME-REALM5.FR
 		SOME-REALM7.UK = SOME-REALM6.US
 		SOME-REALM7.UK = SOME-REALM5.FR
+		SOME-REALM8.UK = SOME-REALM6.US
 	}
         H4.H2.TEST.H5L.SE = {
                 H1.TEST.H5L.SE = H3.H2.TEST.H5L.SE
                 H1.TEST.H5L.SE = H2.TEST.H5L.SE
                 H1.TEST.H5L.SE = TEST.H5L.SE

Vulnerable code: 	/* not us, so add the previous realm to transited set */
				 krb5_principal_get_realm(context, krbtgt->entry.principal));
Fixed code: Release Notes - Heimdal - Version Heimdal 7.3

 Security

 - Fix transit path validation.  Commit f469fc6 (2010-10-02) inadvertently
   caused the previous hop realm to not be added to the transit path
   of issued tickets.  This may, in some cases, enable bypass of capath
   policy in Heimdal versions 1.5 through 7.2.

   Note, this may break sites that rely on the bug.  With the bug some
   incomplete [capaths] worked, that should not have.  These may now break
   authentication in some cross-realm configurations.
   (CVE-2017-6594)


    /*
     * If the realm of the presented tgt is neither the client nor the server
     * realm, it is a transit realm and must be added to transited set.
     */
               const char *tgt_realm,
				 tgt_realm);
    const char *tgt_realm = /* Realm of TGT issuer */
        krb5_principal_get_realm(context, krbtgt->entry.principal);
                         tgt_realm,
R8=SOME-REALM8.UK
${kadmin} \
    init \
    --realm-max-ticket-life=1day \
    --realm-max-renewable-life=1month \
    ${R8} || exit 1

${kadmin} add -p foo --use-defaults foo@${R8} || exit 1
${kadmin} add -p cross1 --use-defaults krbtgt/${R8}@${R6} || exit 1
${kadmin} add -p cross2 --use-defaults krbtgt/${R6}@${R8} || exit 1

${kadmin} check ${R8} || exit 1
echo "Should not get x-realm tickets with capaths for $R -> $R8"
${kgetcred} foo@${R8} && { ec=1 ; eval "${testfailed}"; }
fgrep "cross-realm ${H3} -> ${H1} via [${H2}, ${R}]" messages.log > /dev/null || { ec=1 ; eval "${testfailed}"; }
fgrep "cross-realm ${H3} -> ${R} via [${H2}]" messages.log > /dev/null || { ec=1 ; eval "${testfailed}"; }
fgrep "cross-realm ${H3} -> ${H2}" messages.log > /dev/null || { ec=1 ; eval "${testfailed}"; }
	SOME-REALM8.UK = {
		kdc = localhost:@port@
	}
		SOME-REALM8.UK = SOME-REALM6.US