[PATCH] Handle EACCES when fetching DOS attributes
Ralph Böhme
slow at samba.org
Mon Aug 7 14:29:32 UTC 2017
Hi!
Attached is a patch for bug 12944.
Patchset is already reviewed by Christof, so I'm going to push in 24 hours if
noone objects. :)
Fwiw: I tried to add a test for this, but unfortunately the POSIX permission
code insists on adding S_IRUSR to the requested permissions when trying to set
the SD for a file from the smbtorture test. As the test requires a file lacking
read access for the user and I couldn't come up with a means to achieve that
from within autobuild without a being shoehorned into a serious battle with the
POSIX ACL subsystem, I skipped the test.
-slow
-------------- next part --------------
From ac0f9caaacbe345f67e1cd61bd9f3937d66cf827 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 8 Jun 2017 19:05:48 +0200
Subject: [PATCH 1/3] s3/smbd: handling of failed DOS attributes reading
Only fall back to using UNIX modes if we get NOT_IMPLEMENTED. This is
exactly what we already do when setting DOS attributes.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12944
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Christof Schmitt <cs at samba.org>
---
source3/smbd/dosmode.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c
index 0ee6894..d53386e 100644
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -622,7 +622,12 @@ uint32_t dos_mode(connection_struct *conn, struct smb_filename *smb_fname)
/* Get the DOS attributes via the VFS if we can */
status = SMB_VFS_GET_DOS_ATTRIBUTES(conn, smb_fname, &result);
if (!NT_STATUS_IS_OK(status)) {
- result |= dos_mode_from_sbuf(conn, smb_fname);
+ /*
+ * Only fall back to using UNIX modes if we get NOT_IMPLEMENTED.
+ */
+ if (NT_STATUS_EQUAL(status, NT_STATUS_NOT_IMPLEMENTED)) {
+ result |= dos_mode_from_sbuf(conn, smb_fname);
+ }
}
if (conn->fs_capabilities & FILE_FILE_COMPRESSION) {
--
2.9.4
From 23dbe776bf91a9fba00af146228ded92c49f72ce Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 8 Jun 2017 19:10:20 +0200
Subject: [PATCH 2/3] s3/smbd: handle EACCES when fetching DOS attributes from
xattr
When trying to fetch the DOS attributes xattr via SMB_VFS_GETXATTR() if
the filesystem doesn't grant read access to the file the xattr read
request fails with EACCESS.
But according to MS-FSA 2.1.5.1.2.1 "Algorithm to Check Access to an
Existing File" FILE_LIST_DIRECTORY on a directory implies
FILE_READ_ATTRIBUTES for directory entries.
So if the user can open the parent directory for reading this implies
FILE_LIST_DIRECTORY and we can safely call SMB_VFS_GETXATTR() as root,
ensuring we can read the DOS attributes xattr.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12944
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Christof Schmitt <cs at samba.org>
---
source3/smbd/dosmode.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/source3/smbd/dosmode.c b/source3/smbd/dosmode.c
index d53386e..3181f2e 100644
--- a/source3/smbd/dosmode.c
+++ b/source3/smbd/dosmode.c
@@ -281,6 +281,42 @@ NTSTATUS get_ea_dos_attribute(connection_struct *conn,
sizeret = SMB_VFS_GETXATTR(conn, smb_fname,
SAMBA_XATTR_DOS_ATTRIB, attrstr,
sizeof(attrstr));
+ if (sizeret == -1 && errno == EACCES) {
+ int saved_errno = 0;
+
+ /*
+ * According to MS-FSA 2.1.5.1.2.1 "Algorithm to Check Access to
+ * an Existing File" FILE_LIST_DIRECTORY on a directory implies
+ * FILE_READ_ATTRIBUTES for directory entries. Being able to
+ * stat() a file implies FILE_LIST_DIRECTORY for the directory
+ * containing the file.
+ */
+
+ if (!VALID_STAT(smb_fname->st)) {
+ /*
+ * Safety net: dos_mode() already checks this, but as we
+ * become root based on this, add an additional layer of
+ * defense.
+ */
+ DBG_ERR("Rejecting root override, invalid stat [%s]\n",
+ smb_fname_str_dbg(smb_fname));
+ return NT_STATUS_ACCESS_DENIED;
+ }
+
+ become_root();
+ sizeret = SMB_VFS_GETXATTR(conn, smb_fname,
+ SAMBA_XATTR_DOS_ATTRIB,
+ attrstr,
+ sizeof(attrstr));
+ if (sizeret == -1) {
+ saved_errno = errno;
+ }
+ unbecome_root();
+
+ if (saved_errno != 0) {
+ errno = saved_errno;
+ }
+ }
if (sizeret == -1) {
DBG_INFO("Cannot get attribute "
"from EA on file %s: Error = %s\n",
--
2.9.4
From 79885f7a6b8880f5f99aae3f87226cbf9787f0b5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 8 Jun 2017 19:18:36 +0200
Subject: [PATCH 3/3] vfs_gpfs: handle EACCES when fetching DOS attributes from
xattr
When trying to fetch the DOS attributes via gpfswrap_get_winattrs_path()
if the filesystem doesn't grant READ_ATTR to the file the function fails
with EACCESS.
But according to MS-FSA 2.1.5.1.2.1 "Algorithm to Check Access to an
Existing File" FILE_LIST_DIRECTORY on a directory implies
FILE_READ_ATTRIBUTES for directory entries.
So if the user can open the parent directory for reading this implies
FILE_LIST_DIRECTORY and we can safely call gpfswrap_get_winattrs_path()
with DAC_OVERRIDE_CAPABILITY.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=12944
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Christof Schmitt <cs at samba.org>
---
source3/modules/vfs_gpfs.c | 69 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 68 insertions(+), 1 deletion(-)
diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
index a552cdd..b2c9244 100644
--- a/source3/modules/vfs_gpfs.c
+++ b/source3/modules/vfs_gpfs.c
@@ -1537,6 +1537,47 @@ static unsigned int vfs_gpfs_dosmode_to_winattrs(uint32_t dosmode)
return winattrs;
}
+static int get_dos_attr_with_capability(struct smb_filename *smb_fname,
+ struct gpfs_winattr *attr)
+{
+ int saved_errno = 0;
+ int ret;
+
+ /*
+ * According to MS-FSA 2.1.5.1.2.1 "Algorithm to Check Access to an
+ * Existing File" FILE_LIST_DIRECTORY on a directory implies
+ * FILE_READ_ATTRIBUTES for directory entries. Being able to stat() a
+ * file implies FILE_LIST_DIRECTORY for the directory containing the
+ * file.
+ */
+
+ if (!VALID_STAT(smb_fname->st)) {
+ /*
+ * Safety net: dos_mode() already checks this, but as we set
+ * DAC_OVERRIDE_CAPABILITY based on this, add an additional
+ * layer of defense.
+ */
+ DBG_ERR("Rejecting DAC override, invalid stat [%s]\n",
+ smb_fname_str_dbg(smb_fname));
+ errno = EACCES;
+ return -1;
+ }
+
+ set_effective_capability(DAC_OVERRIDE_CAPABILITY);
+
+ ret = gpfswrap_get_winattrs_path(smb_fname->base_name, attr);
+ if (ret == -1) {
+ saved_errno = errno;
+ }
+
+ drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
+
+ if (saved_errno != 0) {
+ errno = saved_errno;
+ }
+ return ret;
+}
+
static NTSTATUS vfs_gpfs_get_dos_attributes(struct vfs_handle_struct *handle,
struct smb_filename *smb_fname,
uint32_t *dosmode)
@@ -1559,7 +1600,9 @@ static NTSTATUS vfs_gpfs_get_dos_attributes(struct vfs_handle_struct *handle,
return SMB_VFS_NEXT_GET_DOS_ATTRIBUTES(handle, smb_fname,
dosmode);
}
-
+ if (ret == -1 && errno == EACCES) {
+ ret = get_dos_attr_with_capability(smb_fname, &attrs);
+ }
if (ret == -1) {
DBG_WARNING("Getting winattrs failed for %s: %s\n",
smb_fname->base_name, strerror(errno));
@@ -1595,6 +1638,30 @@ static NTSTATUS vfs_gpfs_fget_dos_attributes(struct vfs_handle_struct *handle,
return SMB_VFS_NEXT_FGET_DOS_ATTRIBUTES(handle, fsp, dosmode);
}
+ if (ret == -1 && errno == EACCES) {
+ int saved_errno = 0;
+
+ /*
+ * According to MS-FSA 2.1.5.1.2.1 "Algorithm to Check Access to
+ * an Existing File" FILE_LIST_DIRECTORY on a directory implies
+ * FILE_READ_ATTRIBUTES for directory entries. Being able to
+ * open a file implies FILE_LIST_DIRECTORY.
+ */
+
+ set_effective_capability(DAC_OVERRIDE_CAPABILITY);
+
+ ret = gpfswrap_get_winattrs(fsp->fh->fd, &attrs);
+ if (ret == -1) {
+ saved_errno = errno;
+ }
+
+ drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
+
+ if (saved_errno != 0) {
+ errno = saved_errno;
+ }
+ }
+
if (ret == -1) {
DBG_WARNING("Getting winattrs failed for %s: %s\n",
fsp->fsp_name->base_name, strerror(errno));
--
2.9.4
More information about the samba-technical
mailing list