[PATCH] selftest: Add sanity-check RODC can't use cache to reveal secrets

[Tim Beale]

Hi,

Attached is a follow-up patch for bug #12977. It adds a test that checks
RODCs can't exploit the cache on the Samba server to leak secrets.

We unintentionally fixed this security hole in 4.7. The new test checks
we don't unintentionally reopen the hole again in the future.

Cheers,
Tim
-------------- next part --------------
From 5b5ce9062871620e2c8d3ea9816317193988b76f Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 2 Oct 2017 14:33:47 +1300
Subject: [PATCH] selftest: Add sanity-check RODC can't use cache to reveal
 secrets

Bug 12977 highlighted that Samba only checks exop GetNcChanges requests
once, when they're first received. This makes sense because valid exop
requests should only ever involve a single request. For regular
(non-exop) GetNcChanges requests, the server stores a cache of the
object GUIDs to return.

What we don't want to happen is for a malicious/compromised RODC to use
this cache to circumvent privilege checks, and receive secrets that it's
normally not permitted to access (e.g. the administrator's password).

The specific scenario we're concerned about is:
- The RODC sends a regular GetNcChanges request for all objects (without
  secrets). (This causes the server to build its GUID array cache).
- The RODC then sends a follow-on request for the next chunk, but sets
  the REPL_SECRET exop this time.

The only thing inadvertently preventing Samba from leaking secrets in
this case is updating msDS-RevealedUsers for auditing. It's possible
that a future code change may alter the codepath and open up a
security-hole without realizing. This patch adds a test case so if that
ever did happen, the selftests would detect the problem.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12977

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source4/torture/drs/python/repl_rodc.py | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/source4/torture/drs/python/repl_rodc.py b/source4/torture/drs/python/repl_rodc.py
index ca3744c..57679ee 100644
--- a/source4/torture/drs/python/repl_rodc.py
+++ b/source4/torture/drs/python/repl_rodc.py
@@ -215,6 +215,39 @@ class DrsRodcTestCase(drs_base.DrsBaseTestCase):
         # Check that the user has been added to msDSRevealedUsers
         self._assert_in_revealed_users(user_dn, expected_user_attributes)
 
+    def test_rodc_repl_secrets_follow_on_req(self):
+        """
+        Checks that an RODC can't subvert an existing (valid) GetNCChanges
+        request to reveal secrets it shouldn't have access to.
+        """
+
+        # send an acceptable request that will match as many GUIDs as possible.
+        # Here we set the SPECIAL_SECRET_PROCESSING flag so that the request gets accepted.
+        # (On the server, this builds up the getnc_state->guids array)
+        req8 = self._exop_req8(dest_dsa=str(self.rodc_ctx.ntds_guid),
+                               invocation_id=self.ldb_dc1.get_invocation_id(),
+                               nc_dn_str=self.ldb_dc1.domain_dn(),
+                               exop=drsuapi.DRSUAPI_EXOP_NONE,
+                               max_objects=1,
+                               replica_flags=drsuapi.DRSUAPI_DRS_SPECIAL_SECRET_PROCESSING)
+        (level, ctr) = self.rodc_drs.DsGetNCChanges(self.rodc_drs_handle, 8, req8)
+
+        # Get the next replication chunk, but set REPL_SECRET this time. This
+        # is following on the the previous accepted request, but we've changed
+        # exop to now request secrets. This request should fail
+        try:
+            req8 = self._exop_req8(dest_dsa=str(self.rodc_ctx.ntds_guid),
+                                   invocation_id=self.ldb_dc1.get_invocation_id(),
+                                   nc_dn_str=self.ldb_dc1.domain_dn(),
+                                   exop=drsuapi.DRSUAPI_EXOP_REPL_SECRET)
+            req8.highwatermark = ctr.new_highwatermark
+
+            (level, ctr) = self.rodc_drs.DsGetNCChanges(self.rodc_drs_handle, 8, req8)
+
+            self.fail("Successfully replicated secrets to an RODC that shouldn't have been replicated.")
+        except RuntimeError as (enum, estr):
+            pass
+
     def test_msDSRevealedUsers_admin(self):
         """
         When a secret attribute is to be replicated to an RODC, the contents
-- 
2.7.4