libcli/security: separate out claim_v1_to_ace_composite_unchecked()

For SDDL Resource ACE conversions we don't want to check too much
claim validity so that a semi-invalid ACE can round-trip through
deserialisation and serialisation. This is because Windows allows it,
but also because if the check puts the values in a sorted order that
makes the round-trip less round (that is, the return string is
semantically the same but possibly different in byte order).

The validity we're talking about is mostly uniqueness. For example
`S:(RA;;;;;WD;("foo",TU,0,7,5,7))` has two 7s, and that would be
invalid as a claim, but this is not checked while in ACE form.

On the other hand `S:(RA;;;;;WD;("foo",TU,0,3,2))` is valid, but the
return string will have 3 and 2 reversed when the check is made. We
prefer the ACE to stay the same while it is just being an ACE.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

libcli/security/claims-conversions.c

@@ -295,12 +295,59 @@ static bool claim_v1_copy(
 	const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *src);
 
 
+
+bool claim_v1_to_ace_composite_unchecked(
+	TALLOC_CTX *mem_ctx,
+	const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim,
+	struct ace_condition_token *result)
+{
+	/*
+	 * This converts a claim object into a conditional ACE
+	 * composite without checking whether it is a valid and sorted
+	 * claim. It is called in two places:
+	 *
+	 * 1. claim_v1_to_ace_token() below (which does do those
+	 * checks, and is the function you want).
+	 *
+	 * 2. sddl_resource_attr_from_claim() in which a resource
+	 * attribute claim needs to pass through a conditional ACE
+	 * composite structure on its way to becoming SDDL. In that
+	 * case we don't want to check validity.
+	 */
+	size_t i;
+	struct ace_condition_token *tokens = NULL;
+	bool ok;
+
+	tokens = talloc_array(mem_ctx,
+			      struct ace_condition_token,
+			      claim->value_count);
+	if (tokens == NULL) {
+		return false;
+	}
+
+	for (i = 0; i < claim->value_count; i++) {
+		ok = claim_v1_offset_to_ace_token(tokens,
+						  claim,
+						  i,
+						  &tokens[i]);
+		if (! ok) {
+			TALLOC_FREE(tokens);
+			return false;
+		}
+	}
+
+	result->type = CONDITIONAL_ACE_TOKEN_COMPOSITE;
+	result->data.composite.tokens = tokens;
+	result->data.composite.n_members = claim->value_count;
+	result->flags = claim->flags & CLAIM_SECURITY_ATTRIBUTE_VALUE_CASE_SENSITIVE;
+	return true;
+}
+
+
 bool claim_v1_to_ace_token(TALLOC_CTX *mem_ctx,
 			   const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim,
 			   struct ace_condition_token *result)
 {
-	size_t i;
-	struct ace_condition_token *tokens = NULL;
 	struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim_copy = NULL;
 	const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *sorted_claim = NULL;
 	NTSTATUS status;
@@ -366,6 +413,12 @@ bool claim_v1_to_ace_token(TALLOC_CTX *mem_ctx,
 		}
 		sorted_claim = claim_copy;
 	}
+	ok = claim_v1_to_ace_composite_unchecked(mem_ctx, sorted_claim, result);
+	if (! ok) {
+		TALLOC_FREE(claim_copy);
+		return false;
+	}
+
 	/*
 	 * The multiple values will get turned into a composite
 	 * literal in the conditional ACE. Each element of the
@@ -374,35 +427,9 @@ bool claim_v1_to_ace_token(TALLOC_CTX *mem_ctx,
 	 * set here (at least the _FROM_ATTR flag) or the child values
 	 * will not be reached.
 	 */
-
+	result->flags |= (
-	result->flags = (
 		CONDITIONAL_ACE_FLAG_TOKEN_FROM_ATTR |
-		CLAIM_SECURITY_ATTRIBUTE_UNIQUE_AND_SORTED |
+		CLAIM_SECURITY_ATTRIBUTE_UNIQUE_AND_SORTED);
-		(sorted_claim->flags & CLAIM_SECURITY_ATTRIBUTE_VALUE_CASE_SENSITIVE));
-
-	tokens = talloc_array(mem_ctx,
-			      struct ace_condition_token,
-			      sorted_claim->value_count);
-	if (tokens == NULL) {
-		TALLOC_FREE(claim_copy);
-		return false;
-	}
-
-	for (i = 0; i < sorted_claim->value_count; i++) {
-		ok = claim_v1_offset_to_ace_token(tokens,
-						  sorted_claim,
-						  i,
-						  &tokens[i]);
-		if (! ok) {
-			TALLOC_FREE(claim_copy);
-			TALLOC_FREE(tokens);
-			return false;
-		}
-	}
-
-	result->type = CONDITIONAL_ACE_TOKEN_COMPOSITE;
-	result->data.composite.tokens = tokens;
-	result->data.composite.n_members = sorted_claim->value_count;
 
 	return true;
 }

libcli/security/claims-conversions.h

@@ -48,6 +48,10 @@ NTSTATUS token_claims_to_claims_v1(TALLOC_CTX *mem_ctx,
 				   struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 **out_claims,
 				   uint32_t *out_n_claims);
 
+bool claim_v1_to_ace_composite_unchecked(TALLOC_CTX *mem_ctx,
+					 const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim,
+					 struct ace_condition_token *result);
+
 NTSTATUS claim_v1_check_and_sort(
 	TALLOC_CTX *mem_ctx,
 	struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim,

libcli/security/sddl_conditional_ace.c

@@ -3386,7 +3386,7 @@ char *sddl_resource_attr_from_claim(
 	}
 	ctx.mem_ctx = tmp_ctx;
 
-	ok = claim_v1_to_ace_token(tmp_ctx, claim, &tok);
+	ok = claim_v1_to_ace_composite_unchecked(tmp_ctx, claim, &tok);
 	if (!ok) {
 		TALLOC_FREE(tmp_ctx);
 		return NULL;