[PATCH] Introduce dom_sid_str_buf

Volker Lendecke Volker.Lendecke at SerNet.DE
Sat Oct 27 20:08:44 UTC 2018


Hi!

Attached find a patch that makes our sid2string conversion a little
safer than before. Eventually I'd like to move all of our
sid_string_tos and dom_sid_string routines into dom_sid_str_buf
wherever the use pattern allows it. That will be a lot of small
patches that can easily be squashed if it spoils commit statistics too
much.

Review appreciated!

Thanks, Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 2923b54ce04c2e43d3fed256c0fdfbc68468ea1e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 Oct 2018 05:46:37 +0200
Subject: [PATCH 01/19] lib: Add dom_sid_str_buf

This is modeled after server_id_str_buf, which as an API to me is easier to
use: I can rely on the compiler to get the buffer size right.

It is designed to violate README.Coding's "Make use of helper variables", but
as this API is simple enough and the output should never be a surprise at all,
I think that's worth it.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 libcli/security/dom_sid.c | 10 ++++++++++
 libcli/security/dom_sid.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
index b876fc8b780..eac648e2f60 100644
--- a/libcli/security/dom_sid.c
+++ b/libcli/security/dom_sid.c
@@ -491,3 +491,13 @@ char *dom_sid_string(TALLOC_CTX *mem_ctx, const struct dom_sid *sid)
 	talloc_set_name_const(result, result);
 	return result;
 }
+
+char *dom_sid_str_buf(const struct dom_sid *sid, struct dom_sid_buf *dst)
+{
+	int ret;
+	ret = dom_sid_string_buf(sid, dst->buf, sizeof(dst->buf));
+	if (ret < 0) {
+		strlcpy(dst->buf, "(INVALID SID)", sizeof(dst->buf));
+	}
+	return dst->buf;
+}
diff --git a/libcli/security/dom_sid.h b/libcli/security/dom_sid.h
index d9f4b3fc8a6..d132628da9f 100644
--- a/libcli/security/dom_sid.h
+++ b/libcli/security/dom_sid.h
@@ -102,6 +102,8 @@ bool dom_sid_is_valid_account_domain(const struct dom_sid *sid);
 int dom_sid_string_buf(const struct dom_sid *sid, char *buf, int buflen);
 char *dom_sid_string(TALLOC_CTX *mem_ctx, const struct dom_sid *sid);
 
+struct dom_sid_buf { char buf[DOM_SID_STR_BUFLEN]; };
+char *dom_sid_str_buf(const struct dom_sid *sid, struct dom_sid_buf *dst);
 
 const char *sid_type_lookup(uint32_t sid_type);
 const struct security_token *get_system_token(void);
-- 
2.11.0


From 23d9020ab9a3bca847f35c897f77050874f0cad8 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 Oct 2018 05:55:04 +0200
Subject: [PATCH 02/19] nbt_server: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/nbt_server/dgram/netlogon.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/source4/nbt_server/dgram/netlogon.c b/source4/nbt_server/dgram/netlogon.c
index 1fee1d8cc1e..e2ee1000d7b 100644
--- a/source4/nbt_server/dgram/netlogon.c
+++ b/source4/nbt_server/dgram/netlogon.c
@@ -163,12 +163,14 @@ static NTSTATUS nbtd_netlogon_samlogon(
 		netlogon->req.logon.nt_version, nbtsrv->task->lp_ctx,
 		&response->data.samlogon, false);
 	if (!NT_STATUS_IS_OK(status)) {
-		char buf[DOM_SID_STR_BUFLEN];
-		dom_sid_string_buf(sid, buf, sizeof(buf));
+		struct dom_sid_buf buf;
 
 		DBG_NOTICE("NBT netlogon query failed domain=%s sid=%s "
-			   "version=%d - %s\n", dst_name->name, buf,
-			   netlogon->req.logon.nt_version, nt_errstr(status));
+			   "version=%d - %s\n",
+			   dst_name->name,
+			   dom_sid_str_buf(sid, &buf),
+			   netlogon->req.logon.nt_version,
+			   nt_errstr(status));
 		TALLOC_FREE(reply_mailslot);
 		TALLOC_FREE(response);
 		return status;
-- 
2.11.0


From b2f294b710fce8213172fe06d75f31fec8d67685 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 Oct 2018 05:55:24 +0200
Subject: [PATCH 03/19] rpc_server4: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/rpc_server/samr/dcesrv_samr.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
index eccf9d2b8c0..3df0c51dfee 100644
--- a/source4/rpc_server/samr/dcesrv_samr.c
+++ b/source4/rpc_server/samr/dcesrv_samr.c
@@ -1532,7 +1532,6 @@ static NTSTATUS dcesrv_samr_GetAliasMembership(struct dcesrv_call_state *dce_cal
 	struct ldb_message **res;
 	uint32_t i;
 	int count = 0;
-	char membersidstr[DOM_SID_STR_BUFLEN];
 
 	DCESRV_PULL_HANDLE(h, r->in.domain_handle, SAMR_HANDLE_DOMAIN);
 
@@ -1548,11 +1547,13 @@ static NTSTATUS dcesrv_samr_GetAliasMembership(struct dcesrv_call_state *dce_cal
 	}
 
 	for (i=0; i<r->in.sids->num_sids; i++) {
-		dom_sid_string_buf(r->in.sids->sids[i].sid,
-				   membersidstr, sizeof(membersidstr));
+		struct dom_sid_buf buf;
+
+		filter = talloc_asprintf_append(
+			filter,
+			"(member=<SID=%s>)",
+			dom_sid_str_buf(r->in.sids->sids[i].sid, &buf));
 
-		filter = talloc_asprintf_append(filter, "(member=<SID=%s>)",
-						membersidstr);
 		if (filter == NULL) {
 			return NT_STATUS_NO_MEMORY;
 		}
-- 
2.11.0


From 94773ebfc7b5db0aaa5a221e0037ed369a887602 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 Oct 2018 05:57:26 +0200
Subject: [PATCH 04/19] dsdb: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/dsdb/samdb/ldb_modules/operational.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c
index cdcec1c1046..8dad9517ced 100644
--- a/source4/dsdb/samdb/ldb_modules/operational.c
+++ b/source4/dsdb/samdb/ldb_modules/operational.c
@@ -1071,13 +1071,12 @@ static int pso_search_by_sids(struct ldb_module *module, TALLOC_CTX *mem_ctx,
 	sid_filter = talloc_strdup(mem_ctx, "");
 
 	for (i = 0; sid_filter && i < num_sids; i++) {
-		char sid_buf[DOM_SID_STR_BUFLEN] = {0,};
+		struct dom_sid_buf sid_buf;
 
-		dom_sid_string_buf(&sid_array[i], sid_buf, sizeof(sid_buf));
-
-		sid_filter = talloc_asprintf_append(sid_filter,
-						    "(msDS-PSOAppliesTo=<SID=%s>)",
-						    sid_buf);
+		sid_filter = talloc_asprintf_append(
+			sid_filter,
+			"(msDS-PSOAppliesTo=<SID=%s>)",
+			dom_sid_str_buf(&sid_array[i], &sid_buf));
 	}
 
 	if (sid_filter == NULL) {
-- 
2.11.0


From 7e0167a12204bced00b2275bad276b455f38d839 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 Oct 2018 06:08:19 +0200
Subject: [PATCH 05/19] auth: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 auth/auth_log.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/auth/auth_log.c b/auth/auth_log.c
index 9e57b1d3968..96c396357c4 100644
--- a/auth/auth_log.c
+++ b/auth/auth_log.c
@@ -578,14 +578,13 @@ static void log_authentication_event_human_readable(
 	local = tsocket_address_string(ui->local_host, frame);
 
 	if (NT_STATUS_IS_OK(status)) {
-		char sid_buf[DOM_SID_STR_BUFLEN];
+		struct dom_sid_buf sid_buf;
 
-		dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
 		logon_line = talloc_asprintf(frame,
 					     " became [%s]\\[%s] [%s].",
 					     log_escape(frame, domain_name),
 					     log_escape(frame, account_name),
-					     sid_buf);
+					     dom_sid_str_buf(sid, &sid_buf));
 	} else {
 		logon_line = talloc_asprintf(
 				frame,
-- 
2.11.0


From 911c6f25f68173b0e1789507d997cb5689749995 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 Oct 2018 06:08:32 +0200
Subject: [PATCH 06/19] idmap: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/winbindd/idmap_script.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/source3/winbindd/idmap_script.c b/source3/winbindd/idmap_script.c
index 7b7f8844c36..e4de1a09ba0 100644
--- a/source3/winbindd/idmap_script.c
+++ b/source3/winbindd/idmap_script.c
@@ -333,7 +333,7 @@ static struct tevent_req *idmap_script_sid2xid_send(
 {
 	struct tevent_req *req, *subreq;
 	struct idmap_script_sid2xid_state *state;
-	char sidbuf[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf sidbuf;
 
 	req = tevent_req_create(mem_ctx, &state,
 				struct idmap_script_sid2xid_state);
@@ -342,10 +342,11 @@ static struct tevent_req *idmap_script_sid2xid_send(
 	}
 	state->idx = idx;
 
-	dom_sid_string_buf(sid, sidbuf, sizeof(sidbuf));
-
-	state->syscmd = talloc_asprintf(state, "%s SIDTOID %s",
-					script, sidbuf);
+	state->syscmd = talloc_asprintf(
+		state,
+		"%s SIDTOID %s",
+		script,
+		dom_sid_str_buf(sid, &sidbuf));
 	if (tevent_req_nomem(state->syscmd, req)) {
 		return tevent_req_post(req, ev);
 	}
-- 
2.11.0


From 1ea31dfeb162ee08e676dece58788548ef22c20c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 18 Oct 2018 06:18:22 +0200
Subject: [PATCH 07/19] net: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/utils/net_cache.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/source3/utils/net_cache.c b/source3/utils/net_cache.c
index 2d6989918f5..4416630b871 100644
--- a/source3/utils/net_cache.c
+++ b/source3/utils/net_cache.c
@@ -463,9 +463,10 @@ static int net_cache_samlogon_show(struct net_context *c,
 	}
 
 	for (i = 0; i < num_user_sids; i++) {
-		char buf[DOM_SID_STR_BUFLEN];
-		dom_sid_string_buf(&user_sids[i], buf, sizeof(buf));
-		d_printf("SID %2" PRIu32 ": %s\n", i, buf);
+		struct dom_sid_buf buf;
+		d_printf("SID %2" PRIu32 ": %s\n",
+			 i,
+			 dom_sid_str_buf(&user_sids[i], &buf));
 	}
 
 	TALLOC_FREE(user_sids);
-- 
2.11.0


From 60e38bb368ade0fbead0c5fcae74dfdc609433cd Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 25 Oct 2018 21:26:38 +0200
Subject: [PATCH 08/19] audit_tests: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 .../dsdb/samdb/ldb_modules/tests/test_audit_util.c   | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/source4/dsdb/samdb/ldb_modules/tests/test_audit_util.c b/source4/dsdb/samdb/ldb_modules/tests/test_audit_util.c
index c2a27395b4d..cf2546d0b7a 100644
--- a/source4/dsdb/samdb/ldb_modules/tests/test_audit_util.c
+++ b/source4/dsdb/samdb/ldb_modules/tests/test_audit_util.c
@@ -538,7 +538,7 @@ static void test_dsdb_audit_get_user_sid(void **state)
 	struct dom_sid sids[2];
 	const char * const SID0 = "S-1-5-21-2470180966-3899876309-2637894779";
 	const char * const SID1 = "S-1-5-21-4284042908-2889457889-3672286761";
-	char sid_buf[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf sid_buf;
 
 
 	TALLOC_CTX *ctx = talloc_new(NULL);
@@ -585,8 +585,8 @@ static void test_dsdb_audit_get_user_sid(void **state)
 	token->sids = sids;
 	sid = dsdb_audit_get_user_sid(module);
 	assert_non_null(sid);
-	dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
-	assert_string_equal(SID0, sid_buf);
+	dom_sid_str_buf(sid, &sid_buf);
+	assert_string_equal(SID0, sid_buf.buf);
 
 	/*
 	 * Add a second SID, should still use the first SID
@@ -595,8 +595,8 @@ static void test_dsdb_audit_get_user_sid(void **state)
 	token->num_sids = 2;
 	sid = dsdb_audit_get_user_sid(module);
 	assert_non_null(sid);
-	dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
-	assert_string_equal(SID0, sid_buf);
+	dom_sid_str_buf(sid, &sid_buf);
+	assert_string_equal(SID0, sid_buf.buf);
 
 
 	/*
@@ -619,7 +619,7 @@ static void test_dsdb_audit_get_actual_sid(void **state)
 	struct dom_sid sids[2];
 	const char * const SID0 = "S-1-5-21-2470180966-3899876309-2637894779";
 	const char * const SID1 = "S-1-5-21-4284042908-2889457889-3672286761";
-	char sid_buf[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf sid_buf;
 
 
 	TALLOC_CTX *ctx = talloc_new(NULL);
@@ -664,8 +664,8 @@ static void test_dsdb_audit_get_actual_sid(void **state)
 	token->sids = sids;
 	sid = dsdb_audit_get_actual_sid(ldb);
 	assert_non_null(sid);
-	dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
-	assert_string_equal(SID0, sid_buf);
+	dom_sid_str_buf(sid, &sid_buf);
+	assert_string_equal(SID0, sid_buf.buf);
 
 	/*
 	 * Add a second SID, should still use the first SID
@@ -674,8 +674,8 @@ static void test_dsdb_audit_get_actual_sid(void **state)
 	token->num_sids = 2;
 	sid = dsdb_audit_get_actual_sid(ldb);
 	assert_non_null(sid);
-	dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
-	assert_string_equal(SID0, sid_buf);
+	dom_sid_str_buf(sid, &sid_buf);
+	assert_string_equal(SID0, sid_buf.buf);
 
 
 	/*
-- 
2.11.0


From a9c7ebaaa68087246fbb3307e68d86c783ca67d8 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Thu, 25 Oct 2018 21:45:05 +0200
Subject: [PATCH 09/19] auth4: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source4/auth/sam.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/source4/auth/sam.c b/source4/auth/sam.c
index 07cfbd06b33..0e47fae43d6 100644
--- a/source4/auth/sam.c
+++ b/source4/auth/sam.c
@@ -632,16 +632,14 @@ _PUBLIC_ NTSTATUS authsam_update_user_info_dc(TALLOC_CTX *mem_ctx,
 	n = user_info_dc->num_sids;
 	for (i = 0; i < n; i++) {
 		struct dom_sid *sid = &user_info_dc->sids[i];
-		char sid_buf[DOM_SID_STR_BUFLEN] = {0,};
-		char dn_str[DOM_SID_STR_BUFLEN*2] = {0,};
+		struct dom_sid_buf sid_buf;
+		char dn_str[sizeof(sid_buf.buf)*2];
 		DATA_BLOB dn_blob = data_blob_null;
-		int len;
 
-		len = dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
-		if (len+1 > sizeof(sid_buf)) {
-			return NT_STATUS_INVALID_SID;
-		}
-		snprintf(dn_str, sizeof(dn_str), "<SID=%s>", sid_buf);
+		snprintf(dn_str,
+			 sizeof(dn_str),
+			 "<SID=%s>",
+			 dom_sid_str_buf(sid, &sid_buf));
 		dn_blob = data_blob_string_const(dn_str);
 
 		/*
-- 
2.11.0


From 7d9382e38da4746b819ff49cda5457359928a779 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 10/19] winbindd: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/winbindd/winbindd_xids_to_sids.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/source3/winbindd/winbindd_xids_to_sids.c b/source3/winbindd/winbindd_xids_to_sids.c
index d1424cdb061..29caccd0468 100644
--- a/source3/winbindd/winbindd_xids_to_sids.c
+++ b/source3/winbindd/winbindd_xids_to_sids.c
@@ -113,17 +113,16 @@ NTSTATUS winbindd_xids_to_sids_recv(struct tevent_req *req,
 	}
 
 	for (i=0; i<state->num_xids; i++) {
-		char sidbuf[DOM_SID_STR_BUFLEN];
+		struct dom_sid_buf sid_buf;
+		const char *str = "-";
 
-		if (is_null_sid(&state->sids[i])) {
-			strlcpy(sidbuf, "-", sizeof(sidbuf));
-		} else {
-			dom_sid_string_buf(&state->sids[i],
-					   sidbuf, sizeof(sidbuf));
+		if (!is_null_sid(&state->sids[i])) {
+			dom_sid_str_buf(&state->sids[i], &sid_buf);
+			str = sid_buf.buf;
 		}
 
 		result = talloc_asprintf_append_buffer(
-			result, "%s\n", sidbuf);
+			result, "%s\n", str);
 		if (result == NULL) {
 			return NT_STATUS_NO_MEMORY;
 		}
-- 
2.11.0


From ba85d4a22b4be8727fc0f6e7b5989de9c764b7b4 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 11/19] winbindd: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/winbindd/wb_lookupusergroups.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/winbindd/wb_lookupusergroups.c b/source3/winbindd/wb_lookupusergroups.c
index 7647fbd5e66..bc14c33a88d 100644
--- a/source3/winbindd/wb_lookupusergroups.c
+++ b/source3/winbindd/wb_lookupusergroups.c
@@ -57,9 +57,9 @@ struct tevent_req *wb_lookupusergroups_send(TALLOC_CTX *mem_ctx,
 
 	domain = find_domain_from_sid_noinit(&state->sid);
 	if (domain == NULL) {
-		char buf[DOM_SID_STR_BUFLEN];
-		dom_sid_string_buf(&state->sid, buf, sizeof(buf));
-		DEBUG(1,("could not find domain entry for sid %s\n", buf));
+		struct dom_sid_buf buf;
+		DBG_WARNING("could not find domain entry for sid %s\n",
+			    dom_sid_str_buf(&state->sid, &buf));
 		tevent_req_nterror(req, NT_STATUS_NO_SUCH_DOMAIN);
 		return tevent_req_post(req, ev);
 	}
-- 
2.11.0


From dd4c316b875737eaf76d217aa4678ab2ebb472c6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 12/19] smbd: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/smbd/nttrans.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index 68470766f77..3c9b0ebb3f5 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -2336,9 +2336,9 @@ static enum ndr_err_code fill_qtlist_from_sids(TALLOC_CTX *mem_ctx,
 
 		ok = sid_to_uid(&sids[i], &list_item->uid);
 		if (!ok) {
-			char buf[DOM_SID_STR_BUFLEN];
-			dom_sid_string_buf(&sids[i], buf, sizeof(buf));
-			DBG_WARNING("Could not convert SID %s to uid\n", buf);
+			struct dom_sid_buf buf;
+			DBG_WARNING("Could not convert SID %s to uid\n",
+				    dom_sid_str_buf(&sids[i], &buf));
 			/* No idea what to return here... */
 			return NDR_ERR_INVALID_POINTER;
 		}
-- 
2.11.0


From f5ca0c5246e4343946746c8cc7a0a2b98e437821 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 13/19] libsmb: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/libsmb/samlogon_cache.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/source3/libsmb/samlogon_cache.c b/source3/libsmb/samlogon_cache.c
index 9638df646f0..494eb10a6a8 100644
--- a/source3/libsmb/samlogon_cache.c
+++ b/source3/libsmb/samlogon_cache.c
@@ -98,7 +98,7 @@ clear:
 
 void netsamlogon_clear_cached_user(const struct dom_sid *user_sid)
 {
-	char keystr[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf keystr;
 
 	if (!netsamlogon_cache_init()) {
 		DEBUG(0,("netsamlogon_clear_cached_user: cannot open "
@@ -108,11 +108,11 @@ void netsamlogon_clear_cached_user(const struct dom_sid *user_sid)
 	}
 
 	/* Prepare key as DOMAIN-SID/USER-RID string */
-	dom_sid_string_buf(user_sid, keystr, sizeof(keystr));
+	dom_sid_str_buf(user_sid, &keystr);
 
-	DEBUG(10,("netsamlogon_clear_cached_user: SID [%s]\n", keystr));
+	DBG_DEBUG("SID [%s]\n", keystr.buf);
 
-	tdb_delete_bystring(netsamlogon_tdb, keystr);
+	tdb_delete_bystring(netsamlogon_tdb, keystr.buf);
 }
 
 /***********************************************************************
@@ -124,7 +124,7 @@ bool netsamlogon_cache_store(const char *username, struct netr_SamInfo3 *info3)
 {
 	uint8_t dummy = 0;
 	TDB_DATA data = { .dptr = &dummy, .dsize = sizeof(dummy) };
-	char keystr[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf keystr;
 	bool result = false;
 	struct dom_sid	user_sid;
 	TALLOC_CTX *tmp_ctx = talloc_stackframe();
@@ -149,22 +149,22 @@ bool netsamlogon_cache_store(const char *username, struct netr_SamInfo3 *info3)
 	 * overwriting potentially other data. We're just interested
 	 * in the existence of that record.
 	 */
-	dom_sid_string_buf(info3->base.domain_sid, keystr, sizeof(keystr));
+	dom_sid_str_buf(info3->base.domain_sid, &keystr);
 
-	ret = tdb_store_bystring(netsamlogon_tdb, keystr, data, TDB_INSERT);
+	ret = tdb_store_bystring(netsamlogon_tdb, keystr.buf, data, TDB_INSERT);
 
 	if ((ret == -1) && (tdb_error(netsamlogon_tdb) != TDB_ERR_EXISTS)) {
 		DBG_WARNING("Could not store domain marker for %s: %s\n",
-			    keystr, tdb_errorstr(netsamlogon_tdb));
+			    keystr.buf, tdb_errorstr(netsamlogon_tdb));
 		goto fail;
 	}
 
 	sid_compose(&user_sid, info3->base.domain_sid, info3->base.rid);
 
 	/* Prepare key as DOMAIN-SID/USER-RID string */
-	dom_sid_string_buf(&user_sid, keystr, sizeof(keystr));
+	dom_sid_str_buf(&user_sid, &keystr);
 
-	DEBUG(10,("netsamlogon_cache_store: SID [%s]\n", keystr));
+	DBG_DEBUG("SID [%s]\n", keystr.buf);
 
 	/* Prepare data */
 
@@ -217,7 +217,7 @@ bool netsamlogon_cache_store(const char *username, struct netr_SamInfo3 *info3)
 	data.dsize = blob.length;
 	data.dptr = blob.data;
 
-	if (tdb_store_bystring(netsamlogon_tdb, keystr, data, TDB_REPLACE) == 0) {
+	if (tdb_store_bystring(netsamlogon_tdb, keystr.buf, data, TDB_REPLACE) == 0) {
 		result = true;
 	}
 
@@ -235,7 +235,7 @@ struct netr_SamInfo3 *netsamlogon_cache_get(TALLOC_CTX *mem_ctx, const struct do
 {
 	struct netr_SamInfo3 *info3 = NULL;
 	TDB_DATA data;
-	char keystr[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf keystr;
 	enum ndr_err_code ndr_err;
 	DATA_BLOB blob;
 	struct netsamlogoncache_entry r;
@@ -247,9 +247,9 @@ struct netr_SamInfo3 *netsamlogon_cache_get(TALLOC_CTX *mem_ctx, const struct do
 	}
 
 	/* Prepare key as DOMAIN-SID/USER-RID string */
-	dom_sid_string_buf(user_sid, keystr, sizeof(keystr));
-	DEBUG(10,("netsamlogon_cache_get: SID [%s]\n", keystr));
-	data = tdb_fetch_bystring( netsamlogon_tdb, keystr );
+	dom_sid_str_buf(user_sid, &keystr);
+	DBG_DEBUG("SID [%s]\n", keystr.buf);
+	data = tdb_fetch_bystring( netsamlogon_tdb, keystr.buf );
 
 	if (!data.dptr) {
 		return NULL;
@@ -268,7 +268,7 @@ struct netr_SamInfo3 *netsamlogon_cache_get(TALLOC_CTX *mem_ctx, const struct do
 
 	if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
 		DEBUG(0,("netsamlogon_cache_get: failed to pull entry from cache\n"));
-		tdb_delete_bystring(netsamlogon_tdb, keystr);
+		tdb_delete_bystring(netsamlogon_tdb, keystr.buf);
 		TALLOC_FREE(info3);
 		goto done;
 	}
@@ -288,7 +288,7 @@ struct netr_SamInfo3 *netsamlogon_cache_get(TALLOC_CTX *mem_ctx, const struct do
 
 bool netsamlogon_cache_have(const struct dom_sid *sid)
 {
-	char keystr[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf keystr;
 	bool ok;
 
 	if (!netsamlogon_cache_init()) {
@@ -296,9 +296,9 @@ bool netsamlogon_cache_have(const struct dom_sid *sid)
 		return false;
 	}
 
-	dom_sid_string_buf(sid, keystr, sizeof(keystr));
+	dom_sid_str_buf(sid, &keystr);
 
-	ok = tdb_exists(netsamlogon_tdb, string_term_tdb_data(keystr));
+	ok = tdb_exists(netsamlogon_tdb, string_term_tdb_data(keystr.buf));
 	return ok;
 }
 
-- 
2.11.0


From c271e7cc7a4a39c91da1dc2ca4bcd11f9eadf7e6 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 14/19] lib: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/namemap_cache.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/source3/lib/namemap_cache.c b/source3/lib/namemap_cache.c
index c153aa608e2..fa179517f9f 100644
--- a/source3/lib/namemap_cache.c
+++ b/source3/lib/namemap_cache.c
@@ -31,8 +31,8 @@ bool namemap_cache_set_sid2name(const struct dom_sid *sid,
 				enum lsa_SidType type, time_t timeout)
 {
 	char typebuf[16];
-	char sidbuf[DOM_SID_STR_BUFLEN];
-	char keybuf[DOM_SID_STR_BUFLEN+10];
+	struct dom_sid_buf sidbuf;
+	char keybuf[sizeof(sidbuf.buf)+10];
 	char *val = NULL;
 	DATA_BLOB data;
 	int ret;
@@ -70,8 +70,8 @@ bool namemap_cache_set_sid2name(const struct dom_sid *sid,
 		goto fail;
 	}
 
-	dom_sid_string_buf(sid, sidbuf, sizeof(sidbuf));
-	snprintf(keybuf, sizeof(keybuf), "SID2NAME/%s", sidbuf);
+	dom_sid_str_buf(sid, &sidbuf);
+	snprintf(keybuf, sizeof(keybuf), "SID2NAME/%s", sidbuf.buf);
 
 	data = data_blob_const(val, talloc_get_size(val));
 
@@ -151,12 +151,12 @@ bool namemap_cache_find_sid(const struct dom_sid *sid,
 	struct namemap_cache_find_sid_state state = {
 		.fn = fn, .private_data = private_data
 	};
-	char sidbuf[DOM_SID_STR_BUFLEN];
-	char keybuf[DOM_SID_STR_BUFLEN+10];
+	struct dom_sid_buf sidbuf;
+	char keybuf[sizeof(sidbuf.buf)+10];
 	bool ok;
 
-	dom_sid_string_buf(sid, sidbuf, sizeof(sidbuf));
-	snprintf(keybuf, sizeof(keybuf), "SID2NAME/%s", sidbuf);
+	dom_sid_str_buf(sid, &sidbuf);
+	snprintf(keybuf, sizeof(keybuf), "SID2NAME/%s", sidbuf.buf);
 
 	ok = gencache_parse(keybuf, namemap_cache_find_sid_parser, &state);
 	if (!ok) {
@@ -179,7 +179,7 @@ bool namemap_cache_set_name2sid(const char *domain, const char *name,
 				time_t timeout)
 {
 	char typebuf[16];
-	char sidbuf[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf sidbuf = {{0}};
 	char *key;
 	char *key_upper;
 	char *val = NULL;
@@ -193,10 +193,8 @@ bool namemap_cache_set_name2sid(const char *domain, const char *name,
 	if (name == NULL) {
 		name = "";
 	}
-	if (type == SID_NAME_UNKNOWN) {
-		sidbuf[0] = '\0';
-	} else {
-		dom_sid_string_buf(sid, sidbuf, sizeof(sidbuf));
+	if (type != SID_NAME_UNKNOWN) {
+		dom_sid_str_buf(sid, &sidbuf);
 	}
 
 	snprintf(typebuf, sizeof(typebuf), "%d", (int)type);
@@ -212,7 +210,7 @@ bool namemap_cache_set_name2sid(const char *domain, const char *name,
 		goto fail;
 	}
 
-	ret = strv_add(key, &val, sidbuf);
+	ret = strv_add(key, &val, sidbuf.buf);
 	if (ret != 0) {
 		DBG_DEBUG("strv_add failed: %s\n", strerror(ret));
 		goto fail;
-- 
2.11.0


From 192723247bd9bf2f21f4bf8229136cd84d845be9 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 15/19] passdb: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/groupdb/mapping_tdb.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/source3/groupdb/mapping_tdb.c b/source3/groupdb/mapping_tdb.c
index 3561057214c..b5b11767f27 100644
--- a/source3/groupdb/mapping_tdb.c
+++ b/source3/groupdb/mapping_tdb.c
@@ -133,15 +133,10 @@ static bool init_group_mapping(void)
 
 static char *group_mapping_key(TALLOC_CTX *mem_ctx, const struct dom_sid *sid)
 {
-	char sidstr[DOM_SID_STR_BUFLEN];
-	int len;
-
-	len = dom_sid_string_buf(sid, sidstr, sizeof(sidstr));
-	if (len >= sizeof(sidstr)) {
-		return NULL;
-	}
+	struct dom_sid_buf sidstr;
 
-	return talloc_asprintf(mem_ctx, "%s%s", GROUP_PREFIX, sidstr);
+	return talloc_asprintf(
+		mem_ctx, "%s%s", GROUP_PREFIX, dom_sid_str_buf(sid, &sidstr));
 }
 
 /****************************************************************************
-- 
2.11.0


From bca847fac0243f3a0734a03786f8848a462f76f0 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 16/19] audit_logging: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 lib/audit_logging/audit_logging.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/audit_logging/audit_logging.c b/lib/audit_logging/audit_logging.c
index c274e971925..acccb9f7ab9 100644
--- a/lib/audit_logging/audit_logging.c
+++ b/lib/audit_logging/audit_logging.c
@@ -826,14 +826,14 @@ int json_add_sid(struct json_object *object,
 			return ret;
 		}
 	} else {
-		char sid_buf[DOM_SID_STR_BUFLEN];
+		struct dom_sid_buf sid_buf;
 
-		dom_sid_string_buf(sid, sid_buf, sizeof(sid_buf));
-		ret = json_add_string(object, name, sid_buf);
+		ret = json_add_string(
+			object, name, dom_sid_str_buf(sid, &sid_buf));
 		if (ret != 0) {
 			DBG_ERR("Unable to add SID [%s] value [%s]\n",
 				name,
-				sid_buf);
+				sid_buf.buf);
 			return ret;
 		}
 	}
-- 
2.11.0


From f4d3c48f4949deef15d8f0b32e1ed062198d9e2c Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 17/19] auth: Use dom_sid_str_buf

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 auth/auth_log.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/auth/auth_log.c b/auth/auth_log.c
index 96c396357c4..2d4e33f53bf 100644
--- a/auth/auth_log.c
+++ b/auth/auth_log.c
@@ -689,7 +689,7 @@ static void log_successful_authz_event_human_readable(
 	const char *ts = NULL;       /* formatted current time      */
 	char *remote_str = NULL;     /* formatted remote host       */
 	char *local_str = NULL;      /* formatted local host        */
-	char sid_buf[DOM_SID_STR_BUFLEN];
+	struct dom_sid_buf sid_buf;
 
 	frame = talloc_stackframe();
 
@@ -699,10 +699,6 @@ static void log_successful_authz_event_human_readable(
 	remote_str = tsocket_address_string(remote, frame);
 	local_str = tsocket_address_string(local, frame);
 
-	dom_sid_string_buf(&session_info->security_token->sids[0],
-			   sid_buf,
-			   sizeof(sid_buf));
-
 	DEBUGC(DBGC_AUTH_AUDIT, debug_level,
 	       ("Successful AuthZ: [%s,%s] user [%s]\\[%s] [%s]"
 		" at [%s]"
@@ -712,7 +708,8 @@ static void log_successful_authz_event_human_readable(
 		auth_type,
 		log_escape(frame, session_info->info->domain_name),
 		log_escape(frame, session_info->info->account_name),
-		sid_buf,
+		dom_sid_str_buf(&session_info->security_token->sids[0],
+				&sid_buf),
 		ts,
 		remote_str,
 		local_str));
-- 
2.11.0


From f9691664df0ac4ed1b283900f85204ec81574ccf Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 08:25:14 +0200
Subject: [PATCH 18/19] lib: Use dom_sid_str_buf

This is the one place where we have to do another strpcy. Many of the
sid_to_fstring calls should be replacable by dom_sid_str_buf, so this
will get less.

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 source3/lib/util_sid.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/lib/util_sid.c b/source3/lib/util_sid.c
index 4d57a92b853..13d5e1ce4c4 100644
--- a/source3/lib/util_sid.c
+++ b/source3/lib/util_sid.c
@@ -34,7 +34,8 @@
 
 char *sid_to_fstring(fstring sidstr_out, const struct dom_sid *sid)
 {
-	dom_sid_string_buf(sid, sidstr_out, sizeof(fstring));
+	struct dom_sid_buf buf;
+	fstrcpy(sidstr_out, dom_sid_str_buf(sid, &buf));
 	return sidstr_out;
 }
 
-- 
2.11.0


From dde4c653103fbb567c55be75933a4c09f828dde0 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Fri, 26 Oct 2018 14:09:32 +0200
Subject: [PATCH 19/19] lib: Make dom_sid_string_buf static

Signed-off-by: Volker Lendecke <vl at samba.org>
---
 libcli/security/dom_sid.c | 2 +-
 libcli/security/dom_sid.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
index eac648e2f60..757c0254384 100644
--- a/libcli/security/dom_sid.c
+++ b/libcli/security/dom_sid.c
@@ -429,7 +429,7 @@ bool dom_sid_is_valid_account_domain(const struct dom_sid *sid)
   string length. If it overflows, return the string length that would
   result (buflen needs to be +1 for the terminating 0).
 */
-int dom_sid_string_buf(const struct dom_sid *sid, char *buf, int buflen)
+static int dom_sid_string_buf(const struct dom_sid *sid, char *buf, int buflen)
 {
 	int i, ofs;
 	uint64_t ia;
diff --git a/libcli/security/dom_sid.h b/libcli/security/dom_sid.h
index d132628da9f..1effdbc2f6c 100644
--- a/libcli/security/dom_sid.h
+++ b/libcli/security/dom_sid.h
@@ -99,7 +99,6 @@ bool dom_sid_in_domain(const struct dom_sid *domain_sid,
 bool dom_sid_is_valid_account_domain(const struct dom_sid *sid);
 
 #define DOM_SID_STR_BUFLEN (15*11+25)
-int dom_sid_string_buf(const struct dom_sid *sid, char *buf, int buflen);
 char *dom_sid_string(TALLOC_CTX *mem_ctx, const struct dom_sid *sid);
 
 struct dom_sid_buf { char buf[DOM_SID_STR_BUFLEN]; };
-- 
2.11.0



More information about the samba-technical mailing list