diff options
| author | Jakob Unterwurzacher | 2017-10-01 13:50:25 +0200 | 
|---|---|---|
| committer | Jakob Unterwurzacher | 2017-10-01 13:50:25 +0200 | 
| commit | 4da245c69d7994efec75e1deaef56a03020d39db (patch) | |
| tree | f9d429bd039774f4ecd06afb18844ece5a4d5bb4 | |
| parent | 0072a96f20c99bf386d8e3f63741def5a3945018 (diff) | |
fusefrontend_reverse: fix 176-byte names
A file with a name of exactly 176 bytes length caused this error:
  ls: cannot access ./tmp/dsg/sXSGJLTuZuW1FarwIkJs0w/b6mGjdxIRpaeanTo0rbh0A/QjMRrQZC_4WLhmHI1UOBcA/gocryptfs.longname.QV-UipdDXeUVdl05WruoEzBNPrQCfpu6OzJL0_QnDKY: No such file or directory
  ls: cannot access ./tmp/dsg/sXSGJLTuZuW1FarwIkJs0w/b6mGjdxIRpaeanTo0rbh0A/QjMRrQZC_4WLhmHI1UOBcA/gocryptfs.longname.QV-UipdDXeUVdl05WruoEzBNPrQCfpu6OzJL0_QnDKY.name: No such file or directory
  -????????? ? ?     ?             ?            ? gocryptfs.longname.QV-UipdDXeUVdl05WruoEzBNPrQCfpu6OzJL0_QnDKY
  -????????? ? ?     ?             ?            ? gocryptfs.longname.QV-UipdDXeUVdl05WruoEzBNPrQCfpu6OzJL0_QnDKY.name
Root cause was a wrong shortNameMax constant that failed to
account for the obligatory padding byte.
Fix the constant and also expand the TestLongnameStat test case
to test ALL file name lengths from 1-255 bytes.
Fixes https://github.com/rfjakob/gocryptfs/issues/143 .
| -rw-r--r-- | internal/fusefrontend_reverse/reverse_longnames.go | 11 | ||||
| -rw-r--r-- | tests/example_filesystems/example_filesystems_test.go | 2 | ||||
| -rw-r--r-- | tests/reverse/correctness_test.go | 36 | 
3 files changed, 30 insertions, 19 deletions
| diff --git a/internal/fusefrontend_reverse/reverse_longnames.go b/internal/fusefrontend_reverse/reverse_longnames.go index ec164f1..3bfe3af 100644 --- a/internal/fusefrontend_reverse/reverse_longnames.go +++ b/internal/fusefrontend_reverse/reverse_longnames.go @@ -17,7 +17,14 @@ import (  )  const ( -	shortNameMax = 176 +	// File names are padded to 16-byte multiples, encrypted and +	// base64-encoded. We can encode at most 176 bytes to stay below the 255 +	// bytes limit: +	// * base64(176 bytes) = 235 bytes +	// * base64(192 bytes) = 256 bytes (over 255!) +	// But the PKCS#7 padding is at least one byte. This means we can only use +	// 175 bytes for the file name. +	shortNameMax = 175  )  var longnameParentCache map[string]string @@ -57,6 +64,7 @@ func (rfs *ReverseFS) findLongnameParent(dir string, dirIV []byte, longname stri  	}  	dirEntries, err := dirfd.Readdirnames(-1)  	if err != nil { +		tlog.Warn.Printf("findLongnameParent: Readdirnames failed: %v\n", err)  		return "", err  	}  	longnameCacheLock.Lock() @@ -78,7 +86,6 @@ func (rfs *ReverseFS) findLongnameParent(dir string, dirIV []byte, longname stri  	if hit == "" {  		return "", syscall.ENOENT  	} -  	return hit, nil  } diff --git a/tests/example_filesystems/example_filesystems_test.go b/tests/example_filesystems/example_filesystems_test.go index cbea251..0732eb6 100644 --- a/tests/example_filesystems/example_filesystems_test.go +++ b/tests/example_filesystems/example_filesystems_test.go @@ -254,6 +254,8 @@ func TestExampleFSv13(t *testing.T) {  // gocryptfs v1.3 introduced HKDF.  // We check the md5 sum of the encrypted version of a file to make sure we don't  // accidentially change the ciphertext generation. +// Create a full crypto round-trip by mounting two times: +// dirA -> reverse mount -> dirB -> forward mount -> dirC  func TestExampleFSv13reverse(t *testing.T) {  	// Prepare directories  	dirA := "v1.3-reverse" diff --git a/tests/reverse/correctness_test.go b/tests/reverse/correctness_test.go index a5719eb..40b53f9 100644 --- a/tests/reverse/correctness_test.go +++ b/tests/reverse/correctness_test.go @@ -1,6 +1,7 @@  package reverse_test  import ( +	"bytes"  	"io/ioutil"  	"os"  	"syscall" @@ -10,25 +11,26 @@ import (  	"github.com/rfjakob/gocryptfs/tests/test_helpers"  ) +// TestLongnameStat checks that file names of all sizes (1 to 255) show up in +// the decrypted reverse view (dirC, mounted in TestMain).  func TestLongnameStat(t *testing.T) { -	fd, err := os.Create(dirA + "/" + x240) -	if err != nil { -		t.Fatal(err) -	} -	path := dirC + "/" + x240 -	if !test_helpers.VerifyExistence(path) { -		t.Fail() -	} -	test_helpers.VerifySize(t, path, 0) -	_, err = fd.Write(make([]byte, 10)) -	if err != nil { -		t.Fatal(err) +	for i := 1; i <= 255; i++ { +		name := string(bytes.Repeat([]byte("x"), i)) +		fd, err := os.Create(dirA + "/" + name) +		if err != nil { +			t.Fatal(err) +		} +		fd.Close() +		path := dirC + "/" + name +		if !test_helpers.VerifyExistence(path) { +			t.Fail() +		} +		test_helpers.VerifySize(t, path, 0) +		// A large number of longname files is a performance problem in +		// reverse mode. Delete the file once we are done with it to speed up +		// the test (2 seconds -> 0.2 seconds) +		syscall.Unlink(dirA + "/" + name)  	} -	fd.Close() -	/* -		time.Sleep(1000 * time.Millisecond) -		test_helpers.VerifySize(t, path, 10) -	*/  }  func TestSymlinks(t *testing.T) { | 
