libcli/security: Optionally disallow device‐specific attributes and operators where...

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/lib/fuzzing/fuzz_conditional_ace_blob.c b/lib/fuzzing/fuzz_conditional_ace_blob.c
index aed1cd37c735db8c1b0ab00ce606e68c5e6906d3..70bb5723c51aad80c98f5922f3a3ef43ebabb0e3 100644 (file)
--- a/lib/fuzzing/fuzz_conditional_ace_blob.c
+++ b/lib/fuzzing/fuzz_conditional_ace_blob.c
@@ -95,6 +95,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *input, size_t len)
        }
 
        s2 = ace_conditions_compile_sddl(mem_ctx,
+                                        ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                         sddl,
                                         &message,
                                         &message_offset,

diff --git a/lib/fuzzing/fuzz_sddl_conditional_ace.c b/lib/fuzzing/fuzz_sddl_conditional_ace.c
index e21c2ec9b12f528506fe400a77f2c28f4bb4728a..636ebf1da9eaad10c9988f651983b0752f435727 100644 (file)
--- a/lib/fuzzing/fuzz_sddl_conditional_ace.c
+++ b/lib/fuzzing/fuzz_sddl_conditional_ace.c
@@ -57,6 +57,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *input, size_t len)
        mem_ctx = talloc_new(NULL);
 
        s1 = ace_conditions_compile_sddl(mem_ctx,
+                                        ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                         sddl_string,
                                         &message,
                                         &message_offset,
@@ -98,6 +99,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *input, size_t len)
        }
 
        s2 = ace_conditions_compile_sddl(mem_ctx,
+                                        ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                         resddl,
                                         &message,
                                         &message_offset,

diff --git a/libcli/security/conditional_ace.h b/libcli/security/conditional_ace.h
index f1204914b01701570d36c27313053704498deaba..e59205679342dd85ef19939c9686bc640a8206ae 100644 (file)
--- a/libcli/security/conditional_ace.h
+++ b/libcli/security/conditional_ace.h
@@ -46,6 +46,7 @@ bool conditional_ace_encode_binary(TALLOC_CTX *mem_ctx,
                                   DATA_BLOB *dest);
 
 struct ace_condition_script * ace_conditions_compile_sddl(TALLOC_CTX *mem_ctx,
+                                                         const enum ace_condition_flags ace_condition_flags,
                                                          const char *sddl,
                                                          const char **message,
                                                          size_t *message_offset,

diff --git a/libcli/security/sddl.c b/libcli/security/sddl.c
index 5d481749a9bcc041d08b97969dd514f30d02ca9f..97e579cfe3290175d8467aa07b0426f3aea06ee1 100644 (file)
--- a/libcli/security/sddl.c
+++ b/libcli/security/sddl.c
@@ -487,6 +487,7 @@ static bool sddl_decode_guid(const char *str, struct GUID *guid)
 
 
 static DATA_BLOB sddl_decode_conditions(TALLOC_CTX *mem_ctx,
+                                       const enum ace_condition_flags ace_condition_flags,
                                        const char *conditions,
                                        size_t *length,
                                        const char **msg,
@@ -495,6 +496,7 @@ static DATA_BLOB sddl_decode_conditions(TALLOC_CTX *mem_ctx,
        DATA_BLOB blob = {0};
        struct ace_condition_script *script = NULL;
        script = ace_conditions_compile_sddl(mem_ctx,
+                                            ace_condition_flags,
                                             conditions,
                                             msg,
                                             msg_offset,
@@ -518,6 +520,7 @@ static DATA_BLOB sddl_decode_conditions(TALLOC_CTX *mem_ctx,
 */
 static bool sddl_decode_ace(TALLOC_CTX *mem_ctx,
                            struct security_ace *ace,
+                           const enum ace_condition_flags ace_condition_flags,
                            char **sddl_copy,
                            struct sddl_transition_state *state,
                            const char **msg, size_t *msg_offset)
@@ -671,7 +674,12 @@ static bool sddl_decode_ace(TALLOC_CTX *mem_ctx,
                DATA_BLOB conditions = {0};
                s = tok[6];
 
-               conditions = sddl_decode_conditions(mem_ctx, s, &length, msg, msg_offset);
+               conditions = sddl_decode_conditions(mem_ctx,
+                                                   ace_condition_flags,
+                                                   s,
+                                                   &length,
+                                                   msg,
+                                                   msg_offset);
                if (conditions.data == NULL) {
                        DBG_WARNING("Conditional ACE compilation failure at %zu: %s\n",
                                    *msg_offset, *msg);
@@ -733,6 +741,7 @@ static const struct flag_map acl_flags[] = {
   decode an ACL
 */
 static struct security_acl *sddl_decode_acl(struct security_descriptor *sd,
+                                           const enum ace_condition_flags ace_condition_flags,
                                            const char **sddlp, uint32_t *flags,
                                            struct sddl_transition_state *state,
                                            const char **msg, size_t *msg_offset)
@@ -795,6 +804,7 @@ static struct security_acl *sddl_decode_acl(struct security_descriptor *sd,
                        return NULL;
                }
                ok = sddl_decode_ace(acl->aces, &acl->aces[acl->num_aces],
+                                    ace_condition_flags,
                                     &sddl_copy, state, msg, msg_offset);
                if (!ok) {
                        *msg_offset += sddl_copy - aces_start;
@@ -818,6 +828,7 @@ static struct security_acl *sddl_decode_acl(struct security_descriptor *sd,
  */
 struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char *sddl,
                                                const struct dom_sid *domain_sid,
+                                               const enum ace_condition_flags ace_condition_flags,
                                                const char **msg, size_t *msg_offset)
 {
        struct sddl_transition_state state = {
@@ -857,13 +868,13 @@ struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char
                switch (c) {
                case 'D':
                        if (sd->dacl != NULL) goto failed;
-                       sd->dacl = sddl_decode_acl(sd, &sddl, &flags, &state, msg, msg_offset);
+                       sd->dacl = sddl_decode_acl(sd, ace_condition_flags, &sddl, &flags, &state, msg, msg_offset);
                        if (sd->dacl == NULL) goto failed;
                        sd->type |= flags | SEC_DESC_DACL_PRESENT;
                        break;
                case 'S':
                        if (sd->sacl != NULL) goto failed;
-                       sd->sacl = sddl_decode_acl(sd, &sddl, &flags, &state, msg, msg_offset);
+                       sd->sacl = sddl_decode_acl(sd, ace_condition_flags, &sddl, &flags, &state, msg, msg_offset);
                        if (sd->sacl == NULL) goto failed;
                        /* this relies on the SEC_DESC_SACL_* flags being
                           1 bit shifted from the SEC_DESC_DACL_* flags */
@@ -909,8 +920,12 @@ struct security_descriptor *sddl_decode(TALLOC_CTX *mem_ctx, const char *sddl,
 {
        const char *msg = NULL;
        size_t msg_offset = 0;
-       struct security_descriptor *sd = sddl_decode_err_msg(mem_ctx, sddl, domain_sid,
-                                                            &msg, &msg_offset);
+       struct security_descriptor *sd = sddl_decode_err_msg(mem_ctx,
+                                                            sddl,
+                                                            domain_sid,
+                                                            ACE_CONDITION_FLAG_ALLOW_DEVICE,
+                                                            &msg,
+                                                            &msg_offset);
        DBG_NOTICE("could not decode '%s'\n", sddl);
        if (msg != NULL) {
                DBG_NOTICE("                  %*c\n", (int)msg_offset, '^');

diff --git a/libcli/security/sddl.h b/libcli/security/sddl.h
index d9693c0badcdcb81814670f18bc3b0971d5d7bb2..03c8a27924db16a7aecef00cc627084f7a0a72c8 100644 (file)
--- a/libcli/security/sddl.h
+++ b/libcli/security/sddl.h
@@ -24,12 +24,14 @@
 #include <talloc.h>
 #include "lib/util/data_blob.h"
 
+#include "librpc/gen_ndr/conditional_ace.h"
 #include "librpc/gen_ndr/security.h"
 
 struct security_descriptor *sddl_decode(TALLOC_CTX *mem_ctx, const char *sddl,
                                        const struct dom_sid *domain_sid);
 struct security_descriptor *sddl_decode_err_msg(TALLOC_CTX *mem_ctx, const char *sddl,
                                                const struct dom_sid *domain_sid,
+                                               const enum ace_condition_flags ace_condition_flags,
                                                const char **msg, size_t *msg_offset);
 char *sddl_encode(TALLOC_CTX *mem_ctx, const struct security_descriptor *sd,
                  const struct dom_sid *domain_sid);

diff --git a/libcli/security/sddl_conditional_ace.c b/libcli/security/sddl_conditional_ace.c
index c4061de265d2a6fd469bcd79c176cb4fafdc8a08..b5787f4a3ca154395b3e50f0bb8467d1ff6390bc 100644 (file)
--- a/libcli/security/sddl_conditional_ace.c
+++ b/libcli/security/sddl_conditional_ace.c
@@ -40,6 +40,8 @@
 #define SDDL_FLAG_EXPECTING_PAREN_LITERAL   128
 #define SDDL_FLAG_NOT_EXPECTING_END_PAREN   256
 
+#define SDDL_FLAG_DEVICE                    512
+
 #define SDDL_FLAG_IS_UNARY_OP               (1 << 20)
 #define SDDL_FLAG_IS_BINARY_OP              (1 << 21)
 
@@ -114,6 +116,7 @@ struct ace_condition_sddl_compiler_context {
        struct dom_sid *domain_sid;
        uint32_t state;
        uint8_t last_token_type;
+       bool allow_device;
 };
 
 struct sddl_data {
@@ -133,7 +136,7 @@ static const struct sddl_data sddl_strings[256] = {
        },
        [CONDITIONAL_ACE_TOKEN_DEVICE_MEMBER_OF] = {
                "Device_Member_of",
-               SDDL_FLAGS_MEMBER_OP,
+               SDDL_FLAGS_MEMBER_OP|SDDL_FLAG_DEVICE,
                SDDL_PRECEDENCE_COMMON,
                1
        },
@@ -146,7 +149,7 @@ static const struct sddl_data sddl_strings[256] = {
        },
        [CONDITIONAL_ACE_TOKEN_DEVICE_MEMBER_OF_ANY] = {
                "Device_Member_of_Any",
-               SDDL_FLAGS_MEMBER_OP,
+               SDDL_FLAGS_MEMBER_OP|SDDL_FLAG_DEVICE,
                SDDL_PRECEDENCE_COMMON,
                1
        },
@@ -158,7 +161,7 @@ static const struct sddl_data sddl_strings[256] = {
        },
        [CONDITIONAL_ACE_TOKEN_NOT_DEVICE_MEMBER_OF] = {
                "Not_Device_Member_of",
-               SDDL_FLAGS_MEMBER_OP,
+               SDDL_FLAGS_MEMBER_OP|SDDL_FLAG_DEVICE,
                SDDL_PRECEDENCE_COMMON,
                1
        },
@@ -170,7 +173,7 @@ static const struct sddl_data sddl_strings[256] = {
        },
        [CONDITIONAL_ACE_TOKEN_NOT_DEVICE_MEMBER_OF_ANY] = {
                "Not_Device_Member_of_Any",
-               SDDL_FLAGS_MEMBER_OP,
+               SDDL_FLAGS_MEMBER_OP|SDDL_FLAG_DEVICE,
                SDDL_PRECEDENCE_COMMON,
                1
        },
@@ -356,7 +359,7 @@ static const struct sddl_data sddl_strings[256] = {
        },
        [CONDITIONAL_ACE_DEVICE_ATTRIBUTE] = {
                "device attribute",
-               SDDL_FLAGS_ATTRIBUTE,
+               SDDL_FLAGS_ATTRIBUTE|SDDL_FLAG_DEVICE,
                SDDL_NOT_AN_OP,
                0
        },
@@ -2256,6 +2259,20 @@ static bool parse_word(struct ace_condition_sddl_compiler_context *comp)
                        size_t o = candidates[j];
                        if (sddl_strings[o].name[i] == '\0') {
                                /* it is this one */
+
+                               if (!comp->allow_device &&
+                                   (sddl_strings[o].flags & SDDL_FLAG_DEVICE))
+                               {
+                                       comp_error(
+                                               comp,
+                                               "a device‐relative expression "
+                                               "will never evaluate to true "
+                                               "in this context (did you "
+                                               "intend a user‐relative "
+                                               "expression?)");
+                                       return false;
+                               }
+
                                token.type = o;
                                token.data.sddl_op.start = comp->offset;
                                comp->offset += i;
@@ -2327,7 +2344,19 @@ static bool parse_attr2(struct ace_condition_sddl_compiler_context *comp)
                                  (const char *) (comp->sddl + comp->offset),
                                  attr_len);
                if (ret == 0) {
-                       token.type = sddl_attr_types[i].code;
+                       const uint8_t code = sddl_attr_types[i].code;
+
+                       if (!comp->allow_device &&
+                           (sddl_strings[code].flags & SDDL_FLAG_DEVICE))
+                       {
+                               comp_error(comp,
+                                          "a device attribute is not "
+                                          "applicable in this context (did "
+                                          "you intend a user attribute?)");
+                               return false;
+                       }
+
+                       token.type = code;
                        comp->offset += attr_len;
                        break;
                }
@@ -2676,6 +2705,7 @@ static bool parse_expression(struct ace_condition_sddl_compiler_context *comp)
 static bool init_compiler_context(
        TALLOC_CTX *mem_ctx,
        struct ace_condition_sddl_compiler_context *comp,
+       const enum ace_condition_flags ace_condition_flags,
        const char *sddl,
        size_t max_length,
        size_t max_stack)
@@ -2713,6 +2743,7 @@ static bool init_compiler_context(
        comp->target_len = &program->length;
        comp->length = strlen(sddl);
        comp->state =  SDDL_FLAG_EXPECTING_PAREN;
+       comp->allow_device = ace_condition_flags & ACE_CONDITION_FLAG_ALLOW_DEVICE;
        return true;
 }
 
@@ -2721,6 +2752,7 @@ static bool init_compiler_context(
  *
  * @param mem_ctx
  * @param sddl - the string to be parsed
+ * @param ace_condition_flags - flags controlling compiler behaviour
  * @param message - on error, a pointer to a compiler message
  * @param message_offset - where the error occurred
  * @param consumed_length - how much of the SDDL was used
@@ -2728,6 +2760,7 @@ static bool init_compiler_context(
  */
 struct ace_condition_script * ace_conditions_compile_sddl(
        TALLOC_CTX *mem_ctx,
+       const enum ace_condition_flags ace_condition_flags,
        const char *sddl,
        const char **message,
        size_t *message_offset,
@@ -2741,6 +2774,7 @@ struct ace_condition_script * ace_conditions_compile_sddl(
 
        ok = init_compiler_context(mem_ctx,
                                   &comp,
+                                  ace_condition_flags,
                                   sddl,
                                   CONDITIONAL_ACE_MAX_LENGTH,
                                   CONDITIONAL_ACE_MAX_TOKENS);
@@ -3026,7 +3060,12 @@ struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *sddl_decode_resource_attr (
        size_t len;
        struct ace_condition_unicode attr_name = {};
 
-       ok = init_compiler_context(mem_ctx, &comp, str, 3, 3);
+       ok = init_compiler_context(mem_ctx,
+                                  &comp,
+                                  ACE_CONDITION_FLAG_ALLOW_DEVICE,
+                                  str,
+                                  3,
+                                  3);
        if (!ok) {
                return NULL;
        }
@@ -3302,7 +3341,12 @@ struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *parse_sddl_literal_as_claim(
        struct CLAIM_SECURITY_ATTRIBUTE_RELATIVE_V1 *claim = NULL;
        struct ace_condition_sddl_compiler_context comp = {};
 
-       ok = init_compiler_context(mem_ctx, &comp, str, 2, 2);
+       ok = init_compiler_context(mem_ctx,
+                                  &comp,
+                                  ACE_CONDITION_FLAG_ALLOW_DEVICE,
+                                  str,
+                                  2,
+                                  2);
        if (!ok) {
                return NULL;
        }

diff --git a/libcli/security/tests/test_sddl_conditional_ace.c b/libcli/security/tests/test_sddl_conditional_ace.c
index 0fddf19810569579947687a4f678777030dbc0a8..3c976108ea21d3fba00ed4930844ec855e55889f 100644 (file)
--- a/libcli/security/tests/test_sddl_conditional_ace.c
+++ b/libcli/security/tests/test_sddl_conditional_ace.c
@@ -94,8 +94,12 @@ static void test_sddl_compile(void **state)
        DATA_BLOB compiled;
        size_t length;
 
-       s = ace_conditions_compile_sddl(mem_ctx, sddl, &message,
-                                       &message_offset, &length);
+       s = ace_conditions_compile_sddl(mem_ctx,
+                                       ACE_CONDITION_FLAG_ALLOW_DEVICE,
+                                       sddl,
+                                       &message,
+                                       &message_offset,
+                                       &length);
        if (message != NULL) {
                print_error_message(sddl, message, message_offset);
        }
@@ -130,8 +134,12 @@ static void test_sddl_compile2(void **state)
        DATA_BLOB compiled;
        size_t length;
 
-       s = ace_conditions_compile_sddl(mem_ctx, sddl, &message,
-                                       &message_offset, &length);
+       s = ace_conditions_compile_sddl(mem_ctx,
+                                       ACE_CONDITION_FLAG_ALLOW_DEVICE,
+                                       sddl,
+                                       &message,
+                                       &message_offset,
+                                       &length);
        if (message != NULL) {
                print_error_message(sddl, message, message_offset);
        }
@@ -624,6 +632,7 @@ static void test_round_trips(void **state)
                DATA_BLOB e1, e2, e3;
                fputs("=======================\n", stderr);
                s1 = ace_conditions_compile_sddl(mem_ctx,
+                                                ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                                 sddl[i],
                                                 &message,
                                                 &message_offset,
@@ -679,6 +688,7 @@ static void test_round_trips(void **state)
                }
                print_message("SDDL: %s\n", resddl1);
                s3 = ace_conditions_compile_sddl(mem_ctx,
+                                                ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                                 resddl1,
                                                 &message,
                                                 &message_offset,
@@ -728,6 +738,7 @@ static void test_a_number_of_valid_strings(void **state)
                size_t message_offset;
 
                s = ace_conditions_compile_sddl(mem_ctx,
+                                               ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                                sddl[i],
                                                &message,
                                                &message_offset,
@@ -803,6 +814,7 @@ static void test_a_number_of_invalid_strings(void **state)
                const char *message = NULL;
                size_t message_offset;
                s = ace_conditions_compile_sddl(mem_ctx,
+                                               ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                                sddl[i],
                                                &message,
                                                &message_offset,
@@ -847,6 +859,7 @@ static void test_valid_strings_with_trailing_crap(void **state)
                const char *message = NULL;
                size_t message_offset;
                s = ace_conditions_compile_sddl(mem_ctx,
+                                               ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                                pairs[i].sddl,
                                                &message,
                                                &message_offset,

diff --git a/librpc/idl/conditional_ace.idl b/librpc/idl/conditional_ace.idl
index 28c1b91545d7e865dfe76375aba30966789bc20b..e36fe9b43a13328f5ba16ce3a34c9f07f5869906 100644 (file)
--- a/librpc/idl/conditional_ace.idl
+++ b/librpc/idl/conditional_ace.idl
@@ -389,6 +389,10 @@ interface conditional_ace
                uint32 length;
        } ace_condition_script;
 
+       typedef enum {
+               ACE_CONDITION_FLAG_ALLOW_DEVICE = 0x01
+       } ace_condition_flags;
+
        /*
         * Flags for ace_condition_token.flags field.
         *

diff --git a/source4/librpc/ndr/py_security.c b/source4/librpc/ndr/py_security.c
index 5f185b69bed99a55b5635514c323162627f9945c..3fcea7a27fcee02f08161f8e44cb1e9360566666 100644 (file)
--- a/source4/librpc/ndr/py_security.c
+++ b/source4/librpc/ndr/py_security.c
@@ -299,6 +299,7 @@ static PyObject *py_descriptor_from_sddl(PyObject *self, PyObject *args)
        }
 
        secdesc = sddl_decode_err_msg(tmp_ctx, sddl, sid,
+                                     ACE_CONDITION_FLAG_ALLOW_DEVICE,
                                      &err_msg, &err_msg_offset);
        if (secdesc == NULL) {
                PyObject *exc = NULL;