[RFC] Advice on SMB client python bindings?

Tim Beale timbeale at catalyst.net.nz
Tue Dec 11 03:24:33 UTC 2018


Hi Metze,

I'm not sure I'm the best person to review all this, but it looked OK to
me. I made some small changes (explained more below), added my review
tags, and pushed it back to CI:
https://gitlab.com/samba-team/devel/samba/pipelines/39657191

Someone else still needs to review the patches that I authored in the
series.

>> python/tests: also initialize the s3 loadparm subsystem

I've dropped this patch for now. It looks fine, but I don't think it
needs to go in until later (when we actually switchover the
samba.tests.smb code to use the source3 bindings). My concern is it only
fixes the tests and won't fix things like samba-tool. I was wondering if
we need to add an extra 'lp' param (i.e. along with 'force_smb1',
'sign', etc), so a valid (s3) loadparm always gets passed in? Anyway,
it's something we can discuss further, but it doesn't need to hold up
these changes.

>> s3:pylibsmb: make use of protocol independent cli_read_send/recv in
py_cli_read()

I made a small tweak to get rid of some unnecessary casts.

>> s3:libsmb: add cli_write_send/recv which work with SMB1/2/3

I added some code comments (in a separate patch) to try to explain the
difference between the new cli_write_send() and cli_push_send().

Cheers,
Tim

On 11/12/18 12:21 PM, Stefan Metzmacher wrote:
> Am 04.12.18 um 17:44 schrieb Stefan Metzmacher via samba-technical:
>> Hi Tim,
>>
>>> Thanks for the patches Metze. Unfortunately that doesn't help me with
>>> supporting SMBv2 at all. Attached is an example test-case
>>> (smbv2-problem.txt) that tries to run the .unlink() API against a DC
>>> with SMBv1 disabled.
>> Yes, sorry! I looked briefly at cli_unlink() and thought _send/_recv
>> would already do the correct thing and only others like cli_mkdir()
>> had the problem. But it seems I was just dreaming...
>>
>>> Taking a step back, the problem I'm trying to solve is this:
>>>
>>> Finally, I don't really understand why using the async APIs in the
>>> Python bindings is so important. For example, smbclient uses the
>>> synchronous APIs. If someone could explain the rationale behind all
>>> this, it might help. Or to put it another way, why is using the async
>>> APIs more important than delivering software that fixes a real problem
>>> for real Samba users?
>> Yes, removing the dependency to SMB1 should be our first priority.
>>
>> The async stuff is mainly for testing the async protocol behavior,
>> in order to simulate multi threaded applications.
>>
>> Would you be able to work from the attached patchset?
> Here's a bit more that's needed for my dcerpc security context
> multiplexing support.
>
> Can I get your review an this?
>
> Then I'll add BUG references to
> https://bugzilla.samba.org/show_bug.cgi?id=13676
> https://bugzilla.samba.org/show_bug.cgi?id=7113
> https://bugzilla.samba.org/show_bug.cgi?id=11892
> and maybe more.
>
> A pipeline is running here:
> https://gitlab.com/samba-team/devel/samba/pipelines/39644216
>
> Thanks!
> metze
>
-------------- next part --------------
From 62aa48bb8832bc01b3eaa038d52a1a8212f1ae44 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 3 Dec 2018 14:37:05 +0100
Subject: [PATCH 01/23] s3:pylibsmb: pass self to py_tevent_req_wait_exc()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/pylibsmb.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 99e5587..75236e7 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -364,7 +364,7 @@ static int py_tevent_req_wait(struct tevent_context *ev,
 
 #endif
 
-static bool py_tevent_req_wait_exc(struct tevent_context *ev,
+static bool py_tevent_req_wait_exc(struct py_cli_state *self,
 				   struct tevent_req *req)
 {
 	int ret;
@@ -373,7 +373,7 @@ static bool py_tevent_req_wait_exc(struct tevent_context *ev,
 		PyErr_NoMemory();
 		return false;
 	}
-	ret = py_tevent_req_wait(ev, req);
+	ret = py_tevent_req_wait(self->ev, req);
 	if (ret != 0) {
 		TALLOC_FREE(req);
 		errno = ret;
@@ -453,7 +453,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	req = cli_full_connection_creds_send(
 		NULL, self->ev, "myname", host, NULL, 0, share, "?????",
 		cli_creds, flags, SMB_SIGNING_DEFAULT);
-	if (!py_tevent_req_wait_exc(self->ev, req)) {
+	if (!py_tevent_req_wait_exc(self, req)) {
 		return -1;
 	}
 	status = cli_full_connection_creds_recv(req, &self->cli);
@@ -612,7 +612,7 @@ static PyObject *py_cli_create(struct py_cli_state *self, PyObject *args,
 				DesiredAccess, FileAttributes, ShareAccess,
 				CreateDisposition, CreateOptions,
 				SecurityFlags);
-	if (!py_tevent_req_wait_exc(self->ev, req)) {
+	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
 	status = cli_ntcreate_recv(req, &fnum, NULL);
@@ -636,7 +636,7 @@ static PyObject *py_cli_close(struct py_cli_state *self, PyObject *args)
 	}
 
 	req = cli_close_send(NULL, self->ev, self->cli, fnum);
-	if (!py_tevent_req_wait_exc(self->ev, req)) {
+	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
 	status = cli_close_recv(req);
@@ -672,7 +672,7 @@ static PyObject *py_cli_write(struct py_cli_state *self, PyObject *args,
 
 	req = cli_write_andx_send(NULL, self->ev, self->cli, fnum, mode,
 				  (uint8_t *)buf, offset, buflen);
-	if (!py_tevent_req_wait_exc(self->ev, req)) {
+	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
 	status = cli_write_andx_recv(req, &written);
@@ -708,7 +708,7 @@ static PyObject *py_cli_read(struct py_cli_state *self, PyObject *args,
 
 	req = cli_read_andx_send(NULL, self->ev, self->cli, fnum,
 				 offset, size);
-	if (!py_tevent_req_wait_exc(self->ev, req)) {
+	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
 	status = cli_read_andx_recv(req, &buflen, &buf);
@@ -740,7 +740,7 @@ static PyObject *py_cli_ftruncate(struct py_cli_state *self, PyObject *args,
 	}
 
 	req = cli_ftruncate_send(NULL, self->ev, self->cli, fnum, size);
-	if (!py_tevent_req_wait_exc(self->ev, req)) {
+	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
 	status = cli_ftruncate_recv(req);
@@ -771,7 +771,7 @@ static PyObject *py_cli_delete_on_close(struct py_cli_state *self,
 
 	req = cli_nt_delete_on_close_send(NULL, self->ev, self->cli, fnum,
 					  flag);
-	if (!py_tevent_req_wait_exc(self->ev, req)) {
+	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
 	status = cli_nt_delete_on_close_recv(req);
@@ -812,7 +812,7 @@ static PyObject *py_cli_list(struct py_cli_state *self,
 
 	req = cli_list_send(NULL, self->ev, self->cli, mask, attribute,
 			    info_level);
-	if (!py_tevent_req_wait_exc(self->ev, req)) {
+	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
 	status = cli_list_recv(req, NULL, &finfos, &num_finfos);
-- 
2.7.4


From 9fdd147e8ca3b5f177c4cc011e6524c0bedccef1 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 3 Dec 2018 15:02:06 +0100
Subject: [PATCH 02/23] s3:pylibsmb: only use poll_mt backend if
 multi_threaded=True is specified

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 python/samba/tests/libsmb_samba_internal.py |  3 +-
 source3/libsmb/pylibsmb.c                   | 52 +++++++++++++++++++++++------
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/python/samba/tests/libsmb_samba_internal.py b/python/samba/tests/libsmb_samba_internal.py
index c88095c..db99c0b 100644
--- a/python/samba/tests/libsmb_samba_internal.py
+++ b/python/samba/tests/libsmb_samba_internal.py
@@ -59,7 +59,8 @@ class LibsmbTestCase(samba.tests.TestCase):
         creds.set_username(os.getenv("USERNAME"))
         creds.set_password(os.getenv("PASSWORD"))
 
-        c = libsmb_samba_internal.Conn(os.getenv("SERVER_IP"), "tmp", creds)
+        c = libsmb_samba_internal.Conn(os.getenv("SERVER_IP"), "tmp",
+                                       creds, multi_threaded=True)
 
         mythreads = []
 
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 75236e7..a18fbaf 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -81,6 +81,8 @@ struct py_cli_state {
 	PyObject_HEAD
 	struct cli_state *cli;
 	struct tevent_context *ev;
+	int (*req_wait_fn)(struct tevent_context *ev,
+			   struct tevent_req *req);
 	struct py_cli_thread *thread_state;
 
 	struct tevent_req *oplock_waiter;
@@ -194,7 +196,10 @@ static int py_cli_thread_destructor(struct py_cli_thread *t)
 	return 0;
 }
 
-static bool py_cli_state_setup_ev(struct py_cli_state *self)
+static int py_tevent_cond_req_wait(struct tevent_context *ev,
+				   struct tevent_req *req);
+
+static bool py_cli_state_setup_mt_ev(struct py_cli_state *self)
 {
 	struct py_cli_thread *t = NULL;
 	int ret;
@@ -206,6 +211,8 @@ static bool py_cli_state_setup_ev(struct py_cli_state *self)
 	samba_tevent_set_debug(self->ev, "pylibsmb_tevent_mt");
 	tevent_set_trace_callback(self->ev, py_cli_state_trace_callback, self);
 
+	self->req_wait_fn = py_tevent_cond_req_wait;
+
 	self->thread_state = talloc_zero(NULL, struct py_cli_thread);
 	if (self->thread_state == NULL) {
 		goto fail;
@@ -303,8 +310,8 @@ fail:
 	return result;
 }
 
-static int py_tevent_req_wait(struct tevent_context *ev,
-			      struct tevent_req *req)
+static int py_tevent_cond_req_wait(struct tevent_context *ev,
+				   struct tevent_req *req)
 {
 	struct py_tevent_cond cond;
 	tevent_req_set_callback(req, py_tevent_signalme, &cond);
@@ -334,7 +341,10 @@ static void py_tevent_signalme(struct tevent_req *req)
 	py_tevent_cond_signal(cond);
 }
 
-#else
+#endif
+
+static int py_tevent_req_wait(struct tevent_context *ev,
+			      struct tevent_req *req);
 
 static bool py_cli_state_setup_ev(struct py_cli_state *self)
 {
@@ -345,6 +355,8 @@ static bool py_cli_state_setup_ev(struct py_cli_state *self)
 
 	samba_tevent_set_debug(self->ev, "pylibsmb_tevent");
 
+	self->req_wait_fn = py_tevent_req_wait;
+
 	return true;
 }
 
@@ -362,8 +374,6 @@ static int py_tevent_req_wait(struct tevent_context *ev,
 	return 0;
 }
 
-#endif
-
 static bool py_tevent_req_wait_exc(struct py_cli_state *self,
 				   struct tevent_req *req)
 {
@@ -373,7 +383,7 @@ static bool py_tevent_req_wait_exc(struct py_cli_state *self,
 		PyErr_NoMemory();
 		return false;
 	}
-	ret = py_tevent_req_wait(self->ev, req);
+	ret = self->req_wait_fn(self->ev, req);
 	if (ret != 0) {
 		TALLOC_FREE(req);
 		errno = ret;
@@ -410,6 +420,8 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	char *host, *share;
 	PyObject *creds = NULL;
 	struct cli_credentials *cli_creds;
+	PyObject *py_multi_threaded = Py_False;
+	bool multi_threaded = false;
 	struct tevent_req *req;
 	bool ret;
 	/*
@@ -421,7 +433,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	int flags = CLI_FULL_CONNECTION_FORCE_SMB1;
 
 	static const char *kwlist[] = {
-		"host", "share", "credentials", NULL
+		"host", "share", "credentials", "multi_threaded", NULL
 	};
 
 	PyTypeObject *py_type_Credentials = get_pytype(
@@ -431,8 +443,10 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	}
 
 	ret = ParseTupleAndKeywords(
-		args, kwds, "ss|O!", kwlist,
-		&host, &share, py_type_Credentials, &creds);
+		args, kwds, "ss|O!O", kwlist,
+		&host, &share,
+		py_type_Credentials, &creds,
+		&py_multi_threaded);
 
 	Py_DECREF(py_type_Credentials);
 
@@ -440,8 +454,24 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 		return -1;
 	}
 
-	if (!py_cli_state_setup_ev(self)) {
+	multi_threaded = PyObject_IsTrue(py_multi_threaded);
+
+	if (multi_threaded) {
+#ifdef HAVE_PTHREAD
+		ret = py_cli_state_setup_mt_ev(self);
+		if (!ret) {
+			return -1;
+		}
+#else
+		PyErr_SetString(PyExc_RuntimeError,
+				"No PTHREAD support available");
 		return -1;
+#endif
+	} else {
+		ret = py_cli_state_setup_ev(self);
+		if (!ret) {
+			return -1;
+		}
 	}
 
 	if (creds == NULL) {
-- 
2.7.4


From af92c5b1b6bb489104640ffb83fc11cd1bf66b38 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 3 Dec 2018 15:42:50 +0100
Subject: [PATCH 03/23] s3:pylibsmb: add sign=True to require signing

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/pylibsmb.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index a18fbaf..8e63cbf 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -422,6 +422,9 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	struct cli_credentials *cli_creds;
 	PyObject *py_multi_threaded = Py_False;
 	bool multi_threaded = false;
+	PyObject *py_sign = Py_False;
+	bool sign = false;
+	int signing_state = SMB_SIGNING_DEFAULT;
 	struct tevent_req *req;
 	bool ret;
 	/*
@@ -433,7 +436,8 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	int flags = CLI_FULL_CONNECTION_FORCE_SMB1;
 
 	static const char *kwlist[] = {
-		"host", "share", "credentials", "multi_threaded", NULL
+		"host", "share", "credentials",
+		"multi_threaded", "sign", NULL
 	};
 
 	PyTypeObject *py_type_Credentials = get_pytype(
@@ -443,10 +447,11 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	}
 
 	ret = ParseTupleAndKeywords(
-		args, kwds, "ss|O!O", kwlist,
+		args, kwds, "ss|O!OO", kwlist,
 		&host, &share,
 		py_type_Credentials, &creds,
-		&py_multi_threaded);
+		&py_multi_threaded,
+		&py_sign);
 
 	Py_DECREF(py_type_Credentials);
 
@@ -455,6 +460,11 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	}
 
 	multi_threaded = PyObject_IsTrue(py_multi_threaded);
+	sign = PyObject_IsTrue(py_sign);
+
+	if (sign) {
+		signing_state = SMB_SIGNING_REQUIRED;
+	}
 
 	if (multi_threaded) {
 #ifdef HAVE_PTHREAD
@@ -482,7 +492,7 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 
 	req = cli_full_connection_creds_send(
 		NULL, self->ev, "myname", host, NULL, 0, share, "?????",
-		cli_creds, flags, SMB_SIGNING_DEFAULT);
+		cli_creds, flags, signing_state);
 	if (!py_tevent_req_wait_exc(self, req)) {
 		return -1;
 	}
-- 
2.7.4


From 7951bcf62b8a3cae917c59bcc51a1fcea8ed5bbe Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 4 Dec 2018 10:40:18 +0100
Subject: [PATCH 04/23] s3:pylibsmb: add force_smb1=True in order to control
 forcing of SMB1

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 python/samba/tests/libsmb_samba_internal.py |  3 ++-
 source3/libsmb/pylibsmb.c                   | 35 ++++++++++++++++++++---------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/python/samba/tests/libsmb_samba_internal.py b/python/samba/tests/libsmb_samba_internal.py
index db99c0b..8918d84 100644
--- a/python/samba/tests/libsmb_samba_internal.py
+++ b/python/samba/tests/libsmb_samba_internal.py
@@ -60,7 +60,8 @@ class LibsmbTestCase(samba.tests.TestCase):
         creds.set_password(os.getenv("PASSWORD"))
 
         c = libsmb_samba_internal.Conn(os.getenv("SERVER_IP"), "tmp",
-                                       creds, multi_threaded=True)
+                                       creds, multi_threaded=True,
+                                       force_smb1=True)
 
         mythreads = []
 
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 8e63cbf..ed3c041 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -425,19 +425,16 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	PyObject *py_sign = Py_False;
 	bool sign = false;
 	int signing_state = SMB_SIGNING_DEFAULT;
+	PyObject *py_force_smb1 = Py_False;
+	bool force_smb1 = false;
 	struct tevent_req *req;
 	bool ret;
-	/*
-	 * For now we only support SMB1,
-	 * as most of the cli_*_send() function
-	 * don't support SMB2, it's only plugged
-	 * into the sync wrapper functions currently.
-	 */
-	int flags = CLI_FULL_CONNECTION_FORCE_SMB1;
+	int flags = 0;
 
 	static const char *kwlist[] = {
 		"host", "share", "credentials",
-		"multi_threaded", "sign", NULL
+		"multi_threaded", "sign", "force_smb1",
+		NULL
 	};
 
 	PyTypeObject *py_type_Credentials = get_pytype(
@@ -447,11 +444,12 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 	}
 
 	ret = ParseTupleAndKeywords(
-		args, kwds, "ss|O!OO", kwlist,
+		args, kwds, "ss|O!OOO", kwlist,
 		&host, &share,
 		py_type_Credentials, &creds,
 		&py_multi_threaded,
-		&py_sign);
+		&py_sign,
+		&py_force_smb1);
 
 	Py_DECREF(py_type_Credentials);
 
@@ -461,11 +459,22 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 
 	multi_threaded = PyObject_IsTrue(py_multi_threaded);
 	sign = PyObject_IsTrue(py_sign);
+	force_smb1 = PyObject_IsTrue(py_force_smb1);
 
 	if (sign) {
 		signing_state = SMB_SIGNING_REQUIRED;
 	}
 
+	if (force_smb1) {
+		/*
+		 * As most of the cli_*_send() function
+		 * don't support SMB2 (it's only plugged
+		 * into the sync wrapper functions currently)
+		 * we have a way to force SMB1.
+		 */
+		flags = CLI_FULL_CONNECTION_FORCE_SMB1;
+	}
+
 	if (multi_threaded) {
 #ifdef HAVE_PTHREAD
 		ret = py_cli_state_setup_mt_ev(self);
@@ -477,6 +486,12 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 				"No PTHREAD support available");
 		return -1;
 #endif
+		if (!force_smb1) {
+			PyErr_SetString(PyExc_RuntimeError,
+					"multi_threaded is only possible on "
+					"SMB1 connections");
+			return -1;
+		}
 	} else {
 		ret = py_cli_state_setup_ev(self);
 		if (!ret) {
-- 
2.7.4


From bb1ee6704db42e7d5074605f97d96963e9f2fede Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Tue, 4 Dec 2018 10:42:55 +0100
Subject: [PATCH 05/23] s3:pylibsmb: remember that a connection uses SMB1

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/pylibsmb.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index ed3c041..e4552a2 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -21,6 +21,7 @@
 #include <Python.h>
 #include "includes.h"
 #include "python/py3compat.h"
+#include "libcli/smb/smbXcli_base.h"
 #include "libsmb/libsmb.h"
 #include "libcli/security/security.h"
 #include "system/select.h"
@@ -80,6 +81,7 @@ struct py_cli_oplock_break {
 struct py_cli_state {
 	PyObject_HEAD
 	struct cli_state *cli;
+	bool is_smb1;
 	struct tevent_context *ev;
 	int (*req_wait_fn)(struct tevent_context *ev,
 			   struct tevent_req *req);
@@ -403,6 +405,7 @@ static PyObject *py_cli_state_new(PyTypeObject *type, PyObject *args,
 		return NULL;
 	}
 	self->cli = NULL;
+	self->is_smb1 = false;
 	self->ev = NULL;
 	self->thread_state = NULL;
 	self->oplock_waiter = NULL;
@@ -519,6 +522,10 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 		return -1;
 	}
 
+	if (smbXcli_conn_protocol(self->cli->conn) < PROTOCOL_SMB2_02) {
+		self->is_smb1 = true;
+	}
+
 	self->oplock_waiter = cli_smb_oplock_break_waiter_send(
 		self->ev, self->ev, self->cli);
 	if (self->oplock_waiter == NULL) {
-- 
2.7.4


From 7825a3939f79e12556091fcd617dbbc2d8ca52cf Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 4 Dec 2018 12:32:58 +1300
Subject: [PATCH 06/23] s3:pylibsmb: .get_oplock_break API is dependent on
 multi_threaded=True

The .get_oplock_break is dependent on the pthread code, which is only
used when creating a SMB connection with multi_threaded=True.

Add an explicit error to the .get_oplock_break() if someone tries to use
it in non-multithreaded mode.

Initializing self->oplock_waiter in non-multithreaded mode is similarly
redundant if the API can never be used.

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/libsmb/pylibsmb.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index e4552a2..19587a8 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -526,6 +526,13 @@ static int py_cli_state_init(struct py_cli_state *self, PyObject *args,
 		self->is_smb1 = true;
 	}
 
+	/*
+	 * Oplocks require a multi threaded connection
+	 */
+	if (self->thread_state == NULL) {
+		return 0;
+	}
+
 	self->oplock_waiter = cli_smb_oplock_break_waiter_send(
 		self->ev, self->ev, self->cli);
 	if (self->oplock_waiter == NULL) {
@@ -585,6 +592,13 @@ static PyObject *py_cli_get_oplock_break(struct py_cli_state *self,
 		return NULL;
 	}
 
+	if (self->thread_state == NULL) {
+		PyErr_SetString(PyExc_RuntimeError,
+				"get_oplock_break() only possible on "
+				"a multi_threaded connection");
+		return NULL;
+	}
+
 	if (self->oplock_cond != NULL) {
 		errno = EBUSY;
 		PyErr_SetFromErrno(PyExc_RuntimeError);
-- 
2.7.4


From 3d03805248d12342bd5eb975f7759490b021cb91 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 11:01:14 +1300
Subject: [PATCH 07/23] tests: Add SMB Py binding .chkpath() test case

chkpath was only tested incidentally (and that assertion was wrong). Add
a proper test to prove it works correctly. We can then clean-up the
incorrect assertion in the next patch.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/smb.py | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index e0e60e3..6f33ff1 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -65,6 +65,26 @@ class SMBTests(samba.tests.TestCase):
         self.conn.unlink(test_file)
         self.assertFalse(self.conn.chkpath(test_file))
 
+    def test_chkpath(self):
+        """Tests .chkpath determines whether or not a directory exists"""
+
+        self.assertTrue(self.conn.chkpath(test_dir))
+
+        # should return False for a non-existent directory
+        bad_dir = self.make_sysvol_path(test_dir, 'dont_exist')
+        self.assertFalse(self.conn.chkpath(bad_dir))
+
+        # should return False for files (because they're not directories)
+        self.conn.savefile(test_file, binary_contents)
+        self.assertFalse(self.conn.chkpath(test_file))
+
+        # check correct result after creating and then deleting a new dir
+        new_dir = self.make_sysvol_path(test_dir, 'test-new')
+        self.conn.mkdir(new_dir)
+        self.assertTrue(self.conn.chkpath(new_dir))
+        self.conn.rmdir(new_dir)
+        self.assertFalse(self.conn.chkpath(new_dir))
+
     def test_save_load_text(self):
 
         self.conn.savefile(test_file, test_contents.encode('utf8'))
@@ -99,3 +119,7 @@ class SMBTests(samba.tests.TestCase):
         contents = self.conn.loadfile(test_file)
         self.assertEquals(contents, binary_contents,
                           msg='contents of test file did not match what was written')
+
+    def make_sysvol_path(self, dirpath, filename):
+        # return the dir + filename as a sysvol path
+        return os.path.join(dirpath, filename).replace('/', '\\')
-- 
2.7.4


From eb3607f756bf55389ef743d33e2ab6d856ffe5e2 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 11:15:14 +1300
Subject: [PATCH 08/23] tests: Fix SMB Py binding .unlink() test case assertion

The current assertion would never detect if the unlink API is broken.
The chkpath() API is only useful for checking if directories exist, so
it will always return False for a regular file (regardless of whether
the file actually exists or not).

Rework the test case so we assert that the file exists by trying to read
its contents (which will throw an error if the file doesn't exist).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/smb.py | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 6f33ff1..e4366ce 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -20,6 +20,8 @@ import os
 import random
 import sys
 from samba import smb
+from samba import NTSTATUSError
+from samba.ntstatus import NT_STATUS_OBJECT_NAME_NOT_FOUND
 
 PY3 = sys.version_info[0] == 3
 addom = 'addom.samba.example.com/'
@@ -57,13 +59,30 @@ class SMBTests(samba.tests.TestCase):
         self.assertIn('Policies', ls,
                       msg='"Policies" directory not found in sysvol')
 
+    def file_exists(self, filepath):
+        """Returns whether a regular file exists (by trying to open it)"""
+        try:
+            self.conn.loadfile(filepath)
+            exists = True;
+        except NTSTATUSError as err:
+            if err.args[0] == NT_STATUS_OBJECT_NAME_NOT_FOUND:
+                exists = False
+            else:
+                raise err
+        return exists
+
     def test_unlink(self):
         """
         The smb.unlink API should delete file
         """
+        # create the test file
+        self.assertFalse(self.file_exists(test_file))
         self.conn.savefile(test_file, binary_contents)
+        self.assertTrue(self.file_exists(test_file))
+
+        # delete it and check that it's gone
         self.conn.unlink(test_file)
-        self.assertFalse(self.conn.chkpath(test_file))
+        self.assertFalse(self.file_exists(test_file))
 
     def test_chkpath(self):
         """Tests .chkpath determines whether or not a directory exists"""
-- 
2.7.4


From ec1f9e57d1106f83d5274605b7f8f6d0d02dec12 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Mon, 3 Dec 2018 17:22:43 +1300
Subject: [PATCH 09/23] tests: Extend SMB Py binding .list() test-case

Extend the tests to better reflect some of the .list() functionality we
expect.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/smb.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index e4366ce..70d6cd8 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -53,11 +53,33 @@ class SMBTests(samba.tests.TestCase):
             pass
 
     def test_list(self):
+        # check a basic listing returns the items we expect
         ls = [f['name'] for f in self.conn.list(addom)]
         self.assertIn('scripts', ls,
                       msg='"scripts" directory not found in sysvol')
         self.assertIn('Policies', ls,
                       msg='"Policies" directory not found in sysvol')
+        self.assertNotIn('..', ls,
+                         msg='Parent (..) found in directory listing')
+        self.assertNotIn('.', ls,
+                         msg='Current dir (.) found in directory listing')
+
+        # using a '*' mask should be the same as using no mask
+        ls_wildcard = [f['name'] for f in self.conn.list(addom, "*")]
+        self.assertEqual(ls, ls_wildcard)
+
+        # applying a mask should only return items that match that mask
+        ls_pol = [f['name'] for f in self.conn.list(addom, "Pol*")]
+        expected = ["Policies"]
+        self.assertEqual(ls_pol, expected)
+
+        # each item in the listing is a has with expected keys
+        expected_keys = ['attrib', 'mtime', 'name', 'short_name', 'size']
+        for item in self.conn.list(addom):
+            for key in expected_keys:
+                self.assertIn(key, item,
+                              msg="Key '%s' not in listing '%s'" % (key, item))
+
 
     def file_exists(self, filepath):
         """Returns whether a regular file exists (by trying to open it)"""
-- 
2.7.4


From 0e394d29730a83edbf2986356c7170f9b3a9b6fd Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Wed, 5 Dec 2018 12:51:22 +1300
Subject: [PATCH 10/23] tests: Extend SMB test_save_load_text case to check
 overwrite

Extend the test case to check overwriting a file as well. Currently this
has the behaviour of appending to the existing file, rather than
overwriting the file with new contents.

It's not clear from the API that this is the intended behaviour in this
case, so I've marked it as a failure.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/smb.py | 7 +++++++
 selftest/knownfail.d/smb  | 3 +++
 2 files changed, 10 insertions(+)
 create mode 100644 selftest/knownfail.d/smb

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index 70d6cd8..e3af777 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -134,6 +134,13 @@ class SMBTests(samba.tests.TestCase):
         self.assertEquals(contents.decode('utf8'), test_contents,
                           msg='contents of test file did not match what was written')
 
+        # check we can overwrite the file with new contents
+        new_contents = 'wxyz' * 128
+        self.conn.savefile(test_file, new_contents.encode('utf8'))
+        contents = self.conn.loadfile(test_file)
+        self.assertEquals(contents.decode('utf8'), new_contents,
+                          msg='contents of test file did not match what was written')
+
     # with python2 this will save/load str type (with embedded nulls)
     # with python3 this will save/load bytes type
     def test_save_load_string_bytes(self):
diff --git a/selftest/knownfail.d/smb b/selftest/knownfail.d/smb
new file mode 100644
index 0000000..32989a8
--- /dev/null
+++ b/selftest/knownfail.d/smb
@@ -0,0 +1,3 @@
+# currently savefile appends rather than overwriting
+samba.tests.smb.samba.tests.smb.SMBTests.test_save_load_text\(ad_dc:local\)
+samba.tests.smb.python2.samba.tests.smb.SMBTests.test_save_load_text\(ad_dc:local\)
-- 
2.7.4


From d2324bb1f100f265c52906b56b994c41befef4af Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 6 Dec 2018 16:03:23 +1300
Subject: [PATCH 11/23] s4:libcli: Fix error in smbcli_deltree()

Commit 094afe614b6282 fixed an uninitialized variable, which meant we
tried to delete the file twice. The 2nd time fails, so the function
returns an error, instead of success (even though the file is now gone).

Note we want to be using the source3 SMB library code going forward.
However, fixing this bug makes it easier to write tests against the
(currently s4) SMB python bindings.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source4/libcli/clideltree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/libcli/clideltree.c b/source4/libcli/clideltree.c
index 7bce95c..e8007f4 100644
--- a/source4/libcli/clideltree.c
+++ b/source4/libcli/clideltree.c
@@ -99,7 +99,7 @@ int smbcli_deltree(struct smbcli_tree *tree, const char *dname)
 
 	/* it might be a file */
 	status = smbcli_unlink(tree, dname);
-	if (NT_STATUS_IS_OK(smbcli_unlink(tree, dname))) {
+	if (NT_STATUS_IS_OK(status)) {
 		return 1;
 	}
 	if (NT_STATUS_EQUAL(smbcli_nt_error(tree), NT_STATUS_OBJECT_NAME_NOT_FOUND) ||
-- 
2.7.4


From 7e2d4242925a12751da9ab929db11af6e9f0a367 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Thu, 6 Dec 2018 16:16:36 +1300
Subject: [PATCH 12/23] tests: Add SMB Py binding .deltree test case

Add a more thorough test case that .deltree works as expected.

Note that we get a slightly different NT_STATUS error in file_exists()
if the parent directory doesn't exist, e.g.
/non-existent-dir/nonexistent.txt

BUG: https://bugzilla.samba.org/show_bug.cgi?id=13676

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 python/samba/tests/smb.py | 62 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/python/samba/tests/smb.py b/python/samba/tests/smb.py
index e3af777..6893647 100644
--- a/python/samba/tests/smb.py
+++ b/python/samba/tests/smb.py
@@ -21,7 +21,8 @@ import random
 import sys
 from samba import smb
 from samba import NTSTATUSError
-from samba.ntstatus import NT_STATUS_OBJECT_NAME_NOT_FOUND
+from samba.ntstatus import (NT_STATUS_OBJECT_NAME_NOT_FOUND,
+                            NT_STATUS_OBJECT_PATH_NOT_FOUND)
 
 PY3 = sys.version_info[0] == 3
 addom = 'addom.samba.example.com/'
@@ -80,6 +81,62 @@ class SMBTests(samba.tests.TestCase):
                 self.assertIn(key, item,
                               msg="Key '%s' not in listing '%s'" % (key, item))
 
+    def test_deltree(self):
+        """The smb.deltree API should delete files and sub-dirs"""
+        # create some test sub-dirs
+        dirpaths = []
+        empty_dirs = []
+        cur_dir = test_dir
+        for subdir in ["subdir-X", "subdir-Y", "subdir-Z"]:
+            path = self.make_sysvol_path(cur_dir, subdir)
+            self.conn.mkdir(path)
+            dirpaths.append(path)
+            cur_dir = path
+
+            # create another empty dir just for kicks
+            path = self.make_sysvol_path(cur_dir, "another")
+            self.conn.mkdir(path)
+            empty_dirs.append(path)
+
+        # create some files in these directories
+        filepaths = []
+        for subdir in dirpaths:
+            for i in range(1, 4):
+                contents = "I'm file {0} in dir {1}!".format(i, subdir)
+                path = self.make_sysvol_path(subdir, "file-{0}.txt".format(i))
+                self.conn.savefile(path, test_contents.encode('utf8'))
+                filepaths.append(path)
+
+        # sanity-check these dirs/files exist
+        for subdir in dirpaths + empty_dirs:
+            self.assertTrue(self.conn.chkpath(subdir),
+                            "Failed to create {0}".format(subdir))
+        for path in filepaths:
+            self.assertTrue(self.file_exists(path),
+                            "Failed to create {0}".format(path))
+
+        # try using deltree to remove a single empty directory
+        path = empty_dirs.pop(0)
+        self.conn.deltree(path)
+        self.assertFalse(self.conn.chkpath(path),
+                         "Failed to delete {0}".format(path))
+
+        # try using deltree to remove a single file
+        path = filepaths.pop(0)
+        self.conn.deltree(path)
+        self.assertFalse(self.file_exists(path),
+                         "Failed to delete {0}".format(path))
+
+        # delete the top-level dir
+        self.conn.deltree(test_dir)
+
+        # now check that all the dirs/files are no longer there
+        for subdir in dirpaths + empty_dirs:
+            self.assertFalse(self.conn.chkpath(subdir),
+                             "Failed to delete {0}".format(subdir))
+        for path in filepaths:
+            self.assertFalse(self.file_exists(path),
+                             "Failed to delete {0}".format(path))
 
     def file_exists(self, filepath):
         """Returns whether a regular file exists (by trying to open it)"""
@@ -87,7 +144,8 @@ class SMBTests(samba.tests.TestCase):
             self.conn.loadfile(filepath)
             exists = True;
         except NTSTATUSError as err:
-            if err.args[0] == NT_STATUS_OBJECT_NAME_NOT_FOUND:
+            if (err.args[0] == NT_STATUS_OBJECT_NAME_NOT_FOUND or
+                err.args[0] == NT_STATUS_OBJECT_PATH_NOT_FOUND):
                 exists = False
             else:
                 raise err
-- 
2.7.4


From c1522c36733855081e9c5a7f3cec2b68f802c0cb Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 13:47:40 +0100
Subject: [PATCH 13/23] s3:pylibsmb: make use of PyBytes_FromStringAndSize() in
 py_cli_read()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/pylibsmb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 19587a8..bb1d2b7 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -794,7 +794,7 @@ static PyObject *py_cli_read(struct py_cli_state *self, PyObject *args,
 		PyErr_SetNTSTATUS(status);
 		return NULL;
 	}
-	result = Py_BuildValue("s#", (char *)buf, (int)buflen);
+	result = PyBytes_FromStringAndSize((const char *)buf, buflen);
 	TALLOC_FREE(req);
 	return result;
 }
-- 
2.7.4


From 59608534f20a5db052af89e9a6fbada633a4c1bc Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 14:04:30 +0100
Subject: [PATCH 14/23] s3:pylibsmb: make use of PYARG_BYTES_LEN in
 py_cli_write()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/pylibsmb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index bb1d2b7..3b1aa73 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -741,7 +741,7 @@ static PyObject *py_cli_write(struct py_cli_state *self, PyObject *args,
 		"fnum", "buffer", "offset", "mode", NULL };
 
 	if (!ParseTupleAndKeywords(
-		    args, kwds, "Is#K|I", kwlist,
+		    args, kwds, "I" PYARG_BYTES_LEN "K|I", kwlist,
 		    &fnum, &buf, &buflen, &offset, &mode)) {
 		return NULL;
 	}
-- 
2.7.4


From 78b9b2cc48a835baa4c0b6d9057b59c1d1f00d9b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 14:26:43 +0100
Subject: [PATCH 15/23] s3:libsmb: add cli_write_send/recv which work with
 SMB1/2/3

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/clireadwrite.c | 116 ++++++++++++++++++++++++++++++++++++++++++
 source3/libsmb/proto.h        |   7 +++
 2 files changed, 123 insertions(+)

diff --git a/source3/libsmb/clireadwrite.c b/source3/libsmb/clireadwrite.c
index 6bf3df6..6d47ccd 100644
--- a/source3/libsmb/clireadwrite.c
+++ b/source3/libsmb/clireadwrite.c
@@ -1065,6 +1065,122 @@ NTSTATUS cli_write_andx_recv(struct tevent_req *req, size_t *pwritten)
 	return NT_STATUS_OK;
 }
 
+struct cli_write_state {
+	struct cli_state *cli;
+	size_t written;
+};
+
+static void cli_write_done(struct tevent_req *subreq);
+
+struct tevent_req *cli_write_send(TALLOC_CTX *mem_ctx,
+				  struct tevent_context *ev,
+				  struct cli_state *cli, uint16_t fnum,
+				  uint16_t mode, const uint8_t *buf,
+				  off_t offset, size_t size)
+{
+	struct tevent_req *req = NULL;
+	struct cli_write_state *state = NULL;
+	struct tevent_req *subreq = NULL;
+
+	req = tevent_req_create(mem_ctx, &state, struct cli_write_state);
+	if (req == NULL) {
+		return NULL;
+	}
+	state->cli = cli;
+
+	if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) {
+		uint32_t max_size;
+		bool ok;
+
+		ok = smb2cli_conn_req_possible(state->cli->conn, &max_size);
+		if (!ok) {
+			tevent_req_nterror(
+				req,
+				NT_STATUS_INSUFFICIENT_RESOURCES);
+			return tevent_req_post(req, ev);
+		}
+
+		/*
+		 * downgrade depending on the available credits
+		 */
+		size = MIN(max_size, size);
+
+		subreq = cli_smb2_write_send(state,
+					     ev,
+					     cli,
+					     fnum,
+					     mode,
+					     buf,
+					     offset,
+					     size);
+	} else {
+		bool ok;
+
+		ok = smb1cli_conn_req_possible(state->cli->conn);
+		if (!ok) {
+			tevent_req_nterror(
+				req,
+				NT_STATUS_INSUFFICIENT_RESOURCES);
+			return tevent_req_post(req, ev);
+		}
+
+		subreq = cli_write_andx_send(state,
+					     ev,
+					     cli,
+					     fnum,
+					     mode,
+					     buf,
+					     offset,
+					     size);
+	}
+	if (tevent_req_nomem(subreq, req)) {
+		return tevent_req_post(req, ev);
+	}
+	tevent_req_set_callback(subreq, cli_write_done, req);
+
+	return req;
+}
+
+static void cli_write_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req =
+		tevent_req_callback_data(subreq,
+		struct tevent_req);
+	struct cli_write_state *state =
+		tevent_req_data(req,
+		struct cli_write_state);
+	NTSTATUS status;
+
+	if (smbXcli_conn_protocol(state->cli->conn) >= PROTOCOL_SMB2_02) {
+		status = cli_smb2_write_recv(subreq, &state->written);
+	} else {
+		status = cli_write_andx_recv(subreq, &state->written);
+	}
+	TALLOC_FREE(subreq);
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+	tevent_req_done(req);
+}
+
+NTSTATUS cli_write_recv(struct tevent_req *req, size_t *pwritten)
+{
+	struct cli_write_state *state =
+		tevent_req_data(req,
+		struct cli_write_state);
+	NTSTATUS status;
+
+	if (tevent_req_is_nterror(req, &status)) {
+		tevent_req_received(req);
+		return status;
+	}
+	if (pwritten != NULL) {
+		*pwritten = state->written;
+	}
+	tevent_req_received(req);
+	return NT_STATUS_OK;
+}
+
 struct cli_smb1_writeall_state {
 	struct tevent_context *ev;
 	struct cli_state *cli;
diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index d0bc3e7..941d154 100644
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -868,6 +868,13 @@ struct tevent_req *cli_write_andx_send(TALLOC_CTX *mem_ctx,
 				       off_t offset, size_t size);
 NTSTATUS cli_write_andx_recv(struct tevent_req *req, size_t *pwritten);
 
+struct tevent_req *cli_write_send(TALLOC_CTX *mem_ctx,
+				  struct tevent_context *ev,
+				  struct cli_state *cli, uint16_t fnum,
+				  uint16_t mode, const uint8_t *buf,
+				  off_t offset, size_t size);
+NTSTATUS cli_write_recv(struct tevent_req *req, size_t *pwritten);
+
 struct tevent_req *cli_writeall_send(
 	TALLOC_CTX *mem_ctx,
 	struct tevent_context *ev,
-- 
2.7.4


From a9c472f7dc368cb467e9b94859e6ed62b2b35ad9 Mon Sep 17 00:00:00 2001
From: Tim Beale <timbeale at catalyst.net.nz>
Date: Tue, 11 Dec 2018 16:05:43 +1300
Subject: [PATCH 16/23] s3:libsmb: add comments for
 cli_write_send/cli_push_send

Added a code comment highlighting this 2 APIs do similar jobs, and tried
to explain why you might want to use one over the other.

Signed-off-by: Tim Beale <timbeale at catalyst.net.nz>
---
 source3/libsmb/clireadwrite.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/source3/libsmb/clireadwrite.c b/source3/libsmb/clireadwrite.c
index 6d47ccd..e953fa5 100644
--- a/source3/libsmb/clireadwrite.c
+++ b/source3/libsmb/clireadwrite.c
@@ -1072,6 +1072,14 @@ struct cli_write_state {
 
 static void cli_write_done(struct tevent_req *subreq);
 
+/*
+ * Used to write to a file remotely.
+ * This is similar in functionality to cli_push_send(), except this is a more
+ * finer-grain API. For example, if the data we want to write exceeds the max
+ * write size of the underlying connection, then it's the caller's
+ * responsibility to handle this.
+ * For writing a small amount of data to file, this is a simpler API to use.
+ */
 struct tevent_req *cli_write_send(TALLOC_CTX *mem_ctx,
 				  struct tevent_context *ev,
 				  struct cli_state *cli, uint16_t fnum,
@@ -1455,6 +1463,16 @@ static void cli_push_setup_chunks(struct tevent_req *req);
 static void cli_push_chunk_ship(struct cli_push_chunk *chunk);
 static void cli_push_chunk_done(struct tevent_req *subreq);
 
+/*
+ * Used to write to a file remotely.
+ * This is similar in functionality to cli_write_send(), except this API
+ * handles writing a large file by breaking the data into chunks (so we don't
+ * exceed the max write size of the underlying connection). To do this, the
+ * (*source) callback handles copying the underlying file data into a message
+ * buffer, one chunk at a time.
+ * This API is recommended when writing a potentially large amount of data,
+ * e.g. when copying a file (or doing a 'put').
+ */
 struct tevent_req *cli_push_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
 				 struct cli_state *cli,
 				 uint16_t fnum, uint16_t mode,
-- 
2.7.4


From c8d00bdeff88e0c12f83dfb18048d524b9f4a847 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 14:28:04 +0100
Subject: [PATCH 17/23] s3:pylibsmb: make use of protocol independent
 cli_write_send/recv in py_cli_write()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/pylibsmb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 3b1aa73..2dabacb 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -746,12 +746,12 @@ static PyObject *py_cli_write(struct py_cli_state *self, PyObject *args,
 		return NULL;
 	}
 
-	req = cli_write_andx_send(NULL, self->ev, self->cli, fnum, mode,
-				  (uint8_t *)buf, offset, buflen);
+	req = cli_write_send(NULL, self->ev, self->cli, fnum, mode,
+			     (uint8_t *)buf, offset, buflen);
 	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
-	status = cli_write_andx_recv(req, &written);
+	status = cli_write_recv(req, &written);
 	TALLOC_FREE(req);
 
 	if (!NT_STATUS_IS_OK(status)) {
-- 
2.7.4


From 4cc39cd02becef8323a73047fb99559b81f1ff3e Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 14:28:04 +0100
Subject: [PATCH 18/23] s3:pylibsmb: make use of protocol independent
 cli_read_send/recv in py_cli_read()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/pylibsmb.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 2dabacb..b6cff0b 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -769,8 +769,8 @@ static PyObject *py_cli_read(struct py_cli_state *self, PyObject *args,
 	unsigned size;
 	struct tevent_req *req;
 	NTSTATUS status;
-	uint8_t *buf;
-	ssize_t buflen;
+	char *buf;
+	size_t received;
 	PyObject *result;
 
 	static const char *kwlist[] = {
@@ -782,20 +782,41 @@ static PyObject *py_cli_read(struct py_cli_state *self, PyObject *args,
 		return NULL;
 	}
 
-	req = cli_read_andx_send(NULL, self->ev, self->cli, fnum,
-				 offset, size);
+	result = PyBytes_FromStringAndSize(NULL, size);
+	if (result == NULL) {
+		return NULL;
+	}
+	buf = PyBytes_AS_STRING(result);
+
+	req = cli_read_send(NULL, self->ev, self->cli, fnum,
+			    buf, offset, size);
 	if (!py_tevent_req_wait_exc(self, req)) {
+		Py_XDECREF(result);
 		return NULL;
 	}
-	status = cli_read_andx_recv(req, &buflen, &buf);
+	status = cli_read_recv(req, &received);
+	TALLOC_FREE(req);
 
 	if (!NT_STATUS_IS_OK(status)) {
-		TALLOC_FREE(req);
+		Py_XDECREF(result);
 		PyErr_SetNTSTATUS(status);
 		return NULL;
 	}
-	result = PyBytes_FromStringAndSize((const char *)buf, buflen);
-	TALLOC_FREE(req);
+
+	if (received > size) {
+		Py_XDECREF(result);
+		PyErr_Format(PyExc_IOError,
+			     "read invalid - got %zu requested %u",
+			     received, size);
+		return NULL;
+	}
+
+	if (received < size) {
+		if (_PyBytes_Resize(&result, received) < 0) {
+			return NULL;
+		}
+	}
+
 	return result;
 }
 
-- 
2.7.4


From 4e3b91f9e39eed4daec97173158898451590e47f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 16:32:05 +0100
Subject: [PATCH 19/23] s3:libsmb: pass impersonation_level to
 cli_smb2_create_fnum_send()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 examples/fuse/clifuse.c        | 6 ++++--
 source3/libsmb/cli_smb2_fnum.c | 5 ++++-
 source3/libsmb/cli_smb2_fnum.h | 1 +
 source3/libsmb/clifile.c       | 4 +++-
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/examples/fuse/clifuse.c b/examples/fuse/clifuse.c
index 3c7e498..b724e64 100644
--- a/examples/fuse/clifuse.c
+++ b/examples/fuse/clifuse.c
@@ -151,7 +151,8 @@ static void cli_ll_create(fuse_req_t freq, fuse_ino_t parent, const char *name,
 
 	req = cli_smb2_create_fnum_send(
 		state, mstate->ev, mstate->cli, state->path,
-		0, FILE_GENERIC_READ|FILE_GENERIC_WRITE, FILE_ATTRIBUTE_NORMAL,
+		0, SMB2_IMPERSONATION_IMPERSONATION,
+		FILE_GENERIC_READ|FILE_GENERIC_WRITE, FILE_ATTRIBUTE_NORMAL,
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
 		FILE_CREATE, FILE_NON_DIRECTORY_FILE);
 	if (req == NULL) {
@@ -836,7 +837,8 @@ static void cli_ll_open(fuse_req_t freq, fuse_ino_t ino,
 
 	req = cli_smb2_create_fnum_send(
 		state, mstate->ev, mstate->cli, istate->path,
-		0, acc, FILE_ATTRIBUTE_NORMAL,
+		0, SMB2_IMPERSONATION_IMPERSONATION,
+		acc, FILE_ATTRIBUTE_NORMAL,
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
 		FILE_OPEN, FILE_NON_DIRECTORY_FILE);
 	if (req == NULL) {
diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
index 35edec8..aef70f1 100644
--- a/source3/libsmb/cli_smb2_fnum.c
+++ b/source3/libsmb/cli_smb2_fnum.c
@@ -171,6 +171,7 @@ struct tevent_req *cli_smb2_create_fnum_send(TALLOC_CTX *mem_ctx,
 					     struct cli_state *cli,
 					     const char *fname,
 					     uint32_t create_flags,
+					     uint32_t impersonation_level,
 					     uint32_t desired_access,
 					     uint32_t file_attributes,
 					     uint32_t share_access,
@@ -262,7 +263,7 @@ struct tevent_req *cli_smb2_create_fnum_send(TALLOC_CTX *mem_ctx,
 				     cli->smb2.tcon,
 				     fname,
 				     flags_to_smb2_oplock(create_flags),
-				     SMB2_IMPERSONATION_IMPERSONATION,
+				     impersonation_level,
 				     desired_access,
 				     file_attributes,
 				     share_access,
@@ -345,6 +346,7 @@ NTSTATUS cli_smb2_create_fnum(struct cli_state *cli,
 	TALLOC_CTX *frame = talloc_stackframe();
 	struct tevent_context *ev;
 	struct tevent_req *req;
+	uint32_t impersonation_level = SMB2_IMPERSONATION_IMPERSONATION;
 	NTSTATUS status = NT_STATUS_NO_MEMORY;
 
 	if (smbXcli_conn_has_async_calls(cli->conn)) {
@@ -359,6 +361,7 @@ NTSTATUS cli_smb2_create_fnum(struct cli_state *cli,
 		goto fail;
 	}
 	req = cli_smb2_create_fnum_send(frame, ev, cli, fname, create_flags,
+					impersonation_level,
 					desired_access, file_attributes,
 					share_access, create_disposition,
 					create_options);
diff --git a/source3/libsmb/cli_smb2_fnum.h b/source3/libsmb/cli_smb2_fnum.h
index 4fce5fc..2edaae0 100644
--- a/source3/libsmb/cli_smb2_fnum.h
+++ b/source3/libsmb/cli_smb2_fnum.h
@@ -30,6 +30,7 @@ struct tevent_req *cli_smb2_create_fnum_send(TALLOC_CTX *mem_ctx,
 					     struct cli_state *cli,
 					     const char *fname,
 					     uint32_t create_flags,
+					     uint32_t impersonation_level,
 					     uint32_t desired_access,
 					     uint32_t file_attributes,
 					     uint32_t share_access,
diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
index b509263..dc3751a 100644
--- a/source3/libsmb/clifile.c
+++ b/source3/libsmb/clifile.c
@@ -2102,6 +2102,7 @@ struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
 {
 	struct tevent_req *req, *subreq;
 	struct cli_ntcreate_state *state;
+	uint32_t impersonation_level = SMB2_IMPERSONATION_IMPERSONATION;
 
 	req = tevent_req_create(mem_ctx, &state, struct cli_ntcreate_state);
 	if (req == NULL) {
@@ -2116,7 +2117,8 @@ struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
 		}
 
 		subreq = cli_smb2_create_fnum_send(
-			state, ev, cli, fname, create_flags, desired_access,
+			state, ev, cli, fname, create_flags,
+			impersonation_level, desired_access,
 			file_attributes, share_access, create_disposition,
 			create_options);
 	} else {
-- 
2.7.4


From 4ac19b1da855a1e4448a3ad9c9e5af187976748a Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 16:42:06 +0100
Subject: [PATCH 20/23] s3:libsmb: pass impersonation_level to
 cli_smb2_create_fnum()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/cli_smb2_fnum.c | 18 +++++++++++++++++-
 source3/libsmb/cli_smb2_fnum.h |  1 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c
index aef70f1..6cba442 100644
--- a/source3/libsmb/cli_smb2_fnum.c
+++ b/source3/libsmb/cli_smb2_fnum.c
@@ -335,6 +335,7 @@ NTSTATUS cli_smb2_create_fnum_recv(struct tevent_req *req, uint16_t *pfnum,
 NTSTATUS cli_smb2_create_fnum(struct cli_state *cli,
 			const char *fname,
 			uint32_t create_flags,
+			uint32_t impersonation_level,
 			uint32_t desired_access,
 			uint32_t file_attributes,
 			uint32_t share_access,
@@ -346,7 +347,6 @@ NTSTATUS cli_smb2_create_fnum(struct cli_state *cli,
 	TALLOC_CTX *frame = talloc_stackframe();
 	struct tevent_context *ev;
 	struct tevent_req *req;
-	uint32_t impersonation_level = SMB2_IMPERSONATION_IMPERSONATION;
 	NTSTATUS status = NT_STATUS_NO_MEMORY;
 
 	if (smbXcli_conn_has_async_calls(cli->conn)) {
@@ -644,6 +644,7 @@ NTSTATUS cli_smb2_mkdir(struct cli_state *cli, const char *dname)
 	status = cli_smb2_create_fnum(cli,
 			dname,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			FILE_READ_ATTRIBUTES,	/* desired_access */
 			FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE, /* share_access */
@@ -682,6 +683,7 @@ NTSTATUS cli_smb2_rmdir(struct cli_state *cli, const char *dname)
 	status = cli_smb2_create_fnum(cli,
 			dname,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			DELETE_ACCESS,		/* desired_access */
 			FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -700,6 +702,7 @@ NTSTATUS cli_smb2_rmdir(struct cli_state *cli, const char *dname)
 		status = cli_smb2_create_fnum(cli,
 			dname,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			DELETE_ACCESS,		/* desired_access */
 			FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -748,6 +751,7 @@ NTSTATUS cli_smb2_unlink(struct cli_state *cli, const char *fname)
 	status = cli_smb2_create_fnum(cli,
 			fname,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			DELETE_ACCESS,		/* desired_access */
 			FILE_ATTRIBUTE_NORMAL, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -766,6 +770,7 @@ NTSTATUS cli_smb2_unlink(struct cli_state *cli, const char *fname)
 		status = cli_smb2_create_fnum(cli,
 			fname,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			DELETE_ACCESS,		/* desired_access */
 			FILE_ATTRIBUTE_NORMAL, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -943,6 +948,7 @@ NTSTATUS cli_smb2_list(struct cli_state *cli,
 	status = cli_smb2_create_fnum(cli,
 			parent_dir,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			SEC_DIR_LIST|SEC_DIR_READ_ATTRIBUTE,/* desired_access */
 			FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE, /* share_access */
@@ -1119,6 +1125,7 @@ NTSTATUS cli_smb2_qpathinfo_basic(struct cli_state *cli,
 	status = cli_smb2_create_fnum(cli,
 			name,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			FILE_READ_ATTRIBUTES,	/* desired_access */
 			FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -1132,6 +1139,7 @@ NTSTATUS cli_smb2_qpathinfo_basic(struct cli_state *cli,
 		status = cli_smb2_create_fnum(cli,
 			name,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			FILE_READ_ATTRIBUTES,		/* desired_access */
 			0, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -1184,6 +1192,7 @@ NTSTATUS cli_smb2_chkpath(struct cli_state *cli,
 	status = cli_smb2_create_fnum(cli,
 			name,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			FILE_READ_ATTRIBUTES,	/* desired_access */
 			FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -1229,6 +1238,7 @@ static NTSTATUS get_fnum_from_path(struct cli_state *cli,
 	status = cli_smb2_create_fnum(cli,
 			name,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			desired_access,
 			0, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -1248,6 +1258,7 @@ static NTSTATUS get_fnum_from_path(struct cli_state *cli,
 		status = cli_smb2_create_fnum(cli,
 			name,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			desired_access,
 			0, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -1262,6 +1273,7 @@ static NTSTATUS get_fnum_from_path(struct cli_state *cli,
 		status = cli_smb2_create_fnum(cli,
 			name,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			desired_access,
 			FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -1997,6 +2009,7 @@ NTSTATUS cli_smb2_dskattr(struct cli_state *cli, const char *path,
 	status = cli_smb2_create_fnum(cli,
 			path,
 			0,			/* create_flags */
+			SMB2_IMPERSONATION_IMPERSONATION,
 			FILE_READ_ATTRIBUTES,	/* desired_access */
 			FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 			FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE, /* share_access */
@@ -2106,6 +2119,7 @@ NTSTATUS cli_smb2_get_fs_full_size_info(struct cli_state *cli,
 	/* First open the top level directory. */
 	status =
 	    cli_smb2_create_fnum(cli, "", 0,		   /* create_flags */
+				 SMB2_IMPERSONATION_IMPERSONATION,
 				 FILE_READ_ATTRIBUTES,     /* desired_access */
 				 FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 				 FILE_SHARE_READ | FILE_SHARE_WRITE |
@@ -2198,6 +2212,7 @@ NTSTATUS cli_smb2_get_fs_attr_info(struct cli_state *cli, uint32_t *fs_attr)
 	/* First open the top level directory. */
 	status =
 	    cli_smb2_create_fnum(cli, "", 0,		   /* create_flags */
+				 SMB2_IMPERSONATION_IMPERSONATION,
 				 FILE_READ_ATTRIBUTES,     /* desired_access */
 				 FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 				 FILE_SHARE_READ | FILE_SHARE_WRITE |
@@ -2283,6 +2298,7 @@ NTSTATUS cli_smb2_get_fs_volume_info(struct cli_state *cli,
 	/* First open the top level directory. */
 	status =
 	    cli_smb2_create_fnum(cli, "", 0,		   /* create_flags */
+				 SMB2_IMPERSONATION_IMPERSONATION,
 				 FILE_READ_ATTRIBUTES,     /* desired_access */
 				 FILE_ATTRIBUTE_DIRECTORY, /* file attributes */
 				 FILE_SHARE_READ | FILE_SHARE_WRITE |
diff --git a/source3/libsmb/cli_smb2_fnum.h b/source3/libsmb/cli_smb2_fnum.h
index 2edaae0..921dc71 100644
--- a/source3/libsmb/cli_smb2_fnum.h
+++ b/source3/libsmb/cli_smb2_fnum.h
@@ -41,6 +41,7 @@ NTSTATUS cli_smb2_create_fnum_recv(struct tevent_req *req, uint16_t *pfnum,
 NTSTATUS cli_smb2_create_fnum(struct cli_state *cli,
 			const char *fname,
 			uint32_t create_flags,
+			uint32_t impersonation_level,
 			uint32_t desired_access,
 			uint32_t file_attributes,
 			uint32_t share_access,
-- 
2.7.4


From 4e7a632e7e3d100ada96c0eea008484bac25442b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 16:35:16 +0100
Subject: [PATCH 21/23] s3:libsmb: pass ImpersonationLevel to
 cli_ntcreate1_send()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/clifile.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
index dc3751a..1694af5 100644
--- a/source3/libsmb/clifile.c
+++ b/source3/libsmb/clifile.c
@@ -1951,6 +1951,7 @@ static struct tevent_req *cli_ntcreate1_send(TALLOC_CTX *mem_ctx,
 					     uint32_t ShareAccess,
 					     uint32_t CreateDisposition,
 					     uint32_t CreateOptions,
+					     uint32_t ImpersonationLevel,
 					     uint8_t SecurityFlags)
 {
 	struct tevent_req *req, *subreq;
@@ -1985,7 +1986,7 @@ static struct tevent_req *cli_ntcreate1_send(TALLOC_CTX *mem_ctx,
 	SIVAL(vwv+17, 1, CreateDisposition);
 	SIVAL(vwv+19, 1, CreateOptions |
 		(cli->backup_intent ? FILE_OPEN_FOR_BACKUP_INTENT : 0));
-	SIVAL(vwv+21, 1, 0x02);	/* ImpersonationLevel */
+	SIVAL(vwv+21, 1, ImpersonationLevel);
 	SCVAL(vwv+23, 1, SecurityFlags);
 
 	bytes = talloc_array(state, uint8_t, 0);
@@ -2126,7 +2127,7 @@ struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
 		subreq = cli_ntcreate1_send(
 			state, ev, cli, fname, create_flags, desired_access,
 			file_attributes, share_access, create_disposition,
-			create_options, security_flags);
+			create_options, impersonation_level, security_flags);
 	}
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
-- 
2.7.4


From 2214273a26b32ce2bfde100c383df78ca2e99708 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 16:38:57 +0100
Subject: [PATCH 22/23] s3:libsmb: pass impersonation_level to
 cli_ntcreate_send()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 examples/winexe/winexe.c             | 3 +++
 source3/libsmb/clifile.c             | 5 +++--
 source3/libsmb/clisymlink.c          | 6 ++++--
 source3/libsmb/proto.h               | 1 +
 source3/libsmb/pylibsmb.c            | 3 ++-
 source3/torture/nbench.c             | 3 ++-
 source3/torture/test_chain3.c        | 3 ++-
 source3/torture/test_notify.c        | 9 ++++++---
 source3/torture/test_notify_online.c | 5 +++--
 source3/torture/test_oplock_cancel.c | 2 +-
 source3/torture/torture.c            | 3 ++-
 11 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/examples/winexe/winexe.c b/examples/winexe/winexe.c
index cf667a6..429ba2f 100644
--- a/examples/winexe/winexe.c
+++ b/examples/winexe/winexe.c
@@ -872,6 +872,7 @@ static struct tevent_req *winexe_out_pipe_send(
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
 		FILE_OPEN,	/* CreateDisposition */
 		0,		/* CreateOptions */
+		SMB2_IMPERSONATION_IMPERSONATION,
 		0);		/* SecurityFlags */
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
@@ -1044,6 +1045,7 @@ static struct tevent_req *winexe_in_pipe_send(
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
 		FILE_OPEN,	/* CreateDisposition */
 		0,		/* CreateOptions */
+		SMB2_IMPERSONATION_IMPERSONATION,
 		0);		/* SecurityFlags */
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
@@ -1462,6 +1464,7 @@ static struct tevent_req *winexe_ctrl_send(
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
 		FILE_OPEN,	/* CreateDisposition */
 		0,		/* CreateOptions */
+		SMB2_IMPERSONATION_IMPERSONATION,
 		0);		/* SecurityFlags */
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c
index 1694af5..6defa38 100644
--- a/source3/libsmb/clifile.c
+++ b/source3/libsmb/clifile.c
@@ -2099,11 +2099,11 @@ struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
 				     uint32_t share_access,
 				     uint32_t create_disposition,
 				     uint32_t create_options,
+				     uint32_t impersonation_level,
 				     uint8_t security_flags)
 {
 	struct tevent_req *req, *subreq;
 	struct cli_ntcreate_state *state;
-	uint32_t impersonation_level = SMB2_IMPERSONATION_IMPERSONATION;
 
 	req = tevent_req_create(mem_ctx, &state, struct cli_ntcreate_state);
 	if (req == NULL) {
@@ -2197,6 +2197,7 @@ NTSTATUS cli_ntcreate(struct cli_state *cli,
 	TALLOC_CTX *frame = talloc_stackframe();
 	struct tevent_context *ev;
 	struct tevent_req *req;
+	uint32_t ImpersonationLevel = SMB2_IMPERSONATION_IMPERSONATION;
 	NTSTATUS status = NT_STATUS_NO_MEMORY;
 
 	if (smbXcli_conn_has_async_calls(cli->conn)) {
@@ -2215,7 +2216,7 @@ NTSTATUS cli_ntcreate(struct cli_state *cli,
 	req = cli_ntcreate_send(frame, ev, cli, fname, CreatFlags,
 				DesiredAccess, FileAttributes, ShareAccess,
 				CreateDisposition, CreateOptions,
-				SecurityFlags);
+				ImpersonationLevel, SecurityFlags);
 	if (req == NULL) {
 		goto fail;
 	}
diff --git a/source3/libsmb/clisymlink.c b/source3/libsmb/clisymlink.c
index 54435e4..1330752 100644
--- a/source3/libsmb/clisymlink.c
+++ b/source3/libsmb/clisymlink.c
@@ -72,7 +72,8 @@ struct tevent_req *cli_symlink_send(TALLOC_CTX *mem_ctx,
 		FILE_READ_ATTRIBUTES|FILE_WRITE_ATTRIBUTES,
 		FILE_ATTRIBUTE_NORMAL, FILE_SHARE_NONE, FILE_CREATE,
 		FILE_OPEN_REPARSE_POINT|FILE_SYNCHRONOUS_IO_NONALERT|
-		FILE_NON_DIRECTORY_FILE, 0);
+		FILE_NON_DIRECTORY_FILE,
+		SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
@@ -274,7 +275,8 @@ struct tevent_req *cli_readlink_send(TALLOC_CTX *mem_ctx,
 	subreq = cli_ntcreate_send(
 		state, ev, cli, fname, 0, FILE_READ_ATTRIBUTES | FILE_READ_EA,
 		0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
-		FILE_OPEN, FILE_OPEN_REPARSE_POINT, 0);
+		FILE_OPEN, FILE_OPEN_REPARSE_POINT,
+		SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index 941d154..bfad4dc 100644
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -397,6 +397,7 @@ struct tevent_req *cli_ntcreate_send(TALLOC_CTX *mem_ctx,
 				     uint32_t ShareAccess,
 				     uint32_t CreateDisposition,
 				     uint32_t CreateOptions,
+				     uint32_t ImpersonationLevel,
 				     uint8_t SecurityFlags);
 NTSTATUS cli_ntcreate_recv(struct tevent_req *req,
 			uint16_t *pfnum,
diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index b6cff0b..3f84d34 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -666,6 +666,7 @@ static PyObject *py_cli_create(struct py_cli_state *self, PyObject *args,
 	unsigned ShareAccess = 0;
 	unsigned CreateDisposition = FILE_OPEN;
 	unsigned CreateOptions = 0;
+	unsigned ImpersonationLevel = SMB2_IMPERSONATION_IMPERSONATION;
 	unsigned SecurityFlags = 0;
 	uint16_t fnum;
 	struct tevent_req *req;
@@ -687,7 +688,7 @@ static PyObject *py_cli_create(struct py_cli_state *self, PyObject *args,
 	req = cli_ntcreate_send(NULL, self->ev, self->cli, fname, CreateFlags,
 				DesiredAccess, FileAttributes, ShareAccess,
 				CreateDisposition, CreateOptions,
-				SecurityFlags);
+				ImpersonationLevel, SecurityFlags);
 	if (!py_tevent_req_wait_exc(self, req)) {
 		return NULL;
 	}
diff --git a/source3/torture/nbench.c b/source3/torture/nbench.c
index 80b5a72..e9a0b4f 100644
--- a/source3/torture/nbench.c
+++ b/source3/torture/nbench.c
@@ -263,7 +263,8 @@ static struct tevent_req *nbench_cmd_send(TALLOC_CTX *mem_ctx,
 			state, ev, nb_state->cli, state->ft->cp.fname, flags,
 			desired_access, 0, share_mode,
 			state->ft->cp.cr_disposition,
-			state->ft->cp.cr_options, 0);
+			state->ft->cp.cr_options,
+			SMB2_IMPERSONATION_IMPERSONATION, 0);
 		break;
 	}
 	case NBENCH_CMD_CLOSE: {
diff --git a/source3/torture/test_chain3.c b/source3/torture/test_chain3.c
index eff39de..d957e51 100644
--- a/source3/torture/test_chain3.c
+++ b/source3/torture/test_chain3.c
@@ -180,7 +180,8 @@ static struct tevent_req *chain3_send(TALLOC_CTX *mem_ctx,
 		GENERIC_READ_ACCESS|GENERIC_WRITE_ACCESS,
 		FILE_ATTRIBUTE_NORMAL,
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
-		FILE_OVERWRITE_IF, 0, 0);
+		FILE_OVERWRITE_IF, 0,
+		SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
diff --git a/source3/torture/test_notify.c b/source3/torture/test_notify.c
index e377875..20b39d1 100644
--- a/source3/torture/test_notify.c
+++ b/source3/torture/test_notify.c
@@ -66,7 +66,8 @@ static struct tevent_req *wait_for_one_notify_send(TALLOC_CTX *mem_ctx,
 		state, state->ev, state->cli, path, 0,
 		MAXIMUM_ALLOWED_ACCESS,
 		0, FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
-		FILE_OPEN, FILE_DIRECTORY_FILE, 0);
+		FILE_OPEN, FILE_DIRECTORY_FILE,
+		SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
@@ -318,7 +319,8 @@ static struct tevent_req *notify_bench3_send(
 		state, state->ev, state->cli, state->dir, 0,
 		MAXIMUM_ALLOWED_ACCESS, 0,
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
-		FILE_OPEN_IF, FILE_DIRECTORY_FILE, 0);
+		FILE_OPEN_IF, FILE_DIRECTORY_FILE,
+		SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
@@ -456,7 +458,8 @@ static void notify_bench3_before_mkdir2(struct tevent_req *subreq)
 		MAXIMUM_ALLOWED_ACCESS,	0,
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
 		FILE_CREATE,
-		FILE_DIRECTORY_FILE, 0);
+		FILE_DIRECTORY_FILE,
+		SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return;
 	}
diff --git a/source3/torture/test_notify_online.c b/source3/torture/test_notify_online.c
index 7f4f521..c8ddf7c 100644
--- a/source3/torture/test_notify_online.c
+++ b/source3/torture/test_notify_online.c
@@ -61,7 +61,7 @@ static struct tevent_req *notify_online_send(
 		state, ev, cli, dname, EXTENDED_RESPONSE_REQUIRED,
 		SEC_FILE_READ_DATA, 0,
 		FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
-		FILE_OPEN, 0, 0);
+		FILE_OPEN, 0, SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
@@ -93,7 +93,8 @@ static void notify_online_opened_dir(struct tevent_req *subreq)
 		state, state->ev, state->cli, state->fname, 0,
 		GENERIC_READ_ACCESS, FILE_ATTRIBUTE_NORMAL,
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
-		FILE_OPEN, FILE_NON_DIRECTORY_FILE, 0);
+		FILE_OPEN, FILE_NON_DIRECTORY_FILE,
+		SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return;
 	}
diff --git a/source3/torture/test_oplock_cancel.c b/source3/torture/test_oplock_cancel.c
index d856650..b003876 100644
--- a/source3/torture/test_oplock_cancel.c
+++ b/source3/torture/test_oplock_cancel.c
@@ -48,7 +48,7 @@ static struct tevent_req *create_cancel_send(
 	subreq = cli_ntcreate_send(
 		mem_ctx, ev, cli, fname, 0, FILE_GENERIC_READ,
 		FILE_ATTRIBUTE_NORMAL, FILE_SHARE_READ|FILE_SHARE_WRITE,
-		FILE_OPEN_IF, 0, 0);
+		FILE_OPEN_IF, 0, SMB2_IMPERSONATION_IMPERSONATION, 0);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index 22810f0..8cf9678 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -8249,7 +8249,8 @@ static struct tevent_req *torture_createdel_send(TALLOC_CTX *mem_ctx,
 		FILE_READ_DATA|FILE_WRITE_DATA|DELETE_ACCESS,
 		FILE_ATTRIBUTE_NORMAL,
 		FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE,
-		FILE_OPEN_IF, FILE_DELETE_ON_CLOSE, 0);
+		FILE_OPEN_IF, FILE_DELETE_ON_CLOSE,
+		SMB2_IMPERSONATION_IMPERSONATION, 0);
 
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
-- 
2.7.4


From e17f432d787bea52fa84dcc373fa8421026e8dd0 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Dec 2018 16:40:10 +0100
Subject: [PATCH 23/23] s3:pylibsmb: allow ImpersonationLevel argument to
 create()

Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Tim Beale <timbeale at samba.org>
---
 source3/libsmb/pylibsmb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/libsmb/pylibsmb.c b/source3/libsmb/pylibsmb.c
index 3f84d34..981e744 100644
--- a/source3/libsmb/pylibsmb.c
+++ b/source3/libsmb/pylibsmb.c
@@ -675,13 +675,13 @@ static PyObject *py_cli_create(struct py_cli_state *self, PyObject *args,
 	static const char *kwlist[] = {
 		"Name", "CreateFlags", "DesiredAccess", "FileAttributes",
 		"ShareAccess", "CreateDisposition", "CreateOptions",
-		"SecurityFlags", NULL };
+		"ImpersonationLevel", "SecurityFlags", NULL };
 
 	if (!ParseTupleAndKeywords(
-		    args, kwds, "s|IIIIIII", kwlist,
+		    args, kwds, "s|IIIIIIII", kwlist,
 		    &fname, &CreateFlags, &DesiredAccess, &FileAttributes,
 		    &ShareAccess, &CreateDisposition, &CreateOptions,
-		    &SecurityFlags)) {
+		    &ImpersonationLevel, &SecurityFlags)) {
 		return NULL;
 	}
 
-- 
2.7.4



More information about the samba-technical mailing list