r3885: Add security descriptor comparison to our RPC-SAMSYNC test. We now

verify that the security descriptor found in the SamSync is the same
as what is available over SAMR.

Unfortunately, the administrator seems unable to retrieve the SACL on
the security descriptor, so I've added a new function to compare with
a mask.

Andrew Bartlett

diff --git a/source/libcli/security/security_descriptor.c b/source/libcli/security/security_descriptor.c
index 5ed5ef5c76cac5352f5fd6962c39f7a79843e27e..a4056e5e71284996b76b1c3b62a255248873f90f 100644 (file)
--- a/source/libcli/security/security_descriptor.c
+++ b/source/libcli/security/security_descriptor.c
@@ -224,3 +224,24 @@ BOOL security_descriptor_equal(const struct security_descriptor *sd1,
 
        return True;    
 }
+
+/*
+  compare two security descriptors, but allow certain (missing) parts
+  to be masked out of the comparison
+*/
+BOOL security_descriptor_mask_equal(const struct security_descriptor *sd1, 
+                                   const struct security_descriptor *sd2, 
+                                   uint32 mask)
+{
+       if (sd1 == sd2) return True;
+       if (!sd1 || !sd2) return False;
+       if (sd1->revision != sd2->revision) return False;
+       if ((sd1->type & mask) != (sd2->type & mask)) return False;
+
+       if (!dom_sid_equal(sd1->owner_sid, sd2->owner_sid)) return False;
+       if (!dom_sid_equal(sd1->group_sid, sd2->group_sid)) return False;
+       if ((mask & SEC_DESC_DACL_PRESENT) && !security_acl_equal(sd1->dacl, sd2->dacl))      return False;
+       if ((mask & SEC_DESC_SACL_PRESENT) && !security_acl_equal(sd1->sacl, sd2->sacl))      return False;
+
+       return True;    
+}

diff --git a/source/librpc/idl/lsa.idl b/source/librpc/idl/lsa.idl
index d0bf2ad3e12568aae4895cb15d14f81957b6cc1e..225979da18d007030f6f3f7293d6f6d3fb7a1ac3 100644 (file)
--- a/source/librpc/idl/lsa.idl
+++ b/source/librpc/idl/lsa.idl
@@ -56,11 +56,6 @@
        /******************/
        /* Function: 0x03 */
 
-       typedef [public] struct {
-               uint32 size;
-               [subcontext(4)] security_descriptor *sd;
-       } sec_desc_buf;
-
        NTSTATUS lsa_QuerySecObj (
                [in,ref]     policy_handle *handle,
                [in]         uint32 sec_info,

diff --git a/source/librpc/idl/samr.idl b/source/librpc/idl/samr.idl
index 358218c2a0885d03290d06e4531b001f4287e731..58e0601606f2783d06654a6fd277bbca4bddb548 100644 (file)
--- a/source/librpc/idl/samr.idl
+++ b/source/librpc/idl/samr.idl
@@ -48,15 +48,10 @@
        /******************/
        /* Function: 0x02 */
 
-       typedef struct {
-               [range(0,0x40000),value(ndr_size_security_descriptor(r->sd))] uint32 sd_size;
-               [subcontext(4)] security_descriptor *sd;
-       } samr_SdBuf;
-
        NTSTATUS samr_SetSecurity (
                [in,ref]          policy_handle *handle,
                [in]              uint32 sec_info,
-               [in,ref]          samr_SdBuf *sdbuf
+               [in,ref]          sec_desc_buf *sdbuf
                );
 
        /******************/
@@ -65,7 +60,7 @@
        NTSTATUS samr_QuerySecurity (
                [in,ref]          policy_handle *handle,
                [in]              uint32 sec_info,
-               [out]             samr_SdBuf *sdbuf
+               [out]             sec_desc_buf *sdbuf
                );
 
        /******************/

diff --git a/source/librpc/idl/security.idl b/source/librpc/idl/security.idl
index 201868e531a2eb02a76e91f02f7ef0a50a3733f5..9625153ec1113b6562b8b9a92ad3bc381b95e0d6 100644 (file)
--- a/source/librpc/idl/security.idl
+++ b/source/librpc/idl/security.idl
@@ -120,6 +120,11 @@ interface security
                [relative] security_acl *dacl; /* user (discretionary) ACL */
        } security_descriptor;
 
+       typedef [public] struct {
+               [range(0,0x40000),value(ndr_size_security_descriptor(r->sd))] uint32 sd_size;
+               [subcontext(4)] security_descriptor *sd;
+       } sec_desc_buf;
+
        typedef [public,printonly] struct {
                /* TODO */
                uint32 flags;

diff --git a/source/rpc_server/samr/dcesrv_samr.c b/source/rpc_server/samr/dcesrv_samr.c
index ef6e45cb1eab464cb75d4fe7d3b4ee253c8c233c..05031bdfcb86ddc2cda8f23d82abdc9dfa711ed6 100644 (file)
--- a/source/rpc_server/samr/dcesrv_samr.c
+++ b/source/rpc_server/samr/dcesrv_samr.c
@@ -118,13 +118,13 @@ static NTSTATUS samr_QuerySecurity(struct dcesrv_call_state *dce_call, TALLOC_CT
                                   struct samr_QuerySecurity *r)
 {
        struct dcesrv_handle *h;
-       struct samr_SdBuf *sd;
+       struct sec_desc_buf *sd;
 
        r->out.sdbuf = NULL;
 
        DCESRV_PULL_HANDLE(h, r->in.handle, DCESRV_HANDLE_ANY);
 
-       sd = talloc_p(mem_ctx, struct samr_SdBuf);
+       sd = talloc_p(mem_ctx, struct sec_desc_buf);
        if (sd == NULL) {
                return NT_STATUS_NO_MEMORY;
        }

diff --git a/source/torture/rpc/samsync.c b/source/torture/rpc/samsync.c
index 7f9cf321dcc77b807ef1d699731b4ceb35bf943b..be56e4593eb2e9777a693125ed30395a5a2812a8 100644 (file)
--- a/source/torture/rpc/samsync.c
+++ b/source/torture/rpc/samsync.c
@@ -164,6 +164,24 @@ static struct policy_handle *samsync_open_domain(TALLOC_CTX *mem_ctx,
        return domain_handle;
 }
 
+static struct sec_desc_buf *samsync_query_sec_desc(TALLOC_CTX *mem_ctx, 
+                                                  struct samsync_state *samsync_state, 
+                                                  struct policy_handle *handle) 
+{
+       struct samr_QuerySecurity r;
+       NTSTATUS status;
+
+       r.in.handle = handle;
+       r.in.sec_info = 0x7;
+
+       status = dcerpc_samr_QuerySecurity(samsync_state->p_samr, mem_ctx, &r);
+       if (!NT_STATUS_IS_OK(status)) {
+               printf("QuerySecurity failed - %s\n", nt_errstr(status));
+               return NULL;
+       }
+
+       return r.out.sdbuf;
+}
 
 #define TEST_UINT64_EQUAL(i1, i2) do {\
        if (i1 != i2) {\
@@ -187,6 +205,7 @@ static struct policy_handle *samsync_open_domain(TALLOC_CTX *mem_ctx,
              ret = False;\
        } \
 } while (0)
+
 #define TEST_STRING_EQUAL(s1, s2) do {\
        if (!((!s1.string || s1.string[0]=='\0') && (!s2.string || s2.string[0]=='\0')) \
            && strcmp_safe(s1.string, s2.string) != 0) {\
@@ -196,6 +215,25 @@ static struct policy_handle *samsync_open_domain(TALLOC_CTX *mem_ctx,
        } \
 } while (0)
 
+/* The ~SEC_DESC_SACL_PRESENT is because we don't, as administrator,
+ * get back the SACL part of the SD when we ask over SAMR */
+
+#define TEST_SEC_DESC_EQUAL(sd1, handle) do {\
+        struct sec_desc_buf *sdbuf = samsync_query_sec_desc(mem_ctx, samsync_state, \
+                                                           handle); \
+       if (!sdbuf || !sdbuf->sd) { \
+               ret = False; \
+        } else {\
+               if (!security_descriptor_mask_equal(sd1.sd, sdbuf->sd, \
+                           ~SEC_DESC_SACL_PRESENT)) {\
+                       printf("Security Descriptor Mismatch for %s:\n", #sd1);\
+                       ndr_print_debug((ndr_print_fn_t)ndr_print_security_descriptor, "SamSync", sd1.sd);\
+                       ndr_print_debug((ndr_print_fn_t)ndr_print_security_descriptor, "SamR", sdbuf->sd);\
+                       ret = False;\
+               }\
+       }\
+} while (0)
+
 static BOOL samsync_handle_domain(TALLOC_CTX *mem_ctx, struct samsync_state *samsync_state,
                           int database_id, struct netr_DELTA_ENUM *delta) 
 {
@@ -279,6 +317,8 @@ static BOOL samsync_handle_domain(TALLOC_CTX *mem_ctx, struct samsync_state *sam
        TEST_TIME_EQUAL(q[13].out.info->info13.domain_create_time, 
                        domain->domain_create_time);
 
+       TEST_SEC_DESC_EQUAL(domain->sdbuf, samsync_state->domain_handle[database_id]);
+
        return ret;
 }
 
@@ -351,6 +391,8 @@ static BOOL samsync_handle_user(TALLOC_CTX *mem_ctx, struct samsync_state *samsy
        q.in.user_handle = &user_handle;
        q.in.level = 21;
 
+       TEST_SEC_DESC_EQUAL(user->sdbuf, &user_handle);
+
        nt_status = dcerpc_samr_QueryUserInfo(samsync_state->p_samr, mem_ctx, &q);
        if (!test_samr_handle_Close(samsync_state->p_samr, mem_ctx, &user_handle)) {
                return False;
@@ -546,6 +588,8 @@ static BOOL samsync_handle_alias(TALLOC_CTX *mem_ctx, struct samsync_state *sams
        q.in.alias_handle = &alias_handle;
        q.in.level = 1;
 
+       TEST_SEC_DESC_EQUAL(alias->sdbuf, &alias_handle);
+
        nt_status = dcerpc_samr_QueryAliasInfo(samsync_state->p_samr, mem_ctx, &q);
        if (!test_samr_handle_Close(samsync_state->p_samr, mem_ctx, &alias_handle)) {
                return False;
@@ -593,6 +637,8 @@ static BOOL samsync_handle_group(TALLOC_CTX *mem_ctx, struct samsync_state *sams
        q.in.group_handle = &group_handle;
        q.in.level = 1;
 
+       TEST_SEC_DESC_EQUAL(group->sdbuf, &group_handle);
+
        nt_status = dcerpc_samr_QueryGroupInfo(samsync_state->p_samr, mem_ctx, &q);
        if (!test_samr_handle_Close(samsync_state->p_samr, mem_ctx, &group_handle)) {
                return False;