symlink owner question
Uri Simchoni
uri at samba.org
Tue Apr 25 19:10:06 UTC 2017
Hi,
Can anyone think of a case where the owner of a symlink matters, that
is, suppose the user creates a symlink via SMB (POSIX extensions), and
the resulting link owned by the wrong user.
We have such behavior if:
1. The user is in "admin users" --> smbd runs as root and link owned by
root.
2. "inherit owner" is enabled - the link has the creator's owner, not
the inherited owner.
*if* it matters, I can't think of a way of reliably fixing it:
- lchown is a bit racy because the symlink may have been superseded with
something else.
- fchown - the only way I found for opening the symlink is using O_PATH,
and that doesn't support fchown (documented and experimentally verified).
I have produced a failing test to demonstrate the issue (see attached),
but then got stuck with fixing it :(, so perhaps it's better to declare
it as a non-issue....
Thoughts?
Uri.
-------------- next part --------------
From db6fe61486c30afb4c931270e367834d43be70cb Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 25 Apr 2017 10:06:59 +0300
Subject: [PATCH 1/8] selftest: harden cleanup in test_inherit_owner.sh
Ensure we properly clean up files and folders created in
the test.
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/script/tests/test_inherit_owner.sh | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh
index 5146d40..ceda14a 100755
--- a/source3/script/tests/test_inherit_owner.sh
+++ b/source3/script/tests/test_inherit_owner.sh
@@ -55,6 +55,8 @@ cleanup_file() {
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
+ $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; ls" 2>/dev/null | grep "$bname" && exit 1
+ echo "$bname successfully removed"
}
cleanup_dir() {
@@ -63,6 +65,8 @@ cleanup_dir() {
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
+ $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; ls" 2>/dev/null | grep "$dname" && exit 1
+ echo "$bname successfully removed"
}
set_win_owner() {
--
2.9.3
From fcb9c3180390c950d031e3e62138b34bdb5bca71 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 25 Apr 2017 10:10:13 +0300
Subject: [PATCH 2/8] selftest: use stat instead of getfacl in
test_inherit_owner.sh
This is in preparation for getting the owner of symlinks
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/script/tests/test_inherit_owner.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh
index ceda14a..48989e3 100755
--- a/source3/script/tests/test_inherit_owner.sh
+++ b/source3/script/tests/test_inherit_owner.sh
@@ -81,7 +81,7 @@ unix_owner_id_is() {
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')
+ actual_id=$($SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "stat $fname" 2>/dev/null | sed -rn 's/.*Uid: ([^[:space:]]*)[[:space:]].*/\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
--
2.9.3
From 94c56c7959dc623f3e223e5d0d5ddddac5a29fd7 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 25 Apr 2017 10:18:46 +0300
Subject: [PATCH 3/8] selftest: add a setup user to test_inherit_owner
Use a separate user for setting up the root folder,
this is required because the parent folder has no write
permissions for arbitrary users by default.
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/script/tests/test_inherit_owner.sh | 26 ++++++++++++++++++--------
source3/selftest/tests.py | 6 +++---
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh
index 48989e3..2079737 100755
--- a/source3/script/tests/test_inherit_owner.sh
+++ b/source3/script/tests/test_inherit_owner.sh
@@ -4,9 +4,9 @@
# currently needs to run in SMB1 mode, because it uses UNIX
# extensions to fetch the UNIX owner of a file.
-if [ $# -lt 9 ]; then
+if [ $# -lt 10 ]; then
cat <<EOF
-Usage: $0 SERVER USERNAME PASSWORD PREFIX SMBCLIENT SMBCACLS SHARE INH_WIN INH_UNIX <additional args>
+Usage: $0 SERVER USERNAME PASSWORD PREFIX SMBCLIENT SMBCACLS SHARE INH_WIN INH_UNIX SETUP_USERNAME <additional args>
EOF
exit 1;
fi
@@ -20,10 +20,14 @@ SMBCACLS="$6"
SHARE="$7"
INH_WIN="$8"
INH_UNIX="$9"
-shift 9
+SETUP_USERNAME="${10}"
+shift 10
ADDARGS="$*"
SMBCLIENT="$VALGRIND ${SMBCLIENT} ${ADDARGS}"
SMBCACLS="$VALGRIND ${SMBCACLS} ${ADDARGS}"
+if [ -z "$SETUP_USERNAME" ] ; then
+SETUP_USERNAME=$USERNAME
+fi
incdir=`dirname $0`/../../../testprogs/blackbox
. $incdir/subunit.sh
@@ -42,11 +46,17 @@ create_file() {
create_dir() {
local share=$1
local dname=$2
+ local user
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
+ if [ -n "$3" ] ; then
+ user=$3
+ else
+ user=$USERNAME
+ fi
+ $SMBCLIENT //$SERVER/$share -U $user%$PASSWORD -c "cd $rem_dirname; rmdir $bname" 2>/dev/null
+ $SMBCLIENT //$SERVER/$share -U $user%$PASSWORD -c "cd $rem_dirname; ls" 2>/dev/null | grep "$dname" && exit 1
+ $SMBCLIENT //$SERVER/$share -U $user%$PASSWORD -c "cd $rem_dirname; mkdir $bname" 2>/dev/null || exit 1
}
cleanup_file() {
@@ -140,8 +150,8 @@ else
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
+testit "$TEST_LABEL - setup root dir" create_dir tmp tmp.$$ $SETUP_USERNAME
+testit "$TEST_LABEL - assign default ACL" $SMBCACLS //$SERVER/tmp tmp.$$ -U $SETUP_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
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 9bb7903..d02909f33 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -225,9 +225,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'])
+ 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', '$USERNAME', '-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', '$USERNAME', '-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', '$USERNAME', '-m', 'NT1'])
plantestsuite("samba3.blackbox.large_acl (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls])
#
--
2.9.3
From a5b03a07a516de6af792cce3776b1064bce00ba5 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 25 Apr 2017 10:21:22 +0300
Subject: [PATCH 4/8] selftest: add an ownership test with an admin user
Add a test for ownership of files created by an admin
user. As the admin user runs as root, it's important
to verify that files and directories it creates have
the uid of the user (rather than 0) as file owner uid.
Signed-off-by: Uri Simchoni <uri at samba.org>
---
selftest/target/Samba3.pm | 3 +++
source3/selftest/tests.py | 1 +
2 files changed, 4 insertions(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index c241bd1..4226bde 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1682,6 +1682,9 @@ sub provision($$$$$$$$)
path = $shrdir
force group = nogroup
guest ok = yes
+[admin_user]
+ copy = tmp
+ admin users = user1
[ro-tmp]
path = $ro_shrdir
guest ok = yes
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index d02909f33..b6310bb 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -228,6 +228,7 @@ for env in ["fileserver"]:
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', '$USERNAME', '-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', '$USERNAME', '-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', '$USERNAME', '-m', 'NT1'])
+ plantestsuite("samba3.blackbox.inherit_owner.admin(%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_inherit_owner.sh"), '$SERVER', 'user1', '$PASSWORD', '$PREFIX', smbclient3, smbcacls, 'admin_user', '0', '0', '$USERNAME', '-m', 'NT1'])
plantestsuite("samba3.blackbox.large_acl (%s)" % env, env, [os.path.join(samba3srcdir, "script/tests/test_large_acl.sh"), '$SERVER', '$USERNAME', '$PASSWORD', smbclient3, smbcacls])
#
--
2.9.3
From 9a38672af312767dabb426103cebcc0604acba2f Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 25 Apr 2017 10:24:38 +0300
Subject: [PATCH 5/8] selftest: add a test for owner of created symlinks
create symlinks in test_inherit_owner.sh and test
their owner.
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/script/tests/test_inherit_owner.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/source3/script/tests/test_inherit_owner.sh b/source3/script/tests/test_inherit_owner.sh
index 2079737..abeeaa6 100755
--- a/source3/script/tests/test_inherit_owner.sh
+++ b/source3/script/tests/test_inherit_owner.sh
@@ -59,6 +59,17 @@ create_dir() {
$SMBCLIENT //$SERVER/$share -U $user%$PASSWORD -c "cd $rem_dirname; mkdir $bname" 2>/dev/null || exit 1
}
+create_symlink() {
+ local share=$1
+ local fname=$2
+ local rem_dirname=$(dirname $fname)
+ local bname=$(basename $fname)
+ touch $PREFIX/target-$bname
+ $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; rm $bname; rm target-$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 target-$bname; symlink target-$bname $bname" 2>/dev/null || exit 1
+}
+
cleanup_file() {
local share=$1
local fname=$2
@@ -69,6 +80,16 @@ cleanup_file() {
echo "$bname successfully removed"
}
+cleanup_symlink() {
+ 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; rm target-$bname" 2>/dev/null
+ $SMBCLIENT //$SERVER/$share -U $USERNAME%$PASSWORD -c "cd $rem_dirname; ls" 2>/dev/null | grep "$bname" && exit 1
+ echo "$bname successfully removed"
+}
+
cleanup_dir() {
local share=$1
local dname=$2
@@ -160,6 +181,8 @@ testit "$TEST_LABEL - verify subdir unix owner" unix_owner_id_is $SHARE tmp.$$/s
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 - create symlink under root" create_symlink $SHARE tmp.$$/slink
+testit "$TEST_LABEL - verify symlink unix owner" unix_owner_id_is $SHARE tmp.$$/slink $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
@@ -168,4 +191,5 @@ testit "$TEST_LABEL - verify file win owner after change" win_owner_is $SHARE tm
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 symlink" cleanup_symlink $SHARE tmp.$$/slink
testit "$TEST_LABEL - cleanup root" cleanup_dir $SHARE tmp.$$
--
2.9.3
From 7dc5129c603b2106f688e61b0c87d52063c04ea6 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 25 Apr 2017 14:18:50 +0300
Subject: [PATCH 6/8] vfs_fake_acls: ensure errno is set on failures
If the underlying xattr is invalid, return 0 to avoid
having to set errno. This avoids the situation of returning
-1 without setting errno.
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/modules/vfs_fake_acls.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/source3/modules/vfs_fake_acls.c b/source3/modules/vfs_fake_acls.c
index 62b53b6..8367ed0 100644
--- a/source3/modules/vfs_fake_acls.c
+++ b/source3/modules/vfs_fake_acls.c
@@ -45,9 +45,13 @@ static int fake_acls_uid(vfs_handle_struct *handle,
if (size == -1 && errno == ENOATTR) {
return 0;
}
- if (size != 4) {
+ if (size == -1) {
return -1;
}
+ if (size != 4) {
+ DBG_WARNING("invalid %s xattr size=%zd\n", FAKE_UID, size);
+ return 0;
+ }
*uid = IVAL(uid_buf, 0);
return 0;
}
@@ -63,9 +67,13 @@ static int fake_acls_gid(vfs_handle_struct *handle,
if (size == -1 && errno == ENOATTR) {
return 0;
}
- if (size != 4) {
+ if (size == -1) {
return -1;
}
+ if (size != 4) {
+ DBG_WARNING("invalid %s xattr size=%zd\n", FAKE_GID, size);
+ return 0;
+ }
*gid = IVAL(gid_buf, 0);
return 0;
}
@@ -81,9 +89,13 @@ static int fake_acls_fuid(vfs_handle_struct *handle,
if (size == -1 && errno == ENOATTR) {
return 0;
}
- if (size != 4) {
+ if (size == -1) {
return -1;
}
+ if (size != 4) {
+ DBG_WARNING("invalid %s xattr size=%zd\n", FAKE_UID, size);
+ return 0;
+ }
*uid = IVAL(uid_buf, 0);
return 0;
}
@@ -99,9 +111,13 @@ static int fake_acls_fgid(vfs_handle_struct *handle,
if (size == -1 && errno == ENOATTR) {
return 0;
}
- if (size != 4) {
+ if (size == -1) {
return -1;
}
+ if (size != 4) {
+ DBG_WARNING("invalid %s xattr size=%zd\n", FAKE_GID, size);
+ return 0;
+ }
*gid = IVAL(gid_buf, 0);
return 0;
}
--
2.9.3
From 67fdac600c7ea07ac42f305a5948f7949462c1db Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 25 Apr 2017 14:25:29 +0300
Subject: [PATCH 7/8] vfs_fake_acls: refactor lstat
Refactor lstat implementation to call helper
functions. In a later commit those functions
shall be replaced by a function that supports
getting the owner/group of a symlink.
Signed-off-by: Uri Simchoni <uri at samba.org>
---
source3/modules/vfs_fake_acls.c | 47 +++++++++++++++++++++++++++++++++--------
1 file changed, 38 insertions(+), 9 deletions(-)
diff --git a/source3/modules/vfs_fake_acls.c b/source3/modules/vfs_fake_acls.c
index 8367ed0..5d44cb8 100644
--- a/source3/modules/vfs_fake_acls.c
+++ b/source3/modules/vfs_fake_acls.c
@@ -122,6 +122,34 @@ static int fake_acls_fgid(vfs_handle_struct *handle,
return 0;
}
+static int
+fake_acls_luid(vfs_handle_struct *handle, const char *path, uid_t *uid)
+{
+ /* works for most test cases, returns wrong
+ * result with POSIX-extension-related tests,
+ * when we're interested in ownership of the
+ * link.
+ * Ignores errors because the link might not
+ * be pointing to an actual target.
+ */
+ fake_acls_uid(handle, path, uid);
+ return 0;
+}
+
+static int
+fake_acls_lgid(vfs_handle_struct *handle, const char *path, gid_t *gid)
+{
+ /* works for most test cases, returns wrong
+ * result with POSIX-extension-related tests,
+ * when we're interested in ownership of the
+ * link.
+ * Ignores errors because the link might not
+ * be pointing to an actual target.
+ */
+ fake_acls_gid(handle, path, gid);
+ return 0;
+}
+
static int fake_acls_stat(vfs_handle_struct *handle,
struct smb_filename *smb_fname)
{
@@ -188,15 +216,16 @@ static int fake_acls_lstat(vfs_handle_struct *handle,
return -1;
}
- /* This isn't quite right (calling getxattr not
- * lgetxattr), but for the test purposes of this
- * module (fake NT ACLs from windows clients), it is
- * close enough. We removed the l*xattr functions
- * because linux doesn't support using them, but we
- * could fake them in xattr_tdb if we really wanted
- * to. We ignore errors because the link might not point anywhere */
- fake_acls_uid(handle, path, &smb_fname->st.st_ex_uid);
- fake_acls_gid(handle, path, &smb_fname->st.st_ex_gid);
+ ret = fake_acls_luid(handle, path, &smb_fname->st.st_ex_uid);
+ if (ret != 0) {
+ TALLOC_FREE(frame);
+ return ret;
+ }
+ ret = fake_acls_lgid(handle, path, &smb_fname->st.st_ex_gid);
+ if (ret != 0) {
+ TALLOC_FREE(frame);
+ return ret;
+ }
TALLOC_FREE(frame);
}
--
2.9.3
From eb9c1080646937ed87a38cd095c3b2243563c450 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Tue, 25 Apr 2017 14:29:54 +0300
Subject: [PATCH 8/8] vfs_fake_acls: use O_PATH to obtain symlink owner
Use O_PATH open flag and fgetxattr() to get the owner and
group of a file or directory, without chasing symlinks.
This correctly implements lstat().
Signed-off-by: Uri Simchoni <uri at samba.org>
---
selftest/knownfail | 4 ++
source3/modules/vfs_fake_acls.c | 119 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 123 insertions(+)
diff --git a/selftest/knownfail b/selftest/knownfail
index 2cc9c70..7f2242d 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -329,3 +329,7 @@
# We currently don't send referrals for LDAP modify of non-replicated attrs
^samba4.ldap.rodc.python\(rodc\).__main__.RodcTests.test_modify_nonreplicated.*
^samba4.ldap.rodc_rwdc.python.*.__main__.RodcRwdcTests.test_change_password_reveal_on_demand_kerberos
+#failures due to fixed fake_acl module
+^samba3.blackbox.inherit_owner.full \(fileserver\).both - verify symlink unix owner\(fileserver\)
+^samba3.blackbox.inherit_owner.unix \(fileserver\).unix - verify symlink unix owner\(fileserver\)
+^samba3.blackbox.inherit_owner.admin\(fileserver\).default - verify symlink unix owner\(fileserver\)
diff --git a/source3/modules/vfs_fake_acls.c b/source3/modules/vfs_fake_acls.c
index 5d44cb8..a36311f 100644
--- a/source3/modules/vfs_fake_acls.c
+++ b/source3/modules/vfs_fake_acls.c
@@ -122,6 +122,124 @@ static int fake_acls_fgid(vfs_handle_struct *handle,
return 0;
}
+#if defined(O_PATH)
+static void fake_acls_close(vfs_handle_struct *handle, files_struct *fsp)
+{
+ if (!fsp) {
+ return;
+ }
+ if (fsp->fh->fd != -1) {
+ SMB_VFS_NEXT_CLOSE(handle, fsp);
+ fsp->fh->fd = -1;
+ }
+
+ file_free(NULL, fsp);
+}
+
+static int fake_acls_lopen(vfs_handle_struct *handle,
+ const char *path,
+ files_struct **result)
+{
+ files_struct *fsp = NULL;
+ NTSTATUS status;
+ int save_errno = 0;
+ int rc = -1;
+
+ status = file_new(NULL, handle->conn, &fsp);
+ if (!NT_STATUS_IS_OK(status)) {
+ save_errno = map_errno_from_nt_status(status);
+ goto out;
+ }
+
+ fsp->fsp_name =
+ synthetic_smb_fname(fsp, path, NULL, NULL, SMB_FILENAME_POSIX_PATH);
+ if (fsp->fsp_name == NULL) {
+ save_errno = ENOMEM;
+ goto out;
+ }
+
+ fsp->fh->fd = SMB_VFS_NEXT_OPEN(handle, fsp->fsp_name, fsp,
+ O_RDONLY | O_NOFOLLOW | O_PATH, 0);
+ if (fsp->fh->fd == -1) {
+ save_errno = errno;
+ goto out;
+ }
+
+ rc = 0;
+ *result = fsp;
+ fsp = NULL;
+out:
+ fake_acls_close(handle, fsp);
+ if (save_errno != 0) {
+ errno = save_errno;
+ }
+ return rc;
+}
+
+static int
+fake_acls_luid(vfs_handle_struct *handle, const char *path, uid_t *uid)
+{
+ /*
+ * We don't have an LGETXATTR, so we open
+ * and then do FGETXATTR instead.
+ */
+ files_struct *fsp = NULL;
+ int save_errno = 0;
+ int rc = -1;
+
+ rc = fake_acls_lopen(handle, path, &fsp);
+ if (rc < 0) {
+ save_errno = errno;
+ goto out;
+ }
+
+ rc = fake_acls_fuid(handle, fsp, uid);
+ if (rc < 0) {
+ save_errno = errno;
+ goto out;
+ }
+
+out:
+ fake_acls_close(handle, fsp);
+
+ if (save_errno != 0) {
+ errno = save_errno;
+ }
+ return rc;
+}
+
+static int
+fake_acls_lgid(vfs_handle_struct *handle, const char *path, gid_t *gid)
+{
+ /*
+ * We don't have an LGETXATTR, so we open
+ * and then do FGETXATTR instead.
+ */
+ files_struct *fsp = NULL;
+ int save_errno = 0;
+ int rc = -1;
+
+ rc = fake_acls_lopen(handle, path, &fsp);
+ if (rc < 0) {
+ save_errno = errno;
+ goto out;
+ }
+
+ rc = fake_acls_fgid(handle, fsp, gid);
+ if (rc < 0) {
+ save_errno = errno;
+ goto out;
+ }
+
+out:
+ fake_acls_close(handle, fsp);
+
+ if (save_errno != 0) {
+ errno = save_errno;
+ }
+ return rc;
+}
+#else /* O_PATH */
static int
fake_acls_luid(vfs_handle_struct *handle, const char *path, uid_t *uid)
{
@@ -149,6 +267,7 @@ fake_acls_lgid(vfs_handle_struct *handle, const char *path, gid_t *gid)
fake_acls_gid(handle, path, gid);
return 0;
}
+#endif /* O_PATH */
static int fake_acls_stat(vfs_handle_struct *handle,
struct smb_filename *smb_fname)
--
2.9.3
More information about the samba-technical
mailing list