[PATCHES] Add vfs_admin vfs module

Uri Simchoni uri at samba.org
Sun Oct 4 18:22:21 UTC 2015


Hi,

This patch set adds vfs_admin, a simple vfs module that fixes file 
ownership for admin users.
When a user is defined as an admin user, smbd runs as root, and files 
created by this user are
owned by root. This patch fixes that by adding a vfs module to change 
the ownership of created objects.

One thing worth noting is that for file creation I chose to override the 
NT-layer create_file function rather than the POSIX layer open, which 
would be more natural since this is a POSIX issue, created when running 
POSIX open() system call or something with similar semantics.

The reason for that is that when open returns, the fsp is still not 
initialized with the just-created file descriptor, and so I cannot use 
SMB_VFS_FCHOWN from within the open handler, and have to resort to 
fchown() (supporting only kernel file systems) or SMB_VFS_CHOWN (racy), 
or hack the fsp (...). The downside of it is having to deal with things 
that don't necessarily translate to POSIX file creation (streams, mkdir) 
and I hope I got that right. I have an alternative implementation that 
overrides the POSIX open instead, with above-mentioned limitations.

Review appreciated.

Thanks,
Uri
-------------- next part --------------
From a93f72288ce4b1f992a9a99ef766eecc1663507b Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 1 Oct 2015 22:53:32 +0300
Subject: [PATCH 1/2] VFS: add vfs_admin

vfs_admin is a VFS module that fixes file owner if the user is
an admin user. For admin users (users matching the per-share
"admin users" conf item), smbd runs as root, and files
created by those users are owned by root instead of by the
user.

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 source3/modules/vfs_admin.c   | 184 ++++++++++++++++++++++++++++++++++++++++++
 source3/modules/wscript_build |   8 ++
 source3/wscript               |   2 +-
 3 files changed, 193 insertions(+), 1 deletion(-)
 create mode 100644 source3/modules/vfs_admin.c

diff --git a/source3/modules/vfs_admin.c b/source3/modules/vfs_admin.c
new file mode 100644
index 0000000..db048ba
--- /dev/null
+++ b/source3/modules/vfs_admin.c
@@ -0,0 +1,184 @@
+/*
+ * admin VFS module. Fixes file ownership for files created by
+ * an admin user in the share.
+ *
+ * Copyright (C) Uri Simchoni, 2015
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "../source3/include/includes.h"
+#include "smbd/smbd.h"
+#include "smbd/globals.h"
+#include "auth.h"
+
+struct admin_data {
+	uid_t orig_uid;
+};
+
+static bool is_admin(vfs_handle_struct *handle)
+{
+	return handle->conn->session_info->unix_token->uid == sec_initial_uid();
+}
+
+static void chown_object(vfs_handle_struct *handle, const char *path)
+{
+	int rc;
+	struct admin_data *ctx;
+
+	SMB_VFS_HANDLE_GET_DATA(handle, ctx, struct admin_data, return );
+
+	rc = SMB_VFS_LCHOWN(handle->conn, path, ctx->orig_uid, -1);
+	if (rc != 0) {
+		DBG_DEBUG("Chowning '%s' failed. Error is '%s'\n", path,
+			  strerror(errno));
+	}
+}
+
+static int admin_connect(vfs_handle_struct *handle, const char *service,
+			 const char *user)
+{
+	int rc;
+	struct admin_data *ctx;
+	struct user_struct *vuser;
+
+	rc = SMB_VFS_NEXT_CONNECT(handle, service, user);
+	if (rc < 0) {
+		return rc;
+	}
+
+	ctx = talloc_zero(handle->conn, struct admin_data);
+	if (!ctx) {
+		DBG_ERR("talloc_zero() failed\n");
+		errno = ENOMEM;
+		return -1;
+	}
+
+	vuser = get_valid_user_struct(handle->conn->sconn, handle->conn->vuid);
+	if (vuser == NULL) {
+		DBG_ERR("No user found for vuid %llu\n",
+			(unsigned long long)handle->conn->vuid);
+		errno = EIO;
+		return -1;
+	}
+
+	ctx->orig_uid = vuser->session_info->unix_token->uid;
+
+	SMB_VFS_HANDLE_SET_DATA(handle, ctx, NULL, struct admin_data,
+				return -1);
+	return 0;
+}
+
+static int admin_mkdir(vfs_handle_struct *handle, const char *path, mode_t mode)
+{
+	int rc = SMB_VFS_NEXT_MKDIR(handle, path, mode);
+	if (is_admin(handle) && rc == 0) {
+		chown_object(handle, path);
+	}
+	return rc;
+}
+
+static NTSTATUS
+admin_create_file(struct vfs_handle_struct *handle, struct smb_request *req,
+		  uint16_t root_dir_fid, struct smb_filename *smb_fname,
+		  uint32_t access_mask, uint32_t share_access,
+		  uint32_t create_disposition, uint32_t create_options,
+		  uint32_t file_attributes, uint32_t oplock_request,
+		  struct smb2_lease *lease, uint64_t allocation_size,
+		  uint32_t private_flags, struct security_descriptor *sd,
+		  struct ea_list *ea_list, files_struct **result, int *pinfo,
+		  const struct smb2_create_blobs *in_context_blobs,
+		  struct smb2_create_blobs *out_context_blobs)
+{
+	NTSTATUS status;
+	int info, rc;
+	struct admin_data *ctx;
+
+	status = SMB_VFS_NEXT_CREATE_FILE(
+	    handle, req, root_dir_fid, smb_fname, access_mask, share_access,
+	    create_disposition, create_options, file_attributes, oplock_request,
+	    lease, allocation_size, private_flags, sd, ea_list, result, &info,
+	    in_context_blobs, out_context_blobs);
+	if (!NT_STATUS_IS_OK(status)) {
+		return status;
+	}
+
+	if (pinfo) {
+		*pinfo = info;
+	}
+
+	if (!is_admin(handle) || info != FILE_WAS_CREATED) {
+		return status;
+	}
+
+	if ((*result)->base_fsp != NULL) {
+		/* That's a stream, don't mess with it */
+		return status;
+	}
+
+	SMB_VFS_HANDLE_GET_DATA(handle, ctx, struct admin_data, return status);
+
+	rc = SMB_VFS_FCHOWN(*result, ctx->orig_uid, -1);
+	if (rc != 0) {
+		DBG_DEBUG("Chowning '%s' failed. Error is '%s'\n",
+			  smb_fname->base_name, strerror(errno));
+	}
+
+	return status;
+}
+
+static int admin_symlink(vfs_handle_struct *handle, const char *oldpath,
+			 const char *newpath)
+{
+	int rc = SMB_VFS_NEXT_SYMLINK(handle, oldpath, newpath);
+	if (is_admin(handle) && rc == 0) {
+		chown_object(handle, newpath);
+	}
+	return rc;
+}
+
+static int admin_mknod(vfs_handle_struct *handle, const char *path, mode_t mode,
+		       SMB_DEV_T dev)
+{
+	int rc = SMB_VFS_NEXT_MKNOD(handle, path, mode, dev);
+	if (is_admin(handle) && rc == 0) {
+		chown_object(handle, path);
+	}
+	return rc;
+}
+
+/* VFS operations structure */
+
+struct vfs_fn_pointers admin_fns = {
+    /* Disk operations */
+
+    .connect_fn = admin_connect,
+
+    /* Directory operations */
+
+    .mkdir_fn = admin_mkdir,
+
+    /* File operations */
+
+    /*	.open_fn = admin_open, */
+    .create_file_fn = admin_create_file,
+    .symlink_fn = admin_symlink,
+    .mknod_fn = admin_mknod,
+};
+
+static_decl_vfs;
+NTSTATUS vfs_admin_init(void)
+{
+	return smb_register_vfs(SMB_VFS_INTERFACE_VERSION, "admin", &admin_fns);
+}
diff --git a/source3/modules/wscript_build b/source3/modules/wscript_build
index fef412a..ccd9181 100644
--- a/source3/modules/wscript_build
+++ b/source3/modules/wscript_build
@@ -499,3 +499,11 @@ bld.SAMBA3_MODULE('vfs_vxfs',
                  init_function='',
                  internal_module=bld.SAMBA3_IS_STATIC_MODULE('vfs_vxfs'),
                  enabled=bld.SAMBA3_IS_ENABLED_MODULE('vfs_vxfs'))
+
+bld.SAMBA3_MODULE('vfs_admin',
+                 subsystem='vfs',
+                 source='vfs_admin.c',
+                 deps='samba-util',
+                 init_function='',
+                 internal_module=bld.SAMBA3_IS_STATIC_MODULE('vfs_admin'),
+                 enabled=bld.SAMBA3_IS_ENABLED_MODULE('vfs_admin'))
diff --git a/source3/wscript b/source3/wscript
index 9ff9c20..64c0b04 100644
--- a/source3/wscript
+++ b/source3/wscript
@@ -1610,7 +1610,7 @@ main() {
                                       vfs_smb_traffic_analyzer vfs_preopen vfs_catia
                                       vfs_media_harmony vfs_unityed_media vfs_fruit vfs_shell_snap
                                       vfs_commit vfs_worm vfs_crossrename vfs_linux_xfs_sgid
-                                      vfs_time_audit
+                                      vfs_time_audit vfs_admin
                                   '''))
     default_shared_modules.extend(TO_LIST('auth_script idmap_tdb2 idmap_script'))
     # these have broken dependencies
-- 
1.9.1


From 2228cb1c7bfa6604315cf00c6f0f0b66e16fb462 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Fri, 2 Oct 2015 23:27:12 +0300
Subject: [PATCH 2/2] vfs_admin: add documentation

Signed-off-by: Uri Simchoni <uri at samba.org>
---
 docs-xml/manpages/vfs_admin.8.xml | 84 +++++++++++++++++++++++++++++++++++++++
 docs-xml/wscript_build            |  1 +
 source3/modules/vfs_admin.c       | 32 ++++++++++-----
 3 files changed, 106 insertions(+), 11 deletions(-)
 create mode 100644 docs-xml/manpages/vfs_admin.8.xml

diff --git a/docs-xml/manpages/vfs_admin.8.xml b/docs-xml/manpages/vfs_admin.8.xml
new file mode 100644
index 0000000..8a50fc4
--- /dev/null
+++ b/docs-xml/manpages/vfs_admin.8.xml
@@ -0,0 +1,84 @@
+<?xml version="1.0" encoding="iso-8859-1"?>
+<!DOCTYPE refentry PUBLIC "-//Samba-Team//DTD DocBook V4.2-Based Variant V1.0//EN" "http://www.samba.org/samba/DTD/samba-doc">
+<refentry id="vfs_admin.8">
+
+<refmeta>
+	<refentrytitle>vfs_admin</refentrytitle>
+	<manvolnum>8</manvolnum>
+	<refmiscinfo class="source">Samba</refmiscinfo>
+	<refmiscinfo class="manual">System Administration tools</refmiscinfo>
+	<refmiscinfo class="version">4.3</refmiscinfo>
+</refmeta>
+
+
+<refnamediv>
+	<refname>vfs_admin</refname>
+	<refpurpose>Fix file and directory ownership for admin users</refpurpose>
+</refnamediv>
+
+<refsynopsisdiv>
+	<cmdsynopsis>
+		<command>vfs objects = admin</command>
+	</cmdsynopsis>
+</refsynopsisdiv>
+
+<refsect1>
+	<title>DESCRIPTION</title>
+
+	<para>This VFS module is part of the
+	<citerefentry><refentrytitle>samba</refentrytitle>
+	<manvolnum>7</manvolnum></citerefentry> suite.</para>
+
+	<para>This VFS module fixes file ownership for admin users.
+	</para>
+
+	<para><citerefentry><refentrytitle>smbd</refentrytitle>
+	<manvolnum>8</manvolnum></citerefentry> supports the concept of
+	"admin users" - users who are
+	granted administrative privileges on the share.
+	When serving an admin user, smbd runs as root rather than the as
+	the user. A consequence is that files created by this user
+	are owned by "root". vfs_admin module fixes this issue by changing
+	owner of the created object (be it a file, a directory, or a special
+	UNIX file) to the UID of the admin user.
+	</para>
+
+	<para>This module is stackable, and uses the VFS to do the actual
+	changing of ownership (i.e. it supports both user-space and kernel
+	file systems). By its purpose and nature it does assume the
+	underlying file system has POSIX semantics.</para>
+
+</refsect1>
+
+
+<refsect1>
+	<title>EXAMPLES</title>
+
+	<para>Use vfs_admin to fix file ownership in a share:</para>
+
+<programlisting>
+        <smbconfsection name="[share]"/>
+	<smbconfoption name="admin users">"SAMDOM\domain admins"</smbconfoption>
+	<smbconfoption name="vfs objects">admin</smbconfoption>
+</programlisting>
+
+</refsect1>
+
+<refsect1>
+	<title>VERSION</title>
+
+	<para>This man page is correct for version 4.4.0 of the Samba suite.
+	</para>
+</refsect1>
+
+<refsect1>
+	<title>AUTHOR</title>
+
+	<para>The original Samba software and related utilities
+	were created by Andrew Tridgell. Samba is now developed
+	by the Samba Team as an Open Source project similar
+	to the way the Linux kernel is developed.</para>
+
+</refsect1>
+
+</refentry>
diff --git a/docs-xml/wscript_build b/docs-xml/wscript_build
index 568eba1..1ebd700 100644
--- a/docs-xml/wscript_build
+++ b/docs-xml/wscript_build
@@ -86,6 +86,7 @@ manpages='''
          manpages/vfs_unityed_media.8
          manpages/vfs_worm.8
          manpages/vfs_xattr_tdb.8
+         manpages/vfs_admin.8
          manpages/vfstest.1
          manpages/wbinfo.1
          manpages/winbindd.8
diff --git a/source3/modules/vfs_admin.c b/source3/modules/vfs_admin.c
index db048ba..ea8ad17 100644
--- a/source3/modules/vfs_admin.c
+++ b/source3/modules/vfs_admin.c
@@ -40,10 +40,8 @@ static void chown_object(vfs_handle_struct *handle, const char *path)
 	SMB_VFS_HANDLE_GET_DATA(handle, ctx, struct admin_data, return );
 
 	rc = SMB_VFS_LCHOWN(handle->conn, path, ctx->orig_uid, -1);
-	if (rc != 0) {
-		DBG_DEBUG("Chowning '%s' failed. Error is '%s'\n", path,
-			  strerror(errno));
-	}
+	DBG_DEBUG("Chowning '%s' to %u .. %s\n", path, ctx->orig_uid,
+		  rc == 0 ? "OK" : strerror(errno));
 }
 
 static int admin_connect(vfs_handle_struct *handle, const char *service,
@@ -118,22 +116,34 @@ admin_create_file(struct vfs_handle_struct *handle, struct smb_request *req,
 		*pinfo = info;
 	}
 
-	if (!is_admin(handle) || info != FILE_WAS_CREATED) {
+	DBG_DEBUG("checking whether to fix owner of %s\n",
+		  smb_fname_str_dbg(smb_fname));
+
+	if (!is_admin(handle)) {
+		DBG_DEBUG("not admin\n");
+		return status;
+	}
+
+	if (info != FILE_WAS_CREATED) {
+		DBG_DEBUG("not new - keep old owner\n");
 		return status;
 	}
 
-	if ((*result)->base_fsp != NULL) {
-		/* That's a stream, don't mess with it */
+	if ((*result)->is_directory) {
+		DBG_DEBUG("directory - handled by mkdir\n");
+		return status;
+	}
+
+	if ((*result)->fh->fd == -1) {
+		DBG_DEBUG("no FD (a stream?)\n");
 		return status;
 	}
 
 	SMB_VFS_HANDLE_GET_DATA(handle, ctx, struct admin_data, return status);
 
 	rc = SMB_VFS_FCHOWN(*result, ctx->orig_uid, -1);
-	if (rc != 0) {
-		DBG_DEBUG("Chowning '%s' failed. Error is '%s'\n",
-			  smb_fname->base_name, strerror(errno));
-	}
+	DBG_DEBUG("Chowning '%s' to %u .. %s\n", smb_fname_str_dbg(smb_fname),
+		  ctx->orig_uid, rc == 0 ? "OK" : strerror(errno));
 
 	return status;
 }
-- 
1.9.1



More information about the samba-technical mailing list