[PATCHES] prepare session lookup and channel validation for multi-channel

Michael Adam obnox at samba.org
Wed Jul 29 10:12:30 UTC 2015


Hi,

This is a patchset that comes as a preparation for
multi-channel. It prepares the code to do correct
validation of connections as channels on the session
in the appropriate places under the right conditions:
Basically, if the request is a channel bind, we do
not have the connection bound yet, so should not
validate it. (More precisely, when in a later step,
the binding session setup is implemented, we need
to verify that the connection does *not* belong to
the session as a channel yet..)

This patchset was done in collaboration with Metze
and already carries enough review/signoff, so I will
push it later if noone raises objections.

Cheers - Michael
-------------- next part --------------
From 73673f0324fda301b701c58f587787a57f6c6cbe Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 8 May 2015 23:12:19 +0200
Subject: [PATCH 1/7] smbXsrv: add a smbXsrv_connection argument to
 smbXsrv_session_local_lookup()

This way, we can verify that a session is valid on the channel.

Pair-Programmed-With: Michael Adam <obnox at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smbXsrv_session.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 3201670..256d820 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -584,6 +584,8 @@ static void smbXsrv_session_local_fetch_parser(TDB_DATA key, TDB_DATA data,
 }
 
 static NTSTATUS smbXsrv_session_local_lookup(struct smbXsrv_session_table *table,
+					     /* conn: optional */
+					     struct smbXsrv_connection *conn,
 					     uint32_t session_local_id,
 					     NTTIME now,
 					     struct smbXsrv_session **_session)
@@ -629,6 +631,19 @@ static NTSTATUS smbXsrv_session_local_lookup(struct smbXsrv_session_table *table
 		return NT_STATUS_USER_SESSION_DELETED;
 	}
 
+	/*
+	 * If a connection is specified check if the session is
+	 * valid on the channel.
+	 */
+	if (conn != NULL) {
+		struct smbXsrv_channel_global0 *c = NULL;
+
+		status = smbXsrv_session_find_channel(state.session, conn, &c);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+	}
+
 	state.session->idle_time = now;
 
 	if (!NT_STATUS_IS_OK(state.session->status)) {
@@ -1719,7 +1734,8 @@ NTSTATUS smb1srv_session_lookup(struct smbXsrv_connection *conn,
 	struct smbXsrv_session_table *table = conn->client->session_table;
 	uint32_t local_id = vuid;
 
-	return smbXsrv_session_local_lookup(table, local_id, now, session);
+	return smbXsrv_session_local_lookup(table, conn, local_id, now,
+					    session);
 }
 
 NTSTATUS smb2srv_session_table_init(struct smbXsrv_connection *conn)
@@ -1742,7 +1758,8 @@ static NTSTATUS smb2srv_session_lookup_raw(struct smbXsrv_session_table *table,
 		return NT_STATUS_USER_SESSION_DELETED;
 	}
 
-	return smbXsrv_session_local_lookup(table, local_id, now, session);
+	return smbXsrv_session_local_lookup(table, NULL, local_id, now,
+					    session);
 }
 
 NTSTATUS smb2srv_session_lookup(struct smbXsrv_connection *conn,
-- 
2.4.3


From fbbadd9a28799065fe45095d084c465b7bb9f2c0 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Fri, 8 May 2015 23:15:51 +0200
Subject: [PATCH 2/7] smbXsrv: add a smbXsrv_connection argument to
 smb2srv_session_lookup_raw

This way, we can verify that the session is valid on a channel.

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/smbd/smbXsrv_session.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 256d820..12cc43e 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -52,6 +52,8 @@ struct smbXsrv_session_table {
 };
 
 static NTSTATUS smb2srv_session_lookup_raw(struct smbXsrv_session_table *table,
+					   /* conn: optional */
+					   struct smbXsrv_connection *conn,
 					   uint64_t session_id, NTTIME now,
 					   struct smbXsrv_session **session);
 
@@ -281,6 +283,7 @@ static void smbXsrv_session_close_loop(struct tevent_req *subreq)
 	}
 
 	status = smb2srv_session_lookup_raw(client->session_table,
+					    NULL, /* smbXsrv_connection */
 					    close_info0->old_session_wire_id,
 					    now, &session);
 	if (NT_STATUS_EQUAL(status, NT_STATUS_USER_SESSION_DELETED)) {
@@ -1748,6 +1751,8 @@ NTSTATUS smb2srv_session_table_init(struct smbXsrv_connection *conn)
 }
 
 static NTSTATUS smb2srv_session_lookup_raw(struct smbXsrv_session_table *table,
+					   /* conn: optional */
+					   struct smbXsrv_connection *conn,
 					   uint64_t session_id, NTTIME now,
 					   struct smbXsrv_session **session)
 {
@@ -1758,7 +1763,7 @@ static NTSTATUS smb2srv_session_lookup_raw(struct smbXsrv_session_table *table,
 		return NT_STATUS_USER_SESSION_DELETED;
 	}
 
-	return smbXsrv_session_local_lookup(table, NULL, local_id, now,
+	return smbXsrv_session_local_lookup(table, conn, local_id, now,
 					    session);
 }
 
@@ -1767,7 +1772,8 @@ NTSTATUS smb2srv_session_lookup(struct smbXsrv_connection *conn,
 				struct smbXsrv_session **session)
 {
 	struct smbXsrv_session_table *table = conn->client->session_table;
-	return smb2srv_session_lookup_raw(table, session_id, now, session);
+	return smb2srv_session_lookup_raw(table, conn, session_id, now,
+					  session);
 }
 
 struct smbXsrv_session_global_traverse_state {
-- 
2.4.3


From 6f46cdd7095e30051496af90e340723b09e32cef Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 29 Jul 2015 10:23:14 +0200
Subject: [PATCH 3/7] smbXsrv: rename smb2srv_session_lookup ->
 smb2srv_session_lookup_conn

This is in preparation of adding a variant that operates
on the client and does in particular not verify that the
connection belongs to a session as a channel.

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/smbd/globals.h         | 6 +++---
 source3/smbd/smb2_break.c      | 8 ++++----
 source3/smbd/smb2_server.c     | 9 +++++----
 source3/smbd/smbXsrv_session.c | 6 +++---
 4 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 35a3ee9..dbdc9e8 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -553,9 +553,9 @@ NTSTATUS smb1srv_session_lookup(struct smbXsrv_connection *conn,
 				uint16_t vuid, NTTIME now,
 				struct smbXsrv_session **session);
 NTSTATUS smb2srv_session_table_init(struct smbXsrv_connection *conn);
-NTSTATUS smb2srv_session_lookup(struct smbXsrv_connection *conn,
-				uint64_t session_id, NTTIME now,
-				struct smbXsrv_session **session);
+NTSTATUS smb2srv_session_lookup_conn(struct smbXsrv_connection *conn,
+				     uint64_t session_id, NTTIME now,
+				     struct smbXsrv_session **session);
 struct smbXsrv_session_global0;
 NTSTATUS smbXsrv_session_global_traverse(
 			int (*fn)(struct smbXsrv_session_global0 *, void *),
diff --git a/source3/smbd/smb2_break.c b/source3/smbd/smb2_break.c
index 5eab0a1..4c5d62e 100644
--- a/source3/smbd/smb2_break.c
+++ b/source3/smbd/smb2_break.c
@@ -450,10 +450,10 @@ void send_break_message_smb2(files_struct *fsp,
 	 */
 	xconn = fsp->conn->sconn->client->connections;
 
-	status = smb2srv_session_lookup(xconn,
-					fsp->vuid,
-					now,
-					&session);
+	status = smb2srv_session_lookup_conn(xconn,
+					     fsp->vuid,
+					     now,
+					     &session);
 	if (NT_STATUS_EQUAL(status, NT_STATUS_USER_SESSION_DELETED) ||
 	    (session == NULL))
 	{
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 2ea997e..442b361 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -393,7 +393,8 @@ static NTSTATUS smbd_smb2_inbuf_parse_compound(struct smbXsrv_connection *xconn,
 				goto inval;
 			}
 
-			status = smb2srv_session_lookup(xconn, uid, now, &s);
+			status = smb2srv_session_lookup_conn(xconn, uid, now,
+							     &s);
 			if (s == NULL) {
 				DEBUG(1, ("invalid session[%llu] in "
 					  "SMB2_TRANSFORM header\n",
@@ -1833,9 +1834,9 @@ static NTSTATUS smbd_smb2_request_check_session(struct smbd_smb2_request *req)
 	req->last_session_id = 0;
 
 	/* lookup an existing session */
-	status = smb2srv_session_lookup(req->xconn,
-					in_session_id, now,
-					&session);
+	status = smb2srv_session_lookup_conn(req->xconn,
+					     in_session_id, now,
+					     &session);
 	if (session) {
 		req->session = session;
 		req->last_session_id = in_session_id;
diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 12cc43e..8e275ad 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -1767,9 +1767,9 @@ static NTSTATUS smb2srv_session_lookup_raw(struct smbXsrv_session_table *table,
 					    session);
 }
 
-NTSTATUS smb2srv_session_lookup(struct smbXsrv_connection *conn,
-				uint64_t session_id, NTTIME now,
-				struct smbXsrv_session **session)
+NTSTATUS smb2srv_session_lookup_conn(struct smbXsrv_connection *conn,
+				     uint64_t session_id, NTTIME now,
+				     struct smbXsrv_session **session)
 {
 	struct smbXsrv_session_table *table = conn->client->session_table;
 	return smb2srv_session_lookup_raw(table, conn, session_id, now,
-- 
2.4.3


From b6ad13c065687150362bb950bea3b3887ed2c92a Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 27 Jul 2015 08:59:57 +0200
Subject: [PATCH 4/7] smbXsrv: add smb2srv_session_lookup_client().

This is a variant of smb2srv_session_lookup_conn() that does
not verify the session on the channel.

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/smbd/globals.h         | 3 +++
 source3/smbd/smbXsrv_session.c | 9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index dbdc9e8..1885629 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -556,6 +556,9 @@ NTSTATUS smb2srv_session_table_init(struct smbXsrv_connection *conn);
 NTSTATUS smb2srv_session_lookup_conn(struct smbXsrv_connection *conn,
 				     uint64_t session_id, NTTIME now,
 				     struct smbXsrv_session **session);
+NTSTATUS smb2srv_session_lookup_client(struct smbXsrv_client *client,
+				       uint64_t session_id, NTTIME now,
+				       struct smbXsrv_session **session);
 struct smbXsrv_session_global0;
 NTSTATUS smbXsrv_session_global_traverse(
 			int (*fn)(struct smbXsrv_session_global0 *, void *),
diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 8e275ad..cb84f71 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -1776,6 +1776,15 @@ NTSTATUS smb2srv_session_lookup_conn(struct smbXsrv_connection *conn,
 					  session);
 }
 
+NTSTATUS smb2srv_session_lookup_client(struct smbXsrv_client *client,
+				       uint64_t session_id, NTTIME now,
+				       struct smbXsrv_session **session)
+{
+	struct smbXsrv_session_table *table = client->session_table;
+	return smb2srv_session_lookup_raw(table, NULL, session_id, now,
+					  session);
+}
+
 struct smbXsrv_session_global_traverse_state {
 	int (*fn)(struct smbXsrv_session_global0 *, void *);
 	void *private_data;
-- 
2.4.3


From b24d401fc9f18668d6c20b6dde6dfee5e249f7c1 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 29 Jul 2015 10:35:08 +0200
Subject: [PATCH 5/7] smbXsrv: use smb2srv_session_lookup_client in
 smbXsrv_session_close_loop

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/smbd/smbXsrv_session.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index cb84f71..ff4f14f 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -282,10 +282,9 @@ static void smbXsrv_session_close_loop(struct tevent_req *subreq)
 		goto next;
 	}
 
-	status = smb2srv_session_lookup_raw(client->session_table,
-					    NULL, /* smbXsrv_connection */
-					    close_info0->old_session_wire_id,
-					    now, &session);
+	status = smb2srv_session_lookup_client(client,
+					       close_info0->old_session_wire_id,
+					       now, &session);
 	if (NT_STATUS_EQUAL(status, NT_STATUS_USER_SESSION_DELETED)) {
 		DEBUG(4,("smbXsrv_session_close_loop: "
 			 "old_session_wire_id %llu not found\n",
-- 
2.4.3


From fda6554faaf3b053cc3d3adf9c220e718ef26f04 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Wed, 29 Jul 2015 11:19:55 +0200
Subject: [PATCH 6/7] s3:smb2_sessetup: check that the connection belongs to
 the session in sess.setup

Signed-off-by: Michael Adam <obnox at samba.org>
Reviewed-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/smb2_sesssetup.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index 11d381f..3233846 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -561,6 +561,7 @@ static struct tevent_req *smbd_smb2_session_setup_send(TALLOC_CTX *mem_ctx,
 	NTSTATUS status;
 	NTTIME now = timeval_to_nttime(&smb2req->request_time);
 	struct tevent_req *subreq;
+	struct smbXsrv_channel_global0 *c = NULL;
 
 	req = tevent_req_create(mem_ctx, &state,
 				struct smbd_smb2_session_setup_state);
@@ -618,6 +619,13 @@ static struct tevent_req *smbd_smb2_session_setup_send(TALLOC_CTX *mem_ctx,
 		}
 	}
 
+	status = smbXsrv_session_find_channel(smb2req->session,
+					      smb2req->xconn, &c);
+	if (!NT_STATUS_IS_OK(status)) {
+		tevent_req_nterror(req, status);
+		return tevent_req_post(req, ev);
+	}
+
 	if (state->session->gensec == NULL) {
 		status = auth_generic_prepare(state->session,
 					      state->smb2req->xconn->remote_address,
-- 
2.4.3


From 504ca58ce34b80af52ab57b1242f6a3908c606c2 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 27 Jul 2015 09:01:55 +0200
Subject: [PATCH 7/7] s3:smb2_server: defer channel/session validation to the
 session setup code.

For session bind, and the channel is only to be bound to the given
session just now, so it is not valid. The early request validation
code can hence not check it, and hence validation is defered to the
actual session setup code, which can look at the session binding flags.

Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Michael Adam <obnox at samba.org>
Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/smbd/smb2_server.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index 442b361..a0b1bfc 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -1833,10 +1833,26 @@ static NTSTATUS smbd_smb2_request_check_session(struct smbd_smb2_request *req)
 
 	req->last_session_id = 0;
 
-	/* lookup an existing session */
-	status = smb2srv_session_lookup_conn(req->xconn,
-					     in_session_id, now,
-					     &session);
+	/* look an existing session up */
+	switch (in_opcode) {
+	case SMB2_OP_SESSSETUP:
+		/*
+		 * For a session bind request, we don't have the
+		 * channel set up at this point yet, so we defer
+		 * the verification that the connection belongs
+		 * to the session to the session setup code, which
+		 * can look at the session binding flags.
+		 */
+		status = smb2srv_session_lookup_client(req->xconn->client,
+						       in_session_id, now,
+						       &session);
+		break;
+	default:
+		status = smb2srv_session_lookup_conn(req->xconn,
+						     in_session_id, now,
+						     &session);
+		break;
+	}
 	if (session) {
 		req->session = session;
 		req->last_session_id = in_session_id;
-- 
2.4.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150729/698e2b28/attachment.sig>


More information about the samba-technical mailing list