The branch, master has been updated
via 4928d66 libcli/security: make sure that we don't grant
SEC_STD_DELETE to the owner by default
via f0ec69b s3:smbd: access checks should not depend on share mode flags
via 3dc999e s4:ntvfs/posix: grant SEC_STD_DELETE if the parent grants
SEC_DIR_DELETE_CHILD
from 1615581 s3: Fix Coverity ID 1048, CHECKED_RETURN
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 4928d66fc2f469b75090c34f8d233026485e4a1e
Author: Stefan Metzmacher <me...@samba.org>
Date: Mon Mar 21 11:21:57 2011 +0100
libcli/security: make sure that we don't grant SEC_STD_DELETE to the owner
by default
In the file server SEC_STD_DELETE is granted on the file/directory
or by FILE_DELETE_CHILD on the parent directory.
metze
Autobuild-User: Stefan Metzmacher <me...@samba.org>
Autobuild-Date: Mon Mar 21 23:25:05 CET 2011 on sn-devel-104
commit f0ec69b53544b7ff702f94d58b3d64c33eaabc7a
Author: Stefan Metzmacher <me...@samba.org>
Date: Fri Mar 18 16:45:08 2011 +0100
s3:smbd: access checks should not depend on share mode flags
metze
commit 3dc999e38ba2605b702e60ac0b9e91a14542f458
Author: Stefan Metzmacher <me...@samba.org>
Date: Mon Mar 21 13:59:27 2011 +0100
s4:ntvfs/posix: grant SEC_STD_DELETE if the parent grants
SEC_DIR_DELETE_CHILD
metze
-----------------------------------------------------------------------
Summary of changes:
libcli/security/access_check.c | 58 +++++++++++++++++----------------
source3/smbd/open.c | 1 -
source4/ntvfs/posix/pvfs_acl.c | 68 ++++++++++++++++++++++++++++++++++------
3 files changed, 88 insertions(+), 39 deletions(-)
Changeset truncated at 500 lines:
diff --git a/libcli/security/access_check.c b/libcli/security/access_check.c
index c5f89af..6bb64ae 100644
--- a/libcli/security/access_check.c
+++ b/libcli/security/access_check.c
@@ -112,9 +112,7 @@ static uint32_t access_check_max_allowed(const struct
security_descriptor *sd,
unsigned i;
if (security_token_has_sid(token, sd->owner_sid)) {
- granted |= SEC_STD_WRITE_DAC | SEC_STD_READ_CONTROL |
SEC_STD_DELETE;
- } else if (security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
- granted |= SEC_STD_DELETE;
+ granted |= SEC_STD_WRITE_DAC | SEC_STD_READ_CONTROL;
}
if (sd->dacl == NULL) {
@@ -171,7 +169,7 @@ NTSTATUS se_access_check(const struct security_descriptor
*sd,
access_desired |= access_check_max_allowed(sd, token);
access_desired &= ~SEC_FLAG_MAXIMUM_ALLOWED;
*access_granted = access_desired;
- bits_remaining = access_desired & ~SEC_STD_DELETE;
+ bits_remaining = access_desired;
DEBUG(10,("se_access_check: MAX desired = 0x%x, granted = 0x%x,
remaining = 0x%x\n",
orig_access_desired,
@@ -190,21 +188,13 @@ NTSTATUS se_access_check(const struct security_descriptor
*sd,
}
}
- /* a NULL dacl allows access */
- if ((sd->type & SEC_DESC_DACL_PRESENT) && sd->dacl == NULL) {
- *access_granted = access_desired;
- return NT_STATUS_OK;
- }
-
- /* the owner always gets SEC_STD_WRITE_DAC, SEC_STD_READ_CONTROL and
SEC_STD_DELETE */
- if ((bits_remaining &
(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL|SEC_STD_DELETE)) &&
+ /* the owner always gets SEC_STD_WRITE_DAC and SEC_STD_READ_CONTROL */
+ if ((bits_remaining & (SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL)) &&
security_token_has_sid(token, sd->owner_sid)) {
- bits_remaining &=
~(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL|SEC_STD_DELETE);
- }
- if ((bits_remaining & SEC_STD_DELETE) &&
- (security_token_has_privilege(token, SEC_PRIV_RESTORE))) {
- bits_remaining &= ~SEC_STD_DELETE;
+ bits_remaining &= ~(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL);
}
+
+ /* TODO: remove this, as it is file server specific */
if ((bits_remaining & SEC_RIGHTS_PRIV_RESTORE) &&
security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
bits_remaining &= ~(SEC_RIGHTS_PRIV_RESTORE);
@@ -214,6 +204,12 @@ NTSTATUS se_access_check(const struct security_descriptor
*sd,
bits_remaining &= ~(SEC_RIGHTS_PRIV_BACKUP);
}
+ /* a NULL dacl allows access */
+ if ((sd->type & SEC_DESC_DACL_PRESENT) && sd->dacl == NULL) {
+ *access_granted = access_desired;
+ return NT_STATUS_OK;
+ }
+
if (sd->dacl == NULL) {
goto done;
}
@@ -295,7 +291,7 @@ NTSTATUS sec_access_check_ds(const struct
security_descriptor *sd,
access_desired |= access_check_max_allowed(sd, token);
access_desired &= ~SEC_FLAG_MAXIMUM_ALLOWED;
*access_granted = access_desired;
- bits_remaining = access_desired & ~SEC_STD_DELETE;
+ bits_remaining = access_desired;
}
if (access_desired & SEC_FLAG_SYSTEM_SECURITY) {
@@ -307,6 +303,22 @@ NTSTATUS sec_access_check_ds(const struct
security_descriptor *sd,
}
}
+ /* the owner always gets SEC_STD_WRITE_DAC and SEC_STD_READ_CONTROL */
+ if ((bits_remaining & (SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL)) &&
+ security_token_has_sid(token, sd->owner_sid)) {
+ bits_remaining &= ~(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL);
+ }
+
+ /* TODO: remove this, as it is file server specific */
+ if ((bits_remaining & SEC_RIGHTS_PRIV_RESTORE) &&
+ security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
+ bits_remaining &= ~(SEC_RIGHTS_PRIV_RESTORE);
+ }
+ if ((bits_remaining & SEC_RIGHTS_PRIV_BACKUP) &&
+ security_token_has_privilege(token, SEC_PRIV_BACKUP)) {
+ bits_remaining &= ~(SEC_RIGHTS_PRIV_BACKUP);
+ }
+
/* a NULL dacl allows access */
if ((sd->type & SEC_DESC_DACL_PRESENT) && sd->dacl == NULL) {
*access_granted = access_desired;
@@ -314,16 +326,6 @@ NTSTATUS sec_access_check_ds(const struct
security_descriptor *sd,
return NT_STATUS_OK;
}
- /* the owner always gets SEC_STD_WRITE_DAC, SEC_STD_READ_CONTROL and
SEC_STD_DELETE */
- if ((bits_remaining &
(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL|SEC_STD_DELETE)) &&
- security_token_has_sid(token, sd->owner_sid)) {
- bits_remaining &=
~(SEC_STD_WRITE_DAC|SEC_STD_READ_CONTROL|SEC_STD_DELETE);
- }
- if ((bits_remaining & SEC_STD_DELETE) &&
- security_token_has_privilege(token, SEC_PRIV_RESTORE)) {
- bits_remaining &= ~SEC_STD_DELETE;
- }
-
if (sd->dacl == NULL) {
goto done;
}
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index ca84f1f..937117b 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3140,7 +3140,6 @@ static NTSTATUS create_file_unixpath(connection_struct
*conn,
if (lp_acl_check_permissions(SNUM(conn))
&& (create_disposition != FILE_CREATE)
- && (share_access & FILE_SHARE_DELETE)
&& (access_mask & DELETE_ACCESS)
&& (!(can_delete_file_in_directory(conn, smb_fname) ||
can_access_file_acl(conn, smb_fname, DELETE_ACCESS)))) {
diff --git a/source4/ntvfs/posix/pvfs_acl.c b/source4/ntvfs/posix/pvfs_acl.c
index 7a3002c..addd680 100644
--- a/source4/ntvfs/posix/pvfs_acl.c
+++ b/source4/ntvfs/posix/pvfs_acl.c
@@ -509,10 +509,10 @@ static bool pvfs_group_member(struct pvfs_state *pvfs,
gid_t gid)
If name is NULL then treat as a new file creation
*/
-NTSTATUS pvfs_access_check_unix(struct pvfs_state *pvfs,
- struct ntvfs_request *req,
- struct pvfs_filename *name,
- uint32_t *access_mask)
+static NTSTATUS pvfs_access_check_unix(struct pvfs_state *pvfs,
+ struct ntvfs_request *req,
+ struct pvfs_filename *name,
+ uint32_t *access_mask)
{
uid_t uid = geteuid();
uint32_t max_bits = SEC_RIGHTS_FILE_READ | SEC_FILE_ALL;
@@ -592,6 +592,7 @@ NTSTATUS pvfs_access_check(struct pvfs_state *pvfs,
struct xattr_NTACL *acl;
NTSTATUS status;
struct security_descriptor *sd;
+ bool allow_delete = false;
/* on SMB2 a blank access mask is always denied */
if (pvfs->ntvfs->ctx->protocol == PROTOCOL_SMB2 &&
@@ -603,6 +604,16 @@ NTSTATUS pvfs_access_check(struct pvfs_state *pvfs,
return NT_STATUS_ACCESS_DENIED;
}
+ if (*access_mask & SEC_FLAG_MAXIMUM_ALLOWED ||
+ *access_mask & SEC_STD_DELETE) {
+ status = pvfs_access_check_parent(pvfs, req,
+ name, SEC_DIR_DELETE_CHILD);
+ if (NT_STATUS_IS_OK(status)) {
+ allow_delete = true;
+ *access_mask &= ~SEC_STD_DELETE;
+ }
+ }
+
acl = talloc(req, struct xattr_NTACL);
if (acl == NULL) {
return NT_STATUS_NO_MEMORY;
@@ -617,7 +628,8 @@ NTSTATUS pvfs_access_check(struct pvfs_state *pvfs,
status = pvfs_acl_load(pvfs, name, -1, acl);
if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) {
talloc_free(acl);
- return pvfs_access_check_unix(pvfs, req, name, access_mask);
+ status = pvfs_access_check_unix(pvfs, req, name, access_mask);
+ goto done;
}
if (!NT_STATUS_IS_OK(status)) {
return status;
@@ -633,15 +645,18 @@ NTSTATUS pvfs_access_check(struct pvfs_state *pvfs,
/* check the acl against the required access mask */
status = se_access_check(sd, token, *access_mask, access_mask);
-
+ talloc_free(acl);
+done:
if (pvfs->ntvfs->ctx->protocol != PROTOCOL_SMB2) {
/* on SMB, this bit is always granted, even if not
asked for */
*access_mask |= SEC_FILE_READ_ATTRIBUTE;
}
- talloc_free(acl);
-
+ if (allow_delete) {
+ *access_mask |= SEC_STD_DELETE;
+ }
+
return status;
}
@@ -673,6 +688,8 @@ NTSTATUS pvfs_access_check_create(struct pvfs_state *pvfs,
{
struct pvfs_filename *parent;
NTSTATUS status;
+ uint32_t parent_mask;
+ bool allow_delete = false;
if (pvfs_read_only(pvfs, *access_mask)) {
return NT_STATUS_ACCESS_DENIED;
@@ -681,8 +698,35 @@ NTSTATUS pvfs_access_check_create(struct pvfs_state *pvfs,
status = pvfs_resolve_parent(pvfs, req, name, &parent);
NT_STATUS_NOT_OK_RETURN(status);
- status = pvfs_access_check_simple(pvfs, req, parent, SEC_DIR_ADD_FILE);
- NT_STATUS_NOT_OK_RETURN(status);
+ if (name->dos.attrib & FILE_ATTRIBUTE_DIRECTORY) {
+ parent_mask = SEC_DIR_ADD_SUBDIR;
+ } else {
+ parent_mask = SEC_DIR_ADD_FILE;
+ }
+ if (*access_mask & SEC_FLAG_MAXIMUM_ALLOWED ||
+ *access_mask & SEC_STD_DELETE) {
+ parent_mask |= SEC_DIR_DELETE_CHILD;
+ }
+
+ status = pvfs_access_check(pvfs, req, parent, &parent_mask);
+ if (NT_STATUS_IS_OK(status)) {
+ if (parent_mask & SEC_DIR_DELETE_CHILD) {
+ allow_delete = true;
+ }
+ } else if (NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
+ /*
+ * on ACCESS_DENIED we get the rejected bits
+ * remove the non critical SEC_DIR_DELETE_CHILD
+ * and check if something else was rejected.
+ */
+ parent_mask &= ~SEC_DIR_DELETE_CHILD;
+ if (parent_mask != 0) {
+ return NT_STATUS_ACCESS_DENIED;
+ }
+ status = NT_STATUS_OK;
+ } else {
+ return status;
+ }
if (*sd == NULL) {
status = pvfs_acl_inherited_sd(pvfs, req, req, parent,
container, sd);
@@ -707,6 +751,10 @@ NTSTATUS pvfs_access_check_create(struct pvfs_state *pvfs,
*access_mask |= SEC_FILE_READ_ATTRIBUTE;
}
+ if (allow_delete) {
+ *access_mask |= SEC_STD_DELETE;
+ }
+
return NT_STATUS_OK;
}
--
Samba Shared Repository
