Cross realm S4U2Self cont.

Isaac Boukris iboukris at gmail.com
Fri Aug 3 13:44:48 UTC 2018


Hi,

On Tue, Jul 24, 2018 at 3:15 PM, Isaac Boukris <iboukris at gmail.com> wrote:
> I made a prove-of-concept patch for these issues, including a test
> using net-kerberos-pac-dump command, see (works against AD trust both
> ways and between samba DCs):
> https://github.com/samba-team/samba/pull/204

I made some progress, see attached updated patch.

The CI on github keeps failing on time, but it passed on gitlab, see branch:
https://gitlab.com/samba-team/devel/samba/commits/iboukris_xrealm_s4u2self
Last pipeline:
https://gitlab.com/samba-team/devel/samba/pipelines/27124742


Note that unlike what I said initially, transitive trust still does
not work (only direct trust). I thought it worked when I was testing
with kgetcred against windows but when I try to actually accept the
ticket by the service, it fails at krb5_check_transited(). Also, there
seem to be the same issue on the KDC side when Samba KDC is in a the
trust path.

> To solve the pac signing issue with full name I used enterprise name
> as a trick so it'll end up fully parsed, but perhaps we need an
> alternative krb5_pac_verify_ref() instead.
> I currently also skip 'check_s4u2self' and delegation checks since we
> don't have ta db handle locally for the service, assuming their kdc
> would check that, but I am not sure that's ok.

I think it is not a problem to skip 'check_s4u2self()', since we are
not issuing a ticket for self but a referral.
Same for delegation and forwardable flag check, MS-SFU 3.2.5.1.x only
specify these check when the KDC replies with a service ticket (not a
referral).

> Currently some tests are failing, namely ones related to s4u,
> enterprise names, canonicalize and so on, and I'm trying to work
> around some of them by falling back to old behavior for cases where we
> as a service wrongly assume the user isn't in our realm due to naming
> differences (netbios, upn).
> A better fix would be to implement "Using the User's Realm and User
> Name to Identify the User" section from MS-SFU 3.1.5.1.1.1 by sending
> AS request and chasing client referrals to find user's actual realm,
> but meanwhile falling back to requesting from own kdc might save some
> tests.

I've implemented the realm lookup for enterprise princiapls using
client referrals, and have adjusted the torture test for it.
However, it still doesn't help for the case where netbios-realm or
lower-realm are used without enterprise name type, so I left the
fallback to the old behavior to request the ticket from the kdc of the
service (which works in these cases, since they are the same domain in
these tests).
I think it might be possible to achieve a better behavior, which could
work for netbios/lower-realm even with a trusted domain, by using the
referral tickets we get in these cases, however we'd first need to fix
the krb5_check_transited() problem (since the client thinks its
chasing referrals).


Thoughts?
-------------- next part --------------
From a0003fd80a3ed83f38d2d32e26f977a55a115dcb 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/6] 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 ffa67d8f525..c3a0534c455 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 671247dc902220af9352af7d8f489607f5e07352 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/6] 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 89954b19493a6884ffe72122eb7ad7d2dc8d0fe3 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/6] 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 f9957308aba0f715db21f205b0930c4491dfd2cc 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/6] 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 1128645b184305bdf635959a13bf67a550601926 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 5/6] 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 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      | 139 ++++++++++++++++++++++++++++++-
 source4/heimdal/lib/krb5/init_creds_pw.c |  21 ++++-
 2 files changed, 156 insertions(+), 4 deletions(-)

diff --git a/source4/heimdal/lib/krb5/get_cred.c b/source4/heimdal/lib/krb5/get_cred.c
index 29ab6eaaa76..5df980e9368 100644
--- a/source4/heimdal/lib/krb5/get_cred.c
+++ b/source4/heimdal/lib/krb5/get_cred.c
@@ -919,6 +919,89 @@ 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 KRB5KDC_ERR_PREAUTH_REQUIRED:
+		ret = code;
+		break;
+	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 KRB5KRB_ERR_RESPONSE_TOO_BIG: {
+		if (krb5_sendto_ctx_get_flags(ctx) & KRB5_KRBHST_FLAGS_LARGE_MSG)
+		break;
+		krb5_sendto_ctx_add_flags(ctx, KRB5_KRBHST_FLAGS_LARGE_MSG);
+		*action = KRB5_SENDTO_RESTART;
+		break;
+	}
+	case KRB5KDC_ERR_SVC_UNAVAILABLE:
+		*action = KRB5_SENDTO_CONTINUE;
+		break;
+	}
+
+	krb5_free_error_contents(context, &error);
+	return ret;
+}
+
+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;
+
+	krb5_get_init_creds_opt_set_win2k(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 */
+	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 +1015,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 +1033,59 @@ 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 we impersonate an enterprise principal, 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_tgt = NULL;
+        krb5_creds **referral_tgts = NULL;
+        int i;
+
+        referral = *in_creds;
+
+        ret = krb5_make_principal(context, &referral.server,
+                                  client_realm, KRB5_TGS_NAME,
+                                  impersonate_principal->realm, NULL);
+        if (ret) {
+            return ret;
+        }
+
+        krb5_principal_set_type(context, referral.server, KRB5_NT_SRV_INST);
+
+        ret = get_cred_kdc_referral(context, flags, ccache, &referral, NULL,
+                                    NULL, &impersonate_tgt, &referral_tgts);
+
+        krb5_free_principal(context, referral.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.
  *
-- 
2.14.3


From ae58076dd7fb04f541e2c01578c17823e57f02dc Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Sat, 21 Jul 2018 12:11:29 +0300
Subject: [PATCH 6/6] WIP: kdc: fix cross realm s4u2self with forest trust

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/kdc/krb5tgs.c                   | 253 ++++++++++++++++--------
 source4/torture/krb5/kdc-canon-heimdal.c        |  34 +++-
 testprogs/blackbox/test_kinit_trusts_heimdal.sh |  17 ++
 3 files changed, 214 insertions(+), 90 deletions(-)

diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index c5456e4cad9..de46489c105 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -1506,6 +1506,11 @@ 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 pac_logon_name = NULL;
+    krb5_principal for_user_principal = NULL;
+    krb5_principal for_user_enterprise = NULL;
+    char *pac_logon_name_str = 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 +1871,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
      */
@@ -1904,8 +1878,11 @@ server_lookup:
     /* by default the tgt principal matches the client principal */
     tp = cp;
     tpn = cpn;
+    pac_logon_name = cp;
+    pac_logon_name_str = cpn;
 
-    if (client) {
+    /* Check for PADATA_FOR_USER */
+    {
 	const PA_DATA *sdata;
 	int i = 0;
 
@@ -1914,7 +1891,6 @@ server_lookup:
 	    krb5_crypto crypto;
 	    krb5_data datack;
 	    PA_S4U2Self self;
-	    const char *str;
 
 	    ret = decode_PA_S4U2Self(sdata->padata_value.data,
 				     sdata->padata_value.length,
@@ -1956,60 +1932,115 @@ server_lookup:
 	    }
 
 	    ret = _krb5_principalname2krb5_principal(context,
-						     &tp,
+						     &for_user_principal,
 						     self.name,
 						     self.realm);
 	    free_PA_S4U2Self(&self);
 	    if (ret)
 		goto out;
 
-	    ret = krb5_unparse_name(context, tp, &tpn);
+	    ret = krb5_unparse_name(context, for_user_principal, &for_user_str);
 	    if (ret)
 		goto out;
 
-	    /* 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;
-		    }
-		}
-	    }
+	    ret = krb5_parse_name_flags(context, for_user_str,
+                                        KRB5_PRINCIPAL_PARSE_ENTERPRISE,
+                                        &for_user_enterprise);
+	    if (ret)
+		goto out;
+
+	    /* If the user is in our krbtgt realm, then we would need a new PAC */
+	    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 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 principal being impersonated does not belong to our
+	     * krbtgt realm then the PAC must be of the impersonated principal,
+	     * with the pac-logon-name in fromat of user at REALM.
+	     */
+
+            if (s4u2self_impersonated_client == NULL) {
+                /*
+		 * We use enterprise-name as a trick so it will get fully
+		 * parsed, including the realm component.
+		 */
+		pac_logon_name = for_user_enterprise;
+		pac_logon_name_str = for_user_str;
+            }
+
+	    /*
+	     * Otherwise, we expect the PAC of the impersonator principal,
+	     * with a matching pac-logon-name field. We will make sure to
+	     * free it later on, and issue a new PAC for our principal.
+	     */
+	}
+    }
+
+    kdc_log(context, config, 0, "Checking PAC of user: %s", pac_logon_name_str);
+
+    ret = check_PAC(context, config, pac_logon_name, 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, pac_logon_name_str, 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;
+    }
+
+    if (for_user_principal != NULL) {
+        krb5_principal pac_sign_principal;
+
+        /*
+	 * 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 to adjust cname and crealm to the impersonated user,
+	 * and also verify that it is actually asking a ticket for self.
+	 */
+	if (client != NULL) {
+	    const char *str;
+            pac_sign_principal = for_user_principal;
+
+            tp = for_user_principal;
+            tpn = for_user_str;
 
 	    /*
 	     * Check that service doing the impersonating is
@@ -2020,7 +2051,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,9 +2066,57 @@ 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);
+        }
+        else {
+            /*
+             * If the service does not belong to our realm, we need to sign
+             * the PAC with full username user at REALM - use enterprise name.
+             */
+            pac_sign_principal = for_user_enterprise;
+        }
+
+        /* 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;
+
+            if (s4u2self_impersonated_client != NULL) {
+                ret = _kdc_pac_generate(context, s4u2self_impersonated_client, NULL, &p);
+                if (ret) {
+                    kdc_log(context, config, 0, "PAC generation failed for -- %s", for_user_str);
+                    goto out;
+                }
+
+                kdc_log(context, config, 0, "Issued new PAC for impersonated user: %s", for_user_str);
+            }
+            else {
+		/* In case the impersonated principal does not belong to our realm, then we
+		 * have checked above that the PAC matches the for-user principal, we need
+		 * to reparse it in order to resign. */
+                ret = krb5_pac_parse(context, rspac.data, rspac.length, &p);
+                if (ret) {
+                    kdc_log(context, config, 0, "PAC reparse failed for -- %s", for_user_str);
+                    goto out;
+                }
+            }
+
+            krb5_data_free(&rspac);
+            krb5_data_zero(&rspac);
+
+            /* Resign the PAC with the correct pac-logon-name */
+            if (p != NULL) {
+                ret = _krb5_pac_sign(context, p, ticket->ticket.authtime,
+                                     pac_sign_principal, ekey,
+                                     &tkey_sign->key, &rspac);
+                krb5_pac_free(context, p);
+                if (ret) {
+                    kdc_log(context, config, 0, "PAC signing failed for -- %s", for_user_str);
+                    goto out;
+                }
+            }
+        }
     }
 
     /*
@@ -2276,12 +2355,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 +2371,10 @@ 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 (for_user_enterprise)
+	krb5_free_principal(context, for_user_enterprise);
     if (cp)
 	krb5_free_principal(context, cp);
     if (dp)
diff --git a/source4/torture/krb5/kdc-canon-heimdal.c b/source4/torture/krb5/kdc-canon-heimdal.c
index 5b782a23fc4..a69ced9346e 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,10 @@ 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) {
+				test_context->test_stage = TEST_TGS_REQ_CANON;
+				test_context->reset_count = true;
+			}
 		}
 
 		free_KRB_ERROR(&error);
@@ -431,7 +437,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 +461,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 +550,12 @@ 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 (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 +686,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 +1375,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 +1836,15 @@ static bool torture_krb5_as_req_canon(struct torture_context *tctx, const void *
 									    opt,
 									    principal),
 					 0, "krb5_get_creds_opt_set_impersonate failed");
+		test_context->impersonating = true;
+		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))
+		{
+			test_context->test_stage = TEST_TGS_REQ_KRBTGT_CANON;
+		}
 	}
 
 	/* Confirm if we can get a ticket to our own name */
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