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>

diff --git a/libcli/security/claims-conversions.c b/libcli/security/claims-conversions.c
index 6c55420fff385da7441537d0e9aa926879428007..22ab252173a813bb21fb11bb3e365e5bae8b69a3 100644 (file)
--- 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 7cddced2ec0e3a8ef65398274bed6d44b4b9abb2..78d8e911bdd0cfc58e9b1f9595ad9ad164038c8b 100644 (file)
--- 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 e6aadb1d394655d167122f7607787306e8393877..5203f40abe29ccec8d5b0b8c26f886b21725b31b 100644 (file)
--- 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;