From 98ab59c96e1c2ff216441dfed99e4e7e91c09063 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Fri, 11 Jun 2021 15:24:39 +0200 Subject: [PATCH 01/29] go.mod: update go-fuse Memory compaction was merged ( https://github.com/hanwen/go-fuse/commit/24a1dfe6b4f8d478275d5cf671d982c4ddd8c904 ) Fixes https://github.com/rfjakob/gocryptfs/issues/569 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index e80a4bc..fa28db8 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/rfjakob/gocryptfs go 1.13 require ( - github.com/hanwen/go-fuse/v2 v2.1.1-0.20210508151621-62c5aa1919a7 + github.com/hanwen/go-fuse/v2 v2.1.1-0.20210611132105-24a1dfe6b4f8 github.com/jacobsa/crypto v0.0.0-20190317225127-9f44e2d11115 github.com/jacobsa/oglematchers v0.0.0-20150720000706-141901ea67cd // indirect github.com/jacobsa/oglemock v0.0.0-20150831005832-e94d794d06ff // indirect diff --git a/go.sum b/go.sum index 8a59eb2..9ef817e 100644 --- a/go.sum +++ b/go.sum @@ -2,8 +2,8 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/hanwen/go-fuse v1.0.0 h1:GxS9Zrn6c35/BnfiVsZVWmsG803xwE7eVRDvcf/BEVc= github.com/hanwen/go-fuse v1.0.0/go.mod h1:unqXarDXqzAk0rt98O2tVndEPIpUgLD9+rwFisZH3Ok= -github.com/hanwen/go-fuse/v2 v2.1.1-0.20210508151621-62c5aa1919a7 h1:9K/MBPvPptwwCYIw8gBi/Sup5Uw8UeYlyKBxxzl931Y= -github.com/hanwen/go-fuse/v2 v2.1.1-0.20210508151621-62c5aa1919a7/go.mod h1:oRyA5eK+pvJyv5otpO/DgccS8y/RvYMaO00GgRLGryc= +github.com/hanwen/go-fuse/v2 v2.1.1-0.20210611132105-24a1dfe6b4f8 h1:1v+7WNqPXx5j/hF9uizHl6xEzLZ7djHFwgbQFXPdZoE= +github.com/hanwen/go-fuse/v2 v2.1.1-0.20210611132105-24a1dfe6b4f8/go.mod h1:oRyA5eK+pvJyv5otpO/DgccS8y/RvYMaO00GgRLGryc= github.com/jacobsa/crypto v0.0.0-20190317225127-9f44e2d11115 h1:YuDUUFNM21CAbyPOpOP8BicaTD/0klJEKt5p8yuw+uY= github.com/jacobsa/crypto v0.0.0-20190317225127-9f44e2d11115/go.mod h1:LadVJg0XuawGk+8L1rYnIED8451UyNxEMdTWCEt5kmU= github.com/jacobsa/oglematchers v0.0.0-20150720000706-141901ea67cd h1:9GCSedGjMcLZCrusBZuo4tyKLpKUPenUUqi34AkuFmA= From 7889346947c891d724412675f148466b3f5dabd1 Mon Sep 17 00:00:00 2001 From: Marcel Bochtler Date: Sat, 5 Jun 2021 17:29:57 +0200 Subject: [PATCH 02/29] README: Rename Mac OS X to its latest name See: https://www.apple.com/macos. --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index cac5f97..5020123 100644 --- a/README.md +++ b/README.md @@ -42,10 +42,10 @@ Platforms Linux is gocryptfs' native platform. -Beta-quality Mac OS X support is available, which means most things work +Beta-quality macOS support is available, which means most things work fine but you may hit an occasional problem. Check out [ticket #15](https://github.com/rfjakob/gocryptfs/issues/15) for the history -of Mac OS X support but please create a new ticket if you hit a problem. +of macOS support but please create a new ticket if you hit a problem. For Windows, an independent C++ reimplementation can be found here: [cppcryptfs](https://github.com/bailey27/cppcryptfs) @@ -62,7 +62,7 @@ On Debian, gocryptfs is available as a deb package: apt install gocryptfs ``` -On Mac OS X, gocryptfs is available as a Homebrew formula: +On macOS, gocryptfs is available as a Homebrew formula: ```bash brew install gocryptfs ``` From 2e738efe8647539b96de60d763df44d237f5389b Mon Sep 17 00:00:00 2001 From: Marcel Bochtler Date: Sat, 5 Jun 2021 18:05:45 +0200 Subject: [PATCH 03/29] README: Add MacPorts install instructions See [1] for the Portfile. [1]: https://github.com/macports/macports-ports/blob/master/fuse/gocryptfs/Portfile. --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 5020123..deb5bd9 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,11 @@ On macOS, gocryptfs is available as a Homebrew formula: brew install gocryptfs ``` +Alternatively, gocryptfs is also available via [MacPorts](https://www.macports.org/) on macOS: +```bash +sudo port install gocryptfs +``` + On Fedora, gocryptfs is available as an rpm package: ```bash sudo dnf install gocryptfs From 9ac77d42d1c2f56018eb5bcb4e2265d78d572e8c Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Thu, 17 Jun 2021 19:19:13 +0200 Subject: [PATCH 04/29] contrib/maxlen.bash: also test dir and path length Move the script from tests to contrib as it may now be useful to somebody else. https://github.com/rfjakob/gocryptfs/issues/552 --- contrib/maxlen.bash | 74 +++++++++++++++++++++++++++++++++++++++++++++ tests/maxlen.bash | 18 ----------- 2 files changed, 74 insertions(+), 18 deletions(-) create mode 100755 contrib/maxlen.bash delete mode 100755 tests/maxlen.bash diff --git a/contrib/maxlen.bash b/contrib/maxlen.bash new file mode 100755 index 0000000..a4aa082 --- /dev/null +++ b/contrib/maxlen.bash @@ -0,0 +1,74 @@ +#!/bin/bash +# +# Find out the maximum supported filename length and print it. +# +# Part of the gocryptfs test suite +# https://nuetzlich.net/gocryptfs/ + +set -eu +MYNAME=$(basename "$0") + +if [[ $# -ne 1 || $1 == -* ]]; then + echo "Usage: $MYNAME DIRECTORY" + exit 1 +fi + +cd "$1" +echo "Testing $PWD" + +echo -n " Maximum filename length: " +# Add one character at a time until we hit an error +NAME="" +while true ; do + NEXT="${NAME}x" + if [[ -e $NEXT ]]; then + echo "error: file $NEXT already exists" + exit 1 + fi + touch $NEXT 2> /dev/null || break + rm $NEXT + NAME="$NEXT" +done +echo "${#NAME}" + +echo -n " Maximum dirname length: " +# Add one character at a time until we hit an error +NAME="" +while true ; do + NEXT="${NAME}x" + mkdir $NEXT 2> /dev/null || break + rmdir $NEXT + NAME="$NEXT" +done +MAX_DIRNAME=${#NAME} +echo "${#NAME}" + +for CHARS_PER_SUBDIR in 1 10 100 $MAX_DIRNAME ; do + echo -n " Maximum path length with $(printf %3d $CHARS_PER_SUBDIR) chars per subdir: " + # Trick from https://stackoverflow.com/a/5349842/1380267 + SUBDIR=$(printf 'x%.0s' $(seq 1 $CHARS_PER_SUBDIR)) + mkdir "$SUBDIR" + P=$SUBDIR + # Create a deep path, one $SUBDIR at a time, until we hit an error + while true ; do + NEXT="$P/$SUBDIR" + mkdir "$NEXT" 2> /dev/null || break + P=$NEXT + echo -n -e "\b\b\b\b" + printf %4d ${#P} + done + # Then add one character at a time until we hit an error + NAME="" + while true ; do + NEXT="${NAME}x" + touch "$P/$NEXT" 2> /dev/null || break + NAME=$NEXT + done + if [[ $NAME != "" ]] ; then + P=$P/$NAME + fi + echo -n -e "\b\b\b\b" + printf %4d ${#P} + echo + rm -R "$SUBDIR" +done diff --git a/tests/maxlen.bash b/tests/maxlen.bash deleted file mode 100755 index fb133e0..0000000 --- a/tests/maxlen.bash +++ /dev/null @@ -1,18 +0,0 @@ -#!/bin/bash -eu -# -# Find out the maximum supported filename length and print it. -# -# Part of the gocryptfs test suite -# https://nuetzlich.net/gocryptfs/ - -NAME="maxlen." -LEN=0 - -while [ $LEN -le 10000 ]; do - touch $NAME 2> /dev/null || break - rm $NAME - LEN=${#NAME} - NAME="${NAME}x" -done - -echo $LEN From cdddd1d711c3398aa1093090296139aa3d331859 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 20 Jun 2021 12:28:20 +0200 Subject: [PATCH 05/29] MANPAGE: describe -badname --- Documentation/MANPAGE.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/Documentation/MANPAGE.md b/Documentation/MANPAGE.md index 9ddf674..648a6c7 100644 --- a/Documentation/MANPAGE.md +++ b/Documentation/MANPAGE.md @@ -152,6 +152,31 @@ other users, subject to file permission checking. Only works if user_allow_other is set in /etc/fuse.conf. This option is equivalent to "allow_other" plus "default_permissions" described in fuse(8). +#### -badname string +When gocryptfs encounters a "bad" file name (cannot be decrypted or decrypts +to garbage), a warning is logged and the file is hidden from the +plaintext view. + +With the `-badname` option, you can select "bad" file names that should +still be shown in the plaintext view instead of hiding them. Bad files +will get ` GOCRYPTFS_BAD_NAME` appended to their name. + +Glob pattern. Can be passed multiple times for multiple patterns. + +Examples: + +Dropbox sync conflicts: + + -badname '*conflicted copy*' + +Syncthing sync conflicts: + + -badname '*.sync-conflict*' + +Show all invalid filenames: + + -badname '*' + #### -ctlsock string Create a control socket at the specified location. The socket can be used to decrypt and encrypt paths inside the filesystem. When using From a611810ff46ed0899d677f24c330a994ad125bfb Mon Sep 17 00:00:00 2001 From: DerDonut Date: Thu, 17 Jun 2021 08:11:33 +0200 Subject: [PATCH 06/29] Badname file content access This proposal is the counterpart of the modifications from the `-badname` parameter. It modifies the plain -> cipher mapping for filenames when using `-badname` parameter. The new function `EncryptAndHashBadName` tries to find a cipher filename for the given plain name with the following steps: 1. If `badname` is disabled or direct mapping is successful: Map directly (default and current behaviour) 2. If a file with badname flag has a valid cipher file, this is returned (=File just ends with the badname flag) 3. If a file with a badname flag exists where only the badname flag was added, this is returned (=File cipher name could not be decrypted by function `DecryptName` and just the badname flag was added) 4. Search for all files which cipher file name extists when cropping more and more characters from the end. If only 1 file is found, return this 5. Return an error otherwise This allows file access in the file browsers but most important it allows that you rename files with undecryptable cipher names in the plain directories. Renaming those files will then generate a proper cipher filename One backdraft: When mounting the cipher dir with -badname parameter, you can never create (or rename to) files whose file name ends with the badname file flag (at the moment this is " GOCRYPTFS_BAD_NAME"). This will cause an error. I modified the CLI test function to cover additional test cases. Test [Case 7](https://github.com/DerDonut/gocryptfs/blob/badnamecontent/tests/cli/cli_test.go#L712) cannot be performed since the cli tests are executed in panic mode. The testing is stopped on error. Since the function`DecryptName` produces internal errors when hitting non-decryptable file names, this test was omitted. This implementation is a proposal where I tried to change the minimum amount of existing code. Another possibility would be instead of creating the new function `EncryptAndHashBadName` to modify the signature of the existing function `EncryptAndHashName(name string, iv []byte)` to `EncryptAndHashName(name string, iv []byte, dirfd int)` and integrate the functionality into this function directly. You may allow calling with dirfd=-1 or other invalid values an then performing the current functionality. --- internal/fusefrontend/node_helpers.go | 10 +- internal/fusefrontend/root_node.go | 6 +- internal/nametransform/diriv.go | 50 ++++++++ internal/nametransform/names.go | 13 ++- tests/cli/cli_test.go | 161 ++++++++++++++++++++++---- 5 files changed, 212 insertions(+), 28 deletions(-) diff --git a/internal/fusefrontend/node_helpers.go b/internal/fusefrontend/node_helpers.go index b2f1d4a..f2d1e5e 100644 --- a/internal/fusefrontend/node_helpers.go +++ b/internal/fusefrontend/node_helpers.go @@ -121,7 +121,15 @@ func (n *Node) prepareAtSyscall(child string) (dirfd int, cName string, errno sy var iv []byte dirfd, iv = rn.dirCache.Lookup(n) if dirfd > 0 { - cName, err := rn.nameTransform.EncryptAndHashName(child, iv) + var cName string + var err error + if rn.nameTransform.HaveBadnamePatterns() { + //BadName allowed, try to determine filenames + cName, err = rn.nameTransform.EncryptAndHashBadName(child, iv, dirfd) + } else { + cName, err = rn.nameTransform.EncryptAndHashName(child, iv) + } + if err != nil { return -1, "", fs.ToErrno(err) } diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go index a830cc4..35b7be0 100644 --- a/internal/fusefrontend/root_node.go +++ b/internal/fusefrontend/root_node.go @@ -245,7 +245,11 @@ func (rn *RootNode) openBackingDir(relPath string) (dirfd int, cName string, err syscall.Close(dirfd) return -1, "", err } - cName, err = rn.nameTransform.EncryptAndHashName(name, iv) + if rn.nameTransform.HaveBadnamePatterns() { + cName, err = rn.nameTransform.EncryptAndHashBadName(name, iv, dirfd) + } else { + cName, err = rn.nameTransform.EncryptAndHashName(name, iv) + } if err != nil { syscall.Close(dirfd) return -1, "", err diff --git a/internal/nametransform/diriv.go b/internal/nametransform/diriv.go index 1d27aa5..d62b3fb 100644 --- a/internal/nametransform/diriv.go +++ b/internal/nametransform/diriv.go @@ -6,11 +6,13 @@ import ( "io" "os" "path/filepath" + "strings" "syscall" "github.com/rfjakob/gocryptfs/internal/cryptocore" "github.com/rfjakob/gocryptfs/internal/syscallcompat" "github.com/rfjakob/gocryptfs/internal/tlog" + "golang.org/x/sys/unix" ) const ( @@ -112,6 +114,54 @@ func (be *NameTransform) EncryptAndHashName(name string, iv []byte) (string, err return cName, nil } +// EncryptAndHashBadName tries to find the "name" substring, which (encrypted and hashed) +// leads to an unique existing file +// Returns ENOENT if cipher file does not exist or is not unique +func (be *NameTransform) EncryptAndHashBadName(name string, iv []byte, dirfd int) (cName string, err error) { + var st unix.Stat_t + var filesFound int + lastFoundName, err := be.EncryptAndHashName(name, iv) + if !strings.HasSuffix(name, BadNameFlag) || err != nil { + //Default mode: same behaviour on error or no BadNameFlag on "name" + return lastFoundName, err + } + //Default mode: Check if File extists without modifications + err = syscallcompat.Fstatat(dirfd, lastFoundName, &st, unix.AT_SYMLINK_NOFOLLOW) + if err == nil { + //file found, return result + return lastFoundName, nil + } + //BadName Mode: check if the name was tranformed without change (badname suffix and undecryptable cipher name) + err = syscallcompat.Fstatat(dirfd, name[:len(name)-len(BadNameFlag)], &st, unix.AT_SYMLINK_NOFOLLOW) + if err == nil { + filesFound++ + lastFoundName = name[:len(name)-len(BadNameFlag)] + } + // search for the longest badname pattern match + for charpos := len(name) - len(BadNameFlag); charpos > 0; charpos-- { + //only use original cipher name and append assumed suffix (without badname flag) + cNamePart, err := be.EncryptName(name[:charpos], iv) + if err != nil { + //expand suffix on error + continue + } + if be.longNames && len(cName) > NameMax { + cNamePart = be.HashLongName(cName) + } + cNameBadReverse := cNamePart + name[charpos:len(name)-len(BadNameFlag)] + err = syscallcompat.Fstatat(dirfd, cNameBadReverse, &st, unix.AT_SYMLINK_NOFOLLOW) + if err == nil { + filesFound++ + lastFoundName = cNameBadReverse + } + } + if filesFound == 1 { + return lastFoundName, nil + } + // more than 1 possible file found, ignore + return "", syscall.ENOENT +} + // Dir is like filepath.Dir but returns "" instead of ".". func Dir(path string) string { d := filepath.Dir(path) diff --git a/internal/nametransform/names.go b/internal/nametransform/names.go index ca28230..f730184 100644 --- a/internal/nametransform/names.go +++ b/internal/nametransform/names.go @@ -15,6 +15,8 @@ import ( const ( // Like ext4, we allow at most 255 bytes for a file name. NameMax = 255 + //BadNameFlag is appended to filenames in plain mode if a ciphername is inavlid but is shown + BadNameFlag = " GOCRYPTFS_BAD_NAME" ) // NameTransformer is an interface used to transform filenames. @@ -22,11 +24,13 @@ type NameTransformer interface { DecryptName(cipherName string, iv []byte) (string, error) EncryptName(plainName string, iv []byte) (string, error) EncryptAndHashName(name string, iv []byte) (string, error) + EncryptAndHashBadName(name string, iv []byte, dirfd int) (string, error) // HashLongName - take the hash of a long string "name" and return // "gocryptfs.longname.[sha256]" // // This function does not do any I/O. HashLongName(name string) string + HaveBadnamePatterns() bool WriteLongNameAt(dirfd int, hashName string, plainName string) error B64EncodeToString(src []byte) string B64DecodeString(s string) ([]byte, error) @@ -70,10 +74,10 @@ func (n *NameTransform) DecryptName(cipherName string, iv []byte) (string, error for charpos := len(cipherName) - 1; charpos >= nameMin; charpos-- { res, err = n.decryptName(cipherName[:charpos], iv) if err == nil { - return res + cipherName[charpos:] + " GOCRYPTFS_BAD_NAME", nil + return res + cipherName[charpos:] + BadNameFlag, nil } } - return cipherName + " GOCRYPTFS_BAD_NAME", nil + return cipherName + BadNameFlag, nil } } } @@ -135,3 +139,8 @@ func (n *NameTransform) B64EncodeToString(src []byte) string { func (n *NameTransform) B64DecodeString(s string) ([]byte, error) { return n.B64.DecodeString(s) } + +// HaveBadnamePatterns returns true if BadName patterns were provided +func (n *NameTransform) HaveBadnamePatterns() bool { + return len(n.BadnamePatterns) > 0 +} diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go index 9248f5d..08c1b83 100644 --- a/tests/cli/cli_test.go +++ b/tests/cli/cli_test.go @@ -16,6 +16,7 @@ import ( "github.com/rfjakob/gocryptfs/internal/configfile" "github.com/rfjakob/gocryptfs/internal/exitcodes" + "github.com/rfjakob/gocryptfs/internal/nametransform" "github.com/rfjakob/gocryptfs/tests/test_helpers" ) @@ -698,18 +699,29 @@ func TestSymlinkedCipherdir(t *testing.T) { // TestBadname tests the `-badname` option func TestBadname(t *testing.T) { + //Supported structure of badname: + //"Visible" shows the success of function DecryptName (cipher -> plain) + //"Access" shows the success of function EncryptAndHashBadName (plain -> cipher) + //Case Visible Access Description + //Case 1 x x Access file without BadName suffix (default mode) + //Case 2 x x Access file with BadName suffix which has a valid cipher file (will only be possible if file was created without badname option) + //Case 3 Access file with valid ciphername + BadName suffix (impossible since this would not be produced by DecryptName) + //Case 4 x x Access file with decryptable part of name and Badname suffix (default badname case) + //Case 5 x x Access file with undecryptable name and BadName suffix (e. g. when part of the cipher name was cut) + //Case 6 x Access file with multiple possible matches. + //Case 7 Access file with BadName suffix and non-matching pattern + dir := test_helpers.InitFS(t) mnt := dir + ".mnt" validFileName := "file" - invalidSuffix := ".invalid_file" + invalidSuffix := "_invalid_file" + var contentCipher [7][]byte + //first mount without badname (see case 2) + test_helpers.MountOrFatal(t, dir, mnt, "-extpass=echo test", "-wpanic=false") - // use static suffix for testing - test_helpers.MountOrFatal(t, dir, mnt, "-badname=*", "-extpass=echo test") - defer test_helpers.UnmountPanic(mnt) - - // write one valid filename (empty content) file := mnt + "/" + validFileName - err := ioutil.WriteFile(file, nil, 0600) + // Case 1: write one valid filename (empty content) + err := ioutil.WriteFile(file, []byte("Content Case 1."), 0600) if err != nil { t.Fatal(err) } @@ -720,7 +732,6 @@ func TestBadname(t *testing.T) { t.Fatal(err) } defer fread.Close() - encryptedfilename := "" ciphernames, err := fread.Readdirnames(0) if err != nil { @@ -733,14 +744,64 @@ func TestBadname(t *testing.T) { break } } + //Generate valid cipherdata for all cases + for i := 0; i < len(contentCipher); i++ { + err := ioutil.WriteFile(file, []byte(fmt.Sprintf("Content Case %d.", i+1)), 0600) + if err != nil { + t.Fatal(err) + } + //save the cipher data for file operations in cipher dir + contentCipher[i], err = ioutil.ReadFile(dir + "/" + encryptedfilename) + if err != nil { + t.Fatal(err) + } + } - // write invalid file which should be decodable - err = ioutil.WriteFile(dir+"/"+encryptedfilename+invalidSuffix, nil, 0600) + //re-write content for case 1 + err = ioutil.WriteFile(file, []byte("Content Case 1."), 0600) if err != nil { t.Fatal(err) } - // write invalid file which is not decodable (cropping the encrpyted file name) - err = ioutil.WriteFile(dir+"/"+encryptedfilename[:len(encryptedfilename)-2]+invalidSuffix, nil, 0600) + + // Case 2: File with invalid suffix in plain name but valid cipher file + file = mnt + "/" + validFileName + nametransform.BadNameFlag + err = ioutil.WriteFile(file, []byte("Content Case 2."), 0600) + if err != nil { + t.Fatal(err) + } + // unmount... + test_helpers.UnmountPanic(mnt) + + // ...and remount with -badname. + test_helpers.MountOrFatal(t, dir, mnt, "-badname=*valid*", "-extpass=echo test", "-wpanic=false") + defer test_helpers.UnmountPanic(mnt) + + // Case 3 is impossible: only BadnameSuffix would mean the cipher name is valid + + // Case 4: write invalid file which should be decodable + err = ioutil.WriteFile(dir+"/"+encryptedfilename+invalidSuffix, contentCipher[3], 0600) + if err != nil { + t.Fatal(err) + } + //Case 5: write invalid file which is not decodable (cropping the encrpyted file name) + err = ioutil.WriteFile(dir+"/"+encryptedfilename[:len(encryptedfilename)-2]+invalidSuffix, contentCipher[4], 0600) + if err != nil { + t.Fatal(err) + } + + // Case 6: Multiple possible matches + // generate two files with invalid cipher names which can both match the badname pattern + err = ioutil.WriteFile(dir+"/mzaZRF9_0IU-_5vv2wPC"+invalidSuffix, contentCipher[5], 0600) + if err != nil { + t.Fatal(err) + } + err = ioutil.WriteFile(dir+"/mzaZRF9_0IU-_5vv2wP"+invalidSuffix, contentCipher[5], 0600) + if err != nil { + t.Fatal(err) + } + + // Case 7: Non-Matching badname pattern + err = ioutil.WriteFile(dir+"/"+encryptedfilename+"wrongPattern", contentCipher[6], 0600) if err != nil { t.Fatal(err) } @@ -755,22 +816,74 @@ func TestBadname(t *testing.T) { if err != nil { t.Fatal(err) } - foundDecodable := false - foundUndecodable := false + + searchstrings := []string{ + validFileName, + validFileName + nametransform.BadNameFlag, + "", + validFileName + invalidSuffix + nametransform.BadNameFlag, + encryptedfilename[:len(encryptedfilename)-2] + invalidSuffix + nametransform.BadNameFlag, + "", + validFileName + "wrongPattern" + nametransform.BadNameFlag} + results := []bool{false, false, true, false, false, true, true} + var filecontent string + var filebytes []byte for _, name := range names { - if strings.Contains(name, validFileName+invalidSuffix+" GOCRYPTFS_BAD_NAME") { - foundDecodable = true - } else if strings.Contains(name, encryptedfilename[:len(encryptedfilename)-2]+invalidSuffix+" GOCRYPTFS_BAD_NAME") { - foundUndecodable = true + if name == searchstrings[0] { + //Case 1: Test access + filebytes, err = ioutil.ReadFile(mnt + "/" + name) + if err != nil { + t.Fatal(err) + } + filecontent = string(filebytes) + if filecontent == "Content Case 1." { + results[0] = true + } + + } else if name == searchstrings[1] { + //Case 2: Test Access + filebytes, err = ioutil.ReadFile(mnt + "/" + name) + if err != nil { + t.Fatal(err) + } + filecontent = string(filebytes) + if filecontent == "Content Case 2." { + results[1] = true + } + } else if name == searchstrings[3] { + //Case 4: Test Access + filebytes, err = ioutil.ReadFile(mnt + "/" + name) + if err != nil { + t.Fatal(err) + } + filecontent = string(filebytes) + if filecontent == "Content Case 4." { + results[3] = true + } + } else if name == searchstrings[4] { + //Case 5: Test Access + filebytes, err = ioutil.ReadFile(mnt + "/" + name) + if err != nil { + t.Fatal(err) + } + filecontent = string(filebytes) + if filecontent == "Content Case 5." { + results[4] = true + } + } else if name == searchstrings[6] { + //Case 7 + results[6] = false } + //Case 3 is always passed + //Case 6 is highly obscure: + //The last part of a valid cipher name must match the badname pattern AND + //the remaining cipher name must still be decryptable. Test case not programmable in a general case } - if !foundDecodable { - t.Errorf("did not find invalid name %s in %v", validFileName+invalidSuffix+" GOCRYPTFS_BAD_NAME", names) - } - - if !foundUndecodable { - t.Errorf("did not find invalid name %s in %v", encryptedfilename[:len(encryptedfilename)-2]+invalidSuffix+" GOCRYPTFS_BAD_NAME", names) + for i := 0; i < len(results); i++ { + if !results[i] { + t.Errorf("Case %d failed: '%s' in [%s]", i+1, searchstrings[i], strings.Join(names, ",")) + } } } From 50630e9f3d7f649e41ab6f1102b40a1e4ec99686 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 20 Jun 2021 12:59:45 +0200 Subject: [PATCH 07/29] fido2: hide "FIDO2" in gocryptfs.conf if not used Result of: $ gocryptfs -init foo $ cat foo/gocryptfs.conf Before: { "Creator": "gocryptfs v2.0.1", "EncryptedKey": "FodEdNHD/cCwv1n5BuyAkbIOnJ/O5gfdCh3YssUCJ2DUr0A8DrQ5NH2SLhREeWRL3V8EMiPO2Ncr5IVwE4SSxQ==", "ScryptObject": { "Salt": "brGaw9Jg1kbPuSXFiwoxqK2oXFTgbniSgpiB+cu+67Y=", "N": 65536, "R": 8, "P": 1, "KeyLen": 32 }, "Version": 2, "FeatureFlags": [ "GCMIV128", "HKDF", "DirIV", "EMENames", "LongNames", "Raw64" ], "FIDO2": { "CredentialID": null, "HMACSalt": null } } After: { "Creator": "gocryptfs v2.0.1-5-gf9718eb-dirty.DerDonut-badnamecontent", "EncryptedKey": "oFMj1lS1ZsM/vEfanNMeCTPw3PZr5VWeL7ap8Jd8YQm6evy2BAhtQ/pd6RzDx84wlCz9TpxqHRihuwSEMnOWWg==", "ScryptObject": { "Salt": "JZ/5mhy4a8EAQ/wDF1POIEe4/Ss38cfJgXgj26DuA4M=", "N": 65536, "R": 8, "P": 1, "KeyLen": 32 }, "Version": 2, "FeatureFlags": [ "GCMIV128", "HKDF", "DirIV", "EMENames", "LongNames", "Raw64" ] } --- internal/configfile/config_file.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/configfile/config_file.go b/internal/configfile/config_file.go index e4921f7..40dda38 100644 --- a/internal/configfile/config_file.go +++ b/internal/configfile/config_file.go @@ -56,7 +56,7 @@ type ConfFile struct { // stored in the superblock. FeatureFlags []string // FIDO2 parameters - FIDO2 FIDO2Params + FIDO2 *FIDO2Params `json:",omitempty"` // Filename is the name of the config file. Not exported to JSON. filename string } @@ -102,8 +102,10 @@ func Create(filename string, password []byte, plaintextNames bool, } if len(fido2CredentialID) > 0 { cf.FeatureFlags = append(cf.FeatureFlags, knownFlags[FlagFIDO2]) - cf.FIDO2.CredentialID = fido2CredentialID - cf.FIDO2.HMACSalt = fido2HmacSalt + cf.FIDO2 = &FIDO2Params{ + CredentialID: fido2CredentialID, + HMACSalt: fido2HmacSalt, + } } { // Generate new random master key From 203e65066fc1197427353eed3ae0a5108a1121ee Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 20 Jun 2021 18:25:07 +0200 Subject: [PATCH 08/29] main: use JSONDump helper for debug output --- mount.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mount.go b/mount.go index f992f94..e35e6c2 100644 --- a/mount.go +++ b/mount.go @@ -2,7 +2,6 @@ package main import ( "bytes" - "encoding/json" "fmt" "log" "log/syslog" @@ -309,8 +308,7 @@ func initFuseFrontend(args *argContainer) (rootNode fs.InodeEmbedder, wipeKeys f if args.allow_other && os.Getuid() == 0 { frontendArgs.PreserveOwner = true } - jsonBytes, _ := json.MarshalIndent(frontendArgs, "", "\t") - tlog.Debug.Printf("frontendArgs: %s", string(jsonBytes)) + tlog.Debug.Printf("frontendArgs: %s", tlog.JSONDump(frontendArgs)) // Init crypto backend cCore := cryptocore.New(masterkey, cryptoBackend, contentenc.DefaultIVBits, args.hkdf, args.forcedecode) From c5d8fa83ae702017fc90769dff178fda6a7942a3 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 20 Jun 2021 19:09:46 +0200 Subject: [PATCH 09/29] nametransform: pass badname patterns via New This means we can unexport the field. --- cli_args.go | 10 ++++++++++ internal/fusefrontend/xattr_unit_test.go | 2 +- internal/nametransform/names.go | 18 +++++++++++------- mount.go | 13 +------------ 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/cli_args.go b/cli_args.go index 7743120..ce15448 100644 --- a/cli_args.go +++ b/cli_args.go @@ -10,6 +10,7 @@ import ( "fmt" "net" "os" + "path/filepath" "strconv" "strings" "time" @@ -290,6 +291,15 @@ func parseCliOpts() (args argContainer) { tlog.Fatal.Printf("Idle timeout cannot be less than 0") os.Exit(exitcodes.Usage) } + // Make sure all badname patterns are valid + for _, pattern := range args.badname { + _, err := filepath.Match(pattern, "") + if err != nil { + tlog.Fatal.Printf("-badname: invalid pattern %q supplied", pattern) + os.Exit(exitcodes.Usage) + } + } + return args } diff --git a/internal/fusefrontend/xattr_unit_test.go b/internal/fusefrontend/xattr_unit_test.go index a0cf4c8..c48781c 100644 --- a/internal/fusefrontend/xattr_unit_test.go +++ b/internal/fusefrontend/xattr_unit_test.go @@ -19,7 +19,7 @@ func newTestFS(args Args) *RootNode { key := make([]byte, cryptocore.KeyLen) cCore := cryptocore.New(key, cryptocore.BackendGoGCM, contentenc.DefaultIVBits, true, false) cEnc := contentenc.New(cCore, contentenc.DefaultBS, false) - n := nametransform.New(cCore.EMECipher, true, true) + n := nametransform.New(cCore.EMECipher, true, true, nil) rn := NewRootNode(args, cEnc, n) oneSec := time.Second options := &fs.Options{ diff --git a/internal/nametransform/names.go b/internal/nametransform/names.go index f730184..afc0f5d 100644 --- a/internal/nametransform/names.go +++ b/internal/nametransform/names.go @@ -44,19 +44,23 @@ type NameTransform struct { // on the Raw64 feature flag B64 *base64.Encoding // Patterns to bypass decryption - BadnamePatterns []string + badnamePatterns []string } // New returns a new NameTransform instance. -func New(e *eme.EMECipher, longNames bool, raw64 bool) *NameTransform { +func New(e *eme.EMECipher, longNames bool, raw64 bool, badname []string) *NameTransform { + tlog.Debug.Printf("nametransform.New: longNames=%v, raw64=%v, badname=%q", + longNames, raw64, badname) + b64 := base64.URLEncoding if raw64 { b64 = base64.RawURLEncoding } return &NameTransform{ - emeCipher: e, - longNames: longNames, - B64: b64, + emeCipher: e, + longNames: longNames, + B64: b64, + badnamePatterns: badname, } } @@ -65,7 +69,7 @@ func New(e *eme.EMECipher, longNames bool, raw64 bool) *NameTransform { func (n *NameTransform) DecryptName(cipherName string, iv []byte) (string, error) { res, err := n.decryptName(cipherName, iv) if err != nil { - for _, pattern := range n.BadnamePatterns { + for _, pattern := range n.badnamePatterns { match, err := filepath.Match(pattern, cipherName) if err == nil && match { // Pattern should have been validated already // Find longest decryptable substring @@ -142,5 +146,5 @@ func (n *NameTransform) B64DecodeString(s string) ([]byte, error) { // HaveBadnamePatterns returns true if BadName patterns were provided func (n *NameTransform) HaveBadnamePatterns() bool { - return len(n.BadnamePatterns) > 0 + return len(n.badnamePatterns) > 0 } diff --git a/mount.go b/mount.go index e35e6c2..7f818d1 100644 --- a/mount.go +++ b/mount.go @@ -313,18 +313,7 @@ func initFuseFrontend(args *argContainer) (rootNode fs.InodeEmbedder, wipeKeys f // Init crypto backend cCore := cryptocore.New(masterkey, cryptoBackend, contentenc.DefaultIVBits, args.hkdf, args.forcedecode) cEnc := contentenc.New(cCore, contentenc.DefaultBS, args.forcedecode) - nameTransform := nametransform.New(cCore.EMECipher, frontendArgs.LongNames, args.raw64) - // Init badname patterns - nameTransform.BadnamePatterns = make([]string, 0) - for _, pattern := range args.badname { - _, err := filepath.Match(pattern, "") // Make sure pattern is valid - if err != nil { - tlog.Fatal.Printf("-badname: invalid pattern %q supplied", pattern) - os.Exit(exitcodes.Usage) - } else { - nameTransform.BadnamePatterns = append(nameTransform.BadnamePatterns, pattern) - } - } + nameTransform := nametransform.New(cCore.EMECipher, frontendArgs.LongNames, args.raw64, []string(args.badname)) // After the crypto backend is initialized, // we can purge the master key from memory. for i := range masterkey { From 6b0e63c1a86946de23f549e7d80ea933a4a105f8 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Mon, 21 Jun 2021 11:32:04 +0200 Subject: [PATCH 10/29] Improve startup debug output The startup debug output was very verbose but still missing some effective crypto settings. --- internal/contentenc/content.go | 3 +++ internal/cryptocore/cryptocore.go | 16 ++++++++++++++++ main.go | 7 +------ mount.go | 4 +--- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/internal/contentenc/content.go b/internal/contentenc/content.go index 747bb4c..e023492 100644 --- a/internal/contentenc/content.go +++ b/internal/contentenc/content.go @@ -73,6 +73,9 @@ type ContentEnc struct { // New returns an initialized ContentEnc instance. func New(cc *cryptocore.CryptoCore, plainBS uint64, forceDecode bool) *ContentEnc { + tlog.Debug.Printf("contentenc.New: plainBS=%d, forceDecode=%v", + plainBS, forceDecode) + if fuse.MAX_KERNEL_WRITE%plainBS != 0 { log.Panicf("unaligned MAX_KERNEL_WRITE=%d", fuse.MAX_KERNEL_WRITE) } diff --git a/internal/cryptocore/cryptocore.go b/internal/cryptocore/cryptocore.go index d66f390..9f5b9bd 100644 --- a/internal/cryptocore/cryptocore.go +++ b/internal/cryptocore/cryptocore.go @@ -36,6 +36,19 @@ const ( BackendAESSIV AEADTypeEnum = 5 ) +func (a AEADTypeEnum) String() string { + switch a { + case BackendOpenSSL: + return "BackendOpenSSL" + case BackendGoGCM: + return "BackendGoGCM" + case BackendAESSIV: + return "BackendAESSIV" + default: + return fmt.Sprintf("%d", a) + } +} + // CryptoCore is the low level crypto implementation. type CryptoCore struct { // EME is used for filename encryption. @@ -58,6 +71,9 @@ type CryptoCore struct { // Note: "key" is either the scrypt hash of the password (when decrypting // a config file) or the masterkey (when finally mounting the filesystem). func New(key []byte, aeadType AEADTypeEnum, IVBitLen int, useHKDF bool, forceDecode bool) *CryptoCore { + tlog.Debug.Printf("cryptocore.New: key=%d bytes, aeadType=%v, IVBitLen=%d, useHKDF=%v, forceDecode=%v", + len(key), aeadType, IVBitLen, useHKDF, forceDecode) + if len(key) != KeyLen { log.Panic(fmt.Sprintf("Unsupported key length %d", len(key))) } diff --git a/main.go b/main.go index edd61ff..9bd0c78 100644 --- a/main.go +++ b/main.go @@ -176,6 +176,7 @@ func main() { if args.debug { tlog.Debug.Enabled = true } + tlog.Debug.Printf("cli args: %q", os.Args) // "-v" if args.version { tlog.Debug.Printf("openssl=%v\n", args.openssl) @@ -282,12 +283,6 @@ func main() { if args.cpuprofile != "" || args.memprofile != "" || args.trace != "" { tlog.Info.Printf("Note: You must unmount gracefully, otherwise the profile file(s) will stay empty!\n") } - // "-openssl" - if !args.openssl { - tlog.Debug.Printf("OpenSSL disabled, using Go GCM") - } else { - tlog.Debug.Printf("OpenSSL enabled") - } // Operation flags nOps := countOpFlags(&args) if nOps == 0 { diff --git a/mount.go b/mount.go index 7f818d1..b146660 100644 --- a/mount.go +++ b/mount.go @@ -117,8 +117,6 @@ func doMount(args *argContainer) { args.noprealloc = true } } - // We cannot use JSON for pretty-printing as the fields are unexported - tlog.Debug.Printf("cli args: %#v", args) // Initialize gocryptfs (read config file, ask for password, ...) fs, wipeKeys := initFuseFrontend(args) // Try to wipe secret keys from memory after unmount @@ -308,7 +306,6 @@ func initFuseFrontend(args *argContainer) (rootNode fs.InodeEmbedder, wipeKeys f if args.allow_other && os.Getuid() == 0 { frontendArgs.PreserveOwner = true } - tlog.Debug.Printf("frontendArgs: %s", tlog.JSONDump(frontendArgs)) // Init crypto backend cCore := cryptocore.New(masterkey, cryptoBackend, contentenc.DefaultIVBits, args.hkdf, args.forcedecode) @@ -321,6 +318,7 @@ func initFuseFrontend(args *argContainer) (rootNode fs.InodeEmbedder, wipeKeys f } masterkey = nil // Spawn fusefrontend + tlog.Debug.Printf("frontendArgs: %s", tlog.JSONDump(frontendArgs)) if args.reverse { if cryptoBackend != cryptocore.BackendAESSIV { log.Panic("reverse mode must use AES-SIV, everything else is insecure") From e244b514913a31293eac1dc728f5b1026ab84b98 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Mon, 21 Jun 2021 11:48:16 +0200 Subject: [PATCH 11/29] tests: cli: add TestZerokey TestZerokey verifies that `gocryptfs -zerokey` uses the same options as `gocryptfs -init`. --- tests/cli/zerokey.go | 60 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 tests/cli/zerokey.go diff --git a/tests/cli/zerokey.go b/tests/cli/zerokey.go new file mode 100644 index 0000000..b809d1f --- /dev/null +++ b/tests/cli/zerokey.go @@ -0,0 +1,60 @@ +package cli + +import ( + "io/ioutil" + "os" + "os/exec" + "testing" + + "github.com/rfjakob/gocryptfs/tests/test_helpers" +) + +// TestZerokey verifies that `gocryptfs -zerokey` uses the same options as +// `gocryptfs -init`. +func TestZerokey(t *testing.T) { + // Create FS + dir := test_helpers.InitFS(t) + + // Change masterkey to all-zero using password change + args := []string{"-q", "-passwd", "-masterkey", + "00000000-00000000-00000000-00000000-00000000-00000000-00000000-00000000"} + args = append(args, dir) + cmd := exec.Command(test_helpers.GocryptfsBinary, args...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + p, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + err = cmd.Start() + if err != nil { + t.Error(err) + } + // New password = old password + p.Write([]byte("test\n")) + p.Close() + err = cmd.Wait() + if err != nil { + t.Error(err) + } + + // Add content + mnt := dir + ".mnt" + test_helpers.MountOrFatal(t, dir, mnt, "-extpass", "echo test") + file1 := mnt + "/file1" + err = ioutil.WriteFile(file1, []byte("somecontent"), 0600) + if err != nil { + t.Fatal(err) + } + test_helpers.UnmountPanic(mnt) + + // Mount using -zerokey and verify we get the same result + test_helpers.MountOrFatal(t, dir, mnt, "-extpass", "echo test") + content, err := ioutil.ReadFile(file1) + if err != nil { + t.Error(err) + } else if string(content) != "somecontent" { + t.Errorf("wrong content: %q", string(content)) + } + test_helpers.UnmountPanic(mnt) +} From 2efef1e270a0e374c479326ab2c296b5e9fdc34d Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Mon, 21 Jun 2021 11:53:33 +0200 Subject: [PATCH 12/29] nametransform: delete NameTransformer interface Useless layer of indirection. --- internal/fusefrontend/root_node.go | 4 +-- .../fusefrontend_reverse/excluder_test.go | 9 ------ internal/fusefrontend_reverse/mocks_test.go | 32 ------------------- internal/fusefrontend_reverse/root_node.go | 4 +-- internal/nametransform/names.go | 17 ---------- tests/defaults/main_test.go | 3 ++ tests/test_helpers/helpers.go | 2 ++ 7 files changed, 9 insertions(+), 62 deletions(-) delete mode 100644 internal/fusefrontend_reverse/mocks_test.go diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go index 35b7be0..af4407e 100644 --- a/internal/fusefrontend/root_node.go +++ b/internal/fusefrontend/root_node.go @@ -27,7 +27,7 @@ type RootNode struct { // states dirIVLock sync.RWMutex // Filename encryption helper - nameTransform nametransform.NameTransformer + nameTransform *nametransform.NameTransform // Content encryption helper contentEnc *contentenc.ContentEnc // This lock is used by openWriteOnlyFile() to block concurrent opens while @@ -54,7 +54,7 @@ type RootNode struct { inoMap inomap.TranslateStater } -func NewRootNode(args Args, c *contentenc.ContentEnc, n nametransform.NameTransformer) *RootNode { +func NewRootNode(args Args, c *contentenc.ContentEnc, n *nametransform.NameTransform) *RootNode { if args.SerializeReads { serialize_reads.InitSerializer() } diff --git a/internal/fusefrontend_reverse/excluder_test.go b/internal/fusefrontend_reverse/excluder_test.go index d6cfef3..e44c0e0 100644 --- a/internal/fusefrontend_reverse/excluder_test.go +++ b/internal/fusefrontend_reverse/excluder_test.go @@ -64,12 +64,3 @@ func TestShouldReturnFalseIfThereAreNoExclusions(t *testing.T) { t.Error("Should not exclude any path if no exclusions were specified") } } - -func TestShouldCallIgnoreParserToCheckExclusion(t *testing.T) { - rfs, ignorerMock := createRFSWithMocks() - - rfs.isExcludedPlain("some/path") - if ignorerMock.calledWith != "some/path" { - t.Error("Failed to call IgnoreParser") - } -} diff --git a/internal/fusefrontend_reverse/mocks_test.go b/internal/fusefrontend_reverse/mocks_test.go deleted file mode 100644 index 2d14c1d..0000000 --- a/internal/fusefrontend_reverse/mocks_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package fusefrontend_reverse - -import ( - "github.com/rfjakob/gocryptfs/internal/nametransform" -) - -type IgnoreParserMock struct { - toExclude string - calledWith string -} - -func (parser *IgnoreParserMock) MatchesPath(f string) bool { - parser.calledWith = f - return f == parser.toExclude -} - -type NameTransformMock struct { - nametransform.NameTransform -} - -func (n *NameTransformMock) DecryptName(cipherName string, iv []byte) (string, error) { - return "mockdecrypt_" + cipherName, nil -} - -func createRFSWithMocks() (*RootNode, *IgnoreParserMock) { - ignorerMock := &IgnoreParserMock{} - nameTransformMock := &NameTransformMock{} - var rfs RootNode - rfs.excluder = ignorerMock - rfs.nameTransform = nameTransformMock - return &rfs, ignorerMock -} diff --git a/internal/fusefrontend_reverse/root_node.go b/internal/fusefrontend_reverse/root_node.go index 10b0d69..b072f85 100644 --- a/internal/fusefrontend_reverse/root_node.go +++ b/internal/fusefrontend_reverse/root_node.go @@ -28,7 +28,7 @@ type RootNode struct { // Stores configuration arguments args fusefrontend.Args // Filename encryption helper - nameTransform nametransform.NameTransformer + nameTransform *nametransform.NameTransform // Content encryption helper contentEnc *contentenc.ContentEnc // Tests whether a path is excluded (hidden) from the user. Used by -exclude. @@ -41,7 +41,7 @@ type RootNode struct { // NewRootNode returns an encrypted FUSE overlay filesystem. // In this case (reverse mode) the backing directory is plain-text and // ReverseFS provides an encrypted view. -func NewRootNode(args fusefrontend.Args, c *contentenc.ContentEnc, n nametransform.NameTransformer) *RootNode { +func NewRootNode(args fusefrontend.Args, c *contentenc.ContentEnc, n *nametransform.NameTransform) *RootNode { rn := &RootNode{ args: args, nameTransform: n, diff --git a/internal/nametransform/names.go b/internal/nametransform/names.go index afc0f5d..2ee52e4 100644 --- a/internal/nametransform/names.go +++ b/internal/nametransform/names.go @@ -19,23 +19,6 @@ const ( BadNameFlag = " GOCRYPTFS_BAD_NAME" ) -// NameTransformer is an interface used to transform filenames. -type NameTransformer interface { - DecryptName(cipherName string, iv []byte) (string, error) - EncryptName(plainName string, iv []byte) (string, error) - EncryptAndHashName(name string, iv []byte) (string, error) - EncryptAndHashBadName(name string, iv []byte, dirfd int) (string, error) - // HashLongName - take the hash of a long string "name" and return - // "gocryptfs.longname.[sha256]" - // - // This function does not do any I/O. - HashLongName(name string) string - HaveBadnamePatterns() bool - WriteLongNameAt(dirfd int, hashName string, plainName string) error - B64EncodeToString(src []byte) string - B64DecodeString(s string) ([]byte, error) -} - // NameTransform is used to transform filenames. type NameTransform struct { emeCipher *eme.EMECipher diff --git a/tests/defaults/main_test.go b/tests/defaults/main_test.go index 8873f8f..e7e065e 100644 --- a/tests/defaults/main_test.go +++ b/tests/defaults/main_test.go @@ -18,6 +18,9 @@ import ( func TestMain(m *testing.M) { test_helpers.ResetTmpDir(true) + // TestZerokey() in tests/cli verifies that mounting with `-zerokey` is equivalent + // to mounting with a config file with all-default options (just the masterkey + // set to all-zero). test_helpers.MountOrExit(test_helpers.DefaultCipherDir, test_helpers.DefaultPlainDir, "-zerokey") r := m.Run() test_helpers.UnmountPanic(test_helpers.DefaultPlainDir) diff --git a/tests/test_helpers/helpers.go b/tests/test_helpers/helpers.go index b98c0f7..5bc1298 100644 --- a/tests/test_helpers/helpers.go +++ b/tests/test_helpers/helpers.go @@ -141,6 +141,8 @@ func isExt4(path string) bool { // gocryptfs -q -init -extpass "echo test" -scryptn=10 $extraArgs $cipherdir // // It returns cipherdir without a trailing slash. +// +// If t is set, t.Fatal() is called on error, log.Panic() otherwise. func InitFS(t *testing.T, extraArgs ...string) string { prefix := "x." if t != nil { From 689b74835bd38ebaf87ba0e205c10b9594e51863 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Mon, 21 Jun 2021 12:08:18 +0200 Subject: [PATCH 13/29] nametransform: gather badname functions in badname.go --- internal/nametransform/badname.go | 91 +++++++++++++++++++++++++++++++ internal/nametransform/diriv.go | 78 -------------------------- internal/nametransform/names.go | 48 +++++++++------- 3 files changed, 118 insertions(+), 99 deletions(-) create mode 100644 internal/nametransform/badname.go diff --git a/internal/nametransform/badname.go b/internal/nametransform/badname.go new file mode 100644 index 0000000..5cc9799 --- /dev/null +++ b/internal/nametransform/badname.go @@ -0,0 +1,91 @@ +package nametransform + +import ( + "crypto/aes" + "path/filepath" + "strings" + "syscall" + + "golang.org/x/sys/unix" + + "github.com/rfjakob/gocryptfs/internal/syscallcompat" +) + +const ( + // BadNameFlag is appended to filenames in plaintext view if a corrupt + // ciphername is shown due to a matching `-badname` pattern + BadNameFlag = " GOCRYPTFS_BAD_NAME" +) + +// EncryptAndHashBadName tries to find the "name" substring, which (encrypted and hashed) +// leads to an unique existing file +// Returns ENOENT if cipher file does not exist or is not unique +func (be *NameTransform) EncryptAndHashBadName(name string, iv []byte, dirfd int) (cName string, err error) { + var st unix.Stat_t + var filesFound int + lastFoundName, err := be.EncryptAndHashName(name, iv) + if !strings.HasSuffix(name, BadNameFlag) || err != nil { + //Default mode: same behaviour on error or no BadNameFlag on "name" + return lastFoundName, err + } + //Default mode: Check if File extists without modifications + err = syscallcompat.Fstatat(dirfd, lastFoundName, &st, unix.AT_SYMLINK_NOFOLLOW) + if err == nil { + //file found, return result + return lastFoundName, nil + } + //BadName Mode: check if the name was tranformed without change (badname suffix and undecryptable cipher name) + err = syscallcompat.Fstatat(dirfd, name[:len(name)-len(BadNameFlag)], &st, unix.AT_SYMLINK_NOFOLLOW) + if err == nil { + filesFound++ + lastFoundName = name[:len(name)-len(BadNameFlag)] + } + // search for the longest badname pattern match + for charpos := len(name) - len(BadNameFlag); charpos > 0; charpos-- { + //only use original cipher name and append assumed suffix (without badname flag) + cNamePart, err := be.EncryptName(name[:charpos], iv) + if err != nil { + //expand suffix on error + continue + } + if be.longNames && len(cName) > NameMax { + cNamePart = be.HashLongName(cName) + } + cNameBadReverse := cNamePart + name[charpos:len(name)-len(BadNameFlag)] + err = syscallcompat.Fstatat(dirfd, cNameBadReverse, &st, unix.AT_SYMLINK_NOFOLLOW) + if err == nil { + filesFound++ + lastFoundName = cNameBadReverse + } + } + if filesFound == 1 { + return lastFoundName, nil + } + // more than 1 possible file found, ignore + return "", syscall.ENOENT +} + +func (n *NameTransform) decryptBadname(cipherName string, iv []byte) (string, error) { + for _, pattern := range n.badnamePatterns { + match, err := filepath.Match(pattern, cipherName) + // Pattern should have been validated already + if err == nil && match { + // Find longest decryptable substring + // At least 16 bytes due to AES --> at least 22 characters in base64 + nameMin := n.B64.EncodedLen(aes.BlockSize) + for charpos := len(cipherName) - 1; charpos >= nameMin; charpos-- { + res, err := n.decryptName(cipherName[:charpos], iv) + if err == nil { + return res + cipherName[charpos:] + BadNameFlag, nil + } + } + return cipherName + BadNameFlag, nil + } + } + return "", syscall.EBADMSG +} + +// HaveBadnamePatterns returns true if `-badname` patterns were provided +func (n *NameTransform) HaveBadnamePatterns() bool { + return len(n.badnamePatterns) > 0 +} diff --git a/internal/nametransform/diriv.go b/internal/nametransform/diriv.go index d62b3fb..b10c899 100644 --- a/internal/nametransform/diriv.go +++ b/internal/nametransform/diriv.go @@ -5,14 +5,11 @@ import ( "fmt" "io" "os" - "path/filepath" - "strings" "syscall" "github.com/rfjakob/gocryptfs/internal/cryptocore" "github.com/rfjakob/gocryptfs/internal/syscallcompat" "github.com/rfjakob/gocryptfs/internal/tlog" - "golang.org/x/sys/unix" ) const ( @@ -95,78 +92,3 @@ func WriteDirIVAt(dirfd int) error { } return nil } - -// encryptAndHashName encrypts "name" and hashes it to a longname if it is -// too long. -// Returns ENAMETOOLONG if "name" is longer than 255 bytes. -func (be *NameTransform) EncryptAndHashName(name string, iv []byte) (string, error) { - // Prevent the user from creating files longer than 255 chars. - if len(name) > NameMax { - return "", syscall.ENAMETOOLONG - } - cName, err := be.EncryptName(name, iv) - if err != nil { - return "", err - } - if be.longNames && len(cName) > NameMax { - return be.HashLongName(cName), nil - } - return cName, nil -} - -// EncryptAndHashBadName tries to find the "name" substring, which (encrypted and hashed) -// leads to an unique existing file -// Returns ENOENT if cipher file does not exist or is not unique -func (be *NameTransform) EncryptAndHashBadName(name string, iv []byte, dirfd int) (cName string, err error) { - var st unix.Stat_t - var filesFound int - lastFoundName, err := be.EncryptAndHashName(name, iv) - if !strings.HasSuffix(name, BadNameFlag) || err != nil { - //Default mode: same behaviour on error or no BadNameFlag on "name" - return lastFoundName, err - } - //Default mode: Check if File extists without modifications - err = syscallcompat.Fstatat(dirfd, lastFoundName, &st, unix.AT_SYMLINK_NOFOLLOW) - if err == nil { - //file found, return result - return lastFoundName, nil - } - //BadName Mode: check if the name was tranformed without change (badname suffix and undecryptable cipher name) - err = syscallcompat.Fstatat(dirfd, name[:len(name)-len(BadNameFlag)], &st, unix.AT_SYMLINK_NOFOLLOW) - if err == nil { - filesFound++ - lastFoundName = name[:len(name)-len(BadNameFlag)] - } - // search for the longest badname pattern match - for charpos := len(name) - len(BadNameFlag); charpos > 0; charpos-- { - //only use original cipher name and append assumed suffix (without badname flag) - cNamePart, err := be.EncryptName(name[:charpos], iv) - if err != nil { - //expand suffix on error - continue - } - if be.longNames && len(cName) > NameMax { - cNamePart = be.HashLongName(cName) - } - cNameBadReverse := cNamePart + name[charpos:len(name)-len(BadNameFlag)] - err = syscallcompat.Fstatat(dirfd, cNameBadReverse, &st, unix.AT_SYMLINK_NOFOLLOW) - if err == nil { - filesFound++ - lastFoundName = cNameBadReverse - } - } - if filesFound == 1 { - return lastFoundName, nil - } - // more than 1 possible file found, ignore - return "", syscall.ENOENT -} - -// Dir is like filepath.Dir but returns "" instead of ".". -func Dir(path string) string { - d := filepath.Dir(path) - if d == "." { - return "" - } - return d -} diff --git a/internal/nametransform/names.go b/internal/nametransform/names.go index 2ee52e4..566f0c7 100644 --- a/internal/nametransform/names.go +++ b/internal/nametransform/names.go @@ -15,8 +15,6 @@ import ( const ( // Like ext4, we allow at most 255 bytes for a file name. NameMax = 255 - //BadNameFlag is appended to filenames in plain mode if a ciphername is inavlid but is shown - BadNameFlag = " GOCRYPTFS_BAD_NAME" ) // NameTransform is used to transform filenames. @@ -51,22 +49,8 @@ func New(e *eme.EMECipher, longNames bool, raw64 bool, badname []string) *NameTr // filename "cipherName", and failing that checks if it can be bypassed func (n *NameTransform) DecryptName(cipherName string, iv []byte) (string, error) { res, err := n.decryptName(cipherName, iv) - if err != nil { - for _, pattern := range n.badnamePatterns { - match, err := filepath.Match(pattern, cipherName) - if err == nil && match { // Pattern should have been validated already - // Find longest decryptable substring - // At least 16 bytes due to AES --> at least 22 characters in base64 - nameMin := n.B64.EncodedLen(aes.BlockSize) - for charpos := len(cipherName) - 1; charpos >= nameMin; charpos-- { - res, err = n.decryptName(cipherName[:charpos], iv) - if err == nil { - return res + cipherName[charpos:] + BadNameFlag, nil - } - } - return cipherName + BadNameFlag, nil - } - } + if err != nil && n.HaveBadnamePatterns() { + return n.decryptBadname(cipherName, iv) } return res, err } @@ -117,6 +101,24 @@ func (n *NameTransform) EncryptName(plainName string, iv []byte) (cipherName64 s return cipherName64, nil } +// EncryptAndHashName encrypts "name" and hashes it to a longname if it is +// too long. +// Returns ENAMETOOLONG if "name" is longer than 255 bytes. +func (be *NameTransform) EncryptAndHashName(name string, iv []byte) (string, error) { + // Prevent the user from creating files longer than 255 chars. + if len(name) > NameMax { + return "", syscall.ENAMETOOLONG + } + cName, err := be.EncryptName(name, iv) + if err != nil { + return "", err + } + if be.longNames && len(cName) > NameMax { + return be.HashLongName(cName), nil + } + return cName, nil +} + // B64EncodeToString returns a Base64-encoded string func (n *NameTransform) B64EncodeToString(src []byte) string { return n.B64.EncodeToString(src) @@ -127,7 +129,11 @@ func (n *NameTransform) B64DecodeString(s string) ([]byte, error) { return n.B64.DecodeString(s) } -// HaveBadnamePatterns returns true if BadName patterns were provided -func (n *NameTransform) HaveBadnamePatterns() bool { - return len(n.badnamePatterns) > 0 +// Dir is like filepath.Dir but returns "" instead of ".". +func Dir(path string) string { + d := filepath.Dir(path) + if d == "." { + return "" + } + return d } From 05b813f2026ebe6bff33cb600775823ee8a7df6e Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Mon, 21 Jun 2021 12:12:44 +0200 Subject: [PATCH 14/29] nametransform: rename BadNameFlag to BadnameSuffix --- internal/nametransform/badname.go | 18 +++++++++--------- tests/cli/cli_test.go | 10 +++++----- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/internal/nametransform/badname.go b/internal/nametransform/badname.go index 5cc9799..44b7d36 100644 --- a/internal/nametransform/badname.go +++ b/internal/nametransform/badname.go @@ -12,9 +12,9 @@ import ( ) const ( - // BadNameFlag is appended to filenames in plaintext view if a corrupt + // BadnameSuffix is appended to filenames in plaintext view if a corrupt // ciphername is shown due to a matching `-badname` pattern - BadNameFlag = " GOCRYPTFS_BAD_NAME" + BadnameSuffix = " GOCRYPTFS_BAD_NAME" ) // EncryptAndHashBadName tries to find the "name" substring, which (encrypted and hashed) @@ -24,7 +24,7 @@ func (be *NameTransform) EncryptAndHashBadName(name string, iv []byte, dirfd int var st unix.Stat_t var filesFound int lastFoundName, err := be.EncryptAndHashName(name, iv) - if !strings.HasSuffix(name, BadNameFlag) || err != nil { + if !strings.HasSuffix(name, BadnameSuffix) || err != nil { //Default mode: same behaviour on error or no BadNameFlag on "name" return lastFoundName, err } @@ -35,13 +35,13 @@ func (be *NameTransform) EncryptAndHashBadName(name string, iv []byte, dirfd int return lastFoundName, nil } //BadName Mode: check if the name was tranformed without change (badname suffix and undecryptable cipher name) - err = syscallcompat.Fstatat(dirfd, name[:len(name)-len(BadNameFlag)], &st, unix.AT_SYMLINK_NOFOLLOW) + err = syscallcompat.Fstatat(dirfd, name[:len(name)-len(BadnameSuffix)], &st, unix.AT_SYMLINK_NOFOLLOW) if err == nil { filesFound++ - lastFoundName = name[:len(name)-len(BadNameFlag)] + lastFoundName = name[:len(name)-len(BadnameSuffix)] } // search for the longest badname pattern match - for charpos := len(name) - len(BadNameFlag); charpos > 0; charpos-- { + for charpos := len(name) - len(BadnameSuffix); charpos > 0; charpos-- { //only use original cipher name and append assumed suffix (without badname flag) cNamePart, err := be.EncryptName(name[:charpos], iv) if err != nil { @@ -51,7 +51,7 @@ func (be *NameTransform) EncryptAndHashBadName(name string, iv []byte, dirfd int if be.longNames && len(cName) > NameMax { cNamePart = be.HashLongName(cName) } - cNameBadReverse := cNamePart + name[charpos:len(name)-len(BadNameFlag)] + cNameBadReverse := cNamePart + name[charpos:len(name)-len(BadnameSuffix)] err = syscallcompat.Fstatat(dirfd, cNameBadReverse, &st, unix.AT_SYMLINK_NOFOLLOW) if err == nil { filesFound++ @@ -76,10 +76,10 @@ func (n *NameTransform) decryptBadname(cipherName string, iv []byte) (string, er for charpos := len(cipherName) - 1; charpos >= nameMin; charpos-- { res, err := n.decryptName(cipherName[:charpos], iv) if err == nil { - return res + cipherName[charpos:] + BadNameFlag, nil + return res + cipherName[charpos:] + BadnameSuffix, nil } } - return cipherName + BadNameFlag, nil + return cipherName + BadnameSuffix, nil } } return "", syscall.EBADMSG diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go index 08c1b83..df36822 100644 --- a/tests/cli/cli_test.go +++ b/tests/cli/cli_test.go @@ -764,7 +764,7 @@ func TestBadname(t *testing.T) { } // Case 2: File with invalid suffix in plain name but valid cipher file - file = mnt + "/" + validFileName + nametransform.BadNameFlag + file = mnt + "/" + validFileName + nametransform.BadnameSuffix err = ioutil.WriteFile(file, []byte("Content Case 2."), 0600) if err != nil { t.Fatal(err) @@ -819,12 +819,12 @@ func TestBadname(t *testing.T) { searchstrings := []string{ validFileName, - validFileName + nametransform.BadNameFlag, + validFileName + nametransform.BadnameSuffix, "", - validFileName + invalidSuffix + nametransform.BadNameFlag, - encryptedfilename[:len(encryptedfilename)-2] + invalidSuffix + nametransform.BadNameFlag, + validFileName + invalidSuffix + nametransform.BadnameSuffix, + encryptedfilename[:len(encryptedfilename)-2] + invalidSuffix + nametransform.BadnameSuffix, "", - validFileName + "wrongPattern" + nametransform.BadNameFlag} + validFileName + "wrongPattern" + nametransform.BadnameSuffix} results := []bool{false, false, true, false, false, true, true} var filecontent string var filebytes []byte From 84e702126ac4f017e12150532bfaed675dee2927 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Fri, 25 Jun 2021 11:33:18 +0200 Subject: [PATCH 15/29] fusefrontend: implement recursive diriv caching The new contrib/maxlen.bash showed that we have exponential runtime with respect to directory depth. The new recursive diriv caching is a lot smarter as it caches intermediate lookups. maxlen.bash now completes in a few seconds. xfstests results same as https://github.com/rfjakob/fuse-xfstests/blob/2d158e4c82be85c15269af77498e353f928f4fab/screenlog.0 : Failures: generic/035 generic/062 generic/080 generic/093 generic/099 generic/215 generic/285 generic/319 generic/426 generic/444 generic/467 generic/477 generic/523 Failed 13 of 580 tests benchmark.bash results are identical: $ ./benchmark.bash Testing gocryptfs at /tmp/benchmark.bash.BdQ: gocryptfs v2.0.1-17-g6b09bc0; go-fuse v2.1.1-0.20210611132105-24a1dfe6b4f8; 2021-06-25 go1.16.5 linux/amd64 /tmp/benchmark.bash.BdQ.mnt is a mountpoint WRITE: 262144000 bytes (262 MB, 250 MiB) copied, 0,4821 s, 544 MB/s READ: 262144000 bytes (262 MB, 250 MiB) copied, 0,266061 s, 985 MB/s UNTAR: 8,280 MD5: 4,564 LS: 1,745 RM: 2,244 --- internal/fusefrontend/dircache.go | 8 +- internal/fusefrontend/node.go | 8 +- internal/fusefrontend/node_dir_ops.go | 4 +- internal/fusefrontend/node_helpers.go | 89 -------------- internal/fusefrontend/node_open_create.go | 2 +- internal/fusefrontend/node_prepare_syscall.go | 116 ++++++++++++++++++ internal/fusefrontend/node_xattr_darwin.go | 8 +- internal/fusefrontend/node_xattr_linux.go | 8 +- internal/fusefrontend/root_node.go | 11 +- 9 files changed, 144 insertions(+), 110 deletions(-) create mode 100644 internal/fusefrontend/node_prepare_syscall.go diff --git a/internal/fusefrontend/dircache.go b/internal/fusefrontend/dircache.go index 6732de1..8c285fa 100644 --- a/internal/fusefrontend/dircache.go +++ b/internal/fusefrontend/dircache.go @@ -7,7 +7,6 @@ import ( "syscall" "time" - "github.com/rfjakob/gocryptfs/internal/nametransform" "github.com/rfjakob/gocryptfs/internal/tlog" ) @@ -50,6 +49,9 @@ func (e *dirCacheEntry) Clear() { type dirCache struct { sync.Mutex + // Expected length of the stored IVs. Only used for sanity checks. + // Usually set to 16, but 0 in plaintextnames mode. + ivLen int // Cache entries entries [dirCacheSize]dirCacheEntry // Where to store the next entry (index into entries) @@ -77,7 +79,7 @@ func (d *dirCache) Clear() { func (d *dirCache) Store(node *Node, fd int, iv []byte) { // Note: package ensurefds012, imported from main, guarantees that dirCache // can never get fds 0,1,2. - if fd <= 0 || len(iv) != nametransform.DirIVLen { + if fd <= 0 || len(iv) != d.ivLen { log.Panicf("Store sanity check failed: fd=%d len=%d", fd, len(iv)) } d.Lock() @@ -139,7 +141,7 @@ func (d *dirCache) Lookup(node *Node) (fd int, iv []byte) { if enableStats { d.hits++ } - if fd <= 0 || len(iv) != nametransform.DirIVLen { + if fd <= 0 || len(iv) != d.ivLen { log.Panicf("Lookup sanity check failed: fd=%d len=%d", fd, len(iv)) } d.dbg("dirCache.Lookup %p hit fd=%d dup=%d iv=%x\n", node, e.fd, fd, iv) diff --git a/internal/fusefrontend/node.go b/internal/fusefrontend/node.go index 8370a22..dd446a3 100644 --- a/internal/fusefrontend/node.go +++ b/internal/fusefrontend/node.go @@ -52,7 +52,7 @@ func (n *Node) Getattr(ctx context.Context, f fs.FileHandle, out *fuse.AttrOut) return f.(fs.FileGetattrer).Getattr(ctx, out) } - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -106,7 +106,7 @@ func (n *Node) Unlink(ctx context.Context, name string) (errno syscall.Errno) { // // Symlink-safe through openBackingDir() + Readlinkat(). func (n *Node) Readlink(ctx context.Context) (out []byte, errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -123,7 +123,7 @@ func (n *Node) Setattr(ctx context.Context, f fs.FileHandle, in *fuse.SetAttrIn, return f2.Setattr(ctx, in, out) } - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -271,7 +271,7 @@ func (n *Node) Link(ctx context.Context, target fs.InodeEmbedder, name string, o defer syscall.Close(dirfd) n2 := toNode(target) - dirfd2, cName2, errno := n2.prepareAtSyscall("") + dirfd2, cName2, errno := n2.prepareAtSyscallMyself() if errno != 0 { return } diff --git a/internal/fusefrontend/node_dir_ops.go b/internal/fusefrontend/node_dir_ops.go index 001c23a..0f48876 100644 --- a/internal/fusefrontend/node_dir_ops.go +++ b/internal/fusefrontend/node_dir_ops.go @@ -154,7 +154,7 @@ func (n *Node) Mkdir(ctx context.Context, name string, mode uint32, out *fuse.En // This function is symlink-safe through use of openBackingDir() and // ReadDirIVAt(). func (n *Node) Readdir(ctx context.Context) (fs.DirStream, syscall.Errno) { - parentDirFd, cDirName, errno := n.prepareAtSyscall("") + parentDirFd, cDirName, errno := n.prepareAtSyscallMyself() if errno != 0 { return nil, errno } @@ -360,7 +360,7 @@ retry: // Opendir is a FUSE call to check if the directory can be opened. func (n *Node) Opendir(ctx context.Context) (errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } diff --git a/internal/fusefrontend/node_helpers.go b/internal/fusefrontend/node_helpers.go index f2d1e5e..ce2e8a9 100644 --- a/internal/fusefrontend/node_helpers.go +++ b/internal/fusefrontend/node_helpers.go @@ -2,16 +2,12 @@ package fusefrontend import ( "context" - "log" - "path/filepath" - "sync/atomic" "syscall" "github.com/hanwen/go-fuse/v2/fs" "github.com/hanwen/go-fuse/v2/fuse" - "github.com/rfjakob/gocryptfs/internal/nametransform" "github.com/rfjakob/gocryptfs/internal/syscallcompat" "github.com/rfjakob/gocryptfs/internal/tlog" ) @@ -80,91 +76,6 @@ func (n *Node) rootNode() *RootNode { return n.Root().Operations().(*RootNode) } -// prepareAtSyscall returns a (dirfd, cName) pair that can be used -// with the "___at" family of system calls (openat, fstatat, unlinkat...) to -// access the backing encrypted directory. -// -// If you pass a `child` file name, the (dirfd, cName) pair will refer to -// a child of this node. -// If `child` is empty, the (dirfd, cName) pair refers to this node itself. For -// the root node, that means (dirfd, "."). -func (n *Node) prepareAtSyscall(child string) (dirfd int, cName string, errno syscall.Errno) { - rn := n.rootNode() - // all filesystem operations go through prepareAtSyscall(), so this is a - // good place to reset the idle marker. - atomic.StoreUint32(&rn.IsIdle, 0) - - // root node itself is special - if child == "" && n.IsRoot() { - var err error - dirfd, cName, err = rn.openBackingDir("") - if err != nil { - errno = fs.ToErrno(err) - } - return - } - - // normal node itself can be converted to child of parent node - if child == "" { - name, p1 := n.Parent() - if p1 == nil || name == "" { - return -1, "", syscall.ENOENT - } - p2 := toNode(p1.Operations()) - return p2.prepareAtSyscall(name) - } - - // Cache lookup - // TODO make it work for plaintextnames as well? - cacheable := (!rn.args.PlaintextNames) - if cacheable { - var iv []byte - dirfd, iv = rn.dirCache.Lookup(n) - if dirfd > 0 { - var cName string - var err error - if rn.nameTransform.HaveBadnamePatterns() { - //BadName allowed, try to determine filenames - cName, err = rn.nameTransform.EncryptAndHashBadName(child, iv, dirfd) - } else { - cName, err = rn.nameTransform.EncryptAndHashName(child, iv) - } - - if err != nil { - return -1, "", fs.ToErrno(err) - } - return dirfd, cName, 0 - } - } - - // Slowpath - if child == "" { - log.Panicf("BUG: child name is empty - this cannot happen") - } - p := filepath.Join(n.Path(), child) - if rn.isFiltered(p) { - errno = syscall.EPERM - return - } - dirfd, cName, err := rn.openBackingDir(p) - if err != nil { - errno = fs.ToErrno(err) - return - } - - // Cache store - if cacheable { - // TODO: openBackingDir already calls ReadDirIVAt(). Avoid duplicate work? - iv, err := nametransform.ReadDirIVAt(dirfd) - if err != nil { - syscall.Close(dirfd) - return -1, "", fs.ToErrno(err) - } - rn.dirCache.Store(n, dirfd, iv) - } - return -} - // newChild attaches a new child inode to n. // The passed-in `st` will be modified to get a unique inode number // (or, in `-sharedstorage` mode, the inode number will be set to zero). diff --git a/internal/fusefrontend/node_open_create.go b/internal/fusefrontend/node_open_create.go index 8b31932..6385bc1 100644 --- a/internal/fusefrontend/node_open_create.go +++ b/internal/fusefrontend/node_open_create.go @@ -16,7 +16,7 @@ import ( // // Symlink-safe through Openat(). func (n *Node) Open(ctx context.Context, flags uint32) (fh fs.FileHandle, fuseFlags uint32, errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } diff --git a/internal/fusefrontend/node_prepare_syscall.go b/internal/fusefrontend/node_prepare_syscall.go new file mode 100644 index 0000000..fa558e8 --- /dev/null +++ b/internal/fusefrontend/node_prepare_syscall.go @@ -0,0 +1,116 @@ +package fusefrontend + +import ( + "sync/atomic" + "syscall" + + "github.com/rfjakob/gocryptfs/internal/tlog" + + "github.com/hanwen/go-fuse/v2/fs" + + "github.com/rfjakob/gocryptfs/internal/nametransform" + "github.com/rfjakob/gocryptfs/internal/syscallcompat" +) + +// prepareAtSyscall returns a (dirfd, cName) pair that can be used +// with the "___at" family of system calls (openat, fstatat, unlinkat...) to +// access the backing encrypted child file. +func (n *Node) prepareAtSyscall(child string) (dirfd int, cName string, errno syscall.Errno) { + if child == "" { + tlog.Warn.Printf("BUG: prepareAtSyscall: child=%q, should have called prepareAtSyscallMyself", child) + return n.prepareAtSyscallMyself() + } + + rn := n.rootNode() + + // All filesystem operations go through here, so this is a good place + // to reset the idle marker. + atomic.StoreUint32(&rn.IsIdle, 0) + + if n.IsRoot() && rn.isFiltered(child) { + return -1, "", syscall.EPERM + } + + var encryptName func(int, string, []byte) (string, error) + if !rn.args.PlaintextNames { + encryptName = func(dirfd int, child string, iv []byte) (cName string, err error) { + // Badname allowed, try to determine filenames + if rn.nameTransform.HaveBadnamePatterns() { + return rn.nameTransform.EncryptAndHashBadName(child, iv, dirfd) + } + return rn.nameTransform.EncryptAndHashName(child, iv) + } + } + + // Cache lookup + var iv []byte + dirfd, iv = rn.dirCache.Lookup(n) + if dirfd > 0 { + if rn.args.PlaintextNames { + return dirfd, child, 0 + } + var err error + cName, err = encryptName(dirfd, child, iv) + if err != nil { + syscall.Close(dirfd) + return -1, "", fs.ToErrno(err) + } + return + } + + // Slowpath: Open ourselves & read diriv + parentDirfd, myCName, errno := n.prepareAtSyscallMyself() + if errno != 0 { + return + } + defer syscall.Close(parentDirfd) + + dirfd, err := syscallcompat.Openat(parentDirfd, myCName, syscall.O_NOFOLLOW|syscall.O_DIRECTORY|syscallcompat.O_PATH, 0) + + // Cache store + if !rn.args.PlaintextNames { + var err error + iv, err = nametransform.ReadDirIVAt(dirfd) + if err != nil { + syscall.Close(dirfd) + return -1, "", fs.ToErrno(err) + } + } + rn.dirCache.Store(n, dirfd, iv) + + if rn.args.PlaintextNames { + return dirfd, child, 0 + } + + cName, err = encryptName(dirfd, child, iv) + if err != nil { + syscall.Close(dirfd) + return -1, "", fs.ToErrno(err) + } + + return +} + +func (n *Node) prepareAtSyscallMyself() (dirfd int, cName string, errno syscall.Errno) { + dirfd = -1 + + // Handle root node + if n.IsRoot() { + var err error + rn := n.rootNode() + dirfd, cName, err = rn.openBackingDir("") + if err != nil { + errno = fs.ToErrno(err) + } + return + } + + // Otherwise convert to prepareAtSyscall of parent node + myName, p1 := n.Parent() + if p1 == nil || myName == "" { + errno = syscall.ENOENT + return + } + parent := toNode(p1.Operations()) + return parent.prepareAtSyscall(myName) +} diff --git a/internal/fusefrontend/node_xattr_darwin.go b/internal/fusefrontend/node_xattr_darwin.go index 0f61a5d..31ba653 100644 --- a/internal/fusefrontend/node_xattr_darwin.go +++ b/internal/fusefrontend/node_xattr_darwin.go @@ -20,7 +20,7 @@ func filterXattrSetFlags(flags int) int { } func (n *Node) getXAttr(cAttr string) (out []byte, errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -42,7 +42,7 @@ func (n *Node) getXAttr(cAttr string) (out []byte, errno syscall.Errno) { } func (n *Node) setXAttr(context *fuse.Context, cAttr string, cData []byte, flags uint32) (errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -64,7 +64,7 @@ func (n *Node) setXAttr(context *fuse.Context, cAttr string, cData []byte, flags } func (n *Node) removeXAttr(cAttr string) (errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -86,7 +86,7 @@ func (n *Node) removeXAttr(cAttr string) (errno syscall.Errno) { } func (n *Node) listXAttr() (out []string, errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } diff --git a/internal/fusefrontend/node_xattr_linux.go b/internal/fusefrontend/node_xattr_linux.go index 9698282..212d4e1 100644 --- a/internal/fusefrontend/node_xattr_linux.go +++ b/internal/fusefrontend/node_xattr_linux.go @@ -17,7 +17,7 @@ func filterXattrSetFlags(flags int) int { } func (n *Node) getXAttr(cAttr string) (out []byte, errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -32,7 +32,7 @@ func (n *Node) getXAttr(cAttr string) (out []byte, errno syscall.Errno) { } func (n *Node) setXAttr(context *fuse.Context, cAttr string, cData []byte, flags uint32) (errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -44,7 +44,7 @@ func (n *Node) setXAttr(context *fuse.Context, cAttr string, cData []byte, flags } func (n *Node) removeXAttr(cAttr string) (errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } @@ -55,7 +55,7 @@ func (n *Node) removeXAttr(cAttr string) (errno syscall.Errno) { } func (n *Node) listXAttr() (out []string, errno syscall.Errno) { - dirfd, cName, errno := n.prepareAtSyscall("") + dirfd, cName, errno := n.prepareAtSyscallMyself() if errno != 0 { return } diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go index af4407e..ab0cdcd 100644 --- a/internal/fusefrontend/root_node.go +++ b/internal/fusefrontend/root_node.go @@ -61,11 +61,16 @@ func NewRootNode(args Args, c *contentenc.ContentEnc, n *nametransform.NameTrans if len(args.Exclude) > 0 { tlog.Warn.Printf("Forward mode does not support -exclude") } + ivLen := nametransform.DirIVLen + if args.PlaintextNames { + ivLen = 0 + } rn := &RootNode{ args: args, nameTransform: n, contentEnc: c, inoMap: inomap.New(), + dirCache: dirCache{ivLen: ivLen}, } // In `-sharedstorage` mode we always set the inode number to zero. // This makes go-fuse generate a new inode number for each lookup. @@ -122,15 +127,15 @@ func (rn *RootNode) reportMitigatedCorruption(item string) { } } -// isFiltered - check if plaintext "path" should be forbidden +// isFiltered - check if plaintext file "child" should be forbidden // // Prevents name clashes with internal files when file names are not encrypted -func (rn *RootNode) isFiltered(path string) bool { +func (rn *RootNode) isFiltered(child string) bool { if !rn.args.PlaintextNames { return false } // gocryptfs.conf in the root directory is forbidden - if path == configfile.ConfDefaultName { + if child == configfile.ConfDefaultName { tlog.Info.Printf("The name /%s is reserved when -plaintextnames is used\n", configfile.ConfDefaultName) return true From 389aba6a6b91f4bbf846caeb57ffdf177fd2cfdc Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Fri, 25 Jun 2021 14:00:20 +0200 Subject: [PATCH 16/29] maxlen.bash: suppress progress output if not on a terminal --- contrib/maxlen.bash | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/contrib/maxlen.bash b/contrib/maxlen.bash index a4aa082..14d40da 100755 --- a/contrib/maxlen.bash +++ b/contrib/maxlen.bash @@ -13,6 +13,13 @@ if [[ $# -ne 1 || $1 == -* ]]; then exit 1 fi +# Only show live progress if connected to a termial +# https://stackoverflow.com/a/911213/1380267 +INTERACTIVE=0 +if [[ -t 1 ]] ; then + INTERACTIVE=1 +fi + cd "$1" echo "Testing $PWD" @@ -22,7 +29,7 @@ NAME="" while true ; do NEXT="${NAME}x" if [[ -e $NEXT ]]; then - echo "error: file $NEXT already exists" + echo "error: file $PWD/$NEXT already exists" exit 1 fi touch $NEXT 2> /dev/null || break @@ -44,7 +51,10 @@ MAX_DIRNAME=${#NAME} echo "${#NAME}" for CHARS_PER_SUBDIR in 1 10 100 $MAX_DIRNAME ; do - echo -n " Maximum path length with $(printf %3d $CHARS_PER_SUBDIR) chars per subdir: " + echo -n " Maximum path length with $(printf %3d $CHARS_PER_SUBDIR) chars per subdir: " + if [[ $INTERACTIVE -eq 1 ]] ; then + echo -n " " + fi # Trick from https://stackoverflow.com/a/5349842/1380267 SUBDIR=$(printf 'x%.0s' $(seq 1 $CHARS_PER_SUBDIR)) mkdir "$SUBDIR" @@ -54,8 +64,10 @@ for CHARS_PER_SUBDIR in 1 10 100 $MAX_DIRNAME ; do NEXT="$P/$SUBDIR" mkdir "$NEXT" 2> /dev/null || break P=$NEXT - echo -n -e "\b\b\b\b" - printf %4d ${#P} + if [[ $INTERACTIVE -eq 1 ]] ; then + echo -n -e "\b\b\b\b" + printf %4d ${#P} + fi done # Then add one character at a time until we hit an error NAME="" @@ -67,7 +79,9 @@ for CHARS_PER_SUBDIR in 1 10 100 $MAX_DIRNAME ; do if [[ $NAME != "" ]] ; then P=$P/$NAME fi - echo -n -e "\b\b\b\b" + if [[ $INTERACTIVE -eq 1 ]] ; then + echo -n -e "\b\b\b\b" + fi printf %4d ${#P} echo rm -R "$SUBDIR" From cbd5e8ba01a8d35c6015f32d1e00b598bb097ffa Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 16:09:04 +0200 Subject: [PATCH 17/29] tests/default: add maxlen.bash test --- tests/defaults/main_test.go | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/tests/defaults/main_test.go b/tests/defaults/main_test.go index e7e065e..4e5ffea 100644 --- a/tests/defaults/main_test.go +++ b/tests/defaults/main_test.go @@ -318,9 +318,9 @@ read(3, "M:\tSylwester Nawrocki \nL:\tlinux"..., 32768) = 32768 -read(3, "ach-spear3xx/\n\nSPEAR6XX MACHINE "..., 32768) = 32768 +read(3, "ach-spear3xx/\n\nSPEAR6XX MACHINE "..., 32768) = 32768 <--- WRONG LENGTH!!! read(3, "", 32768) = 0 -lseek(3, 0, SEEK_CUR) = 196608 +lseek(3, 0, SEEK_CUR) = 196608 <--- WRONG LENGTH!!! close(3) = 0 fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0x2), ...}) = 0 write(1, "279b6ab0491e7532132e8f32afe6c04d"..., 56279b6ab0491e7532132e8f32afe6c04d linux-3.0/MAINTAINERS @@ -373,3 +373,27 @@ func TestMd5sumMaintainers(t *testing.T) { t.Logf("full output:\n%s", out) } } + +func TestMaxlen(t *testing.T) { + workDir := filepath.Join(test_helpers.DefaultPlainDir, t.Name()) + if err := os.Mkdir(workDir, 0700); err != nil { + t.Fatal(err) + } + cmd := exec.Command("../../contrib/maxlen.bash", workDir) + out, err := cmd.CombinedOutput() + if err != nil { + t.Log(string(out)) + t.Fatal(err) + } + want := ` + Maximum filename length: 255 + Maximum dirname length: 255 + Maximum path length with 1 chars per subdir: 4095 + Maximum path length with 10 chars per subdir: 4095 + Maximum path length with 100 chars per subdir: 4095 + Maximum path length with 255 chars per subdir: 4095 +` + if !strings.HasSuffix(string(out), want) { + t.Errorf("wrong output: %s", string(out)) + } +} From ee59b5269be45154107dc6840c7bf810aed18ae2 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 16:22:37 +0200 Subject: [PATCH 18/29] fusefrontend: convert openBackingDir tests to prepareAtSyscall openBackingDir will be removed. --- ...ingdir_test.go => prepare_syscall_test.go} | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-) rename internal/fusefrontend/{openbackingdir_test.go => prepare_syscall_test.go} (71%) diff --git a/internal/fusefrontend/openbackingdir_test.go b/internal/fusefrontend/prepare_syscall_test.go similarity index 71% rename from internal/fusefrontend/openbackingdir_test.go rename to internal/fusefrontend/prepare_syscall_test.go index ba4014f..28e655c 100644 --- a/internal/fusefrontend/openbackingdir_test.go +++ b/internal/fusefrontend/prepare_syscall_test.go @@ -13,7 +13,7 @@ import ( "github.com/rfjakob/gocryptfs/tests/test_helpers" ) -func TestOpenBackingDir(t *testing.T) { +func TestPrepareAtSyscall(t *testing.T) { cipherdir := test_helpers.InitFS(t) t.Logf("cipherdir = %q", cipherdir) args := Args{ @@ -33,9 +33,9 @@ func TestOpenBackingDir(t *testing.T) { t.Fatal(errno) } - dirfd, cName, err := rn.openBackingDir("") - if err != nil { - t.Fatal(err) + dirfd, cName, errno := rn.prepareAtSyscallMyself() + if errno != 0 { + t.Fatal(errno) } if cName != "." { t.Fatal("cName should be .") @@ -44,15 +44,15 @@ func TestOpenBackingDir(t *testing.T) { // Again, but populate the cache for "" by looking up a non-existing file rn.Lookup(nil, "xyz1234", &fuse.EntryOut{}) - dirfd, cName, err = rn.openBackingDir("") - if err != nil { - t.Fatal(err) + dirfd, cName, errno = rn.prepareAtSyscallMyself() + if errno != 0 { + t.Fatal(errno) } if cName != "." { t.Fatal("cName should be .") } - err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) + err := syscallcompat.Faccessat(dirfd, cName, unix.R_OK) if err != nil { t.Error(err) } @@ -62,7 +62,7 @@ func TestOpenBackingDir(t *testing.T) { } syscall.Close(dirfd) - dirfd, cName, err = rn.openBackingDir("dir1") + dirfd, cName, errno = rn.prepareAtSyscall("dir1") if err != nil { t.Fatal(err) } @@ -75,9 +75,9 @@ func TestOpenBackingDir(t *testing.T) { } syscall.Close(dirfd) - dirfd, cName, err = rn.openBackingDir("dir1/dir2") - if err != nil { - t.Fatal(err) + dirfd, cName, errno = dir1.prepareAtSyscall("dir2") + if errno != 0 { + t.Fatal(errno) } if cName == "" { t.Fatal("cName should not be empty") @@ -90,9 +90,9 @@ func TestOpenBackingDir(t *testing.T) { n255 := strings.Repeat("n", 255) dir1.Mkdir(nil, n255, 0700, out) - dirfd, cName, err = rn.openBackingDir("dir1/" + n255) - if err != nil { - t.Fatal(err) + dirfd, cName, errno = dir1.prepareAtSyscall(n255) + if errno != 0 { + t.Fatal(errno) } if cName == "" { t.Fatal("cName should not be empty") @@ -107,32 +107,34 @@ func TestOpenBackingDir(t *testing.T) { syscall.Close(dirfd) } -func TestOpenBackingDirPlaintextNames(t *testing.T) { +func TestPrepareAtSyscallPlaintextnames(t *testing.T) { cipherdir := test_helpers.InitFS(t, "-plaintextnames") args := Args{ Cipherdir: cipherdir, PlaintextNames: true, } - fs := newTestFS(args) + rn := newTestFS(args) out := &fuse.EntryOut{} - _, errno := fs.Mkdir(nil, "dir1", 0700, out) + child, errno := rn.Mkdir(nil, "dir1", 0700, out) if errno != 0 { t.Fatal(errno) } - _, errno = fs.Mkdir(nil, "dir1/dir2", 0700, out) + rn.AddChild("dir1", child, false) + dir1 := toNode(child.Operations()) + _, errno = dir1.Mkdir(nil, "dir2", 0700, out) if errno != 0 { t.Fatal(errno) } - dirfd, cName, err := fs.openBackingDir("") - if err != nil { - t.Fatal(err) + dirfd, cName, errno := rn.prepareAtSyscallMyself() + if errno != 0 { + t.Fatal(errno) } if cName != "." { t.Fatal("cName should be .") } - err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) + err := syscallcompat.Faccessat(dirfd, cName, unix.R_OK) if err != nil { t.Error(err) } @@ -142,9 +144,9 @@ func TestOpenBackingDirPlaintextNames(t *testing.T) { } syscall.Close(dirfd) - dirfd, cName, err = fs.openBackingDir("dir1") - if err != nil { - t.Fatal(err) + dirfd, cName, errno = rn.prepareAtSyscall("dir1") + if errno != 0 { + t.Fatal(errno) } if cName != "dir1" { t.Fatalf("wrong cName: %q", cName) @@ -155,9 +157,9 @@ func TestOpenBackingDirPlaintextNames(t *testing.T) { } syscall.Close(dirfd) - dirfd, cName, err = fs.openBackingDir("dir1/dir2") - if err != nil { - t.Fatal(err) + dirfd, cName, errno = dir1.prepareAtSyscall("dir2") + if errno != 0 { + t.Fatal(errno) } if cName != "dir2" { t.Fatalf("wrong cName: %q", cName) From f9f4bd214f772b88233d55c2cf270726bfa603c6 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 18:39:23 +0200 Subject: [PATCH 19/29] fusefrontend: convert ctlsock from openBackingDir to prepareAtSyscall openBackingDir will be removed. Also, remove leftover debug printfs. --- internal/fusefrontend/ctlsock_interface.go | 55 ++++++++++++++-------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/internal/fusefrontend/ctlsock_interface.go b/internal/fusefrontend/ctlsock_interface.go index f34739a..07d742f 100644 --- a/internal/fusefrontend/ctlsock_interface.go +++ b/internal/fusefrontend/ctlsock_interface.go @@ -1,7 +1,6 @@ package fusefrontend import ( - "fmt" "path" "path/filepath" "strings" @@ -18,7 +17,7 @@ var _ ctlsocksrv.Interface = &RootNode{} // Verify that interface is implemented // EncryptPath implements ctlsock.Backend // // Symlink-safe through openBackingDir(). -func (rn *RootNode) EncryptPath(plainPath string) (string, error) { +func (rn *RootNode) EncryptPath(plainPath string) (cipherPath string, err error) { if plainPath == "" { // Empty string gets encrypted as empty string return plainPath, nil @@ -26,22 +25,42 @@ func (rn *RootNode) EncryptPath(plainPath string) (string, error) { if rn.args.PlaintextNames { return plainPath, nil } - // Encrypt path level by level using openBackingDir. Pretty inefficient, - // but does not matter here. + + dirfd, _, errno := rn.prepareAtSyscallMyself() + if errno != 0 { + return "", errno + } + defer syscall.Close(dirfd) + + // Encrypt path level by level parts := strings.Split(plainPath, "/") - wd := "" - cPath := "" - for _, part := range parts { - wd = filepath.Join(wd, part) - dirfd, cName, err := rn.openBackingDir(wd) + wd := dirfd + for i, part := range parts { + dirIV, err := nametransform.ReadDirIVAt(wd) if err != nil { return "", err } - syscall.Close(dirfd) - cPath = filepath.Join(cPath, cName) + cPart, err := rn.nameTransform.EncryptAndHashName(part, dirIV) + if err != nil { + return "", err + } + cipherPath = filepath.Join(cipherPath, cPart) + // Last path component? We are done. + if i == len(parts)-1 { + break + } + // Descend into next directory + wd, err = syscallcompat.Openat(wd, cPart, syscall.O_NOFOLLOW|syscall.O_DIRECTORY|syscallcompat.O_PATH, 0) + if err != nil { + return "", err + } + // Yes this is somewhat wasteful in terms of used file descriptors: + // we keep them all open until the function returns. But it is simple + // and reliable. + defer syscall.Close(wd) } - tlog.Debug.Printf("encryptPath '%s' -> '%s'", plainPath, cPath) - return cPath, nil + tlog.Debug.Printf("EncryptPath %q -> %q", plainPath, cipherPath) + return cipherPath, nil } // DecryptPath implements ctlsock.Backend @@ -49,9 +68,9 @@ func (rn *RootNode) EncryptPath(plainPath string) (string, error) { // DecryptPath is symlink-safe because openBackingDir() and decryptPathAt() // are symlink-safe. func (rn *RootNode) DecryptPath(cipherPath string) (plainPath string, err error) { - dirfd, _, err := rn.openBackingDir("") - if err != nil { - return "", err + dirfd, _, errno := rn.prepareAtSyscallMyself() + if errno != 0 { + return "", errno } defer syscall.Close(dirfd) return rn.decryptPathAt(dirfd, cipherPath) @@ -69,20 +88,17 @@ func (rn *RootNode) decryptPathAt(dirfd int, cipherPath string) (plainPath strin for i, part := range parts { dirIV, err := nametransform.ReadDirIVAt(wd) if err != nil { - fmt.Printf("ReadDirIV: %v\n", err) return "", err } longPart := part if nametransform.IsLongContent(part) { longPart, err = nametransform.ReadLongNameAt(wd, part) if err != nil { - fmt.Printf("ReadLongName: %v\n", err) return "", err } } name, err := rn.nameTransform.DecryptName(longPart, dirIV) if err != nil { - fmt.Printf("DecryptName: %v\n", err) return "", err } plainPath = path.Join(plainPath, name) @@ -100,6 +116,5 @@ func (rn *RootNode) decryptPathAt(dirfd int, cipherPath string) (plainPath strin // and reliable. defer syscall.Close(wd) } - return plainPath, nil } From 45648e567ae5e6d40bb42ae954406c8762fc577f Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 18:42:36 +0200 Subject: [PATCH 20/29] fusefrontend: ctlsock: get rid of unneccessary wrapper function --- internal/fusefrontend/ctlsock_interface.go | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/internal/fusefrontend/ctlsock_interface.go b/internal/fusefrontend/ctlsock_interface.go index 07d742f..9e8cffc 100644 --- a/internal/fusefrontend/ctlsock_interface.go +++ b/internal/fusefrontend/ctlsock_interface.go @@ -18,11 +18,7 @@ var _ ctlsocksrv.Interface = &RootNode{} // Verify that interface is implemented // // Symlink-safe through openBackingDir(). func (rn *RootNode) EncryptPath(plainPath string) (cipherPath string, err error) { - if plainPath == "" { - // Empty string gets encrypted as empty string - return plainPath, nil - } - if rn.args.PlaintextNames { + if rn.args.PlaintextNames || plainPath == "" { return plainPath, nil } @@ -68,21 +64,17 @@ func (rn *RootNode) EncryptPath(plainPath string) (cipherPath string, err error) // DecryptPath is symlink-safe because openBackingDir() and decryptPathAt() // are symlink-safe. func (rn *RootNode) DecryptPath(cipherPath string) (plainPath string, err error) { + if rn.args.PlaintextNames || cipherPath == "" { + return cipherPath, nil + } + dirfd, _, errno := rn.prepareAtSyscallMyself() if errno != 0 { return "", errno } defer syscall.Close(dirfd) - return rn.decryptPathAt(dirfd, cipherPath) -} -// decryptPathAt decrypts a ciphertext path relative to dirfd. -// -// Symlink-safe through ReadDirIVAt() and ReadLongNameAt(). -func (rn *RootNode) decryptPathAt(dirfd int, cipherPath string) (plainPath string, err error) { - if rn.args.PlaintextNames || cipherPath == "" { - return cipherPath, nil - } + // Decrypt path level by level parts := strings.Split(cipherPath, "/") wd := dirfd for i, part := range parts { From 1f29542b39654bda417ed436b8640b2f2e4f0d6e Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 18:43:15 +0200 Subject: [PATCH 21/29] tests: better error message on ctlsock query failure --- tests/defaults/ctlsock_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/defaults/ctlsock_test.go b/tests/defaults/ctlsock_test.go index 64b72a2..ac64f42 100644 --- a/tests/defaults/ctlsock_test.go +++ b/tests/defaults/ctlsock_test.go @@ -66,7 +66,7 @@ func TestCtlSockDecrypt(t *testing.T) { } response := test_helpers.QueryCtlSock(t, sock, req) if response.Result == "" || response.ErrNo != 0 { - t.Fatalf("got an error reply: %+v", response) + t.Fatalf("got an error for query %+v: %+v", req, response) } // Check if the encrypted path actually exists cPath := response.Result From 5306fc345be41371820ea50a3f64efab08f2e74c Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 18:44:52 +0200 Subject: [PATCH 22/29] fusefrontend: convert last callers from openBackingDir to prepareAtSyscall --- internal/fusefrontend/node_dir_ops.go | 12 +++++------- internal/fusefrontend/node_prepare_syscall.go | 7 ++++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/internal/fusefrontend/node_dir_ops.go b/internal/fusefrontend/node_dir_ops.go index 0f48876..6d03544 100644 --- a/internal/fusefrontend/node_dir_ops.go +++ b/internal/fusefrontend/node_dir_ops.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "io" - "path/filepath" "runtime" "syscall" @@ -239,15 +238,14 @@ func (n *Node) Readdir(ctx context.Context) (fs.DirStream, syscall.Errno) { // Symlink-safe through Unlinkat() + AT_REMOVEDIR. func (n *Node) Rmdir(ctx context.Context, name string) (code syscall.Errno) { rn := n.rootNode() - p := filepath.Join(n.Path(), name) - parentDirFd, cName, err := rn.openBackingDir(p) - if err != nil { - return fs.ToErrno(err) + parentDirFd, cName, errno := n.prepareAtSyscall(name) + if errno != 0 { + return errno } defer syscall.Close(parentDirFd) if rn.args.PlaintextNames { // Unlinkat with AT_REMOVEDIR is equivalent to Rmdir - err = unix.Unlinkat(parentDirFd, cName, unix.AT_REMOVEDIR) + err := unix.Unlinkat(parentDirFd, cName, unix.AT_REMOVEDIR) return fs.ToErrno(err) } // Unless we are running as root, we need read, write and execute permissions @@ -256,7 +254,7 @@ func (n *Node) Rmdir(ctx context.Context, name string) (code syscall.Errno) { var origMode uint32 if !rn.args.PreserveOwner { var st unix.Stat_t - err = syscallcompat.Fstatat(parentDirFd, cName, &st, unix.AT_SYMLINK_NOFOLLOW) + err := syscallcompat.Fstatat(parentDirFd, cName, &st, unix.AT_SYMLINK_NOFOLLOW) if err != nil { return fs.ToErrno(err) } diff --git a/internal/fusefrontend/node_prepare_syscall.go b/internal/fusefrontend/node_prepare_syscall.go index fa558e8..283096f 100644 --- a/internal/fusefrontend/node_prepare_syscall.go +++ b/internal/fusefrontend/node_prepare_syscall.go @@ -98,11 +98,12 @@ func (n *Node) prepareAtSyscallMyself() (dirfd int, cName string, errno syscall. if n.IsRoot() { var err error rn := n.rootNode() - dirfd, cName, err = rn.openBackingDir("") + // Open cipherdir (following symlinks) + dirfd, err = syscallcompat.Open(rn.args.Cipherdir, syscall.O_DIRECTORY|syscallcompat.O_PATH, 0) if err != nil { - errno = fs.ToErrno(err) + return -1, "", fs.ToErrno(err) } - return + return dirfd, ".", 0 } // Otherwise convert to prepareAtSyscall of parent node From 4fd95b718b9f7b022c1960711fc7dfe7b5350c15 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 18:45:21 +0200 Subject: [PATCH 23/29] fusefrontend: delete openBackingDir --- internal/fusefrontend/root_node.go | 65 ------------------------------ 1 file changed, 65 deletions(-) diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go index ab0cdcd..c82078d 100644 --- a/internal/fusefrontend/root_node.go +++ b/internal/fusefrontend/root_node.go @@ -2,7 +2,6 @@ package fusefrontend import ( "os" - "path/filepath" "strings" "sync" "syscall" @@ -210,70 +209,6 @@ func (rn *RootNode) openWriteOnlyFile(dirfd int, cName string, newFlags int) (rw return syscallcompat.Openat(dirfd, cName, newFlags, 0) } -// openBackingDir opens the parent ciphertext directory of plaintext path -// "relPath". It returns the dirfd (opened with O_PATH) and the encrypted -// basename. -// -// The caller should then use Openat(dirfd, cName, ...) and friends. -// For convenience, if relPath is "", cName is going to be ".". -// -// openBackingDir is secure against symlink races by using Openat and -// ReadDirIVAt. -// -// Retries on EINTR. -func (rn *RootNode) openBackingDir(relPath string) (dirfd int, cName string, err error) { - dirRelPath := nametransform.Dir(relPath) - // With PlaintextNames, we don't need to read DirIVs. Easy. - if rn.args.PlaintextNames { - dirfd, err = syscallcompat.OpenDirNofollow(rn.args.Cipherdir, dirRelPath) - if err != nil { - return -1, "", err - } - // If relPath is empty, cName is ".". - cName = filepath.Base(relPath) - return dirfd, cName, nil - } - // Open cipherdir (following symlinks) - dirfd, err = syscallcompat.Open(rn.args.Cipherdir, syscall.O_DIRECTORY|syscallcompat.O_PATH, 0) - if err != nil { - return -1, "", err - } - // If relPath is empty, cName is ".". - if relPath == "" { - return dirfd, ".", nil - } - // Walk the directory tree - parts := strings.Split(relPath, "/") - for i, name := range parts { - iv, err := nametransform.ReadDirIVAt(dirfd) - if err != nil { - syscall.Close(dirfd) - return -1, "", err - } - if rn.nameTransform.HaveBadnamePatterns() { - cName, err = rn.nameTransform.EncryptAndHashBadName(name, iv, dirfd) - } else { - cName, err = rn.nameTransform.EncryptAndHashName(name, iv) - } - if err != nil { - syscall.Close(dirfd) - return -1, "", err - } - // Last part? We are done. - if i == len(parts)-1 { - break - } - // Not the last part? Descend into next directory. - dirfd2, err := syscallcompat.Openat(dirfd, cName, syscall.O_NOFOLLOW|syscall.O_DIRECTORY|syscallcompat.O_PATH, 0) - syscall.Close(dirfd) - if err != nil { - return -1, "", err - } - dirfd = dirfd2 - } - return dirfd, cName, nil -} - // encryptSymlinkTarget: "data" is encrypted like file contents (GCM) // and base64-encoded. // The empty string encrypts to the empty string. From 446c3d7e931747cd833ae6211a3742cb9d02578b Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 18:58:29 +0200 Subject: [PATCH 24/29] tests: matrix: show content detail on mismatch --- tests/matrix/concurrency_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/matrix/concurrency_test.go b/tests/matrix/concurrency_test.go index 24d4578..6a9b9aa 100644 --- a/tests/matrix/concurrency_test.go +++ b/tests/matrix/concurrency_test.go @@ -119,7 +119,8 @@ func TestConcurrentReadCreate(t *testing.T) { } buf := buf0[:n] if bytes.Compare(buf, content) != 0 { - log.Fatal("content mismatch") + // Calling t.Fatal() from a goroutine hangs the test so we use log.Fatal + log.Fatalf("%s: content mismatch: have=%q want=%q", t.Name(), string(buf), string(content)) } f.Close() } From ad3eeaedc5a9042682f236f43dd939b2e620d07c Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 19:12:52 +0200 Subject: [PATCH 25/29] tests, maxlen.bash: speed up TestMaxlen using QUICK=1 From >6 to <1 second. --- contrib/maxlen.bash | 37 ++++++++++++++++++++++++------------- tests/defaults/main_test.go | 5 +---- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/contrib/maxlen.bash b/contrib/maxlen.bash index 14d40da..99ffb1f 100755 --- a/contrib/maxlen.bash +++ b/contrib/maxlen.bash @@ -32,25 +32,36 @@ while true ; do echo "error: file $PWD/$NEXT already exists" exit 1 fi - touch $NEXT 2> /dev/null || break + echo -n 2> /dev/null > $NEXT || break rm $NEXT NAME="$NEXT" done echo "${#NAME}" -echo -n " Maximum dirname length: " -# Add one character at a time until we hit an error -NAME="" -while true ; do - NEXT="${NAME}x" - mkdir $NEXT 2> /dev/null || break - rmdir $NEXT - NAME="$NEXT" -done -MAX_DIRNAME=${#NAME} -echo "${#NAME}" +# Set to 0 if undefined +: ${QUICK:=0} -for CHARS_PER_SUBDIR in 1 10 100 $MAX_DIRNAME ; do +if [[ $QUICK -ne 1 ]]; then + echo -n " Maximum dirname length: " + # Add one character at a time until we hit an error + NAME="" + while true ; do + NEXT="${NAME}x" + mkdir $NEXT 2> /dev/null || break + rmdir $NEXT + NAME="$NEXT" + done + MAX_DIRNAME=${#NAME} + echo "${#NAME}" +fi + +if [[ $QUICK -eq 1 ]]; then + CHARS_TODO=100 +else + CHARS_TODO="1 10 100 $MAX_DIRNAME" +fi + +for CHARS_PER_SUBDIR in $CHARS_TODO ; do echo -n " Maximum path length with $(printf %3d $CHARS_PER_SUBDIR) chars per subdir: " if [[ $INTERACTIVE -eq 1 ]] ; then echo -n " " diff --git a/tests/defaults/main_test.go b/tests/defaults/main_test.go index 4e5ffea..b356f41 100644 --- a/tests/defaults/main_test.go +++ b/tests/defaults/main_test.go @@ -380,6 +380,7 @@ func TestMaxlen(t *testing.T) { t.Fatal(err) } cmd := exec.Command("../../contrib/maxlen.bash", workDir) + cmd.Env = []string{"QUICK=1"} out, err := cmd.CombinedOutput() if err != nil { t.Log(string(out)) @@ -387,11 +388,7 @@ func TestMaxlen(t *testing.T) { } want := ` Maximum filename length: 255 - Maximum dirname length: 255 - Maximum path length with 1 chars per subdir: 4095 - Maximum path length with 10 chars per subdir: 4095 Maximum path length with 100 chars per subdir: 4095 - Maximum path length with 255 chars per subdir: 4095 ` if !strings.HasSuffix(string(out), want) { t.Errorf("wrong output: %s", string(out)) From 49507ea869e8496b1890bea96b182ccc93923d79 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 19:27:58 +0200 Subject: [PATCH 26/29] tests/fsck: delete obsolete script run_fsck.bash Not called by anybody. --- tests/fsck/run_fsck.bash | 2 -- 1 file changed, 2 deletions(-) delete mode 100755 tests/fsck/run_fsck.bash diff --git a/tests/fsck/run_fsck.bash b/tests/fsck/run_fsck.bash deleted file mode 100755 index 9637381..0000000 --- a/tests/fsck/run_fsck.bash +++ /dev/null @@ -1,2 +0,0 @@ -#!/bin/bash -exec ../../gocryptfs -fsck -extpass "echo test" broken_fs_v1.4 From fe616ddad59b1bb926343e0856d721454d671967 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 26 Jun 2021 20:57:39 +0200 Subject: [PATCH 27/29] doc: update performance.txt --- Documentation/performance.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/performance.txt b/Documentation/performance.txt index ab35de1..24265f5 100644 --- a/Documentation/performance.txt +++ b/Documentation/performance.txt @@ -72,6 +72,7 @@ v2.0-beta2-36-g6aae2aa 505 1000 16.1 8.2 6.3 7.7 v2.0-beta2-37-g24d5d39 558 1000 12.3 6.4 4.4 2.8 fs: add initial dirfd caching v2.0-beta2-42-g4a07d65 549 1000 8.2 4.7 1.8 2.4 fusefrontend: make dirCache work for "node itself" v2.0 420 1000 8.5 4.5 1.8 2.3 go1.16.5, Linux 5.11.21 +v2.0.1-28-g49507ea 471 991 8.6 4.5 1.7 2.2 Results for EncFS for comparison (benchmark.bash -encfs): From d6c8d892ffacf92f13798ee71112447100aa5a50 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 27 Jun 2021 11:12:40 +0200 Subject: [PATCH 28/29] fido2: pretty-print fidoCommand in debug output Related: https://github.com/rfjakob/gocryptfs/issues/571 --- internal/fido2/fido2.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/fido2/fido2.go b/internal/fido2/fido2.go index cd63483..f62967b 100644 --- a/internal/fido2/fido2.go +++ b/internal/fido2/fido2.go @@ -22,6 +22,20 @@ const ( assertWithPIN fidoCommand = iota ) +// String pretty-prints for debug output +func (fc fidoCommand) String() string { + switch fc { + case cred: + return "cred" + case assert: + return "assert" + case assertWithPIN: + return "assertWithPIN" + default: + return fmt.Sprintf("%d", fc) + } +} + const relyingPartyID = "gocryptfs" func callFidoCommand(command fidoCommand, device string, stdin []string) ([]string, error) { @@ -34,7 +48,7 @@ func callFidoCommand(command fidoCommand, device string, stdin []string) ([]stri case assertWithPIN: cmd = exec.Command("fido2-assert", "-G", "-h", "-v", device) } - tlog.Debug.Printf("callFidoCommand: executing %q with args %q", cmd.Path, cmd.Args) + tlog.Debug.Printf("callFidoCommand %s: executing %q with args %q", command, cmd.Path, cmd.Args) cmd.Stderr = os.Stderr in, err := cmd.StdinPipe() if err != nil { From 2a9d70d48f4cc715a6864849cdec91ab08b6fd03 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 27 Jun 2021 11:17:29 +0200 Subject: [PATCH 29/29] fido2: drop `-v` option (PIN request) We used to pass `-v` on `gocryptfs -init` but not for mount, which seems strange by itself, but more importantly, `-v` does not work on Yubikeys. Drop `-v`. Fixes https://github.com/rfjakob/gocryptfs/issues/571 --- README.md | 3 +++ internal/fido2/fido2.go | 19 +++++-------------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index deb5bd9..503c037 100644 --- a/README.md +++ b/README.md @@ -208,6 +208,9 @@ RM: 2,367 Changelog --------- +v2.1 (IN PROGRESS) +* fido2: do not request PIN on `gocryptfs -init` ([#571](https://github.com/rfjakob/gocryptfs/issues/571)) + v2.0.1, 2021-06-07 * Fix symlink creation reporting the wrong size, causing git to report it as modified ([#574](https://github.com/rfjakob/gocryptfs/issues/574)) diff --git a/internal/fido2/fido2.go b/internal/fido2/fido2.go index f62967b..30121c0 100644 --- a/internal/fido2/fido2.go +++ b/internal/fido2/fido2.go @@ -17,9 +17,8 @@ import ( type fidoCommand int const ( - cred fidoCommand = iota - assert fidoCommand = iota - assertWithPIN fidoCommand = iota + cred fidoCommand = iota + assert fidoCommand = iota ) // String pretty-prints for debug output @@ -29,8 +28,6 @@ func (fc fidoCommand) String() string { return "cred" case assert: return "assert" - case assertWithPIN: - return "assertWithPIN" default: return fmt.Sprintf("%d", fc) } @@ -45,8 +42,6 @@ func callFidoCommand(command fidoCommand, device string, stdin []string) ([]stri cmd = exec.Command("fido2-cred", "-M", "-h", "-v", device) case assert: cmd = exec.Command("fido2-assert", "-G", "-h", device) - case assertWithPIN: - cmd = exec.Command("fido2-assert", "-G", "-h", "-v", device) } tlog.Debug.Printf("callFidoCommand %s: executing %q with args %q", command, cmd.Path, cmd.Args) cmd.Stderr = os.Stderr @@ -92,15 +87,11 @@ func Secret(device string, credentialID []byte, salt []byte) (secret []byte) { crid := base64.StdEncoding.EncodeToString(credentialID) hmacsalt := base64.StdEncoding.EncodeToString(salt) stdin := []string{cdh, relyingPartyID, crid, hmacsalt} - // try asserting without PIN first + // call fido2-assert out, err := callFidoCommand(assert, device, stdin) if err != nil { - // if that fails, let's assert with PIN - out, err = callFidoCommand(assertWithPIN, device, stdin) - if err != nil { - tlog.Fatal.Println(err) - os.Exit(exitcodes.FIDO2Error) - } + tlog.Fatal.Println(err) + os.Exit(exitcodes.FIDO2Error) } secret, err = base64.StdEncoding.DecodeString(out[4]) if err != nil {