]> git.ipfire.org Git - thirdparty/samba.git/commitdiff
libcli/security: access_check with MAXIMUM_ALLOWED checks callbacks
authorDouglas Bagnall <douglas.bagnall@catalyst.net.nz>
Wed, 13 Sep 2023 05:25:52 +0000 (17:25 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Tue, 26 Sep 2023 23:45:36 +0000 (23:45 +0000)
To help clarify the logic, we make new functions that separate the
deny and allow cases, which helps keep track of what 'yes' and 'no'
mean and which incorporate the logic of token->evaluate_claims
handling, which determines when we want to run a conditional ACE, when
we want to ignore it, and when we want to take offence. In the case
when we decide to run it, we then need to decide whether to apply it
or ignore it based on the result. This last bit differs between allow
and deny aces, hence the two functions.

These functions will replace check_callback_ace_access() over the next
few commits.

In the case where token->evaluate_claims is
CLAIMS_EVALUATION_INVALID_STATE and the DACL contains a conditional
ACE, the maximum allowed is 0, as if it was a "deny everything" ACE.

This is an unexpected case. Most likely the evaluate_claims state
will be NEVER or ALWAYS. In the NEVER case the conditional ACE is
skipped, as would have happened in all cases before 4.20, while in the
ALWAYS case the conditional ACE is run and applied if successful.

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

index 76c1d1d93d08ba4d882633fcf5868089f68f03c6..e876f2e2bd573cdf87133e5a46300f88fa1b0b58 100644 (file)
@@ -105,6 +105,130 @@ void se_map_standard(uint32_t *access_mask, const struct standard_mapping *mappi
        }
 }
 
+enum ace_callback_result {
+       ACE_CALLBACK_DENY,
+       ACE_CALLBACK_ALLOW,
+       ACE_CALLBACK_SKIP,      /* do not apply this ACE */
+       ACE_CALLBACK_INVALID    /* we don't want to process the conditional ACE */
+};
+
+
+static enum ace_callback_result check_callback_ace_allow(
+       const struct security_ace *ace,
+       const struct security_token *token,
+       const struct security_descriptor *sd)
+{
+       bool ok;
+       int result;
+
+       switch (token->evaluate_claims) {
+       case CLAIMS_EVALUATION_ALWAYS:
+               break;
+
+       case CLAIMS_EVALUATION_INVALID_STATE:
+               DBG_WARNING("Refusing to evaluate ACL with "
+                           "conditional ACE against security "
+                           "token with CLAIMS_EVALUATION_INVALID_STATE\n");
+               return ACE_CALLBACK_INVALID;
+       case CLAIMS_EVALUATION_NEVER:
+       default:
+               /*
+                * We are asked to pretend we never understood this
+                * ACE type.
+                *
+                * By returning SKIP, this ACE will not adjust any
+                * permission bits making it an effective no-op, which
+                * was the default behaviour up to Samba 4.19.
+                */
+               return ACE_CALLBACK_SKIP;
+       }
+
+       if (ace->type != SEC_ACE_TYPE_ACCESS_ALLOWED_CALLBACK &&
+           ace->type != SEC_ACE_TYPE_ACCESS_ALLOWED_CALLBACK_OBJECT) {
+               /* This indicates a programming error */
+               DBG_ERR("bad conditional allow ACE type: %u\n", ace->type);
+               return ACE_CALLBACK_INVALID;
+       }
+
+       /*
+        * Until we discover otherwise, we assume all callback ACEs
+        * are conditional ACEs.
+        */
+       ok = access_check_conditional_ace(ace, token, sd, &result);
+       if (!ok) {
+               /*
+                * An error in processing the conditional ACE is
+                * treated as UNKNOWN, which amounts to a DENY/SKIP
+                * result.
+                *
+                * This is different from the INVALID result which
+                * means we should not be thinking about conditional
+                * ACES at all, and will abort the whole access check.
+                */
+               DBG_WARNING("callback ACE was not a valid conditional ACE\n");
+               return ACE_CALLBACK_SKIP;
+       }
+       if (result == ACE_CONDITION_TRUE) {
+               return ACE_CALLBACK_ALLOW;
+       }
+       /* UNKNOWN means do not allow */
+       return ACE_CALLBACK_SKIP;
+}
+
+
+static enum ace_callback_result check_callback_ace_deny(
+       const struct security_ace *ace,
+       const struct security_token *token,
+       const struct security_descriptor *sd)
+{
+       bool ok;
+       int result;
+
+       switch (token->evaluate_claims) {
+       case CLAIMS_EVALUATION_ALWAYS:
+               break;
+
+       case CLAIMS_EVALUATION_INVALID_STATE:
+               DBG_WARNING("Refusing to evaluate ACL with "
+                           "conditional ACE against security "
+                           "token with CLAIMS_EVALUATION_INVALID_STATE\n");
+               return ACE_CALLBACK_INVALID;
+       case CLAIMS_EVALUATION_NEVER:
+       default:
+               /*
+                * We are asked to pretend we never understood this
+                * ACE type.
+                */
+               return ACE_CALLBACK_SKIP;
+       }
+
+       if (ace->type != SEC_ACE_TYPE_ACCESS_DENIED_CALLBACK &&
+           ace->type != SEC_ACE_TYPE_ACCESS_DENIED_CALLBACK_OBJECT) {
+               DBG_ERR("bad conditional deny ACE type: %u\n", ace->type);
+               return ACE_CALLBACK_INVALID;
+       }
+
+       /*
+        * Until we discover otherwise, we assume all callback ACEs
+        * are conditional ACEs.
+        */
+       ok = access_check_conditional_ace(ace, token, sd, &result);
+       if (!ok) {
+               /*
+                * An error in processing the conditional ACE is
+                * treated as UNKNOWN, which means DENY.
+                */
+               DBG_WARNING("callback ACE was not a valid conditional ACE\n");
+               return ACE_CALLBACK_DENY;
+       }
+       if (result != ACE_CONDITION_FALSE) {
+               /* UNKNOWN means deny */
+               return ACE_CALLBACK_DENY;
+       }
+       return ACE_CALLBACK_SKIP;
+}
+
+
 /*
   perform a SEC_FLAG_MAXIMUM_ALLOWED access check
 */
@@ -193,6 +317,33 @@ static uint32_t access_check_max_allowed(const struct security_descriptor *sd,
                case SEC_ACE_TYPE_ACCESS_DENIED_OBJECT:
                        denied |= ~granted & ace->access_mask;
                        break;
+
+               case SEC_ACE_TYPE_ACCESS_ALLOWED_CALLBACK:
+               {
+                       enum ace_callback_result allow =
+                               check_callback_ace_allow(ace, token, sd);
+                       if (allow == ACE_CALLBACK_INVALID) {
+                               return 0;
+                       }
+                       if (allow == ACE_CALLBACK_ALLOW) {
+                               granted |= ace->access_mask;
+                       }
+                       break;
+               }
+
+               case SEC_ACE_TYPE_ACCESS_DENIED_CALLBACK:
+               {
+                       enum ace_callback_result deny =
+                               check_callback_ace_deny(ace, token, sd);
+                       if (deny == ACE_CALLBACK_INVALID) {
+                               return 0;
+                       }
+                       if (deny == ACE_CALLBACK_DENY) {
+                               denied |= ~granted & ace->access_mask;
+                       }
+                       break;
+               }
+
                default:        /* Other ACE types not handled/supported */
                        break;
                }