The branch, master has been updated
via 2898485 Move the SEC_DIR_LIST check into dptr_create for SMB2 and
now for SMB1.
via 7ff5a55 can_write_to_file() does now take share permissions into
account. Fix comment.
via bbcb589 No longer do the pre-check on DELETE_ACCESS - we're
correctly checking the ACL every time now.
via b988a32 Remove can_access_file_acl(). We no longer need this
duplicate code (hurrah!).
via 60b7414 Remove can_access_file_data() - make it use the standard
smbd_check_access_rights() instead.
via 4851219 Add const to the smb_filename argument of
smbd_check_access_rights().
via a30f84a Expose smbd_check_access_rights() to other modules.
via 32edc1d Rename smbd_check_open_rights() to
smbd_check_access_rights() as we're going to remove the static from this.
via 0c886ee Replace smb1_file_se_access_check() with just
se_access_check().
via 55b9ba7 Move root check out of smb1_file_se_access_check() in
preparation for deleting this function.
via 07edf6c smb1_file_se_access_check() is now static to smbd/open.c
via 1fab17d Revert "Change function signature of check_parent_access()
to take char * instead of struct smb_filename."
via d433af9 Revert "Call check_parent_access() on readdir."
from 51c86c8 Properly fix bug #8384 - Windows XP clients seem to crash
smbd process every once in a while.
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 28984858486b9e61142afd3d34ae1e822e0ccef1
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 10:51:29 2011 -0700
Move the SEC_DIR_LIST check into dptr_create for SMB2 and now for SMB1.
The pathname check still needs fixing.
Autobuild-User: Jeremy Allison <[email protected]>
Autobuild-Date: Sat Nov 5 01:38:00 CET 2011 on sn-devel-104
commit 7ff5a5584f7d299be1c46bd881adf3ebc27a2a20
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 16:01:16 2011 -0700
can_write_to_file() does now take share permissions into account. Fix
comment.
commit bbcb589ef5a9cf1ad98b70fc2ea00d346323c57e
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 15:56:15 2011 -0700
No longer do the pre-check on DELETE_ACCESS - we're correctly checking the
ACL every time now.
commit b988a3233fae309c6f034724ae106d935b1489f7
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 15:55:11 2011 -0700
Remove can_access_file_acl(). We no longer need this duplicate code
(hurrah!).
commit 60b741415d9357f11ae1db80f93035058fdbe4e8
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 15:45:13 2011 -0700
Remove can_access_file_data() - make it use the standard
smbd_check_access_rights() instead.
commit 48512193338663df5dc4cd52179bc94337eb7113
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 15:39:55 2011 -0700
Add const to the smb_filename argument of smbd_check_access_rights().
commit a30f84a21c9d4e702ae0faace9bdf435b9882af7
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 14:37:26 2011 -0700
Expose smbd_check_access_rights() to other modules.
commit 32edc1d047480a0bcf92c9a172ec8161748f3d74
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 14:28:08 2011 -0700
Rename smbd_check_open_rights() to smbd_check_access_rights() as we're
going to remove the static from this.
commit 0c886eeb89e8a0ba85788ef87002c6853d6798e6
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 14:21:35 2011 -0700
Replace smb1_file_se_access_check() with just se_access_check().
commit 55b9ba79f8c612d6413e8e673b39dd4e0548dc82
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 14:07:23 2011 -0700
Move root check out of smb1_file_se_access_check() in preparation for
deleting this function.
commit 07edf6c65e514064f15ef0b31b5a98250568a505
Author: Jeremy Allison <[email protected]>
Date: Fri Nov 4 13:11:01 2011 -0700
smb1_file_se_access_check() is now static to smbd/open.c
commit 1fab17de94a38294918e9eab7d6f85b94d6db421
Author: Jeremy Allison <[email protected]>
Date: Thu Nov 3 11:49:22 2011 -0700
Revert "Change function signature of check_parent_access() to take char *
instead of struct smb_filename."
This reverts commit a11c0a41a35aa2b1c14333552045a65e3e50df1e.
Not needed.
commit d433af92b9085e862b6fd3e3f4aea90051d9a2fc
Author: Jeremy Allison <[email protected]>
Date: Thu Nov 3 11:49:05 2011 -0700
Revert "Call check_parent_access() on readdir."
This reverts commit a763edaf9c76afe2546c035fc090370301dd347b.
Checking the wrong thing..
-----------------------------------------------------------------------
Summary of changes:
source3/modules/onefs_open.c | 4 +-
source3/smbd/dir.c | 23 +++----
source3/smbd/file_access.c | 129 +++-------------------------------
source3/smbd/open.c | 157 +++++++++++++++++++-----------------------
source3/smbd/proto.h | 20 +----
source3/smbd/smb2_find.c | 5 --
6 files changed, 96 insertions(+), 242 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/modules/onefs_open.c b/source3/modules/onefs_open.c
index dd4eb90..d7b2af3 100644
--- a/source3/modules/onefs_open.c
+++ b/source3/modules/onefs_open.c
@@ -1042,8 +1042,8 @@ NTSTATUS onefs_open_file_ntcreate(connection_struct *conn,
if (((can_access_mask & FILE_WRITE_DATA) &&
!CAN_WRITE(conn)) ||
- !can_access_file_data(conn, smb_fname,
- can_access_mask)) {
+ !NT_STATUS_IS_OK(smbd_check_access_rights(conn,
+ smb_fname, can_access_mask))) {
can_access = False;
}
diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
index 18c5935..a11c131 100644
--- a/source3/smbd/dir.c
+++ b/source3/smbd/dir.c
@@ -427,7 +427,6 @@ NTSTATUS dptr_create(connection_struct *conn, files_struct
*fsp,
struct smbd_server_connection *sconn = conn->sconn;
struct dptr_struct *dptr = NULL;
struct smb_Dir *dir_hnd;
- NTSTATUS status;
if (fsp && fsp->is_directory && fsp->fh->fd != -1) {
path = fsp->fsp_name->base_name;
@@ -444,19 +443,13 @@ NTSTATUS dptr_create(connection_struct *conn,
files_struct *fsp,
return NT_STATUS_INVALID_PARAMETER;
}
- status = check_parent_access(conn,
- path,
- SEC_DIR_LIST,
- NULL);
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(5,("dptr_create: parent access check for path "
- "%s failed with %s\n",
- path,
- nt_errstr(status)));
- return status;
- }
-
if (fsp) {
+ if (!(fsp->access_mask & SEC_DIR_LIST)) {
+ DEBUG(5,("dptr_create: directory %s "
+ "not open for LIST access\n",
+ path));
+ return NT_STATUS_ACCESS_DENIED;
+ }
dir_hnd = OpenDir_fsp(NULL, conn, fsp, wcard, attr);
} else {
dir_hnd = OpenDir(NULL, conn, path, wcard, attr);
@@ -1174,7 +1167,9 @@ static bool user_can_read_file(connection_struct *conn,
return True;
}
- return can_access_file_acl(conn, smb_fname, FILE_READ_DATA);
+ return NT_STATUS_IS_OK(smbd_check_access_rights(conn,
+ smb_fname,
+ FILE_READ_DATA));
}
/*******************************************************************
diff --git a/source3/smbd/file_access.c b/source3/smbd/file_access.c
index 7485564..4a473d7 100644
--- a/source3/smbd/file_access.c
+++ b/source3/smbd/file_access.c
@@ -27,60 +27,13 @@
#undef DBGC_CLASS
#define DBGC_CLASS DBGC_ACLS
-/**
- * Security descriptor / NT Token level access check function.
- */
-bool can_access_file_acl(struct connection_struct *conn,
- const struct smb_filename *smb_fname,
- uint32_t access_mask)
-{
- NTSTATUS status;
- uint32_t access_granted;
- struct security_descriptor *secdesc = NULL;
- bool ret;
-
- if (get_current_uid(conn) == (uid_t)0) {
- /* I'm sorry sir, I didn't know you were root... */
- return true;
- }
-
- status = SMB_VFS_GET_NT_ACL(conn, smb_fname->base_name,
- (SECINFO_OWNER |
- SECINFO_GROUP |
- SECINFO_DACL),
- &secdesc);
- if (!NT_STATUS_IS_OK(status)) {
- DEBUG(5, ("Could not get acl: %s\n", nt_errstr(status)));
- ret = false;
- goto out;
- }
-
- status = se_access_check(secdesc, get_current_nttok(conn),
- access_mask, &access_granted);
- ret = NT_STATUS_IS_OK(status);
-
- if (DEBUGLEVEL >= 10) {
- DEBUG(10,("can_access_file_acl for file %s "
- "access_mask 0x%x, access_granted 0x%x "
- "access %s\n",
- smb_fname_str_dbg(smb_fname),
- (unsigned int)access_mask,
- (unsigned int)access_granted,
- ret ? "ALLOWED" : "DENIED" ));
- NDR_PRINT_DEBUG(security_descriptor, secdesc);
- }
- out:
- TALLOC_FREE(secdesc);
- return ret;
-}
-
/****************************************************************************
Actually emulate the in-kernel access checking for delete access. We need
this to successfully return ACCESS_DENIED on a file open for delete access.
****************************************************************************/
bool can_delete_file_in_directory(connection_struct *conn,
- struct smb_filename *smb_fname)
+ const struct smb_filename *smb_fname)
{
TALLOC_CTX *ctx = talloc_tos();
char *dname = NULL;
@@ -130,18 +83,10 @@ bool can_delete_file_in_directory(connection_struct *conn,
/* sticky bit means delete only by owner of file or by root or
* by owner of directory. */
if (smb_fname_parent->st.st_ex_mode & S_ISVTX) {
- if(SMB_VFS_STAT(conn, smb_fname) != 0) {
- if (errno == ENOENT) {
- /* If the file doesn't already exist then
- * yes we'll be able to delete it. */
- ret = true;
- goto out;
- }
- DEBUG(10,("can_delete_file_in_directory: can't "
- "stat file %s (%s)",
- smb_fname_str_dbg(smb_fname),
- strerror(errno) ));
- ret = false;
+ if (!VALID_STAT(smb_fname->st)) {
+ /* If the file doesn't already exist then
+ * yes we'll be able to delete it. */
+ ret = true;
goto out;
}
@@ -177,7 +122,9 @@ bool can_delete_file_in_directory(connection_struct *conn,
* check the file DELETE permission separately.
*/
- ret = can_access_file_acl(conn, smb_fname_parent, FILE_DELETE_CHILD);
+ ret = NT_STATUS_IS_OK(smbd_check_access_rights(conn,
+ smb_fname_parent,
+ FILE_DELETE_CHILD));
out:
TALLOC_FREE(dname);
TALLOC_FREE(smb_fname_parent);
@@ -185,69 +132,15 @@ bool can_delete_file_in_directory(connection_struct *conn,
}
/****************************************************************************
- Actually emulate the in-kernel access checking for read/write access. We need
- this to successfully check for ability to write for dos filetimes.
- Note this doesn't take into account share write permissions.
-****************************************************************************/
-
-bool can_access_file_data(connection_struct *conn,
- const struct smb_filename *smb_fname,
- uint32 access_mask)
-{
- if (!(access_mask & (FILE_READ_DATA|FILE_WRITE_DATA))) {
- return False;
- }
- access_mask &= (FILE_READ_DATA|FILE_WRITE_DATA);
-
- /* some fast paths first */
-
- DEBUG(10,("can_access_file_data: requesting 0x%x on file %s\n",
- (unsigned int)access_mask, smb_fname_str_dbg(smb_fname)));
-
- if (get_current_uid(conn) == (uid_t)0) {
- /* I'm sorry sir, I didn't know you were root... */
- return True;
- }
-
- SMB_ASSERT(VALID_STAT(smb_fname->st));
-
- /* Check primary owner access. */
- if (get_current_uid(conn) == smb_fname->st.st_ex_uid) {
- switch (access_mask) {
- case FILE_READ_DATA:
- return (smb_fname->st.st_ex_mode & S_IRUSR) ?
- True : False;
-
- case FILE_WRITE_DATA:
- return (smb_fname->st.st_ex_mode & S_IWUSR) ?
- True : False;
-
- default: /* FILE_READ_DATA|FILE_WRITE_DATA */
-
- if ((smb_fname->st.st_ex_mode &
- (S_IWUSR|S_IRUSR)) ==
- (S_IWUSR|S_IRUSR)) {
- return True;
- } else {
- return False;
- }
- }
- }
-
- /* now for ACL checks */
-
- return can_access_file_acl(conn, smb_fname, access_mask);
-}
-
-/****************************************************************************
Userspace check for write access.
- Note this doesn't take into account share write permissions.
****************************************************************************/
bool can_write_to_file(connection_struct *conn,
const struct smb_filename *smb_fname)
{
- return can_access_file_data(conn, smb_fname, FILE_WRITE_DATA);
+ return NT_STATUS_IS_OK(smbd_check_access_rights(conn,
+ smb_fname,
+ FILE_WRITE_DATA));
}
/****************************************************************************
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index e8c24a0..575503f 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -38,40 +38,13 @@ struct deferred_open_record {
};
/****************************************************************************
- SMB1 file varient of se_access_check. Never test FILE_READ_ATTRIBUTES.
-****************************************************************************/
-
-NTSTATUS smb1_file_se_access_check(struct connection_struct *conn,
- const struct security_descriptor *sd,
- const struct security_token *token,
- uint32_t access_desired,
- uint32_t *access_granted)
-{
- *access_granted = 0;
-
- if (get_current_uid(conn) == (uid_t)0) {
- /* I'm sorry sir, I didn't know you were root... */
- *access_granted = access_desired;
- if (access_desired & SEC_FLAG_MAXIMUM_ALLOWED) {
- *access_granted |= FILE_GENERIC_ALL;
- }
- return NT_STATUS_OK;
- }
-
- return se_access_check(sd,
- token,
- (access_desired & ~FILE_READ_ATTRIBUTES),
- access_granted);
-}
-
-/****************************************************************************
If the requester wanted DELETE_ACCESS and was rejected because
the file ACL didn't include DELETE_ACCESS, see if the parent ACL
ovverrides this.
****************************************************************************/
static bool parent_override_delete(connection_struct *conn,
- struct smb_filename *smb_fname,
+ const struct smb_filename *smb_fname,
uint32_t access_mask,
uint32_t rejected_mask)
{
@@ -87,8 +60,8 @@ static bool parent_override_delete(connection_struct *conn,
Check if we have open rights.
****************************************************************************/
-static NTSTATUS smbd_check_open_rights(struct connection_struct *conn,
- struct smb_filename *smb_fname,
+NTSTATUS smbd_check_access_rights(struct connection_struct *conn,
+ const struct smb_filename *smb_fname,
uint32_t access_mask)
{
/* Check if we have rights to open. */
@@ -100,7 +73,7 @@ static NTSTATUS smbd_check_open_rights(struct
connection_struct *conn,
rejected_share_access = access_mask & ~(conn->share_access);
if (rejected_share_access) {
- DEBUG(10, ("smbd_check_open_rights: rejected share access 0x%x "
+ DEBUG(10, ("smbd_check_access_rights: rejected share access
0x%x "
"on %s (0x%x)\n",
(unsigned int)access_mask,
smb_fname_str_dbg(smb_fname),
@@ -108,8 +81,17 @@ static NTSTATUS smbd_check_open_rights(struct
connection_struct *conn,
return NT_STATUS_ACCESS_DENIED;
}
+ if (get_current_uid(conn) == (uid_t)0) {
+ /* I'm sorry sir, I didn't know you were root... */
+ DEBUG(10,("smbd_check_access_rights: root override "
+ "on %s. Granting 0x%x\n",
+ smb_fname_str_dbg(smb_fname),
+ (unsigned int)access_mask ));
+ return NT_STATUS_OK;
+ }
+
if ((access_mask & DELETE_ACCESS) &&
!lp_acl_check_permissions(SNUM(conn))) {
- DEBUG(10,("smbd_check_open_rights: not checking ACL "
+ DEBUG(10,("smbd_check_access_rights: not checking ACL "
"on DELETE_ACCESS on file %s. Granting 0x%x\n",
smb_fname_str_dbg(smb_fname),
(unsigned int)access_mask ));
@@ -122,20 +104,23 @@ static NTSTATUS smbd_check_open_rights(struct
connection_struct *conn,
SECINFO_DACL),&sd);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("smbd_check_open_rights: Could not get acl "
+ DEBUG(10, ("smbd_check_access_rights: Could not get acl "
"on %s: %s\n",
smb_fname_str_dbg(smb_fname),
nt_errstr(status)));
return status;
}
- status = smb1_file_se_access_check(conn,
- sd,
+ /*
+ * Never test FILE_READ_ATTRIBUTES. se_access_check() also takes care of
+ * owner WRITE_DAC and READ_CONTROL.
+ */
+ status = se_access_check(sd,
get_current_nttok(conn),
- access_mask,
+ (access_mask & ~FILE_READ_ATTRIBUTES),
&rejected_mask);
- DEBUG(10,("smbd_check_open_rights: file %s requesting "
+ DEBUG(10,("smbd_check_access_rights: file %s requesting "
"0x%x returning 0x%x (%s)\n",
smb_fname_str_dbg(smb_fname),
(unsigned int)access_mask,
@@ -144,7 +129,7 @@ static NTSTATUS smbd_check_open_rights(struct
connection_struct *conn,
if (!NT_STATUS_IS_OK(status)) {
if (DEBUGLEVEL >= 10) {
- DEBUG(10,("smbd_check_open_rights: acl for %s is:\n",
+ DEBUG(10,("smbd_check_access_rights: acl for %s is:\n",
smb_fname_str_dbg(smb_fname) ));
NDR_PRINT_DEBUG(security_descriptor, sd);
}
@@ -166,7 +151,7 @@ static NTSTATUS smbd_check_open_rights(struct
connection_struct *conn,
lp_map_system(SNUM(conn)))) {
rejected_mask &= ~FILE_WRITE_ATTRIBUTES;
- DEBUG(10,("smbd_check_open_rights: "
+ DEBUG(10,("smbd_check_access_rights: "
"overrode "
"FILE_WRITE_ATTRIBUTES "
"on file %s\n",
@@ -187,7 +172,7 @@ static NTSTATUS smbd_check_open_rights(struct
connection_struct *conn,
rejected_mask &= ~DELETE_ACCESS;
- DEBUG(10,("smbd_check_open_rights: "
+ DEBUG(10,("smbd_check_access_rights: "
"overrode "
"DELETE_ACCESS on "
"file %s\n",
@@ -201,8 +186,8 @@ static NTSTATUS smbd_check_open_rights(struct
connection_struct *conn,
}
}
-NTSTATUS check_parent_access(struct connection_struct *conn,
- const char *path,
+static NTSTATUS check_parent_access(struct connection_struct *conn,
+ struct smb_filename *smb_fname,
uint32_t access_mask,
char **pp_parent_dir)
{
@@ -212,12 +197,25 @@ NTSTATUS check_parent_access(struct connection_struct
*conn,
uint32_t access_granted = 0;
if (!parent_dirname(talloc_tos(),
- path,
+ smb_fname->base_name,
&parent_dir,
NULL)) {
return NT_STATUS_NO_MEMORY;
}
+ if (pp_parent_dir) {
+ *pp_parent_dir = parent_dir;
+ }
+
+ if (get_current_uid(conn) == (uid_t)0) {
+ /* I'm sorry sir, I didn't know you were root... */
+ DEBUG(10,("check_parent_access: root override "
+ "on %s. Granting 0x%x\n",
+ smb_fname_str_dbg(smb_fname),
+ (unsigned int)access_mask ));
+ return NT_STATUS_OK;
+ }
+
status = SMB_VFS_GET_NT_ACL(conn,
parent_dir,
SECINFO_DACL,
@@ -231,26 +229,26 @@ NTSTATUS check_parent_access(struct connection_struct
*conn,
return status;
}
- status = smb1_file_se_access_check(conn,
- parent_sd,
- get_current_nttok(conn),
- access_mask,
- &access_granted);
+ /*
+ * Never test FILE_READ_ATTRIBUTES. se_access_check() also takes care of
+ * owner WRITE_DAC and READ_CONTROL.
+ */
+ status = se_access_check(parent_sd,
+ get_current_nttok(conn),
+ (access_mask & ~FILE_READ_ATTRIBUTES),
+ &access_granted);
if(!NT_STATUS_IS_OK(status)) {
DEBUG(5,("check_parent_access: access check "
"on directory %s for "
"path %s for mask 0x%x returned (0x%x) %s\n",
parent_dir,
- path,
+ smb_fname->base_name,
access_mask,
access_granted,
nt_errstr(status) ));
return status;
}
- if (pp_parent_dir) {
- *pp_parent_dir = parent_dir;
- }
return NT_STATUS_OK;
}
@@ -613,12 +611,12 @@ static NTSTATUS open_file(files_struct *fsp,
if (!fsp->base_fsp) {
/* Only do this check on non-stream open. */
if (file_existed) {
- status = smbd_check_open_rights(conn,
+ status = smbd_check_access_rights(conn,
smb_fname,
access_mask);
} else if (local_flags & O_CREAT){
status = check_parent_access(conn,
- smb_fname->base_name,
+ smb_fname,
SEC_DIR_ADD_FILE,
NULL);
} else {
@@ -630,7 +628,7 @@ static NTSTATUS open_file(files_struct *fsp,
"%s on file "
"%s returned %s\n",
file_existed ?
- "smbd_check_open_rights" :
+ "smbd_check_access_rights" :
"check_parent_access",
smb_fname_str_dbg(smb_fname),
nt_errstr(status) ));
@@ -658,7 +656,7 @@ static NTSTATUS open_file(files_struct *fsp,
return NT_STATUS_OBJECT_NAME_NOT_FOUND;
}
- status = smbd_check_open_rights(conn,
+ status = smbd_check_access_rights(conn,
smb_fname,
access_mask);
@@ -676,7 +674,7 @@ static NTSTATUS open_file(files_struct *fsp,
if (!NT_STATUS_IS_OK(status)) {
DEBUG(10,("open_file: "
- "smbd_check_open_rights on file "
+ "smbd_check_access_rights on file "
"%s returned %s\n",
smb_fname_str_dbg(smb_fname),
nt_errstr(status) ));
@@ -1474,7 +1472,9 @@ NTSTATUS smbd_calculate_access_mask(connection_struct
*conn,
/* Calculate MAXIMUM_ALLOWED_ACCESS if requested. */
if (access_mask & MAXIMUM_ALLOWED_ACCESS) {
- if (file_existed) {
+ if (get_current_uid(conn) == (uid_t)0) {
--
Samba Shared Repository
