libcli/security: claim_v1_to_ace_token(): avoid unnecessary re-sort

If it is a wire claim (which is probably most common), the checking
and sorting has already happened. We don't need to make a copy to
sort and check.

In either case, there is still a copy step to make the conditional ACE
token.

This shuffles around some knownfails because the claim_v1_copy()
function we were using is checking for duplicates, which we don't
always want. That will be fixed soon.

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 770795e29e7ad363befcf0afd0534d12b2003d38..6c55420fff385da7441537d0e9aa926879428007 100644 (file)
--- a/libcli/security/claims-conversions.c
+++ b/libcli/security/claims-conversions.c
@@ -301,9 +301,13 @@ bool claim_v1_to_ace_token(TALLOC_CTX *mem_ctx,
 {
        size_t i;
        struct ace_condition_token *tokens = NULL;
-       struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *sorted_claim = NULL;
+       struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim_copy = NULL;
+       const struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *sorted_claim = NULL;
        NTSTATUS status;
        bool ok;
+       bool case_sensitive = claim->flags &                    \
+               CLAIM_SECURITY_ATTRIBUTE_VALUE_CASE_SENSITIVE;
+
        if (claim->value_count < 1 ||
            claim->value_count >= CONDITIONAL_ACE_MAX_TOKENS) {
                DBG_WARNING("rejecting claim with %"PRIu32" tokens\n",
@@ -322,18 +326,46 @@ bool claim_v1_to_ace_token(TALLOC_CTX *mem_ctx,
                                                    result);
        }
 
-       ok = claim_v1_copy(mem_ctx, sorted_claim, claim);
-       if (!ok) {
-               return false;
-       }
+       if (claim->flags & CLAIM_SECURITY_ATTRIBUTE_UNIQUE_AND_SORTED) {
+               /*
+                * We can avoid making a sorted copy.
+                *
+                * This is normal case for wire claims, where the
+                * sorting and duplicate checking happens earlier in
+                * token_claims_to_claims_v1().
+               */
+               sorted_claim = claim;
+       } else {
+               /*
+                * This is presumably a resource attribute ACE, which
+                * is stored in the ACE as struct
+                * CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1, and we don't
+                * really want to mutate that copy -- even if there
+                * aren't currently realistic pathways that read an
+                * ACE, trigger this, and write it back (outside of
+                * tests).
+                */
+               claim_copy = talloc(mem_ctx, struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1);
+               if (claim_copy == NULL) {
+                       return false;
+               }
 
-       status = claim_v1_check_and_sort(mem_ctx, sorted_claim);
-       if (!NT_STATUS_IS_OK(status)) {
-               DBG_WARNING("resource attribute claim sort failed with %s\n",
-                           nt_errstr(status));
-               return false;
-       }
+               ok = claim_v1_copy(claim_copy, claim_copy, claim);
+               if (!ok) {
+                       TALLOC_FREE(claim_copy);
+                       return false;
+               }
 
+               status = claim_v1_check_and_sort(claim_copy, claim_copy,
+                                                case_sensitive);
+               if (!NT_STATUS_IS_OK(status)) {
+                       DBG_WARNING("resource attribute claim sort failed with %s\n",
+                                   nt_errstr(status));
+                       TALLOC_FREE(claim_copy);
+                       return false;
+               }
+               sorted_claim = claim_copy;
+       }
        /*
         * The multiple values will get turned into a composite
         * literal in the conditional ACE. Each element of the
@@ -352,15 +384,17 @@ bool claim_v1_to_ace_token(TALLOC_CTX *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++) {
-               bool ok = claim_v1_offset_to_ace_token(tokens,
-                                                      sorted_claim,
-                                                      i,
-                                                      &tokens[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;
                }

diff --git a/selftest/knownfail.d/conditional_ace_claims b/selftest/knownfail.d/conditional_ace_claims
index 8fb6730b6f80a843d90a17599d6872cc56a35f90..40e342afab734d8e20f0dd56a8afefc7020d7fc6 100644 (file)
--- a/selftest/knownfail.d/conditional_ace_claims
+++ b/selftest/knownfail.d/conditional_ace_claims
@@ -1,4 +1 @@
 ^samba.tests.conditional_ace_claims.+ConditionalAceLargeComposites.test_allow_002-0-vs-0
-^samba.tests.conditional_ace_claims.+ConditionalAceLargeComposites.test_deny_001-90-disorderly-strings-claim-vs-claim-case-sensitive-with-dupes
-^samba.tests.conditional_ace_claims.+ConditionalAceLargeComposites.test_deny_022-102+1-dupe-vs-102+1-dupe
-^samba.tests.conditional_ace_claims.+ConditionalAceLargeComposites.test_deny_024-2+1-dupe-vs-2+1-dupe

diff --git a/selftest/knownfail.d/run_conditional_ace b/selftest/knownfail.d/run_conditional_ace
deleted file mode 100644 (file)
index 4527c82..0000000
--- a/selftest/knownfail.d/run_conditional_ace
+++ /dev/null
@@ -1,2 +0,0 @@
-^samba.unittests.run_conditional_ace.test_composite_different_order_with_SID_dupes$
-^samba.unittests.run_conditional_ace.test_composite_different_order_with_dupes$