CVE-2020-25719 mit-samba: Rework PAC handling in kdb_samba_db_sign_auth_data()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14561

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

2 files changed, 93 insertions, 29 deletions

diff --git a/selftest/knownfail_mit_kdc b/selftest/knownfail_mit_kdc
index 04efccf4a59..53aa05814c2 100644
--- a/selftest/knownfail_mit_kdc
+++ b/selftest/knownfail_mit_kdc
@@ -278,12 +278,13 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_
 #
 # KDC TGS PAC tests
 #
+^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_client_no_auth_data_required\(ad_dc\)
+^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_no_pac_client_no_auth_data_required\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_no_pac_service_no_auth_data_required\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_remove_pac\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_remove_pac_client_no_auth_data_required\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_remove_pac_service_no_auth_data_required\(ad_dc\)
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_request_no_pac\(ad_dc\)
-^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_service_no_auth_data_required\(ad_dc\)
 #
 # MIT currently fails the following MS-KILE tests.
 #
@@ -501,11 +502,9 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_
 #
 # PAC request tests
 #
-^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_renew_pac_request_false
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_pac_request_false
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_pac_request_none
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_s4u2self_pac_request_true
-^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_pac_request_false
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_pac_request_false
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_pac_request_none
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_tgs_rodc_pac_request_true
@@ -515,7 +514,6 @@ samba.tests.krb5.as_canonicalization_tests.samba.tests.krb5.as_canonicalization_
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_user2user_user_pac_request_false
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_user2user_user_pac_request_none
 ^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_user2user_user_pac_request_true
-^samba.tests.krb5.kdc_tgs_tests.samba.tests.krb5.kdc_tgs_tests.KdcTgsTests.test_validate_pac_request_false
 #
 # PAC requester SID tests
 #
diff --git a/source4/kdc/mit-kdb/kdb_samba_policies.c b/source4/kdc/mit-kdb/kdb_samba_policies.c
index dce87c50049..7bc9a7b3347 100644
--- a/source4/kdc/mit-kdb/kdb_samba_policies.c
+++ b/source4/kdc/mit-kdb/kdb_samba_policies.c
@@ -4,7 +4,7 @@
    Samba KDB plugin for MIT Kerberos
 
    Copyright (c) 2010      Simo Sorce <idra@samba.org>.
-   Copyright (c) 2014      Andreas Schneider <asn@samba.org>
+   Copyright (c) 2014-2021 Andreas Schneider <asn@samba.org>
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -325,11 +325,16 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
 #endif
 	krb5_const_principal ks_client_princ = NULL;
 	krb5_db_entry *client_entry = NULL;
+	krb5_authdata **pac_auth_data = NULL;
 	krb5_authdata **authdata = NULL;
 	krb5_boolean is_as_req;
 	krb5_error_code code;
 	krb5_pac pac = NULL;
 	krb5_data pac_data;
+	bool with_pac = false;
+	bool generate_pac = false;
+	char *client_name = NULL;
+
 
 #if KRB5_KDB_API_VERSION >= 10
 	krbtgt = krbtgt == NULL ? local_krbtgt : krbtgt;
@@ -374,8 +379,6 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
 							0,
 							&client_entry);
 				if (code != 0) {
-					char *client_name = NULL;
-
 					(void)krb5_unparse_name(context,
 								ks_client_princ,
 								&client_name);
@@ -407,43 +410,105 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
 		client_entry = client;
 	}
 
-	if (is_as_req && (flags & KRB5_KDB_FLAG_INCLUDE_PAC)) {
+	if (is_as_req) {
+		with_pac = mit_samba_princ_needs_pac(client_entry);
+	} else {
+		with_pac = mit_samba_princ_needs_pac(server);
+	}
+
+	code = krb5_unparse_name(context,
+				 client_princ,
+				 &client_name);
+	if (code != 0) {
+		goto done;
+	}
+
+	if (is_as_req && (flags & KRB5_KDB_FLAG_INCLUDE_PAC) != 0) {
+		generate_pac = true;
+	}
+
+	DBG_DEBUG("*** Sign data for client principal: %s [%s %s%s]\n",
+		  client_name,
+		  is_as_req ? "AS-REQ" : "TGS_REQ",
+		  with_pac ? is_as_req ? "WITH_PAC" : "FIND_PAC" : "NO_PAC",
+		  generate_pac ? " GENERATE_PAC" : "");
+
+	/*
+	 * Generate PAC for the AS-REQ or check or generate one for the TGS if
+	 * needed.
+	 */
+	if (with_pac && generate_pac) {
+		DBG_DEBUG("Generate PAC for AS-REQ [%s]\n", client_name);
 		code = ks_get_pac(context, client_entry, client_key, &pac);
 		if (code != 0) {
 			goto done;
 		}
-	}
-
-	if (!is_as_req) {
-		code = ks_verify_pac(context,
-				     flags,
-				     ks_client_princ,
-				     client_entry,
-				     server,
-				     krbtgt,
-				     server_key,
-				     krbtgt_key,
-				     authtime,
-				     tgt_auth_data,
-				     &pac);
+	} else if (with_pac && !is_as_req) {
+		/*
+		 * Find the PAC in the TGS, if one exists.
+		 */
+		code = krb5_find_authdata(context,
+					  tgt_auth_data,
+					  NULL,
+					  KRB5_AUTHDATA_WIN2K_PAC,
+					  &pac_auth_data);
 		if (code != 0) {
+			DBG_ERR("krb5_find_authdata failed: %d\n", code);
 			goto done;
 		}
-	}
+		DBG_DEBUG("Found PAC data for TGS-REQ [%s]\n", client_name);
 
-	if (pac == NULL) {
+		if (pac_auth_data != NULL && pac_auth_data[0] != NULL) {
+			if (pac_auth_data[1] != NULL) {
+				DBG_ERR("Invalid PAC data!\n");
+				code = KRB5KDC_ERR_BADOPTION;
+				goto done;
+			}
 
-		code = ks_get_pac(context, client_entry, client_key, &pac);
-		if (code != 0) {
-			goto done;
+			DBG_DEBUG("Verify PAC for TGS [%s]\n",
+				client_name);
+
+			code = ks_verify_pac(context,
+					     flags,
+					     ks_client_princ,
+					     client_entry,
+					     server,
+					     krbtgt,
+					     server_key,
+					     krbtgt_key,
+					     authtime,
+					     tgt_auth_data,
+					     &pac);
+			if (code != 0) {
+				goto done;
+			}
+		} else {
+			if (flags & KRB5_KDB_FLAG_CONSTRAINED_DELEGATION) {
+				DBG_DEBUG("Generate PAC for constrained"
+					  "delegation TGS [%s]\n",
+					  client_name);
+
+				code = ks_get_pac(context,
+						  client_entry,
+						  client_key,
+						  &pac);
+				if (code != 0 && code != ENOENT) {
+					goto done;
+				}
+			}
 		}
 	}
 
 	if (pac == NULL) {
-		code = KRB5_KDB_DBTYPE_NOSUP;
+		DBG_DEBUG("No PAC data - we're done [%s]\n", client_name);
+		*signed_auth_data = NULL;
+		code = 0;
 		goto done;
 	}
 
+	DBG_DEBUG("Signing PAC for %s [%s]\n",
+		  is_as_req ? "AS-REQ" : "TGS-REQ",
+		  client_name);
 	code = krb5_pac_sign(context, pac, authtime, ks_client_princ,
 			server_key, krbtgt_key, &pac_data);
 	if (code != 0) {
@@ -481,8 +546,9 @@ done:
 	if (client_entry != NULL && client_entry != client) {
 		ks_free_principal(context, client_entry);
 	}
-	krb5_pac_free(context, pac);
+	SAFE_FREE(client_name);
 	krb5_free_authdata(context, authdata);
+	krb5_pac_free(context, pac);
 
 	return code;
 }