The branch, master has been updated via 5a4b050ff7b samba-tool ntacl: better messages for missing files via dc9f29e5c35 pysmbd: set_nt_acl() can raise FileNotFoundError via 1b4938c3b1a pysmbd: get_nt_acl() raises FileNotFoundError if appropriate via a5eeed52efa pysmbd: avoid leaks in get_nt_acl() via dfc92d2922f pybindings: xattr_native raises OSError not TypeError via a64839bc297 pytest: posixacl getntacl should raise OSError via 8df9fdc551a pytest: samba-tool ntacl should report errors better from 84002281410 samba-tool domain: use string_to_level helper()
https://git.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 5a4b050ff7b790f892c4f0edb9ecd9745184e0f4 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Aug 11 11:26:44 2022 +1200 samba-tool ntacl: better messages for missing files BUG: https://bugzilla.samba.org/show_bug.cgi?id=14937 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> Autobuild-User(master): Douglas Bagnall <dbagn...@samba.org> Autobuild-Date(master): Wed Sep 7 06:02:20 UTC 2022 on sn-devel-184 commit dc9f29e5c35982e7ce2cb5135ce906e9960579af Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Sep 1 01:18:12 2022 +0000 pysmbd: set_nt_acl() can raise FileNotFoundError rather than an NTStatusError, which is harder to decipher, and which carries less information (namely, not the name of the problematic file). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14937 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 1b4938c3b1afc8600d693ef92b6944b18e449415 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Sep 1 11:25:26 2022 +1200 pysmbd: get_nt_acl() raises FileNotFoundError if appropriate rather than an NTStatusError, which is harder to decipher, and which carries less information (namely, not the name of the problematic file). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14937 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a5eeed52efa3656fc44ec44874f72790e82c9d91 Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Sep 1 11:06:03 2022 +1200 pysmbd: avoid leaks in get_nt_acl() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14937 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit dfc92d2922fb773a3e5246d91631417a9de4adaf Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Sep 7 12:56:37 2022 +1200 pybindings: xattr_native raises OSError not TypeError Most likely it is a bad filename or attribute, not the wrong type of argument. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14937 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit a64839bc297bdb8b71db446ac6b55fb4503bdc0e Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Wed Sep 7 12:46:42 2022 +1200 pytest: posixacl getntacl should raise OSError Not TypeError, which is supposed to be about Python data types. This way we get to check/see an errno and strerror, and will allow us to set the filename which will be useful for some errors. Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> commit 8df9fdc551a2bf2feca24f2d80fc20825441cecc Author: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Date: Thu Sep 1 10:29:59 2022 +1200 pytest: samba-tool ntacl should report errors better We want `samba-tool ntacl sysvolreset` and `samba-tool ntacl sysvolcheck` to fail when the Policies folder is not in place, but not to produce an inscrutable stacktrace. https://bugzilla.samba.org/show_bug.cgi?id=14937 Signed-off-by: Douglas Bagnall <douglas.bagn...@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abart...@samba.org> ----------------------------------------------------------------------- Summary of changes: python/samba/netcmd/ntacl.py | 26 ++++++++++++++++++-------- python/samba/tests/posixacl.py | 13 ++++++++++--- python/samba/tests/samba_tool/ntacl.py | 27 +++++++++++++++++++++++++++ source3/smbd/pysmbd.c | 28 ++++++++++++++++++++++++++-- source4/ntvfs/posix/python/pyxattr_native.c | 6 +++--- 5 files changed, 84 insertions(+), 16 deletions(-) Changeset truncated at 500 lines: diff --git a/python/samba/netcmd/ntacl.py b/python/samba/netcmd/ntacl.py index 7df75d247fe..8675719017d 100644 --- a/python/samba/netcmd/ntacl.py +++ b/python/samba/netcmd/ntacl.py @@ -409,10 +409,15 @@ class cmd_ntacl_sysvolreset(Command): if use_ntvfs: logger.warning("Please note that POSIX permissions have NOT been changed, only the stored NT ACL") - provision.setsysvolacl(samdb, netlogon, sysvol, - LA_uid, BA_gid, domain_sid, - lp.get("realm").lower(), samdb.domain_dn(), - lp, use_ntvfs=use_ntvfs) + try: + provision.setsysvolacl(samdb, netlogon, sysvol, + LA_uid, BA_gid, domain_sid, + lp.get("realm").lower(), samdb.domain_dn(), + lp, use_ntvfs=use_ntvfs) + except OSError as e: + if not e.filename: + raise + raise CommandError(f"Could not access {e.filename}: {e.strerror}", e) class cmd_ntacl_sysvolcheck(Command): @@ -440,10 +445,15 @@ class cmd_ntacl_sysvolcheck(Command): domain_sid = security.dom_sid(samdb.domain_sid) - provision.checksysvolacl(samdb, netlogon, sysvol, - domain_sid, - lp.get("realm").lower(), samdb.domain_dn(), - lp) + try: + provision.checksysvolacl(samdb, netlogon, sysvol, + domain_sid, + lp.get("realm").lower(), samdb.domain_dn(), + lp) + except OSError as e: + if not e.filename: + raise + raise CommandError(f"Could not access {e.filename}: {e.strerror}", e) class cmd_ntacl(SuperCommand): diff --git a/python/samba/tests/posixacl.py b/python/samba/tests/posixacl.py index 4fcf7bb21ed..ed3c29a14be 100644 --- a/python/samba/tests/posixacl.py +++ b/python/samba/tests/posixacl.py @@ -28,6 +28,7 @@ from samba.samba3 import param as s3param from samba import auth from samba.samdb import SamDB from samba.auth_util import system_session_unix +from errno import ENODATA DOM_SID = "S-1-5-21-2212615479-2695158682-2101375467" ACL = "O:S-1-5-21-2212615479-2695158682-2101375467-512G:S-1-5-21-2212615479-2695158682-2101375467-513D:(A;OICI;0x001f01ff;;;S-1-5-21-2212615479-2695158682-2101375467-512)" @@ -89,8 +90,11 @@ class PosixAclMappingTests(SmbdBaseTests): smbd.set_simple_acl(self.tempf, 0o640, self.get_session_info()) # However, this only asks the xattr - self.assertRaises( - TypeError, getntacl, self.lp, self.tempf, self.get_session_info(), direct_db_access=True) + with self.assertRaises(OSError) as cm: + getntacl(self.lp, self.tempf, self.get_session_info(), + direct_db_access=True) + + self.assertEqual(cm.exception.errno, ENODATA) def test_setntacl_invalidate_getntacl(self): acl = ACL @@ -202,7 +206,10 @@ class PosixAclMappingTests(SmbdBaseTests): def test_setposixacl_getntacl(self): smbd.set_simple_acl(self.tempf, 0o750, self.get_session_info()) # We don't expect the xattr to be filled in in this case - self.assertRaises(TypeError, getntacl, self.lp, self.tempf, self.get_session_info()) + with self.assertRaises(OSError) as cm: + getntacl(self.lp, self.tempf, self.get_session_info()) + + self.assertEqual(cm.exception.errno, ENODATA) def test_setposixacl_getntacl_smbd(self): s4_passdb = passdb.PDB(self.lp.get("passdb backend")) diff --git a/python/samba/tests/samba_tool/ntacl.py b/python/samba/tests/samba_tool/ntacl.py index c9a14ec0ec8..d0e315456e8 100644 --- a/python/samba/tests/samba_tool/ntacl.py +++ b/python/samba/tests/samba_tool/ntacl.py @@ -22,6 +22,7 @@ import os import time import ldb from samba.tests.samba_tool.base import SambaToolCmdTest +from samba.tests import env_loadparm import random @@ -69,6 +70,32 @@ class NtACLCmdSysvolTestCase(SambaToolCmdTest): self.assertEqual(err, "", "Shouldn't be any error messages") self.assertEqual(out, "", "Shouldn't be any output messages") + def test_with_missing_files(self): + lp = env_loadparm() + sysvol = lp.get('path', 'sysvol') + realm = lp.get('realm').lower() + + src = os.path.join(sysvol, realm, 'Policies') + dest = os.path.join(sysvol, realm, 'Policies-NOT-IN-THE-EXPECTED-PLACE') + try: + os.rename(src, dest) + + for args in (["sysvolreset", "--use-s3fs"], + ["sysvolreset", "--use-ntvfs"], + ["sysvolreset"], + ["sysvolcheck"] + ): + + (result, out, err) = self.runsubcmd("ntacl", *args) + self.assertCmdFail(result, f"succeeded with {args} with missing dir") + self.assertNotIn("uncaught exception", err, + "Shouldn't be uncaught exception") + self.assertNotRegex(err, '^\s*File [^,]+, line \d+, in', + "Shouldn't be lines of traceback") + self.assertEqual(out, "", "Shouldn't be any output messages") + finally: + os.rename(dest, src) + class NtACLCmdGetSetTestCase(SambaToolCmdTest): """Tests for samba-tool ntacl get/set subcommands""" diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c index 658de43d2c0..af8a08d7c65 100644 --- a/source3/smbd/pysmbd.c +++ b/source3/smbd/pysmbd.c @@ -810,7 +810,17 @@ static PyObject *py_smbd_set_nt_acl(PyObject *self, PyObject *args, PyObject *kw status = set_nt_acl_conn(fname, security_info_sent, sd, conn); TALLOC_FREE(frame); - PyErr_NTSTATUS_IS_ERR_RAISE(status); + if (NT_STATUS_IS_ERR(status)) { + if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + /* + * This will show up as a FileNotFoundError in python. + */ + PyErr_SetFromErrnoWithFilename(PyExc_OSError, fname); + } else { + PyErr_SetNTSTATUS(status); + } + return NULL; + } Py_RETURN_NONE; } @@ -865,6 +875,7 @@ static PyObject *py_smbd_get_nt_acl(PyObject *self, PyObject *args, PyObject *kw "Expected auth_session_info for " "session_info argument got %s", pytalloc_get_name(py_session)); + TALLOC_FREE(frame); return NULL; } @@ -875,7 +886,20 @@ static PyObject *py_smbd_get_nt_acl(PyObject *self, PyObject *args, PyObject *kw } status = get_nt_acl_conn(frame, fname, conn, security_info_wanted, &sd); - PyErr_NTSTATUS_IS_ERR_RAISE(status); + if (NT_STATUS_IS_ERR(status)) { + if (NT_STATUS_EQUAL(status, NT_STATUS_OBJECT_NAME_NOT_FOUND)) { + /* + * This will show up as a FileNotFoundError in python, + * from which samba-tool can at least produce a short + * message containing the problematic filename. + */ + PyErr_SetFromErrnoWithFilename(PyExc_OSError, fname); + } else { + PyErr_SetNTSTATUS(status); + } + TALLOC_FREE(frame); + return NULL; + } py_sd = py_return_ndr_struct("samba.dcerpc.security", "descriptor", sd, sd); diff --git a/source4/ntvfs/posix/python/pyxattr_native.c b/source4/ntvfs/posix/python/pyxattr_native.c index d242cd98a5d..7511e5c02bf 100644 --- a/source4/ntvfs/posix/python/pyxattr_native.c +++ b/source4/ntvfs/posix/python/pyxattr_native.c @@ -52,7 +52,7 @@ static PyObject *py_wrap_setxattr(PyObject *self, PyObject *args) if (errno == ENOTSUP) { PyErr_SetFromErrno(PyExc_IOError); } else { - PyErr_SetFromErrno(PyExc_TypeError); + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); } return NULL; } @@ -74,7 +74,7 @@ static PyObject *py_wrap_getxattr(PyObject *self, PyObject *args) if (errno == ENOTSUP) { PyErr_SetFromErrno(PyExc_IOError); } else { - PyErr_SetFromErrno(PyExc_TypeError); + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); } talloc_free(mem_ctx); return NULL; @@ -86,7 +86,7 @@ static PyObject *py_wrap_getxattr(PyObject *self, PyObject *args) if (errno == ENOTSUP) { PyErr_SetFromErrno(PyExc_IOError); } else { - PyErr_SetFromErrno(PyExc_TypeError); + PyErr_SetFromErrnoWithFilename(PyExc_OSError, filename); } talloc_free(mem_ctx); return NULL; -- Samba Shared Repository