Attachment 18111 Details for Bug 15476 – Patches for v4-17-test

Patches for v4-17-test

bug15476-v4-17.txt (text/plain), 7.81 KB, created by Stefan Metzmacher on 2023-09-19 22:49:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2023-09-19 22:49:31 UTC

Size: 7.81 KB

patch

obsolete

>From 9080a8fba35389ba70bb774c3c698440de12b58c Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Sat, 24 Sep 2022 12:36:25 +1200
>Subject: [PATCH 1/3] lib:krb5_wrap: Add helper functions to make krb5_data
> structure
>
>These will be used in following commits.
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 84796220965527a56ac492d04f220b39ce279cf4)
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15476
>---
> lib/krb5_wrap/krb5_samba.c               | 30 ++++++++++++++++++++++++
> lib/krb5_wrap/krb5_samba.h               |  5 ++++
> source4/kdc/mit-kdb/kdb_samba.h          |  2 --
> source4/kdc/mit-kdb/kdb_samba_common.c   | 11 ---------
> source4/kdc/mit-kdb/kdb_samba_policies.c |  4 +++-
> 5 files changed, 38 insertions(+), 14 deletions(-)
>
>diff --git a/lib/krb5_wrap/krb5_samba.c b/lib/krb5_wrap/krb5_samba.c
>index 81265cd4a731..70012e08cf18 100644
>--- a/lib/krb5_wrap/krb5_samba.c
>+++ b/lib/krb5_wrap/krb5_samba.c
>@@ -938,6 +938,36 @@ krb5_error_code smb_krb5_copy_data_contents(krb5_data *p,
> #endif
> }
> 
>+/*
>+ * @brief put a buffer reference into a krb5_data struct
>+ *
>+ * @param[in] data		The data to reference
>+ * @param[in] length		The length of the data to reference
>+ * @return krb5_data
>+ *
>+ * Caller should not free krb5_data.
>+ */
>+krb5_data smb_krb5_make_data(void *data,
>+			     size_t len)
>+{
>+	krb5_data d;
>+
>+#ifdef SAMBA4_USES_HEIMDAL
>+	d.data = (uint8_t *)data;
>+	d.length = len;
>+#else
>+	d.magic = KV5M_DATA;
>+	d.data = data;
>+	d.length = len;
>+#endif
>+	return d;
>+}
>+
>+krb5_data smb_krb5_data_from_blob(DATA_BLOB blob)
>+{
>+	return smb_krb5_make_data(blob.data, blob.length);
>+}
>+
> bool smb_krb5_get_smb_session_key(TALLOC_CTX *mem_ctx,
> 				  krb5_context context,
> 				  krb5_auth_context auth_context,
>diff --git a/lib/krb5_wrap/krb5_samba.h b/lib/krb5_wrap/krb5_samba.h
>index 942f787d12a4..2bb04ba5a464 100644
>--- a/lib/krb5_wrap/krb5_samba.h
>+++ b/lib/krb5_wrap/krb5_samba.h
>@@ -383,6 +383,11 @@ krb5_error_code smb_krb5_copy_data_contents(krb5_data *p,
> 					    const void *data,
> 					    size_t len);
> 
>+krb5_data smb_krb5_make_data(void *data,
>+			     size_t len);
>+
>+krb5_data smb_krb5_data_from_blob(DATA_BLOB blob);
>+
> int smb_krb5_principal_get_type(krb5_context context,
> 				krb5_const_principal principal);
> 
>diff --git a/source4/kdc/mit-kdb/kdb_samba.h b/source4/kdc/mit-kdb/kdb_samba.h
>index 138105ef2111..acd5a7779736 100644
>--- a/source4/kdc/mit-kdb/kdb_samba.h
>+++ b/source4/kdc/mit-kdb/kdb_samba.h
>@@ -50,8 +50,6 @@ void ks_free_principal(krb5_context context, krb5_db_entry *entry);
> 
> bool ks_data_eq_string(krb5_data d, const char *s);
> 
>-krb5_data ks_make_data(void *data, unsigned int len);
>-
> krb5_boolean ks_is_kadmin(krb5_context context,
> 			  krb5_const_principal princ);
> 
>diff --git a/source4/kdc/mit-kdb/kdb_samba_common.c b/source4/kdc/mit-kdb/kdb_samba_common.c
>index 1ad1c1485968..5ba845ec2668 100644
>--- a/source4/kdc/mit-kdb/kdb_samba_common.c
>+++ b/source4/kdc/mit-kdb/kdb_samba_common.c
>@@ -71,17 +71,6 @@ bool ks_data_eq_string(krb5_data d, const char *s)
> 	return true;
> }
> 
>-krb5_data ks_make_data(void *data, unsigned int len)
>-{
>-	krb5_data d;
>-
>-	d.magic = KV5M_DATA;
>-	d.data = data;
>-	d.length = len;
>-
>-	return d;
>-}
>-
> krb5_boolean ks_is_kadmin(krb5_context context,
> 			  krb5_const_principal princ)
> {
>diff --git a/source4/kdc/mit-kdb/kdb_samba_policies.c b/source4/kdc/mit-kdb/kdb_samba_policies.c
>index 000485455af4..b5a2e25ec170 100644
>--- a/source4/kdc/mit-kdb/kdb_samba_policies.c
>+++ b/source4/kdc/mit-kdb/kdb_samba_policies.c
>@@ -26,6 +26,8 @@
> #include "lib/util/debug.h"
> #include "lib/util/fault.h"
> #include "lib/util/memory.h"
>+#include "libcli/util/ntstatus.h"
>+#include "lib/krb5_wrap/krb5_samba.h"
> 
> #include <profile.h>
> #include <kdb.h>
>@@ -147,7 +149,7 @@ krb5_error_code kdb_samba_db_check_policy_as(krb5_context context,
> 		/* make sure the mapped return code is returned - gd */
> 		int code_tmp;
> 
>-		d = ks_make_data(int_data.data, int_data.length);
>+		d = smb_krb5_data_from_blob(int_data);
> 
> 		code_tmp = decode_krb5_padata_sequence(&d, &e_data);
> 		if (code_tmp == 0) {
>-- 
>2.34.1
>
>
>From efe98015baa722412c64d8b5c3d4f41230308deb Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Fri, 17 Mar 2023 09:25:52 +1300
>Subject: [PATCH 2/3] s4:kdc: Don't pass a NULL pointer into
> krb5_pac_add_buffer()
>
>Heimdal contains an assertion that the data pointer is not NULL. We need
>to pass in a pointer to some dummy data instead.
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 47ef49fd91f050ce4a79a8471b3e66c808f48752)
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15476
>---
> source4/kdc/pac-glue.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c
>index 6692619065bc..2b9553588b3e 100644
>--- a/source4/kdc/pac-glue.c
>+++ b/source4/kdc/pac-glue.c
>@@ -1827,6 +1827,9 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
> 		DATA_BLOB type_blob = data_blob_null;
> 		uint32_t type;
> 
>+		static char null_byte = '\0';
>+		const krb5_data null_data = smb_krb5_make_data(&null_byte, 0);
>+
> 		if (forced_next_type != 0) {
> 			/*
> 			 * We need to inject possible missing types
>@@ -1986,10 +1989,14 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
> 			}
> 		}
> 
>+		/*
>+		 * Passing a NULL pointer into krb5_pac_add_buffer() is
>+		 * not allowed, so pass null_data instead if needed.
>+		 */
> 		code = krb5_pac_add_buffer(context,
> 					   new_pac,
> 					   type,
>-					   &type_data);
>+					   (type_data.data != NULL) ? &type_data : &null_data);
> 		smb_krb5_free_data_contents(context, &type_data);
> 		if (code != 0) {
> 			goto done;
>-- 
>2.34.1
>
>
>From 8dcee0267985caadd77ea35b76702b2a501c31fe Mon Sep 17 00:00:00 2001
>From: Joseph Sutton <josephsutton@catalyst.net.nz>
>Date: Fri, 17 Mar 2023 09:16:17 +1300
>Subject: [PATCH 3/3] s4:kdc: Avoid copying data if not needed
>
>krb5_pac_add_buffer() makes its own copy of the data we pass in. We
>don't need to make yet another copy.
>
>Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit fa901e7346d36ae64a7ceab5dcf76bc210a67c93)
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15476
>---
> source4/kdc/pac-glue.c | 27 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
>diff --git a/source4/kdc/pac-glue.c b/source4/kdc/pac-glue.c
>index 2b9553588b3e..00c803fbe271 100644
>--- a/source4/kdc/pac-glue.c
>+++ b/source4/kdc/pac-glue.c
>@@ -1973,12 +1973,9 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
> 		}
> 
> 		if (type_blob.length != 0) {
>-			code = smb_krb5_copy_data_contents(&type_data,
>-							   type_blob.data,
>-							   type_blob.length);
>-			if (code != 0) {
>-				goto done;
>-			}
>+			type_data = smb_krb5_data_from_blob(type_blob);
>+			code = krb5_pac_add_buffer(context, new_pac,
>+						   type, &type_data);
> 		} else {
> 			code = krb5_pac_get_buffer(context,
> 						   old_pac,
>@@ -1987,17 +1984,17 @@ krb5_error_code samba_kdc_update_pac(TALLOC_CTX *mem_ctx,
> 			if (code != 0) {
> 				goto done;
> 			}
>+			/*
>+			 * Passing a NULL pointer into krb5_pac_add_buffer() is
>+			 * not allowed, so pass null_data instead if needed.
>+			 */
>+			code = krb5_pac_add_buffer(context,
>+						   new_pac,
>+						   type,
>+						   (type_data.data != NULL) ? &type_data : &null_data);
>+			smb_krb5_free_data_contents(context, &type_data);
> 		}
> 
>-		/*
>-		 * Passing a NULL pointer into krb5_pac_add_buffer() is
>-		 * not allowed, so pass null_data instead if needed.
>-		 */
>-		code = krb5_pac_add_buffer(context,
>-					   new_pac,
>-					   type,
>-					   (type_data.data != NULL) ? &type_data : &null_data);
>-		smb_krb5_free_data_contents(context, &type_data);
> 		if (code != 0) {
> 			goto done;
> 		}
>-- 
>2.34.1
>

Flags:

metze: review? (abartlet)
jsutton: review+

Actions: View

Attachments on bug 15476: 18108 | 18109 | 18110 | 18111