From f6a3838e60c9549b3f4e05ca8a91992e2d89df09 Mon Sep 17 00:00:00 2001 From: benrubson Date: Wed, 28 Jun 2017 15:59:00 +0200 Subject: [PATCH] Fix xattr behavoir on symlinks * Make all xattr operations on link themselves * Be sure to test links and not their target * Add extended attributes tests --- CMakeLists.txt | 14 -------------- encfs/encfs.cpp | 19 ++++++++----------- encfs/main.cpp | 6 ++++++ tests/normal.t.pl | 42 ++++++++++++++++++++++++++++++++++++++++-- tests/reverse.t.pl | 19 +++++++++++++++---- 5 files changed, 69 insertions(+), 31 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5927470..81dbb38 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -93,20 +93,6 @@ check_cxx_source_compiles ("#include int main() { getxattr(0,0,0,0,0,0); return 1; } " XATTR_ADD_OPT) -# Check if xattr function llistxattr exists. -include (CheckCXXSourceCompiles) -if (XATTR_ADD_OPT) -check_cxx_source_compiles ("#include - #include - int main() { llistxattr(0,0,0,0); return 1; } - " XATTR_LLIST) -else (XATTR_ADD_OPT) -check_cxx_source_compiles ("#include - #include - int main() { llistxattr(0,0,0); return 1; } - " XATTR_LLIST) -endif (XATTR_ADD_OPT) - # Check if we have some standard functions. include (CheckFuncs) check_function_exists_glibc (lchmod HAVE_LCHMOD) diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 3b2e308..65993be 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -687,7 +687,7 @@ int encfs_statfs(const char *path, struct statvfs *st) { #ifdef XATTR_ADD_OPT int _do_setxattr(EncFS_Context *, const string &cyName, const char *name, const char *value, size_t size, uint32_t pos) { - int options = 0; + int options = XATTR_NOFOLLOW; return ::setxattr(cyName.c_str(), name, value, size, pos, options); } int encfs_setxattr(const char *path, const char *name, const char *value, @@ -700,7 +700,7 @@ int encfs_setxattr(const char *path, const char *name, const char *value, #else int _do_setxattr(EncFS_Context *, const string &cyName, const char *name, const char *value, size_t size, int flags) { - return ::setxattr(cyName.c_str(), name, value, size, flags); + return ::lsetxattr(cyName.c_str(), name, value, size, flags); } int encfs_setxattr(const char *path, const char *name, const char *value, size_t size, int flags) { @@ -713,7 +713,7 @@ int encfs_setxattr(const char *path, const char *name, const char *value, #ifdef XATTR_ADD_OPT int _do_getxattr(EncFS_Context *, const string &cyName, const char *name, void *value, size_t size, uint32_t pos) { - int options = 0; + int options = XATTR_NOFOLLOW; return ::getxattr(cyName.c_str(), name, value, size, pos, options); } int encfs_getxattr(const char *path, const char *name, char *value, size_t size, @@ -725,7 +725,7 @@ int encfs_getxattr(const char *path, const char *name, char *value, size_t size, #else int _do_getxattr(EncFS_Context *, const string &cyName, const char *name, void *value, size_t size) { - return ::getxattr(cyName.c_str(), name, value, size); + return ::lgetxattr(cyName.c_str(), name, value, size); } int encfs_getxattr(const char *path, const char *name, char *value, size_t size) { @@ -737,12 +737,9 @@ int encfs_getxattr(const char *path, const char *name, char *value, int _do_listxattr(EncFS_Context *, const string &cyName, char *list, size_t size) { -#ifndef XATTR_LLIST -#define llistxattr listxattr -#endif #ifdef XATTR_ADD_OPT - int options = 0; - int res = ::llistxattr(cyName.c_str(), list, size, options); + int options = XATTR_NOFOLLOW; + int res = ::listxattr(cyName.c_str(), list, size, options); #else int res = ::llistxattr(cyName.c_str(), list, size); #endif @@ -756,10 +753,10 @@ int encfs_listxattr(const char *path, char *list, size_t size) { int _do_removexattr(EncFS_Context *, const string &cyName, const char *name) { #ifdef XATTR_ADD_OPT - int options = 0; + int options = XATTR_NOFOLLOW; int res = ::removexattr(cyName.c_str(), name, options); #else - int res = ::removexattr(cyName.c_str(), name); + int res = ::lremovexattr(cyName.c_str(), name); #endif return (res == -1) ? -errno : res; } diff --git a/encfs/main.cpp b/encfs/main.cpp index 28e2f1c..93b6a50 100644 --- a/encfs/main.cpp +++ b/encfs/main.cpp @@ -355,6 +355,12 @@ static bool processArgs(int argc, char *argv[], case 'V': // xgroup(usage) cerr << autosprintf(_("encfs version %s"), VERSION) << endl; +#if defined(HAVE_XATTR) + // "--verbose" has to be passed before "--version" for this to work. + if (out->isVerbose) { + cerr << "Compiled with : HAVE_XATTR" << endl; + } +#endif exit(EXIT_SUCCESS); break; case 'H': diff --git a/tests/normal.t.pl b/tests/normal.t.pl index d15951a..5011c09 100755 --- a/tests/normal.t.pl +++ b/tests/normal.t.pl @@ -2,7 +2,7 @@ # Test EncFS normal and paranoid mode -use Test::More tests => 116; +use Test::More tests => 122; use File::Path; use File::Copy; use File::Temp; @@ -12,6 +12,35 @@ require("tests/common.pl"); my $tempDir = $ENV{'TMPDIR'} || "/tmp"; +if($^O eq "linux" and $tempDir eq "/tmp") { + # On Linux, /tmp is often a tmpfs mount that does not support + # extended attributes. Use /var/tmp instead. + $tempDir = "/var/tmp"; +} + +# Find attr binary +# Linux +my $setattr = "attr -s encfs -V hello"; +my $getattr = "attr -g encfs"; +if(system("which xattr > /dev/null 2>&1") == 0) +{ + # Mac OS X + $setattr = "xattr -sw encfs hello"; + $getattr = "xattr -sp encfs"; +} +if(system("which lsextattr > /dev/null 2>&1") == 0) +{ + # FreeBSD + $setattr = "setextattr -h user encfs hello"; + $getattr = "getextattr -h user encfs"; +} +# Do we support xattr ? +my $have_xattr = 1; +if(system("./build/encfs --verbose --version 2>&1 | grep -q HAVE_XATTR") != 0) +{ + $have_xattr = 0; +} + # test filesystem in standard config mode &runTests('standard'); @@ -270,7 +299,7 @@ sub encName return $enc; } -# Test symlinks & hardlinks +# Test symlinks & hardlinks, and extended attributes sub links { my $hardlinkTests = shift; @@ -295,6 +324,15 @@ sub links ok( link("$crypt/data", "$crypt/data.2"), "hard link"); checkContents("$crypt/data.2", $contents, "hardlink read"); }; + + # extended attributes + my $return_code = ($have_xattr) ? system("$setattr $crypt/data") : 0; + is($return_code, 0, "extended attributes can be set (return code was $return_code)"); + $return_code = ($have_xattr) ? system("$getattr $crypt/data") : 0; + is($return_code, 0, "extended attributes can be get (return code was $return_code)"); + # this is suppused to fail, so get rid of the error message + $return_code = ($have_xattr) ? system("$getattr $crypt/data-rel 2> /dev/null") : 1; + isnt($return_code, 0, "extended attributes operations do not follow symlinks (return code was $return_code)"); } # Test mount diff --git a/tests/reverse.t.pl b/tests/reverse.t.pl index bea37ab..d8277a9 100755 --- a/tests/reverse.t.pl +++ b/tests/reverse.t.pl @@ -13,18 +13,30 @@ require("tests/common.pl"); my $tempDir = $ENV{'TMPDIR'} || "/tmp"; +if($^O eq "linux" and $tempDir eq "/tmp") { + # On Linux, /tmp is often a tmpfs mount that does not support + # extended attributes. Use /var/tmp instead. + $tempDir = "/var/tmp"; +} + # Find attr binary # Linux my @binattr = ("attr", "-l"); if(system("which xattr > /dev/null 2>&1") == 0) { # Mac OS X - @binattr = ("xattr", "-l"); + @binattr = ("xattr", "-s"); } if(system("which lsextattr > /dev/null 2>&1") == 0) { # FreeBSD - @binattr = ("lsextattr", "user"); + @binattr = ("lsextattr", "-h", "user"); +} +# Do we support xattr ? +my $have_xattr = 1; +if(system("./build/encfs --verbose --version 2>&1 | grep -q HAVE_XATTR") != 0) +{ + $have_xattr = 0; } # Helper function @@ -98,8 +110,7 @@ sub symlink_test $dec = readlink("$decrypted/symlink"); ok( $dec eq $target, "symlink to '$target'") or print("# (original) $target' != '$dec' (decrypted)\n"); - system(@binattr, "$decrypted/symlink"); - my $return_code = $?; + my $return_code = ($have_xattr) ? system(@binattr, "$decrypted/symlink") : 0; is($return_code, 0, "symlink to '$target' extended attributes can be read (return code was $return_code)"); unlink("$plain/symlink"); }