From 7732466277794afded6c5b1f015cf60ea97c85ff Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 29 Nov 2014 13:35:02 +0100 Subject: [PATCH 01/10] tests: Replace calls to dd with native writeZeroes --- tests/common.inc | 21 +++++++++++++++++++++ tests/normal.pl | 2 +- tests/reverse.pl | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/tests/common.inc b/tests/common.inc index 9effbd2..d997114 100644 --- a/tests/common.inc +++ b/tests/common.inc @@ -72,5 +72,26 @@ sub waitForFile return 0; } +# writeZeroes($filename, $size) +# Write zeroes of size $size to file $filename +sub writeZeroes +{ + my $filename = shift; + my $size = shift; + open(my $fh, ">", $filename); + my $bs = 4096; # 4 KiB + my $block = "\0" x $bs; + my $remain; + for($remain = $size; $remain >= $bs; $remain -= $bs) + { + print($fh $block) or BAIL_OUT("Could not write to $filename: $!"); + } + if($remain > 0) + { + $block = "\0" x $remain; + print($fh $block) or BAIL_OUT("Could not write to $filename: $!"); + } +} + # As this file will be require()'d, it needs to return true return 1; diff --git a/tests/normal.pl b/tests/normal.pl index 2badb01..45930c6 100644 --- a/tests/normal.pl +++ b/tests/normal.pl @@ -88,7 +88,7 @@ sub corruption sub internalModification { $ofile = "$workingDir/crypt-internal-$$"; - qx(dd if=/dev/urandom of=$ofile bs=2k count=2 2> /dev/null); + writeZeroes($ofile, 2*1024); ok(copy($ofile, "$crypt/internal"), "copying crypt-internal file"); open(my $out1, "+<", "$crypt/internal"); diff --git a/tests/reverse.pl b/tests/reverse.pl index cc3f3af..5939652 100755 --- a/tests/reverse.pl +++ b/tests/reverse.pl @@ -129,7 +129,7 @@ sub grow { } sub largeRead { - system("dd if=/dev/zero of=$plain/largeRead bs=1M count=1 2> /dev/null"); + writeZeroes("$plain/largeRead", 1024*1024); # ciphertext file name my $cname = encName("largeRead"); # cfh ... ciphertext file handle From d1363578fc2659478ac836b2d3547096985449c7 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 29 Nov 2014 20:04:31 +0100 Subject: [PATCH 02/10] reverse: Make uniqueIV configurable in expert mode --- encfs/FileUtils.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/encfs/FileUtils.cpp b/encfs/FileUtils.cpp index 1c563bf..277b6a8 100644 --- a/encfs/FileUtils.cpp +++ b/encfs/FileUtils.cpp @@ -967,7 +967,6 @@ RootPtr createV6Config(EncFS_Context *ctx, const shared_ptr &opts) { long desiredKDFDuration = NormalKDFDuration; if (reverseEncryption) { - uniqueIV = false; chainedIV = false; externalIV = false; blockMACBytes = 0; @@ -976,7 +975,7 @@ RootPtr createV6Config(EncFS_Context *ctx, const shared_ptr &opts) { if (configMode == Config_Paranoia || answer[0] == 'p') { if (reverseEncryption) { - rError(_("Paranoia configuration not supported for --reverse")); + rError(_("Paranoia configuration not supported for reverse encryption")); return rootInfo; } @@ -1011,7 +1010,7 @@ RootPtr createV6Config(EncFS_Context *ctx, const shared_ptr &opts) { uniqueIV = true; if (reverseEncryption) { - cout << _("--reverse specified, not using chained IV") << "\n"; + cout << _("reverse encryption - chained IV disabled") << "\n"; } else { chainedIV = true; } @@ -1035,7 +1034,8 @@ RootPtr createV6Config(EncFS_Context *ctx, const shared_ptr &opts) { blockSize = selectBlockSize(alg); nameIOIface = selectNameCoding(); if (reverseEncryption) { - cout << _("--reverse specified, not using unique/chained IV") << "\n"; + cout << _("reverse encryption - chained IV and MAC disabled") << "\n"; + uniqueIV = selectUniqueIV(); } else { chainedIV = selectChainedIV(); uniqueIV = selectUniqueIV(); From 32102447e04913438c8eb14806bc4cc6bb251bff Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 30 Nov 2014 14:13:25 +0100 Subject: [PATCH 03/10] reverse: Filesystem is read-only of uniqueIV is enabled Writing to the ciphertext files can rewrite the header. This would mean we had to re-encrypt the whole file with the new IV. This could be made more fine-grained, for example allowing writes to everywhere but the header. However, this is something that needs a lot of testing to ensure correctness. Writing to the ciphertext is a niche use case of the niche use case of using reverse mode, so it is unlikely it would get the test coverage it needs. To be safe, we deny all modifications of the ciphertext with read-only filesystem error (EROFS) if uniqueIV is enabled. Reverse mode with uniqueIV disabled still supports writing, if somebody really needs it. This use case is not covered by the test suite at the moment. --- encfs/FSConfig.h | 3 +++ encfs/FileUtils.cpp | 2 ++ encfs/FileUtils.h | 8 ++++++++ encfs/encfs.cpp | 44 ++++++++++++++++++++++++++++++++++++++++++-- encfs/encfs.h | 1 + encfs/main.cpp | 33 +++++++++++++++++++++------------ tests/reverse.pl | 31 +++++++++++++++++++++++++++++-- 7 files changed, 106 insertions(+), 16 deletions(-) diff --git a/encfs/FSConfig.h b/encfs/FSConfig.h index 4e82856..5cc8fe7 100644 --- a/encfs/FSConfig.h +++ b/encfs/FSConfig.h @@ -41,6 +41,9 @@ struct EncFS_Opts; class Cipher; class NameIO; +/** + * Persistent configuration (stored in config file .encfs6.xml) + */ struct EncFSConfig { ConfigType cfgType; diff --git a/encfs/FileUtils.cpp b/encfs/FileUtils.cpp index 277b6a8..0199d52 100644 --- a/encfs/FileUtils.cpp +++ b/encfs/FileUtils.cpp @@ -1036,6 +1036,8 @@ RootPtr createV6Config(EncFS_Context *ctx, const shared_ptr &opts) { if (reverseEncryption) { cout << _("reverse encryption - chained IV and MAC disabled") << "\n"; uniqueIV = selectUniqueIV(); + if (uniqueIV == 0) + opts->readOnly = 0; // Reverse supports rw mode only if uniqueIV is disabled } else { chainedIV = selectChainedIV(); uniqueIV = selectUniqueIV(); diff --git a/encfs/FileUtils.h b/encfs/FileUtils.h index 85ae23b..0cedb14 100644 --- a/encfs/FileUtils.h +++ b/encfs/FileUtils.h @@ -58,6 +58,11 @@ typedef shared_ptr RootPtr; enum ConfigMode { Config_Prompt, Config_Standard, Config_Paranoia }; +/** + * EncFS_Opts stores internal settings + * + * See struct EncFS_Args (main.cpp) for the parsed command line arguments + */ struct EncFS_Opts { std::string rootDir; bool createIfNotFound; // create filesystem if not found @@ -81,6 +86,8 @@ struct EncFS_Opts { * behind the back of EncFS (for example, in reverse mode). * See main.cpp for a longer explaination. */ + bool readOnly; // Mount read-only + ConfigMode configMode; EncFS_Opts() { @@ -96,6 +103,7 @@ struct EncFS_Opts { reverseEncryption = false; configMode = Config_Prompt; noCache = false; + readOnly = false; } }; diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 9ae1ce1..4d72b00 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -70,6 +70,18 @@ static EncFS_Context *context() { return (EncFS_Context *)fuse_get_context()->private_data; } +/** + * Helper function - determine if the filesystem is read-only + * Optionally takes a pointer to the EncFS_Context, will get it from FUSE + * if the argument is NULL. + */ +static bool isReadOnly(EncFS_Context *ctx) { + if (ctx == NULL) + ctx = (EncFS_Context *)fuse_get_context()->private_data; + + return ctx->opts->readOnly; +} + // helper function -- apply a functor to a cipher path, given the plain path static int withCipherPath(const char *opName, const char *path, function op, @@ -130,11 +142,10 @@ static int withFileNode(const char *opName, const char *path, } /* - The rLog messages below always print out encrypted filenames, not + The rLog messages below always prints out encrypted filenames, not plaintext. The reason is so that it isn't possible to leak information about the encrypted data through rlog interfaces. - The purpose of this layer of code is to take the FUSE request and dispatch to the internal interfaces. Any marshaling of arguments and return types can be done here. @@ -215,6 +226,8 @@ int encfs_getdir(const char *path, fuse_dirh_t h, fuse_dirfil_t filler) { int encfs_mknod(const char *path, mode_t mode, dev_t rdev) { EncFS_Context *ctx = context(); + if (isReadOnly(ctx)) return -EROFS; + int res = -EIO; shared_ptr FSRoot = ctx->getRoot(&res); if (!FSRoot) return res; @@ -255,6 +268,8 @@ int encfs_mkdir(const char *path, mode_t mode) { fuse_context *fctx = fuse_get_context(); EncFS_Context *ctx = context(); + if (isReadOnly(ctx)) return -EROFS; + int res = -EIO; shared_ptr FSRoot = ctx->getRoot(&res); if (!FSRoot) return res; @@ -287,6 +302,8 @@ int encfs_mkdir(const char *path, mode_t mode) { int encfs_unlink(const char *path) { EncFS_Context *ctx = context(); + if (isReadOnly(ctx)) return -EROFS; + int res = -EIO; shared_ptr FSRoot = ctx->getRoot(&res); if (!FSRoot) return res; @@ -307,6 +324,7 @@ int _do_rmdir(EncFS_Context *, const string &cipherPath) { } int encfs_rmdir(const char *path) { + if (isReadOnly(NULL)) return -EROFS; return withCipherPath("rmdir", path, bind(_do_rmdir, _1, _2)); } @@ -346,6 +364,8 @@ int encfs_readlink(const char *path, char *buf, size_t size) { int encfs_symlink(const char *from, const char *to) { EncFS_Context *ctx = context(); + if (isReadOnly(ctx)) return -EROFS; + int res = -EIO; shared_ptr FSRoot = ctx->getRoot(&res); if (!FSRoot) return res; @@ -384,6 +404,8 @@ int encfs_symlink(const char *from, const char *to) { int encfs_link(const char *from, const char *to) { EncFS_Context *ctx = context(); + if (isReadOnly(ctx)) return -EROFS; + int res = -EIO; shared_ptr FSRoot = ctx->getRoot(&res); if (!FSRoot) return res; @@ -400,6 +422,8 @@ int encfs_link(const char *from, const char *to) { int encfs_rename(const char *from, const char *to) { EncFS_Context *ctx = context(); + if (isReadOnly(ctx)) return -EROFS; + int res = -EIO; shared_ptr FSRoot = ctx->getRoot(&res); if (!FSRoot) return res; @@ -418,6 +442,7 @@ int _do_chmod(EncFS_Context *, const string &cipherPath, mode_t mode) { } int encfs_chmod(const char *path, mode_t mode) { + if (isReadOnly(NULL)) return -EROFS; return withCipherPath("chmod", path, bind(_do_chmod, _1, _2, mode)); } @@ -427,16 +452,19 @@ int _do_chown(EncFS_Context *, const string &cyName, uid_t u, gid_t g) { } int encfs_chown(const char *path, uid_t uid, gid_t gid) { + if (isReadOnly(NULL)) return -EROFS; return withCipherPath("chown", path, bind(_do_chown, _1, _2, uid, gid)); } int _do_truncate(FileNode *fnode, off_t size) { return fnode->truncate(size); } int encfs_truncate(const char *path, off_t size) { + if (isReadOnly(NULL)) return -EROFS; return withFileNode("truncate", path, NULL, bind(_do_truncate, _1, size)); } int encfs_ftruncate(const char *path, off_t size, struct fuse_file_info *fi) { + if (isReadOnly(NULL)) return -EROFS; return withFileNode("ftruncate", path, fi, bind(_do_truncate, _1, size)); } @@ -446,6 +474,7 @@ int _do_utime(EncFS_Context *, const string &cyName, struct utimbuf *buf) { } int encfs_utime(const char *path, struct utimbuf *buf) { + if (isReadOnly(NULL)) return -EROFS; return withCipherPath("utime", path, bind(_do_utime, _1, _2, buf)); } @@ -462,12 +491,16 @@ int _do_utimens(EncFS_Context *, const string &cyName, } int encfs_utimens(const char *path, const struct timespec ts[2]) { + if (isReadOnly(NULL)) return -EROFS; return withCipherPath("utimens", path, bind(_do_utimens, _1, _2, ts)); } int encfs_open(const char *path, struct fuse_file_info *file) { EncFS_Context *ctx = context(); + if (isReadOnly(ctx) && (file->flags & O_WRONLY || file->flags & O_RDWR)) + return -EROFS; + int res = -EIO; shared_ptr FSRoot = ctx->getRoot(&res); if (!FSRoot) return res; @@ -508,6 +541,7 @@ int _do_flush(FileNode *fnode) { return res; } +// Called on each close() of a file descriptor int encfs_flush(const char *path, struct fuse_file_info *fi) { return withFileNode("flush", path, fi, bind(_do_flush, _1)); } @@ -545,6 +579,7 @@ int _do_fsync(FileNode *fnode, int dataSync) { } int encfs_fsync(const char *path, int dataSync, struct fuse_file_info *file) { + if (isReadOnly(NULL)) return -EROFS; return withFileNode("fsync", path, file, bind(_do_fsync, _1, dataSync)); } @@ -557,6 +592,7 @@ int _do_write(FileNode *fnode, unsigned char *ptr, size_t size, off_t offset) { int encfs_write(const char *path, const char *buf, size_t size, off_t offset, struct fuse_file_info *file) { + if (isReadOnly(NULL)) return -EROFS; return withFileNode("write", path, file, bind(_do_write, _1, (unsigned char *)buf, size, offset)); } @@ -595,6 +631,7 @@ int _do_setxattr(EncFS_Context *, const string &cyName, const char *name, } int encfs_setxattr(const char *path, const char *name, const char *value, size_t size, int flags, uint32_t position) { + if (isReadOnly(NULL)) return -EROFS; (void)flags; return withCipherPath("setxattr", path, bind(_do_setxattr, _1, _2, name, value, size, position)); @@ -606,6 +643,7 @@ int _do_setxattr(EncFS_Context *, const string &cyName, const char *name, } int encfs_setxattr(const char *path, const char *name, const char *value, size_t size, int flags) { + if (isReadOnly(NULL)) return -EROFS; return withCipherPath("setxattr", path, bind(_do_setxattr, _1, _2, name, value, size, flags)); } @@ -663,6 +701,8 @@ int _do_removexattr(EncFS_Context *, const string &cyName, const char *name) { } int encfs_removexattr(const char *path, const char *name) { + if (isReadOnly(NULL)) return -EROFS; + return withCipherPath("removexattr", path, bind(_do_removexattr, _1, _2, name)); } diff --git a/encfs/encfs.h b/encfs/encfs.h index 684ab1d..7177046 100644 --- a/encfs/encfs.h +++ b/encfs/encfs.h @@ -83,6 +83,7 @@ int encfs_write(const char *path, const char *buf, size_t size, off_t offset, int encfs_statfs(const char *, struct statvfs *fst); int encfs_flush(const char *, struct fuse_file_info *info); int encfs_fsync(const char *path, int flags, struct fuse_file_info *info); +int encfs_readonly_error(...); #ifdef HAVE_XATTR diff --git a/encfs/main.cpp b/encfs/main.cpp index 1af861a..728517f 100644 --- a/encfs/main.cpp +++ b/encfs/main.cpp @@ -65,6 +65,12 @@ using gnu::autosprintf; // Maximum number of arguments that we're going to pass on to fuse. Doesn't // affect how many arguments we can handle, just how many we can pass on.. const int MaxFuseArgs = 32; +/** + * EncFS_Args stores the parsed command-line arguments + * + * See also: struct EncFS_Opts (FileUtils.h), stores internal settings that are + * derived from the arguments + */ struct EncFS_Args { string mountPoint; // where to make filesystem visible bool isDaemon; // true == spawn in background, log to syslog @@ -273,20 +279,23 @@ static bool processArgs(int argc, char *argv[], case 'D': out->opts->forceDecode = true; break; - /* By default, the kernel caches file metadata for one second. - * This is fine for EncFS' normal mode, but for --reverse, this - * means that the encrypted view will be up to one second out of - * date. - * Quoting Goswin von Brederlow: - * "Caching only works correctly if you implement a disk based - * filesystem, one where only the fuse process can alter - * metadata and all access goes only through fuse. Any overlay - * filesystem where something can change the underlying - * filesystem without going through fuse can run into - * inconsistencies." - * Enabling reverse automatically enables noCache. */ case 'r': out->opts->reverseEncryption = true; + /* Reverse encryption does not support writing unless uniqueIV + * is disabled (expert mode) */ + out->opts->readOnly = true; + /* By default, the kernel caches file metadata for one second. + * This is fine for EncFS' normal mode, but for --reverse, this + * means that the encrypted view will be up to one second out of + * date. + * Quoting Goswin von Brederlow: + * "Caching only works correctly if you implement a disk based + * filesystem, one where only the fuse process can alter + * metadata and all access goes only through fuse. Any overlay + * filesystem where something can change the underlying + * filesystem without going through fuse can run into + * inconsistencies." + * Enabling reverse automatically enables noCache */ case 514: /* Disable EncFS block cache * Causes reverse grow tests to fail because short reads diff --git a/tests/reverse.pl b/tests/reverse.pl index 5939652..844507f 100755 --- a/tests/reverse.pl +++ b/tests/reverse.pl @@ -3,10 +3,11 @@ # Test EncFS --reverse mode use warnings; -use Test::More qw( no_plan ); +use Test::More tests => 25; use File::Path; use File::Temp; use IO::Handle; +use Errno qw(EROFS); require("tests/common.inc"); @@ -68,7 +69,6 @@ sub copy_test { ok(system("cp -a encfs $plain")==0, "copying files to plain"); ok(system("diff -r -q $plain $decrypted")==0, "decrypted files are identical"); - ok(-f "$plain/encfs/encfs.cpp", "file exists"); unlink("$plain/encfs/encfs.cpp"); ok(! -f "$decrypted/encfs.cpp", "file deleted"); @@ -137,6 +137,32 @@ sub largeRead { ok(sizeVerify($cfh, 1024*1024+8), "1M file size"); } +# Check that the reverse mount is read-only +# (writing is not supported in reverse mode because of the added +# complexity and the marginal use case) +sub writesDenied { + $fn = "$plain/writesDenied"; + writeZeroes($fn, 1024); + my $efn = $ciphertext . "/" . encName("writesDenied"); + open(my $fh, ">", $efn); + if( ok( $! == EROFS, "open for write denied, EROFS")) { + ok( 1, "writing denied, filhandle not open"); + } + else { + print($fh "foo"); + ok( $! == EROFS, "writing denied, EROFS"); + } + $target = $ciphertext . "/" . encName("writesDenied2"); + rename($efn, $target); + ok( $! == EROFS, "rename denied, EROFS") or die(); + unlink($efn); + ok( $! == EROFS, "unlink denied, EROFS"); + utime(undef, undef, $efn) ; + ok( $! == EROFS, "touch denied, EROFS"); + truncate($efn, 10); + ok( $! == EROFS, "truncate denied, EROFS"); +} + # Setup mounts newWorkingDir(); mount(); @@ -149,6 +175,7 @@ symlink_test("/"); # absolute symlink_test("foo"); # relative symlink_test("/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/15/17/18"); # long symlink_test("!§\$%&/()\\<>#+="); # special characters +writesDenied(); # Umount and delete files cleanup(); From 52f189b2320c8af24bed70a3f06b86e0aae8c033 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 30 Nov 2014 20:55:18 +0100 Subject: [PATCH 04/10] encfs_symlink: Fix argument naming (was reversed) It is symlink(target, link_name), see man 3 symlink --- encfs/encfs.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 4d72b00..40c5933 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -361,7 +361,10 @@ int encfs_readlink(const char *path, char *buf, size_t size) { bind(_do_readlink, _1, _2, buf, size)); } -int encfs_symlink(const char *from, const char *to) { +/** + * Create a symbolic link pointing to "to" named "from" + */ +int encfs_symlink(const char *to, const char *from) { EncFS_Context *ctx = context(); if (isReadOnly(ctx)) return -EROFS; @@ -372,8 +375,8 @@ int encfs_symlink(const char *from, const char *to) { try { // allow fully qualified names in symbolic links. - string fromCName = FSRoot->relativeCipherPath(from); - string toCName = FSRoot->cipherPath(to); + string fromCName = FSRoot->cipherPath(from); + string toCName = FSRoot->relativeCipherPath(to); rLog(Info, "symlink %s -> %s", fromCName.c_str(), toCName.c_str()); @@ -386,7 +389,7 @@ int encfs_symlink(const char *from, const char *to) { olduid = setfsuid(context->uid); oldgid = setfsgid(context->gid); } - res = ::symlink(fromCName.c_str(), toCName.c_str()); + res = ::symlink(toCName.c_str(), fromCName.c_str()); if (olduid >= 0) setfsuid(olduid); if (oldgid >= 0) setfsgid(oldgid); From 91919929dd2dbb34140eb1780bee57c390c8330a Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 30 Nov 2014 22:22:20 +0100 Subject: [PATCH 05/10] tests: reverse: symlink absolute path inside the plaintext dir This test currently fails because of a bug in EncFS --- tests/reverse.pl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/reverse.pl b/tests/reverse.pl index 844507f..dc7cd5b 100755 --- a/tests/reverse.pl +++ b/tests/reverse.pl @@ -78,9 +78,11 @@ sub copy_test # Parameter: symlink target sub symlink_test { - my $target = shift(@_); + my $target = shift; symlink($target, "$plain/symlink"); - ok( readlink("$decrypted/symlink") eq "$target", "symlink to '$target'"); + $dec = readlink("$decrypted/symlink"); + ok( $dec eq $target, "symlink to '$target'") or + print("# (original) $target' != '$dec' (decrypted)\n"); unlink("$plain/symlink"); } @@ -175,6 +177,7 @@ symlink_test("/"); # absolute symlink_test("foo"); # relative symlink_test("/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/15/17/18"); # long symlink_test("!§\$%&/()\\<>#+="); # special characters +symlink_test("$plain/foo"); writesDenied(); # Umount and delete files From 34d15bbeaa2f523775740df0be5e5d1d6f19b84d Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 30 Nov 2014 22:28:12 +0100 Subject: [PATCH 06/10] Remove buggy prefix check from plainPath In reverse mode, this caused symlinks pointing to the absolute plaintext directory to be stripped. This is what the test in commit tests: reverse: symlink absolute path inside the plaintext dir checks for. Ignoring encfsctl, plainPath() is only called from encfs.cpp, in _do_readlink() and _do_getattr(). Both functions get the path passed in from FUSE. Paths from FUSE are always anchored at the mountpoint (they start with "/", and "/" means the root of the mount). This suggests that the check can never trigger - I have verified that it does not trigger when running the test suite. With this patch, the full test suite passes. --- encfs/DirNode.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/encfs/DirNode.cpp b/encfs/DirNode.cpp index 908dded..a3f44c0 100644 --- a/encfs/DirNode.cpp +++ b/encfs/DirNode.cpp @@ -285,10 +285,6 @@ string DirNode::cipherPathWithoutRoot(const char *plaintextPath) { string DirNode::plainPath(const char *cipherPath_) { try { - if (!strncmp(cipherPath_, rootDir.c_str(), rootDir.length())) { - return naming->decodePath(cipherPath_ + rootDir.length()); - } - // Handle special absolute path encodings. char mark = fsConfig->reverseEncryption ? '/' : '+'; if (cipherPath_[0] == mark) { From 8eea3be2db9cfe0f19a04bfd06a3a11eb0fac8a3 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 30 Nov 2014 22:42:51 +0100 Subject: [PATCH 07/10] Add comments to path-handling functions in DirNode.cpp --- encfs/DirNode.cpp | 18 ++++++++++++++++++ encfs/encfs.cpp | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/encfs/DirNode.cpp b/encfs/DirNode.cpp index a3f44c0..3b88c37 100644 --- a/encfs/DirNode.cpp +++ b/encfs/DirNode.cpp @@ -275,14 +275,32 @@ string DirNode::rootDirectory() { return string(rootDir, 0, rootDir.length() - 1); } +/** + * Encrypt a plain-text file path to the ciphertext path with the + * ciphertext root directory name prefixed. + * + * Example: + * $ encfs -f -v cipher plain + * $ cd plain + * $ touch foobar + * cipherPath: /foobar encoded to cipher/NKAKsn2APtmquuKPoF4QRPxS + */ string DirNode::cipherPath(const char *plaintextPath) { return rootDir + naming->encodePath(plaintextPath); } +/** + * Same as cipherPath(), but does not prefix the ciphertext root directory + */ string DirNode::cipherPathWithoutRoot(const char *plaintextPath) { return naming->encodePath(plaintextPath); } +/** + * Return the decrypted version of cipherPath + * + * In reverse mode, returns the encrypted version of cipherPath + */ string DirNode::plainPath(const char *cipherPath_) { try { // Handle special absolute path encodings. diff --git a/encfs/encfs.cpp b/encfs/encfs.cpp index 40c5933..42beac4 100644 --- a/encfs/encfs.cpp +++ b/encfs/encfs.cpp @@ -374,8 +374,8 @@ int encfs_symlink(const char *to, const char *from) { if (!FSRoot) return res; try { - // allow fully qualified names in symbolic links. string fromCName = FSRoot->cipherPath(from); + // allow fully qualified names in symbolic links. string toCName = FSRoot->relativeCipherPath(to); rLog(Info, "symlink %s -> %s", fromCName.c_str(), toCName.c_str()); From 73b2f7c8505f2b7a81d30b04bcf5a5256b252ccd Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 30 Nov 2014 22:44:16 +0100 Subject: [PATCH 08/10] Replace ternary operators in cipherPathWithoutRoot with if clause Adds a few lines but makes clear what is happening. --- encfs/DirNode.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/encfs/DirNode.cpp b/encfs/DirNode.cpp index 3b88c37..7d08b77 100644 --- a/encfs/DirNode.cpp +++ b/encfs/DirNode.cpp @@ -304,10 +304,14 @@ string DirNode::cipherPathWithoutRoot(const char *plaintextPath) { string DirNode::plainPath(const char *cipherPath_) { try { // Handle special absolute path encodings. - char mark = fsConfig->reverseEncryption ? '/' : '+'; + char mark = '+'; + string prefix = "/"; + if (fsConfig->reverseEncryption) { + mark = '/'; + prefix = "+"; + } if (cipherPath_[0] == mark) { - return string(fsConfig->reverseEncryption ? "+" : "/") + - naming->decodeName(cipherPath_ + 1, strlen(cipherPath_ + 1)); + return prefix + naming->decodeName(cipherPath_ + 1, strlen(cipherPath_ + 1)); } // Default. From 8b8130782d788b32ad1515324d91796faa6f968e Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 30 Nov 2014 22:45:51 +0100 Subject: [PATCH 09/10] tests: Unset ENCFS6_CONFIG before testing This prevents unexpected failures when you have set that variable. Also, give Test::More the number of tests that will be run for more informative output. --- tests/normal.pl | 5 +++-- tests/reverse.pl | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/normal.pl b/tests/normal.pl index 45930c6..572ef03 100644 --- a/tests/normal.pl +++ b/tests/normal.pl @@ -2,7 +2,7 @@ # Test EncFS normal and paranoid mode -use Test::More qw( no_plan ); +use Test::More tests => 101; use File::Path; use File::Copy; use File::Temp; @@ -289,7 +289,7 @@ sub links is( readlink("$crypt/data-rel"), "data", "read rel symlink"); SKIP: { - skip "No hardlink support" unless $hardlinkTests; + skip "No hardlink support", 2 unless $hardlinkTests; ok( link("$crypt/data", "$crypt/data.2"), "hard link"); checkContents("$crypt/data.2", $contents, "hardlink read"); @@ -306,6 +306,7 @@ sub mount mkdir($raw) || BAIL_OUT("Could not create $raw: $!"); mkdir($crypt) || BAIL_OUT("Could not create $crypt: $!"); + delete $ENV{"ENCFS6_CONFIG"}; qx(./encfs/encfs --extpass="echo test" $args $raw $crypt); ok( -f "$raw/.encfs6.xml", "created control file"); diff --git a/tests/reverse.pl b/tests/reverse.pl index dc7cd5b..2c073e8 100755 --- a/tests/reverse.pl +++ b/tests/reverse.pl @@ -3,7 +3,7 @@ # Test EncFS --reverse mode use warnings; -use Test::More tests => 25; +use Test::More tests => 26; use File::Path; use File::Temp; use IO::Handle; @@ -45,6 +45,7 @@ sub cleanup # Directory structure: plain -[encrypt]-> ciphertext -[decrypt]-> decrypted sub mount { + delete $ENV{"ENCFS6_CONFIG"}; system("./encfs/encfs --extpass=\"echo test\" --standard $plain $ciphertext --reverse"); ok(waitForFile("$plain/.encfs6.xml"), "plain .encfs6.xml exists") or BAIL_OUT("'$plain/.encfs6.xml'"); my $e = encName(".encfs6.xml"); @@ -148,7 +149,7 @@ sub writesDenied { my $efn = $ciphertext . "/" . encName("writesDenied"); open(my $fh, ">", $efn); if( ok( $! == EROFS, "open for write denied, EROFS")) { - ok( 1, "writing denied, filhandle not open"); + ok( 1, "writing denied, filehandle not open"); } else { print($fh "foo"); From 8c7cf98af6a9b67ee3730f2e4a2addef5501db0a Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Mon, 1 Dec 2014 20:13:40 +0100 Subject: [PATCH 10/10] Clarify read-only handling of reverse mounts without uniqueIV Also, delete unused define Both issues spottet by Valient Gough's review --- encfs/FileUtils.cpp | 7 +++++-- encfs/encfs.h | 1 - 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/encfs/FileUtils.cpp b/encfs/FileUtils.cpp index 0199d52..ccfaf0b 100644 --- a/encfs/FileUtils.cpp +++ b/encfs/FileUtils.cpp @@ -1036,8 +1036,11 @@ RootPtr createV6Config(EncFS_Context *ctx, const shared_ptr &opts) { if (reverseEncryption) { cout << _("reverse encryption - chained IV and MAC disabled") << "\n"; uniqueIV = selectUniqueIV(); - if (uniqueIV == 0) - opts->readOnly = 0; // Reverse supports rw mode only if uniqueIV is disabled + /* Reverse mounts are read-only by default (set in main.cpp). + * If uniqueIV is off, writing can be allowed, because there + * is no header that could be overwritten */ + if (uniqueIV == false) + opts->readOnly = false; } else { chainedIV = selectChainedIV(); uniqueIV = selectUniqueIV(); diff --git a/encfs/encfs.h b/encfs/encfs.h index 7177046..684ab1d 100644 --- a/encfs/encfs.h +++ b/encfs/encfs.h @@ -83,7 +83,6 @@ int encfs_write(const char *path, const char *buf, size_t size, off_t offset, int encfs_statfs(const char *, struct statvfs *fst); int encfs_flush(const char *, struct fuse_file_info *info); int encfs_fsync(const char *path, int flags, struct fuse_file_info *info); -int encfs_readonly_error(...); #ifdef HAVE_XATTR