From c0cd257ef1918983390365a889b95696f2fa3291 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 4 Jul 2011 17:02:34 +0200 Subject: [PATCH 01/10] s3: Return "granted" from share_access_check Signed-off-by: Stefan Metzmacher (cherry picked from commit 1c022d2e414607633323e65abbc63bb3aeaaa6a4) --- source3/include/proto.h | 6 ++++-- source3/lib/sharesec.c | 10 ++++++++-- source3/rpc_server/srvsvc/srv_srvsvc_nt.c | 4 ++-- source3/smbd/service.c | 13 +++++++------ source3/smbd/uid.c | 11 +++++++---- 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/source3/include/proto.h b/source3/include/proto.h index d072502..6291f11 100644 --- a/source3/include/proto.h +++ b/source3/include/proto.h @@ -329,8 +329,10 @@ struct security_descriptor *get_share_security( TALLOC_CTX *ctx, const char *ser size_t *psize); bool set_share_security(const char *share_name, struct security_descriptor *psd); bool delete_share_security(const char *servicename); -bool share_access_check(const struct security_token *token, const char *sharename, - uint32 desired_access); +bool share_access_check(const struct security_token *token, + const char *sharename, + uint32 desired_access, + uint32_t *pgranted); bool parse_usershare_acl(TALLOC_CTX *ctx, const char *acl_str, struct security_descriptor **ppsd); /* The following definitions come from lib/smbrun.c */ diff --git a/source3/lib/sharesec.c b/source3/lib/sharesec.c index c2494e2..410fc13 100644 --- a/source3/lib/sharesec.c +++ b/source3/lib/sharesec.c @@ -410,8 +410,10 @@ bool delete_share_security(const char *servicename) Can this user access with share with the required permissions ? ********************************************************************/ -bool share_access_check(const struct security_token *token, const char *sharename, - uint32 desired_access) +bool share_access_check(const struct security_token *token, + const char *sharename, + uint32 desired_access, + uint32_t *pgranted) { uint32 granted; NTSTATUS status; @@ -428,6 +430,10 @@ bool share_access_check(const struct security_token *token, const char *sharenam TALLOC_FREE(psd); + if (pgranted != NULL) { + *pgranted = granted; + } + return NT_STATUS_IS_OK(status); } diff --git a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c index 472a318..a078395 100644 --- a/source3/rpc_server/srvsvc/srv_srvsvc_nt.c +++ b/source3/rpc_server/srvsvc/srv_srvsvc_nt.c @@ -539,8 +539,8 @@ static bool is_enumeration_allowed(struct pipes_struct *p, if (!lp_access_based_share_enum(snum)) return true; - return share_access_check(p->session_info->security_token, lp_servicename(snum), - FILE_READ_DATA); + return share_access_check(p->session_info->security_token, + lp_servicename(snum), FILE_READ_DATA, NULL); } /******************************************************************* diff --git a/source3/smbd/service.c b/source3/smbd/service.c index a8cd756..6c147b2 100644 --- a/source3/smbd/service.c +++ b/source3/smbd/service.c @@ -856,14 +856,15 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn, { bool can_write = False; - can_write = share_access_check(conn->session_info->security_token, - lp_servicename(snum), - FILE_WRITE_DATA); + can_write = share_access_check( + conn->session_info->security_token, + lp_servicename(snum), FILE_WRITE_DATA, NULL); if (!can_write) { - if (!share_access_check(conn->session_info->security_token, - lp_servicename(snum), - FILE_READ_DATA)) { + if (!share_access_check( + conn->session_info->security_token, + lp_servicename(snum), FILE_READ_DATA, + NULL)) { /* No access, read or write. */ DEBUG(0,("make_connection: connection to %s " "denied due to security " diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c index 7b04713..7a48cb2 100644 --- a/source3/smbd/uid.c +++ b/source3/smbd/uid.c @@ -121,8 +121,9 @@ static bool check_user_ok(connection_struct *conn, conn); if (!readonly_share && - !share_access_check(session_info->security_token, lp_servicename(snum), - FILE_WRITE_DATA)) { + !share_access_check(session_info->security_token, + lp_servicename(snum), FILE_WRITE_DATA, + NULL)) { /* smb.conf allows r/w, but the security descriptor denies * write. Fall back to looking at readonly. */ readonly_share = True; @@ -130,9 +131,11 @@ static bool check_user_ok(connection_struct *conn, "security descriptor\n")); } - if (!share_access_check(session_info->security_token, lp_servicename(snum), + if (!share_access_check(session_info->security_token, + lp_servicename(snum), readonly_share ? - FILE_READ_DATA : FILE_WRITE_DATA)) { + FILE_READ_DATA : FILE_WRITE_DATA, + NULL)) { return False; } -- 1.7.4.1 From e0b2281166d537a6c21d2b2debf4d9df2108ece6 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 4 Jul 2011 18:35:21 +0200 Subject: [PATCH 02/10] s3: Calculate&store the maximum share access mask Signed-off-by: Stefan Metzmacher (cherry picked from commit 720fa46f9443ccbe471b265f1c2b9cb9782a3c26) --- source3/include/smb.h | 1 + source3/smbd/service.c | 37 +++++++++++++++---------------------- 2 files changed, 16 insertions(+), 22 deletions(-) diff --git a/source3/include/smb.h b/source3/include/smb.h index 3e68a99..3a64af7 100644 --- a/source3/include/smb.h +++ b/source3/include/smb.h @@ -408,6 +408,7 @@ typedef struct connection_struct { bool printer; bool ipc; bool read_only; /* Attributes for the current user of the share. */ + uint32_t share_access; /* Does this filesystem honor sub second timestamps on files and directories when setting time ? */ diff --git a/source3/smbd/service.c b/source3/smbd/service.c index 6c147b2..d88c02c 100644 --- a/source3/smbd/service.c +++ b/source3/smbd/service.c @@ -853,28 +853,21 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn, * */ - { - bool can_write = False; - - can_write = share_access_check( - conn->session_info->security_token, - lp_servicename(snum), FILE_WRITE_DATA, NULL); - - if (!can_write) { - if (!share_access_check( - conn->session_info->security_token, - lp_servicename(snum), FILE_READ_DATA, - NULL)) { - /* No access, read or write. */ - DEBUG(0,("make_connection: connection to %s " - "denied due to security " - "descriptor.\n", - lp_servicename(snum))); - *pstatus = NT_STATUS_ACCESS_DENIED; - goto err_root_exit; - } else { - conn->read_only = True; - } + share_access_check(conn->session_info->security_token, + lp_servicename(snum), MAXIMUM_ALLOWED_ACCESS, + &conn->share_access); + + 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(snum))); + *pstatus = NT_STATUS_ACCESS_DENIED; + goto err_root_exit; + } else { + conn->read_only = True; } } /* Initialise VFS function pointers */ -- 1.7.4.1 From ec13fa3e36b95526dc305ed1eb9b2c2de9e8672b Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 5 Jul 2011 11:13:07 +0200 Subject: [PATCH 03/10] s3: Fix bug 8102 We can't allow open with access that has been denied via the share security descriptor Signed-off-by: Stefan Metzmacher Autobuild-User: Stefan Metzmacher Autobuild-Date: Tue Jul 5 16:21:54 CEST 2011 on sn-devel-104 (cherry picked from commit 4deca5d72804a40e68158a1183f5633dabf24761) --- source3/smbd/open.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 44b1835..d58744f 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -76,6 +76,14 @@ NTSTATUS smbd_check_open_rights(struct connection_struct *conn, /* Check if we have rights to open. */ NTSTATUS status; struct security_descriptor *sd = NULL; + uint32_t rejected_share_access; + + rejected_share_access = access_mask & ~(conn->share_access); + + if (rejected_share_access) { + *access_granted = rejected_share_access; + return NT_STATUS_ACCESS_DENIED; + } if ((access_mask & DELETE_ACCESS) && !lp_acl_check_permissions(SNUM(conn))) { *access_granted = access_mask; -- 1.7.4.1 From 44a0eaf0a542a71f14788c817baad5c134c0a82d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 12 Jul 2011 17:31:13 +0200 Subject: [PATCH 04/10] s3:smbd/msdfs: let create_conn_struct() check the share security descriptor metze (cherry picked from commit 18f967a24881aa899b39f7676fc70a7f7aaca07b) --- source3/smbd/msdfs.c | 30 ++++++++++++++++++++++++++++++ 1 files changed, 30 insertions(+), 0 deletions(-) diff --git a/source3/smbd/msdfs.c b/source3/smbd/msdfs.c index ab67ac8..3bdedb8 100644 --- a/source3/smbd/msdfs.c +++ b/source3/smbd/msdfs.c @@ -27,6 +27,7 @@ #include "smbd/globals.h" #include "msdfs.h" #include "auth.h" +#include "libcli/security/security.h" /********************************************************************** Parse a DFS pathname of the form \hostname\service\reqpath @@ -278,6 +279,35 @@ NTSTATUS create_conn_struct(TALLOC_CTX *ctx, set_conn_connectpath(conn, connpath); + /* + * 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. + * + */ + if (conn->session_info) { + share_access_check(conn->session_info->security_token, + lp_servicename(snum), MAXIMUM_ALLOWED_ACCESS, + &conn->share_access); + + if ((conn->share_access & FILE_WRITE_DATA) == 0) { + if ((conn->share_access & FILE_READ_DATA) == 0) { + /* No access, read or write. */ + DEBUG(0,("create_conn_struct: connection to %s " + "denied due to security " + "descriptor.\n", + lp_servicename(snum))); + conn_free(conn); + return NT_STATUS_ACCESS_DENIED; + } else { + conn->read_only = true; + } + } + } else { + conn->share_access = 0; + conn->read_only = true; + } + if (!smbd_vfs_init(conn)) { NTSTATUS status = map_nt_error_from_unix(errno); DEBUG(0,("create_conn_struct: smbd_vfs_init failed.\n")); -- 1.7.4.1 From db1e1d2517bb8105967902b89bf8c2a29146d52d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sun, 10 Jul 2011 13:00:25 +0200 Subject: [PATCH 05/10] s3:smbd: make smbd_calculate_access_mask() non-static metze (cherry picked from commit ce66d4e4a885add09edfa8e6d5eab0f3b5d63081) --- source3/smbd/globals.h | 5 +++++ source3/smbd/open.c | 29 +++++++++++++++-------------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 97d75fd..510c08c 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -224,6 +224,11 @@ bool smbd_dirptr_lanman2_entry(TALLOC_CTX *ctx, int *_last_entry_off, struct ea_list *name_list); +NTSTATUS smbd_calculate_access_mask(connection_struct *conn, + const struct smb_filename *smb_fname, + bool file_existed, + uint32_t access_mask, + uint32_t *access_mask_out); NTSTATUS smbd_check_open_rights(struct connection_struct *conn, const struct smb_filename *smb_fname, uint32_t access_mask, diff --git a/source3/smbd/open.c b/source3/smbd/open.c index d58744f..58102e4 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1522,11 +1522,11 @@ static void schedule_defer_open(struct share_mode_lock *lck, Work out what access_mask to use from what the client sent us. ****************************************************************************/ -static NTSTATUS calculate_access_mask(connection_struct *conn, - const struct smb_filename *smb_fname, - bool file_existed, - uint32_t access_mask, - uint32_t *access_mask_out) +NTSTATUS smbd_calculate_access_mask(connection_struct *conn, + const struct smb_filename *smb_fname, + bool file_existed, + uint32_t access_mask, + uint32_t *access_mask_out) { NTSTATUS status; @@ -1549,8 +1549,8 @@ static NTSTATUS calculate_access_mask(connection_struct *conn, SECINFO_DACL),&sd); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("calculate_access_mask: Could not get acl " - "on file %s: %s\n", + DEBUG(10,("smbd_calculate_access_mask: " + "Could not get acl on file %s: %s\n", smb_fname_str_dbg(smb_fname), nt_errstr(status))); return NT_STATUS_ACCESS_DENIED; @@ -1565,8 +1565,9 @@ static NTSTATUS calculate_access_mask(connection_struct *conn, TALLOC_FREE(sd); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("calculate_access_mask: Access denied on " - "file %s: when calculating maximum access\n", + DEBUG(10, ("smbd_calculate_access_mask: " + "Access denied on file %s: " + "when calculating maximum access\n", smb_fname_str_dbg(smb_fname))); return NT_STATUS_ACCESS_DENIED; } @@ -1898,11 +1899,11 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, } } - status = calculate_access_mask(conn, smb_fname, file_existed, + status = smbd_calculate_access_mask(conn, smb_fname, file_existed, access_mask, &access_mask); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("open_file_ntcreate: calculate_access_mask " + DEBUG(10, ("open_file_ntcreate: smbd_calculate_access_mask " "on file %s returned %s\n", smb_fname_str_dbg(smb_fname), nt_errstr(status))); return status; @@ -2745,10 +2746,10 @@ static NTSTATUS open_directory(connection_struct *conn, return NT_STATUS_NOT_A_DIRECTORY; } - status = calculate_access_mask(conn, smb_dname, dir_existed, - access_mask, &access_mask); + status = smbd_calculate_access_mask(conn, smb_dname, dir_existed, + access_mask, &access_mask); if (!NT_STATUS_IS_OK(status)) { - DEBUG(10, ("open_directory: calculate_access_mask " + DEBUG(10, ("open_directory: smbd_calculate_access_mask " "on file %s returned %s\n", smb_fname_str_dbg(smb_dname), nt_errstr(status))); -- 1.7.4.1 From 90a13f101725af7018dc7ae24e7272fa4376a18b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sun, 10 Jul 2011 13:03:51 +0200 Subject: [PATCH 06/10] s3:smbd: check the share level access mask in smbd_calculate_access_mask() I think we should reject invalid access early, before we might create new files. Also smbd_check_open_rights() is only called if the file existed. metze (cherry picked from commit 896f105ed40dc04f83bcbfac367b309c8d957f86) --- source3/smbd/open.c | 17 +++++++++++++++++ 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 58102e4..81d4e693 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -1529,6 +1529,8 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn, uint32_t *access_mask_out) { NTSTATUS status; + uint32_t orig_access_mask = access_mask; + uint32_t rejected_share_access; /* * Convert GENERIC bits to specific bits. @@ -1576,6 +1578,21 @@ NTSTATUS smbd_calculate_access_mask(connection_struct *conn, } else { access_mask = FILE_GENERIC_ALL; } + + access_mask &= conn->share_access; + } + + rejected_share_access = access_mask & ~(conn->share_access); + + if (rejected_share_access) { + DEBUG(10, ("smbd_calculate_access_mask: Access denied on " + "file %s: rejected by share access mask[0x%08X] " + "orig[0x%08X] mapped[0x%08X] reject[0x%08X]\n", + smb_fname_str_dbg(smb_fname), + conn->share_access, + orig_access_mask, access_mask, + rejected_share_access)); + return NT_STATUS_ACCESS_DENIED; } *access_mask_out = access_mask; -- 1.7.4.1 From b8a2fa246c1d2638a16f49bbf1d83585b1897207 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sun, 10 Jul 2011 13:59:40 +0200 Subject: [PATCH 07/10] s3:smbd: use smbd_calculate_access_mask() also for fake_files metze (cherry picked from commit 581d8fa36b73abab030168dc35fb631ccd42a388) --- source3/smbd/fake_file.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/source3/smbd/fake_file.c b/source3/smbd/fake_file.c index 81f7686..68967fb 100644 --- a/source3/smbd/fake_file.c +++ b/source3/smbd/fake_file.c @@ -19,6 +19,7 @@ #include "includes.h" #include "smbd/smbd.h" +#include "smbd/globals.h" #include "fake_file.h" #include "auth.h" @@ -128,6 +129,18 @@ NTSTATUS open_fake_file(struct smb_request *req, connection_struct *conn, files_struct *fsp = NULL; NTSTATUS status; + status = smbd_calculate_access_mask(conn, smb_fname, + false, /* fake files do not exist */ + access_mask, &access_mask); + if (!NT_STATUS_IS_OK(status)) { + DEBUG(10, ("open_fake_file: smbd_calculate_access_mask " + "on service[%s] file[%s] returned %s\n", + lp_servicename(SNUM(conn)), + smb_fname_str_dbg(smb_fname), + nt_errstr(status))); + return status; + } + /* access check */ if (geteuid() != sec_initial_uid()) { DEBUG(3, ("open_fake_file_shared: access_denied to " -- 1.7.4.1 From 41bbc22838b60462285551e0d3a3047978d6b48d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 11 Jul 2011 16:12:57 +0200 Subject: [PATCH 08/10] s3:smbd: return the real share access mask in the SMBtconX response metze (cherry picked from commit 58eed1b295afeff6acfb8c1f10b0bb02280fd491) --- source3/smbd/reply.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 099a36e..c594a6b 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -835,9 +835,7 @@ void reply_tcon_and_X(struct smb_request *req) perm1 = FILE_ALL_ACCESS; perm2 = FILE_ALL_ACCESS; } else { - perm1 = CAN_WRITE(conn) ? - SHARE_ALL_ACCESS : - SHARE_READ_ONLY; + perm1 = conn->share_access; } SIVAL(req->outbuf, smb_vwv3, perm1); -- 1.7.4.1 From 5f6d8f415906beac0f5062e81e8c6d27c3aee29c Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sun, 10 Jul 2011 13:02:11 +0200 Subject: [PATCH 09/10] s3:smb2_tcon: return the correct maximal_access on the share metze (cherry picked from commit a1046389ffcc476456ac76cb701a4325d1c42ef9) --- source3/smbd/smb2_tcon.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c index 75cfe54..ea38d1e 100644 --- a/source3/smbd/smb2_tcon.c +++ b/source3/smbd/smb2_tcon.c @@ -271,7 +271,7 @@ static NTSTATUS smbd_smb2_tree_connect(struct smbd_smb2_request *req, break; } - *out_maximal_access = FILE_GENERIC_ALL; + *out_maximal_access = tcon->compat_conn->share_access; *out_tree_id = tcon->tid; return NT_STATUS_OK; -- 1.7.4.1 From 3f7c5d5d07d202b4c7b56da473aeae4f6029a9e2 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Sun, 10 Jul 2011 13:09:06 +0200 Subject: [PATCH 10/10] s3:smb2_create: use smbd_calculate_access_mask() instead of smbd_check_open_rights() metze Autobuild-User: Stefan Metzmacher Autobuild-Date: Mon Jul 11 22:45:01 CEST 2011 on sn-devel-104 (cherry picked from commit f5d320ac0fb74d4ad95a03969366096e9b074379) --- source3/smbd/smb2_create.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c index 3478f34..fcd8945 100644 --- a/source3/smbd/smb2_create.c +++ b/source3/smbd/smb2_create.c @@ -736,8 +736,13 @@ static struct tevent_req *smbd_smb2_create_send(TALLOC_CTX *mem_ctx, uint32_t max_access_granted; DATA_BLOB blob = data_blob_const(p, sizeof(p)); - status = smbd_check_open_rights(smb1req->conn, + status = smbd_calculate_access_mask(smb1req->conn, result->fsp_name, + /* + * at this stage + * it exists + */ + true, SEC_FLAG_MAXIMUM_ALLOWED, &max_access_granted); -- 1.7.4.1