diff options
| author | Jakob Unterwurzacher | 2018-10-14 20:11:49 +0200 | 
|---|---|---|
| committer | Jakob Unterwurzacher | 2019-01-01 16:24:09 +0100 | 
| commit | 932efbd4593fe6be6c86f0dafeaea32910b7c246 (patch) | |
| tree | a950de9c1434a180f9b6f908d522d8222cecb768 /internal/fusefrontend | |
| parent | 0e2e7c13cfc6d37f2521db99bf0393c37a5549d6 (diff) | |
fusefrontend: make openBackingDir() symlink-safe
openBackingDir() used encryptPath(), which is not symlink-safe
itself. Drop encryptPath() and implement our own directory walk.
Adds three seconds to untar and two seconds to rm:
$ ./benchmark.bash
Testing gocryptfs at /tmp/benchmark.bash.MzG: gocryptfs v1.6-36-g8fb3c2f-dirty; go-fuse v20170619-66-g6df8ddc; 2018-10-14 go1.11
WRITE: 262144000 bytes (262 MB, 250 MiB) copied, 1.25078 s, 210 MB/s
READ:  262144000 bytes (262 MB, 250 MiB) copied, 1.0318 s, 254 MB/s
UNTAR: 20.941
MD5:   11.568
LS:    1.638
RM:    5.337
Diffstat (limited to 'internal/fusefrontend')
| -rw-r--r-- | internal/fusefrontend/names.go | 51 | ||||
| -rw-r--r-- | internal/fusefrontend/names_test.go | 152 | ||||
| -rw-r--r-- | internal/fusefrontend/xattr_unit_test.go | 5 | 
3 files changed, 198 insertions, 10 deletions
| diff --git a/internal/fusefrontend/names.go b/internal/fusefrontend/names.go index 3bf64d5..5ec252b 100644 --- a/internal/fusefrontend/names.go +++ b/internal/fusefrontend/names.go @@ -4,8 +4,11 @@ package fusefrontend  import (  	"path/filepath" +	"strings" +	"syscall"  	"github.com/rfjakob/gocryptfs/internal/configfile" +	"github.com/rfjakob/gocryptfs/internal/nametransform"  	"github.com/rfjakob/gocryptfs/internal/syscallcompat"  	"github.com/rfjakob/gocryptfs/internal/tlog"  ) @@ -42,19 +45,53 @@ func (fs *FS) getBackingPath(relPath string) (string, error) {  // openBackingDir opens the parent ciphertext directory of plaintext path  // "relPath" and returns the dirfd and the encrypted basename. +//  // The caller should then use Openat(dirfd, cName, ...) and friends. -// openBackingDir is secure against symlink races. +// For convenience, if relPath is "", cName is going to be ".". +// +// openBackingDir is secure against symlink races by using Openat and +// ReadDirIVAt.  func (fs *FS) openBackingDir(relPath string) (dirfd int, cName string, err error) { -	cRelPath, err := fs.encryptPath(relPath) -	if err != nil { -		return -1, "", err +	// With PlaintextNames, we don't need to read DirIVs. Easy. +	if fs.args.PlaintextNames { +		dir := nametransform.Dir(relPath) +		dirfd, err = syscallcompat.OpenDirNofollow(fs.args.Cipherdir, dir) +		if err != nil { +			return -1, "", err +		} +		// If relPath is empty, cName is ".". +		cName = filepath.Base(relPath) +		return dirfd, cName, nil  	} -	// Open parent dir -	dirfd, err = syscallcompat.OpenDirNofollow(fs.args.Cipherdir, filepath.Dir(cRelPath)) +	// Open cipherdir (following symlinks) +	dirfd, err = syscall.Open(fs.args.Cipherdir, syscall.O_RDONLY|syscall.O_DIRECTORY|syscallcompat.O_PATH, 0)  	if err != nil {  		return -1, "", err  	} -	cName = filepath.Base(cRelPath) +	// 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 { +			return -1, "", err +		} +		cName = fs.nameTransform.EncryptAndHashName(name, iv) +		// 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_RDONLY|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  } diff --git a/internal/fusefrontend/names_test.go b/internal/fusefrontend/names_test.go new file mode 100644 index 0000000..8453e52 --- /dev/null +++ b/internal/fusefrontend/names_test.go @@ -0,0 +1,152 @@ +package fusefrontend + +import ( +	"strings" +	"syscall" +	"testing" + +	"golang.org/x/sys/unix" + +	"github.com/rfjakob/gocryptfs/internal/syscallcompat" +	"github.com/rfjakob/gocryptfs/tests/test_helpers" +) + +func TestOpenBackingDir(t *testing.T) { +	cipherdir := test_helpers.InitFS(t) +	args := Args{ +		Cipherdir: cipherdir, +	} +	fs := newTestFS(args) + +	code := fs.Mkdir("dir1", 0700, nil) +	if !code.Ok() { +		t.Fatal(code) +	} +	code = fs.Mkdir("dir1/dir2", 0700, nil) +	if !code.Ok() { +		t.Fatal(code) +	} + +	dirfd, cName, err := fs.openBackingDir("") +	if err != nil { +		t.Fatal(err) +	} +	if cName != "." { +		t.Fatal("cName should be .") +	} +	err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	err = syscallcompat.Faccessat(dirfd, ".", unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	syscall.Close(dirfd) + +	dirfd, cName, err = fs.openBackingDir("dir1") +	if err != nil { +		t.Fatal(err) +	} +	if cName == "" { +		t.Fatal("cName should not be empty") +	} +	err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	syscall.Close(dirfd) + +	dirfd, cName, err = fs.openBackingDir("dir1/dir2") +	if err != nil { +		t.Fatal(err) +	} +	if cName == "" { +		t.Fatal("cName should not be empty") +	} +	err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	syscall.Close(dirfd) + +	n255 := strings.Repeat("n", 255) +	path := "dir1/" + n255 +	fs.Mkdir(path, 0700, nil) +	dirfd, cName, err = fs.openBackingDir(path) +	if err != nil { +		t.Fatal(err) +	} +	if cName == "" { +		t.Fatal("cName should not be empty") +	} +	if len(cName) >= 255 { +		t.Fatalf("cName is too long: %q", cName) +	} +	err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	syscall.Close(dirfd) +} + +func TestOpenBackingDirPlaintextNames(t *testing.T) { +	cipherdir := test_helpers.InitFS(t, "-plaintextnames") +	args := Args{ +		Cipherdir:      cipherdir, +		PlaintextNames: true, +	} +	fs := newTestFS(args) + +	code := fs.Mkdir("dir1", 0700, nil) +	if !code.Ok() { +		t.Fatal(code) +	} +	code = fs.Mkdir("dir1/dir2", 0700, nil) +	if !code.Ok() { +		t.Fatal(code) +	} + +	dirfd, cName, err := fs.openBackingDir("") +	if err != nil { +		t.Fatal(err) +	} +	if cName != "." { +		t.Fatal("cName should be .") +	} +	err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	err = syscallcompat.Faccessat(dirfd, ".", unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	syscall.Close(dirfd) + +	dirfd, cName, err = fs.openBackingDir("dir1") +	if err != nil { +		t.Fatal(err) +	} +	if cName != "dir1" { +		t.Fatalf("wrong cName: %q", cName) +	} +	err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	syscall.Close(dirfd) + +	dirfd, cName, err = fs.openBackingDir("dir1/dir2") +	if err != nil { +		t.Fatal(err) +	} +	if cName != "dir2" { +		t.Fatalf("wrong cName: %q", cName) +	} +	err = syscallcompat.Faccessat(dirfd, cName, unix.R_OK) +	if err != nil { +		t.Error(err) +	} +	syscall.Close(dirfd) +} diff --git a/internal/fusefrontend/xattr_unit_test.go b/internal/fusefrontend/xattr_unit_test.go index c5c9360..b43aeb7 100644 --- a/internal/fusefrontend/xattr_unit_test.go +++ b/internal/fusefrontend/xattr_unit_test.go @@ -11,18 +11,17 @@ import (  	"github.com/rfjakob/gocryptfs/internal/nametransform"  ) -func newTestFS() *FS { +func newTestFS(args Args) *FS {  	// Init crypto backend  	key := make([]byte, cryptocore.KeyLen)  	cCore := cryptocore.New(key, cryptocore.BackendGoGCM, contentenc.DefaultIVBits, true, false)  	cEnc := contentenc.New(cCore, contentenc.DefaultBS, false)  	nameTransform := nametransform.New(cCore.EMECipher, true, true) -	args := Args{}  	return NewFS(args, cEnc, nameTransform)  }  func TestEncryptDecryptXattrName(t *testing.T) { -	fs := newTestFS() +	fs := newTestFS(Args{})  	attr1 := "user.foo123456789"  	cAttr := fs.encryptXattrName(attr1)  	t.Logf("cAttr=%v", cAttr) | 
