CheckTrailingGarbage was called even when "-passfile" was
used, which is stupid, and causes false positives:
https://github.com/rfjakob/gocryptfs/issues/391
(false error "Received trailing garbage after the password"
when using -passfile in .bash_profile)
Instead of trying to improve the logic to handle that case
and make everything even more complicated, delete the function.
It is unclear if actually helps in some cases, and it definitely
harms as shown by the above bug report.
This adds support for gitignore-like wildcards and exclude patters in
reverse mode. It (somewhat) fixes#273: no regexp support, but the
syntax should be powerful enough to satisfy most needs.
Also, since adding a lot of --exclude options can be tedious, it adds
the --exclude-from option to read patterns from a file (or files).
Ironically, the test for DirIV races had a data race itself
by writing to a bool without taking a lock.
Found by running "./test.bash -race":
WARNING: DATA RACE
Write at 0x00c00001dea5 by goroutine 22:
github.com/rfjakob/gocryptfs/tests/defaults.TestDirIVRace.func1()
/home/jakob/go/src/github.com/rfjakob/gocryptfs/tests/defaults/diriv_test.go:39 +0x38
github.com/rfjakob/gocryptfs/tests/defaults.TestDirIVRace()
/home/jakob/go/src/github.com/rfjakob/gocryptfs/tests/defaults/diriv_test.go:73 +0x65c
testing.tRunner()
/usr/local/go/src/testing/testing.go:865 +0x163
Previous read at 0x00c00001dea5 by goroutine 23:
github.com/rfjakob/gocryptfs/tests/defaults.TestDirIVRace.func2()
/home/jakob/go/src/github.com/rfjakob/gocryptfs/tests/defaults/diriv_test.go:51 +0x8b
Goroutine 22 (running) created at:
testing.(*T).Run()
/usr/local/go/src/testing/testing.go:916 +0x699
testing.runTests.func1()
/usr/local/go/src/testing/testing.go:1157 +0xa8
testing.tRunner()
/usr/local/go/src/testing/testing.go:865 +0x163
testing.runTests()
/usr/local/go/src/testing/testing.go:1155 +0x523
testing.(*M).Run()
/usr/local/go/src/testing/testing.go:1072 +0x2eb
github.com/rfjakob/gocryptfs/tests/defaults.TestMain()
/home/jakob/go/src/github.com/rfjakob/gocryptfs/tests/defaults/main_test.go:21 +0xe1
main.main()
_testmain.go:76 +0x222
Goroutine 23 (running) created at:
github.com/rfjakob/gocryptfs/tests/defaults.TestDirIVRace()
/home/jakob/go/src/github.com/rfjakob/gocryptfs/tests/defaults/diriv_test.go:43 +0x48d
testing.tRunner()
/usr/local/go/src/testing/testing.go:865 +0x163
==================
--- FAIL: TestDirIVRace (0.00s)
testing.go:809: race detected during execution of test
FAIL
This should get rid of
Openat: O_NOFOLLOW missing: flags = 0x0
Fchmodat: adding missing AT_SYMLINK_NOFOLLOW flag
sys_common_test.go:203: chmod on symlink should have failed, but did not. New mode=0333
UnmountErr: "[...]/057376762.mnt" was not found in MountInfo, cannot check for FD leak
and add some context to
--- FAIL: TestUtimesNano (0.00s)
matrix_test.go:628: no such file or directory
See https://github.com/rfjakob/gocryptfs/pull/343#issuecomment-453888006
for full test output
Setting/removing extended attributes on directories was partially fixed with
commit eff35e60b6. However, on most file systems
it is also possible to do these operations without read access (see tests).
Since we cannot open a write-access fd to a directory, we have to use the
/proc/self/fd trick (already used for ListXAttr) for the other operations aswell.
For simplicity, let's separate the Linux and Darwin code again (basically revert
commit f320b76fd1), and always use the
/proc/self/fd trick on Linux. On Darwin we use the best-effort approach with
openBackingFile() as a fallback.
More discussion about the available options is available in
https://github.com/rfjakob/gocryptfs/issues/308.
Give the gocryptfs process one extra millisecond to close
files. Allows us to drop several other sleeps.
UnmountErr now really returns an error when it detects an fd leak
instead of just printing a message.
An Open() a fifo blocks until it is opened for writing.
This meant that xattr operations on FIFOs would block.
Pass O_NONBLOCK to fix that, and add a test.
Retry with length 1000 if length 4000 fails, which
should work on all filesystems.
Failure was:
--- FAIL: TestTooLongSymlink (0.00s)
correctness_test.go:198: symlink xxx[...]xxxx /tmp/xfs.mnt/gocryptfs-test-parent/549823072/365091391/TooLongSymlink: file name too long
https://github.com/rfjakob/gocryptfs/issues/267
Instead of reporting the consequence:
matrix_test.go:906: modeHave 0664 != modeWant 0777
Report it if chmod itself fails, and also report the old file mode:
matrix_test.go:901: chmod 000 -> 777 failed: bad file descriptor
The cipherdir path is used as the fsname, as displayed
in "df -T". Now, having a comma in fsname triggers a sanity check
in go-fuse, aborting the mount with:
/bin/fusermount: mount failed: Invalid argument
fuse.NewServer failed: fusermount exited with code 256
Sanitize fsname by replacing any commas with underscores.
https://github.com/rfjakob/gocryptfs/issues/262
Rename openBackingPath to openBackingDir and use OpenDirNofollow
to be safe against symlink races. Note that openBackingDir is
not used in several important code paths like Create().
But it is used in Unlink, and the performance impact in the RM benchmark
to be acceptable:
Before
$ ./benchmark.bash
Testing gocryptfs at /tmp/benchmark.bash.bYO: gocryptfs v1.6-12-g930c37e-dirty; go-fuse v20170619-49-gb11e293; 2018-09-08 go1.10.3
WRITE: 262144000 bytes (262 MB, 250 MiB) copied, 1.07979 s, 243 MB/s
READ: 262144000 bytes (262 MB, 250 MiB) copied, 0.882413 s, 297 MB/s
UNTAR: 16.703
MD5: 7.606
LS: 1.349
RM: 3.237
After
$ ./benchmark.bash
Testing gocryptfs at /tmp/benchmark.bash.jK3: gocryptfs v1.6-13-g84d6faf-dirty; go-fuse v20170619-49-gb11e293; 2018-09-08 go1.10.3
WRITE: 262144000 bytes (262 MB, 250 MiB) copied, 1.06261 s, 247 MB/s
READ: 262144000 bytes (262 MB, 250 MiB) copied, 0.947228 s, 277 MB/s
UNTAR: 17.197
MD5: 7.540
LS: 1.364
RM: 3.410
If we encounter a 128KB block of zeros, try to skip to the next
data section by calling File.SeekData().
This fixes xfstests generic/285, which creates a 17TB sparse file,
and runs fsck afterwards. Without this optimization, fsck would
take ages.
TrezorPayload stores 32 random bytes used for unlocking
the master key using a Trezor security module. The randomness makes sure
that a unique unlock value is used for each gocryptfs filesystem.
When mounted via /etc/fstab like this,
/a /b fuse.gocryptfs default 0 0
we always get extra options passed. As reported by @mahkoh
at https://github.com/rfjakob/gocryptfs/pull/233 :
mount passes `-o noexec` if `-o user` is set and `-o exec` is not set.
If both `-o user` and `-o exec` are set, it passes `-o exec`.
Make these options work, and in addtion, also make -suid and -rw
work the same way.
Reported-by: @mahkoh
mv is unhappy when we return EPERM when it tries to set
system.posix_acl_access:
mv: preserving permissions for ‘b/x’: Operation not permitted
Now we return EOPNOTSUPP like tmpfs does and mv seems happy.
Values a binary-safe, there is no need to base64-encode them.
Old, base64-encoded values are supported transparently
on reading. Writing xattr values now always writes them binary.
The tests write to the example_filesystems folder, which
1) May leave your source tree in a modified state
2) Triggers test failures when the fsck tests run concurrently,
which happens on Travis CI every now and then.
Fix both problem by copying the example_filesystems folder
to a private location in /tmp.
And fix two in test_helpers.Mount().
Leftover fds can cause an unmount failure like this later:
fusermount: failed to unmount /tmp/gocryptfs-test-parent/873632270/default-plain: Device or resource busy
so try to catch them early.
Most corruption cases except xattr should be covered.
With test filesystem.
The output is still pretty ugly. xattr support will
be added in the next commits.
SwitchTestParentDir changes testParentDir. This is used when you want
to perform tests on a special filesystem. For example, the xattr tests
cannot run on tmpfs and use /var/tmp instead of /tmp.
Extracting the symlink fails with
linux-3.0/arch/microblaze/boot/dts/system.dts: Can't set permissions to 0755
so just exclude it.
The ln error Looks scary but is harmless, so get rid of it.
The symlink is only created to make it more convenient to view the
csv log.
macos does not have lazy unmount, so let's not use it
on linux either.
If the unmount fails, run "lsof" to find the open file.
Also fix the first bug we found this way.
We relied on the finalizer to close a few fds.
For some reason, this did not cause problems on Linux,
but on MacOS, it causes unmount failures:
umount(/private/tmp/gocryptfs-test-parent/194654785/default-plain): Resource busy -- try 'diskutil unmount'
To Go test logic waits for stderr and stdout to close, so
when we share it with a subprocess, it will wait for it to
exit as well.
We don't want the tests to hang when the unmount fails.
Seen on MacOS as reported at
https://github.com/rfjakob/gocryptfs/issues/213
Unfortunately, faccessat in Linux ignores AT_SYMLINK_NOFOLLOW,
so this is not completely atomic.
Given that the information you get from access is not very
interesting, it seems good enough.
https://github.com/rfjakob/gocryptfs/issues/165
In PlaintextNames mode the "gocryptfs.longname." prefix does not have any
special meaning. We should not attempt to delete any .name files.
Partially fixes https://github.com/rfjakob/gocryptfs/issues/174
In PlaintextNames mode the "gocryptfs.longname." prefix does not have any
special meaning. We should not attempt to read the directory IV or to
create special .name files.
Partially fixes https://github.com/rfjakob/gocryptfs/issues/174
If the user manages to replace the directory with
a symlink at just the right time, we could be tricked
into chown'ing the wrong file.
This change fixes the race by using fchownat, which
unfortunately is not available on darwin, hence a compat
wrapper is added.
Scenario, as described by @slackner at
https://github.com/rfjakob/gocryptfs/issues/177 :
1. Create a forward mount point with `plaintextnames` enabled
2. Mount as root user with `allow_other`
3. For testing purposes create a file `/tmp/file_owned_by_root`
which is owned by the root user
4. As a regular user run inside of the GoCryptFS mount:
```
mkdir tempdir
mknod tempdir/file_owned_by_root p &
mv tempdir tempdir2
ln -s /tmp tempdir
```
When the steps are done fast enough and in the right order
(run in a loop!), the device file will be created in
`tempdir`, but the `lchown` will be executed by following
the symlink. As a result, the ownership of the file located
at `/tmp/file_owned_by_root` will be changed.
If the symlink target gets too long due to base64 encoding, we should
return ENAMETOOLONG instead of having the kernel reject the data and
returning an I/O error to the user.
Fixes https://github.com/rfjakob/gocryptfs/issues/167
Fixes https://github.com/rfjakob/gocryptfs/issues/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.
Fixes https://github.com/rfjakob/gocryptfs/issues/170
Steps to reproduce the problem:
* Create a regular forward mount point
* Create a file with a shortname and one with a long filename
* Try to run 'mv <shortname> <longname>'
This should actually work and replace the existing file, but instead it
fails with:
mv: cannot move '<shortname>' to '<longname>': File exists
The problem is the creation of the .name file. If the target already exists
we can safely ignore the EEXIST error and just keep the existing .name file.
Allows to use /dev/random for generating the master key instead of the
default Go implementation. When the kernel random generator has been
properly initialized both are considered equally secure, however:
* Versions of Go prior to 1.9 just fall back to /dev/urandom if the
getrandom() syscall would be blocking (Go Bug #19274)
* Kernel versions prior to 3.17 do not support getrandom(), and there
is no check if the random generator has been properly initialized
before reading from /dev/urandom
This is especially useful for embedded hardware with low-entroy. Please
note that generation of the master key might block indefinitely if the
kernel cannot harvest enough entropy.
The extended TestLongnameStat() exposes a pathological case
when run on ext4, as ext4 reuses inode numbers immediately.
This change modifies the test to not delete the files immediately,
so the inode numbers cannot be reused immediately.
Fix for the underlying issue is a TODO.
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 .
I have added a subset of fsstress-gocryptfs.bash to EncFS as
fsstress-encfs.sh, improving the code a bit.
This change forward-ports these improvements to
fsstress-gocryptfs.bash.
Due to RMW, we always need read permissions on the backing file. This is a
problem if the file permissions do not allow reading (i.e. 0200 permissions).
This patch works around that problem by chmod'ing the file, obtaining a fd,
and chmod'ing it back.
Test included.
Issue reported at: https://github.com/rfjakob/gocryptfs/issues/125
This commit defines all exit codes in one place in the exitcodes
package.
Also, it adds a test to verify the exit code on incorrect
password, which is what SiriKali cares about the most.
Fixes https://github.com/rfjakob/gocryptfs/issues/77 .
Misspell Finds commonly misspelled English words
gocryptfs/internal/configfile/scrypt.go
Line 41: warning: "paramter" is a misspelling of "parameter" (misspell)
gocryptfs/internal/ctlsock/ctlsock_serve.go
Line 1: warning: "implementes" is a misspelling of "implements" (misspell)
gocryptfs/tests/test_helpers/helpers.go
Line 27: warning: "compatability" is a misspelling of "compatibility" (misspell)
This test reproduces the problem xfstests generic/124 uncovered.
The warning itself is harmless, but we should either (1) add locking
so that this cannot happen anymore or (2) drop the warning.
Currently fails:
$ go test -v
=== RUN Test1980Tar
--- PASS: Test1980Tar (0.00s)
=== RUN TestCtlSock
--- PASS: TestCtlSock (0.10s)
=== RUN TestOpenTruncateRead
--- PASS: TestOpenTruncateRead (0.00s)
=== RUN TestWORead
--- PASS: TestWORead (0.00s)
=== RUN TestXfs124
cipherSize 18 == header size: interrupted write?
-wpanic turns this warning into a panic: cipherSize 18 == header size: interrupted write?
We do not have to track the writeOnly status because the kernel
will not forward read requests on a write-only FD to us anyway.
I have verified this behavoir manually on a 4.10.8 kernel and also
added a testcase.
The filesystem was created with a gocryptfs version that ignored
the HKDF flag (hence everything was actually encrypted WITHOUT hkdf).
Fix it by recreating it.
TestMain() runs all tests twice, once with plaintextnames=true and once
with false. Several tests mount their own filesystem and ignore the
plaintextnames variable. It makes no sense to run them twice, so
skip execution when plaintextnames is set.
Prior to this commit, gocryptfs's reverse mode did not report correct
directory entry sizes for symbolic links, where the dentry size needs to
be the same as the length of a string containing the target path.
This commit corrects this issue and adds a test case to verify the
correctness of the implementation.
This issue was discovered during the use of a strict file copying program
on a reverse-mounted gocryptfs file system.
From the comment:
// CheckTrailingGarbage tries to read one byte from stdin and exits with a
// fatal error if the read returns any data.
// This is meant to be called after reading the password, when there is no more
// data expected. This helps to catch problems with third-party tools that
// interface with gocryptfs.
Test that we get the right timestamp when extracting a tarball.
Also simplify the workaround in doTestUtimesNano() and fix the
fact that it was running no test at all.
