aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakob Unterwurzacher2018-10-14 20:11:49 +0200
committerJakob Unterwurzacher2019-01-01 16:24:09 +0100
commit932efbd4593fe6be6c86f0dafeaea32910b7c246 (patch)
treea950de9c1434a180f9b6f908d522d8222cecb768
parent0e2e7c13cfc6d37f2521db99bf0393c37a5549d6 (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
-rw-r--r--internal/fusefrontend/names.go51
-rw-r--r--internal/fusefrontend/names_test.go152
-rw-r--r--internal/fusefrontend/xattr_unit_test.go5
-rw-r--r--internal/nametransform/diriv.go9
4 files changed, 204 insertions, 13 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)
diff --git a/internal/nametransform/diriv.go b/internal/nametransform/diriv.go
index b45ae52..c2b9bb1 100644
--- a/internal/nametransform/diriv.go
+++ b/internal/nametransform/diriv.go
@@ -121,7 +121,7 @@ func WriteDirIV(dirfd int, dir string) error {
// encryptAndHashName encrypts "name" and hashes it to a longname if it is
// too long.
-func (be *NameTransform) encryptAndHashName(name string, iv []byte) string {
+func (be *NameTransform) EncryptAndHashName(name string, iv []byte) string {
cName := be.EncryptName(name, iv)
if be.longNames && len(cName) > unix.NAME_MAX {
return be.HashLongName(cName)
@@ -132,6 +132,9 @@ func (be *NameTransform) encryptAndHashName(name string, iv []byte) string {
// EncryptPathDirIV - encrypt relative plaintext path "plainPath" using EME with
// DirIV. "rootDir" is the backing storage root directory.
// Components that are longer than 255 bytes are hashed if be.longnames == true.
+//
+// TODO: EncryptPathDirIV is NOT SAFE against symlink races. This function
+// should eventually be deleted.
func (be *NameTransform) EncryptPathDirIV(plainPath string, rootDir string) (string, error) {
var err error
// Empty string means root directory
@@ -148,7 +151,7 @@ func (be *NameTransform) EncryptPathDirIV(plainPath string, rootDir string) (str
// in the tar extract benchmark.
parentDir := Dir(plainPath)
if iv, cParentDir := be.DirIVCache.Lookup(parentDir); iv != nil {
- cBaseName := be.encryptAndHashName(baseName, iv)
+ cBaseName := be.EncryptAndHashName(baseName, iv)
return filepath.Join(cParentDir, cBaseName), nil
}
// We have to walk the directory tree, starting at the root directory.
@@ -166,7 +169,7 @@ func (be *NameTransform) EncryptPathDirIV(plainPath string, rootDir string) (str
}
be.DirIVCache.Store(plainWD, iv, cipherWD)
}
- cipherName := be.encryptAndHashName(plainName, iv)
+ cipherName := be.EncryptAndHashName(plainName, iv)
cipherWD = filepath.Join(cipherWD, cipherName)
plainWD = filepath.Join(plainWD, plainName)
}