[PATCHES] site-aware Kerberos authentication during domain join

Jeremy Allison jra at samba.org
Tue Jun 14 20:53:02 UTC 2016


On Tue, Jun 14, 2016 at 10:50:28PM +0300, Alexander Bokovoy wrote:
> On Mon, 07 Mar 2016, Jeremy Allison wrote:
> > On Thu, Mar 03, 2016 at 09:44:46AM +0200, Uri Simchoni wrote:
> > > Hi,
> > > 
> > > Attached please find a fix for
> > > https://bugzilla.samba.org/show_bug.cgi?id=11769.
> > > 
> > > The bug description explains why this may be important.
> > > 
> > > The fix enables site-aware Kerberos during execution of "net ads
> > > join -k", even if winbindd is not started (so the locator cannot be
> > > used).
> > > 
> > > This works only if the user specified the domain's DNS name (which
> > > is assumed to be equal to the Kerberos realm). If the user didn't
> > > specify it (e.g. only specified flat domain name or server to use),
> > > we need to securely contact a DC to determine the domain's DNS name,
> > > so we cannot pre-configure Kerberos.
> > > 
> > > Review appreciated.
> > 
> > LGTM. Pushed ! Thanks.
> I think this is not enough. We don't use discovered site name later in
> the process as private krb5.conf will be rewritten after the call to
> libnet_join_check_config.
> 
> Attached patch makes sure we pass sitename to the new krb5.conf.

LGTM. Pushed.

Alexander can you review the two attached patchset ? They pass
local make test (and even have regression tests :-).

Thanks !

Jeremy.
-------------- next part --------------
From db6693bca16c33497644bd33bbeb7f4d5b49fedb Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 10 Jun 2016 16:15:22 -0700
Subject: [PATCH 1/4] s4: libcli: Internal SMB1 pid is already stored as and
 uses 32-bits. Correct getpid() cast.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/libcli/raw/clisession.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source4/libcli/raw/clisession.c b/source4/libcli/raw/clisession.c
index 23d7fd1..0a026c0 100644
--- a/source4/libcli/raw/clisession.c
+++ b/source4/libcli/raw/clisession.c
@@ -52,7 +52,7 @@ struct smbcli_session *smbcli_session_init(struct smbcli_transport *transport,
 	} else {
 		session->transport = talloc_reference(session, transport);
 	}
-	session->pid = (uint16_t)getpid();
+	session->pid = (uint32_t)getpid();
 	session->vuid = UID_FIELD_INVALID;
 	session->options = options;
 
-- 
2.7.4


From d91a49525dcee8b3bcdaf9a11328d095504ba27c Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 10 Jun 2016 16:51:11 -0700
Subject: [PATCH 2/4] s3: libsmb: Widen the internal client smb1.pid to 32-bits
 as is used on the wire and in libcli/smb/smb1*.c

Note: This has *NO* effect on the lock context code, as on the
wire for all SMB1 locking requests, the pid used as the lock
context is already truncated down to 16-bits - the field is only
16-bits wide.

This allows the cli_XXX() calls to correctly set pidlow AND pidhigh
in SMB1 requests put on the wire by the libcli/smb/smb1*.c code.

Note that currently the smbd server doesn't correctly return
pidhigh yet - a fix (and tests) for that will follow.

As pidhigh is not checked in any client code (mid is used
to differentiate different requests) this has no effect
other than a correctness fix.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/include/client.h   | 2 +-
 source3/libsmb/clientgen.c | 8 ++++----
 source3/libsmb/proto.h     | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/source3/include/client.h b/source3/include/client.h
index 0024c04..43ec39b 100644
--- a/source3/include/client.h
+++ b/source3/include/client.h
@@ -91,7 +91,7 @@ struct cli_state {
 	const char *remote_realm;
 
 	struct {
-		uint16_t pid;
+		uint32_t pid;
 		uint16_t vc_num;
 		struct smbXcli_session *session;
 		struct smbXcli_tcon *tcon;
diff --git a/source3/libsmb/clientgen.c b/source3/libsmb/clientgen.c
index cfb3b16..bf31bb1 100644
--- a/source3/libsmb/clientgen.c
+++ b/source3/libsmb/clientgen.c
@@ -225,7 +225,7 @@ struct cli_state *cli_state_create(TALLOC_CTX *mem_ctx,
 		goto error;
 	}
 
-	cli->smb1.pid = (uint16_t)getpid();
+	cli->smb1.pid = (uint32_t)getpid();
 	cli->smb1.vc_num = cli->smb1.pid;
 	cli->smb1.tcon = smbXcli_tcon_create(cli);
 	if (cli->smb1.tcon == NULL) {
@@ -327,14 +327,14 @@ uint16_t cli_state_get_vc_num(struct cli_state *cli)
  Set the PID to use for smb messages. Return the old pid.
 ****************************************************************************/
 
-uint16_t cli_setpid(struct cli_state *cli, uint16_t pid)
+uint32_t cli_setpid(struct cli_state *cli, uint32_t pid)
 {
-	uint16_t ret = cli->smb1.pid;
+	uint32_t ret = cli->smb1.pid;
 	cli->smb1.pid = pid;
 	return ret;
 }
 
-uint16_t cli_getpid(struct cli_state *cli)
+uint32_t cli_getpid(struct cli_state *cli)
 {
 	return cli->smb1.pid;
 }
diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h
index 1e358f7..c5e74c9 100644
--- a/source3/libsmb/proto.h
+++ b/source3/libsmb/proto.h
@@ -172,8 +172,8 @@ void cli_nt_pipes_close(struct cli_state *cli);
 void cli_shutdown(struct cli_state *cli);
 const char *cli_state_remote_realm(struct cli_state *cli);
 uint16_t cli_state_get_vc_num(struct cli_state *cli);
-uint16_t cli_setpid(struct cli_state *cli, uint16_t pid);
-uint16_t cli_getpid(struct cli_state *cli);
+uint32_t cli_setpid(struct cli_state *cli, uint32_t pid);
+uint32_t cli_getpid(struct cli_state *cli);
 bool cli_state_has_tcon(struct cli_state *cli);
 uint16_t cli_state_get_tid(struct cli_state *cli);
 uint16_t cli_state_set_tid(struct cli_state *cli, uint16_t tid);
-- 
2.7.4


From a212a599cc8267cdc66ceb08f1c3501b9f5ea60e Mon Sep 17 00:00:00 2001
From: Per Forlin <per.forlin at gmail.com>
Date: Fri, 10 Jun 2016 17:00:55 -0700
Subject: [PATCH 3/4] s3: smbd: Correctly reflect back SMB_PIDHIGH to a client.

Torture test to follow.

Signed-off-by: Per Forlin <per.forlin at gmail.com>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/process.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 34939f0..e3c32f9 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -2077,6 +2077,7 @@ static void construct_reply_common(uint8_t cmd, const uint8_t *inbuf,
 
 	SSVAL(outbuf,smb_tid,SVAL(inbuf,smb_tid));
 	SSVAL(outbuf,smb_pid,SVAL(inbuf,smb_pid));
+	SSVAL(outbuf,smb_pidhigh,SVAL(inbuf,smb_pidhigh));
 	SSVAL(outbuf,smb_uid,SVAL(inbuf,smb_uid));
 	SSVAL(outbuf,smb_mid,SVAL(inbuf,smb_mid));
 }
-- 
2.7.4


From 0b9ea81e2c0d00c945194878b8f8422efd7fad43 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Sat, 11 Jun 2016 17:51:16 -0700
Subject: [PATCH 4/4] s3: torture: Add test that proves Win2k12 correctly
 returns pidlow and pidhigh in SMB1 requests.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 selftest/knownfail        |   1 +
 source3/selftest/tests.py |   2 +-
 source3/torture/torture.c | 159 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+), 1 deletion(-)

diff --git a/selftest/knownfail b/selftest/knownfail
index 2f2d6bf..be6c70c 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -19,6 +19,7 @@
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-ACL # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-SYMLINK-EA # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).POSIX-OFD-LOCK # Fails against the s4 ntvfs server
+^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).PIDHIGH # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).NTTRANS-FSCTL # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).SMB2-NEGPROT # Fails against the s4 ntvfs server
 ^samba3.smbtorture_s3.plain\(ad_dc_ntvfs\).BAD-NBT-SESSION # Fails against the s4 ntvfs server
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index b96df8a..4d66a5d 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -50,7 +50,7 @@ tests = ["FDPASS", "LOCK1", "LOCK2", "LOCK3", "LOCK4", "LOCK5", "LOCK6", "LOCK7"
         "DIR", "DIR1", "DIR-CREATETIME", "TCON", "TCONDEV", "RW1", "RW2", "RW3", "LARGE_READX", "RW-SIGNING",
         "OPEN", "XCOPY", "RENAME", "DELETE", "DELETE-LN", "WILDDELETE", "PROPERTIES", "W2K",
         "TCON2", "IOCTL", "CHKPATH", "FDSESS", "CHAIN1", "CHAIN2",
-        "CHAIN3",
+        "CHAIN3", "PIDHIGH",
         "GETADDRINFO", "UID-REGRESSION-TEST", "SHORTNAME-TEST",
         "CASE-INSENSITIVE-CREATE", "SMB2-BASIC", "NTTRANS-FSCTL", "SMB2-NEGPROT",
         "SMB2-SESSION-REAUTH", "SMB2-SESSION-RECONNECT",
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index ea0fc01..0926690 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -8622,6 +8622,164 @@ static bool run_streamerror(int dummy)
 	return ret;
 }
 
+struct pidtest_state {
+	bool success;
+	uint16_t vwv[1];
+	DATA_BLOB data;
+};
+
+static void pid_echo_done(struct tevent_req *subreq);
+
+static struct tevent_req *pid_echo_send(TALLOC_CTX *mem_ctx,
+			struct tevent_context *ev,
+			struct cli_state *cli)
+{
+	struct tevent_req *req, *subreq;
+	struct pidtest_state *state;
+
+	req = tevent_req_create(mem_ctx, &state, struct pidtest_state);
+	if (req == NULL) {
+		return NULL;
+	}
+
+	SSVAL(state->vwv, 0, 1);
+	state->data = data_blob_const("hello", 5);
+
+	subreq = smb1cli_req_send(state,
+				ev,
+				cli->conn,
+				SMBecho,
+				0, 0, /* *_flags */
+				0, 0, /* *_flags2 */
+				cli->timeout,
+				0xDEADBEEF, /* pid */
+				NULL, /* tcon */
+				NULL, /* session */
+				ARRAY_SIZE(state->vwv), state->vwv,
+				state->data.length, state->data.data);
+
+	if (tevent_req_nomem(subreq, req)) {
+		return tevent_req_post(req, ev);
+	}
+	tevent_req_set_callback(subreq, pid_echo_done, req);
+	return req;
+}
+
+static void pid_echo_done(struct tevent_req *subreq)
+{
+	struct tevent_req *req = tevent_req_callback_data(
+		subreq, struct tevent_req);
+	struct pidtest_state *state = tevent_req_data(
+		req, struct pidtest_state);
+	NTSTATUS status;
+	uint32_t num_bytes;
+	uint8_t *bytes = NULL;
+	struct iovec *recv_iov = NULL;
+	uint8_t *phdr = NULL;
+	uint16_t pidlow = 0;
+	uint16_t pidhigh = 0;
+	struct smb1cli_req_expected_response expected[] = {
+	{
+		.status = NT_STATUS_OK,
+		.wct    = 1,
+	},
+	};
+
+	status = smb1cli_req_recv(subreq, state,
+				&recv_iov,
+				&phdr,
+				NULL, /* pwct */
+				NULL, /* pvwv */
+				NULL, /* pvwv_offset */
+				&num_bytes,
+				&bytes,
+				NULL, /* pbytes_offset */
+				NULL, /* pinbuf */
+				expected, ARRAY_SIZE(expected));
+
+	TALLOC_FREE(subreq);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		tevent_req_nterror(req, status);
+		return;
+	}
+
+	if (num_bytes != state->data.length) {
+		tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE);
+		return;
+	}
+
+	if (memcmp(bytes, state->data.data, num_bytes) != 0) {
+		tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE);
+		return;
+	}
+
+	/* Check pid low/high == DEADBEEF */
+	pidlow = SVAL(phdr, HDR_PID);
+	if (pidlow != 0xBEEF){
+		printf("Incorrect pidlow 0x%x, should be 0xBEEF\n",
+			(unsigned int)pidlow);
+		tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE);
+		return;
+	}
+	pidhigh = SVAL(phdr, HDR_PIDHIGH);
+	if (pidhigh != 0xDEAD){
+		printf("Incorrect pidhigh 0x%x, should be 0xDEAD\n",
+			(unsigned int)pidhigh);
+		tevent_req_nterror(req, NT_STATUS_INVALID_NETWORK_RESPONSE);
+		return;
+	}
+
+	tevent_req_done(req);
+}
+
+static NTSTATUS pid_echo_recv(struct tevent_req *req)
+{
+	return tevent_req_simple_recv_ntstatus(req);
+}
+
+static bool run_pidhigh(int dummy)
+{
+	bool success = false;
+	struct cli_state *cli = NULL;
+	NTSTATUS status;
+	struct tevent_context *ev = NULL;
+	struct tevent_req *req = NULL;
+	TALLOC_CTX *frame = talloc_stackframe();
+
+	printf("starting pid high test\n");
+	if (!torture_open_connection(&cli, 0)) {
+		return false;
+	}
+	smbXcli_conn_set_sockopt(cli->conn, sockops);
+
+	ev = samba_tevent_context_init(frame);
+	if (ev == NULL) {
+                goto fail;
+	}
+
+	req = pid_echo_send(frame, ev, cli);
+	if (req == NULL) {
+		goto fail;
+	}
+
+	if (!tevent_req_poll_ntstatus(req, ev, &status)) {
+		goto fail;
+	}
+
+	status = pid_echo_recv(req);
+	if (NT_STATUS_IS_OK(status)) {
+		printf("pid high test ok\n");
+		success = true;
+	}
+
+ fail:
+
+	TALLOC_FREE(frame);
+	torture_close_connection(cli);
+	return success;
+}
+
 static bool run_local_substitute(int dummy)
 {
 	bool ok = true;
@@ -10197,6 +10355,7 @@ static struct {
 	{ "CLEANUP3", run_cleanup3 },
 	{ "CLEANUP4", run_cleanup4 },
 	{ "OPLOCK-CANCEL", run_oplock_cancel },
+	{ "PIDHIGH", run_pidhigh },
 	{ "LOCAL-SUBSTITUTE", run_local_substitute, 0},
 	{ "LOCAL-GENCACHE", run_local_gencache, 0},
 	{ "LOCAL-TALLOC-DICT", run_local_talloc_dict, 0},
-- 
2.7.4

-------------- next part --------------
From 25b74b9f8f28f60c70b4cf7f7e9b45b1245453cd Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 13 Jun 2016 09:20:43 -0700
Subject: [PATCH 1/5] s3: smbd: Remove unused 'req' argument from
 setup_readX_header()

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/reply.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index e0e55c6..0328ae9 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -3922,8 +3922,7 @@ strict_unlock:
  Setup readX header.
 ****************************************************************************/
 
-static int setup_readX_header(struct smb_request *req, char *outbuf,
-			      size_t smb_maxcnt)
+static int setup_readX_header(char *outbuf, size_t smb_maxcnt)
 {
 	int outsize;
 
@@ -4010,7 +4009,7 @@ static void send_file_readX(connection_struct *conn, struct smb_request *req,
 		header = data_blob_const(headerbuf, sizeof(headerbuf));
 
 		construct_reply_common_req(req, (char *)headerbuf);
-		setup_readX_header(req, (char *)headerbuf, smb_maxcnt);
+		setup_readX_header((char *)headerbuf, smb_maxcnt);
 
 		nread = SMB_VFS_SENDFILE(xconn->transport.sock, fsp, &header,
 					 startpos, smb_maxcnt);
@@ -4111,7 +4110,7 @@ normal_read:
 		}
 
 		construct_reply_common_req(req, (char *)headerbuf);
-		setup_readX_header(req, (char *)headerbuf, smb_maxcnt);
+		setup_readX_header((char *)headerbuf, smb_maxcnt);
 
 		/* Send out the header. */
 		ret = write_data(xconn->transport.sock, (char *)headerbuf,
@@ -4161,7 +4160,7 @@ nosendfile_read:
 		return;
 	}
 
-	setup_readX_header(req, (char *)req->outbuf, nread);
+	setup_readX_header((char *)req->outbuf, nread);
 
 	DEBUG(3, ("send_file_readX %s max=%d nread=%d\n",
 		  fsp_fnum_dbg(fsp), (int)smb_maxcnt, (int)nread));
-- 
2.8.0.rc3.226.g39d4020


From 98fb2cfaed50e96330a3095936b99c4bfbecea83 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 13 Jun 2016 09:22:56 -0700
Subject: [PATCH 2/5] s3: smbd: Make setup_readX_header() externally accessible

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/proto.h | 1 +
 source3/smbd/reply.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 3612034..81bdc87 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -932,6 +932,7 @@ ssize_t sendfile_short_send(struct smbXsrv_connection *xconn,
 			    size_t smb_maxcnt);
 void reply_readbraw(struct smb_request *req);
 void reply_lockread(struct smb_request *req);
+int setup_readX_header(char *outbuf, size_t smb_maxcnt);
 void reply_read(struct smb_request *req);
 void reply_read_and_X(struct smb_request *req);
 void error_to_writebrawerr(struct smb_request *req);
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 0328ae9..559aab0 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -3922,7 +3922,7 @@ strict_unlock:
  Setup readX header.
 ****************************************************************************/
 
-static int setup_readX_header(char *outbuf, size_t smb_maxcnt)
+int setup_readX_header(char *outbuf, size_t smb_maxcnt)
 {
 	int outsize;
 
-- 
2.8.0.rc3.226.g39d4020


From 59cac8fd68ac241edb7d6846257fee89347c506f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 13 Jun 2016 09:25:02 -0700
Subject: [PATCH 3/5] s3: smbd: Use common function setup_readX_header() in aio
 read code.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/aio.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/source3/smbd/aio.c b/source3/smbd/aio.c
index 2958ac3..ff1be13 100644
--- a/source3/smbd/aio.c
+++ b/source3/smbd/aio.c
@@ -272,7 +272,6 @@ static void aio_pread_smb1_done(struct tevent_req *req)
 	files_struct *fsp = aio_ex->fsp;
 	int outsize;
 	char *outbuf = (char *)aio_ex->outbuf.data;
-	char *data = smb_buf(outbuf) + 1 /* padding byte */;
 	ssize_t nread;
 	struct vfs_aio_state vfs_aio_state;
 
@@ -301,13 +300,7 @@ static void aio_pread_smb1_done(struct tevent_req *req)
 		ERROR_NT(map_nt_error_from_unix(vfs_aio_state.error));
 		outsize = srv_set_message(outbuf,0,0,true);
 	} else {
-		outsize = srv_set_message(outbuf, 12,
-					  nread + 1 /* padding byte */, false);
-		SSVAL(outbuf,smb_vwv2, 0xFFFF); /* Remaining - must be * -1. */
-		SSVAL(outbuf,smb_vwv5, nread);
-		SSVAL(outbuf,smb_vwv6, smb_offset(data,outbuf));
-		SSVAL(outbuf,smb_vwv7, ((nread >> 16) & 1));
-		SSVAL(smb_buf(outbuf), -2, nread);
+		outsize = setup_readX_header(outbuf, nread);
 
 		aio_ex->fsp->fh->pos = aio_ex->offset + nread;
 		aio_ex->fsp->fh->position_information = aio_ex->fsp->fh->pos;
-- 
2.8.0.rc3.226.g39d4020


From 704c9e0cb90de645481183b30b73c1fc920681b9 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 13 Jun 2016 09:30:25 -0700
Subject: [PATCH 4/5] s3: smbd: In reply_read_and_X() SMB1 server is
 overwriting part of the 'reserved' zero fields with reply data length.

This occurred due to old code that used to do:

SSVAL(smb_buf(req->outbuf),-2,nread);

to set the reply length. This code was not needed,
as srv_set_message() was already correctly setting
the bcc length and was probably left from much
earlier legacy code.

However, in commit ddaa65ef6e049a185281c4d5deca4045e3b085e2
this was converted to do:

SSVAL(req->outbuf,smb_vwv11,smb_maxcnt);

This code actually overwrites the last 'reserved'
field in the SMB_COM_READ_ANDX packet reply, but we
never noticed as no client (or server code) looks at or
checks vwv11 in a SMB_COM_READ_ANDX reply.

[MS-SMB] shows for SMB_COM_READ_ANDX reply:

SMB_Parameters
{
        UCHAR WordCount;
        Words
        {
                UCHAR AndXCommand;
                UCHAR AndXReserved;
                USHORT AndXOffset;
                USHORT Available;
                USHORT DataCompactionMode;
                USHORT Reserved1;
                USHORT DataLength;
                USHORT DataOffset;
                USHORT DataLengthHigh;
                USHORT Reserved2[4];
        }
}
SMB_Data
{
        USHORT ByteCount;
        Bytes
        {
                UCHAR Pad[] (optional);
                UCHAR Data[variable];
        }

and indeed checking wireshark from Win2012R2
we find that smbd is writing the returned
read length into smb_vwv11 and Windows leaves
it as zeros (reserved).

Also fix the same problem in the named pipes code.

Torture test to ensure Reserved2[4] replies
are zero to follow.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/pipes.c | 1 -
 source3/smbd/reply.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/source3/smbd/pipes.c b/source3/smbd/pipes.c
index 2c9516d..bdc5af0 100644
--- a/source3/smbd/pipes.c
+++ b/source3/smbd/pipes.c
@@ -492,7 +492,6 @@ static void pipe_read_andx_done(struct tevent_req *subreq)
 	      + 12 * sizeof(uint16_t) /* vwv */
 	      + 2		/* the buflen field */
 	      + 1);		/* padding byte */
-	SSVAL(req->outbuf,smb_vwv11,state->smb_maxcnt);
 
 	DEBUG(3,("readX-IPC min=%d max=%d nread=%d\n",
 		 state->smb_mincnt, state->smb_maxcnt, (int)nread));
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 559aab0..0b7a4fb 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -3941,7 +3941,6 @@ int setup_readX_header(char *outbuf, size_t smb_maxcnt)
 	      + 2		/* the buflen field */
 	      + 1);		/* padding byte */
 	SSVAL(outbuf,smb_vwv7,(smb_maxcnt >> 16));
-	SSVAL(outbuf,smb_vwv11,smb_maxcnt);
 	SCVAL(smb_buf(outbuf), 0, 0); /* padding byte */
 	/* Reset the outgoing length, set_message truncates at 0x1FFFF. */
 	_smb_setlen_large(outbuf,
-- 
2.8.0.rc3.226.g39d4020


From b277e7670e6095df3e94692cb23feae6d2aa48e3 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 10 Jun 2016 09:32:32 -0700
Subject: [PATCH 5/5] s4: torture: Added raw readX test to ensure 'reserved'
 fields are zero.

Passes against Win2k12+, and smbd with the previous patch.

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

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source4/torture/raw/read.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/source4/torture/raw/read.c b/source4/torture/raw/read.c
index 6abf08b..6160e3e 100644
--- a/source4/torture/raw/read.c
+++ b/source4/torture/raw/read.c
@@ -19,6 +19,7 @@
 
 #include "includes.h"
 #include "libcli/raw/libcliraw.h"
+#include "libcli/raw/raw_proto.h"
 #include "system/time.h"
 #include "system/filesys.h"
 #include "libcli/libcli.h"
@@ -373,6 +374,8 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 	const char *fname = BASEDIR "\\test.txt";
 	const char *test_data = "TEST DATA";
 	unsigned int seed = time(NULL);
+	struct smbcli_request *smbreq = NULL;
+	unsigned int i;
 
 	buf = talloc_zero_array(tctx, uint8_t, maxsize);
 
@@ -422,6 +425,47 @@ static bool test_readx(struct torture_context *tctx, struct smbcli_state *cli)
 
 	smbcli_write(cli->tree, fnum, 0, test_data, 0, strlen(test_data));
 
+	printf("Checking reserved fields are [0]\n");
+	io.readx.in.file.fnum = fnum;
+	io.readx.in.offset = 0;
+	io.readx.in.remaining = 0;
+	io.readx.in.read_for_execute = false;
+	io.readx.in.mincnt = strlen(test_data);
+	io.readx.in.maxcnt = strlen(test_data);
+	smbreq = smb_raw_read_send(cli->tree, &io);
+	if (smbreq == NULL) {
+		ret = false;
+		torture_fail_goto(tctx, done, "smb_raw_read_send failed\n");
+	}
+	if (!smbcli_request_receive(smbreq) ||
+	     smbcli_request_is_error(smbreq)) {
+		status = smbcli_request_destroy(smbreq);
+		torture_fail_goto(tctx, done, "receive failed\n");
+	}
+
+	if (smbreq->in.wct != 12) {
+		ret = false;
+		printf("Incorrect wct %u (should be 12)\n",
+			(unsigned int)smbreq->in.wct);
+		status = smbcli_request_destroy(smbreq);
+		torture_fail_goto(tctx, done, "bad wct\n");
+	}
+
+	/* Ensure VWV8 - WVW11 are zero. */
+	for (i = 8; i < 12; i++) {
+		uint16_t br = SVAL(smbreq->in.vwv, VWV(i));
+		if (br != 0) {
+			status = smbcli_request_destroy(smbreq);
+			ret = false;
+			printf("reserved field %u is %u not zero\n",
+				i,
+				(unsigned int)br);
+			torture_fail_goto(tctx, done, "bad reserved field\n");
+		}
+	}
+
+	smbcli_request_destroy(smbreq);
+
 	printf("Trying small read\n");
 	io.readx.in.file.fnum = fnum;
 	io.readx.in.offset = 0;
-- 
2.8.0.rc3.226.g39d4020



More information about the samba-technical mailing list