[PATCHES] add an option to inherit only the UNIX owner
Jeremy Allison
jra at samba.org
Tue Aug 9 23:00:06 UTC 2016
On Mon, Aug 08, 2016 at 08:15:19AM +0300, Uri Simchoni wrote:
> Hi,
>
> The attached patch (and associated test) add granularity to the "inherit
> owner" parameter - in addition to today's behavior of inheriting both
> the UNIX and the Windows owner of the file, it adds the option of
> inheriting only the UNIX owner. This facilitates emulation of something
> very similar to folder quotas.
>
> With the use of acl_xattr and acl_xattr:ignore system acls, the security
> of the file access can be governed only by the Windows security
> descriptor and enforced only by smbd, thus freeing the unix owner for
> other purposes.
>
> One such use is for quota enforcement - the unix owner can be set on a
> parent directory, and with the help of unix-only inheritance, propagated
> to subdirectories and files, thereby causing all files created in that
> directory to consume that unix user's quota.
>
> Review appreciated,
> Uri.
OK, couple of changes then I think I'm good with it.
1). Please use INHERIT_OWNER_WINDOWS_AND_UNIX, not INHERIT_OWNER_WINDOWS_N_UNIX.
INHERIT_OWNER_WINDOWS_N_UNIX reminds me of "Fish 'n' Chips" restaurants
from holidays in my youth. We're better than that - we can spell :-).
2). Can you add "yes" as an enum synonym for INHERIT_OWNER_WINDOWS_AND_UNIX ?
That way people who currently have this set in their smb.conf files won't
get any surprises when they upgrade. "yes" == INHERIT_OWNER_WINDOWS_AND_UNIX
describes the current behavior.
Other than that, "Reviewed-by: Jeremy Allison <jra at samba.org>
> From f54ba96370b80688d44285c6ee0b213b08b3b0d9 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Tue, 2 Aug 2016 09:37:00 +0300
> Subject: [PATCH 1/2] smbd: add an option to inherit only the UNIX owner
>
> This can be used to emulate folder quotas, as explained in the
> modified manpage.
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> docs-xml/smbdotconf/security/inheritowner.xml | 37 +++++++++++++++++++++++++--
> lib/param/loadparm.h | 7 +++++
> lib/param/param_table.c | 5 ++++
> source3/smbd/open.c | 7 ++---
> source3/smbd/posix_acls.c | 8 ++++++
> 5 files changed, 59 insertions(+), 5 deletions(-)
>
> diff --git a/docs-xml/smbdotconf/security/inheritowner.xml b/docs-xml/smbdotconf/security/inheritowner.xml
> index ab7da57..e2dbadc 100644
> --- a/docs-xml/smbdotconf/security/inheritowner.xml
> +++ b/docs-xml/smbdotconf/security/inheritowner.xml
> @@ -1,6 +1,7 @@
> <samba:parameter name="inherit owner"
> context="S"
> - type="boolean"
> + type="enum"
> + enumlist="enum_inherit_owner_vals"
> xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
> <description>
> <para>The ownership of new files and directories
> @@ -8,11 +9,43 @@
> This option allows the Samba administrator to specify that
> the ownership for new files and directories should be controlled
> by the ownership of the parent directory.</para>
> -
> +
> + <para>Valid options are:</para>
> + <itemizedlist>
> + <listitem><para><constant>no</constant> -
> + Both the Windows (SID) owner and the UNIX (uid) owner of the file are
> + governed by the identity of the user that created the file.
> + </para></listitem>
> +
> + <listitem><para><constant>windows and unix</constant> -
> + The Windows (SID) owner and the UNIX (uid) owner of new files and
> + directories are set to the respective owner of the parent directory.
> + </para></listitem>
> +
> + <listitem><para><constant>unix only</constant> -
> + Only the UNIX owner is set to the UNIX owner of the parent directory.
> + </para></listitem>
> + </itemizedlist>
> +
> <para>Common scenarios where this behavior is useful is in
> implementing drop-boxes, where users can create and edit files but
> not delete them and ensuring that newly created files in a user's
> roaming profile directory are actually owned by the user.</para>
> +
> + <para>The <constant>unix only</constant> option effectively
> + breaks the tie between the Windows owner of a file and the
> + UNIX owner. As a logical consequence, in this mode,
> + setting the the Windows owner of a file does not modify the UNIX
> + owner. Using this mode should typically be combined with a
> + backing store that can emulate the full NT ACL model without
> + affecting the POSIX permissions, such as the acl_xattr
> + VFS module, coupled with
> + <smbconfoption name="acl_xattr:ignore system acls">yes</smbconfoption>.
> + This can be used to emulate folder quotas, when files are
> + exposed only via SMB (without UNIX extensions).
> + The UNIX owner of a directory is locally set
> + and inherited by all subdirectories and files, and they all
> + consume the same quota.</para>
> </description>
>
> <related>inherit permissions</related>
> diff --git a/lib/param/loadparm.h b/lib/param/loadparm.h
> index aa256c1..e525ea7 100644
> --- a/lib/param/loadparm.h
> +++ b/lib/param/loadparm.h
> @@ -225,6 +225,13 @@ enum mapreadonly_options {MAP_READONLY_NO, MAP_READONLY_YES, MAP_READONLY_PERMIS
> /* case handling */
> enum case_handling {CASE_LOWER,CASE_UPPER};
>
> +/* inherit owner options */
> +enum inheritowner_options {
> + INHERIT_OWNER_NO,
> + INHERIT_OWNER_WINDOWS_N_UNIX,
> + INHERIT_OWNER_UNIX_ONLY
> +};
> +
> /*
> * Default passwd chat script.
> */
> diff --git a/lib/param/param_table.c b/lib/param/param_table.c
> index d8d9144..0f408e4 100644
> --- a/lib/param/param_table.c
> +++ b/lib/param/param_table.c
> @@ -299,6 +299,11 @@ static const struct enum_list enum_case[] = {
> {-1, NULL}
> };
>
> +static const struct enum_list enum_inherit_owner_vals[] = {
> + {INHERIT_OWNER_NO, "no"},
> + {INHERIT_OWNER_WINDOWS_N_UNIX, "windows and unix"},
> + {INHERIT_OWNER_UNIX_ONLY, "unix only"},
> + {-1, NULL}};
>
> /* Note: We do not initialise the defaults union - it is not allowed in ANSI C
> *
> diff --git a/source3/smbd/open.c b/source3/smbd/open.c
> index 2ae6f83..d763ef2 100644
> --- a/source3/smbd/open.c
> +++ b/source3/smbd/open.c
> @@ -930,7 +930,7 @@ static NTSTATUS open_file(files_struct *fsp,
> }
>
> /* Change the owner if required. */
> - if (lp_inherit_owner(SNUM(conn))) {
> + if (lp_inherit_owner(SNUM(conn)) != INHERIT_OWNER_NO) {
> change_file_owner_to_parent(conn, parent_dir,
> fsp);
> need_re_stat = true;
> @@ -3375,7 +3375,7 @@ static NTSTATUS mkdir_internal(connection_struct *conn,
> }
>
> /* Change the owner if required. */
> - if (lp_inherit_owner(SNUM(conn))) {
> + if (lp_inherit_owner(SNUM(conn)) != INHERIT_OWNER_NO) {
> change_dir_owner_to_parent(conn, parent_dir,
> smb_dname->base_name,
> &smb_dname->st);
> @@ -4017,7 +4017,8 @@ static NTSTATUS inherit_new_acl(files_struct *fsp)
> const struct dom_sid *group_sid = NULL;
> uint32_t security_info_sent = (SECINFO_OWNER | SECINFO_GROUP | SECINFO_DACL);
> struct security_token *token = fsp->conn->session_info->security_token;
> - bool inherit_owner = lp_inherit_owner(SNUM(fsp->conn));
> + bool inherit_owner =
> + (lp_inherit_owner(SNUM(fsp->conn)) == INHERIT_OWNER_WINDOWS_N_UNIX);
> bool inheritable_components = false;
> bool try_builtin_administrators = false;
> const struct dom_sid *BA_U_sid = NULL;
> diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
> index c575568..e8e819c 100644
> --- a/source3/smbd/posix_acls.c
> +++ b/source3/smbd/posix_acls.c
> @@ -3754,6 +3754,14 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32_t security_info_sent, const struct
> security_info_sent &= ~SECINFO_GROUP;
> }
>
> + /* If UNIX owner is inherited and Windows isn't, then
> + * setting the UNIX owner based on Windows owner conflicts
> + * with the inheritance rule
> + */
> + if (lp_inherit_owner(SNUM(conn)) == INHERIT_OWNER_UNIX_ONLY) {
> + security_info_sent &= ~SECINFO_OWNER;
> + }
> +
> status = unpack_nt_owners( conn, &user, &grp, security_info_sent, psd);
> if (!NT_STATUS_IS_OK(status)) {
> return status;
> --
> 2.5.5
>
>
> From 21fbeb5b89a10c4254c487d2ff662a872e25ecfa Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Sun, 7 Aug 2016 21:11:14 +0300
> Subject: [PATCH 2/2] selftest: add tests to cover "inherit owner"
>
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
> selftest/target/Samba3.pm | 9 ++
> source3/script/tests/test_inherit_owner.sh | 158 +++++++++++++++++++++++++++++
> source3/selftest/tests.py | 3 +
> 3 files changed, 170 insertions(+)
> create mode 100755 source3/script/tests/test_inherit_owner.sh
>
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index e8f35e3..0678f64 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -671,6 +671,15 @@ sub setup_fileserver($$)
> path = $share_dir
> comment = ignore system acls
> acl_xattr:ignore system acls = yes
> +[inherit_owner]
> + path = $share_dir
> + comment = inherit owner
> + inherit owner = windows and unix
> +[inherit_owner_u]
> + path = $share_dir
> + comment = inherit only unix owner
> + inherit owner = unix only
> + acl_xattr:ignore system acls = yes
> ";
>
> my $vars = $self->provision($path,
> diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh
> new file mode 100755
> index 0000000..4a7ed76
> --- /dev/null
> +++ b/source3/script/tests/test_inherit_owner.sh
> @@ -0,0 +1,158 @@
> +#!/bin/sh
> +
> +# this tests "inherit owner" config parameter
> +# currently needs to run in SMB1 mode, because it uses UNIX
> +# extensions to fetch the UNIX owner of a file.
> +
> +if [ $# -lt 9 ]; then
> +cat <<EOF
> +Usage: $0 SERVER USERNAME PASSWORD PREFIX SMBCLIENT SMBCACLS SHARE INH_WIN INH_UNIX <additional args>
> +EOF
> +exit 1;
> +fi
> +
> +SERVER="$1"
> +USERNAME="$2"
> +PASSWORD="$3"
> +PREFIX="$4"
> +SMBCLIENT="$5"
> +SMBCACLS="$6"
> +SHARE="$7"
> +INH_WIN="$8"
> +INH_UNIX="$9"
> +shift 9
> +ADDARGS="$*"
> +SMBCLIENT="$VALGRIND ${SMBCLIENT} ${ADDARGS}"
> +SMBCACLS="$VALGRIND ${SMBCACLS} ${ADDARGS}"
> +
> +incdir=`dirname $0`/../../../testprogs/blackbox
> +. $incdir/subunit.sh
> +
> +create_file() {
> + local share=$1
> + local fname=$2
> + local rem_dirname=$(dirname $fname)
> + local bname=$(basename $fname)
> + touch $PREFIX/$bname
> + $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; rm $bname" 2>/dev/null
> + $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; ls" 2>/dev/null | grep "$bname" && exit 1
> + $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "lcd $PREFIX; cd $rem_dirname; put $bname" 2>/dev/null || exit 1
> +}
> +
> +create_dir() {
> + local share=$1
> + local dname=$2
> + local rem_dirname=$(dirname $dname)
> + local bname=$(basename $dname)
> + $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; rmdir $bname" 2>/dev/null
> + $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; ls" 2>/dev/null | grep "$dname" && exit 1
> + $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; mkdir $bname" 2>/dev/null || exit 1
> +}
> +
> +cleanup_file() {
> + local share=$1
> + local fname=$2
> + local rem_dirname=$(dirname $fname)
> + local bname=$(basename $fname)
> + $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; rm $bname" 2>/dev/null || exit 1
> +}
> +
> +cleanup_dir() {
> + local share=$1
> + local dname=$2
> + local rem_dirname=$(dirname $dname)
> + local bname=$(basename $dname)
> + $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; rmdir $bname" 2>/dev/null || exit 1
> +}
> +
> +set_win_owner() {
> + local share=$1
> + local fname=$2
> + local owner=$3
> + $SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD -C $owner 2>/dev/null || exit 1
> +}
> +
> +unix_owner_id_is() {
> + local share=$1
> + local fname=$2
> + local expected_id=$3
> + local actual_id
> + actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "getfacl $fname" 2>/dev/null | sed -rn 's/^# owner: (.*)/\1/p')
> + if ! test "x$actual_id" = "x$expected_id" ; then
> + echo "Actual uid of $share/$fname is [$actual_id] expected [$expected_id]"
> + exit 1
> + fi
> +}
> +
> +get_unix_id() {
> + local user=$1
> + local ent
> + ent=$(getent passwd $user) || exit 1
> + echo "$ent" | awk -F: '{print $3}'
> +}
> +
> +win_owner_is() {
> + local share=$1
> + local fname=$2
> + local expected_owner=$3
> + local actual_owner
> + actual_owner=$($SMBCACLS //$SERVER/$share $fname -U $USERNAME%$PASSWORD 2>/dev/null | sed -rn 's/^OWNER:(.*)/\1/p')
> + if ! test "x$actual_owner" = "x$expected_owner" ; then
> + echo "Actual owner of $share/$fname is [$actual_owner] expected [$expected_owner]"
> + exit 1
> + fi
> +}
> +
> +default_uid=$(get_unix_id $USERNAME)
> +alt_uid=$(get_unix_id force_user)
> +
> +if [ "$INH_WIN" == "0" ] && [ "$INH_UNIX" == "0" ] ; then
> + #default - file owned by creator, change-owner modifies both
> + WIN_OWNER_AFTER_CREATE="$SERVER/$USERNAME"
> + UNIX_OWNER_AFTER_CREATE=$(get_unix_id $USERNAME)
> + WIN_OWNER_AFTER_CHOWN="$SERVER/smbget_user"
> + UNIX_OWNER_AFTER_CHOWN=$(get_unix_id smbget_user)
> + TEST_LABEL="default"
> +elif [ "$INH_WIN" == "1" ] && [ "$INH_UNIX" == "1" ] ; then
> + #inherit owner=windows and unix - file owned by parent
> + #owner, change-owner modifies both
> + WIN_OWNER_AFTER_CREATE="$SERVER/force_user"
> + UNIX_OWNER_AFTER_CREATE=$(get_unix_id force_user)
> + WIN_OWNER_AFTER_CHOWN="$SERVER/smbget_user"
> + UNIX_OWNER_AFTER_CHOWN=$(get_unix_id smbget_user)
> + TEST_LABEL="both"
> +elif [ "$INH_WIN" == "0" ] && [ "$INH_UNIX" == "1" ] ; then
> + #inherit owner=unix only - windows owner is creator,
> + #unix owner inherited, upon change-owner only windows
> + #owner is changed
> + WIN_OWNER_AFTER_CREATE="$SERVER/$USERNAME"
> + UNIX_OWNER_AFTER_CREATE=$(get_unix_id force_user)
> + WIN_OWNER_AFTER_CHOWN="$SERVER/smbget_user"
> + UNIX_OWNER_AFTER_CHOWN=$(get_unix_id force_user)
> + TEST_LABEL="unix"
> +else
> + echo "Unknown combination INH_WIN=$INH_WIN INH_UNIX=$INH_UNIX"
> + exit 1
> +fi
> +
> +# SETUP
> +testit "$TEST_LABEL - setup root dir" create_dir tmp tmp.$$
> +testit "$TEST_LABEL - assign default ACL" $SMBCACLS //$SERVER/tmp tmp.$$ -U $USERNAME%$PASSWORD -S "REVISION:1,OWNER:$SERVER\force_user,GROUP:$SERVER\domusers,ACL:Everyone:ALLOWED/0x3/FULL" 2>/dev/null
> +# END SETUP
> +
> +testit "$TEST_LABEL - create subdir under root" create_dir $SHARE tmp.$$/subdir
> +testit "$TEST_LABEL - verify subdir win owner" win_owner_is $SHARE tmp.$$/subdir "$WIN_OWNER_AFTER_CREATE"
> +testit "$TEST_LABEL - verify subdir unix owner" unix_owner_id_is $SHARE tmp.$$/subdir $UNIX_OWNER_AFTER_CREATE
> +testit "$TEST_LABEL - create file under root" create_file $SHARE tmp.$$/afile
> +testit "$TEST_LABEL - verify file win owner" win_owner_is $SHARE tmp.$$/afile "$WIN_OWNER_AFTER_CREATE"
> +testit "$TEST_LABEL - verify file unix owner" unix_owner_id_is $SHARE tmp.$$/afile $UNIX_OWNER_AFTER_CREATE
> +testit "$TEST_LABEL - change dir owner" set_win_owner $SHARE tmp.$$/subdir "$SERVER\smbget_user"
> +testit "$TEST_LABEL - verify subdir win owner after change" win_owner_is $SHARE tmp.$$/subdir "$WIN_OWNER_AFTER_CHOWN"
> +testit "$TEST_LABEL - verify subdir unix owner after change" unix_owner_id_is $SHARE tmp.$$/subdir $UNIX_OWNER_AFTER_CHOWN
> +testit "$TEST_LABEL - change file owner" set_win_owner $SHARE tmp.$$/afile "$SERVER\smbget_user"
> +testit "$TEST_LABEL - verify file win owner after change" win_owner_is $SHARE tmp.$$/afile "$WIN_OWNER_AFTER_CHOWN"
> +testit "$TEST_LABEL - verify file unix owner after change" unix_owner_id_is $SHARE tmp.$$/afile $UNIX_OWNER_AFTER_CHOWN
> +testit "$TEST_LABEL - cleanup subdir" cleanup_dir $SHARE tmp.$$/subdir
> +testit "$TEST_LABEL - cleanup file" cleanup_file $SHARE tmp.$$/afile
> +testit "$TEST_LABEL - cleanup root" cleanup_dir $SHARE tmp.$$
> +
> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
> index 4736ebc..23fb37d 100755
> --- a/source3/selftest/tests.py
> +++ b/source3/selftest/tests.py
> @@ -193,6 +193,9 @@ for env in ["fileserver"]:
> plantestsuite("samba3.blackbox.netshareenum (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_shareenum.sh"), '$SERVER', '$USERNAME', '$PASSWORD', rpcclient])
> plantestsuite("samba3.blackbox.acl_xattr (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_acl_xattr.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls])
> plantestsuite("samba3.blackbox.smb2.not_casesensitive (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_smb2_not_casesensitive.sh"), '//$SERVER/tmp', '$SERVER_IP', '$USERNAME', '$PASSWORD', '$LOCAL_PATH', smbclient3])
> + plantestsuite("samba3.blackbox.inherit_owner.default(%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'tmp', '0', '0', '-m', 'NT1'])
> + plantestsuite("samba3.blackbox.inherit_owner.full (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'inherit_owner', '1', '1', '-m', 'NT1'])
> + plantestsuite("samba3.blackbox.inherit_owner.unix (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', '$USERNAME', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'inherit_owner_u', '0', '1', '-m', 'NT1'])
>
> #
> # tar command tests
> --
> 2.5.5
>
More information about the samba-technical
mailing list