[PATCH] Master fix for bug #9518 - conn->share_access appears not be be reset between users
Jeremy Allison
jra at samba.org
Tue Jan 8 18:21:41 MST 2013
On Tue, Jan 08, 2013 at 02:01:34PM -0800, Jeremy Allison wrote:
> On Wed, Jan 09, 2013 at 08:58:53AM +1100, Andrew Bartlett wrote:
> >
> > Thanks. It should be the first patch in the series, to preserve the git
> > bisect. (I like to do that when it is as simple a getting the patch
> > order right).
>
> No problem, will do. Thanks for helping track this down !
Ok. Here is the complete set, starting with your fix,
then my changes to change all callers of "conn = talloc_zero(ctx, connection_struct)"
to call create_conn_struct() instead, then the rest of the fix.
The change for 4.0.x I'll attach to the bug report will be
simpler as it dowsn't need these first changes as it doesn't
redefine the vuid array.
If you're happy, please push.
Cheers,
Jeremy.
-------------- next part --------------
>From a209f3061e31fda802fa8935e12ef8ca78078ea0 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 9 Jan 2013 08:02:23 +1100
Subject: [PATCH 01/28] smbd: Rework create_conn_struct to use conn_new()
This avoids issues with required structure elements not being initialised.
In particular, once conn->vuid_cache becomes a pointer, this ensures
that it is initalised in all cases.
Andrew Bartlett
Reviewed by: Jeremy Allison <jra at samba.org>
---
source3/smbd/msdfs.c | 40 ++++++++++++++++++----------------------
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c
index e8a0610..8e80cbb 100644
--- a/source3/smbd/msdfs.c
+++ b/source3/smbd/msdfs.c
@@ -234,12 +234,29 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx,
connection_struct *conn;
char *connpath;
const char *vfs_user;
+ struct smbd_server_connection *sconn;
- conn = talloc_zero(ctx, connection_struct);
+ sconn = talloc_zero(ctx, struct smbd_server_connection);
+ if (sconn == NULL) {
+ return NT_STATUS_NO_MEMORY;
+ }
+
+ sconn->ev_ctx = ev;
+ sconn->msg_ctx = msg;
+ sconn->sock = -1;
+ sconn->smb1.echo_handler.trusted_fd = -1;
+ sconn->smb1.echo_handler.socket_lock_fd = -1;
+
+ conn = conn_new(sconn);
if (conn == NULL) {
+ TALLOC_FREE(sconn);
return NT_STATUS_NO_MEMORY;
}
+ /* Now we have conn, we need to make sconn a child of conn,
+ * for a proper talloc tree */
+ talloc_steal(conn, sconn);
+
connpath = talloc_strdup(conn, path);
if (!connpath) {
TALLOC_FREE(conn);
@@ -254,32 +271,11 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx,
return NT_STATUS_NO_MEMORY;
}
- conn->sconn = talloc_zero(conn, struct smbd_server_connection);
- if (conn->sconn == NULL) {
- TALLOC_FREE(conn);
- return NT_STATUS_NO_MEMORY;
- }
-
- conn->sconn->ev_ctx = ev;
- conn->sconn->msg_ctx = msg;
- conn->sconn->sock = -1;
- conn->sconn->smb1.echo_handler.trusted_fd = -1;
- conn->sconn->smb1.echo_handler.socket_lock_fd = -1;
-
/* needed for smbd_vfs_init() */
- if (!(conn->params = talloc_zero(conn, struct share_params))) {
- DEBUG(0, ("TALLOC failed\n"));
- TALLOC_FREE(conn);
- return NT_STATUS_NO_MEMORY;
- }
-
conn->params->service = snum;
conn->cnum = TID_FIELD_INVALID;
- DLIST_ADD(conn->sconn->connections, conn);
- conn->sconn->num_connections++;
-
if (session_info != NULL) {
conn->session_info = copy_session_info(conn, session_info);
if (conn->session_info == NULL) {
--
1.8.1
>From 3bbdc8e2fd031c207875eeddb30f8df075ef7c68 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 8 Jan 2013 16:26:50 -0800
Subject: [PATCH 02/28] Allow create_conn_struct() to be called with snum ==
-1.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/msdfs.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c
index 8e80cbb..4bf7aba 100644
--- a/source3/smbd/msdfs.c
+++ b/source3/smbd/msdfs.c
@@ -235,6 +235,7 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx,
char *connpath;
const char *vfs_user;
struct smbd_server_connection *sconn;
+ const char *servicename = lp_const_servicename(snum);
sconn = talloc_zero(ctx, struct smbd_server_connection);
if (sconn == NULL) {
@@ -257,6 +258,10 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx,
* for a proper talloc tree */
talloc_steal(conn, sconn);
+ if (snum == -1 && servicename == NULL) {
+ servicename = "Unknown Service (snum == -1)";
+ }
+
connpath = talloc_strdup(conn, path);
if (!connpath) {
TALLOC_FREE(conn);
@@ -265,7 +270,7 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx,
connpath = talloc_string_sub(conn,
connpath,
"%S",
- lp_servicename(talloc_tos(), snum));
+ servicename);
if (!connpath) {
TALLOC_FREE(conn);
return NT_STATUS_NO_MEMORY;
@@ -299,7 +304,7 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx,
*/
if (conn->session_info) {
share_access_check(conn->session_info->security_token,
- lp_servicename(talloc_tos(), snum),
+ servicename,
MAXIMUM_ALLOWED_ACCESS,
&conn->share_access);
@@ -309,7 +314,7 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx,
DEBUG(0,("create_conn_struct: connection to %s "
"denied due to security "
"descriptor.\n",
- lp_servicename(talloc_tos(), snum)));
+ servicename));
conn_free(conn);
return NT_STATUS_ACCESS_DENIED;
} else {
@@ -329,7 +334,7 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx,
}
/* this must be the first filesystem operation that we do */
- if (SMB_VFS_CONNECT(conn, lp_servicename(talloc_tos(), snum), vfs_user) < 0) {
+ if (SMB_VFS_CONNECT(conn, servicename, vfs_user) < 0) {
DEBUG(0,("VFS connect failed!\n"));
conn_free(conn);
return NT_STATUS_UNSUCCESSFUL;
--
1.8.1
>From dc9c7835ef8f3d11af72413da903a9ba0bb29c05 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 8 Jan 2013 14:17:51 -0800
Subject: [PATCH 03/28] smbd/posix_acls.c: Use create_conn_struct(). Don't
hand-create connection structs.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/posix_acls.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index 0f3951f..fbeb662 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -4610,27 +4610,19 @@ NTSTATUS get_nt_acl_no_snum(TALLOC_CTX *ctx, const char *fname,
return NT_STATUS_NO_MEMORY;
}
- conn = talloc_zero(frame, connection_struct);
- if (conn == NULL) {
- TALLOC_FREE(frame);
- DEBUG(0, ("talloc failed\n"));
- return NT_STATUS_NO_MEMORY;
- }
+ status = create_conn_struct(ctx,
+ server_event_context(),
+ server_messaging_context(),
+ &conn,
+ -1,
+ "/",
+ NULL);
- if (!(conn->params = talloc(conn, struct share_params))) {
- DEBUG(0, ("talloc failed\n"));
- TALLOC_FREE(frame);
- return NT_STATUS_NO_MEMORY;
- }
-
- conn->params->service = -1;
-
- set_conn_connectpath(conn, "/");
-
- if (!smbd_vfs_init(conn)) {
- DEBUG(0,("smbd_vfs_init() failed!\n"));
+ if (!NT_STATUS_IS_OK(status)) {
+ DEBUG(0,("create_conn_struct returned %s.\n",
+ nt_errstr(status)));
TALLOC_FREE(frame);
- return NT_STATUS_INTERNAL_ERROR;
+ return status;
}
status = SMB_VFS_GET_NT_ACL(conn, fname, security_info_wanted, ctx, sd);
--
1.8.1
>From 46162e8e261446e97e3b849abe22ec7cd28f28be Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 8 Jan 2013 14:18:27 -0800
Subject: [PATCH 04/28] source3/smbd/pysmbd.c: Always use create_conn_struct().
Don't hand create connection structs.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/pysmbd.c | 34 ++++++++++------------------------
1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/source3/smbd/pysmbd.c b/source3/smbd/pysmbd.c
index 1bac9c2..873fead 100644
--- a/source3/smbd/pysmbd.c
+++ b/source3/smbd/pysmbd.c
@@ -46,6 +46,9 @@ static connection_struct *get_conn(TALLOC_CTX *mem_ctx, const char *service)
{
connection_struct *conn;
TALLOC_CTX *frame = talloc_stackframe();
+ int snum = -1;
+ NTSTATUS status;
+
if (!posix_locking_init(false)) {
PyErr_NoMemory();
TALLOC_FREE(frame);
@@ -53,39 +56,22 @@ static connection_struct *get_conn(TALLOC_CTX *mem_ctx, const char *service)
}
if (service) {
- NTSTATUS status;
- int snum = lp_servicenumber(service);
+ snum = lp_servicenumber(service);
if (snum == -1) {
TALLOC_FREE(frame);
PyErr_SetString(PyExc_RuntimeError, "unknown service");
return NULL;
}
- status = create_conn_struct(mem_ctx, NULL, NULL, &conn, snum, "/",
- NULL);
- PyErr_NTSTATUS_IS_ERR_RAISE(status);
- } else {
- conn = talloc_zero(mem_ctx, connection_struct);
- if (conn == NULL) {
- DEBUG(0, ("talloc failed\n"));
- TALLOC_FREE(frame);
- PyErr_NoMemory();
- return NULL;
- }
-
- if (!(conn->params = talloc(conn, struct share_params))) {
- TALLOC_FREE(frame);
- DEBUG(0,("get_conn: talloc() failed!\n"));
- PyErr_NoMemory();
- return NULL;
- }
- conn->params->service = -1;
+ }
- set_conn_connectpath(conn, "/");
+ status = create_conn_struct(mem_ctx, NULL, NULL, &conn, snum, "/",
+ NULL);
+ PyErr_NTSTATUS_IS_ERR_RAISE(status);
- smbd_vfs_init(conn);
- }
TALLOC_FREE(frame);
+ /* Ignore read-only and share restrictions */
conn->read_only = false;
+ conn->share_access = SEC_RIGHTS_FILE_ALL;
talloc_set_destructor(conn, conn_free_wrapper);
return conn;
}
--
1.8.1
>From 6bd4eb78e6cf5dd8710126bf14cb53896d25b2a8 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 8 Jan 2013 14:18:55 -0800
Subject: [PATCH 05/28] torture/vfstest.c: Always use create_conn_struct().
Don't hand create connection structs.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/torture/vfstest.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/source3/torture/vfstest.c b/source3/torture/vfstest.c
index dd3787b..de7e732 100644
--- a/source3/torture/vfstest.c
+++ b/source3/torture/vfstest.c
@@ -454,6 +454,7 @@ int main(int argc, char *argv[])
char cwd[MAXPATHLEN];
TALLOC_CTX *frame = talloc_stackframe();
struct tevent_context *ev = tevent_context_init(NULL);
+ struct auth_session_info *session_info = NULL;
NTSTATUS status = NT_STATUS_OK;
/* make sure the vars that get altered (4th field) are in
@@ -515,22 +516,33 @@ int main(int argc, char *argv[])
locking_init();
serverid_parent_init(NULL);
vfs = talloc_zero(NULL, struct vfs_state);
- vfs->conn = talloc_zero(vfs, connection_struct);
+ if (vfs == NULL) {
+ return 1;
+ }
+ status = make_session_info_guest(vfs, &session_info);
+ if (!NT_STATUS_IS_OK(status)) {
+ return 1;
+ }
+
+ status = create_conn_struct(vfs,
+ ev,
+ messaging_init(vfs, ev),
+ &vfs->conn,
+ -1,
+ getcwd(cwd, sizeof(cwd)),
+ session_info);
+ if (!NT_STATUS_IS_OK(status)) {
+ return 1;
+ }
+
vfs->conn->share_access = FILE_GENERIC_ALL;
- vfs->conn->params = talloc_zero(vfs->conn, struct share_params);
- vfs->conn->sconn = talloc_zero(NULL, struct smbd_server_connection);
- vfs->conn->sconn->msg_ctx = messaging_init(vfs->conn->sconn, ev);
- vfs->conn->sconn->ev_ctx = ev;
+ vfs->conn->read_only = false;
+
serverid_register(messaging_server_id(vfs->conn->sconn->msg_ctx), 0);
- make_session_info_guest(NULL, &vfs->conn->session_info);
file_init(vfs->conn->sconn);
- set_conn_connectpath(vfs->conn, getcwd(cwd, sizeof(cwd)));
for (i=0; i < 1024; i++)
vfs->files[i] = NULL;
- /* some advanced initialization stuff */
- smbd_vfs_init(vfs->conn);
-
if (!posix_locking_init(false)) {
return 1;
}
--
1.8.1
>From e70f07eb56f7bf7d9873b249f726eddb59e4e3e5 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:50:25 -0800
Subject: [PATCH 06/28] Start to tidy-up check_user_ok().
Now we have removed "security=share" we cannot be
called with vuid == UID_FIELD_INVALID.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 30c7154..2aa9fff 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -90,12 +90,11 @@ static bool check_user_ok(connection_struct *conn,
const struct auth_session_info *session_info,
int snum)
{
- bool valid_vuid = (vuid != UID_FIELD_INVALID);
unsigned int i;
bool readonly_share;
bool admin_user;
- if (valid_vuid) {
+ {
struct vuid_cache_entry *ent;
for (i=0; i<VUID_CACHE_SIZE; i++) {
@@ -145,7 +144,7 @@ static bool check_user_ok(connection_struct *conn,
session_info->info->domain_name,
NULL, session_info->security_token, lp_admin_users(snum));
- if (valid_vuid) {
+ {
struct vuid_cache_entry *ent =
&conn->vuid_cache.array[conn->vuid_cache.next_entry];
--
1.8.1
>From e975b50aa5e17b1d6078e6d6309040f811aa180b Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:51:55 -0800
Subject: [PATCH 07/28] Move the definition of struct vuid_cache_entry *ent
outside blocks.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 2aa9fff..9361e49 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -93,10 +93,9 @@ static bool check_user_ok(connection_struct *conn,
unsigned int i;
bool readonly_share;
bool admin_user;
+ struct vuid_cache_entry *ent = NULL;
{
- struct vuid_cache_entry *ent;
-
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache.array[i];
if (ent->vuid == vuid) {
@@ -145,8 +144,7 @@ static bool check_user_ok(connection_struct *conn,
NULL, session_info->security_token, lp_admin_users(snum));
{
- struct vuid_cache_entry *ent =
- &conn->vuid_cache.array[conn->vuid_cache.next_entry];
+ ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry];
conn->vuid_cache.next_entry =
(conn->vuid_cache.next_entry + 1) % VUID_CACHE_SIZE;
--
1.8.1
>From fae12854377349f3b6cc4c865938724516c12653 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:52:27 -0800
Subject: [PATCH 08/28] Remove one set of enclosing {} braces, no longer
needed.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9361e49..9109f05 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -95,15 +95,13 @@ static bool check_user_ok(connection_struct *conn,
bool admin_user;
struct vuid_cache_entry *ent = NULL;
- {
- for (i=0; i<VUID_CACHE_SIZE; i++) {
- ent = &conn->vuid_cache.array[i];
- if (ent->vuid == vuid) {
- free_conn_session_info_if_unused(conn);
- conn->session_info = ent->session_info;
- conn->read_only = ent->read_only;
- return(True);
- }
+ for (i=0; i<VUID_CACHE_SIZE; i++) {
+ ent = &conn->vuid_cache.array[i];
+ if (ent->vuid == vuid) {
+ free_conn_session_info_if_unused(conn);
+ conn->session_info = ent->session_info;
+ conn->read_only = ent->read_only;
+ return(True);
}
}
--
1.8.1
>From 1122ff85e361594e8d2f6248df6298d9474161f5 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:53:11 -0800
Subject: [PATCH 09/28] Remove the second set of {} braces, no longer needed.
---
source3/smbd/uid.c | 38 ++++++++++++++++++--------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9109f05..9fc5d7c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -141,33 +141,31 @@ static bool check_user_ok(connection_struct *conn,
session_info->info->domain_name,
NULL, session_info->security_token, lp_admin_users(snum));
- {
- ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry];
+ ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry];
- conn->vuid_cache.next_entry =
- (conn->vuid_cache.next_entry + 1) % VUID_CACHE_SIZE;
+ conn->vuid_cache.next_entry =
+ (conn->vuid_cache.next_entry + 1) % VUID_CACHE_SIZE;
- TALLOC_FREE(ent->session_info);
+ TALLOC_FREE(ent->session_info);
- /*
- * If force_user was set, all session_info's are based on the same
- * username-based faked one.
- */
-
- ent->session_info = copy_session_info(
- conn, conn->force_user ? conn->session_info : session_info);
+ /*
+ * If force_user was set, all session_info's are based on the same
+ * username-based faked one.
+ */
- if (ent->session_info == NULL) {
- ent->vuid = UID_FIELD_INVALID;
- return false;
- }
+ ent->session_info = copy_session_info(
+ conn, conn->force_user ? conn->session_info : session_info);
- ent->vuid = vuid;
- ent->read_only = readonly_share;
- free_conn_session_info_if_unused(conn);
- conn->session_info = ent->session_info;
+ if (ent->session_info == NULL) {
+ ent->vuid = UID_FIELD_INVALID;
+ return false;
}
+ ent->vuid = vuid;
+ ent->read_only = readonly_share;
+ free_conn_session_info_if_unused(conn);
+ conn->session_info = ent->session_info;
+
conn->read_only = readonly_share;
if (admin_user) {
DEBUG(2,("check_user_ok: user %s is an admin user. "
--
1.8.1
>From 668ffcf7b05ba6c81bce4adf87af3f2a76730a28 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:54:07 -0800
Subject: [PATCH 10/28] Remove dead code now vuser can no longer be NULL.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9fc5d7c..298fa83 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -299,14 +299,6 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
}
session_info = vuser->session_info;
-
- if (!conn->force_user && vuser == NULL) {
- DEBUG(2,("Invalid vuid used %llu in accessing share %s.\n",
- (unsigned long long)vuid,
- lp_servicename(talloc_tos(), snum)));
- return False;
- }
-
return change_to_user_internal(conn, session_info, vuid);
}
--
1.8.1
>From 70dcc28fa6b0e41243cb671c175b876ae81955f0 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 11:55:09 -0800
Subject: [PATCH 11/28] Remove unneeded variable "const struct
auth_session_info *session_info"
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 298fa83..9244f29 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -271,7 +271,6 @@ static bool change_to_user_internal(connection_struct *conn,
bool change_to_user(connection_struct *conn, uint64_t vuid)
{
- const struct auth_session_info *session_info = NULL;
struct user_struct *vuser;
int snum = SNUM(conn);
@@ -298,8 +297,7 @@ bool change_to_user(connection_struct *conn, uint64_t vuid)
return false;
}
- session_info = vuser->session_info;
- return change_to_user_internal(conn, session_info, vuid);
+ return change_to_user_internal(conn, vuser->session_info, vuid);
}
static bool change_to_user_by_session(connection_struct *conn,
--
1.8.1
>From 816a82cc9e8cb958b547e7bacfd672b25dd7efc0 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 20 Dec 2012 14:42:55 -0800
Subject: [PATCH 12/28] Clean up struct connection_struct, make struct
vuid_cache a pointer not inline.
Change VFS ABI to 31 for 4.1.0.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/include/vfs.h | 8 ++++++--
source3/modules/vfs_readonly.c | 4 ++--
source3/smbd/conn.c | 3 ++-
source3/smbd/uid.c | 10 +++++-----
4 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 2992c1d..7da022e 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -147,7 +147,11 @@
/* Bump to version 30 - Samba 4.0.0 will ship with interface version 30 */
/* Leave at 30 - not yet released. Added conn->cwd to save vfs_GetWd() calls. */
/* Leave at 30 - not yet released. Changed sys_acl_blob_get_file interface to remove type */
-#define SMB_VFS_INTERFACE_VERSION 30
+/* Bump to version 31 - Samba 4.1.0 will ship with interface version 31 */
+/* Leave at 31 - not yet released. Make struct vuid_cache_entry in
+ connection_struct a pointer. */
+
+#define SMB_VFS_INTERFACE_VERSION 31
/*
All intercepted VFS operations must be declared as static functions inside module source
@@ -306,7 +310,7 @@ typedef struct connection_struct {
uint32_t cnum; /* an index passed over the wire */
struct share_params *params;
bool force_user;
- struct vuid_cache vuid_cache;
+ struct vuid_cache *vuid_cache;
bool printer;
bool ipc;
bool read_only; /* Attributes for the current user of the share. */
diff --git a/source3/modules/vfs_readonly.c b/source3/modules/vfs_readonly.c
index 7919dbc..f75db09 100644
--- a/source3/modules/vfs_readonly.c
+++ b/source3/modules/vfs_readonly.c
@@ -82,12 +82,12 @@ static int readonly_connect(vfs_handle_struct *handle,
/* Wipe out the VUID cache. */
for (i=0; i< VUID_CACHE_SIZE; i++) {
- struct vuid_cache_entry *ent = &conn->vuid_cache.array[i];
+ struct vuid_cache_entry *ent = &conn->vuid_cache->array[i];
ent->vuid = UID_FIELD_INVALID;
TALLOC_FREE(ent->session_info);
ent->read_only = false;
}
- conn->vuid_cache.next_entry = 0;
+ conn->vuid_cache->next_entry = 0;
}
return 0;
diff --git a/source3/smbd/conn.c b/source3/smbd/conn.c
index bc5a03b..1d4444f 100644
--- a/source3/smbd/conn.c
+++ b/source3/smbd/conn.c
@@ -63,6 +63,7 @@ connection_struct *conn_new(struct smbd_server_connection *sconn)
if (!(conn=talloc_zero(NULL, connection_struct)) ||
!(conn->params = talloc(conn, struct share_params)) ||
+ !(conn->vuid_cache = talloc_zero(conn, struct vuid_cache)) ||
!(conn->connectpath = talloc_strdup(conn, "")) ||
!(conn->origpath = talloc_strdup(conn, ""))) {
DEBUG(0,("TALLOC_ZERO() failed!\n"));
@@ -89,7 +90,7 @@ static void conn_clear_vuid_cache(connection_struct *conn, uint64_t vuid)
for (i=0; i<VUID_CACHE_SIZE; i++) {
struct vuid_cache_entry *ent;
- ent = &conn->vuid_cache.array[i];
+ ent = &conn->vuid_cache->array[i];
if (ent->vuid == vuid) {
ent->vuid = UID_FIELD_INVALID;
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9244f29..f9b5716 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -68,7 +68,7 @@ static void free_conn_session_info_if_unused(connection_struct *conn)
for (i = 0; i < VUID_CACHE_SIZE; i++) {
struct vuid_cache_entry *ent;
- ent = &conn->vuid_cache.array[i];
+ ent = &conn->vuid_cache->array[i];
if (ent->vuid != UID_FIELD_INVALID &&
conn->session_info == ent->session_info) {
return;
@@ -96,7 +96,7 @@ static bool check_user_ok(connection_struct *conn,
struct vuid_cache_entry *ent = NULL;
for (i=0; i<VUID_CACHE_SIZE; i++) {
- ent = &conn->vuid_cache.array[i];
+ ent = &conn->vuid_cache->array[i];
if (ent->vuid == vuid) {
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
@@ -141,10 +141,10 @@ static bool check_user_ok(connection_struct *conn,
session_info->info->domain_name,
NULL, session_info->security_token, lp_admin_users(snum));
- ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry];
+ ent = &conn->vuid_cache->array[conn->vuid_cache->next_entry];
- conn->vuid_cache.next_entry =
- (conn->vuid_cache.next_entry + 1) % VUID_CACHE_SIZE;
+ conn->vuid_cache->next_entry =
+ (conn->vuid_cache->next_entry + 1) % VUID_CACHE_SIZE;
TALLOC_FREE(ent->session_info);
--
1.8.1
>From 6b95685b23d419f073d4bb5a690c686bb3196d9a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Dec 2012 09:22:16 -0800
Subject: [PATCH 13/28] Add uint32_t share_access to vuid_cache_entry.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/include/vfs.h | 2 ++
source3/modules/vfs_readonly.c | 1 +
source3/smbd/conn.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 7da022e..2bce1b7 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -150,6 +150,7 @@
/* Bump to version 31 - Samba 4.1.0 will ship with interface version 31 */
/* Leave at 31 - not yet released. Make struct vuid_cache_entry in
connection_struct a pointer. */
+/* Leave at 31 - not yet released. Add share_access to vuid_cache_entry. */
#define SMB_VFS_INTERFACE_VERSION 31
@@ -279,6 +280,7 @@ struct vuid_cache_entry {
struct auth_session_info *session_info;
uint64_t vuid; /* SMB2 compat */
bool read_only;
+ uint32_t share_access;
};
struct vuid_cache {
diff --git a/source3/modules/vfs_readonly.c b/source3/modules/vfs_readonly.c
index f75db09..445f947 100644
--- a/source3/modules/vfs_readonly.c
+++ b/source3/modules/vfs_readonly.c
@@ -86,6 +86,7 @@ static int readonly_connect(vfs_handle_struct *handle,
ent->vuid = UID_FIELD_INVALID;
TALLOC_FREE(ent->session_info);
ent->read_only = false;
+ ent->share_access = 0;
}
conn->vuid_cache->next_entry = 0;
}
diff --git a/source3/smbd/conn.c b/source3/smbd/conn.c
index 1d4444f..8f472c0 100644
--- a/source3/smbd/conn.c
+++ b/source3/smbd/conn.c
@@ -118,6 +118,7 @@ static void conn_clear_vuid_cache(connection_struct *conn, uint64_t vuid)
TALLOC_FREE(ent->session_info);
}
ent->read_only = False;
+ ent->share_access = 0;
}
}
}
--
1.8.1
>From 07dc0c184ff8da3817614a32d523e64d7489f596 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Dec 2012 09:35:31 -0800
Subject: [PATCH 14/28] Remove static from create_share_access_mask().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 1 +
source3/smbd/service.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index d7bfa65..5eb899f 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,6 +979,7 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
+void create_share_access_mask(connection_struct *conn, int snum);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 2214ac0..828c036 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -515,7 +515,7 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
Setup the share access mask for a connection.
****************************************************************************/
-static void create_share_access_mask(connection_struct *conn, int snum)
+void create_share_access_mask(connection_struct *conn, int snum)
{
const struct security_token *token = conn->session_info->security_token;
--
1.8.1
>From d16171102e4edee726cc07e78deb8a1b6c541c9a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 21 Dec 2012 09:45:03 -0800
Subject: [PATCH 15/28] Fix API for create_share_access_mask().
Return the uint32_t share_access rather than directly
changing the conn struct.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 2 +-
source3/smbd/service.c | 19 +++++++++++--------
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 5eb899f..abc6b28 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,7 +979,7 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
-void create_share_access_mask(connection_struct *conn, int snum);
+uint32_t create_share_access_mask(connection_struct *conn, int snum);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 828c036..1cd12a6 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -515,34 +515,37 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
Setup the share access mask for a connection.
****************************************************************************/
-void create_share_access_mask(connection_struct *conn, int snum)
+uint32_t create_share_access_mask(connection_struct *conn, int snum)
{
const struct security_token *token = conn->session_info->security_token;
+ uint32_t share_access = 0;
share_access_check(token,
lp_servicename(talloc_tos(), snum),
MAXIMUM_ALLOWED_ACCESS,
- &conn->share_access);
+ &share_access);
if (!CAN_WRITE(conn)) {
- conn->share_access &=
+ share_access &=
~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA |
SEC_FILE_WRITE_EA | SEC_FILE_WRITE_ATTRIBUTE |
SEC_DIR_DELETE_CHILD );
}
if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) {
- conn->share_access |= SEC_FLAG_SYSTEM_SECURITY;
+ share_access |= SEC_FLAG_SYSTEM_SECURITY;
}
if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
- conn->share_access |= (SEC_RIGHTS_PRIV_RESTORE);
+ share_access |= (SEC_RIGHTS_PRIV_RESTORE);
}
if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
- conn->share_access |= (SEC_RIGHTS_PRIV_BACKUP);
+ share_access |= (SEC_RIGHTS_PRIV_BACKUP);
}
if (security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
- conn->share_access |= (SEC_STD_WRITE_OWNER);
+ share_access |= (SEC_STD_WRITE_OWNER);
}
+
+ return share_access;
}
/****************************************************************************
@@ -654,7 +657,7 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
*
*/
- create_share_access_mask(conn, snum);
+ conn->share_access = create_share_access_mask(conn, snum);
if ((conn->share_access & FILE_WRITE_DATA) == 0) {
if ((conn->share_access & FILE_READ_DATA) == 0) {
--
1.8.1
>From 74670d7f35cc5b3139092ef2452353986334fef9 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 11:43:10 -0800
Subject: [PATCH 16/28] Change API for create_share_access_mask() to pass in
the token.
Don't automatically use the one from conn->session_info->security_token.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 4 +++-
source3/smbd/service.c | 9 ++++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index abc6b28..bfaaf6d 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,7 +979,9 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
-uint32_t create_share_access_mask(connection_struct *conn, int snum);
+uint32_t create_share_access_mask(connection_struct *conn,
+ int snum,
+ const struct security_token *token);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 1cd12a6..3e1d87f 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -515,9 +515,10 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
Setup the share access mask for a connection.
****************************************************************************/
-uint32_t create_share_access_mask(connection_struct *conn, int snum)
+uint32_t create_share_access_mask(connection_struct *conn,
+ int snum,
+ const struct security_token *token)
{
- const struct security_token *token = conn->session_info->security_token;
uint32_t share_access = 0;
share_access_check(token,
@@ -657,7 +658,9 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
*
*/
- conn->share_access = create_share_access_mask(conn, snum);
+ conn->share_access = create_share_access_mask(conn,
+ snum,
+ conn->session_info->security_token);
if ((conn->share_access & FILE_WRITE_DATA) == 0) {
if ((conn->share_access & FILE_READ_DATA) == 0) {
--
1.8.1
>From d1923eceba300b2c1e73d79e3cd3f9e432fa965a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:24:13 -0800
Subject: [PATCH 17/28] Change API for create_share_access_mask() - remove conn
struct.
Eventually this will be indepentent of conn, just pass in the
readonly flag.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 4 ++--
source3/smbd/service.c | 10 +++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index bfaaf6d..2198ccd 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,8 +979,8 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
-uint32_t create_share_access_mask(connection_struct *conn,
- int snum,
+uint32_t create_share_access_mask(int snum,
+ bool readonly_share,
const struct security_token *token);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 3e1d87f..10f4b53 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -515,8 +515,8 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
Setup the share access mask for a connection.
****************************************************************************/
-uint32_t create_share_access_mask(connection_struct *conn,
- int snum,
+uint32_t create_share_access_mask(int snum,
+ bool readonly_share,
const struct security_token *token)
{
uint32_t share_access = 0;
@@ -526,7 +526,7 @@ uint32_t create_share_access_mask(connection_struct *conn,
MAXIMUM_ALLOWED_ACCESS,
&share_access);
- if (!CAN_WRITE(conn)) {
+ if (readonly_share) {
share_access &=
~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA |
SEC_FILE_WRITE_EA | SEC_FILE_WRITE_ATTRIBUTE |
@@ -658,8 +658,8 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
*
*/
- conn->share_access = create_share_access_mask(conn,
- snum,
+ conn->share_access = create_share_access_mask(snum,
+ !CAN_WRITE(conn),
conn->session_info->security_token);
if ((conn->share_access & FILE_WRITE_DATA) == 0) {
--
1.8.1
>From 1770640c21c50009ca299e0017dd6ab7858a55a5 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:25:55 -0800
Subject: [PATCH 18/28] Correctly setup the conn->share_access based on the
current user token.
Also use this to set conn->read_only. Cache the share_access in the
struct vuid_cache_entry struct so we only evaluate this once per new
user access on this share.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index f9b5716..397380c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -94,6 +94,7 @@ static bool check_user_ok(connection_struct *conn,
bool readonly_share;
bool admin_user;
struct vuid_cache_entry *ent = NULL;
+ uint32_t share_access = 0;
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache->array[i];
@@ -101,6 +102,7 @@ static bool check_user_ok(connection_struct *conn,
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
conn->read_only = ent->read_only;
+ conn->share_access = ent->share_access;
return(True);
}
}
@@ -116,11 +118,24 @@ static bool check_user_ok(connection_struct *conn,
session_info->security_token,
conn);
+ share_access = create_share_access_mask(snum,
+ readonly_share,
+ session_info->security_token);
+
+ if ((share_access & FILE_WRITE_DATA) == 0) {
+ if ((share_access & FILE_READ_DATA) == 0) {
+ /* No access, read or write. */
+ DEBUG(0,("user %s connection to %s "
+ "denied due to share security "
+ "descriptor.\n",
+ session_info->unix_info->unix_name,
+ lp_servicename(talloc_tos(), snum)));
+ return false;
+ }
+ }
+
if (!readonly_share &&
- !share_access_check(session_info->security_token,
- lp_servicename(talloc_tos(), snum),
- FILE_WRITE_DATA,
- NULL)) {
+ !(share_access & FILE_WRITE_DATA)) {
/* smb.conf allows r/w, but the security descriptor denies
* write. Fall back to looking at readonly. */
readonly_share = True;
@@ -128,14 +143,6 @@ static bool check_user_ok(connection_struct *conn,
"security descriptor\n"));
}
- if (!share_access_check(session_info->security_token,
- lp_servicename(talloc_tos(), snum),
- readonly_share ?
- FILE_READ_DATA : FILE_WRITE_DATA,
- NULL)) {
- return False;
- }
-
admin_user = token_contains_name_in_list(
session_info->unix_info->unix_name,
session_info->info->domain_name,
@@ -163,10 +170,13 @@ static bool check_user_ok(connection_struct *conn,
ent->vuid = vuid;
ent->read_only = readonly_share;
+ ent->share_access = share_access;
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
conn->read_only = readonly_share;
+ conn->share_access = share_access;
+
if (admin_user) {
DEBUG(2,("check_user_ok: user %s is an admin user. "
"Setting uid as %d\n",
--
1.8.1
>From 7fbf97883f5e6d7a085dec42b331334063a7dbdf Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:27:18 -0800
Subject: [PATCH 19/28] Add check_user_share_access() which factors out the
share security and read_only flag setting code.
Allows this to be called from both make_connection_snum() as well as check_user_ok().
Gives a consistent share security check function.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 4 ++++
source3/smbd/uid.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 2198ccd..e8ee873 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1099,6 +1099,10 @@ void reply_transs2(struct smb_request *req);
/* The following definitions come from smbd/uid.c */
bool change_to_guest(void);
+NTSTATUS check_user_share_access(connection_struct *conn,
+ const struct auth_session_info *session_info,
+ uint32_t *p_share_access,
+ bool *p_readonly_share);
bool change_to_user(connection_struct *conn, uint64_t vuid);
bool change_to_root_user(void);
bool smbd_change_to_root_user(void);
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 397380c..f551e50 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -79,6 +79,62 @@ static void free_conn_session_info_if_unused(connection_struct *conn)
}
/*******************************************************************
+ Calculate access mask and if this user can access this share.
+********************************************************************/
+
+NTSTATUS check_user_share_access(connection_struct *conn,
+ const struct auth_session_info *session_info,
+ uint32_t *p_share_access,
+ bool *p_readonly_share)
+{
+ int snum = SNUM(conn);
+ uint32_t share_access = 0;
+ bool readonly_share = false;
+
+ if (!user_ok_token(session_info->unix_info->unix_name,
+ session_info->info->domain_name,
+ session_info->security_token, snum)) {
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ readonly_share = is_share_read_only_for_token(
+ session_info->unix_info->unix_name,
+ session_info->info->domain_name,
+ session_info->security_token,
+ conn);
+
+ share_access = create_share_access_mask(snum,
+ readonly_share,
+ session_info->security_token);
+
+ if ((share_access & FILE_WRITE_DATA) == 0) {
+ if ((share_access & FILE_READ_DATA) == 0) {
+ /* No access, read or write. */
+ DEBUG(0,("user %s connection to %s "
+ "denied due to share security "
+ "descriptor.\n",
+ session_info->unix_info->unix_name,
+ lp_servicename(talloc_tos(), snum)));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+ }
+
+ if (!readonly_share &&
+ !(share_access & FILE_WRITE_DATA)) {
+ /* smb.conf allows r/w, but the security descriptor denies
+ * write. Fall back to looking at readonly. */
+ readonly_share = True;
+ DEBUG(5,("falling back to read-only access-evaluation due to "
+ "security descriptor\n"));
+ }
+
+ *p_share_access = share_access;
+ *p_readonly_share = readonly_share;
+
+ return NT_STATUS_OK;
+}
+
+/*******************************************************************
Check if a username is OK.
This sets up conn->session_info with a copy related to this vuser that
--
1.8.1
>From 8d2f840bb2f0ea59abb5bdc40f6f815f5418c048 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Thu, 3 Jan 2013 16:06:40 -0800
Subject: [PATCH 20/28] Initialize stack variables. Prelude to factoring out
calls to check_user_share_access().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index f551e50..9802056 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -147,8 +147,8 @@ static bool check_user_ok(connection_struct *conn,
int snum)
{
unsigned int i;
- bool readonly_share;
- bool admin_user;
+ bool readonly_share = false;
+ bool admin_user = false;
struct vuid_cache_entry *ent = NULL;
uint32_t share_access = 0;
--
1.8.1
>From a9b9941882ac9c8a211f710c0fcc28e891e6c6f9 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:35:46 -0800
Subject: [PATCH 21/28] Factor code out of check_user_ok() into a call to
check_user_share_access().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 41 +++++++----------------------------------
1 file changed, 7 insertions(+), 34 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 9802056..efdd824 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -151,6 +151,7 @@ static bool check_user_ok(connection_struct *conn,
bool admin_user = false;
struct vuid_cache_entry *ent = NULL;
uint32_t share_access = 0;
+ NTSTATUS status;
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache->array[i];
@@ -163,40 +164,12 @@ static bool check_user_ok(connection_struct *conn,
}
}
- if (!user_ok_token(session_info->unix_info->unix_name,
- session_info->info->domain_name,
- session_info->security_token, snum))
- return(False);
-
- readonly_share = is_share_read_only_for_token(
- session_info->unix_info->unix_name,
- session_info->info->domain_name,
- session_info->security_token,
- conn);
-
- share_access = create_share_access_mask(snum,
- readonly_share,
- session_info->security_token);
-
- if ((share_access & FILE_WRITE_DATA) == 0) {
- if ((share_access & FILE_READ_DATA) == 0) {
- /* No access, read or write. */
- DEBUG(0,("user %s connection to %s "
- "denied due to share security "
- "descriptor.\n",
- session_info->unix_info->unix_name,
- lp_servicename(talloc_tos(), snum)));
- return false;
- }
- }
-
- if (!readonly_share &&
- !(share_access & FILE_WRITE_DATA)) {
- /* smb.conf allows r/w, but the security descriptor denies
- * write. Fall back to looking at readonly. */
- readonly_share = True;
- DEBUG(5,("falling back to read-only access-evaluation due to "
- "security descriptor\n"));
+ status = check_user_share_access(conn,
+ session_info,
+ &share_access,
+ &readonly_share);
+ if (!NT_STATUS_IS_OK(status)) {
+ return false;
}
admin_user = token_contains_name_in_list(
--
1.8.1
>From 38a23484a3aa4c8ad60e40851098149508389d04 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:40:05 -0800
Subject: [PATCH 22/28] Fix bug #9518 - conn->share_access appears not be be
reset between users.
Ensure make_connection_snum() uses the same logic as
check_user_ok() to decide if a user can access a share.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/service.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 10f4b53..fabc5a3 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -652,29 +652,17 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
}
/*
- * New code to check if there's a share security descripter
- * added from NT server manager. This is done after the
- * smb.conf checks are done as we need a uid and token. JRA.
- *
+ * Set up the share security descripter
*/
- conn->share_access = create_share_access_mask(snum,
- !CAN_WRITE(conn),
- conn->session_info->security_token);
-
- if ((conn->share_access & FILE_WRITE_DATA) == 0) {
- if ((conn->share_access & FILE_READ_DATA) == 0) {
- /* No access, read or write. */
- DEBUG(0,("make_connection: connection to %s "
- "denied due to security "
- "descriptor.\n",
- lp_servicename(talloc_tos(), snum)));
- status = NT_STATUS_ACCESS_DENIED;
- goto err_root_exit;
- } else {
- conn->read_only = True;
- }
+ status = check_user_share_access(conn,
+ conn->session_info,
+ &conn->share_access,
+ &conn->read_only);
+ if (!NT_STATUS_IS_OK(status)) {
+ goto err_root_exit;
}
+
/* Initialise VFS function pointers */
if (!smbd_vfs_init(conn)) {
--
1.8.1
>From d45066072b62458c61386aaf9552ae729771e8fb Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 4 Jan 2013 14:42:23 -0800
Subject: [PATCH 23/28] Move create_share_access_mask() from smbd/service.c to
smbd/uid.c.
Make it static. Only called from uid.c now.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/proto.h | 3 ---
source3/smbd/service.c | 38 --------------------------------------
source3/smbd/uid.c | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 41 deletions(-)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index e8ee873..7727302 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -979,9 +979,6 @@ void smbd_exit_server_cleanly(const char *const reason) _NORETURN_;
bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
-uint32_t create_share_access_mask(int snum,
- bool readonly_share,
- const struct security_token *token);
bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
void load_registry_shares(void);
int add_home_service(const char *service, const char *username, const char *homedir);
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index fabc5a3..8f6d485 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -512,44 +512,6 @@ NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
}
/****************************************************************************
- Setup the share access mask for a connection.
-****************************************************************************/
-
-uint32_t create_share_access_mask(int snum,
- bool readonly_share,
- const struct security_token *token)
-{
- uint32_t share_access = 0;
-
- share_access_check(token,
- lp_servicename(talloc_tos(), snum),
- MAXIMUM_ALLOWED_ACCESS,
- &share_access);
-
- if (readonly_share) {
- share_access &=
- ~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA |
- SEC_FILE_WRITE_EA | SEC_FILE_WRITE_ATTRIBUTE |
- SEC_DIR_DELETE_CHILD );
- }
-
- if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) {
- share_access |= SEC_FLAG_SYSTEM_SECURITY;
- }
- if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
- share_access |= (SEC_RIGHTS_PRIV_RESTORE);
- }
- if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
- share_access |= (SEC_RIGHTS_PRIV_BACKUP);
- }
- if (security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
- share_access |= (SEC_STD_WRITE_OWNER);
- }
-
- return share_access;
-}
-
-/****************************************************************************
Make a connection, given the snum to connect to, and the vuser of the
connecting user if appropriate.
****************************************************************************/
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index efdd824..b9cebce 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -78,6 +78,44 @@ static void free_conn_session_info_if_unused(connection_struct *conn)
TALLOC_FREE(conn->session_info);
}
+/****************************************************************************
+ Setup the share access mask for a connection.
+****************************************************************************/
+
+static uint32_t create_share_access_mask(int snum,
+ bool readonly_share,
+ const struct security_token *token)
+{
+ uint32_t share_access = 0;
+
+ share_access_check(token,
+ lp_servicename(talloc_tos(), snum),
+ MAXIMUM_ALLOWED_ACCESS,
+ &share_access);
+
+ if (readonly_share) {
+ share_access &=
+ ~(SEC_FILE_WRITE_DATA | SEC_FILE_APPEND_DATA |
+ SEC_FILE_WRITE_EA | SEC_FILE_WRITE_ATTRIBUTE |
+ SEC_DIR_DELETE_CHILD );
+ }
+
+ if (security_token_has_privilege(token, SEC_PRIV_SECURITY)) {
+ share_access |= SEC_FLAG_SYSTEM_SECURITY;
+ }
+ if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
+ share_access |= (SEC_RIGHTS_PRIV_RESTORE);
+ }
+ if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
+ share_access |= (SEC_RIGHTS_PRIV_BACKUP);
+ }
+ if (security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
+ share_access |= (SEC_STD_WRITE_OWNER);
+ }
+
+ return share_access;
+}
+
/*******************************************************************
Calculate access mask and if this user can access this share.
********************************************************************/
--
1.8.1
>From 7dd6fd5a3a5ba6a925c240e9834a7820413d180e Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 6 Jan 2013 14:39:07 +0100
Subject: [PATCH 24/28] smbd: Fix a typo
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/service.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 8f6d485..4fe3809 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -614,7 +614,7 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn,
}
/*
- * Set up the share security descripter
+ * Set up the share security descriptor
*/
status = check_user_share_access(conn,
--
1.8.1
>From aaa0b582b4f9cd8deb13edaf7aa30f9741578759 Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 6 Jan 2013 14:41:24 +0100
Subject: [PATCH 25/28] smbd: Simplify an if-expression
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index b9cebce..5ab0dc4 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -145,16 +145,13 @@ NTSTATUS check_user_share_access(connection_struct *conn,
readonly_share,
session_info->security_token);
- if ((share_access & FILE_WRITE_DATA) == 0) {
- if ((share_access & FILE_READ_DATA) == 0) {
- /* No access, read or write. */
- DEBUG(0,("user %s connection to %s "
- "denied due to share security "
- "descriptor.\n",
- session_info->unix_info->unix_name,
- lp_servicename(talloc_tos(), snum)));
- return NT_STATUS_ACCESS_DENIED;
- }
+ if ((share_access & (FILE_READ_DATA|FILE_WRITE_DATA)) == 0) {
+ /* No access, read or write. */
+ DEBUG(0,("user %s connection to %s denied due to share "
+ "security descriptor.\n",
+ session_info->unix_info->unix_name,
+ lp_servicename(talloc_tos(), snum)));
+ return NT_STATUS_ACCESS_DENIED;
}
if (!readonly_share &&
--
1.8.1
>From 3edac379004e13fa45887fae75c658044f45d8af Mon Sep 17 00:00:00 2001
From: Volker Lendecke <vl at samba.org>
Date: Sun, 6 Jan 2013 14:50:33 +0100
Subject: [PATCH 26/28] smbd: Remove some ()
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index 5ab0dc4..b2fe39c 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -104,13 +104,13 @@ static uint32_t create_share_access_mask(int snum,
share_access |= SEC_FLAG_SYSTEM_SECURITY;
}
if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
- share_access |= (SEC_RIGHTS_PRIV_RESTORE);
+ share_access |= SEC_RIGHTS_PRIV_RESTORE;
}
if (security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
- share_access |= (SEC_RIGHTS_PRIV_BACKUP);
+ share_access |= SEC_RIGHTS_PRIV_BACKUP;
}
if (security_token_has_privilege(token, SEC_PRIV_TAKE_OWNERSHIP)) {
- share_access |= (SEC_STD_WRITE_OWNER);
+ share_access |= SEC_STD_WRITE_OWNER;
}
return share_access;
--
1.8.1
>From b692463874e0b309bc19c1d7a864780057fbb223 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 8 Jan 2013 11:02:16 -0800
Subject: [PATCH 27/28] Fixup the change_to_user_by_session() case as called
from become_user_by_session()
Use inside source3/printing/nt_printing.c:get_correct_cversion().
Allow check_user_ok() to be called with vuid==UID_FIELD_INVALID.
All this should do is throw away one entry in the vuid cache.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/uid.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index b2fe39c..a795eef 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -191,6 +191,13 @@ static bool check_user_ok(connection_struct *conn,
for (i=0; i<VUID_CACHE_SIZE; i++) {
ent = &conn->vuid_cache->array[i];
if (ent->vuid == vuid) {
+ if (vuid == UID_FIELD_INVALID) {
+ /*
+ * Slow path, we don't care
+ * about the array traversal.
+ */
+ continue;
+ }
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
conn->read_only = ent->read_only;
@@ -232,11 +239,26 @@ static bool check_user_ok(connection_struct *conn,
return false;
}
+ /*
+ * It's actually OK to call check_user_ok() with
+ * vuid == UID_FIELD_INVALID as called from change_to_user_by_session().
+ * All this will do is throw away one entry in the cache.
+ */
+
ent->vuid = vuid;
ent->read_only = readonly_share;
ent->share_access = share_access;
free_conn_session_info_if_unused(conn);
conn->session_info = ent->session_info;
+ if (vuid == UID_FIELD_INVALID) {
+ /*
+ * Not strictly needed, just make it really
+ * clear this entry is actually an unused one.
+ */
+ ent->read_only = false;
+ ent->share_access = 0;
+ ent->session_info = NULL;
+ }
conn->read_only = readonly_share;
conn->share_access = share_access;
--
1.8.1
>From 066471e506050331aece6399b59608c86f221d1f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 20 Dec 2012 23:05:55 +1100
Subject: [PATCH 28/28] selftest: show that Samba honours "write list" and
valid users
Reviewed-by: Jeremy Allison <jra at samba.org>
---
selftest/target/Samba3.pm | 7 +++++++
source3/script/tests/test_smbclient_machine_auth.sh | 4 ++++
source3/script/tests/test_smbclient_s3.sh | 8 ++++++--
source3/selftest/tests.py | 5 +++++
4 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index adca52f..8546bd2 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -963,6 +963,13 @@ sub provision($$$$$$)
[ro-tmp]
path = $ro_shrdir
guest ok = yes
+[write-list-tmp]
+ path = $shrdir
+ read only = yes
+ write list = $unix_name
+[valid-users-tmp]
+ path = $shrdir
+ valid users = $unix_name
[msdfs-share]
path = $msdfs_shrdir
msdfs root = yes
diff --git a/source3/script/tests/test_smbclient_machine_auth.sh b/source3/script/tests/test_smbclient_machine_auth.sh
index f67256d..a890d48 100755
--- a/source3/script/tests/test_smbclient_machine_auth.sh
+++ b/source3/script/tests/test_smbclient_machine_auth.sh
@@ -19,3 +19,7 @@ incdir=`dirname $0`/../../../testprogs/blackbox
. $incdir/subunit.sh
testit "smbclient //$SERVER/tmp" $SMBCLIENT //$SERVER/tmp --machine-pass -I $SERVER_IP -p 139 -c quit $ADDARGS
+
+# Testing these here helps because we know the machine account isn't already this user/group
+testit "smbclient //$SERVER/forceuser" $SMBCLIENT //$SERVER/tmp --machine-pass -I $SERVER_IP -p 139 -c quit $ADDARGS
+testit "smbclient //$SERVER/forcegroup" $SMBCLIENT //$SERVER/tmp --machine-pass -I $SERVER_IP -p 139 -c quit $ADDARGS
diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh
index fb518c5..b240da0 100755
--- a/source3/script/tests/test_smbclient_s3.sh
+++ b/source3/script/tests/test_smbclient_s3.sh
@@ -212,7 +212,7 @@ mkdir a_test_dir
quit
EOF
- cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U% //$SERVER/ro-tmp -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
+ cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT -U% //$SERVER/$1" -I $SERVER_IP $ADDARGS < $tmpfile 2>&1'
eval echo "$cmd"
out=`eval $cmd`
ret=$?
@@ -581,7 +581,11 @@ testit "creating a good symlink and deleting it by path" \
failed=`expr $failed + 1`
testit "writing into a read-only directory fails" \
- test_read_only_dir || \
+ test_read_only_dir ro-tmp || \
+ failed=`expr $failed + 1`
+
+testit "writing into a read-only share fails" \
+ test_read_only_dir valid-users-tmp || \
failed=`expr $failed + 1`
testit "Reading a owner-only file fails" \
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 57a67ed..44efe18 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -333,6 +333,11 @@ for t in tests:
elif t == "smb2.durable-open" or t == "smb2.durable-v2-open":
plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/durable -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER_IP/durable -U$USERNAME%$PASSWORD')
+ elif t == "base.rw1":
+ plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
+ plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/valid-users-tmp -U$USERNAME%$PASSWORD')
+ plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/write-list-tmp -U$USERNAME%$PASSWORD')
+ plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
else:
plansmbtorture4testsuite(t, "s3dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "plugin_s4_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
--
1.8.1
More information about the samba-technical
mailing list