s3:tests: Adapt winbind_call_depth_trace to depth=3

Signed-off-by: Pavel Filipenský <pfilipensky@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

Autobuild-User(master): Pavel Filipensky <pfilipensky@samba.org>
Autobuild-Date(master): Thu Dec  5 15:54:57 UTC 2024 on atb-devel-224

libcli/security/dom_sid.c

@@ -39,20 +39,27 @@ int dom_sid_compare_auth(const struct dom_sid *sid1,
 {
 	int i;
 
-	if (sid1 == sid2)
+	if (sid1 == sid2) {
 		return 0;
-	if (!sid1)
+	}
+
+	if (sid1 == NULL) {
 		return -1;
-	if (!sid2)
+	}
+
+	if (sid2 == NULL) {
 		return 1;
+	}
 
-	if (sid1->sid_rev_num != sid2->sid_rev_num)
+	if (sid1->sid_rev_num != sid2->sid_rev_num) {
 		return NUMERIC_CMP(sid1->sid_rev_num, sid2->sid_rev_num);
+	}
 
-	for (i = 0; i < 6; i++)
+	for (i = 0; i < 6; i++) {
 		if (sid1->id_auth[i] != sid2->id_auth[i]) {
 			return NUMERIC_CMP(sid1->id_auth[i], sid2->id_auth[i]);
 		}
+	}
 
 	return 0;
 }
@@ -65,12 +72,17 @@ int dom_sid_compare(const struct dom_sid *sid1, const struct dom_sid *sid2)
 {
 	int i;
 
-	if (sid1 == sid2)
+	if (sid1 == sid2) {
 		return 0;
-	if (!sid1)
+	}
+
+	if (sid1 == NULL) {
 		return -1;
-	if (!sid2)
+	}
+
+	if (sid2 == NULL) {
 		return 1;
+	}
 
 	/* Compare most likely different rids, first: i.e start at end */
 	if (sid1->num_auths != sid2->num_auths) {

libcli/security/sddl.c

@@ -201,21 +201,21 @@ static const struct {
   decode a SID
   It can either be a special 2 letter code, or in S-* format
 */
-static struct dom_sid *sddl_transition_decode_sid(TALLOC_CTX *mem_ctx, const char **sddlp,
+static bool sddl_transition_decode_sid(const char **sddlp,
-						  struct sddl_transition_state *state)
+				       struct sddl_transition_state *state,
+				       struct dom_sid *sid)
 {
 	const char *sddl = (*sddlp);
 	size_t i;
 
 	/* see if its in the numeric format */
 	if (strncasecmp(sddl, "S-", 2) == 0) {
-		struct dom_sid *sid = NULL;
-		char *sid_str = NULL;
-		const char *end = NULL;
-		bool ok;
 		size_t len = strspn(sddl + 2, "-0123456789ABCDEFabcdefxX") + 2;
 		if (len < 5) { /* S-1-x */
-			return NULL;
+			return false;
+		}
+		if (len > DOM_SID_STR_BUFLEN) { /* Invalid SID */
+			return false;
 		}
 		if (sddl[len - 1] == 'D' && sddl[len] == ':') {
 			/*
@@ -226,38 +226,28 @@ static struct dom_sid *sddl_transition_decode_sid(TALLOC_CTX *mem_ctx, const cha
 			len--;
 		}
 
-		sid_str = talloc_strndup(mem_ctx, sddl, len);
+		{
-		if (sid_str == NULL) {
+			const char *end = NULL;
-			return NULL;
+			char sid_str[DOM_SID_STR_BUFLEN + 1];
+			bool ok;
+
+			memcpy(sid_str, sddl, len);
+			sid_str[len] = '\0';
+
+			ok = dom_sid_parse_endp(sid_str, sid, &end);
+			if (!ok) {
+				DBG_WARNING("could not parse SID '%s'\n",
+					    sid_str);
+				return false;
+			}
+			if (sid_str + len != end) {
+				DBG_WARNING("trailing junk after SID '%s'\n",
+					    sid_str);
+				return false;
+			}
 		}
-		if (sid_str[0] == 's') {
-			/*
-			 * In SDDL, but not in the dom_sid parsers, a
-			 * lowercase "s-1-1-0" is accepted.
-			 */
-			sid_str[0] = 'S';
-		}
-		sid = talloc(mem_ctx, struct dom_sid);
-		if (sid == NULL) {
-			TALLOC_FREE(sid_str);
-			return NULL;
-		};
-		ok = dom_sid_parse_endp(sid_str, sid, &end);
-		if (!ok) {
-			DBG_WARNING("could not parse SID '%s'\n", sid_str);
-			TALLOC_FREE(sid_str);
-			TALLOC_FREE(sid);
-			return NULL;
-		}
-		if (end - sid_str != len) {
-			DBG_WARNING("trailing junk after SID '%s'\n", sid_str);
-			TALLOC_FREE(sid_str);
-			TALLOC_FREE(sid);
-			return NULL;
-		}
-		TALLOC_FREE(sid_str);
 		(*sddlp) += len;
-		return sid;
+		return true;
 	}
 
 	/* now check for one of the special codes */
@@ -266,28 +256,46 @@ static struct dom_sid *sddl_transition_decode_sid(TALLOC_CTX *mem_ctx, const cha
 	}
 	if (i == ARRAY_SIZE(sid_codes)) {
 		DEBUG(1,("Unknown sddl sid code '%2.2s'\n", sddl));
-		return NULL;
+		return false;
 	}
 
 	(*sddlp) += 2;
 
 
 	if (sid_codes[i].machine_rid != 0) {
-		return dom_sid_add_rid(mem_ctx, state->machine_sid,
+		return sid_compose(sid,
-				       sid_codes[i].machine_rid);
+				   state->machine_sid,
+				   sid_codes[i].machine_rid);
 	}
 
 	if (sid_codes[i].domain_rid != 0) {
-		return dom_sid_add_rid(mem_ctx, state->domain_sid,
+		return sid_compose(sid,
-				       sid_codes[i].domain_rid);
+				   state->domain_sid,
+				   sid_codes[i].domain_rid);
 	}
 
 	if (sid_codes[i].forest_rid != 0) {
-		return dom_sid_add_rid(mem_ctx, state->forest_sid,
+		return sid_compose(sid,
-				       sid_codes[i].forest_rid);
+				   state->forest_sid,
+				   sid_codes[i].forest_rid);
 	}
 
-	return dom_sid_parse_talloc(mem_ctx, sid_codes[i].sid);
+	return dom_sid_parse(sid_codes[i].sid, sid);
+}
+
+static struct dom_sid *sddl_transition_decode_sid_talloc(
+	TALLOC_CTX *mem_ctx,
+	const char **sddlp,
+	struct sddl_transition_state *state)
+{
+	struct dom_sid sid;
+	bool ok;
+
+	ok = sddl_transition_decode_sid(sddlp, state, &sid);
+	if (!ok) {
+		return NULL;
+	}
+	return dom_sid_dup(mem_ctx, &sid);
 }
 
 struct dom_sid *sddl_decode_sid(TALLOC_CTX *mem_ctx, const char **sddlp,
@@ -303,7 +311,8 @@ struct dom_sid *sddl_decode_sid(TALLOC_CTX *mem_ctx, const char **sddlp,
 		.domain_sid = domain_sid,
 		.forest_sid = domain_sid,
 	};
-	return sddl_transition_decode_sid(mem_ctx, sddlp, &state);
+
+	return sddl_transition_decode_sid_talloc(mem_ctx, sddlp, &state);
 }
 
 
@@ -528,7 +537,6 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx,
 	const char *tok[7];
 	const char *s;
 	uint32_t v;
-	struct dom_sid *sid;
 	bool ok;
 	size_t len;
 	size_t count = 0;
@@ -682,16 +690,14 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx,
 
 	/* trustee */
 	s = tok[5];
-	sid = sddl_transition_decode_sid(mem_ctx, &s, state);
+	ok = sddl_transition_decode_sid(&s, state, &ace->trustee);
-	if (sid == NULL) {
+	if (!ok) {
 		*msg = talloc_strdup(
 			mem_ctx,
 			"could not parse trustee SID");
 		*msg_offset = tok[5] - *sddl_copy;
 		return false;
 	}
-	ace->trustee = *sid;
-	talloc_free(sid);
 	if (*s != '\0') {
 		*msg = talloc_strdup(
 			mem_ctx,
@@ -923,12 +929,10 @@ struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char
 	*msg = NULL;
 	*msg_offset = 0;
 
-	sd = talloc_zero(mem_ctx, struct security_descriptor);
+	sd = security_descriptor_initialise(mem_ctx);
 	if (sd == NULL) {
 		return NULL;
 	}
-	sd->revision = SECURITY_DESCRIPTOR_REVISION_1;
-	sd->type     = SEC_DESC_SELF_RELATIVE;
 
 	while (*sddl) {
 		uint32_t flags;
@@ -957,12 +961,14 @@ struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char
 			break;
 		case 'O':
 			if (sd->owner_sid != NULL) goto failed;
-			sd->owner_sid = sddl_transition_decode_sid(sd, &sddl, &state);
+			sd->owner_sid = sddl_transition_decode_sid_talloc(
+				sd, &sddl, &state);
 			if (sd->owner_sid == NULL) goto failed;
 			break;
 		case 'G':
 			if (sd->group_sid != NULL) goto failed;
-			sd->group_sid = sddl_transition_decode_sid(sd, &sddl, &state);
+			sd->group_sid = sddl_transition_decode_sid_talloc(
+				sd, &sddl, &state);
 			if (sd->group_sid == NULL) goto failed;
 			break;
 		default:

libcli/security/security_descriptor.c

@@ -34,18 +34,17 @@ struct security_descriptor *security_descriptor_initialise(TALLOC_CTX *mem_ctx)
 	if (!sd) {
 		return NULL;
 	}
+	*sd = (struct security_descriptor){
+		.revision = SD_REVISION,
 
-	sd->revision = SD_REVISION;
+		/*
-	/* we mark as self relative, even though it isn't while it remains
+		 * we mark as self relative, even though it isn't
-	   a pointer in memory because this simplifies the ndr code later.
+		 * while it remains a pointer in memory because this
-	   All SDs that we store/emit are in fact SELF_RELATIVE
+		 * simplifies the ndr code later.  All SDs that we
-	*/
+		 * store/emit are in fact SELF_RELATIVE
-	sd->type = SEC_DESC_SELF_RELATIVE;
+		 */
-
+		.type = SEC_DESC_SELF_RELATIVE,
-	sd->owner_sid = NULL;
+	};
-	sd->group_sid = NULL;
-	sd->sacl = NULL;
-	sd->dacl = NULL;
 
 	return sd;
 }

source3/script/tests/test_winbind_call_depth_trace.sh

@@ -34,43 +34,22 @@ EOF
 	exit 0
 fi
 
-saved_level=1
-
-get_winbind_loglevel()
-{
-	s1=$(${SMBCONTROL} "${CONFIGURATION}" winbind debuglevel)
-	# We need to get the all level from output like this:
-	# "PID 664474: all:1 tdb:1 printdrivers:1  lanman:1 smb:1 rpc_parse:1 rpc_srv:1 rpc_cli:1 passdb:1 sam:1..."
-	# 1. remove PID 664474:
-	s2=${s1#PID*: }
-	# "all:1 tdb:1 printdrivers:1  lanman:1 smb:1 rpc_parse:1 rpc_srv:1 rpc_cli:1 passdb"
-	# 2. remove " tdb:1 printdrivers:1 ..."
-	s3=${s2%% *}
-	# "all:1"
-	# 3. remove "all:"
-	saved_level=${s3#all:}
-}
-
 # Example of trace line
 # [2023/01/25 00:20:33.307038,  5, pid=535581, effective(0, 0), real(0, 0), class=winbind, traceid=78, depth=4] ../../source3/winbindd/wb_group_members.c:310(wb_group_members_send)
 test_winbind_call_depth_trace()
 {
-	get_winbind_loglevel
+	global_inject_conf=$(dirname $SMB_CONF_PATH)/global_inject.conf
-
+	echo "debug syslog format = no" >$global_inject_conf
-	# If loglevel < 10, set it to 10.
+	echo "log level = 10" >>$global_inject_conf
-	if [ "$saved_level" -lt 10 ]; then
+	${SMBCONTROL} "${CONFIGURATION}" winbind reload-config
-		${SMBCONTROL}  "${CONFIGURATION}" winbind debug 10
-	fi
 
 	COUNT1=$(grep -c wb_group_members_send "$LOGFILE")
 
 	id ADDOMAIN/alice
 	ret=$?
 
-	# Restore loglevel, if it was changed.
+	echo "" >$global_inject_conf
-	if [ "$saved_level" -lt 10 ]; then
+	${SMBCONTROL} "${CONFIGURATION}" winbind reload-config
-		${SMBCONTROL} "${CONFIGURATION}" winbind debug "$saved_level"
-	fi
 
 	if [ $ret != 0 ]; then
 		echo "Command 'id ADDOMAIN/alice' failed!"
@@ -84,17 +63,17 @@ test_winbind_call_depth_trace()
 		return 1
 	fi
 
-	# Test that the depth of last line with 'wb_group_members_send' is: depth=4
+	# Test that the depth of last line with 'wb_group_members_send' is: depth=3
-	COUNT3=$(grep wb_group_members_send "$LOGFILE" | tail -1 | grep -c depth=4)
+	COUNT3=$(grep wb_group_members_send "$LOGFILE" | tail -1 | grep -c depth=3)
 	if [ "$COUNT3" -ne 1 ]; then
-		echo "The last line with wb_group_members_send should have depth=4."
+		echo "The last line with wb_group_members_send should have depth=3."
 		return 1
 	fi
 
 	# Test that the indentation of the line below last 'wb_group_members_send' is indented by 2+4*4 spaces:
-	COUNT4=$(grep -A1 wb_group_members_send "$LOGFILE" | tail -1| grep -c '^                  WB command group_members start')
+	COUNT4=$(grep 'WB command group_members start' "$LOGFILE" | tail -1| grep -c '^              WB command group_members start')
 	if [ "$COUNT4" -ne 1 ]; then
-		echo "The line after the last line with wb_group_members_send should be indented by 18 spaces."
+		echo "The line after the last line with wb_group_members_send should be indented by 14 spaces."
 		return 1
 	fi