Revert "vfs_acl_xattr.c: prefer capabilities over become_root"

This reverts commit 0e3836e3961f2b7c39173ce1023d3c92addef630.

With capabilities preferred over become_root() we failed to achieve
the basic goal of storing NT ACLs in xattrs using vfs_acl_xattr. This
is due to the fact that apart from CAP_DAC_OVERRIDE it is manadatory
to have CAP_SYS_ADMIN for write access to xattrs from security
namespace[1]. Despite the option to configure the xattr name within
the module we should not anticipate and miss to consider xattrs from
security namespace which is far more protected even with our default
name "security.NTACL".

Theorotically we could make it work by adding another capability on
top of existing ones. But given the functions designed around this
area we may not be able to come up with a cleaner change which can
handle the fallback mechanism to become_root(). Any failure to set
the very first capability would put us in become_root() path where
further capabilities are mostly not required. Thus reverting to old
behaviour to always become_root() until we have a cleaner approach
to handle the fallback while modifying multiple capabilities at once.

[1] https://www.man7.org/linux/man-pages/man7/xattr.7.html

Signed-off-by: Anoop C S <anoopcs@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
index ee247a312f7384c2b6aaccc3c397977636177863..1a3ab34d659d264b959875b6e340ed9ebdff5d02 100644 (file)
--- a/source3/modules/vfs_acl_xattr.c
+++ b/source3/modules/vfs_acl_xattr.c
@@ -46,12 +46,12 @@ static ssize_t getxattr_do(vfs_handle_struct *handle,
        ssize_t sizeret;
        int saved_errno = 0;
 
-       set_effective_capability(DAC_OVERRIDE_CAPABILITY);
+       become_root();
        sizeret = SMB_VFS_FGETXATTR(fsp, xattr_name, val, size);
        if (sizeret == -1) {
                saved_errno = errno;
        }
-       drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
+       unbecome_root();
 
        if (saved_errno != 0) {
                errno = saved_errno;
@@ -132,13 +132,13 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct *handle,
        DEBUG(10,("store_acl_blob_fsp: storing blob length %u on file %s\n",
                  (unsigned int)pblob->length, fsp_str_dbg(fsp)));
 
-       set_effective_capability(DAC_OVERRIDE_CAPABILITY);
+       become_root();
        ret = SMB_VFS_FSETXATTR(fsp, XATTR_NTACL_NAME,
                        pblob->data, pblob->length, 0);
        if (ret) {
                saved_errno = errno;
        }
-       drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
+       unbecome_root();
        if (ret) {
                DEBUG(5, ("store_acl_blob_fsp: setting attr failed for file %s"
                        "with error %s\n",
@@ -175,9 +175,9 @@ static int sys_acl_set_fd_xattr(vfs_handle_struct *handle,
                return 0;
        }
 
-       set_effective_capability(DAC_OVERRIDE_CAPABILITY);
+       become_root();
        SMB_VFS_FREMOVEXATTR(fsp, XATTR_NTACL_NAME);
-       drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
+       unbecome_root();
 
        return 0;
 }