[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Mon Nov 27 02:11:01 UTC 2023


The branch, master has been updated
       via  14b17c3de6d libcli/security/tests: gunzip the oversized-acls test vectors
       via  19129660dfe libcli/security/tests: remove duplicate TX-integer tests from oversized-ACLs
       via  cf295c94aad libcli/security:sddl: remove vestiges of shared conditional/resource ACE SID parsing
       via  20ffec711d6 libcli/security: improve error messages in RA ACE SDDL
       via  dc60891de9f libcli/security: conditional ACE sid parser no longer expects RA ACEs
       via  0a2e335e378 libcli/security: parse resource attribute ace SIDs separately
       via  79292c8d1ef libcli/security/sddl: write RA octet strings the Windows way
       via  38e7b4dcbdb libcli/security: add a parser for resource attribute ACE byte strings
       via  9ef71399cee libcli/security: sddl_conditional_ace: remove check_resource_attr_type()
       via  059610a62e5 libcli/security: sddl_conditional_ace: add parse_bool for RA aces
       via  800f770e111 libcli/security: sddl_conditional_ace: add parse_uint for RA aces
       via  33caae43812 libcli/security: un-invert parse_resource_attr_list, check type first
       via  cda9371b59c libcli/security/test_sddl_conditional_ace: adjust RA octet parse tests
       via  4ab9cb19074 libcli/security:sddl_condtional_ace: log compiler errors at some debug levels
       via  f18ffd11829 libcli/security: initialise conditional ACE token flags
       via  8e3be66a496 pytest: security_descriptors tests get enumerator in name
       via  5e925f9755f dosmode: prefer capabilities over become_root
       via  1dd81928a2f libgpo: fix wrong lineending in admx files
      from  f5c76c3c814 Revert "README.Coding.md: add DBG_STARTUP_NOTICE macro"

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 14b17c3de6d478d1c86d23996f9c0acb7f2c07e1
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri Nov 24 17:15:55 2023 +1300

    libcli/security/tests: gunzip the oversized-acls test vectors
    
    These are just as readable with `less` as they were with `zless`.
    
    This file has been slightly manually edited to add line-breaks. There
    is not an easy setting in Python's json module to get good formatting.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>
    
    Autobuild-User(master): Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date(master): Mon Nov 27 02:10:12 UTC 2023 on atb-devel-224

commit 19129660dfe7312585b057a90b51ad9405661478
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri Nov 24 16:59:05 2023 +1300

    libcli/security/tests: remove duplicate TX-integer tests from oversized-ACLs
    
    We had two sets of test vectors (Windows ground-truth for SDDL
    compilation) that got mixed up.
    
    The "oversized ACLs" set is ACLs that contain repeated ACEs, like
    "D:P(D;;;;;MP)(D;;;;;MP)" -- Windows will assign a size to the ACL
    that is greater than the sum of the ACEs, while Samba will not (in
    part because we don't actually store a size for the ACL, instead
    calculating it on the fly from the size of the ACEs).
    
    The "TX integers" set is for resource attribute ACEs with octet-string
    data that contains pure integers (lacking '#' characters) in their
    SDDL, like «(RA;;;;;WD;("bar",TX,0x0,0077,00,0077,00))». We used to
    think that was weird, and that RA-TX ACEs should contain octet-strings
    in the conditional ACE style. But now we have realised it's not weird,
    it's normal, and we have fixed our handling of these ACEs.
    
    As a result of this mix-up, some of the tests labelled as "oversized
    ACLs" started passing when we fixed the TX integer problem, and that
    was confusing. All of the removed tests are already on the TX integer
    set -- the removed ones were duplicates.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit cf295c94aad9e2bafad398a338669f90d605fb5f
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Nov 2 15:48:20 2023 +1300

    libcli/security:sddl: remove vestiges of shared conditional/resource ACE SID parsing
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 20ffec711d60ba0d48e7677fddeb0886b9468139
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Nov 2 15:42:11 2023 +1300

    libcli/security: improve error messages in RA ACE SDDL
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit dc60891de9f1d4341b38e71c630c2fd70f900f11
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Nov 2 15:41:33 2023 +1300

    libcli/security: conditional ACE sid parser no longer expects RA ACEs
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 0a2e335e378a985d08d74fb1935d0e852480ee3c
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Nov 2 15:37:33 2023 +1300

    libcli/security: parse resource attribute ace SIDs separately
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 79292c8d1ef7b549099959eac8914a1c0a475c81
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri Nov 24 16:24:00 2023 +1300

    libcli/security/sddl: write RA octet strings the Windows way
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 38e7b4dcbdb18272717112cc947c7628109a35cf
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Nov 2 15:28:15 2023 +1300

    libcli/security: add a parser for resource attribute ACE byte strings
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 9ef71399cee31c56c3b390f5d53be930f290f6eb
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Wed Nov 22 13:17:17 2023 +1300

    libcli/security: sddl_conditional_ace: remove check_resource_attr_type()
    
    This is unneeded, as now all the checks are done in the relevant
    parse_* functions.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 059610a62e5290d259e43312dfacc2ab74698a15
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Wed Nov 22 13:24:21 2023 +1300

    libcli/security: sddl_conditional_ace: add parse_bool for RA aces
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 800f770e1112082067da975fe14db83b6ef437b4
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Wed Nov 22 13:23:26 2023 +1300

    libcli/security: sddl_conditional_ace: add parse_uint for RA aces
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 33caae438125f4a4a99dd6dc0f048be2f17e4863
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Nov 2 15:25:06 2023 +1300

    libcli/security: un-invert parse_resource_attr_list, check type first
    
    We were reusing parse_literal() because it almost does what we need,
    but it is different enough that check_resource_attr_type() is large
    and complicated, and can't handle all the cases (in particular octet-
    strings and SIDs are different in resource ACEs).
    
    This way is better because we know the type in advance, so we can use
    that to choose the parser, which will help with octet-strings that are
    only digits.
    
    In this commit we're leaving the check there, but it soon won't do
    anything that the parse_* functions don't, and we will remove it.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit cda9371b59c2692a27aa1bbc848583f44bd58322
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri Nov 10 11:35:42 2023 +1300

    libcli/security/test_sddl_conditional_ace: adjust RA octet parse tests
    
    We are going to parse octet strings like Windows (as opposed to like
    Windows docs), so the tests need changing.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 4ab9cb1907461f529feb010b9d77968d6c611802
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Nov 2 15:11:58 2023 +1300

    libcli/security:sddl_condtional_ace: log compiler errors at some debug levels
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit f18ffd11829d2e666348a4932481fd7a267b67be
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri Nov 17 16:34:23 2023 +1300

    libcli/security: initialise conditional ACE token flags
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 8e3be66a4967461b550c9372e0ea38cd6d479335
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Mon Nov 20 11:45:32 2023 +1300

    pytest: security_descriptors tests get enumerator in name
    
    This will make knownfails easier, given the names contain so many
    regular expression metacharacters.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 5e925f9755fad180863861157aa7548d83dd3fde
Author: Björn Jacke <bj at sernet.de>
Date:   Mon Nov 20 12:36:00 2023 +0100

    dosmode: prefer capabilities over become_root
    
    Signed-off-by: Bjoern Jacke <bjacke at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 1dd81928a2f83ea55d70fc9478969303a8b76c1a
Author: Björn Jacke <bj at sernet.de>
Date:   Wed Nov 22 18:46:30 2023 +0100

    libgpo: fix wrong lineending in admx files
    
    When changing or reviewing admx file patches, make sure, that those files are
    dos fileformat and they need to have the magic ^M at the end of each line ...
    
    Signed-off-by: Bjoern Jacke <bjacke at samba.org>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 libcli/security/conditional_ace.c                 |   1 +
 libcli/security/sddl_conditional_ace.c            | 405 +++++++++++++---------
 libcli/security/tests/data/oversize-acls.json     |  20 ++
 libcli/security/tests/data/oversize-acls.json.gz  | Bin 2676 -> 0 bytes
 libcli/security/tests/test_run_conditional_ace.c  |   2 +-
 libcli/security/tests/test_sddl_conditional_ace.c |   8 +-
 libgpo/admx/en-US/samba.adml                      |  22 +-
 libgpo/admx/ru-RU/samba.adml                      |   2 +-
 python/samba/tests/security_descriptors.py        |   7 +-
 selftest/knownfail.d/security-descriptors         |   4 +-
 source3/smbd/dosmode.c                            |   8 +-
 11 files changed, 296 insertions(+), 183 deletions(-)
 create mode 100644 libcli/security/tests/data/oversize-acls.json
 delete mode 100644 libcli/security/tests/data/oversize-acls.json.gz


Changeset truncated at 500 lines:

diff --git a/libcli/security/conditional_ace.c b/libcli/security/conditional_ace.c
index e4e83856451..5d945c0b0bb 100644
--- a/libcli/security/conditional_ace.c
+++ b/libcli/security/conditional_ace.c
@@ -580,6 +580,7 @@ struct ace_condition_script *parse_conditional_ace(TALLOC_CTX *mem_ctx,
 		size_t available;
 		bool ok;
 		tok->type = data.data[i];
+		tok->flags = 0;
 		i++;
 		tok_data = data.data + i;
 		available = data.length - i;
diff --git a/libcli/security/sddl_conditional_ace.c b/libcli/security/sddl_conditional_ace.c
index b5787f4a3ca..f98daba7bbc 100644
--- a/libcli/security/sddl_conditional_ace.c
+++ b/libcli/security/sddl_conditional_ace.c
@@ -46,12 +46,6 @@
 #define SDDL_FLAG_IS_BINARY_OP              (1 << 21)
 
 
-/*
- * A resource attribute ACE has a slightly different syntax for SIDs.
- */
-#define SDDL_FLAG_IS_RESOURCE_ATTR_ACE      (1 << 30)
-
-
 #define SDDL_FLAGS_EXPR_START (SDDL_FLAG_EXPECTING_UNARY_OP | \
 			       SDDL_FLAG_EXPECTING_LOCAL_ATTR | \
 			       SDDL_FLAG_EXPECTING_NON_LOCAL_ATTR | \
@@ -876,6 +870,22 @@ static bool sddl_write_octet_string(struct sddl_write_context *ctx,
 	return ok;
 }
 
+/*
+ * For octet strings, the Resource attribute ACE SDDL differs from conditional
+ * ACE SDDL, lacking the leading '#'.
+ */
+static bool sddl_write_ra_octet_string(struct sddl_write_context *ctx,
+				       const struct ace_condition_token *tok)
+{
+	bool ok;
+	char *hex  = hex_encode_talloc(ctx->mem_ctx,
+				       tok->data.bytes.data,
+				       tok->data.bytes.length);
+	ok = sddl_write(ctx, hex);
+	talloc_free(hex);
+	return ok;
+}
+
 
 static bool sddl_write_sid(struct sddl_write_context *ctx,
 			   struct ace_condition_token *tok)
@@ -1294,10 +1304,12 @@ static void comp_error(struct ace_condition_sddl_compiler_context *comp,
 	if (comp->message == NULL) {
 		goto fail;
 	}
+	DBG_NOTICE("%s\n", comp->message);
 	return;
 fail:
 	comp->message = talloc_strdup(comp->mem_ctx,
 				      "failed to set error message");
+	DBG_WARNING("%s\n", comp->message);
 }
 
 
@@ -1935,49 +1947,108 @@ static bool parse_octet_string(struct ace_condition_sddl_compiler_context *comp)
 }
 
 
-static bool parse_sid(struct ace_condition_sddl_compiler_context *comp)
+static bool parse_ra_octet_string(struct ace_condition_sddl_compiler_context *comp)
 {
-	struct dom_sid *sid = NULL;
-	const uint8_t *sidstr = NULL;
+	/*
+	 * Resource attribute octet strings resemble conditional ace octet
+	 * strings, but have some important differences:
+	 *
+	 * 1. The '#' at the start is optional, and if present is
+	 * counted as a zero.
+	 *
+	 * 2. An odd number of characters is implicitly left-padded with a zero.
+	 *
+	 * That is, "abc" means "0abc", "#12" means "0012", "f##"
+	 * means "0f00", and "##" means 00.
+	 */
 	struct ace_condition_token token = {};
-	size_t end;
-	bool expecting_bare_sids =
-		comp->state & SDDL_FLAG_IS_RESOURCE_ATTR_ACE ? true : false;
+	size_t string_length, bytes_length, i, j;
+	bool ok;
+	char pair[2];
 
-	if ((comp->state & SDDL_FLAG_EXPECTING_LITERAL) == 0) {
-		comp_error(comp, "did not expect a SID here");
+	string_length = strspn((const char*)(comp->sddl + comp->offset),
+			"#0123456789abcdefABCDEF");
+
+	bytes_length = (string_length + 1) / 2;
+
+	if (bytes_length == 0) {
+		comp_error(comp, "zero length octet bytes");
+		return false;
+	}
+
+	token.data.bytes = data_blob_talloc_zero(comp->mem_ctx, bytes_length);
+	if (token.data.bytes.data == NULL) {
 		return false;
 	}
-	if (expecting_bare_sids) {
+	token.type = CONDITIONAL_ACE_TOKEN_OCTET_STRING;
+
+	j = comp->offset;
+	i = 0;
+	if (string_length & 1) {
 		/*
-		 *  This flag is set for a resource ACE which doesn't have the
-		 *  SID() wrapper around the SID string, and not for a
-		 *  conditional ACE, which must have the "SID(...)".
-		 *
-		 * The resource ACE doesn't need this because there is no
-		 * ambiguity with local attribute names, besides which the
-		 * type has already been specified earlier in the ACE.
+		 * An odd number of characters means the first
+		 * character gains an implicit 0 for the high nybble.
 		 */
-		if (comp->length - comp->offset < 2){
-			comp_error(comp, "no room for a complete SID");
-			return false;
+		pair[0] = 0;
+		pair[1] = (comp->sddl[0] == '#') ? '0' : comp->sddl[0];
+
+		ok = hex_byte(pair, &token.data.bytes.data[i]);
+		if (!ok) {
+			goto fail;
 		}
-	} else {
-		if (comp->length - comp->offset < 7){
-			/* minimum: "SID(AA)" */
-			comp_error(comp, "no room for a complete SID");
-			return false;
+		j++;
+		i++;
+	}
+
+	for (; i < bytes_length; i++) {
+		/*
+		 * Why not just strhex_to_str() ?
+		 *
+		 * Because we need to treat '#' as '0' in octet string values.
+		 */
+		if (comp->length - j < 2) {
+			goto fail;
 		}
-		/* conditional ACE SID string */
-		if (comp->sddl[comp->offset    ] != 'S' ||
-		    comp->sddl[comp->offset + 1] != 'I' ||
-		    comp->sddl[comp->offset + 2] != 'D' ||
-		    comp->sddl[comp->offset + 3] != '(') {
-			comp_error(comp, "malformed SID() constructor");
-			return false;
-		} else {
-			comp->offset += 4;
+
+		pair[0] = (comp->sddl[j]     == '#') ? '0' : comp->sddl[j];
+		pair[1] = (comp->sddl[j + 1] == '#') ? '0' : comp->sddl[j + 1];
+
+		ok = hex_byte(pair, &token.data.bytes.data[i]);
+		if (!ok) {
+			goto fail;
 		}
+		j += 2;
+	}
+	comp->offset = j;
+	return write_sddl_token(comp, token);
+
+fail:
+	comp_error(comp, "inexplicable error in octet string");
+	talloc_free(token.data.bytes.data);
+	return false;
+}
+
+
+static bool parse_sid(struct ace_condition_sddl_compiler_context *comp)
+{
+	struct dom_sid *sid = NULL;
+	const uint8_t *sidstr = NULL;
+	struct ace_condition_token token = {};
+	size_t end;
+	if (comp->length - comp->offset < 7) {
+		/* minimum: "SID(AA)" */
+		comp_error(comp, "no room for a complete SID");
+		return false;
+	}
+	/* conditional ACE SID string */
+	if (comp->sddl[comp->offset    ] != 'S' ||
+	    comp->sddl[comp->offset + 1] != 'I' ||
+	    comp->sddl[comp->offset + 2] != 'D' ||
+	    comp->sddl[comp->offset + 3] != '(') {
+		comp_error(comp, "malformed SID() constructor");
+		return false;
+	} else {
+		comp->offset += 4;
 	}
 
 	sidstr = comp->sddl + comp->offset;
@@ -1996,19 +2067,64 @@ static bool parse_sid(struct ace_condition_sddl_compiler_context *comp)
 		return false;
 	}
 	comp->offset = end;
-	if (expecting_bare_sids) {
-		/* no trailing ')' in a resource attribute ACE */
-	} else {
-		/*
-		 * offset is now at the end of the SID, but we need to account
-		 * for the ')'.
-		 */
-		if (comp->sddl[comp->offset] != ')') {
-			comp_error(comp, "expected ')' to follow SID");
-			return false;
-		}
-		comp->offset++;
+	/*
+	 * offset is now at the end of the SID, but we need to account
+	 * for the ')'.
+	 */
+	if (comp->sddl[comp->offset] != ')') {
+		comp_error(comp, "expected ')' to follow SID");
+		return false;
+	}
+	comp->offset++;
+
+	token.type = CONDITIONAL_ACE_TOKEN_SID;
+	token.data.sid.sid = *sid;
+	return write_sddl_token(comp, token);
+}
+
+
+
+static bool parse_ra_sid(struct ace_condition_sddl_compiler_context *comp)
+{
+	struct dom_sid *sid = NULL;
+	const uint8_t *sidstr = NULL;
+	struct ace_condition_token token = {};
+	size_t end;
+
+	if ((comp->state & SDDL_FLAG_EXPECTING_LITERAL) == 0) {
+		comp_error(comp, "did not expect a SID here");
+		return false;
 	}
+	/*
+	 *  Here we are parsing a resource attribute ACE which doesn't
+	 *  have the SID() wrapper around the SID string (unlike a
+	 *  conditional ACE).
+	 *
+	 * The resource ACE doesn't need this because there is no
+	 * ambiguity with local attribute names, besides which the
+	 * type has already been specified earlier in the ACE.
+	 */
+	if (comp->length - comp->offset < 2){
+		comp_error(comp, "no room for a complete SID");
+		return false;
+	}
+
+	sidstr = comp->sddl + comp->offset;
+
+	sid = sddl_decode_sid(comp->mem_ctx,
+			      (const char **)&sidstr,
+			      comp->domain_sid);
+
+	if (sid == NULL) {
+		comp_error(comp, "could not parse SID");
+		return false;
+	}
+	end = sidstr - comp->sddl;
+	if (end >= comp->length || end < comp->offset) {
+		comp_error(comp, "apparent overflow in SID parsing");
+		return false;
+	}
+	comp->offset = end;
 	token.type = CONDITIONAL_ACE_TOKEN_SID;
 	token.data.sid.sid = *sid;
 	return write_sddl_token(comp, token);
@@ -2083,6 +2199,53 @@ static bool parse_int(struct ace_condition_sddl_compiler_context *comp)
 }
 
 
+static bool parse_uint(struct ace_condition_sddl_compiler_context *comp)
+{
+	struct ace_condition_token *tok = NULL;
+	bool ok = parse_int(comp);
+	if (ok == false) {
+		return false;
+	}
+	/*
+	 * check that the token's value is positive.
+	 */
+	if (comp->target_len == 0) {
+		return false;
+	}
+	tok = &comp->target[*comp->target_len - 1];
+	if (tok->type != CONDITIONAL_ACE_TOKEN_INT64) {
+		return false;
+	}
+	if (tok->data.int64.value < 0) {
+		comp_error(comp, "invalid resource ACE value for unsigned TU claim");
+		return false;
+	}
+	return true;
+}
+
+
+static bool parse_bool(struct ace_condition_sddl_compiler_context *comp)
+{
+	struct ace_condition_token *tok = NULL;
+	bool ok = parse_int(comp);
+	if (ok == false || comp->target_len == 0) {
+		return false;
+	}
+	/*
+	 * check that the token is 0 or 1.
+	 */
+	tok = &comp->target[*comp->target_len - 1];
+	if (tok->type != CONDITIONAL_ACE_TOKEN_INT64) {
+		return false;
+	}
+	if (tok->data.int64.value != 0 && tok->data.int64.value != 1) {
+		comp_error(comp, "invalid resource ACE Boolean value");
+		return false;
+	}
+	return true;
+}
+
+
 static bool could_be_an_int(struct ace_condition_sddl_compiler_context *comp)
 {
 	const char *start = (const char*)(comp->sddl + comp->offset);
@@ -2412,10 +2575,6 @@ static bool parse_literal(struct ace_condition_sddl_compiler_context *comp,
 		if (strchr("1234567890-+", c) != NULL) {
 			return parse_int(comp);
 		}
-		if ((comp->state & SDDL_FLAG_IS_RESOURCE_ATTR_ACE) &&
-		    isupper(c)) {
-			return parse_sid(comp);
-		}
 	}
 	if (c > 31 && c < 127) {
 		comp_error(comp,
@@ -2805,94 +2964,6 @@ struct ace_condition_script * ace_conditions_compile_sddl(
 
 
 
-static bool check_resource_attr_type(struct ace_condition_token *tok, char c)
-{
-	/*
-	 * Check that a token matches the expected resource ace type (TU, TS,
-	 * etc).
-	 *
-	 * We're sticking to the [IUSDXB] codes rather than using converting
-	 * earlier to tok->type (whereby this whole thing becomes "if (tok->type
-	 * == type)") to enable bounds checks on the various integer types.
-	 */
-	switch(c) {
-	case 'I':
-		/* signed int */
-		if (tok->type != CONDITIONAL_ACE_TOKEN_INT64) {
-			goto wrong_type;
-		}
-		return true;
-	case 'U':
-		/* unsigned int, let's check the range */
-		if (tok->type != CONDITIONAL_ACE_TOKEN_INT64) {
-			goto wrong_type;
-		}
-		if (tok->data.int64.value < 0) {
-			DBG_WARNING(
-				"invalid resource ACE value for unsigned TU\n");
-			goto error;
-		}
-		return true;
-	case 'S':
-		/* unicode string */
-		if (tok->type != CONDITIONAL_ACE_TOKEN_UNICODE) {
-			goto wrong_type;
-		}
-		return true;
-	case 'D':
-		/* SID */
-		if (tok->type != CONDITIONAL_ACE_TOKEN_SID) {
-			goto wrong_type;
-		}
-		return true;
-	case 'X':
-		/* Octet string */
-		if (tok->type != CONDITIONAL_ACE_TOKEN_OCTET_STRING) {
-			if (tok->type == CONDITIONAL_ACE_TOKEN_INT64)  {
-				/*
-				 * Windows 2022 will also accept even
-				 * numbers of digits, like "1234"
-				 * instead of "#1234". Samba does not.
-				 *
-				 * Fixing this is complicated by the
-				 * fact that a leading '0' will have
-				 * cast the integer to octal, while an
-				 * A-F character will have caused it
-				 * to not parse as a literal at all.
-				 *
-				 * This behaviour is not mentioned in
-				 * MS-DTYP or elsewhere.
-				 */
-				DBG_WARNING("Octet sequence uses bare digits, "
-					    "please prefix a '#'\n");
-			}
-			goto wrong_type;
-		}
-		return true;
-	case 'B':
-		/* Boolean, meaning an int that is 0 or 1 */
-		if (tok->type != CONDITIONAL_ACE_TOKEN_INT64) {
-			goto wrong_type;
-		}
-		if (tok->data.int64.value != 0 &&
-		    tok->data.int64.value != 1) {
-			DBG_WARNING("invalid resource ACE value for boolean TB "
-				    "(should be 0 or 1).\n");
-			goto error;
-		}
-		return true;
-	default:
-		DBG_WARNING("Unknown resource ACE type T%c\n", c);
-		goto error;
-	};
-  wrong_type:
-	DBG_WARNING("resource ace type T%c doesn't match value\n", c);
-  error:
-	return false;
-}
-
-
-
 static bool parse_resource_attr_list(
 	struct ace_condition_sddl_compiler_context *comp,
 	char attr_type_char)
@@ -2917,8 +2988,7 @@ static bool parse_resource_attr_list(
 	struct ace_condition_token *old_target = comp->target;
 	uint32_t *old_target_len = comp->target_len;
 
-	comp->state = (SDDL_FLAG_EXPECTING_LITERAL |
-		       SDDL_FLAG_IS_RESOURCE_ATTR_ACE);
+	comp->state = SDDL_FLAG_EXPECTING_LITERAL;
 
 	/*
 	 * the worst case is one token for every two bytes: {1,1,1}, and we
@@ -2964,7 +3034,8 @@ static bool parse_resource_attr_list(
 		if (!first) {
 			if (c != ',') {
 				comp_error(comp,
-					   "malformed composite (expected comma)");
+					   "malformed resource attribute ACE "
+					   "(expected comma)");
 				goto fail;
 			}
 			comp->offset++;
@@ -2977,24 +3048,42 @@ static bool parse_resource_attr_list(
 		first = false;
 		if (*comp->target_len >= alloc_size) {
 			comp_error(comp,
-				   "Too many tokens in composite "
+				   "Too many tokens in resource attribute ACE "
 				   "(>= %"PRIu32" tokens)",
 				   *comp->target_len);
 			goto fail;
 		}
-		ok = parse_literal(comp, true);
-		if (!ok) {
+		switch(attr_type_char) {
+		case 'X':
+			ok = parse_ra_octet_string(comp);
+			break;
+		case 'S':
+			ok = parse_unicode(comp);
+			break;
+		case 'U':
+			ok = parse_uint(comp);
+			break;
+		case 'B':
+			ok = parse_bool(comp);
+			break;
+		case 'I':
+			ok = parse_int(comp);
+			break;
+		case 'D':
+			ok = parse_ra_sid(comp);
+			break;
+		default:
+			/* it's a mystery we got this far */
+			comp_error(comp,
+				   "unknown attribute type T%c",
+				   attr_type_char);


-- 
Samba Shared Repository



More information about the samba-cvs mailing list