From f81c90ce0cf5becf17b711c9297f19d2ffc54938 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:50:25 -0800 Subject: [PATCH 01/19] 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 (cherry picked from commit f0450e0d80c2ff56c4834b2f1271a7f84132ca5b) --- source3/smbd/uid.c | 5 ++--- 1 files 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; iinfo->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.7.7.3 From bd98c66e49f3c3c3316cf60fb1a3fe9d470cb35d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:51:55 -0800 Subject: [PATCH 02/19] Move the definition of struct vuid_cache_entry *ent outside blocks. Signed-off-by: Jeremy Allison (cherry picked from commit 092c9517acf5a4b11577ef7b5f1d645e5e463f6d) --- source3/smbd/uid.c | 6 ++---- 1 files 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; ivuid_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.7.7.3 From e9b92399e6257732262651c2746a435eb17ce533 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:52:27 -0800 Subject: [PATCH 03/19] Remove one set of enclosing {} braces, no longer needed. Signed-off-by: Jeremy Allison (cherry picked from commit d64ea67c78a5b09559971ff6953cd67feb2b1ec2) --- source3/smbd/uid.c | 16 +++++++--------- 1 files 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; ivuid_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; ivuid_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.7.7.3 From 762978790846fdab49389040e508f39fe7ec6550 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:53:11 -0800 Subject: [PATCH 04/19] Remove the second set of {} braces, no longer needed. (cherry picked from commit ed0a34d163f777b2a0d4a2b358b7fb1b170d7686) --- source3/smbd/uid.c | 38 ++++++++++++++++++-------------------- 1 files 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.7.7.3 From d030807f2ad54bc3def71b4c8e60de41ab6268d4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:54:07 -0800 Subject: [PATCH 05/19] Remove dead code now vuser can no longer be NULL. Signed-off-by: Jeremy Allison (cherry picked from commit d35ba04e25eb3c396f791ea80c0ebb74543d4005) --- source3/smbd/uid.c | 8 -------- 1 files changed, 0 insertions(+), 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.7.7.3 From 4f7f3d695103877fd9b3f4b520682f8a34253116 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:55:09 -0800 Subject: [PATCH 06/19] Remove unneeded variable "const struct auth_session_info *session_info" Signed-off-by: Jeremy Allison (cherry picked from commit 5a3cda176f5eecd65b289c74132b0126357c5ef0) --- source3/smbd/uid.c | 4 +--- 1 files changed, 1 insertions(+), 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.7.7.3 From 59d4a324a5204f9375bf095879190124c6c15fa9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Dec 2012 09:35:31 -0800 Subject: [PATCH 07/19] Remove static from create_share_access_mask(). Signed-off-by: Jeremy Allison (cherry picked from commit 48187220ff47efe70616361fcef1a794aef765b4) --- source3/smbd/proto.h | 1 + source3/smbd/service.c | 2 +- 2 files changed, 2 insertions(+), 1 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index b48cf44..e004980 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -962,6 +962,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.7.7.3 From 65fb650826b96f89c68def0a4b8e6e3d28632808 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Dec 2012 09:45:03 -0800 Subject: [PATCH 08/19] 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 (cherry picked from commit 33167c070c085b30569317666a3fca079d970321) --- 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 e004980..97ae1db 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -962,7 +962,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.7.7.3 From a94baa9dc11286b1b7956aeb6295bbbc649ab7ab Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Jan 2013 12:01:17 -0800 Subject: [PATCH 09/19] 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 --- 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 97ae1db..f0d2c93 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -962,7 +962,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.7.7.3 From e04f8229dd69f53dba53d8d28b420de8551c279f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Jan 2013 15:04:26 -0800 Subject: [PATCH 10/19] 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 --- 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 f0d2c93..4fdcd26 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -962,8 +962,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.7.7.3 From ea0942b969d3f57af8aac9f5b5380258edddb0a2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 11 Jan 2013 10:47:56 -0800 Subject: [PATCH 11/19] Add parallel cache for share_access entries, one per connection struct. Needed as we cannot change the VFS ABI for 4.0.x, but need to add the equivalent of 'uint32_t share_access' to the struct vuid_cache referenced in connection_struct. Exports 2 accessor functions - lifetime managed by talloc on the conn struct list. Signed-off-by: Jeremy Allison --- source3/smbd/conn.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++- source3/smbd/proto.h | 5 +++ 2 files changed, 89 insertions(+), 1 deletions(-) diff --git a/source3/smbd/conn.c b/source3/smbd/conn.c index bc5a03b..e6f81a9 100644 --- a/source3/smbd/conn.c +++ b/source3/smbd/conn.c @@ -24,6 +24,84 @@ #include "smbd/globals.h" #include "lib/util/bitmap.h" +/******************************************************************* + Static cache for storing per-user share access value. This really + belongs inside the vuid_cache.array struct but we can't change the + VFS ABI for 4.0.x. This is fixed in 4.1.x. JRA. +********************************************************************/ + +struct connection_share_access_list { + struct connection_share_access_list *next, *prev; + connection_struct *conn; + uint32_t vuid_cache_share_access_array[VUID_CACHE_SIZE]; +}; + +static struct connection_share_access_list *conn_share_access_list; + +/******************************************************************* + Destructor function for per-user share access value. +********************************************************************/ + +static int free_csal_entry(struct connection_share_access_list *csal) +{ + DLIST_REMOVE(conn_share_access_list, csal); + return 0; +} + +/******************************************************************* + Utility function to find a per-user share access value struct. +********************************************************************/ + +static struct connection_share_access_list *find_csal_entry(connection_struct *conn) +{ + struct connection_share_access_list *csal; + + for (csal = conn_share_access_list; csal; csal = csal->next) { + if (csal->conn == conn) { + DLIST_PROMOTE(conn_share_access_list, csal); + return csal; + } + } + return NULL; +} + +/******************************************************************* + Accessor functions for per-user share access value. + These are the only two functions exposed externally. +********************************************************************/ + +uint32_t get_connection_share_access_list_entry(connection_struct *conn, + unsigned int i) +{ + struct connection_share_access_list *csal = + find_csal_entry(conn); + + if (csal == NULL) { + /* + * This is a faked up connection struct + * for internal purposes. + * Return full access. + */ + return SEC_RIGHTS_FILE_ALL; + } + + return csal->vuid_cache_share_access_array[i]; +} + +void set_connection_share_access_list_entry(connection_struct *conn, + unsigned int i, + uint32_t val) +{ + struct connection_share_access_list *csal = + find_csal_entry(conn); + + if (csal == NULL) { + return; + } + + csal->vuid_cache_share_access_array[i] = val; +} + /**************************************************************************** Return the number of open connections. ****************************************************************************/ @@ -60,19 +138,24 @@ bool conn_snum_used(struct smbd_server_connection *sconn, connection_struct *conn_new(struct smbd_server_connection *sconn) { connection_struct *conn; + struct connection_share_access_list *csal; if (!(conn=talloc_zero(NULL, connection_struct)) || !(conn->params = talloc(conn, struct share_params)) || !(conn->connectpath = talloc_strdup(conn, "")) || - !(conn->origpath = talloc_strdup(conn, ""))) { + !(conn->origpath = talloc_strdup(conn, "")) || + !(csal = talloc_zero(conn, struct connection_share_access_list))) { DEBUG(0,("TALLOC_ZERO() failed!\n")); TALLOC_FREE(conn); return NULL; } + talloc_set_destructor(csal, free_csal_entry); + conn->sconn = sconn; conn->force_group_gid = (gid_t)-1; DLIST_ADD(sconn->connections, conn); + DLIST_ADD(conn_share_access_list, csal); sconn->num_connections++; return conn; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 4fdcd26..6cf915a 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -145,6 +145,11 @@ bool recursive_rmdir(TALLOC_CTX *ctx, /* The following definitions come from smbd/conn.c */ +uint32_t get_connection_share_access_list_entry(connection_struct *conn, + unsigned int i); +void set_connection_share_access_list_entry(connection_struct *conn, + unsigned int i, + uint32_t val); int conn_num_open(struct smbd_server_connection *sconn); bool conn_snum_used(struct smbd_server_connection *sconn, int snum); connection_struct *conn_new(struct smbd_server_connection *sconn); -- 1.7.7.3 From e9576675efb2ffba1db9491f4ac5c30f8e910701 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Jan 2013 15:06:35 -0800 Subject: [PATCH 12/19] Correctly setup the conn->share_access based on the current user token. Also use this to set conn->read_only. Cache the share_access so we only evaluate this once per new user access on this share. Signed-off-by: Jeremy Allison --- source3/smbd/uid.c | 40 ++++++++++++++++++++++++++++------------ 1 files changed, 28 insertions(+), 12 deletions(-) diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index 9244f29..bed0b54 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -94,6 +94,8 @@ 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; + unsigned int share_array_index; for (i=0; ivuid_cache.array[i]; @@ -101,6 +103,9 @@ 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 = get_connection_share_access_list_entry( + conn, + i); return(True); } } @@ -116,11 +121,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,19 +146,12 @@ 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, NULL, session_info->security_token, lp_admin_users(snum)); + share_array_index = conn->vuid_cache.next_entry; ent = &conn->vuid_cache.array[conn->vuid_cache.next_entry]; conn->vuid_cache.next_entry = @@ -163,10 +174,15 @@ static bool check_user_ok(connection_struct *conn, ent->vuid = vuid; ent->read_only = readonly_share; + set_connection_share_access_list_entry(conn, + share_array_index, + 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.7.7.3 From ed4aba7d5a18c19fd6a1ec4cd5d5fe529d6355db Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 11 Jan 2013 11:01:25 -0800 Subject: [PATCH 13/19] Add check_user_share_access() This factors out the share security and read_only flag setting code so this can 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 --- source3/smbd/proto.h | 4 +++ source3/smbd/uid.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 0 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 6cf915a..f9cf5fb 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1087,6 +1087,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 bed0b54..e60f108 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -79,6 +79,61 @@ 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.7.7.3 From b0df9e1c1b33329039482c7448a6a0ea40dcbd03 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Jan 2013 11:05:03 -0800 Subject: [PATCH 14/19] Initialize stack variables. Prelude to factoring out calls to check_user_share_access(). Signed-off-by: Jeremy Allison --- source3/smbd/uid.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index e60f108..3add64f 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -146,8 +146,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; unsigned int share_array_index; -- 1.7.7.3 From 06b5c98e47cdabf3fcef339ac9fcdd763fac4884 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Jan 2013 15:13:53 -0800 Subject: [PATCH 15/19] Factor code out of check_user_ok() into a call to check_user_share_access(). Signed-off-by: Jeremy Allison --- source3/smbd/uid.c | 40 +++++++--------------------------------- 1 files changed, 7 insertions(+), 33 deletions(-) diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index 3add64f..562867a 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -151,6 +151,7 @@ static bool check_user_ok(connection_struct *conn, struct vuid_cache_entry *ent = NULL; uint32_t share_access = 0; unsigned int share_array_index; + NTSTATUS status; for (i=0; ivuid_cache.array[i]; @@ -165,41 +166,14 @@ 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; - } + status = check_user_share_access(conn, + session_info, + &share_access, + &readonly_share); + if (!NT_STATUS_IS_OK(status)) { + 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")); - } admin_user = token_contains_name_in_list( session_info->unix_info->unix_name, -- 1.7.7.3 From 244a7c72f56951ca6bec80a28c4a857937d7a057 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 4 Jan 2013 15:15:59 -0800 Subject: [PATCH 16/19] 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 --- source3/smbd/service.c | 34 +++++++++++----------------------- 1 files changed, 11 insertions(+), 23 deletions(-) diff --git a/source3/smbd/service.c b/source3/smbd/service.c index 10f4b53..d3cda73 100644 --- a/source3/smbd/service.c +++ b/source3/smbd/service.c @@ -651,30 +651,18 @@ static NTSTATUS make_connection_snum(struct smbd_server_connection *sconn, TALLOC_FREE(s); } - /* - * 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. - * - */ - - 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; - } + /* + * Set up the share security descripter + */ + + 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.7.7.3 From a84e0cd0010e896f2a0a016a6d741e6c988faf41 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 11 Jan 2013 11:12:15 -0800 Subject: [PATCH 17/19] 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 --- 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 f9cf5fb..fae1407 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -967,9 +967,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 d3cda73..b3abdd8 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 562867a..a3d4f08 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.7.7.3 From 899cee47a3369381a4878e999c49c4d3847f972d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 11 Jan 2013 11:14:48 -0800 Subject: [PATCH 18/19] 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 --- source3/smbd/uid.c | 23 +++++++++++++++++++++++ 1 files changed, 23 insertions(+), 0 deletions(-) diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index a3d4f08..852ae39 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -194,6 +194,13 @@ static bool check_user_ok(connection_struct *conn, for (i=0; ivuid_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; @@ -239,6 +246,11 @@ 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; set_connection_share_access_list_entry(conn, @@ -246,6 +258,17 @@ static bool check_user_ok(connection_struct *conn, 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; + set_connection_share_access_list_entry(conn, + share_array_index, + 0); + ent->session_info = NULL; + } conn->read_only = readonly_share; conn->share_access = share_access; -- 1.7.7.3 From d819b21f6047e7392f2d160d5cc76f2c0b0de57c Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 20 Dec 2012 23:05:55 +1100 Subject: [PATCH 19/19] selftest: show that Samba honours "write list" and valid users Reviewed-by: Jeremy Allison Reviewed-by: Stefan Metzmacher --- selftest/target/Samba3.pm | 7 +++++++ .../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 2037a2e..6c63413 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -956,6 +956,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 3341c62..aa2ba77 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -211,7 +211,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=$? @@ -529,7 +529,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 9b0527c..57c80f2 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -330,6 +330,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.7.7.3