Commit Graph

972 Commits

Author SHA1 Message Date
Jakob Unterwurzacher e97c23e083 syscallcompat: check that we get NOFOLLOW wherever possible
...and fix the instances where the AT_SYMLINK_NOFOLLOW /
O_NOFOLLOW / O_EXCL flag was missing.
2017-11-30 19:40:53 +01:00
Jakob Unterwurzacher 22282aefe6 syscallcompat: add tests for emulated syscalls
Also fix the bug in emulateFchmodat that was found by the tests.
2017-11-30 19:10:21 +01:00
Jakob Unterwurzacher bd79a8cd0d syscallcompat: build emulated syscalls under linux, too
This will allow to test them under linux as well.
2017-11-30 17:07:55 +01:00
Sebastian Lackner 614745ee57 fusefrontend: allow_other: close race between mkdir and chown
Fixes the same problem as described in 72b975867a,
except for directories instead of device nodes.
2017-11-29 13:28:04 +01:00
Sebastian Lackner 67bcbe81e8 fusefrontend: Use Fchownat to implement Chown 2017-11-29 13:05:46 +01:00
Sebastian Lackner 0162392a28 fusefrontend: Use Fchmodat to implement Chmod 2017-11-29 12:55:41 +01:00
Sebastian Lackner 0f44c617d0 syscallcompat: Introduce unlinkat syscall with flags argument 2017-11-29 12:41:23 +01:00
Sebastian Lackner 5d44a31b41 fusefrontend: Use openBackingPath in Unlink and simplify code 2017-11-28 09:28:06 +01:00
Sebastian Lackner 2591900b69 fusefrontend: Handle PlaintextNames mode in Unlink
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
2017-11-28 09:28:06 +01:00
Sebastian Lackner eba49402e4 fusefrontend: Introduce a openBackingPath helper and use it to simplify Mknod and Symlink 2017-11-28 09:28:06 +01:00
Sebastian Lackner ad2720e0f9 fusefrontend: allow_other: close race between symlink and chown
Fixes the same problem as described in 72b975867a,
except for symlinks instead of device nodes.
2017-11-28 09:28:06 +01:00
Sebastian Lackner 5a56810603 fusefrontend: Use the Symlinkat syscall for longname handling 2017-11-28 09:28:06 +01:00
Sebastian Lackner 295c4c2b85 fusefrontend: Set owner after symlink creation in PlaintextNames mode
This is already done in regular mode, but was missing when PlaintextNames mode
is enabled. As a result, symlinks created by non-root users were still owned
by root afterwards.

Fixes https://github.com/rfjakob/gocryptfs/issues/176
2017-11-28 09:28:06 +01:00
Sebastian Lackner 3f68b0c09a fusefrontend: Handle PlaintextNames mode in Mknod
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
2017-11-28 09:28:06 +01:00
Sebastian Lackner 8c5069c637 syscallcompat: Fix Fchownat syscall wrapper on darwin
* Acquire the lock before reading the current directory
* Fix a file descriptor leak
2017-11-28 09:28:06 +01:00
Jakob Unterwurzacher 72b975867a fusefronted: allow_other: close race between mknod and chown
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.
2017-11-27 21:04:45 +01:00
Jakob Unterwurzacher 1bb47b6796 reverse: reject too-long symlink target reads with ENAMETOOLONG
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
2017-11-26 21:37:12 +01:00
Sebastian Lackner 90687215a4 fusefrontend_reverse: Do not mix up cache information for different directories
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.
2017-11-25 16:20:48 +01:00
Sebastian Lackner 95870e841e fusefrontend: Skip gocryptfs.diriv handling when directory was deleted successfully
Fixes https://github.com/rfjakob/gocryptfs/issues/171

Steps to reproduce:

* Create a regular forward mount point
* Create a new directory in the mount point
* Manually delete the gocryptfs.diriv file from the corresponding ciphertext
  directory
* Attempt to delete the directory with 'rmdir <dirname>'

Although the code explicitly checks for empty directories, it will still attempt
to move the non-existent gocryptfs.diriv file and fails with:

    rmdir: failed to remove '<dirname>': No such file or directory
2017-11-25 16:20:00 +01:00
Sebastian Lackner 9f56b33e0c fusefrontend: Fix longname handling for renames with existing target
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.
2017-11-25 16:19:09 +01:00
Sebastian Lackner d257bb34c1 tests: Add test for access to encrypted version of '.' and '..'
To show that https://github.com/rfjakob/gocryptfs/issues/163 has been fixed.
2017-11-23 08:48:00 +01:00
Sebastian Lackner f80f19f589 fusefrontend_reverse: Add a missing Close() call 2017-11-22 23:42:49 +01:00
Sebastian Lackner c547673529 nametransform: Return error if decrypted name is '.' or '..' 2017-11-22 23:42:08 +01:00
Sebastian Lackner f3c777d5ea main: Add '-devrandom' commandline option
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.
2017-11-21 23:37:06 +01:00
Jakob Unterwurzacher 1b0426bcb2 main: print clear error message if CIPHERDIR is missing
Getting just the help text in response to

	gocryptfs -info -config external.config

is confusing: https://github.com/rfjakob/gocryptfs/issues/157
2017-11-15 20:30:21 +01:00
Jakob Unterwurzacher e36a0ebf18 main: add "-sharedstorage" flag
At the moment, it does two things:

1. Disable stat() caching so changes to the backing storage show up
   immediately.
2. Disable hard link tracking, as the inode numbers on the backing
   storage are not stable when files are deleted and re-created behind
   our back. This would otherwise produce strange "file does not exist"
   and other errors.

Mitigates https://github.com/rfjakob/gocryptfs/issues/156
2017-11-12 20:06:13 +01:00
Jakob Unterwurzacher 9ab6cdb9b9 test.bash: don't run "go tool vet" if vendor dir exists
...this fails in a thousand ways:

[...]
vendor/golang.org/x/crypto/sha3/keccakf_amd64.s:324: [amd64] keccakF1600: unknown variable state; offset 0 is a+0(FP)
vendor/golang.org/x/crypto/ssh/certs.go:172: declaration of "err" shadows declaration at vendor/golang.org/x/crypto/ssh/certs.go:166
vendor/golang.org/x/crypto/ssh/certs.go:187: declaration of "rest" shadows declaration at vendor/golang.org/x/crypto/ssh/certs.go:161
vendor/golang.org/x/crypto/ssh/certs.go:187: declaration of "ok" shadows declaration at vendor/golang.org/x/crypto/ssh/certs.go:161
vendor/golang.org/x/crypto/ssh/client_auth.go:226: declaration of "err" shadows declaration at vendor/golang.org/x/crypto/ssh/client_auth.go:193
vendor/golang.org/x/crypto/ssh/client_auth.go:394: declaration of "err" shadows declaration at vendor/golang.org/x/crypto/ssh/client_auth.go:380
vendor/golang.org/x/crypto/ssh/client_auth.go:405: declaration of "err" shadows declaration at vendor/golang.org/x/crypto/ssh/client_auth.go:380
vendor/golang.org/x/crypto/ssh/handshake.go:566: declaration of "err" shadows declaration at vendor/golang.org/x/crypto/ssh/handshake.go:547
vendor/golang.org/x/crypto/ssh/handshake.go:592: declaration of "err" shadows declaration at vendor/golang.org/x/crypto/ssh/handshake.go:547
vendor/golang.org/x/crypto/ssh/handshake.go:630: declaration of "err" shadows declaration at vendor/golang.org/x/crypto/ssh/handshake.go:620
[...]
2017-11-12 13:05:27 +01:00
Jakob Unterwurzacher 843138168f package-source.bash: replace plus sign in file name
Plus signs are apparently not supported on github,
and replaced by a dot.
2017-11-01 20:05:15 +01:00
Jakob Unterwurzacher 5428567fa0 package-source.bash: create VERSION file
...and delete if after packaging is done.
2017-11-01 19:41:47 +01:00
Jakob Unterwurzacher 6c2c2b9a8b README: Update changelog for v1.4.2 2017-11-01 19:29:11 +01:00
Jakob Unterwurzacher 2aaf9c6387 dep: update dependencies 2017-11-01 19:11:26 +01:00
Jakob Unterwurzacher 4c11e8a4e5 travis: update Go versions
...to latest point releases acc. to https://golang.org/dl/
2017-11-01 19:02:34 +01:00
Jakob Unterwurzacher 39839ade70 Add source packaging script 2017-11-01 18:58:10 +01:00
Jakob Unterwurzacher 9a3791fbc1 build.bash: support VERSION file and vendored go-fuse
Prepares for the release of all-in-one source tarballs
that include all non-stdlib dependencies.
2017-11-01 16:09:47 +01:00
Jakob Unterwurzacher a1a98abfbb main: disallow recursively encrypting ourselves
From https://github.com/rfjakob/gocryptfs/issues/150:

  mkdir a
  mkdir a/b
  gocryptsfs -init -reverse a/
  gocryptfs -reverse a/ a/b

  Now directory a/b/ contains encrypted view of 'a' but it
  is possible to descend into encrypted version of b (e.g.
  a/b/43873uhj538765387/) which contains double encrypted
  'a' and so on.

Reported-by: https://github.com/tigmac
2017-10-31 19:48:01 +01:00
Jakob Unterwurzacher 34547a6c39 tests: don't read /proc, the number of entries changes too quickly
This could lead to test failures like this:

  --- FAIL: TestGetdents (0.02s)
  	getdents_test.go:57: len(getdentsEntries)=362, len(readdirEntries)=360
  FAIL
2017-10-22 18:13:08 +02:00
Jakob Unterwurzacher e9f6c7ad67 Revert "test.bash: use "go vet" instead of "go tool vet""
"go vet" on Go 1.8 and older does not support flags:

$ go version
go version go1.8.3 linux/amd64

$ ./test.bash
gocryptfs v1.4.1-27-g8c1b363 without_openssl; go-fuse v20170619-21-gcf21bc2; 2017-10-22 go1.8.3
gocryptfs v1.4.1-27-g8c1b363; go-fuse v20170619-21-gcf21bc2; 2017-10-22 go1.8.3
flag provided but not defined: -all
usage: vet [-n] [-x] [build flags] [packages]
Vet runs the Go vet command on the packages named by the import paths.
For more about vet, see 'go doc cmd/vet'.
For more about specifying packages, see 'go help packages'.
To run the vet tool with specific options, run 'go tool vet'.
The -n flag prints commands that would be executed.
The -x flag prints commands as they are executed.
For more about build flags, see 'go help build'.
See also: go fmt, go fix.

This reverts commit a1170be979.
2017-10-22 15:00:19 +02:00
Jakob Unterwurzacher 8c1b363f74 reverse mode: disable ClientInodes (hard link tracking)
Disable hard link tracking to avoid strange breakage on duplicate
inode numbers ( https://github.com/rfjakob/gocryptfs/issues/149 ).

Reverse mode is read-only, so we don't need a working link().
2017-10-22 14:43:24 +02:00
Jakob Unterwurzacher a1170be979 test.bash: use "go vet" instead of "go tool vet"
"go vet" automatically skips the vendor directory.
"go tool vet" does not, and it will complain about a lot
of things in there.
2017-10-22 14:37:44 +02:00
Jakob Unterwurzacher 4954c87979 Always set "max_read" kernel option
We use fixed-size byte slice pools (sync.Pool) and cannot
handle larger requests. So ask the kernel to not send
bigger ones.

Fixes https://github.com/rfjakob/gocryptfs/issues/145
2017-10-21 18:06:55 +02:00
Jakob Unterwurzacher 268e0484e2 Revert most of "fusefrontend: clamp oversized reads"
We cannot return less data than requested to the kernel!

From https://libfuse.github.io/doxygen/structfuse__operations.html:

  Read should return exactly the number of bytes
  requested except on EOF or error, otherwise the
  rest of the data will be substituted with
  zeroes.

Reverts commit 3009ec9852 minus
the formatting improvements we want to keep.

Fixes https://github.com/rfjakob/gocryptfs/issues/147
Reopens https://github.com/rfjakob/gocryptfs/issues/145
2017-10-21 17:43:21 +02:00
Jakob Unterwurzacher b3c20e512f MANPAGE: explain that you may have to pass -aessiv with -masterkey
...if the filesystem was created with that option (or reverse
mode).

Mitigates https://github.com/rfjakob/gocryptfs/issues/148
2017-10-19 22:04:46 +02:00
Jakob Unterwurzacher 29445c976d contentenc: reserve one additional block in CReqPool
...to account for unaligned reads.

I have not seen this happen in the wild because the kernel
always seems to issue 4k-aligned requests. But the cost
of the additional block in the pool is low and prevents
a buffer overrun panic when an unaligned read does happen.
2017-10-19 09:23:10 +02:00
Jakob Unterwurzacher 2783eadc8f main: call logger with full path
If $PATH contains the mountpoint, searching through it
will lock us up. Use an absolute path to avoid looking
at $PATH.

Fixes https://github.com/rfjakob/gocryptfs/issues/146
2017-10-18 20:25:34 +02:00
Jakob Unterwurzacher 3009ec9852 fusefrontend: clamp oversized reads
Our byte cache pools are sized acc. to MAX_KERNEL_WRITE, but the
running kernel may have a higher limit set. Clamp to what we can
handle.

Fixes a panic on a Synology NAS reported at
https://github.com/rfjakob/gocryptfs/issues/145
2017-10-17 21:48:29 +02:00
Jakob Unterwurzacher 64e5906ffa fusefrontend_reverse: workaround ext4 test failure
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.
2017-10-03 21:15:17 +02:00
Jakob Unterwurzacher 4da245c69d 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 .
2017-10-01 13:50:25 +02:00
Jakob Unterwurzacher 0072a96f20 siv_aead: fix trivial typo in comment 2017-09-17 11:42:46 +02:00
Jakob Unterwurzacher 885fdcabda contentenc: deduplicate AD packing into new concatAD() func
The encrypt and decrypt path both had a copy that were equivalent
but ordered differently, which was confusing.

Consolidate it in a new dedicated function.
2017-09-17 11:21:48 +02:00
Jakob Unterwurzacher 4bd2c6736a contentenc: DecryptBlocks: give block number counter a clearer name
Using firstBlockNo as the counter is confusing, create a
copy named "blockNo" and use that.
2017-09-17 10:59:04 +02:00