From f0450e0d80c2ff56c4834b2f1271a7f84132ca5b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:50:25 -0800 Subject: [PATCH 01/15] 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 --- 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 092c9517acf5a4b11577ef7b5f1d645e5e463f6d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:51:55 -0800 Subject: [PATCH 02/15] Move the definition of struct vuid_cache_entry *ent outside blocks. Signed-off-by: Jeremy Allison --- 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 d64ea67c78a5b09559971ff6953cd67feb2b1ec2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:52:27 -0800 Subject: [PATCH 03/15] Remove one set of enclosing {} braces, no longer needed. Signed-off-by: Jeremy Allison --- 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 ed0a34d163f777b2a0d4a2b358b7fb1b170d7686 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:53:11 -0800 Subject: [PATCH 04/15] Remove the second set of {} braces, no longer needed. --- 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 d35ba04e25eb3c396f791ea80c0ebb74543d4005 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:54:07 -0800 Subject: [PATCH 05/15] Remove dead code now vuser can no longer be NULL. Signed-off-by: Jeremy Allison --- 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 5a3cda176f5eecd65b289c74132b0126357c5ef0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 11:55:09 -0800 Subject: [PATCH 06/15] Remove unneeded variable "const struct auth_session_info *session_info" Signed-off-by: Jeremy Allison --- 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 b5bf2a0222f9de5044af8cc2dfd46ab35cd3483f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 20 Dec 2012 14:42:55 -0800 Subject: [PATCH 07/15] 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 --- 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; ivuid_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; ivuid_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.7.7.3 From fc5643f50fa2422e4e4d75c39cf45b9bb33d3f68 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Dec 2012 09:22:16 -0800 Subject: [PATCH 08/15] Add uint32_t share_access to vuid_cache_entry. Signed-off-by: Jeremy Allison --- source3/include/vfs.h | 2 ++ source3/modules/vfs_readonly.c | 1 + source3/smbd/conn.c | 1 + 3 files changed, 4 insertions(+), 0 deletions(-) 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.7.7.3 From 48187220ff47efe70616361fcef1a794aef765b4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Dec 2012 09:35:31 -0800 Subject: [PATCH 09/15] Remove static from create_share_access_mask(). Signed-off-by: Jeremy Allison --- 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 9a9a010..c9efefc 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -971,6 +971,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 33167c070c085b30569317666a3fca079d970321 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Dec 2012 09:45:03 -0800 Subject: [PATCH 10/15] 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 --- 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 c9efefc..abcc2e5 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -971,7 +971,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 3e4393e20498021f3924a273c562d1abb36d632e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 21 Dec 2012 09:49:53 -0800 Subject: [PATCH 11/15] 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 --- source3/smbd/uid.c | 32 ++++++++++++++++++++------------ 1 files changed, 20 insertions(+), 12 deletions(-) diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index f9b5716..a1153ae 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; ivuid_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,22 @@ static bool check_user_ok(connection_struct *conn, session_info->security_token, conn); + share_access = create_share_access_mask(conn, snum); + + 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 +141,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 +168,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.7.7.3 From bb7de313995ceabcbbed7ccaafaac61b7c9aedf2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Jan 2013 16:02:30 -0800 Subject: [PATCH 12/15] Add check_user_share_access() which 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 | 5 ++++ source3/smbd/uid.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 0 deletions(-) diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index abcc2e5..e667e0d 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1089,6 +1089,11 @@ 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, + int snum, + 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 a1153ae..5d2f240 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -79,6 +79,60 @@ 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, + int snum, + const struct auth_session_info *session_info, + uint32_t *p_share_access, + bool *p_readonly_share) +{ + 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(conn, snum); + + 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 20febea00d2011e34e202b320057945b93c0c0de Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Jan 2013 16:06:40 -0800 Subject: [PATCH 13/15] 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 5d2f240..520a03e 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -145,8 +145,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.7.7.3 From 42f6d47c021cc02b3e4fdcffad26b72c3e2086e9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Jan 2013 16:09:57 -0800 Subject: [PATCH 14/15] 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, 8 insertions(+), 32 deletions(-) diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index 520a03e..f3bdc08 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -149,6 +149,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; ivuid_cache->array[i]; @@ -161,38 +162,13 @@ 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(conn, snum); - - 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, + snum, + session_info, + &share_access, + &readonly_share); + if (!NT_STATUS_IS_OK(status)) { + return false; } admin_user = token_contains_name_in_list( -- 1.7.7.3 From 2b6b66189f27f095fa7e50725bd42b5f7e291f57 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Jan 2013 16:41:29 -0800 Subject: [PATCH 15/15] 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 | 29 ++++++++--------------------- 1 files changed, 8 insertions(+), 21 deletions(-) diff --git a/source3/smbd/service.c b/source3/smbd/service.c index 1cd12a6..00e0799 100644 --- a/source3/smbd/service.c +++ b/source3/smbd/service.c @@ -650,28 +650,15 @@ 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(conn, snum); - - 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, + snum, + 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