From d7fe04205f8dedd61404c2aa03f1dda7d2dc72b7 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 26 Oct 2023 17:46:35 +1300 Subject: [PATCH] s4/librpc/py_security: use SDDLValueError for better error messages The aim is to allow samba-tool to tell users where their SDDL went wrong. Some tests would turn into errors (not knownfail-able failures) if they were not changed at the same time, so they are changed too. Signed-off-by: Douglas Bagnall Reviewed-by: Andrew Bartlett --- python/samba/tests/sddl.py | 2 +- python/samba/tests/security.py | 2 +- source4/librpc/ndr/py_security.c | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/python/samba/tests/sddl.py b/python/samba/tests/sddl.py index 7951af01b60..5317543010b 100644 --- a/python/samba/tests/sddl.py +++ b/python/samba/tests/sddl.py @@ -82,7 +82,7 @@ class SddlDecodeEncodeBase(TestCase): self.assertEqual(sddl, canonical) def _test_sddl_should_fail_with_args(self, s, canonical): - with self.assertRaises(ValueError): + with self.assertRaises(security.SDDLValueError): sd = security.descriptor.from_sddl(s, self.domain_sid) print(sd.as_sddl(self.domain_sid)) diff --git a/python/samba/tests/security.py b/python/samba/tests/security.py index 3b7eb3fad48..fb2897e4dfc 100644 --- a/python/samba/tests/security.py +++ b/python/samba/tests/security.py @@ -67,7 +67,7 @@ class SecurityDescriptorTests(samba.tests.TestCase): self.assertEqual(desc.type, 0x8004) def test_from_sddl_invalidsddl(self): - self.assertRaises(ValueError, security.descriptor.from_sddl, "foo", + self.assertRaises(security.SDDLValueError, security.descriptor.from_sddl, "foo", security.dom_sid("S-1-2-3")) def test_from_sddl_invalidtype1(self): diff --git a/source4/librpc/ndr/py_security.c b/source4/librpc/ndr/py_security.c index 14228c79ab4..0daeaf7aab3 100644 --- a/source4/librpc/ndr/py_security.c +++ b/source4/librpc/ndr/py_security.c @@ -276,6 +276,8 @@ static PyObject *py_descriptor_from_sddl(PyObject *self, PyObject *args) char *sddl; PyObject *py_sid; struct dom_sid *sid; + const char *err_msg = NULL; + size_t err_msg_offset = 0; if (!PyArg_ParseTuple(args, "sO!", &sddl, &dom_sid_Type, &py_sid)) return NULL; @@ -289,9 +291,35 @@ static PyObject *py_descriptor_from_sddl(PyObject *self, PyObject *args) sid = pytalloc_get_ptr(py_sid); - secdesc = sddl_decode(NULL, sddl, sid); + secdesc = sddl_decode_err_msg(NULL, sddl, sid, + &err_msg, &err_msg_offset); if (secdesc == NULL) { - PyErr_SetString(PyExc_ValueError, "Unable to parse SDDL"); + PyObject *exc = NULL; + if (err_msg == NULL) { + err_msg = "unknown error"; + } + /* + * Some notes about this exception value: + * + * We don't want to add the offset first, so as not to + * confuse those who are used to the integer error + * code coming first. + * + * The errant sddl is added so that the exception can + * be caught some distance away from the call and we + * still know what the messages refer to. + */ + exc = Py_BuildValue("(s, s, i, s)", + "Unable to parse SDDL", + err_msg, + err_msg_offset, + sddl); + if (exc == NULL) { + /* an exception was set by Py_BuildValue() */ + return NULL; + } + PyErr_SetObject(PyExc_SDDLValueError, exc); + Py_DECREF(exc); return NULL; } -- 2.47.3