summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--internal/fusefrontend/node_xattr.go16
-rw-r--r--internal/fusefrontend/root_node.go9
-rw-r--r--internal/fusefrontend/xattr_unit_test.go5
-rw-r--r--internal/fusefrontend_reverse/ctlsock_interface.go5
-rw-r--r--internal/fusefrontend_reverse/node_dir_ops.go6
-rw-r--r--internal/fusefrontend_reverse/root_node.go7
-rw-r--r--internal/nametransform/diriv.go5
-rw-r--r--internal/nametransform/longnames.go5
-rw-r--r--internal/nametransform/names.go27
-rw-r--r--internal/nametransform/names_test.go24
-rw-r--r--internal/nametransform/valid.go27
-rw-r--r--tests/example_filesystems/example_filesystems_test.go3
12 files changed, 108 insertions, 31 deletions
diff --git a/internal/fusefrontend/node_xattr.go b/internal/fusefrontend/node_xattr.go
index 3855b55..925dcbf 100644
--- a/internal/fusefrontend/node_xattr.go
+++ b/internal/fusefrontend/node_xattr.go
@@ -56,12 +56,14 @@ func (n *Node) Getxattr(ctx context.Context, attr string, dest []byte) (uint32,
}
} else {
// encrypted user xattr
- cAttr := rn.encryptXattrName(attr)
+ cAttr, err := rn.encryptXattrName(attr)
+ if err != nil {
+ return minus1, syscall.EIO
+ }
cData, errno := n.getXAttr(cAttr)
if errno != 0 {
return 0, errno
}
- var err error
data, err = rn.decryptXattrValue(cData)
if err != nil {
tlog.Warn.Printf("GetXAttr: %v", err)
@@ -91,7 +93,10 @@ func (n *Node) Setxattr(ctx context.Context, attr string, data []byte, flags uin
return n.setXAttr(attr, data, flags)
}
- cAttr := rn.encryptXattrName(attr)
+ cAttr, err := rn.encryptXattrName(attr)
+ if err != nil {
+ return syscall.EINVAL
+ }
cData := rn.encryptXattrValue(data)
return n.setXAttr(cAttr, cData, flags)
}
@@ -107,7 +112,10 @@ func (n *Node) Removexattr(ctx context.Context, attr string) syscall.Errno {
return n.removeXAttr(attr)
}
- cAttr := rn.encryptXattrName(attr)
+ cAttr, err := rn.encryptXattrName(attr)
+ if err != nil {
+ return syscall.EINVAL
+ }
return n.removeXAttr(cAttr)
}
diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go
index e998e9d..a830cc4 100644
--- a/internal/fusefrontend/root_node.go
+++ b/internal/fusefrontend/root_node.go
@@ -311,10 +311,13 @@ func (rn *RootNode) decryptXattrValue(cData []byte) (data []byte, err error) {
}
// encryptXattrName transforms "user.foo" to "user.gocryptfs.a5sAd4XAa47f5as6dAf"
-func (rn *RootNode) encryptXattrName(attr string) (cAttr string) {
+func (rn *RootNode) encryptXattrName(attr string) (string, error) {
// xattr names are encrypted like file names, but with a fixed IV.
- cAttr = xattrStorePrefix + rn.nameTransform.EncryptName(attr, xattrNameIV)
- return cAttr
+ cAttr, err := rn.nameTransform.EncryptName(attr, xattrNameIV)
+ if err != nil {
+ return "", err
+ }
+ return xattrStorePrefix + cAttr, nil
}
func (rn *RootNode) decryptXattrName(cAttr string) (attr string, err error) {
diff --git a/internal/fusefrontend/xattr_unit_test.go b/internal/fusefrontend/xattr_unit_test.go
index f6c0469..a0cf4c8 100644
--- a/internal/fusefrontend/xattr_unit_test.go
+++ b/internal/fusefrontend/xattr_unit_test.go
@@ -33,7 +33,10 @@ func newTestFS(args Args) *RootNode {
func TestEncryptDecryptXattrName(t *testing.T) {
fs := newTestFS(Args{})
attr1 := "user.foo123456789"
- cAttr := fs.encryptXattrName(attr1)
+ cAttr, err := fs.encryptXattrName(attr1)
+ if err != nil {
+ t.Fatal(err)
+ }
t.Logf("cAttr=%v", cAttr)
attr2, err := fs.decryptXattrName(cAttr)
if attr1 != attr2 || err != nil {
diff --git a/internal/fusefrontend_reverse/ctlsock_interface.go b/internal/fusefrontend_reverse/ctlsock_interface.go
index 6bf2e6a..2157044 100644
--- a/internal/fusefrontend_reverse/ctlsock_interface.go
+++ b/internal/fusefrontend_reverse/ctlsock_interface.go
@@ -23,7 +23,10 @@ func (rn *RootNode) EncryptPath(plainPath string) (string, error) {
parts := strings.Split(plainPath, "/")
for _, part := range parts {
dirIV := pathiv.Derive(cipherPath, pathiv.PurposeDirIV)
- encryptedPart := rn.nameTransform.EncryptName(part, dirIV)
+ encryptedPart, err := rn.nameTransform.EncryptName(part, dirIV)
+ if err != nil {
+ return "", err
+ }
if rn.args.LongNames && len(encryptedPart) > unix.NAME_MAX {
encryptedPart = rn.nameTransform.HashLongName(encryptedPart)
}
diff --git a/internal/fusefrontend_reverse/node_dir_ops.go b/internal/fusefrontend_reverse/node_dir_ops.go
index 22f8122..c287284 100644
--- a/internal/fusefrontend_reverse/node_dir_ops.go
+++ b/internal/fusefrontend_reverse/node_dir_ops.go
@@ -64,7 +64,11 @@ func (n *Node) Readdir(ctx context.Context) (stream fs.DirStream, errno syscall.
!rn.args.ConfigCustom {
cName = configfile.ConfDefaultName
} else {
- cName = rn.nameTransform.EncryptName(entries[i].Name, dirIV)
+ cName, err = rn.nameTransform.EncryptName(entries[i].Name, dirIV)
+ if err != nil {
+ entries[i].Name = "___GOCRYPTFS_INVALID_NAME___"
+ continue
+ }
if len(cName) > unix.NAME_MAX {
cName = rn.nameTransform.HashLongName(cName)
dotNameFile := fuse.DirEntry{
diff --git a/internal/fusefrontend_reverse/root_node.go b/internal/fusefrontend_reverse/root_node.go
index b7a259a..10b0d69 100644
--- a/internal/fusefrontend_reverse/root_node.go
+++ b/internal/fusefrontend_reverse/root_node.go
@@ -71,9 +71,12 @@ func (rn *RootNode) findLongnameParent(fd int, diriv []byte, longname string) (p
if len(entry.Name) <= shortNameMax {
continue
}
- cFullName = rn.nameTransform.EncryptName(entry.Name, diriv)
+ cFullName, err = rn.nameTransform.EncryptName(entry.Name, diriv)
+ if err != nil {
+ continue
+ }
if len(cFullName) <= unix.NAME_MAX {
- // Entry should have been skipped by the "continue" above
+ // Entry should have been skipped by the shortNameMax check above
log.Panic("logic error or wrong shortNameMax constant?")
}
hName := rn.nameTransform.HashLongName(cFullName)
diff --git a/internal/nametransform/diriv.go b/internal/nametransform/diriv.go
index b2f165b..1d27aa5 100644
--- a/internal/nametransform/diriv.go
+++ b/internal/nametransform/diriv.go
@@ -102,7 +102,10 @@ func (be *NameTransform) EncryptAndHashName(name string, iv []byte) (string, err
if len(name) > NameMax {
return "", syscall.ENAMETOOLONG
}
- cName := be.EncryptName(name, iv)
+ cName, err := be.EncryptName(name, iv)
+ if err != nil {
+ return "", err
+ }
if be.longNames && len(cName) > NameMax {
return be.HashLongName(cName), nil
}
diff --git a/internal/nametransform/longnames.go b/internal/nametransform/longnames.go
index aa463a1..74ddb07 100644
--- a/internal/nametransform/longnames.go
+++ b/internal/nametransform/longnames.go
@@ -132,7 +132,10 @@ func (n *NameTransform) WriteLongNameAt(dirfd int, hashName string, plainName st
if err != nil {
return err
}
- cName := n.EncryptName(plainName, dirIV)
+ cName, err := n.EncryptName(plainName, dirIV)
+ if err != nil {
+ return err
+ }
// Write the encrypted name into hashName.name
fdRaw, err := syscallcompat.Openat(dirfd, hashName+LongNameSuffix,
diff --git a/internal/nametransform/names.go b/internal/nametransform/names.go
index 119d592..ca28230 100644
--- a/internal/nametransform/names.go
+++ b/internal/nametransform/names.go
@@ -2,7 +2,6 @@
package nametransform
import (
- "bytes"
"crypto/aes"
"encoding/base64"
"path/filepath"
@@ -21,7 +20,7 @@ const (
// NameTransformer is an interface used to transform filenames.
type NameTransformer interface {
DecryptName(cipherName string, iv []byte) (string, error)
- EncryptName(plainName string, iv []byte) string
+ EncryptName(plainName string, iv []byte) (string, error)
EncryptAndHashName(name string, iv []byte) (string, error)
// HashLongName - take the hash of a long string "name" and return
// "gocryptfs.longname.[sha256]"
@@ -99,22 +98,14 @@ func (n *NameTransform) decryptName(cipherName string, iv []byte) (string, error
bin = n.emeCipher.Decrypt(iv, bin)
bin, err = unPad16(bin)
if err != nil {
- tlog.Debug.Printf("DecryptName: unPad16 error detail: %v", err)
- // unPad16 returns detailed errors including the position of the
- // incorrect bytes. Kill the padding oracle by lumping everything into
- // a generic error.
+ tlog.Warn.Printf("DecryptName %q: unPad16 error: %v", cipherName, err)
return "", syscall.EBADMSG
}
- // A name can never contain a null byte or "/". Make sure we never return those
- // to the kernel, even when we read a corrupted (or fuzzed) filesystem.
- if bytes.Contains(bin, []byte{0}) || bytes.Contains(bin, []byte("/")) {
- return "", syscall.EBADMSG
- }
- // The name should never be "." or "..".
- if bytes.Equal(bin, []byte(".")) || bytes.Equal(bin, []byte("..")) {
+ plain := string(bin)
+ if err := IsValidName(plain); err != nil {
+ tlog.Warn.Printf("DecryptName %q: invalid name after decryption: %v", cipherName, err)
return "", syscall.EBADMSG
}
- plain := string(bin)
return plain, err
}
@@ -123,12 +114,16 @@ func (n *NameTransform) decryptName(cipherName string, iv []byte) (string, error
//
// This function is exported because in some cases, fusefrontend needs access
// to the full (not hashed) name if longname is used.
-func (n *NameTransform) EncryptName(plainName string, iv []byte) (cipherName64 string) {
+func (n *NameTransform) EncryptName(plainName string, iv []byte) (cipherName64 string, err error) {
+ if err := IsValidName(plainName); err != nil {
+ tlog.Warn.Printf("EncryptName %q: invalid plainName: %v", plainName, err)
+ return "", syscall.EBADMSG
+ }
bin := []byte(plainName)
bin = pad16(bin)
bin = n.emeCipher.Encrypt(iv, bin)
cipherName64 = n.B64.EncodeToString(bin)
- return cipherName64
+ return cipherName64, nil
}
// B64EncodeToString returns a Base64-encoded string
diff --git a/internal/nametransform/names_test.go b/internal/nametransform/names_test.go
index 9d77c2d..b4e98d4 100644
--- a/internal/nametransform/names_test.go
+++ b/internal/nametransform/names_test.go
@@ -2,6 +2,7 @@ package nametransform
import (
"bytes"
+ "strings"
"testing"
)
@@ -51,3 +52,26 @@ func TestUnpad16Garbage(t *testing.T) {
}
}
}
+
+func TestIsValidName(t *testing.T) {
+ cases := []struct {
+ in string
+ want bool
+ }{
+ {"", false},
+ {".", false},
+ {"..", false},
+ {"...", true},
+ {"asdasd/asdasd", false},
+ {"asdasd\000asdasd", false},
+ {"hello", true},
+ {strings.Repeat("x", 255), true},
+ {strings.Repeat("x", 256), false},
+ }
+ for _, c := range cases {
+ have := IsValidName(c.in)
+ if (have == nil) != c.want {
+ t.Errorf("IsValidName(%q): want %v have %v", c.in, c.want, have)
+ }
+ }
+}
diff --git a/internal/nametransform/valid.go b/internal/nametransform/valid.go
new file mode 100644
index 0000000..b991074
--- /dev/null
+++ b/internal/nametransform/valid.go
@@ -0,0 +1,27 @@
+package nametransform
+
+import (
+ "fmt"
+ "strings"
+)
+
+// IsValidName checks if `name` is a valid name for a normal file
+// (does not contain null bytes or "/" etc...).
+func IsValidName(name string) error {
+ if name == "" {
+ return fmt.Errorf("empty input")
+ }
+ if len(name) > NameMax {
+ return fmt.Errorf("too long")
+ }
+ // A name can never contain a null byte or "/". Make sure we never return those
+ // to the kernel, even when we read a corrupted (or fuzzed) filesystem.
+ if strings.Contains(name, "\000") || strings.Contains(name, "/") {
+ return fmt.Errorf("contains forbidden bytes")
+ }
+ // The name should never be "." or "..".
+ if name == "." || name == ".." {
+ return fmt.Errorf(". and .. are forbidden names")
+ }
+ return nil
+}
diff --git a/tests/example_filesystems/example_filesystems_test.go b/tests/example_filesystems/example_filesystems_test.go
index 88132c0..36fb554 100644
--- a/tests/example_filesystems/example_filesystems_test.go
+++ b/tests/example_filesystems/example_filesystems_test.go
@@ -333,7 +333,8 @@ func TestExampleFSv13reverse(t *testing.T) {
}
dirA = tmpFsPath + dirA
// Mount using password
- test_helpers.MountOrFatal(t, dirA, dirB, "-reverse", "-extpass", "echo test", opensslOpt)
+ // We pass "-wpanic=false" because the '..' and '.' tests deliverately trigger warnings
+ test_helpers.MountOrFatal(t, dirA, dirB, "-reverse", "-extpass", "echo test", "-wpanic=false", opensslOpt)
c := dirB + "/gocryptfs.conf"
if !test_helpers.VerifyExistence(t, c) {
t.Errorf("%s missing", c)
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-03 01:40:45 +0000