From a7c755672cd277497da3df4714f6d9457b6ac5ae Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose@redhat.com>
Date: Thu, 28 May 2020 15:02:43 +0200
Subject: [PATCH] ad_gpo_ndr.c: more ndr updates
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch add another update to the ndr code which was previously
updated by commit c031adde4f532f39845a0efd78693600f1f8b2f4 and
1fdd8fa2fded1985fbfc6aa67394eebcdbb6a2fc.

As missing update in ndr_pull_security_ace() cased
a failure in ad_gpo_parse_sd(). A unit-test for ad_gpo_parse_sd() was
added to prevent similar issues in future.

Resolves: https://github.com/SSSD/sssd/issues/5183

Reviewed-by: Pavel Březina <pbrezina@redhat.com>
---
 src/providers/ad/ad_gpo_ndr.c  |  1 +
 src/tests/cmocka/test_ad_gpo.c | 57 ++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/src/providers/ad/ad_gpo_ndr.c b/src/providers/ad/ad_gpo_ndr.c
index acd7b77c8..71d6d40f2 100644
--- a/src/providers/ad/ad_gpo_ndr.c
+++ b/src/providers/ad/ad_gpo_ndr.c
@@ -317,6 +317,7 @@ ndr_pull_security_ace(struct ndr_pull *ndr,
         ndr->offset += pad;
     }
     if (ndr_flags & NDR_BUFFERS) {
+        NDR_CHECK(ndr_pull_set_switch_value(ndr, &r->object, r->type));
         NDR_CHECK(ndr_pull_security_ace_object_ctr
                   (ndr, NDR_BUFFERS, &r->object));
     }
diff --git a/src/tests/cmocka/test_ad_gpo.c b/src/tests/cmocka/test_ad_gpo.c
index 97f70408a..d1f7a6915 100644
--- a/src/tests/cmocka/test_ad_gpo.c
+++ b/src/tests/cmocka/test_ad_gpo.c
@@ -347,6 +347,60 @@ void test_ad_gpo_ace_includes_host_sid_true(void **state)
                                         group_size, ace_dom_sid, true);
 }
 
+uint8_t test_sid_data[] = {
+0x01, 0x00, 0x04, 0x9c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x14, 0x00, 0x00, 0x00, 0x04, 0x00, 0x34, 0x01, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x24, 0x00,
+0xbd, 0x00, 0x0e, 0x00, 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x15, 0x00, 0x00, 0x00,
+0xda, 0x0e, 0xba, 0x60, 0x0f, 0xa2, 0xf4, 0x55, 0xb5, 0x57, 0x47, 0xf8, 0x00, 0x02, 0x00, 0x00,
+0x00, 0x0a, 0x24, 0x00, 0xff, 0x00, 0x0f, 0x00, 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05,
+0x15, 0x00, 0x00, 0x00, 0xda, 0x0e, 0xba, 0x60, 0x0f, 0xa2, 0xf4, 0x55, 0xb5, 0x57, 0x47, 0xf8,
+0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x24, 0x00, 0xbd, 0x00, 0x0e, 0x00, 0x01, 0x05, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x05, 0x15, 0x00, 0x00, 0x00, 0xda, 0x0e, 0xba, 0x60, 0x0f, 0xa2, 0xf4, 0x55,
+0xb5, 0x57, 0x47, 0xf8, 0x07, 0x02, 0x00, 0x00, 0x00, 0x0a, 0x24, 0x00, 0xff, 0x00, 0x0f, 0x00,
+0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x15, 0x00, 0x00, 0x00, 0xda, 0x0e, 0xba, 0x60,
+0x0f, 0xa2, 0xf4, 0x55, 0xb5, 0x57, 0x47, 0xf8, 0x07, 0x02, 0x00, 0x00, 0x00, 0x00, 0x24, 0x00,
+0xbd, 0x00, 0x0e, 0x00, 0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x15, 0x00, 0x00, 0x00,
+0xda, 0x0e, 0xba, 0x60, 0x0f, 0xa2, 0xf4, 0x55, 0xb5, 0x57, 0x47, 0xf8, 0x00, 0x02, 0x00, 0x00,
+0x00, 0x0a, 0x14, 0x00, 0xff, 0x00, 0x0f, 0x00, 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x14, 0x00, 0xff, 0x00, 0x0f, 0x00, 0x01, 0x01, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x05, 0x12, 0x00, 0x00, 0x00, 0x00, 0x02, 0x14, 0x00, 0x94, 0x00, 0x02, 0x00,
+0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x0b, 0x00, 0x00, 0x00, 0x05, 0x02, 0x28, 0x00,
+0x00, 0x01, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x8f, 0xfd, 0xac, 0xed, 0xb3, 0xff, 0xd1, 0x11,
+0xb4, 0x1d, 0x00, 0xa0, 0xc9, 0x68, 0xf9, 0x39, 0x01, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05,
+0x0b, 0x00, 0x00, 0x00, 0x00, 0x02, 0x14, 0x00, 0x94, 0x00, 0x02, 0x00, 0x01, 0x01, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x05, 0x09, 0x00, 0x00, 0x00
+};
+
+void test_ad_gpo_parse_sd(void **state)
+{
+    int ret;
+    struct security_descriptor *sd = NULL;
+
+    ret = ad_gpo_parse_sd(test_ctx, NULL, 0, &sd);
+    assert_int_equal(ret, EINVAL);
+
+    ret = ad_gpo_parse_sd(test_ctx, test_sid_data, sizeof(test_sid_data), &sd);
+    assert_int_equal(ret, EOK);
+    assert_non_null(sd);
+    assert_int_equal(sd->revision, 1);
+    assert_int_equal(sd->type, 39940);
+    assert_null(sd->owner_sid);
+    assert_null(sd->group_sid);
+    assert_null(sd->sacl);
+    assert_non_null(sd->dacl);
+    assert_int_equal(sd->dacl->revision, 4);
+    assert_int_equal(sd->dacl->size, 308);
+    assert_int_equal(sd->dacl->num_aces, 10);
+    assert_int_equal(sd->dacl->aces[0].type, 0);
+    assert_int_equal(sd->dacl->aces[0].flags, 0);
+    assert_int_equal(sd->dacl->aces[0].size, 36);
+    assert_int_equal(sd->dacl->aces[0].access_mask, 917693);
+    /* There are more components and ACEs in the security_descriptor struct
+     * which are not checked here. */
+
+    talloc_free(sd);
+}
+
 int main(int argc, const char *argv[])
 {
     poptContext pc;
@@ -385,6 +439,9 @@ int main(int argc, const char *argv[])
         cmocka_unit_test_setup_teardown(test_ad_gpo_ace_includes_host_sid_true,
                                         ad_gpo_test_setup,
                                         ad_gpo_test_teardown),
+        cmocka_unit_test_setup_teardown(test_ad_gpo_parse_sd,
+                                        ad_gpo_test_setup,
+                                        ad_gpo_test_teardown),
     };
 
     /* Set debug level to invalid value so we can decide if -d 0 was used. */
-- 
2.21.1