Attachment 17248 Details for Bug 14951 – Patch for v4-14-test

Patch for v4-14-test

bfixes-tmp414.txt (text/plain), 4.38 KB, created by Stefan Metzmacher on 2022-03-25 12:44:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2022-03-25 12:44:19 UTC

Size: 4.38 KB

patch

obsolete

>From 77a5de2b13708ddd49833900aa516d2993547c1c Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Wed, 16 Feb 2022 14:11:10 +0100
>Subject: [PATCH] s4:kdc: strictly have 2 16-bit parts in krbtgt kvnos
>
>Even if the msDS-KeyVersionNumber of the main krbtgt
>account if larger than 65535, we need to have
>the 16 upper bits all zero in order to avoid
>mixing the keys with an RODC.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14951
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(similar to commit ab0946a75d51b8f4826d98c61c3ad503615009fe)
>---
> source4/kdc/db-glue.c | 51 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 43 insertions(+), 8 deletions(-)
>
>diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
>index aff74f2ee715..3a7e21766530 100644
>--- a/source4/kdc/db-glue.c
>+++ b/source4/kdc/db-glue.c
>@@ -44,6 +44,9 @@
> #define SAMBA_KVNO_GET_KRBTGT(kvno) \
> 	((uint16_t)(((uint32_t)kvno) >> 16))
> 
>+#define SAMBA_KVNO_GET_VALUE(kvno) \
>+	((uint16_t)(((uint32_t)kvno) & 0xFFFF))
>+
> #define SAMBA_KVNO_AND_KRBTGT(kvno, krbtgt) \
> 	((krb5_kvno)((((uint32_t)kvno) & 0xFFFF) | \
> 	 ((((uint32_t)krbtgt) << 16) & 0xFFFF0000)))
>@@ -323,6 +326,7 @@ static krb5_error_code samba_kdc_message2entry_keys(krb5_context context,
> 						    enum samba_kdc_ent_type ent_type,
> 						    struct sdb_entry_ex *entry_ex)
> {
>+	struct sdb_entry *entry = &entry_ex->entry;
> 	krb5_error_code ret = 0;
> 	enum ndr_err_code ndr_err;
> 	struct samr_Password *hash;
>@@ -333,10 +337,12 @@ static krb5_error_code samba_kdc_message2entry_keys(krb5_context context,
> 	struct package_PrimaryKerberosBlob _pkb;
> 	struct package_PrimaryKerberosCtr3 *pkb3 = NULL;
> 	struct package_PrimaryKerberosCtr4 *pkb4 = NULL;
>+	bool is_krbtgt = false;
>+	int krbtgt_number = 0;
>+	uint32_t current_kvno;
>+	uint32_t returned_kvno = 0;
> 	uint16_t i;
> 	uint16_t allocated_keys = 0;
>-	int rodc_krbtgt_number = 0;
>-	int kvno = 0;
> 	uint32_t supported_enctypes
> 		= ldb_msg_find_attr_as_uint(msg,
> 					    "msDS-SupportedEncryptionTypes",
>@@ -345,6 +351,7 @@ static krb5_error_code samba_kdc_message2entry_keys(krb5_context context,
> 	if (rid == DOMAIN_RID_KRBTGT || is_rodc) {
> 		/* KDCs (and KDCs on RODCs) use AES */
> 		supported_enctypes |= ENC_HMAC_SHA1_96_AES128 | ENC_HMAC_SHA1_96_AES256;
>+		is_krbtgt = true;
> 	} else if (userAccountControl & (UF_PARTIAL_SECRETS_ACCOUNT|UF_SERVER_TRUST_ACCOUNT)) {
> 		/* DCs and RODCs comptuer accounts use AES */
> 		supported_enctypes |= ENC_HMAC_SHA1_96_AES128 | ENC_HMAC_SHA1_96_AES256;
>@@ -369,9 +376,12 @@ static krb5_error_code samba_kdc_message2entry_keys(krb5_context context,
> 
> 	/* Is this the krbtgt or a RODC krbtgt */
> 	if (is_rodc) {
>-		rodc_krbtgt_number = ldb_msg_find_attr_as_int(msg, "msDS-SecondaryKrbTgtNumber", -1);
>+		krbtgt_number = ldb_msg_find_attr_as_int(msg, "msDS-SecondaryKrbTgtNumber", -1);
> 
>-		if (rodc_krbtgt_number == -1) {
>+		if (krbtgt_number == -1) {
>+			return EINVAL;
>+		}
>+		if (krbtgt_number == 0) {
> 			return EINVAL;
> 		}
> 	}
>@@ -454,11 +464,20 @@ static krb5_error_code samba_kdc_message2entry_keys(krb5_context context,
> 		goto out;
> 	}
> 
>-	kvno = ldb_msg_find_attr_as_int(msg, "msDS-KeyVersionNumber", 0);
>-	if (is_rodc) {
>-		kvno = SAMBA_KVNO_AND_KRBTGT(kvno, rodc_krbtgt_number);
>+	current_kvno = ldb_msg_find_attr_as_int(msg, "msDS-KeyVersionNumber", 0);
>+	if (is_krbtgt) {
>+		/*
>+		 * Even for the main krbtgt account
>+		 * we have to strictly split the kvno into
>+		 * two 16-bit parts and the upper 16-bit
>+		 * need to be all zero, even if
>+		 * the msDS-KeyVersionNumber has a value
>+		 * larger than 65535.
>+		 *
>+		 * See https://bugzilla.samba.org/show_bug.cgi?id=14951
>+		 */
>+		current_kvno = SAMBA_KVNO_GET_VALUE(current_kvno);
> 	}
>-	entry_ex->entry.kvno = kvno;
> 
> 	/* Get keys from the db */
> 
>@@ -709,6 +728,22 @@ static krb5_error_code samba_kdc_message2entry_keys(krb5_context context,
> 		}
> 	}
> 
>+	returned_kvno = current_kvno;
>+	if (is_krbtgt) {
>+		/*
>+		 * Even for the main krbtgt account
>+		 * we have to strictly split the kvno into
>+		 * two 16-bit parts and the upper 16-bit
>+		 * need to be all zero, even if
>+		 * the msDS-KeyVersionNumber has a value
>+		 * larger than 65535.
>+		 *
>+		 * See https://bugzilla.samba.org/show_bug.cgi?id=14951
>+		 */
>+		returned_kvno = SAMBA_KVNO_AND_KRBTGT(returned_kvno, krbtgt_number);
>+	}
>+	entry->kvno = returned_kvno;
>+
> out:
> 	if (ret != 0) {
> 		entry_ex->entry.keys.len = 0;
>-- 
>2.25.1
>

Flags:

abartlet: review+

Actions: View

Attachments on bug 14951: 17104 | 17246 | 17247 | 17248 | 17249 | 17255