[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Mon Mar 4 03:21:01 UTC 2024


The branch, master has been updated
       via  9b0330ea3f5 pytest:samba-tool domain kds root-key: test with normal user
       via  ccfa16e2ec4 samba-tool: tidy up uncaught insufficient rights LdbError
      from  ee94d708557 ldb: Update ldb.get_opaque() to return talloc‐managed opaque values

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


- Log -----------------------------------------------------------------
commit 9b0330ea3f5d5b41f84356ec54a2e5a6ecbbaccd
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Mon Mar 4 10:46:02 2024 +1300

    pytest:samba-tool domain kds root-key: test with normal user
    
    It would be bad if samba-tool let ordinary users read root-key secrets.
    
    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 Mar  4 03:20:46 UTC 2024 on atb-devel-224

commit ccfa16e2ec48da4ab601ca6b8b0ccfc77d625085
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Mon Mar 4 10:43:17 2024 +1300

    samba-tool: tidy up uncaught insufficient rights LdbError
    
    It is likely that many sub-commands will produce a traceback when people
    go `-H ldap://server -Ubob` when they needed to go `-UAdministrator`.
    
    We can catch these and show only the core message.
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

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

Summary of changes:
 python/samba/netcmd/__init__.py                    |   5 +-
 .../samba/tests/samba_tool/domain_kds_root_key.py  | 105 +++++++++++++++++++++
 2 files changed, 109 insertions(+), 1 deletion(-)


Changeset truncated at 500 lines:

diff --git a/python/samba/netcmd/__init__.py b/python/samba/netcmd/__init__.py
index 3e1f1c45aef..7d743526207 100644
--- a/python/samba/netcmd/__init__.py
+++ b/python/samba/netcmd/__init__.py
@@ -23,7 +23,7 @@ import textwrap
 import traceback
 
 import samba
-from ldb import ERR_INVALID_CREDENTIALS, LdbError
+from ldb import ERR_INVALID_CREDENTIALS, ERR_INSUFFICIENT_ACCESS_RIGHTS, LdbError
 from samba import colour
 from samba.auth import system_session
 from samba.getopt import Option, OptionParser
@@ -242,6 +242,9 @@ class Command(object):
             elif ldb_emsg.startswith("Unable to open tdb "):
                 self._print_error(message, ldb_emsg, 'ldb')
                 force_traceback = False
+            elif ldb_ecode == ERR_INSUFFICIENT_ACCESS_RIGHTS:
+                self._print_error("User has insufficient access rights")
+                force_traceback = False
             else:
                 self._print_error(message, ldb_emsg, 'ldb')
 
diff --git a/python/samba/tests/samba_tool/domain_kds_root_key.py b/python/samba/tests/samba_tool/domain_kds_root_key.py
index ad8e6e97f90..3a6613a14c0 100644
--- a/python/samba/tests/samba_tool/domain_kds_root_key.py
+++ b/python/samba/tests/samba_tool/domain_kds_root_key.py
@@ -39,6 +39,9 @@ HOST = "ldap://{DC_SERVER}".format(**os.environ)
 CREDS = "-U{DC_USERNAME}%{DC_PASSWORD}".format(**os.environ)
 SMBCONF = os.environ['SERVERCONFFILE']
 
+# alice%Secret007
+NON_ADMIN_CREDS = "-U{DOMAIN_USER}%{DOMAIN_USER_PASSWORD}".format(**os.environ)
+
 TIMESTAMP_RE = r'\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{6}\+00:00'
 
 NOWISH = 'about now'
@@ -500,6 +503,22 @@ class KdsRootKeyTests(KdsRootKeyTestsBase):
                          f"created root key {new_guids[0]}, usable from {TIMESTAMP_RE}")
         self._delete_root_key(new_guids[0])
 
+    def test_create_json_non_admin(self):
+        """can you create a root-key without being admin?"""
+        pre_create = self._get_root_key_guids()
+
+        result, out, err = self.runcmd("domain", "kds", "root-key", "create",
+                                       "-H", HOST, NON_ADMIN_CREDS, "--json")
+        self.assertCmdFail(result)
+
+        post_create = self._get_root_key_guids()
+
+        self.assertEqual(set(pre_create), set(post_create))
+        data = json.loads(out)
+        self.assertEqual(data['status'], 'error')
+        self.assertEqual(data['message'], 'User has insufficient access rights')
+        self.assertEqual(err, "", "not expecting stderr messages")
+
     def test_create_json_1997(self):
         """does create work?"""
         pre_create = self._get_root_key_guids()
@@ -640,6 +659,81 @@ class KdsRootKeyTests(KdsRootKeyTestsBase):
         self.assertIn(guid, pre_names)
         self.assertNotIn(guid, post_names)
 
+    def test_delete_non_admin(self):
+        """does delete as non-admin fail?"""
+        # make one to delete, and get the list as JSON
+        _guid, dn, _created, _used = self._create_root_key_timediff()
+        guid = str(_guid)
+
+        result, out, err = self.runcmd("domain", "kds", "root-key", "list", "--json",
+                                       "-H", HOST, CREDS)
+        pre_delete = json.loads(out)
+
+        result, out, err = self.runcmd("domain", "kds", "root-key", "delete",
+                                       "-H", HOST, NON_ADMIN_CREDS,
+                                       "--name", guid)
+        self.assertCmdFail(result)
+        self.assertIn(f"ERROR: no such root key: {guid}", err)
+
+        # a bad guid should be just like a good guid
+        guid2 = 'eeeeeeee-1111-eeee-1111-000000000000'
+        result, out2, err2 = self.runcmd("domain", "kds", "root-key", "delete",
+                                         "-H", HOST, NON_ADMIN_CREDS,
+                                         "--name", guid2)
+        self.assertCmdFail(result)
+        self.assertIn(f"ERROR: no such root key: {guid2}", err2)
+
+        result, out, err = self.runcmd("domain", "kds", "root-key", "list", "--json",
+                                       "-H", HOST, CREDS)
+        post_delete = json.loads(out)
+
+        self.assertEqual(len(pre_delete), len(post_delete))
+
+        post_names = [x['name'] for x in post_delete]
+        pre_names = [x['name'] for x in pre_delete]
+
+        self.assertIn(guid, pre_names)
+        self.assertIn(guid, post_names)
+
+    def test_list_non_admin(self):
+        """There are root keys, but non-admins can't see them"""
+        result, out, err = self.runcmd("domain", "kds", "root-key", "list",
+                                       "-H", HOST, NON_ADMIN_CREDS)
+        self.assertCmdSuccess(result, out, err)
+        self.assertEqual(err, "", "not expecting error messages")
+        self.assertEqual(out, "no root keys found.\n")
+
+    def test_list_json_non_admin(self):
+        """Insufficient rights should look like an empty list."""
+        # this is a copy of the KdsNoRootKeyTests test below --
+        # non-admin should look exactly like an empty list.
+        for extra in ([], ["-v"]):
+            result, out, err = self.runcmd("domain", "kds", "root-key", "list",
+                                           "-H", HOST, NON_ADMIN_CREDS, "--json", *extra)
+            self.assertCmdSuccess(result, out, err)
+            self.assertEqual(err, "", "not expecting error messages")
+            data = json.loads(out)
+            self.assertEqual(data, [])
+
+    def test_view_key_non_admin(self):
+        """should not appear to non-admin"""
+        guid, dn, _created, _used = self._create_root_key_timediff_cleanup()
+
+        result, out, err = self.runcmd("domain", "kds", "root-key", "view",
+                                       "--json",
+                                       "--name", str(guid),
+                                       "-H", HOST, NON_ADMIN_CREDS)
+        self.assertCmdFail(result)
+        self.assertEqual(err, "", "not expecting error messages")
+        data = json.loads(out)
+        data = json.loads(out)
+        self.assertEqual(
+            data,
+            {
+                "message": f"no such root key: {guid}",
+                "status": "error"
+            })
+
 
 class KdsNoRootKeyTests(KdsRootKeyTestsBase):
     """Here we test the case were there are no root keys, which we need to
@@ -679,6 +773,17 @@ class KdsNoRootKeyTests(KdsRootKeyTestsBase):
             data = json.loads(out)
             self.assertEqual(data, [])
 
+    def test_list_empty_json_non_admin(self):
+        """Insufficient rights should look like an empty list."""
+        # verbose flag makes no difference here.
+        for extra in ([], ["-v"]):
+            result, out, err = self.runcmd("domain", "kds", "root-key", "list",
+                                           "-H", HOST, NON_ADMIN_CREDS, "--json", *extra)
+            self.assertCmdSuccess(result, out, err)
+            self.assertEqual(err, "", "not expecting error messages")
+            data = json.loads(out)
+            self.assertEqual(data, [])
+
     def test_view_latest_non_existent(self):
         """With no root keys, --latest should return an error"""
 


-- 
Samba Shared Repository



More information about the samba-cvs mailing list