A question about S4U2Self with MIT KDC

[Alexander Bokovoy]

On ke, 06 helmi 2019, Isaac Boukris via samba-technical wrote:
> 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

According to MS-PAC 4.1.2 Authorization Validation and Filtering,

--------

When a PAC is conveyed across a trust boundary, the receiving server
addresses the threat of forged identities in the PAC. For example, the
PAC might contain SIDs that are actually from the receiving server's
domain rather than from the domain of the principal the PAC is supposed
to represent. While a correctly functioning domain controller would not
do that, if a domain controller were compromised by an attacker, the
attacker might create arbitrary PACs in an effort to attack other
domains.  To mitigate this threat, any KDC accepting a PAC from another
domain through an interdomain trust has to filter out any SIDs that are
not correct. To filter the SIDs and client names correctly and safely,
an implementation can use the guidelines discussed in the following
sections.<21><22>

--------

My reading of this is that it implies PAC validation cross-realm.

> 
> 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.

> 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
> 


-- 
/ Alexander Bokovoy