From: Andrew Bartlett <abartlet@samba.org>
Date: Wed, 6 Mar 2024 04:43:47 +0000 (+1300)
Subject: s4-auth/kerberos: Do not add true duplicates to exported keytab
X-Git-Tag: tdb-1.4.11~1446
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7b662a928784c889f0d0e41244444b723fa6fd20;p=thirdparty%2Fsamba.git

s4-auth/kerberos: Do not add true duplicates to exported keytab

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jo Sutton <josutton@catalyst.net.nz>
---

diff --git a/source4/auth/kerberos/kerberos_util.c b/source4/auth/kerberos/kerberos_util.c
index ba8d822f638..30d9400bffb 100644
--- a/source4/auth/kerberos/kerberos_util.c
+++ b/source4/auth/kerberos/kerberos_util.c
@@ -719,3 +719,106 @@ done:
 	talloc_free(tmp_ctx);
 	return code;
 }
+
+/*
+ * Walk the keytab, looking for entries of this principal name,
+ * with KVNO and key equal
+ *
+ * These entries do not need to be replaced, so we want to tell the caller not to add them again
+ *
+ * Inspired by the code in Samba3 for 'use kerberos keytab'.
+ */
+krb5_error_code smb_krb5_is_exact_entry_in_keytab(TALLOC_CTX *mem_ctx,
+						  krb5_context context,
+						  krb5_keytab keytab,
+						  krb5_keytab_entry *to_match,
+						  bool *found,
+						  const char **error_string)
+{
+	TALLOC_CTX *tmp_ctx;
+	krb5_error_code code;
+	krb5_kt_cursor cursor;
+
+	tmp_ctx = talloc_new(mem_ctx);
+	if (tmp_ctx == NULL) {
+		*error_string = "Cannot allocate tmp_ctx";
+		return ENOMEM;
+	}
+
+	*found = false;
+
+	code = krb5_kt_start_seq_get(context, keytab, &cursor);
+	switch (code) {
+	case 0:
+		break;
+#ifdef HEIM_ERR_OPNOTSUPP
+	case HEIM_ERR_OPNOTSUPP:
+#endif
+	case ENOENT:
+	case KRB5_KT_END:
+		/* no point enumerating if there isn't anything here */
+		code = 0;
+		goto done;
+	default:
+		*error_string = talloc_asprintf(mem_ctx,
+						"failed to open keytab for read of existing entries: %s\n",
+						smb_get_krb5_error_message(context, code, mem_ctx));
+		goto done;
+	}
+
+	do {
+		krb5_keytab_entry entry;
+		bool matched = false;
+		krb5_boolean ok;
+
+		code = krb5_kt_next_entry(context, keytab, &entry, &cursor);
+		if (code) {
+			break;
+		}
+
+		ok = smb_krb5_kt_compare(context,
+					 &entry,
+					 to_match->principal,
+					 to_match->vno,
+					 KRB5_KEY_TYPE(KRB5_KT_KEY(to_match)));
+		if (ok) {
+			/* This is not a security check, constant time is not required */
+			if ((KRB5_KEY_LENGTH(KRB5_KT_KEY(&entry)) == KRB5_KEY_LENGTH(KRB5_KT_KEY(to_match)))
+			    && memcmp(KRB5_KEY_DATA(KRB5_KT_KEY(&entry)), KRB5_KEY_DATA(KRB5_KT_KEY(to_match)),
+				      KRB5_KEY_LENGTH(KRB5_KT_KEY(&entry))) == 0) {
+				matched = true;
+			}
+		}
+
+		/* Free the entry, we don't need it any more */
+		krb5_kt_free_entry(context, &entry);
+		/* Make sure we do not double free */
+		ZERO_STRUCT(entry);
+		if (matched) {
+			*found = true;
+			break;
+		}
+	} while (code == 0);
+
+	krb5_kt_end_seq_get(context, keytab, &cursor);
+
+	switch (code) {
+	case 0:
+		break;
+	case ENOENT:
+	case KRB5_KT_END:
+		break;
+	default:
+		*error_string = talloc_asprintf(mem_ctx,
+						"failed in checking old entries for principal: %s\n",
+						smb_get_krb5_error_message(context,
+									   code,
+									   mem_ctx));
+		goto done;
+	}
+
+	code = 0;
+done:
+	talloc_free(tmp_ctx);
+	return code;
+}
diff --git a/source4/auth/kerberos/srv_keytab.c b/source4/auth/kerberos/srv_keytab.c
index 899a677bc61..fdc2fb4012f 100644
--- a/source4/auth/kerberos/srv_keytab.c
+++ b/source4/auth/kerberos/srv_keytab.c
@@ -84,11 +84,36 @@ static krb5_error_code keytab_add_keys(TALLOC_CTX *parent_ctx,
 			return ret;
 		}
 
-                entry.vno = kvno;
+		entry.vno = kvno;
 
 		for (p = 0; p < num_principals; p++) {
+			bool found = false;
+
 			unparsed = NULL;
 			entry.principal = principals[p];
+
+			ret = smb_krb5_is_exact_entry_in_keytab(parent_ctx,
+								context,
+								keytab,
+								&entry,
+								&found,
+								error_string);
+			if (ret != 0) {
+				krb5_free_keyblock_contents(context,
+							    KRB5_KT_KEY(&entry));
+				return ret;
+			}
+
+			/*
+			 * Do not add the exact same key twice, this
+			 * will allow "samba-tool domain exportkeytab"
+			 * to refresh a keytab rather than infinitely
+			 * extend it
+			 */
+			if (found) {
+				continue;
+			}
+
 			ret = krb5_kt_add_entry(context, keytab, &entry);
 			if (ret != 0) {
 				char *k5_error_string =
@@ -206,7 +231,6 @@ NTSTATUS smb_krb5_fill_keytab_gmsa_keys(TALLOC_CTX *mem_ctx,
 					krb5_principal principal,
 					struct ldb_context *samdb,
 					struct ldb_dn *dn,
-					bool include_previous,
 					const char **error_string)
 {
 	const char *gmsa_attrs[] = {
@@ -378,7 +402,7 @@ NTSTATUS smb_krb5_fill_keytab_gmsa_keys(TALLOC_CTX *mem_ctx,
 				   &principal,
 				   context,
 				   keytab,
-				   include_previous,
+				   true,
 				   error_string);
 	if (ret) {
 		*error_string = talloc_asprintf(mem_ctx,
diff --git a/source4/libnet/libnet_export_keytab.c b/source4/libnet/libnet_export_keytab.c
index 6374f2b0e56..2d3b0ba7c3a 100644
--- a/source4/libnet/libnet_export_keytab.c
+++ b/source4/libnet/libnet_export_keytab.c
@@ -148,7 +148,6 @@ static NTSTATUS sdb_kt_copy(TALLOC_CTX *mem_ctx,
 								sentry.principal,
 								db_ctx->samdb,
 								dn,
-								found_previous ? false : true,
 								error_string);
 			if (NT_STATUS_IS_OK(status)) {
 				keys_exported = true;