Attachment 15254 Details for Bug 14000 – WIP patch for master

WIP patch for master

tmp.diff.txt (text/plain), 9.06 KB, created by Stefan Metzmacher on 2019-06-18 12:12:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Metzmacher

Created: 2019-06-18 12:12:12 UTC

Size: 9.06 KB

patch

obsolete

>From a2e420e25ef82293f1b067127c24b7d1a1b07f48 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 18 Jun 2019 13:41:46 +0200
>Subject: [PATCH 1/5] smbd: introduce a static change_to_user_by_vuid()
> function
>
>This will allow us to split the public change_to_user*() and
>become_user*() layers.
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/smbd/uid.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
>diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
>index a4bcb747d37e..65fb45271494 100644
>--- a/source3/smbd/uid.c
>+++ b/source3/smbd/uid.c
>@@ -432,7 +432,7 @@ static bool change_to_user_internal(connection_struct *conn,
> 	return true;
> }
> 
>-bool change_to_user(connection_struct *conn, uint64_t vuid)
>+static bool change_to_user_by_vuid(connection_struct *conn, uint64_t vuid)
> {
> 	struct user_struct *vuser;
> 	int snum = SNUM(conn);
>@@ -454,9 +454,14 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
> 	return change_to_user_internal(conn, vuser->session_info, vuid);
> }
> 
>+bool change_to_user(connection_struct *conn, uint64_t vuid)
>+{
>+	return change_to_user_by_vuid(conn, vuid);
>+}
>+
> bool change_to_user_by_fsp(struct files_struct *fsp)
> {
>-	return change_to_user(fsp->conn, fsp->vuid);
>+	return change_to_user_by_vuid(fsp->conn, fsp->vuid);
> }
> 
> static bool change_to_user_by_session(connection_struct *conn,
>@@ -625,7 +630,7 @@ void smbd_unbecome_root(void)
> }
> 
> /****************************************************************************
>- Push the current security context then force a change via change_to_user().
>+ Push the current security context then force a change via change_to_user_by_vuid().
>  Saves and restores the connection context.
> ****************************************************************************/
> 
>@@ -636,7 +641,7 @@ bool become_user(connection_struct *conn, uint64_t vuid)
> 
> 	push_conn_ctx();
> 
>-	if (!change_to_user(conn, vuid)) {
>+	if (!change_to_user_by_vuid(conn, vuid)) {
> 		pop_sec_ctx();
> 		pop_conn_ctx();
> 		return False;
>-- 
>2.17.1
>
>
>From 78afcdc4d098e0f755927f2e644a0af0baedc7df Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 18 Jun 2019 13:44:36 +0200
>Subject: [PATCH 2/5] TODO: smbd: assert that
> change_to_{user,user_by_fsp,guest,root_user}() are never stacked
>
>They should never be called from within a become_{root,user}() block!
>---
> source3/smbd/uid.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
>index 65fb45271494..ea423f8502f2 100644
>--- a/source3/smbd/uid.c
>+++ b/source3/smbd/uid.c
>@@ -30,6 +30,16 @@
> /* what user is current? */
> extern struct current_user current_user;
> 
>+static void smbd_assert_sec_ctx_stack_empty(const char *func)
>+{
>+	if (sec_ctx_stack_ndx == 0) {
>+		return;
>+	}
>+	DBG_ERR("Security context stack not empty in %s()!\n", func);
>+	// TODO: dump whole sec_ctx and conn_ctx stacks here!!!
>+	smb_panic("Security context stack not empty!");
>+}
>+
> /****************************************************************************
>  Become the guest user without changing the security context stack.
> ****************************************************************************/
>@@ -38,6 +48,8 @@ bool change_to_guest(void)
> {
> 	struct passwd *pass;
> 
>+	smbd_assert_sec_ctx_stack_empty(__func__);
>+
> 	pass = Get_Pwnam_alloc(talloc_tos(), lp_guest_account());
> 	if (!pass) {
> 		return false;
>@@ -456,11 +468,13 @@ static bool change_to_user_by_vuid(connection_struct *conn, uint64_t vuid)
> 
> bool change_to_user(connection_struct *conn, uint64_t vuid)
> {
>+	smbd_assert_sec_ctx_stack_empty(__func__);
> 	return change_to_user_by_vuid(conn, vuid);
> }
> 
> bool change_to_user_by_fsp(struct files_struct *fsp)
> {
>+	smbd_assert_sec_ctx_stack_empty(__func__);
> 	return change_to_user_by_vuid(fsp->conn, fsp->vuid);
> }
> 
>@@ -480,6 +494,8 @@ static bool change_to_user_by_session(connection_struct *conn,
> 
> bool smbd_change_to_root_user(void)
> {
>+	smbd_assert_sec_ctx_stack_empty(__func__);
>+
> 	set_root_sec_ctx();
> 
> 	DEBUG(5,("change_to_root_user: now uid=(%d,%d) gid=(%d,%d)\n",
>-- 
>2.17.1
>
>
>From c1edc531112edaa7b10e33bb02a8d1d8f68ee0b5 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 18 Jun 2019 14:00:53 +0200
>Subject: [PATCH 3/5] smbd: assert that become_user*() is never called in a
> become_root() block
>
>Signed-off-by: Stefan Metzmacher <metze@samba.org>
>---
> source3/smbd/uid.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
>diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
>index ea423f8502f2..a6b8c637ae0e 100644
>--- a/source3/smbd/uid.c
>+++ b/source3/smbd/uid.c
>@@ -30,6 +30,8 @@
> /* what user is current? */
> extern struct current_user current_user;
> 
>+static int first_root_sec_ctx;
>+
> static void smbd_assert_sec_ctx_stack_empty(const char *func)
> {
> 	if (sec_ctx_stack_ndx == 0) {
>@@ -517,6 +519,14 @@ bool smbd_change_to_root_user(void)
> 
> bool smbd_become_authenticated_pipe_user(struct auth_session_info *session_info)
> {
>+	if (first_root_sec_ctx != 0) {
>+		/*
>+		 * become_user*() is not allowed
>+		 * from within become_root()
>+		 */
>+		return false;
>+	}
>+
> 	if (!push_sec_ctx())
> 		return False;
> 
>@@ -633,6 +643,10 @@ void smbd_become_root(void)
> 	if (!push_sec_ctx()) {
> 		smb_panic("become_root: push_sec_ctx failed");
> 	}
>+	if (first_root_sec_ctx == 0) {
>+		SMB_ASSERT(sec_ctx_stack_ndx != 0);
>+		first_root_sec_ctx = sec_ctx_stack_ndx;
>+	}
> 	push_conn_ctx();
> 	set_root_sec_ctx();
> }
>@@ -641,6 +655,10 @@ void smbd_become_root(void)
> 
> void smbd_unbecome_root(void)
> {
>+	SMB_ASSERT(first_root_sec_ctx != 0);
>+	if (first_root_sec_ctx == sec_ctx_stack_ndx) {
>+		first_root_sec_ctx = 0;
>+	}
> 	pop_sec_ctx();
> 	pop_conn_ctx();
> }
>@@ -652,6 +670,14 @@ void smbd_unbecome_root(void)
> 
> bool become_user(connection_struct *conn, uint64_t vuid)
> {
>+	if (first_root_sec_ctx != 0) {
>+		/*
>+		 * become_user*() is not allowed
>+		 * from within become_root()
>+		 */
>+		return false;
>+	}
>+
> 	if (!push_sec_ctx())
> 		return False;
> 
>@@ -674,6 +700,14 @@ bool become_user_by_fsp(struct files_struct *fsp)
> bool become_user_by_session(connection_struct *conn,
> 			    const struct auth_session_info *session_info)
> {
>+	if (first_root_sec_ctx != 0) {
>+		/*
>+		 * become_user*() is not allowed
>+		 * from within become_root()
>+		 */
>+		return false;
>+	}
>+
> 	if (!push_sec_ctx())
> 		return false;
> 
>-- 
>2.17.1
>
>
>From 85b1ba0f0c1c00104d6b54fa900828fdb4954816 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 18 Jun 2019 14:02:09 +0200
>Subject: [PATCH 4/5] TODO smbd: make sure vfs_ChDir() and SMB_VFS_*_CHDIR()
> fails within a become_root() block
>
>---
> source3/smbd/uid.c |  4 ++++
> source3/smbd/vfs.c | 17 +++++++++++++++++
> 2 files changed, 21 insertions(+)
>
>diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
>index a6b8c637ae0e..ba5322f3f7c0 100644
>--- a/source3/smbd/uid.c
>+++ b/source3/smbd/uid.c
>@@ -30,6 +30,8 @@
> /* what user is current? */
> extern struct current_user current_user;
> 
>+bool smb_vfs_disable_chdir;
>+
> static int first_root_sec_ctx;
> 
> static void smbd_assert_sec_ctx_stack_empty(const char *func)
>@@ -646,6 +648,7 @@ void smbd_become_root(void)
> 	if (first_root_sec_ctx == 0) {
> 		SMB_ASSERT(sec_ctx_stack_ndx != 0);
> 		first_root_sec_ctx = sec_ctx_stack_ndx;
>+		smb_vfs_disable_chdir = true;
> 	}
> 	push_conn_ctx();
> 	set_root_sec_ctx();
>@@ -658,6 +661,7 @@ void smbd_unbecome_root(void)
> 	SMB_ASSERT(first_root_sec_ctx != 0);
> 	if (first_root_sec_ctx == sec_ctx_stack_ndx) {
> 		first_root_sec_ctx = 0;
>+		smb_vfs_disable_chdir = false;
> 	}
> 	pop_sec_ctx();
> 	pop_conn_ctx();
>diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
>index d3bb9c5d63fb..82ac7558f0ea 100644
>--- a/source3/smbd/vfs.c
>+++ b/source3/smbd/vfs.c
>@@ -788,6 +788,14 @@ int vfs_ChDir(connection_struct *conn, const struct smb_filename *smb_fname)
> {
> 	int ret;
> 	struct smb_filename *old_cwd = conn->cwd_fname;
>+	extern bool smb_vfs_disable_chdir;
>+
>+	if (unlikely(smb_vfs_disable_chdir)) {
>+		DBG_ERR("TODO...\n");
>+		log_stack_trace();
>+		errno = EACCES;
>+		return -1;
>+	}
> 
> 	if (!LastDir) {
> 		LastDir = SMB_STRDUP("");
>@@ -2128,6 +2136,15 @@ NTSTATUS vfs_chown_fsp(files_struct *fsp, uid_t uid, gid_t gid)
> int smb_vfs_call_chdir(struct vfs_handle_struct *handle,
> 			const struct smb_filename *smb_fname)
> {
>+	extern bool smb_vfs_disable_chdir;
>+
>+	if (unlikely(smb_vfs_disable_chdir)) {
>+		DBG_ERR("TODO...\n");
>+		log_stack_trace();
>+		errno = EACCES;
>+		return -1;
>+	}
>+
> 	VFS_FIND(chdir);
> 	return handle->fns->chdir_fn(handle, smb_fname);
> }
>-- 
>2.17.1
>
>
>From 1e8d7b91d14f1628dd9ea9fc0307d2a70fa87175 Mon Sep 17 00:00:00 2001
>From: Stefan Metzmacher <metze@samba.org>
>Date: Tue, 18 Jun 2019 14:04:08 +0200
>Subject: [PATCH 5/5] smbd: make sure we reset current_user.{need,done}_chdir
> in become_root()
>
>---
> source3/smbd/uid.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
>index ba5322f3f7c0..558e3ef9de8d 100644
>--- a/source3/smbd/uid.c
>+++ b/source3/smbd/uid.c
>@@ -652,6 +652,9 @@ void smbd_become_root(void)
> 	}
> 	push_conn_ctx();
> 	set_root_sec_ctx();
>+
>+	current_user.need_chdir = false;
>+	current_user.done_chdir = false;
> }
> 
> /* Unbecome the root user */
>-- 
>2.17.1
>

Actions: View

Attachments on bug 14000: 15253 | 15254