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.bagn...@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.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@samba.org>
Autobuild-User(master): Andrew Bartlett <abart...@samba.org>
Autobuild-Date(master): Mon Mar 4 03:20:46 UTC 2024 on atb-devel-224
commit ccfa16e2ec48da4ab601ca6b8b0ccfc77d625085
Author: Douglas Bagnall <douglas.bagn...@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.bagn...@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abart...@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
