Cross realm S4U2Self cont.

Isaac Boukris iboukris at gmail.com
Sun Aug 12 21:27:23 UTC 2018


Update:
I've reworked the kdc commit and have updated some others, new patch attached.
I submitted this to a new branch on gitlab, see:
https://gitlab.com/samba-team/devel/samba/commits/iboukris_xrealm_s4u2self_v2

Also submitted the poc changes needed for transitive trust to a new
branch, see (last three patches):
https://gitlab.com/samba-team/devel/samba/commits/iboukris_xrealm_s4u2self_v3

 I am trying to find how we can test transitive trust within smaba
only. I think I need to create a child domain but can't make
samba-tool-join-subdomain to work. I read some email suggesting it is
experimental but it seems to be invoked in tests, so perhaps it could
work.

Regards.
-------------- next part --------------
From ebde2948a28015f535258547136b08890366b57e 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 1/7] 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 eded6ddd1a68106600162c86a11e3760603dbccb 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 2/7] 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 39042b413cc3ebcf2d480405a6856385ca47c9ad 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 3/7] 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 24006dfc0937e369a5bfc40d7ed266b658158e57 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 4/7] 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 fc7df9493074fae4f0b0fba93b2c41bb69543ead Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Sun, 12 Aug 2018 10:34:47 +0300
Subject: [PATCH 5/7] kdc: fix cross realm s4u2self

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 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 | 470 ++++++++++++++++++++++++++----------------
 1 file changed, 290 insertions(+), 180 deletions(-)

diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index c5456e4cad9..c9234e1d6ab 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -275,14 +275,148 @@ check_KRB5SignedPath(krb5_context context,
  *
  */
 
+static krb5_error_code
+check_pac_data(krb5_context context,
+	       krb5_kdc_configuration *config,
+	       const krb5_principal client_principal,
+	       const krb5_principal impersonate_principal,
+	       const krb5_principal delegated_proxy_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,
+	       heim_octet_string *ad,
+	       krb5_data *rspac,
+	       int *signedpath)
+{
+    krb5_principal pac_principal, impersonate_enterprise = NULL;
+    char *principal_str = NULL;
+    char *impersonate_str = NULL;
+    hdb_entry_ex *db_client;
+    krb5_error_code ret = 0;
+    int signed_pac = 0;
+    krb5_pac pac = NULL;
+
+    pac_principal = client_principal;
+    db_client = client;
+
+    if (impersonate_principal != NULL) {
+	ret = krb5_unparse_name(context, impersonate_principal, &impersonate_str);
+	if (ret)
+	    return ret;
+
+	/*
+	 * In cross realm s4u2self the PAC_CLIENT_INFO field may contain
+	 * the full principal name including the realm, while the pac verify
+	 * function compares only the principal name without the realm.
+	 * To work around this, we parse the full name as an enterprise
+	 * principal name, so that the realm will be kept when unparsed
+	 * by the pac verify function.
+	 * Note: in case the impersonate principal was originally an
+	 * enterprise principal name, then these two names would be
+	 * identical - but that is Ok.
+	 */
+	ret = krb5_parse_name_flags(context, impersonate_str,
+				    KRB5_PRINCIPAL_PARSE_ENTERPRISE,
+				    &impersonate_enterprise);
+	if (ret)
+	    goto out;
+
+	/*
+	 * If the impersonate principal does not belong to our realm,
+	 * then we expect the PAC of the impesonate principal and the
+	 * PAC_CLIENT_INFO in format of user at REALM
+	 */
+	if (s4u2self_impersonated_client == NULL) {
+	    pac_principal = impersonate_enterprise;
+	    db_client = NULL;
+	}
+    }
+
+    ret = krb5_unparse_name(context, pac_principal, &principal_str);
+    if (ret)
+	return ret;
+
+    kdc_log(context, config, 3, "Checking PAC of principal: %s", principal_str);
+
+    ret = krb5_pac_parse(context, ad->data, ad->length, &pac);
+    if (ret)
+	goto out;
+
+    ret = krb5_pac_verify(context, pac, tkt->authtime, pac_principal,
+			  server_check_key, NULL);
+    if (ret) {
+	/* TODO: possibly overwrite with err-policy */
+	goto out;
+    }
+
+    ret = _kdc_pac_verify(context, pac_principal, delegated_proxy_principal,
+			  db_client, server, krbtgt, &pac, &signed_pac);
+    if (ret) {
+        goto out;
+    }
+
+    /* If the principal being impersonated is in our db, then we
+     * need to issue 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) {
+	    kdc_log(context, config, 0, "PAC generation failed for: %s", impersonate_str);
+	    goto out;
+	}
+
+	kdc_log(context, config, 3, "Issued new PAC for impersonated principal: %s",
+			impersonate_str);
+    }
+
+    /* If the impersonating service does not belong to our realm,
+     * then we need to sign the pac in format user at REALM */
+    if (impersonate_principal != NULL) {
+	if (client != NULL)
+	    pac_principal = impersonate_principal;
+	else
+	    pac_principal = impersonate_enterprise;
+    }
+
+    /*
+     * 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, pac_principal,
+			     server_sign_key, krbtgt_sign_key, rspac);
+    }
+
+out:
+    free(principal_str);
+    free(impersonate_str);
+    krb5_free_principal(context, impersonate_enterprise);
+    if (pac)
+	krb5_pac_free(context, pac);
+
+    return ret;
+}
+
 static krb5_error_code
 check_PAC(krb5_context context,
 	  krb5_kdc_configuration *config,
 	  const krb5_principal client_principal,
+	  const krb5_principal impersonated_principal,
 	  const krb5_principal delegated_proxy_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,
@@ -315,48 +449,15 @@ 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);
+		ret = check_pac_data(context, config, client_principal,
+				     impersonated_principal,
+				     delegated_proxy_principal, client, server,
+				     krbtgt, s4u2self_impersonated_client,
+				     server_check_key,
+				     server_sign_key, krbtgt_sign_key, tkt,
+				     &child.val[j].ad_data, rspac, signedpath);
 		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;
 	    }
 	}
@@ -1487,6 +1588,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,
@@ -1506,6 +1675,8 @@ tgs_build_reply(krb5_context context,
     krb5_error_code ret;
     krb5_principal cp = NULL, sp = NULL, tp = NULL, dp = NULL;
     krb5_principal krbtgt_principal = NULL;
+    krb5_principal for_user_principal = NULL;
+    char *for_user_str = NULL;
     char *spn = NULL, *cpn = NULL, *tpn = NULL, *dpn = NULL;
     hdb_entry_ex *server = NULL, *client = NULL, *s4u2self_impersonated_client = NULL;
     HDB *clientdb, *s4u2self_impersonated_clientdb;
@@ -1866,37 +2037,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
      */
@@ -1905,111 +2045,49 @@ server_lookup:
     tp = cp;
     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;
-	    }
+    /* Check for PADATA_FOR_USER */
+    ret = check_padata_for_user(context, config, req, &tgt->key,
+				&for_user_principal);
+    if (ret)
+	goto out;
 
-	    ret = _krb5_s4u2self_to_checksumdata(context, &self, &datack);
-	    if (ret)
-		goto out;
+    if (for_user_principal != NULL) {
+	ret = krb5_unparse_name(context, for_user_principal, &for_user_str);
+	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;
-	    }
+	/* Check if the user 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;
 
-	    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;
-	    }
+	    /*
+	     * If the impersonated client belongs to the same realm as
+	     * our krbtgt, it should exist in the local database.
+	     */
 
-	    ret = _krb5_principalname2krb5_principal(context,
-						     &tp,
-						     self.name,
-						     self.realm);
-	    free_PA_S4U2Self(&self);
-	    if (ret)
-		goto out;
+            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;
+	}
 
-	    ret = krb5_unparse_name(context, tp, &tpn);
-	    if (ret)
-		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;
 
-	    /* 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;
-		    }
-		}
-	    }
+	    tp = for_user_principal;
+	    tpn = for_user_str;
 
 	    /*
 	     * Check that service doing the impersonating is
@@ -2020,7 +2098,7 @@ server_lookup:
 		kdc_log(context, config, 0, "S4U2Self: %s is not allowed "
 			"to impersonate to service "
 			"(tried for user %s to service %s)",
-			cpn, tpn, spn);
+			cpn, for_user_str, spn);
 		goto out;
 	    }
 
@@ -2035,11 +2113,45 @@ server_lookup:
 		b->kdc_options.forwardable = 0;
 		str = "";
 	    }
+
 	    kdc_log(context, config, 0, "s4u2self %s impersonating %s to "
-		    "service %s %s", cpn, tpn, spn, str);
+                    "service %s %s", cpn, for_user_str, spn, str);
 	}
     }
 
+    ret = check_PAC(context, config, cp,
+		    for_user_principal, NULL,
+		    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 ?: "none", 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;
+    }
+
     /*
      * Constrained delegation
      */
@@ -2136,8 +2248,8 @@ server_lookup:
 	 * TODO: pass in t->sname and t->realm and build
 	 * a S4U_DELEGATION_INFO blob to the PAC.
 	 */
-	ret = check_PAC(context, config, tp, dp,
-			client, server, krbtgt,
+	ret = check_PAC(context, config, tp, NULL, dp,
+			client, server, krbtgt, NULL,
 			&clientkey->key,
 			ekey, &tkey_sign->key,
 			&adtkt, &rspac, &ad_signedpath);
@@ -2276,12 +2388,10 @@ server_lookup:
 			 reply);
 
 out:
-    if (tpn != cpn)
-	    free(tpn);
+    free(for_user_str);
     free(spn);
     free(cpn);
-    if (dpn)
-	free(dpn);
+    free(dpn);
 
     krb5_data_free(&rspac);
     krb5_free_keyblock_contents(context, &sessionkey);
@@ -2294,8 +2404,8 @@ out:
     if(s4u2self_impersonated_client)
 	_kdc_free_ent(context, s4u2self_impersonated_client);
 
-    if (tp && tp != cp)
-	krb5_free_principal(context, tp);
+    if (for_user_principal)
+	krb5_free_principal(context, for_user_principal);
     if (cp)
 	krb5_free_principal(context, cp);
     if (dp)
-- 
2.14.3


From d79d53f22e6f8fb57c770e4780f46c82cdbe6fea Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Fri, 20 Jul 2018 07:57:24 +0300
Subject: [PATCH 6/7] get_cred: fix s4u2self client 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.

Also, 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      | 131 ++++++++++++++++++++++++++++++-
 source4/heimdal/lib/krb5/init_creds_pw.c |  21 ++++-
 source4/torture/krb5/kdc-canon-heimdal.c |  50 +++++++++++-
 3 files changed, 195 insertions(+), 7 deletions(-)

diff --git a/source4/heimdal/lib/krb5/get_cred.c b/source4/heimdal/lib/krb5/get_cred.c
index 29ab6eaaa76..8c40606b6ca 100644
--- a/source4/heimdal/lib/krb5/get_cred.c
+++ b/source4/heimdal/lib/krb5/get_cred.c
@@ -919,6 +919,80 @@ get_cred_kdc_capath(krb5_context context,
     return ret;
 }
 
+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_referral(krb5_context context,
 		      krb5_kdc_flags flags,
@@ -932,7 +1006,7 @@ get_cred_kdc_referral(krb5_context context,
     krb5_const_realm client_realm;
     krb5_error_code ret;
     krb5_creds tgt, referral, ticket;
-    int loop = 0;
+    int loop = 0, have_tgt = 0;
     int ok_as_delegate = 1;
 
     if (in_creds->server->name.name_string.len < 2 && !flags.b.canonicalize) {
@@ -950,7 +1024,60 @@ get_cred_kdc_referral(krb5_context context,
 
     client_realm = krb5_principal_get_realm(context, in_creds->client);
 
-    /* find tgt for the clients base realm */
+    /* If the impersonate principal is an enterprise name, try lookup its realm */
+    if (impersonate_principal != NULL) {
+	ret = lookup_principal_realm(context, impersonate_principal, client_realm);
+	if (ret)
+		return ret;
+    }
+
+    /*
+     * 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);
+            have_tgt = 1;
+        }
+
+        /*
+	 * 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 (have_tgt == 0)
     {
 	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/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 c12f58bd07dc83598c4f9013c3e273e7cf04a883 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 7/7] s4u2self: add cross realm 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