A question about S4U2Self with MIT KDC

[Isaac Boukris]

Hi team,

Given the cross-realm S4U2Self patches have landed upstream mit-krb5
(see PR #853), I'd like to work on Samba side as well. However I've
been struggling with the following question (which applies to in-realm
as well); when processing a S4U2Self TGS-REQ is it required to verify
the of the PAC in the TGT or can it be ignored?

Technically, we should be able to verify it (we have both krbtgt and
server key, or at least the server key in case of cross-realm), and
Heimdal indeed does that. But the sign_authdata() plugin method in
MIT, currently does not provide us with the tgt-client principal name
to match against the PAC (but only with the impersonate principal to
construct a new PAC).
For in-realm requests we could workaround this by matching against
'server->princ' since it is the same principal (assuming the PAC was
signed using the canonical name) but in cross realm this isn't the
case, so we may need a new API to be able to verify the PAC.

I've raised this question back then on krbdev (referenced in
aforementioned PR) and we couldn't think of a sensible reason why we'd
need to verify the PAC. However it seems that Windows KDC requires
that the TGT does include a PAC, since the following command yields
err_c_principal_unknown:
$ echo -n pwd | kinit apache at ACME.COM --no-request-pac && kvno
-Uuser at abc@ACME.COM apache at ACME.COM

This does not prove that it actually verifies the PAC but does imply
that. I tried to prove it further by  trying to join a Samba DC to a
Windows domain in order to forge a TGT with a PAC using a wrong
principal and signatures, but so far I have not succeeded joining as a
DC to a Windows domain. I'm getting WERR_DS_NO_CROSSREF_FOR_NC error
against w2k8 and w2k16 - any help on this will be appreciated as well.

See attached initial patch for in-realm S4U2Self with MIT KDC (it does
not currently verify the TGT PAC, but simply discards it).

Thank you for your thoughts.
-------------- next part --------------
From 8a99452812a4f7034568c05d3a9ec18516efa495 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Mon, 4 Feb 2019 10:24:22 +0200
Subject: [PATCH] [WIP] MIT-KDC: Fix S4U2Self handling in Samba plugin

When processing S4U2Self TGS-REQ and the impersonate principal
is from local realm, then we need to discard the PAC from TGT
and create a new one for the impersonate principal.

TODO:
Do we need to verify the PAC in the TGT before we discard it?

Also, we can skip check of KRB5_KDB_FLAG_INCLUDE_PAC flag, as
the server would not have called us if it was off, see:
https://lists.samba.org/archive/samba-technical/2018-September/130323.html

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/kdc/mit-kdb/kdb_samba_policies.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/source4/kdc/mit-kdb/kdb_samba_policies.c b/source4/kdc/mit-kdb/kdb_samba_policies.c
index fc80329f221..9efb663c833 100644
--- a/source4/kdc/mit-kdb/kdb_samba_policies.c
+++ b/source4/kdc/mit-kdb/kdb_samba_policies.c
@@ -309,7 +309,7 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
 	krb5_data pac_data;
 
 	/* Prefer canonicalised name from client entry */
-	if (client != NULL) {
+	if (client != NULL && !(flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION)) {
 		ks_client_princ = client->princ;
 	} else {
 		ks_client_princ = client_princ;
@@ -317,14 +317,12 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
 
 	is_as_req = ((flags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) != 0);
 
-	if (is_as_req && (flags & KRB5_KDB_FLAG_INCLUDE_PAC)) {
+	if (is_as_req || (flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION)) {
 		code = ks_get_pac(context, client, client_key, &pac);
 		if (code != 0) {
 			goto done;
 		}
-	}
-
-	if (!is_as_req) {
+	} else {
 		code = ks_verify_pac(context,
 				     flags,
 				     ks_client_princ,
@@ -341,14 +339,6 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
 		}
 	}
 
-	if (pac == NULL && client != NULL) {
-
-		code = ks_get_pac(context, client, client_key, &pac);
-		if (code != 0) {
-			goto done;
-		}
-	}
-
 	if (pac == NULL) {
 		code = KRB5_KDB_DBTYPE_NOSUP;
 		goto done;
-- 
2.14.4