Skip to content

Commit

Permalink
fusefrontend_reverse: Do not mix up cache information for different d…
Browse files Browse the repository at this point in the history
…irectories

Fixes #168

Steps to reproduce the problem:

* Create a regular reverse mount point
* Create files with the same very long name in multiple directories - so far
  everything works as expected, and it will appear with a different name each
  time, for example, gocryptfs.longname.A in directory A and
  gocryptfs.longname.B in directory B
* Try to access a path with A/gocryptfs.longname.B or B/gocryptfs.longname.A -
  this should fail, but it actually works.

The problem is that the longname cache only uses the path as key and not the
dir or divIV. Assume an attacker can directly interact with a reverse mount and
knows the relation longname path -> unencoded path in one directory, it allows
to test if the same unencoded filename appears in any other directory.
  • Loading branch information
slackner authored and rfjakob committed Nov 25, 2017
1 parent 95870e8 commit 9068721
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
4 changes: 2 additions & 2 deletions internal/fusefrontend_reverse/reverse_longnames.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func initLongnameCache() {
// findLongnameParent converts "gocryptfs.longname.XYZ" to the plaintext name
func (rfs *ReverseFS) findLongnameParent(dir string, dirIV []byte, longname string) (plaintextName string, err error) {
longnameCacheLock.Lock()
hit := longnameParentCache[longname]
hit := longnameParentCache[dir + "/" + longname]
longnameCacheLock.Unlock()
if hit != "" {
return hit, nil
Expand Down Expand Up @@ -79,7 +79,7 @@ func (rfs *ReverseFS) findLongnameParent(dir string, dirIV []byte, longname stri
log.Panic("logic error or wrong shortNameMax constant?")
}
hName := rfs.nameTransform.HashLongName(cName)
longnameParentCache[hName] = plaintextName
longnameParentCache[dir + "/" + hName] = plaintextName
if longname == hName {
hit = plaintextName
}
Expand Down
15 changes: 14 additions & 1 deletion tests/reverse/ctlsock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@ func TestCtlSockPathOps(t *testing.T) {
test_helpers.MountOrFatal(t, "ctlsock_reverse_test_fs", mnt, "-reverse", "-extpass", "echo test", "-ctlsock="+sock)
defer test_helpers.UnmountPanic(mnt)
var req ctlsock.RequestStruct
var response ctlsock.ResponseStruct
for i, tc := range ctlSockTestCases {
// Decrypt
req = ctlsock.RequestStruct{DecryptPath: tc[0]}
response := test_helpers.QueryCtlSock(t, sock, req)
response = test_helpers.QueryCtlSock(t, sock, req)
if response.ErrNo != 0 {
t.Errorf("Testcase %d Decrypt: %q ErrNo=%d ErrText=%s", i, tc[0], response.ErrNo, response.ErrText)
} else if response.Result != tc[1] {
Expand All @@ -53,6 +54,18 @@ func TestCtlSockPathOps(t *testing.T) {
t.Errorf("Testcase %d Encrypt: Want %q got %q", i, tc[1], response.Result)
}
}
// At this point the longname parent cache should be populated.
// Check that we do not mix up information for different directories.
req = ctlsock.RequestStruct{DecryptPath: "gocryptfs.longname.y6rxCn6Id8hIZL2t_STpdLZpu-aE2HpprJR25xD60mk="}
response = test_helpers.QueryCtlSock(t, sock, req)
if response.ErrNo != -1 {
t.Errorf("File should not exist: ErrNo=%d ErrText=%s", response.ErrNo, response.ErrText)
}
req = ctlsock.RequestStruct{DecryptPath: "v6puXntoQOk7Mhl8zJ4Idg==/gocryptfs.longname.ZQCAoi5li3xvDZRO8McBV0L_kzJc4IcAOEzuW-2S1Y4="}
response = test_helpers.QueryCtlSock(t, sock, req)
if response.ErrNo != -1 {
t.Errorf("File should not exist: ErrNo=%d ErrText=%s", response.ErrNo, response.ErrText)
}
}

// We should not panic when somebody feeds requests that make no sense
Expand Down

0 comments on commit 9068721

Please sign in to comment.