From: Douglas Bagnall Date: Fri, 17 Nov 2023 00:58:12 +0000 (+1300) Subject: libcli/security: separate out claim_v1_to_ace_composite_unchecked() X-Git-Tag: talloc-2.4.2~497 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6a07d2fe44e0b699b4485963d3d47510e2ef275d;p=thirdparty%2Fsamba.git 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 Reviewed-by: Andrew Bartlett --- diff --git a/libcli/security/claims-conversions.c b/libcli/security/claims-conversions.c index 6c55420fff3..22ab252173a 100644 --- a/libcli/security/claims-conversions.c +++ b/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 | - (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; + CLAIM_SECURITY_ATTRIBUTE_UNIQUE_AND_SORTED); return true; } diff --git a/libcli/security/claims-conversions.h b/libcli/security/claims-conversions.h index 7cddced2ec0..78d8e911bdd 100644 --- a/libcli/security/claims-conversions.h +++ b/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, diff --git a/libcli/security/sddl_conditional_ace.c b/libcli/security/sddl_conditional_ace.c index e6aadb1d394..5203f40abe2 100644 --- a/libcli/security/sddl_conditional_ace.c +++ b/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;