libcli/security/dom_sid.c

@@ -39,27 +39,20 @@ int dom_sid_compare_auth(const struct dom_sid *sid1,
 {
 	int i;
 
-	if (sid1 == sid2) {
+	if (sid1 == sid2)
 		return 0;
-	}
-
-	if (sid1 == NULL) {
+	if (!sid1)
 		return -1;
-	}
-
-	if (sid2 == NULL) {
+	if (!sid2)
 		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;
 }
@@ -72,17 +65,12 @@ 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 == NULL) {
+	if (!sid1)
 		return -1;
-	}
-
-	if (sid2 == NULL) {
+	if (!sid2)
 		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 bool sddl_transition_decode_sid(const char **sddlp,
-				       struct sddl_transition_state *state,
-				       struct dom_sid *sid)
+static struct dom_sid *sddl_transition_decode_sid(TALLOC_CTX *mem_ctx, const char **sddlp,
+						  struct sddl_transition_state *state)
 {
 	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 false;
-		}
-		if (len > DOM_SID_STR_BUFLEN) { /* Invalid SID */
-			return false;
+			return NULL;
 		}
 		if (sddl[len - 1] == 'D' && sddl[len] == ':') {
 			/*
@@ -226,28 +226,38 @@ static bool sddl_transition_decode_sid(const char **sddlp,
 			len--;
 		}
 
-		{
-			const char *end = 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;
-			}
+		sid_str = talloc_strndup(mem_ctx, sddl, len);
+		if (sid_str == NULL) {
+			return NULL;
 		}
+		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 true;
+		return sid;
 	}
 
 	/* now check for one of the special codes */
@@ -256,46 +266,28 @@ static bool sddl_transition_decode_sid(const char **sddlp,
 	}
 	if (i == ARRAY_SIZE(sid_codes)) {
 		DEBUG(1,("Unknown sddl sid code '%2.2s'\n", sddl));
-		return false;
+		return NULL;
 	}
 
 	(*sddlp) += 2;
 
 
 	if (sid_codes[i].machine_rid != 0) {
-		return sid_compose(sid,
-				   state->machine_sid,
-				   sid_codes[i].machine_rid);
+		return dom_sid_add_rid(mem_ctx, state->machine_sid,
+				       sid_codes[i].machine_rid);
 	}
 
 	if (sid_codes[i].domain_rid != 0) {
-		return sid_compose(sid,
-				   state->domain_sid,
-				   sid_codes[i].domain_rid);
+		return dom_sid_add_rid(mem_ctx, state->domain_sid,
+				       sid_codes[i].domain_rid);
 	}
 
 	if (sid_codes[i].forest_rid != 0) {
-		return sid_compose(sid,
-				   state->forest_sid,
-				   sid_codes[i].forest_rid);
+		return dom_sid_add_rid(mem_ctx, state->forest_sid,
+				       sid_codes[i].forest_rid);
 	}
 
-	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);
+	return dom_sid_parse_talloc(mem_ctx, sid_codes[i].sid);
 }
 
 struct dom_sid *sddl_decode_sid(TALLOC_CTX *mem_ctx, const char **sddlp,
@@ -311,8 +303,7 @@ 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_talloc(mem_ctx, sddlp, &state);
+	return sddl_transition_decode_sid(mem_ctx, sddlp, &state);
 }
 
 
@@ -537,6 +528,7 @@ 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;
@@ -690,14 +682,16 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx,
 
 	/* trustee */
 	s = tok[5];
-	ok = sddl_transition_decode_sid(&s, state, &ace->trustee);
-	if (!ok) {
+	sid = sddl_transition_decode_sid(mem_ctx, &s, state);
+	if (sid == NULL) {
 		*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,
@@ -929,10 +923,12 @@ struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char
 	*msg = NULL;
 	*msg_offset = 0;
 
-	sd = security_descriptor_initialise(mem_ctx);
+	sd = talloc_zero(mem_ctx, struct security_descriptor);
 	if (sd == NULL) {
 		return NULL;
 	}
+	sd->revision = SECURITY_DESCRIPTOR_REVISION_1;
+	sd->type     = SEC_DESC_SELF_RELATIVE;
 
 	while (*sddl) {
 		uint32_t flags;
@@ -961,14 +957,12 @@ 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_talloc(
-				sd, &sddl, &state);
+			sd->owner_sid = sddl_transition_decode_sid(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_talloc(
-				sd, &sddl, &state);
+			sd->group_sid = sddl_transition_decode_sid(sd, &sddl, &state);
 			if (sd->group_sid == NULL) goto failed;
 			break;
 		default:

libcli/security/security_descriptor.c

@@ -34,17 +34,18 @@ struct security_descriptor *security_descriptor_initialise(TALLOC_CTX *mem_ctx)
 	if (!sd) {
 		return NULL;
 	}
-	*sd = (struct security_descriptor){
-		.revision = SD_REVISION,
 
-		/*
-		 * we mark as self relative, even though it isn't
-		 * while it remains a pointer in memory because this
-		 * simplifies the ndr code later.  All SDs that we
-		 * store/emit are in fact SELF_RELATIVE
-		 */
-		.type = SEC_DESC_SELF_RELATIVE,
-	};
+	sd->revision = SD_REVISION;
+	/* we mark as self relative, even though it isn't while it remains
+	   a pointer in memory because this simplifies the ndr code later.
+	   All SDs that we store/emit are in fact SELF_RELATIVE
+	*/
+	sd->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,22 +34,43 @@ 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()
 {
-	global_inject_conf=$(dirname $SMB_CONF_PATH)/global_inject.conf
-	echo "debug syslog format = no" >$global_inject_conf
-	echo "log level = 10" >>$global_inject_conf
-	${SMBCONTROL} "${CONFIGURATION}" winbind reload-config
+	get_winbind_loglevel
+
+	# If loglevel < 10, set it to 10.
+	if [ "$saved_level" -lt 10 ]; then
+		${SMBCONTROL}  "${CONFIGURATION}" winbind debug 10
+	fi
 
 	COUNT1=$(grep -c wb_group_members_send "$LOGFILE")
 
 	id ADDOMAIN/alice
 	ret=$?
 
-	echo "" >$global_inject_conf
-	${SMBCONTROL} "${CONFIGURATION}" winbind reload-config
+	# Restore loglevel, if it was changed.
+	if [ "$saved_level" -lt 10 ]; then
+		${SMBCONTROL} "${CONFIGURATION}" winbind debug "$saved_level"
+	fi
 
 	if [ $ret != 0 ]; then
 		echo "Command 'id ADDOMAIN/alice' failed!"
@@ -63,17 +84,17 @@ test_winbind_call_depth_trace()
 		return 1
 	fi
 
-	# 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=3)
+	# Test that the depth of last line with 'wb_group_members_send' is: depth=4
+	COUNT3=$(grep wb_group_members_send "$LOGFILE" | tail -1 | grep -c depth=4)
 	if [ "$COUNT3" -ne 1 ]; then
-		echo "The last line with wb_group_members_send should have depth=3."
+		echo "The last line with wb_group_members_send should have depth=4."
 		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 'WB command group_members start' "$LOGFILE" | tail -1| grep -c '^              WB command group_members start')
+	COUNT4=$(grep -A1 wb_group_members_send "$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 14 spaces."
+		echo "The line after the last line with wb_group_members_send should be indented by 18 spaces."
 		return 1
 	fi