Cross realm S4U2Self cont.

Isaac Boukris iboukris at gmail.com
Tue Sep 4 14:40:48 UTC 2018


Hi,

On Sun, Aug 26, 2018 at 2:06 PM Andrew Bartlett <abartlet at samba.org> wrote:
>
> On Sun, 2018-08-26 at 13:44 +0530, Isaac Boukris via samba-technical
> wrote:
> > I hope to update the patch-set soon, with the change I mentioned in
> > the last email (for clarity), and will then also summaries the work
> > and insights so far.
>
> The aim of course it to get upstream Heimdal and Samba's Heimdal
> (lorikeet-heimdal) to be the same thing, but in the short term my next
> task is to try and at least reduce the diff.


I have reworked the kdc fix as I said, see new patch attached and on gitlab:
https://gitlab.com/samba-team/devel/samba/commits/iboukris_xrealm_s4u2self_new

In an effort to make it easier to minimize the diff with Heimdal
upstream, I ported all the changes to upstream (see PR #403) and then
ported it back to samba's heimdal, and noted the necessary adjustments
(see comments below).

Here's a summary of the commits.

The first two are not directly related, but needed to run the blackbox tests:
1b0b64ba779 net-ads-search: use specified user when given
b41b05ca53d net-ads-search: fix using machine account on a domain controller

The next two are bugs I noticed in samba kdc before any of my changes:
359b41e04cc tgs_build_reply: fix possible use after free of krbtgt_principal
dafde4e3c0f krb5tgs: use string representation in logs

The following six are back-ported from heimdal's new PR #403, most of
them apply cleanly but some needed adjustments:
4caab5c38fe heimdal: Let verify and sign PAC including the realm in logon-name
4ab222049fd heimdal: kdc: split check_PAC function
95c3f77dec6 heimdal: kdc: factor out parsing of padata_for_user to its
own  function
fd28dff006d heimdal: kdc: fix cross realm S4U2Self processing;
[heimdal port note: changed krbtgt_out_principal to krbtgt_principal,
added NULL parameter to _kdc_pac_generate and slightly updated the
cleanup section.]

5f30eb45a6c heimdal: s4u2self: lookup enterprise principal's realm
ec5b106b118 heimdal: s4u2self: fix service code in cross realm trust;
[heimdal port note: changed start_realm to client_realm, added
referral_tgts required by current API and its cleanup.]

And at last, torture test adjustments and the new tests:
465f2b6b38d s4u2self: adjust krb5_canon torture tests to the cross realm changes
34d2c0e3dda s4u2self: add cross realm blackbox tests

Thanks!
-------------- next part --------------
From 1b0b64ba779f80b4fa003dcc8ed267f3d56457d7 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Mon, 22 Jan 2018 18:40:17 +0000
Subject: [PATCH 01/12] net-ads-search: use specified user when given

This fixes 'net ads search -U' on a domain controller.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source3/utils/net_ads.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/utils/net_ads.c b/source3/utils/net_ads.c
index afe47dad839..d050964dd5e 100644
--- a/source3/utils/net_ads.c
+++ b/source3/utils/net_ads.c
@@ -305,7 +305,8 @@ retry:
 		}
        }
 
-	status = ads_connect(ads);
+	status = c->opt_user_specified ? ads_connect_user_creds(ads) :
+					 ads_connect(ads);
 
 	if (!ADS_ERR_OK(status)) {
 
-- 
2.14.3


From b41b05ca53dd3b929f1be091edd2c7c8eb9fd4c3 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Mon, 23 Jul 2018 13:28:18 +0300
Subject: [PATCH 02/12] net-ads-search: fix using machine account on a domain
 controller

Set opt_user_specified to true once setting the machine name at
opt_user_name. This helps for the net tool to work on a DC with
machine credentials (-P), by acquiring credentials for host$@realm
(same as on domain member) instead of trying to acquire creds
for lp_workgroup()@realm and failing.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source3/utils/net_util.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/utils/net_util.c b/source3/utils/net_util.c
index a84b4f5500e..0a15ff0274f 100644
--- a/source3/utils/net_util.c
+++ b/source3/utils/net_util.c
@@ -279,6 +279,7 @@ int net_use_krb_machine_account(struct net_context *c)
 		return -1;
 	}
 	c->opt_user_name = user_name;
+	c->opt_user_specified = true;
 	return 0;
 }
 
-- 
2.14.3


From 359b41e04ccf24e7508a94f13835585148290536 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Sat, 21 Jul 2018 09:03:13 +0300
Subject: [PATCH 03/12] tgs_build_reply: fix possible use after free of
 krbtgt_principal

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/kdc/krb5tgs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index a888788bb6f..a4ebecba077 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -1795,7 +1795,6 @@ server_lookup:
     }
 
     ret = _kdc_db_fetch(context, config, krbtgt_principal, HDB_F_GET_KRBTGT, NULL, NULL, &krbtgt_out);
-    krb5_free_principal(context, krbtgt_principal);
     if (ret) {
 	krb5_error_code ret2;
 	char *ktpn, *ktpn2;
@@ -2303,6 +2302,8 @@ out:
 	krb5_free_principal(context, dp);
     if (sp)
 	krb5_free_principal(context, sp);
+    if (krbtgt_principal)
+        krb5_free_principal(context, krbtgt_principal);
     if (ref_realm)
 	free(ref_realm);
     free_METHOD_DATA(&enc_pa_data);
-- 
2.14.3


From dafde4e3c0fc8a690f5460a51247c7d0f2edcf2a Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Wed, 25 Jul 2018 18:16:37 +0300
Subject: [PATCH 04/12] krb5tgs: use string representation in logs

Found by binary printouts in logs.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/kdc/krb5tgs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index a4ebecba077..c5456e4cad9 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -1627,7 +1627,7 @@ server_lookup:
 			NULL, NULL, &server);
 
     if(ret == HDB_ERR_NOT_FOUND_HERE) {
-	kdc_log(context, config, 5, "target %s does not have secrets at this KDC, need to proxy", sp);
+	kdc_log(context, config, 5, "target %s does not have secrets at this KDC, need to proxy", spn);
 	goto out;
     } else if (ret == HDB_ERR_WRONG_REALM) {
 	if (ref_realm)
-- 
2.14.3


From 4caab5c38fec01b5dc29cb4f019776c2a4529fca Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Tue, 4 Sep 2018 11:16:59 +0300
Subject: [PATCH 05/12] heimdal: Let verify and sign PAC including the realm in
 logon-name

Needed for cross realm S4U2Self tickets.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/lib/krb5/pac.c              | 67 +++++++++++++++++++++++------
 source4/heimdal/lib/krb5/version-script.map |  2 +
 2 files changed, 56 insertions(+), 13 deletions(-)

diff --git a/source4/heimdal/lib/krb5/pac.c b/source4/heimdal/lib/krb5/pac.c
index 26aae107b32..55028307b3d 100644
--- a/source4/heimdal/lib/krb5/pac.c
+++ b/source4/heimdal/lib/krb5/pac.c
@@ -592,7 +592,8 @@ verify_logonname(krb5_context context,
 		 const struct PAC_INFO_BUFFER *logon_name,
 		 const krb5_data *data,
 		 time_t authtime,
-		 krb5_const_principal principal)
+		 krb5_const_principal principal,
+		 krb5_boolean with_realm)
 {
     krb5_error_code ret;
     uint32_t time1, time2;
@@ -601,6 +602,7 @@ verify_logonname(krb5_context context,
     char *s = NULL;
     char *principal_string = NULL;
     char *logon_string = NULL;
+    int parse_flags;
 
     sp = krb5_storage_from_readonly_mem((const char *)data->data + logon_name->offset_lo,
 					logon_name->buffersize);
@@ -678,10 +680,12 @@ verify_logonname(krb5_context context,
 	    return ret;
 	}
     }
-    ret = krb5_unparse_name_flags(context, principal,
-				  KRB5_PRINCIPAL_UNPARSE_NO_REALM |
-				  KRB5_PRINCIPAL_UNPARSE_DISPLAY,
-				  &principal_string);
+
+    parse_flags = KRB5_PRINCIPAL_UNPARSE_DISPLAY;
+    if (!with_realm)
+	parse_flags |= KRB5_PRINCIPAL_UNPARSE_NO_REALM;
+
+    ret = krb5_unparse_name_flags(context, principal, parse_flags, &principal_string);
     if (ret) {
 	free(logon_string);
 	return ret;
@@ -708,13 +712,15 @@ static krb5_error_code
 build_logon_name(krb5_context context,
 		 time_t authtime,
 		 krb5_const_principal principal,
-		 krb5_data *logon)
+		 krb5_data *logon,
+		 krb5_boolean with_realm)
 {
     krb5_error_code ret;
     krb5_storage *sp;
     uint64_t t;
     char *s, *s2;
     size_t s2_len;
+    int parse_flags;
 
     t = unix2nttime(authtime);
 
@@ -729,10 +735,11 @@ build_logon_name(krb5_context context,
     CHECK(ret, krb5_store_uint32(sp, t & 0xffffffff), out);
     CHECK(ret, krb5_store_uint32(sp, t >> 32), out);
 
-    ret = krb5_unparse_name_flags(context, principal,
-				  KRB5_PRINCIPAL_UNPARSE_NO_REALM |
-				  KRB5_PRINCIPAL_UNPARSE_DISPLAY,
-				  &s);
+    parse_flags = KRB5_PRINCIPAL_UNPARSE_DISPLAY;
+    if (!with_realm)
+	parse_flags |= KRB5_PRINCIPAL_UNPARSE_NO_REALM;
+
+    ret = krb5_unparse_name_flags(context, principal, parse_flags, &s);
     if (ret)
 	goto out;
 
@@ -804,7 +811,6 @@ out:
     return ret;
 }
 
-
 /**
  * Verify the PAC.
  *
@@ -828,6 +834,23 @@ krb5_pac_verify(krb5_context context,
 		krb5_const_principal principal,
 		const krb5_keyblock *server,
 		const krb5_keyblock *privsvr)
+{
+    return krb5_pac_verify_ex(context, pac, authtime, principal,
+			      server, privsvr, false);
+}
+
+/* Verify PAC, with option to include the realm when
+ * comparing the logon-name. Needed for KDC processing
+ * of cross realm s4u2self tickets */
+
+KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
+krb5_pac_verify_ex(krb5_context context,
+		   const krb5_pac pac,
+		   time_t authtime,
+		   krb5_const_principal principal,
+		   const krb5_keyblock *server,
+		   const krb5_keyblock *privsvr,
+		   krb5_boolean with_realm)
 {
     krb5_error_code ret;
 
@@ -848,7 +871,8 @@ krb5_pac_verify(krb5_context context,
 			   pac->logon_name,
 			   &pac->data,
 			   authtime,
-			   principal);
+			   principal,
+			   with_realm);
     if (ret)
 	return ret;
 
@@ -966,6 +990,23 @@ _krb5_pac_sign(krb5_context context,
 	       const krb5_keyblock *server_key,
 	       const krb5_keyblock *priv_key,
 	       krb5_data *data)
+{
+    return _krb5_pac_sign_ex(context, p, authtime, principal,
+			     server_key, priv_key, data, false);
+}
+
+/* Sign PAC, with option to include the realm in the logon-name.
+ * Needed for KDC issuing cross realm s4u2self tickets */
+
+KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
+_krb5_pac_sign_ex(krb5_context context,
+		  krb5_pac p,
+		  time_t authtime,
+		  krb5_principal principal,
+		  const krb5_keyblock *server_key,
+		  const krb5_keyblock *priv_key,
+		  krb5_data *data,
+		  krb5_boolean with_realm)
 {
     krb5_error_code ret;
     krb5_storage *sp = NULL, *spdata = NULL;
@@ -1047,7 +1088,7 @@ _krb5_pac_sign(krb5_context context,
     }
 
     /* Calculate LOGON NAME */
-    ret = build_logon_name(context, authtime, principal, &logon);
+    ret = build_logon_name(context, authtime, principal, &logon, with_realm);
     if (ret)
 	goto out;
 
diff --git a/source4/heimdal/lib/krb5/version-script.map b/source4/heimdal/lib/krb5/version-script.map
index ddae2a06764..c4a00c49d77 100644
--- a/source4/heimdal/lib/krb5/version-script.map
+++ b/source4/heimdal/lib/krb5/version-script.map
@@ -474,6 +474,7 @@ HEIMDAL_KRB5_2.0 {
 		krb5_pac_init;
 		krb5_pac_parse;
 		krb5_pac_verify;
+		krb5_pac_verify_ex;
 		krb5_padata_add;
 		krb5_parse_address;
 		krb5_parse_name;
@@ -751,6 +752,7 @@ HEIMDAL_KRB5_2.0 {
 		_krb5_get_host_realm_int;
 		_krb5_get_int;
 		_krb5_pac_sign;
+		_krb5_pac_sign_ex;
 		_krb5_parse_moduli;
 		_krb5_pk_kdf;
 		_krb5_pk_load_id;
-- 
2.14.3


From 4ab222049fd788255dc93a316b87d3615f2752db Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Tue, 4 Sep 2018 11:21:40 +0300
Subject: [PATCH 06/12] heimdal: kdc: split check_PAC function

Introduction commit, no change intended.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/kdc/krb5tgs.c | 111 ++++++++++++++++++++++++------------------
 1 file changed, 63 insertions(+), 48 deletions(-)

diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index c5456e4cad9..0bd1a2b97ec 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -276,19 +276,9 @@ check_KRB5SignedPath(krb5_context context,
  */
 
 static krb5_error_code
-check_PAC(krb5_context context,
-	  krb5_kdc_configuration *config,
-	  const krb5_principal client_principal,
-	  const krb5_principal delegated_proxy_principal,
-	  hdb_entry_ex *client,
-	  hdb_entry_ex *server,
-	  hdb_entry_ex *krbtgt,
-	  const EncryptionKey *server_check_key,
-	  const EncryptionKey *server_sign_key,
-	  const EncryptionKey *krbtgt_sign_key,
-	  EncTicketPart *tkt,
-	  krb5_data *rspac,
-	  int *signedpath)
+get_PAC(krb5_context context,
+	EncTicketPart *tkt,
+	krb5_pac *pac)
 {
     AuthorizationData *ad = tkt->authorization_data;
     unsigned i, j;
@@ -315,47 +305,13 @@ check_PAC(krb5_context context,
 	for (j = 0; j < child.len; j++) {
 
 	    if (child.val[j].ad_type == KRB5_AUTHDATA_WIN2K_PAC) {
-		int signed_pac = 0;
-		krb5_pac pac;
 
 		/* Found PAC */
 		ret = krb5_pac_parse(context,
 				     child.val[j].ad_data.data,
 				     child.val[j].ad_data.length,
-				     &pac);
+				     pac);
 		free_AuthorizationData(&child);
-		if (ret)
-		    return ret;
-
-		ret = krb5_pac_verify(context, pac, tkt->authtime,
-				      client_principal,
-				      server_check_key, NULL);
-		if (ret) {
-		    krb5_pac_free(context, pac);
-		    return ret;
-		}
-
-		ret = _kdc_pac_verify(context, client_principal,
-				      delegated_proxy_principal,
-				      client, server, krbtgt, &pac, &signed_pac);
-		if (ret) {
-		    krb5_pac_free(context, pac);
-		    return ret;
-		}
-
-		/*
-		 * Only re-sign PAC if we could verify it with the PAC
-		 * function. The no-verify case happens when we get in
-		 * a PAC from cross realm from a Windows domain and
-		 * that there is no PAC verification function.
-		 */
-		if (signed_pac) {
-		    *signedpath = 1;
-		    ret = _krb5_pac_sign(context, pac, tkt->authtime,
-					 client_principal,
-					 server_sign_key, krbtgt_sign_key, rspac);
-		}
-		krb5_pac_free(context, pac);
 
 		return ret;
 	    }
@@ -365,6 +321,65 @@ check_PAC(krb5_context context,
     return 0;
 }
 
+static krb5_error_code
+check_PAC(krb5_context context,
+	  krb5_kdc_configuration *config,
+	  const krb5_principal client_principal,
+	  const krb5_principal delegated_proxy_principal,
+	  hdb_entry_ex *client,
+	  hdb_entry_ex *server,
+	  hdb_entry_ex *krbtgt,
+	  const EncryptionKey *server_check_key,
+	  const EncryptionKey *server_sign_key,
+	  const EncryptionKey *krbtgt_sign_key,
+	  EncTicketPart *tkt,
+	  krb5_data *rspac,
+	  int *signedpath)
+{
+    int signed_pac = 0;
+    krb5_pac pac = NULL;
+    krb5_error_code ret;
+
+    ret = get_PAC(context, tkt, &pac);
+    if (ret) {
+	return ret;
+    }
+
+    if (pac != NULL) {
+	ret = krb5_pac_verify(context, pac, tkt->authtime,
+			      client_principal, server_check_key, NULL);
+	if (ret) {
+	    krb5_pac_free(context, pac);
+	    return ret;
+	}
+
+	ret = _kdc_pac_verify(context, client_principal,
+			      delegated_proxy_principal,
+			      client, server, krbtgt, &pac, &signed_pac);
+	if (ret) {
+	    krb5_pac_free(context, pac);
+	    return ret;
+	}
+
+	/*
+	 * Only re-sign PAC if we could verify it with the PAC
+	 * function. The no-verify case happens when we get in
+	 * a PAC from cross realm from a Windows domain and
+	 * that there is no PAC verification function.
+	 */
+	if (signed_pac) {
+	    *signedpath = 1;
+	    ret = _krb5_pac_sign(context, pac, tkt->authtime,
+				 client_principal,
+				 server_sign_key, krbtgt_sign_key, rspac);
+	}
+
+	krb5_pac_free(context, pac);
+    }
+
+    return ret;
+}
+
 /*
  *
  */
-- 
2.14.3


From 95c3f77dec6ce46ee652b9e6426d2fd8d0867f91 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Tue, 4 Sep 2018 11:22:55 +0300
Subject: [PATCH 07/12] heimdal: kdc: factor out parsing of padata_for_user to
 its own  function

Introduction commit, no change intended.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/kdc/krb5tgs.c | 131 ++++++++++++++++++++++++------------------
 1 file changed, 76 insertions(+), 55 deletions(-)

diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index 0bd1a2b97ec..ccd6cd92856 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -1502,6 +1502,74 @@ eout:
     return ENOMEM;
 }
 
+static krb5_error_code
+check_padata_for_user(krb5_context context,
+		      krb5_kdc_configuration *config,
+		      KDC_REQ *req,
+		      const krb5_keyblock *key,
+		      krb5_principal *impersonated)
+{
+    krb5_error_code ret = 0;
+    const PA_DATA *sdata;
+    int i = 0;
+
+    sdata = _kdc_find_padata(req, &i, KRB5_PADATA_FOR_USER);
+    if (sdata) {
+	krb5_crypto crypto;
+	krb5_data datack;
+	PA_S4U2Self self;
+
+	ret = decode_PA_S4U2Self(sdata->padata_value.data,
+				 sdata->padata_value.length,
+				 &self, NULL);
+	if (ret) {
+	    kdc_log(context, config, 0, "Failed to decode PA-S4U2Self");
+	    return ret;
+	}
+
+	ret = _krb5_s4u2self_to_checksumdata(context, &self, &datack);
+	if (ret) {
+	    free_PA_S4U2Self(&self);
+	    return ret;
+	}
+
+	ret = krb5_crypto_init(context, key, 0, &crypto);
+	if (ret) {
+	    const char *msg = krb5_get_error_message(context, ret);
+	    free_PA_S4U2Self(&self);
+	    krb5_data_free(&datack);
+	    kdc_log(context, config, 0, "krb5_crypto_init failed: %s", msg);
+	    krb5_free_error_message(context, msg);
+	    return ret;
+	}
+
+	ret = krb5_verify_checksum(context,
+				   crypto,
+				   KRB5_KU_OTHER_CKSUM,
+				   datack.data,
+				   datack.length,
+				   &self.cksum);
+	krb5_data_free(&datack);
+	krb5_crypto_destroy(context, crypto);
+	if (ret) {
+	    const char *msg = krb5_get_error_message(context, ret);
+	    free_PA_S4U2Self(&self);
+	    kdc_log(context, config, 0,
+		    "krb5_verify_checksum failed for S4U2Self: %s", msg);
+	    krb5_free_error_message(context, msg);
+	    return ret;
+	}
+
+	ret = _krb5_principalname2krb5_principal(context,
+						 impersonated,
+						 self.name,
+						 self.realm);
+	free_PA_S4U2Self(&self);
+    }
+
+    return ret;
+}
+
 static krb5_error_code
 tgs_build_reply(krb5_context context,
 		krb5_kdc_configuration *config,
@@ -1921,63 +1989,16 @@ server_lookup:
     tpn = cpn;
 
     if (client) {
-	const PA_DATA *sdata;
-	int i = 0;
-
-	sdata = _kdc_find_padata(req, &i, KRB5_PADATA_FOR_USER);
-	if (sdata) {
-	    krb5_crypto crypto;
-	    krb5_data datack;
-	    PA_S4U2Self self;
-	    const char *str;
-
-	    ret = decode_PA_S4U2Self(sdata->padata_value.data,
-				     sdata->padata_value.length,
-				     &self, NULL);
-	    if (ret) {
-		kdc_log(context, config, 0, "Failed to decode PA-S4U2Self");
-		goto out;
-	    }
-
-	    ret = _krb5_s4u2self_to_checksumdata(context, &self, &datack);
-	    if (ret)
-		goto out;
-
-	    ret = krb5_crypto_init(context, &tgt->key, 0, &crypto);
-	    if (ret) {
-		const char *msg = krb5_get_error_message(context, ret);
-		free_PA_S4U2Self(&self);
-		krb5_data_free(&datack);
-		kdc_log(context, config, 0, "krb5_crypto_init failed: %s", msg);
-		krb5_free_error_message(context, msg);
-		goto out;
-	    }
+	krb5_principal for_user_principal = NULL;
+	const char *str;
 
-	    ret = krb5_verify_checksum(context,
-				       crypto,
-				       KRB5_KU_OTHER_CKSUM,
-				       datack.data,
-				       datack.length,
-				       &self.cksum);
-	    krb5_data_free(&datack);
-	    krb5_crypto_destroy(context, crypto);
-	    if (ret) {
-		const char *msg = krb5_get_error_message(context, ret);
-		free_PA_S4U2Self(&self);
-		kdc_log(context, config, 0,
-			"krb5_verify_checksum failed for S4U2Self: %s", msg);
-		krb5_free_error_message(context, msg);
-		goto out;
-	    }
-
-	    ret = _krb5_principalname2krb5_principal(context,
-						     &tp,
-						     self.name,
-						     self.realm);
-	    free_PA_S4U2Self(&self);
-	    if (ret)
-		goto out;
+	ret = check_padata_for_user(context, config, req, &tgt->key,
+                                    &for_user_principal);
+        if (ret)
+	    goto out;
 
+	if (for_user_principal != NULL) {
+	    tp = for_user_principal;
 	    ret = krb5_unparse_name(context, tp, &tpn);
 	    if (ret)
 		goto out;
-- 
2.14.3


From 7a1de2a3971c6c86314c3d88893626efcff99e3a Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Tue, 4 Sep 2018 11:30:10 +0300
Subject: [PATCH 08/12] heimdal: kdc: fix cross realm S4U2Self processing

According to MS-SFU 3.2.5.x, in a s4u2self referral ticket,
the PAC_CLIENT_INFO field may include the realm, so we need
to account for this before verifying and signing the PAC.

Make sure to base our expectations on db lookups rather
than strcmp of the service and impersonate principals.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/kdc/krb5tgs.c | 290 +++++++++++++++++++++++++++---------------
 1 file changed, 188 insertions(+), 102 deletions(-)

diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index ccd6cd92856..5aa0b134d0c 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -321,6 +321,98 @@ get_PAC(krb5_context context,
     return 0;
 }
 
+
+static krb5_error_code
+check_s4u2self_PAC(krb5_context context,
+		   const krb5_principal client_principal,
+		   const krb5_principal impersonate_principal,
+		   hdb_entry_ex *client,
+		   hdb_entry_ex *server,
+		   hdb_entry_ex *krbtgt,
+		   hdb_entry_ex *s4u2self_impersonated_client,
+		   const EncryptionKey *server_check_key,
+		   const EncryptionKey *server_sign_key,
+		   const EncryptionKey *krbtgt_sign_key,
+		   EncTicketPart *tkt,
+		   krb5_data *rspac,
+		   int *signedpath)
+{
+    krb5_pac pac = NULL;
+    krb5_error_code ret;
+
+    ret = get_PAC(context, tkt, &pac);
+    if (ret) {
+	return ret;
+    }
+
+    if (pac != NULL) {
+	int signed_pac = 0;
+	krb5_principal pac_principal;
+	hdb_entry_ex *db_client;
+	krb5_boolean with_realm;
+
+	/* In case the impersonated principal belongs to our krbtgt realm,
+	 * then we expect the PAC of the impersonating service */
+	if (s4u2self_impersonated_client != NULL) {
+	    pac_principal = client_principal;
+	    db_client = client;
+	    with_realm = false;
+	}
+	else {
+	    /* Otherwise, the PAC is of the impersonated principal and
+	     * the PAC_CLIENT_INFO field includes the realm */
+	    pac_principal = impersonate_principal;
+	    db_client = NULL;
+	    with_realm = true;
+	}
+
+	ret = krb5_pac_verify_ex(context, pac, tkt->authtime,
+			         pac_principal, server_check_key,
+				 NULL, with_realm);
+	if (ret) {
+	    krb5_pac_free(context, pac);
+	    return ret;
+	}
+
+	ret = _kdc_pac_verify(context, pac_principal, NULL, db_client,
+			      server, krbtgt, &pac, &signed_pac);
+	if (ret) {
+	    krb5_pac_free(context, pac);
+	    return ret;
+	}
+
+	/* If the principal being impersonated is in our db, then we
+	 * need a new PAC */
+	if (s4u2self_impersonated_client != NULL) {
+	    krb5_pac_free(context, pac);
+	    pac = NULL;
+
+	    ret = _kdc_pac_generate(context, s4u2self_impersonated_client,
+				    NULL, &pac);
+	    if (ret || pac == NULL) {
+		return ret;
+	    }
+	}
+
+	/* Only re-sign PAC if we could verify it with the PAC function. */
+	if (signed_pac) {
+	    *signedpath = 1;
+
+	     /* If the impersonating service does not belong to our realm,
+	      * then we need to sign including the realm component */
+	    with_realm = client ? false : true;
+
+	    ret = _krb5_pac_sign_ex(context, pac, tkt->authtime,
+				    impersonate_principal, server_sign_key,
+				    krbtgt_sign_key, rspac, with_realm);
+	}
+
+	krb5_pac_free(context, pac);
+    }
+
+    return ret;
+}
+
 static krb5_error_code
 check_PAC(krb5_context context,
 	  krb5_kdc_configuration *config,
@@ -1614,6 +1706,8 @@ tgs_build_reply(krb5_context context,
     Key *tkey_check;
     Key *tkey_sign;
     int flags = HDB_F_FOR_TGS_REQ;
+    krb5_principal for_user_principal = NULL;
+    char *for_user_str = NULL;
 
     memset(&sessionkey, 0, sizeof(sessionkey));
     memset(&adtkt, 0, sizeof(adtkt));
@@ -1949,37 +2043,6 @@ server_lookup:
 	krb5_free_error_message(context, msg);
     }
 
-    ret = check_PAC(context, config, cp, NULL,
-		    client, server, krbtgt,
-		    &tkey_check->key,
-		    ekey, &tkey_sign->key,
-		    tgt, &rspac, &signedpath);
-    if (ret) {
-	const char *msg = krb5_get_error_message(context, ret);
-	kdc_log(context, config, 0,
-		"Verify PAC failed for %s (%s) from %s with %s",
-		spn, cpn, from, msg);
-	krb5_free_error_message(context, msg);
-	goto out;
-    }
-
-    /* also check the krbtgt for signature */
-    ret = check_KRB5SignedPath(context,
-			       config,
-			       krbtgt,
-			       cp,
-			       tgt,
-			       &spp,
-			       &signedpath);
-    if (ret) {
-	const char *msg = krb5_get_error_message(context, ret);
-	kdc_log(context, config, 0,
-		"KRB5SignedPath check failed for %s (%s) from %s with %s",
-		spn, cpn, from, msg);
-	krb5_free_error_message(context, msg);
-	goto out;
-    }
-
     /*
      * Process request
      */
@@ -1988,64 +2051,49 @@ server_lookup:
     tp = cp;
     tpn = cpn;
 
-    if (client) {
-	krb5_principal for_user_principal = NULL;
-	const char *str;
+    /* Check for PADATA_FOR_USER */
+    ret = check_padata_for_user(context, config, req, &tgt->key,
+				&for_user_principal);
+    if (ret)
+	goto out;
 
-	ret = check_padata_for_user(context, config, req, &tgt->key,
-                                    &for_user_principal);
-        if (ret)
+    if (for_user_principal != NULL) {
+	ret = krb5_unparse_name(context, for_user_principal, &for_user_str);
+	if (ret)
 	    goto out;
 
-	if (for_user_principal != NULL) {
-	    tp = for_user_principal;
-	    ret = krb5_unparse_name(context, tp, &tpn);
-	    if (ret)
-		goto out;
+	/* Check if the principal being impersonated belongs to our krbtgt realm */
+	ret = _kdc_db_fetch(context, config, for_user_principal, HDB_F_GET_CLIENT | flags,
+			    NULL, &s4u2self_impersonated_clientdb, &s4u2self_impersonated_client);
+	if (ret && krb5_realm_compare(context, for_user_principal, krbtgt_principal)) {
+	    const char *msg;
 
-	    /* If we were about to put a PAC into the ticket, we better fix it to be the right PAC */
-	    if(rspac.data) {
-		krb5_pac p = NULL;
-		krb5_data_free(&rspac);
-		ret = _kdc_db_fetch(context, config, tp, HDB_F_GET_CLIENT | flags,
-				    NULL, &s4u2self_impersonated_clientdb, &s4u2self_impersonated_client);
-		if (ret) {
-		    const char *msg;
-
-		    /*
-		     * If the client belongs to the same realm as our krbtgt, it
-		     * should exist in the local database.
-		     *
-		     */
-
-		    if (ret == HDB_ERR_NOENTRY)
-			ret = KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN;
-		    msg = krb5_get_error_message(context, ret);
-		    kdc_log(context, config, 1,
-			    "S2U4Self principal to impersonate %s not found in database: %s",
-			    tpn, msg);
-		    krb5_free_error_message(context, msg);
-		    goto out;
-		}
-		ret = _kdc_pac_generate(context, s4u2self_impersonated_client, NULL, &p);
-		if (ret) {
-		    kdc_log(context, config, 0, "PAC generation failed for -- %s",
-			    tpn);
-		    goto out;
-		}
-		if (p != NULL) {
-		    ret = _krb5_pac_sign(context, p, ticket->ticket.authtime,
-					 s4u2self_impersonated_client->entry.principal,
-					 ekey, &tkey_sign->key,
-					 &rspac);
-		    krb5_pac_free(context, p);
-		    if (ret) {
-			kdc_log(context, config, 0, "PAC signing failed for -- %s",
-				tpn);
-			goto out;
-		    }
-		}
-	    }
+	    /*
+	     * If the impersonated client belongs to the same realm as
+	     * our krbtgt, it should exist in the local database.
+	     */
+
+            if (ret == HDB_ERR_NOENTRY)
+	        ret = KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN;
+	    msg = krb5_get_error_message(context, ret);
+	    kdc_log(context, config, 1,
+		    "S4U2Self principal to impersonate %s not found in database: %s",
+		    for_user_str, msg);
+	    krb5_free_error_message(context, msg);
+	    goto out;
+	}
+
+	/*
+	 * If the impersonating service belongs to our krbtgt realm, then we
+	 * are going to reply with a service ticket (that is, not a referral),
+	 * so we need to adjust cname and crealm to the impersonated user,
+	 * and also to verify that it is actually asking a ticket for self.
+	 */
+	if (client != NULL) {
+	    const char *str;
+
+	    tp = for_user_principal;
+	    tpn = for_user_str;
 
 	    /*
 	     * Check that service doing the impersonating is
@@ -2074,6 +2122,52 @@ server_lookup:
 	    kdc_log(context, config, 0, "s4u2self %s impersonating %s to "
 		    "service %s %s", cpn, tpn, spn, str);
 	}
+
+	ret = check_s4u2self_PAC(context, cp, for_user_principal,
+				 client, server, krbtgt,
+				 s4u2self_impersonated_client,
+				 &tkey_check->key, ekey, &tkey_sign->key,
+				 tgt, &rspac, &signedpath);
+	if (ret) {
+	    const char *msg = krb5_get_error_message(context, ret);
+	    kdc_log(context, config, 0,
+		    "Verify PAC failed for %s (%s, for-user: %s) from %s with %s",
+		    spn, cpn, for_user_str, from, msg);
+	    krb5_free_error_message(context, msg);
+	    goto out;
+        }
+    }
+    else {
+	ret = check_PAC(context, config, cp, NULL,
+			client, server, krbtgt,
+			&tkey_check->key,
+			ekey, &tkey_sign->key,
+			tgt, &rspac, &signedpath);
+	if (ret) {
+	    const char *msg = krb5_get_error_message(context, ret);
+	    kdc_log(context, config, 0,
+		    "Verify PAC failed for %s (%s) from %s with %s",
+		    spn, cpn, from, msg);
+	    krb5_free_error_message(context, msg);
+	    goto out;
+        }
+    }
+
+    /* also check the krbtgt for signature */
+    ret = check_KRB5SignedPath(context,
+			       config,
+			       krbtgt,
+			       cp,
+			       tgt,
+			       &spp,
+			       &signedpath);
+    if (ret) {
+	const char *msg = krb5_get_error_message(context, ret);
+	kdc_log(context, config, 0,
+		"KRB5SignedPath check failed for %s (%s) from %s with %s",
+		spn, cpn, from, msg);
+	krb5_free_error_message(context, msg);
+	goto out;
     }
 
     /*
@@ -2312,12 +2406,11 @@ server_lookup:
 			 reply);
 
 out:
-    if (tpn != cpn)
-	    free(tpn);
+    free(for_user_str);
+    free(ref_realm);
     free(spn);
     free(cpn);
-    if (dpn)
-	free(dpn);
+    free(dpn);
 
     krb5_data_free(&rspac);
     krb5_free_keyblock_contents(context, &sessionkey);
@@ -2330,20 +2423,13 @@ out:
     if(s4u2self_impersonated_client)
 	_kdc_free_ent(context, s4u2self_impersonated_client);
 
-    if (tp && tp != cp)
-	krb5_free_principal(context, tp);
-    if (cp)
-	krb5_free_principal(context, cp);
-    if (dp)
-	krb5_free_principal(context, dp);
-    if (sp)
-	krb5_free_principal(context, sp);
-    if (krbtgt_principal)
-        krb5_free_principal(context, krbtgt_principal);
-    if (ref_realm)
-	free(ref_realm);
-    free_METHOD_DATA(&enc_pa_data);
+    krb5_free_principal(context, for_user_principal);
+    krb5_free_principal(context, krbtgt_principal);
+    krb5_free_principal(context, cp);
+    krb5_free_principal(context, dp);
+    krb5_free_principal(context, sp);
 
+    free_METHOD_DATA(&enc_pa_data);
     free_EncTicketPart(&adtkt);
 
     return ret;
-- 
2.14.3


From 8fda55a7060a4dd059af82781897c0e3990daf15 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Tue, 4 Sep 2018 12:31:07 +0300
Subject: [PATCH 09/12] heimdal: s4u2self: lookup enterprise principal's realm

In case the imperosnate principal is an enterprise name,
then first lookup its realm by chasing client referrals.
See MS-SFU section 3.1.5.1.1.1

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/lib/krb5/get_cred.c         | 83 +++++++++++++++++++++++++++++
 source4/heimdal/lib/krb5/init_creds_pw.c    | 21 +++++++-
 source4/heimdal/lib/krb5/version-script.map |  1 +
 3 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/source4/heimdal/lib/krb5/get_cred.c b/source4/heimdal/lib/krb5/get_cred.c
index 29ab6eaaa76..420155f4ce0 100644
--- a/source4/heimdal/lib/krb5/get_cred.c
+++ b/source4/heimdal/lib/krb5/get_cred.c
@@ -884,6 +884,80 @@ get_cred(server)
 	return get_cred_tgt(server, tgt)
 	*/
 
+static krb5_error_code
+lookup_realm_kdc_retry(krb5_context context, krb5_sendto_ctx ctx, void *data,
+		       const krb5_data *reply, int *action)
+{
+	krb5_error_code code, ret = 0;
+	KRB_ERROR error;
+	krb5_principal principal = (krb5_principal) data;
+
+	if(krb5_rd_error(context, reply, &error))
+		return 0;
+
+	code = krb5_error_from_rd_error(context, &error, NULL);
+
+	switch(code) {
+	case KRB5_KDC_ERR_WRONG_REALM: {
+		if (principal && error.crealm && error.crealm[0] && error.crealm[0][0])
+			ret = krb5_principal_set_realm(context, principal, error.crealm[0]);
+		break;
+	}
+	case KRB5KDC_ERR_PREAUTH_REQUIRED:
+		ret = code;
+		break;
+	}
+
+	krb5_free_error_contents(context, &error);
+
+	if (ret)
+		return ret;
+
+	return _krb5_kdc_retry(context, ctx, NULL, reply, action);
+}
+
+static krb5_error_code
+lookup_principal_realm(krb5_context context, krb5_principal principal,
+		       krb5_const_realm start_realm)
+{
+	krb5_error_code ret;
+	krb5_get_init_creds_opt *opt = NULL;
+	krb5_init_creds_context ctx = NULL;
+
+	if (KRB5_NT_ENTERPRISE_PRINCIPAL !=
+		krb5_principal_get_type(context, principal))
+		return 0;
+
+	ret = krb5_principal_set_realm(context, principal, start_realm);
+	if (ret)
+		return ret;
+
+	ret = krb5_get_init_creds_opt_alloc(context, &opt);
+	if (ret)
+		return ret;
+
+	ret = krb5_get_init_creds_opt_set_canonicalize(context, opt, TRUE);
+	if (ret)
+		goto out;
+
+	ret = krb5_init_creds_init(context, principal, NULL, NULL, 0, opt, &ctx);
+	if (ret)
+		goto out;
+
+	ret = krb5_init_creds_get_retry(context, ctx, lookup_realm_kdc_retry, principal);
+
+	/* Overwrite ret */
+	if (ret == KRB5KDC_ERR_PREAUTH_REQUIRED)
+		ret = 0;
+out:
+	if (opt)
+		krb5_get_init_creds_opt_free(context, opt);
+	if (ctx)
+		krb5_init_creds_free(context, ctx);
+
+	return ret;
+}
+
 static krb5_error_code
 get_cred_kdc_capath(krb5_context context,
 		    krb5_kdc_flags flags,
@@ -950,6 +1024,15 @@ get_cred_kdc_referral(krb5_context context,
 
     client_realm = krb5_principal_get_realm(context, in_creds->client);
 
+    /* If the impersonate principal is an enterprise name, try lookup its realm */
+    if (impersonate_principal != NULL) {
+	ret = lookup_principal_realm(context, impersonate_principal,
+				     in_creds->client->realm);
+	if (ret) {
+	    return ret;
+	}
+    }
+
     /* find tgt for the clients base realm */
     {
 	krb5_principal tgtname;
diff --git a/source4/heimdal/lib/krb5/init_creds_pw.c b/source4/heimdal/lib/krb5/init_creds_pw.c
index fb39704a9e5..3c68fedd1e9 100644
--- a/source4/heimdal/lib/krb5/init_creds_pw.c
+++ b/source4/heimdal/lib/krb5/init_creds_pw.c
@@ -1925,12 +1925,15 @@ krb5_init_creds_free(krb5_context context,
  *
  * @param context A Kerberos 5 context.
  * @param ctx The krb5_init_creds_context to process.
+ * @param func The krb5_kdc_retry fucntion to use.
+ * @param func The data to pass to krb5_kdc_retry fucntion.
  *
  * @ingroup krb5_credential
  */
 
 KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
-krb5_init_creds_get(krb5_context context, krb5_init_creds_context ctx)
+krb5_init_creds_get_retry(krb5_context context, krb5_init_creds_context ctx,
+			  krb5_sendto_ctx_func func, void *data)
 {
     krb5_sendto_ctx stctx = NULL;
     krb5_krbhst_info *hostinfo = NULL;
@@ -1944,7 +1947,7 @@ krb5_init_creds_get(krb5_context context, krb5_init_creds_context ctx)
     ret = krb5_sendto_ctx_alloc(context, &stctx);
     if (ret)
 	goto out;
-    krb5_sendto_ctx_set_func(stctx, _krb5_kdc_retry, NULL);
+    krb5_sendto_ctx_set_func(stctx, func, data);
 
     while (1) {
 	flags = 0;
@@ -1970,6 +1973,20 @@ krb5_init_creds_get(krb5_context context, krb5_init_creds_context ctx)
     return ret;
 }
 
+/**
+ * Get new credentials as setup by the krb5_init_creds_context.
+ *
+ * @param context A Kerberos 5 context.
+ * @param ctx The krb5_init_creds_context to process.
+ *
+ * @ingroup krb5_credential
+ */
+
+KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL
+krb5_init_creds_get(krb5_context context, krb5_init_creds_context ctx)
+{
+    return krb5_init_creds_get_retry(context, ctx, _krb5_kdc_retry, NULL);
+}
 /**
  * Get new credentials using password.
  *
diff --git a/source4/heimdal/lib/krb5/version-script.map b/source4/heimdal/lib/krb5/version-script.map
index c4a00c49d77..07c4fd620f8 100644
--- a/source4/heimdal/lib/krb5/version-script.map
+++ b/source4/heimdal/lib/krb5/version-script.map
@@ -387,6 +387,7 @@ HEIMDAL_KRB5_2.0 {
 		krb5_init_ets;
 		krb5_init_creds_init;
 		krb5_init_creds_get_error;
+		krb5_init_creds_get_retry;
 		krb5_init_creds_free;
 		krb5_initlog;
 		krb5_is_config_principal;
-- 
2.14.3


From b9f4d8f23180438f98f59101c443331e64ce8de6 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Tue, 4 Sep 2018 12:46:51 +0300
Subject: [PATCH 10/12] heimdal: s4u2self: fix service code in cross realm
 trust

When impersonating a user from another realm, we first need
to obtain a TGT to TGS in that user's realm.
Only then we can ask for a ticket for self and follow back
referrals to our own realm.
See MS-SFU sections 3.1.5.1.1.1 and 4.2

In case we fail to obtain that krbtgt, fall back to asking
from the service's own realm, as it may be the same realm
using different name such as netbios or lower realm.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/lib/krb5/get_cred.c | 48 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/source4/heimdal/lib/krb5/get_cred.c b/source4/heimdal/lib/krb5/get_cred.c
index 420155f4ce0..6ae8cb5f507 100644
--- a/source4/heimdal/lib/krb5/get_cred.c
+++ b/source4/heimdal/lib/krb5/get_cred.c
@@ -1007,6 +1007,7 @@ get_cred_kdc_referral(krb5_context context,
     krb5_error_code ret;
     krb5_creds tgt, referral, ticket;
     int loop = 0;
+    krb5_boolean got_tgt = false;
     int ok_as_delegate = 1;
 
     if (in_creds->server->name.name_string.len < 2 && !flags.b.canonicalize) {
@@ -1033,7 +1034,52 @@ get_cred_kdc_referral(krb5_context context,
 	}
     }
 
-    /* find tgt for the clients base realm */
+    /*
+     * find tgt for the clients base realm, if we are impersonating a user
+     * from another realm, then we actually need a referral tgt to that realm.
+     */
+    if (impersonate_principal != NULL &&
+	!krb5_realm_compare(context, impersonate_principal, in_creds->client))
+    {
+	krb5_creds impersonate_ref;
+	krb5_creds *impersonate_tgt = NULL;
+	krb5_creds **referral_tgts = NULL;
+	int i;
+
+	impersonate_ref = *in_creds;
+
+	ret = krb5_make_principal(context, &impersonate_ref.server,
+				  client_realm, KRB5_TGS_NAME,
+				  impersonate_principal->realm, NULL);
+	if (ret) {
+	    return ret;
+	}
+
+	krb5_principal_set_type(context, impersonate_ref.server, KRB5_NT_SRV_INST);
+
+	ret = get_cred_kdc_referral(context, flags, ccache, &impersonate_ref, NULL,
+				    NULL, &impersonate_tgt, &referral_tgts);
+	krb5_free_principal(context, impersonate_ref.server);
+	for(i = 0; referral_tgts && referral_tgts[i]; i++) {
+	    krb5_free_creds(context, referral_tgts[i]);
+	}
+	free(referral_tgts);
+
+	if (ret == 0) {
+	    tgt = *impersonate_tgt;
+	    free(impersonate_tgt);
+
+	    client_realm = krb5_principal_get_realm(context, impersonate_principal);
+	    got_tgt = true;
+	}
+
+	/*
+	 * The user might be in local realm but using an alternative realm name type.
+	 * Fall back to our krbtgt realm in case of failure.
+	 */
+    }
+
+    if (!got_tgt)
     {
 	krb5_principal tgtname;
 
-- 
2.14.3


From e7cd9a07e8ddaf9fe720d59da8afdef06f2ef2e5 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Sun, 2 Sep 2018 19:09:46 +0300
Subject: [PATCH 11/12] s4u2self: adjust krb5_canon torture tests to the cross
 realm changes

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/torture/krb5/kdc-canon-heimdal.c | 50 ++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/source4/torture/krb5/kdc-canon-heimdal.c b/source4/torture/krb5/kdc-canon-heimdal.c
index 5b782a23fc4..0f3a1945e1e 100644
--- a/source4/torture/krb5/kdc-canon-heimdal.c
+++ b/source4/torture/krb5/kdc-canon-heimdal.c
@@ -86,6 +86,8 @@ struct torture_krb5_context {
 	struct addrinfo *server;
 	struct test_data *test_data;
 	int packet_count;
+	int impersonating;
+	bool reset_count;
 	enum test_stage test_stage;
 	AS_REQ as_req;
 	AS_REP as_rep;
@@ -356,6 +358,13 @@ static bool torture_krb5_post_recv_as_req_test(struct torture_krb5_context *test
 						 error.error_code,
 						 KRB5KDC_ERR_PREAUTH_REQUIRED - KRB5KDC_ERR_NONE,
 						 "Got wrong error.error_code");
+			if (test_context->impersonating) {
+				/* We reach here when the impersonating service
+				 * is done looking up the imperosnated principal
+				 * realm, the next stage is to impersonate. */
+				test_context->test_stage = TEST_TGS_REQ_CANON;
+				test_context->reset_count = true;
+			}
 		}
 
 		free_KRB_ERROR(&error);
@@ -431,7 +440,7 @@ static bool torture_krb5_post_recv_as_req_test(struct torture_krb5_context *test
  *
  * Confirm that the outgoing TGS-REQ packet from krb5_get_creds()
  * for the krbtgt/realm principal meets certain expectations, like
- * that the canonicalize bit is not set
+ * that the canonicalize bit is set
  *
  */
 
@@ -455,7 +464,7 @@ static bool torture_krb5_pre_send_tgs_req_krbtgt_canon_test(struct torture_krb5_
 
 	torture_assert_int_equal(test_context->tctx,
 				 test_context->tgs_req.req_body.sname->name_type,
-				 KRB5_NT_PRINCIPAL,
+				 test_context->impersonating ? KRB5_NT_SRV_INST : KRB5_NT_PRINCIPAL,
 				 "Mismatch in name_type between request and expected request");
 
 	torture_assert_str_equal(test_context->tctx,
@@ -544,6 +553,15 @@ static bool torture_krb5_post_recv_tgs_req_krbtgt_canon_test(struct torture_krb5
 		       test_context->packet_count < 2,
 		       "too many packets");
 	free_TGS_REQ(&test_context->tgs_req);
+
+	/* If we are at the second packet, then the library is about to fail with
+	 * KRB5_GET_IN_TKT_LOOP. In the impersonating case, at this point the library
+	 * will fall back to request a ticket from the service's own realm. */
+	if (test_context->impersonating && (test_context->packet_count == 1)) {
+		test_context->test_stage = TEST_TGS_REQ_CANON;
+		test_context->reset_count = true;
+	}
+
 	return true;
 }
 
@@ -674,6 +692,7 @@ static bool torture_krb5_post_recv_tgs_req_canon_test(struct torture_krb5_contex
 					 0, "Unexpecedly got a RODC number in the KVNO, should just be principal KVNO");
 		free_TGS_REP(&test_context->tgs_rep);
 	}
+
 	torture_assert(test_context->tctx, test_context->packet_count == 0, "too many packets");
 	free_TGS_REQ(&test_context->tgs_req);
 
@@ -1362,7 +1381,13 @@ static krb5_error_code smb_krb5_send_and_recv_func_canon_override(krb5_context c
 		return EINVAL;
 	}
 
-	test_context->packet_count++;
+	if (test_context->reset_count) {
+		test_context->reset_count = false;
+		test_context->packet_count = 0;
+	}
+	else {
+		test_context->packet_count++;
+	}
 
 	return k5ret;
 }
@@ -1817,6 +1842,25 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 									    opt,
 									    principal),
 					 0, "krb5_get_creds_opt_set_impersonate failed");
+		/*
+		 * When impersonating, we may pass thru other stages with slight difrerences
+		 * for which we need to account, so we need a flag to identify this case.
+		 */
+		test_context->impersonating = true;
+
+		/* S4U2Self for enterprise principal name always start with AS request
+		 * for realm lookup, we later change the stage in the callback */
+		if (test_data->enterprise) {
+			test_context->test_stage = TEST_AS_REQ;
+		}
+		else if (test_data->canonicalize &&
+			(test_data->upper_realm == false || test_data->netbios_realm == true))
+		{
+			/* In this case the library thinks the service and imperosnate user
+			 * are not from the same realm due to netbios or lower realm used,
+			 * and will request a krbtgt to the impersonated realm. */
+			test_context->test_stage = TEST_TGS_REQ_KRBTGT_CANON;
+		}
 	}
 
 	/* Confirm if we can get a ticket to our own name */
-- 
2.14.3


From 6ec93d211e73019416f14c09bb51e2eeba628df4 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Sun, 12 Aug 2018 17:07:51 +0300
Subject: [PATCH 12/12] s4u2self: add cross realm blackbox tests

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 testprogs/blackbox/test_kinit_trusts_heimdal.sh | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/testprogs/blackbox/test_kinit_trusts_heimdal.sh b/testprogs/blackbox/test_kinit_trusts_heimdal.sh
index 4a1c0f7f213..ce88071818c 100755
--- a/testprogs/blackbox/test_kinit_trusts_heimdal.sh
+++ b/testprogs/blackbox/test_kinit_trusts_heimdal.sh
@@ -36,6 +36,9 @@ smbclient="$samba4bindir/smbclient"
 wbinfo="$samba4bindir/wbinfo"
 rpcclient="$samba4bindir/rpcclient"
 samba_tool="$samba4bindir/samba-tool"
+net_tool="$samba4bindir/net -s $SMB_CONF_PATH"
+
+local_service="HOST/$(hostname)@$REALM"
 
 . `dirname $0`/subunit.sh
 . `dirname $0`/common_test_fns.inc
@@ -49,6 +52,7 @@ KRB5CCNAME="FILE:$KRB5CCNAME_PATH"
 export KRB5CCNAME
 rm -rf $KRB5CCNAME_PATH
 
+
 echo $TRUST_PASSWORD > $PREFIX/tmppassfile
 testit "kinit with password" $samba4kinit $enctype --password-file=$PREFIX/tmppassfile --request-pac $TRUST_USERNAME@$TRUST_REALM   || failed=`expr $failed + 1`
 test_smbclient "Test login with user kerberos ccache" 'ls' "$unc" -k yes || failed=`expr $failed + 1`
@@ -94,5 +98,18 @@ testit "wbinfo check outgoing trust pw" $VALGRIND $wbinfo --check-secret --domai
 
 test_smbclient "Test user login with the changed outgoing secret" 'ls' "$unc" -k yes -U$USERNAME@$REALM%$PASSWORD || failed=`expr $failed + 1`
 
+
+# Test net-ads-kerberos-pac with impersonate
+testit "dump pac of local machine" $VALGRIND $net_tool -P ads kerberos pac dump || failed=`expr $failed + 1`
+testit "dump pac via impersonate" $VALGRIND $net_tool -P ads kerberos pac dump impersonate=$USERNAME@$REALM || failed=`expr $failed + 1`
+testit "dump pac via impersonate to SPN" $VALGRIND $net_tool -P ads kerberos pac dump impersonate=$USERNAME@$REALM local_service=$local_service || failed=`expr $failed + 1`
+testit "dump pac via impersonate with netbios domain" $VALGRIND $net_tool -P ads kerberos pac dump impersonate=$USERNAME@$DOMAIN local_service=$local_service || failed=`expr $failed + 1`
+
+# Test cross realm S4U2Self with forest trust
+if test x"${TYPE}" = x"forest" ;then
+	testit "dump pac via impersonate with trust" $VALGRIND $net_tool -P ads kerberos pac dump impersonate=$TRUST_USERNAME@$TRUST_REALM local_service=$local_service || failed=`expr $failed + 1`
+fi
+
+
 rm -f $PREFIX/tmpccache tmpccfile tmppassfile tmpuserpassfile tmpuserccache
 exit $failed
-- 
2.14.3



More information about the samba-technical mailing list