[PATCH] Fix domain backups when SMBv1 is disabled
Tim Beale
timbeale at catalyst.net.nz
Fri Jan 11 03:22:23 UTC 2019
Thanks, I've fixed up the ParseTuple() args. Updated patch-set attached.
Douglas now thinks he's stared at this long enough to count as a
reviewer, but it'd be nice if someone who knows a bit more about SMB
could take a quick look at patch #1.
Getting this patch into 4.10 will mean the 'domain backup' commands will
finally work when SMBv1 is disabled.
On 10/01/19 6:08 PM, Douglas Bagnall wrote:
> On 9/01/19 5:30 PM, Tim Beale via samba-technical wrote:
>> Here's the last few patches required to make domain backups work against
>> a DC with SMBv1 disabled. These patches relies on the
>> libsmb_samba_internal and cli_smb2_list() patches I sent out earlier today.
>>
>> There's not a lot to it - it adds an extra 'get_acl' Python API (to make
>> it consistent with the source4 bindings), then switches over the backup
>> code to use the source3 bindings.
>>
>> CI link: https://gitlab.com/catalyst-samba/samba/pipelines/42628003
>>
>> Review appreciated. Thanks.
> RB+ on 2/3 and 3/3, but somebody who knows SMB really ought to look at 1/3.
>
>> +/*
>> + * Read ACL on a given file/directory as a security descriptor object
>> + */
>> +static PyObject *py_smb_getacl(struct py_cli_state *self, PyObject *args)
>> +{
>> + NTSTATUS status;
>> + const char *filename = NULL;
>> + uint32_t sinfo = SECINFO_DEFAULT_FLAGS;
>> + int access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
> ^^^^^^^^^^^^^^^
>
> One thing I did notice is cli_ntcreate() wants uint32_t for the access
> mask, which would affect the ParseTuple format string too.
>
>> + uint16_t fnum;
>> + struct security_descriptor *sd = NULL;
>> +
>> + /* there's no async version of cli_query_security_descriptor() */
>> + if (self->thread_state != NULL) {
>> + PyErr_SetString(PyExc_RuntimeError,
> (I don't really like that we use RuntimeError everywhere, but this is
> one of the few cases where it perhaps makes sense).
>
>
> Douglas
-------------- next part --------------
From 7fd15d586954a392edec1be66804c4146abf561b Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 12 Dec 2018 16:14:43 +1300
Subject: [PATCH 1/3] s3:pylibsmb: Add .get_acl() API to SMB py bindings
There is no obvious async-equivalent of cli_query_security_descriptor(),
so it will throw an error if anyone tries to use it in multi-threaded
mode. Currently only samba-tool and tests use the (s4) .get_acl() API,
both of which will be fine using the synchronous API.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
source3/libsmb/pylibsmb.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 9acfbb2..e0ce518 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -32,10 +32,16 @@
#include "auth/credentials/pycredentials.h"
#include "trans2.h"
#include "libsmb/clirap.h"
+#include "librpc/rpc/pyrpc_util.h"
#define LIST_ATTRIBUTE_MASK \
(FILE_ATTRIBUTE_DIRECTORY|FILE_ATTRIBUTE_SYSTEM|FILE_ATTRIBUTE_HIDDEN)
+#define SECINFO_DEFAULT_FLAGS \
+ (SECINFO_OWNER | SECINFO_GROUP | \
+ SECINFO_DACL | SECINFO_PROTECTED_DACL | SECINFO_UNPROTECTED_DACL | \
+ SECINFO_SACL | SECINFO_PROTECTED_SACL | SECINFO_UNPROTECTED_SACL)
+
static PyTypeObject *get_pytype(const char *module, const char *type)
{
PyObject *mod;
@@ -1475,6 +1481,50 @@ static PyObject *py_smb_deltree(struct py_cli_state *self, PyObject *args)
Py_RETURN_NONE;
}
+/*
+ * Read ACL on a given file/directory as a security descriptor object
+ */
+static PyObject *py_smb_getacl(struct py_cli_state *self, PyObject *args)
+{
+ NTSTATUS status;
+ const char *filename = NULL;
+ unsigned int sinfo = SECINFO_DEFAULT_FLAGS;
+ unsigned int access_mask = SEC_FLAG_MAXIMUM_ALLOWED;
+ uint16_t fnum;
+ struct security_descriptor *sd = NULL;
+
+ /* there's no async version of cli_query_security_descriptor() */
+ if (self->thread_state != NULL) {
+ PyErr_SetString(PyExc_RuntimeError,
+ "get_acl() is not supported on "
+ "a multi_threaded connection");
+ return NULL;
+ }
+
+ if (!PyArg_ParseTuple(args, "s|II:get_acl", &filename, &sinfo,
+ &access_mask)) {
+ return NULL;
+ }
+
+ /* get a file handle with the desired access */
+ status = cli_ntcreate(self->cli, filename, 0, access_mask, 0,
+ FILE_SHARE_READ|FILE_SHARE_WRITE,
+ FILE_OPEN, 0x0, 0x0, &fnum, NULL);
+ PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+ /* query the security descriptor for this file */
+ status = cli_query_security_descriptor(self->cli, fnum, sinfo,
+ NULL, &sd);
+ PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+ /* close the file handle and convert the SD to a python struct */
+ status = cli_close(self->cli, fnum);
+ PyErr_NTSTATUS_IS_ERR_RAISE(status);
+
+ return py_return_ndr_struct("samba.dcerpc.security", "descriptor",
+ sd, sd);
+}
+
static PyMethodDef py_cli_state_methods[] = {
{ "settimeout", (PyCFunction)py_cli_settimeout, METH_VARARGS,
"settimeout(new_timeout_msecs) => return old_timeout_msecs" },
@@ -1524,6 +1574,9 @@ static PyMethodDef py_cli_state_methods[] = {
{ "deltree", (PyCFunction)py_smb_deltree, METH_VARARGS,
"deltree(path) -> None\n\n"
"\t\tDelete a directory and all its contents." },
+ { "get_acl", (PyCFunction)py_smb_getacl, METH_VARARGS,
+ "get_acl(path[, security_info=0]) -> security_descriptor object\n\n"
+ "\t\tGet security descriptor for file." },
{ NULL, NULL, 0, NULL }
};
--
2.7.4
From 867cbb25153f8318600a32c3c0c723b5fb4cce27 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 13 Dec 2018 16:05:36 +1300
Subject: [PATCH 2/3] tests: Change ntaclsbackup tests over to use s3 Py
bindings
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
python/samba/tests/ntacls_backup.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/python/samba/tests/ntacls_backup.py b/python/samba/tests/ntacls_backup.py
index 0ee044f..03ee821 100644
--- a/python/samba/tests/ntacls_backup.py
+++ b/python/samba/tests/ntacls_backup.py
@@ -19,7 +19,7 @@
"""Tests for samba ntacls backup"""
import os
-from samba import smb
+from samba.samba3 import libsmb_samba_internal as libsmb
from samba.samba3 import smbd
from samba import samdb
from samba import ntacls
@@ -60,7 +60,7 @@ class NtaclsBackupRestoreTests(TestCaseInTempDir):
self.dom_sid)
self.lp = self.ntacls_helper.lp
- self.smb_conn = smb.SMB(
+ self.smb_conn = libsmb.Conn(
self.server, self.service, lp=self.lp, creds=self.creds)
self.smb_helper = ntacls.SMBHelper(self.smb_conn, self.dom_sid)
--
2.7.4
From 8e5bec795e0f1d1df88fa2de00647340083dd559 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 13 Dec 2018 17:31:23 +1300
Subject: [PATCH 3/3] netcmd: Change domain backup commands to use s3 SMB Py
bindings
This means we can now backup a DC that has SMBv1 disabled.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676
Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
python/samba/netcmd/domain_backup.py | 8 ++++++--
selftest/knownfail.d/domain_backup | 12 ------------
2 files changed, 6 insertions(+), 14 deletions(-)
delete mode 100644 selftest/knownfail.d/domain_backup
diff --git a/python/samba/netcmd/domain_backup.py b/python/samba/netcmd/domain_backup.py
index 58d5a4c..4cacf57 100644
--- a/python/samba/netcmd/domain_backup.py
+++ b/python/samba/netcmd/domain_backup.py
@@ -27,7 +27,8 @@ import tdb
import samba.getopt as options
from samba.samdb import SamDB, get_default_backend_store
import ldb
-from samba import smb
+from samba.samba3 import libsmb_samba_internal as libsmb
+from samba.samba3 import param as s3param
from samba.ntacls import backup_online, backup_restore, backup_offline
from samba.auth import system_session
from samba.join import DCJoinContext, join_clone, DCCloneAndRenameContext
@@ -103,7 +104,10 @@ def get_sid_for_restore(samdb):
def smb_sysvol_conn(server, lp, creds):
"""Returns an SMB connection to the sysvol share on the DC"""
- return smb.SMB(server, "sysvol", lp=lp, creds=creds, sign=True)
+ # the SMB bindings rely on having a s3 loadparm
+ s3_lp = s3param.get_context()
+ s3_lp.load(lp.configfile)
+ return libsmb.Conn(server, "sysvol", lp=s3_lp, creds=creds, sign=True)
def get_timestamp():
diff --git a/selftest/knownfail.d/domain_backup b/selftest/knownfail.d/domain_backup
deleted file mode 100644
index 24f4d87..0000000
--- a/selftest/knownfail.d/domain_backup
+++ /dev/null
@@ -1,12 +0,0 @@
-# these tests only work with SMBv1, which is disabled on the restoredc
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_one_way_links\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_untar\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_restore_with_conf\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_restore_no_secrets\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_restore_into_site\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupRename.test_backup_restore\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_untar\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_restore_with_conf\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_restore_no_secrets\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_restore_into_site\(restoredc:local\)
-samba.tests.domain_backup.samba.tests.domain_backup.DomainBackupOnline.test_backup_restore\(restoredc:local\)
--
2.7.4
More information about the samba-technical
mailing list