From 7cc44282e4d7b4c27851b6631c08b8aa14887721 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 24 Mar 2017 13:07:23 +1300 Subject: [PATCH 1/6] selftest: Add more tests for "samba-tool processes" Signed-off-by: Andrew Bartlett Reviewed-by: Garming Sam BUG: https://bugzilla.samba.org/show_bug.cgi?id=12705 (cherry picked from commit f21c17c6d0bfa304c54d80aaceb91aad1a3a6bb1) --- python/samba/tests/samba_tool/processes.py | 6 ++++++ selftest/knownfail | 1 + 2 files changed, 7 insertions(+) diff --git a/python/samba/tests/samba_tool/processes.py b/python/samba/tests/samba_tool/processes.py index 5b8f5024183..cff9cad1b27 100644 --- a/python/samba/tests/samba_tool/processes.py +++ b/python/samba/tests/samba_tool/processes.py @@ -29,6 +29,12 @@ class ProcessCmdTestCase(SambaToolCmdTest): (result, out, err) = self.runcmd("processes", "--name", "samba") self.assertCmdSuccess(result, out, err, "Ensuring processes ran successfully") + def test_unknown_name(self): + """Run processes command with an not-existing --name""" + (result, out, err) = self.runcmd("processes", "--name", "not-existing-samba") + self.assertCmdSuccess(result, out, err, "Ensuring processes ran successfully") + self.assertEqual(out, "") + def test_all(self): """Run processes command""" (result, out, err) = self.runcmd("processes") diff --git a/selftest/knownfail b/selftest/knownfail index d16d7231711..a42b423da2a 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -306,3 +306,4 @@ ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.* ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.* ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.* +^samba.tests.samba_tool.processes.samba.tests.samba_tool.processes.ProcessCmdTestCase.test_unknown_name\(ad_dc_ntvfs:local\) -- 2.11.0 From ec5074cb4d626d3625c5f110018d76455d398347 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Fri, 24 Mar 2017 13:07:06 +1300 Subject: [PATCH 2/6] samba-tool: Ensure that samba-tool processes --name=not-existing does not error Signed-off-by: Andrew Bartlett Reviewed-by: Garming Sam BUG: https://bugzilla.samba.org/show_bug.cgi?id=12705 (cherry picked from commit a47a8e41bd3acc20d40ba78449d89775bcdd73ed) --- python/samba/netcmd/processes.py | 6 +++++- selftest/knownfail | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/python/samba/netcmd/processes.py b/python/samba/netcmd/processes.py index b25a2e453ec..c8000b7e49e 100644 --- a/python/samba/netcmd/processes.py +++ b/python/samba/netcmd/processes.py @@ -60,7 +60,11 @@ class cmd_processes(Command): msg_ctx = Messaging() if name is not None: - ids = msg_ctx.irpc_servers_byname(name) + try: + ids = msg_ctx.irpc_servers_byname(name) + except KeyError: + ids = [] + for server_id in ids: self.outf.write("%d\n" % server_id.pid) elif pid is not None: diff --git a/selftest/knownfail b/selftest/knownfail index a42b423da2a..d16d7231711 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -306,4 +306,3 @@ ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.* ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.* ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.* -^samba.tests.samba_tool.processes.samba.tests.samba_tool.processes.ProcessCmdTestCase.test_unknown_name\(ad_dc_ntvfs:local\) -- 2.11.0 From 0c3cd67095eaf6a7d4f1f0e125557eb0ab05a1fd Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 8 Mar 2017 14:53:26 +1300 Subject: [PATCH 3/6] pymessaging: Add support for irpc_add_name This allows tests to be indirectly added for server_id_db_lookup() Signed-off-by: Andrew Bartlett Reviewed-by: Garming Sam Pair-Programmed-by: Gary Lockyer Signed-off-by: Gary Lockyer BUG: https://bugzilla.samba.org/show_bug.cgi?id=12705 (cherry picked from commit 3bd9e5f4ed2362f5006144433295cde2276272c5) --- python/samba/tests/messaging.py | 4 ++++ source4/lib/messaging/pymessaging.c | 26 +++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py index 5d32d608862..1c5dfe505b6 100644 --- a/python/samba/tests/messaging.py +++ b/python/samba/tests/messaging.py @@ -49,6 +49,10 @@ class MessagingTests(TestCase): x = self.get_context() self.assertTrue(isinstance(x.server_id, server_id)) + def test_add_name(self): + x = self.get_context() + x.irpc_add_name("samba.messaging test") + def test_ping_speed(self): server_ctx = self.get_context((0, 1)) def ping_callback(src, data): diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index f62354b8a02..5c20c186faf 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -241,6 +241,25 @@ static PyObject *py_imessaging_deregister(PyObject *self, PyObject *args, PyObje Py_RETURN_NONE; } +static PyObject *py_irpc_add_name(PyObject *self, PyObject *args, PyObject *kwargs) +{ + imessaging_Object *iface = (imessaging_Object *)self; + char *server_name; + NTSTATUS status; + + if (!PyArg_ParseTuple(args, "s", &server_name)) { + return NULL; + } + + status = irpc_add_name(iface->msg_ctx, server_name); + if (!NT_STATUS_IS_OK(status)) { + PyErr_SetNTSTATUS(status); + return NULL; + } + + Py_RETURN_NONE; +} + static PyObject *py_irpc_servers_byname(PyObject *self, PyObject *args, PyObject *kwargs) { imessaging_Object *iface = (imessaging_Object *)self; @@ -341,10 +360,15 @@ static PyMethodDef py_imessaging_methods[] = { "S.register(callback, msg_type=None) -> msg_type\nRegister a message handler" }, { "deregister", (PyCFunction)py_imessaging_deregister, METH_VARARGS|METH_KEYWORDS, "S.deregister(callback, msg_type) -> None\nDeregister a message handler" }, + { "irpc_add_name", (PyCFunction)py_irpc_add_name, METH_VARARGS, + "S.irpc_add_name(name) -> None\n" + "Add this context to the list of server_id values that " + "are registered for a particular name" }, { "irpc_servers_byname", (PyCFunction)py_irpc_servers_byname, METH_VARARGS, "S.irpc_servers_byname(name) -> list\nGet list of server_id values that are registered for a particular name" }, { "irpc_all_servers", (PyCFunction)py_irpc_all_servers, METH_NOARGS, - "S.irpc_servers_byname() -> list\nGet list of all registered names and the associated server_id values" }, + "S.irpc_all_servers() -> list\n" + "Get list of all registered names and the associated server_id values" }, { NULL, NULL, 0, NULL } }; -- 2.11.0 From 3d63e83a55aa92abe91451e0655abdc206db65a9 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 14 Mar 2017 13:39:00 +1300 Subject: [PATCH 4/6] pymessaging: Add irpc_remove_name This allows tests to be indirectly added for server_id_db_lookup() and server_id_db_prune_name() Signed-off-by: Andrew Bartlett Reviewed-by: Garming Sam BUG: https://bugzilla.samba.org/show_bug.cgi?id=12705 (cherry picked from commit e77c18019aef9c98caa0b66cb2e9da5a6f58e600) --- python/samba/tests/messaging.py | 8 ++++++++ source4/lib/messaging/pymessaging.c | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py index 1c5dfe505b6..3eeab529527 100644 --- a/python/samba/tests/messaging.py +++ b/python/samba/tests/messaging.py @@ -22,6 +22,7 @@ import samba from samba.messaging import Messaging from samba.tests import TestCase from samba.dcerpc.server_id import server_id +from samba.ndr import ndr_print class MessagingTests(TestCase): @@ -52,6 +53,13 @@ class MessagingTests(TestCase): def test_add_name(self): x = self.get_context() x.irpc_add_name("samba.messaging test") + name_list = x.irpc_servers_byname("samba.messaging test") + self.assertEqual(len(name_list), 1) + self.assertEqual(ndr_print(x.server_id), + ndr_print(name_list[0])) + x.irpc_remove_name("samba.messaging test") + self.assertEqual([], + x.irpc_servers_byname("samba.messaging test")) def test_ping_speed(self): server_ctx = self.get_context((0, 1)) diff --git a/source4/lib/messaging/pymessaging.c b/source4/lib/messaging/pymessaging.c index 5c20c186faf..5b5408caddb 100644 --- a/source4/lib/messaging/pymessaging.c +++ b/source4/lib/messaging/pymessaging.c @@ -260,6 +260,20 @@ static PyObject *py_irpc_add_name(PyObject *self, PyObject *args, PyObject *kwar Py_RETURN_NONE; } +static PyObject *py_irpc_remove_name(PyObject *self, PyObject *args, PyObject *kwargs) +{ + imessaging_Object *iface = (imessaging_Object *)self; + char *server_name; + + if (!PyArg_ParseTuple(args, "s", &server_name)) { + return NULL; + } + + irpc_remove_name(iface->msg_ctx, server_name); + + Py_RETURN_NONE; +} + static PyObject *py_irpc_servers_byname(PyObject *self, PyObject *args, PyObject *kwargs) { imessaging_Object *iface = (imessaging_Object *)self; @@ -364,6 +378,10 @@ static PyMethodDef py_imessaging_methods[] = { "S.irpc_add_name(name) -> None\n" "Add this context to the list of server_id values that " "are registered for a particular name" }, + { "irpc_remove_name", (PyCFunction)py_irpc_remove_name, METH_VARARGS, + "S.irpc_remove_name(name) -> None\n" + "Remove this context from the list of server_id values that " + "are registered for a particular name" }, { "irpc_servers_byname", (PyCFunction)py_irpc_servers_byname, METH_VARARGS, "S.irpc_servers_byname(name) -> list\nGet list of server_id values that are registered for a particular name" }, { "irpc_all_servers", (PyCFunction)py_irpc_all_servers, METH_NOARGS, -- 2.11.0 From 6995536730a505f97ca581f0d3af3cf312a2b23c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Tue, 14 Mar 2017 16:07:46 +1300 Subject: [PATCH 5/6] selftest: Test server_id database add and removal This tests indirectly server_id_db_lookup() and server_id_db_prune_name(), as well as the imessaging and the imessaging python bindings. Signed-off-by: Andrew Bartlett Reviewed-by: Garming Sam BUG: https://bugzilla.samba.org/show_bug.cgi?id=12705 (cherry picked from commit 0c25c40315a8255362780486d2f2e27ea0dbbff4) --- python/samba/tests/messaging.py | 19 +++++++++++++------ selftest/knownfail | 1 + 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/python/samba/tests/messaging.py b/python/samba/tests/messaging.py index 3eeab529527..a70be96edc2 100644 --- a/python/samba/tests/messaging.py +++ b/python/samba/tests/messaging.py @@ -23,6 +23,7 @@ from samba.messaging import Messaging from samba.tests import TestCase from samba.dcerpc.server_id import server_id from samba.ndr import ndr_print +import random class MessagingTests(TestCase): @@ -46,20 +47,26 @@ class MessagingTests(TestCase): for name in x.irpc_all_servers(): self.assertTrue(isinstance(x.irpc_servers_byname(name.name), list)) + def test_unknown_name(self): + x = self.get_context() + self.assertRaises(KeyError, + x.irpc_servers_byname, "samba.messaging test NONEXISTING") + def test_assign_server_id(self): x = self.get_context() self.assertTrue(isinstance(x.server_id, server_id)) - def test_add_name(self): + def test_add_remove_name(self): x = self.get_context() - x.irpc_add_name("samba.messaging test") - name_list = x.irpc_servers_byname("samba.messaging test") + name = "samba.messaging test-%d" % random.randint(1, 1000000) + x.irpc_add_name(name) + name_list = x.irpc_servers_byname(name) self.assertEqual(len(name_list), 1) self.assertEqual(ndr_print(x.server_id), ndr_print(name_list[0])) - x.irpc_remove_name("samba.messaging test") - self.assertEqual([], - x.irpc_servers_byname("samba.messaging test")) + x.irpc_remove_name(name) + self.assertRaises(KeyError, + x.irpc_servers_byname, name) def test_ping_speed(self): server_ctx = self.get_context((0, 1)) diff --git a/selftest/knownfail b/selftest/knownfail index d16d7231711..fdb6fe8c140 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -306,3 +306,4 @@ ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.* ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.* ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.* +^samba.tests.messaging.samba.tests.messaging.MessagingTests.test_add_remove_name -- 2.11.0 From 86a136b522fd0613a6bd500006cdcf2a39aa133a Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 23 Mar 2017 15:48:25 +0100 Subject: [PATCH 6/6] server_id_db: Protect against non-0-terminated data records Remove the failing test from knownfail. Signed-off-by: Andrew Bartlett Signed-off-by: Volker Lendecke Reviewed-by: Jeremy Allison Reviewed-by: Garming Sam BUG: https://bugzilla.samba.org/show_bug.cgi?id=12705 (cherry picked from commit e92a20781ca45b8696397cdef424fe8b92bee66b) --- lib/util/server_id_db.c | 22 +++++++++++++++++++++- selftest/knownfail | 1 - 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/util/server_id_db.c b/lib/util/server_id_db.c index 1e65ce25426..49f15b1a5a8 100644 --- a/lib/util/server_id_db.c +++ b/lib/util/server_id_db.c @@ -137,6 +137,7 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name, char idbuf[idbuf_len]; TDB_DATA key; uint8_t *data; + size_t datalen; char *ids, *id; int ret; @@ -155,6 +156,13 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name, return ret; } + datalen = talloc_get_size(data); + if ((datalen == 0) || (data[datalen-1] != '\0')) { + tdb_chainunlock(tdb, key); + TALLOC_FREE(data); + return EINVAL; + } + ids = (char *)data; id = strv_find(ids, idbuf); @@ -165,7 +173,12 @@ int server_id_db_prune_name(struct server_id_db *db, const char *name, } strv_delete(&ids, id); - ret = tdb_store(tdb, key, talloc_tdb_data(ids), TDB_MODIFY); + + if (talloc_get_size(ids) == 0) { + ret = tdb_delete(tdb, key); + } else { + ret = tdb_store(tdb, key, talloc_tdb_data(ids), TDB_MODIFY); + } TALLOC_FREE(data); tdb_chainunlock(tdb, key); @@ -199,6 +212,7 @@ int server_id_db_lookup(struct server_id_db *db, const char *name, struct tdb_context *tdb = db->tdb->tdb; TDB_DATA key; uint8_t *data; + size_t datalen; char *ids, *id; unsigned num_servers; struct server_id *servers; @@ -211,6 +225,12 @@ int server_id_db_lookup(struct server_id_db *db, const char *name, return ret; } + datalen = talloc_get_size(data); + if ((datalen == 0) || (data[datalen-1] != '\0')) { + TALLOC_FREE(data); + return EINVAL; + } + ids = (char *)data; num_servers = strv_count(ids); diff --git a/selftest/knownfail b/selftest/knownfail index fdb6fe8c140..d16d7231711 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -306,4 +306,3 @@ ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_add_duplicate_different_type.* ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_rank_none.* ^samba.tests.dcerpc.dnsserver.samba.tests.dcerpc.dnsserver.DnsserverTests.test_security_descriptor.* -^samba.tests.messaging.samba.tests.messaging.MessagingTests.test_add_remove_name -- 2.11.0