From: Joseph Sutton Date: Fri, 3 Nov 2023 01:57:02 +0000 (+1300) Subject: libcli/security: Optionally disallow device‐specific attributes and operators where... X-Git-Tag: talloc-2.4.2~789 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=935f4edd81f8115c390daa8f35c35dda64e99cfb;p=thirdparty%2Fsamba.git libcli/security: Optionally disallow device‐specific attributes and operators where they are not applicable Signed-off-by: Joseph Sutton Reviewed-by: Andrew Bartlett --- diff --git a/lib/fuzzing/fuzz_conditional_ace_blob.c b/lib/fuzzing/fuzz_conditional_ace_blob.c index aed1cd37c73..70bb5723c51 100644 --- 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 e21c2ec9b12..636ebf1da9e 100644 --- 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 f1204914b01..e5920567934 100644 --- 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 5d481749a9b..97e579cfe32 100644 --- 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 d9693c0badc..03c8a27924d 100644 --- a/libcli/security/sddl.h +++ b/libcli/security/sddl.h @@ -24,12 +24,14 @@ #include #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 c4061de265d..b5787f4a3ca 100644 --- 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 0fddf198105..3c976108ea2 100644 --- 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 28c1b91545d..e36fe9b43a1 100644 --- 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 5f185b69bed..3fcea7a27fc 100644 --- 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;