Skip to content

Commit

Permalink
Fix xattr behavoir on symlinks
Browse files Browse the repository at this point in the history 
* Make all xattr operations on link themselves
* Be sure to test links and not their target
* Add extended attributes tests
  • Loading branch information
@rfjakob
benrubson authored and rfjakob committed Jul 30, 2017
1 parent 8a2c03d commit f6a3838
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 31 deletions.
14 changes: 0 additions & 14 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,6 @@ check_cxx_source_compiles ("#include <sys/types.h>
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 <sys/types.h>
#include <sys/xattr.h>
int main() { llistxattr(0,0,0,0); return 1; }
" XATTR_LLIST)
else (XATTR_ADD_OPT)
check_cxx_source_compiles ("#include <sys/types.h>
#include <sys/xattr.h>
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)
Expand Down
19 changes: 8 additions & 11 deletions encfs/encfs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand All @@ -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,
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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;
}
Expand Down
6 changes: 6 additions & 0 deletions encfs/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down
42 changes: 40 additions & 2 deletions tests/normal.t.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -12,6 +12,35 @@

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');

Expand Down Expand Up @@ -270,7 +299,7 @@ sub encName
return $enc;
}

# Test symlinks & hardlinks
# Test symlinks & hardlinks, and extended attributes
sub links
{
my $hardlinkTests = shift;
Expand All @@ -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
Expand Down
19 changes: 15 additions & 4 deletions tests/reverse.t.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,30 @@

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
Expand Down Expand Up @@ -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");
}
Expand Down

0 comments on commit f6a3838

Please sign in to comment.