Age | Commit message (Collapse) | Author |
|
Makes it robust against symlink races.
Final piece, closes https://github.com/rfjakob/gocryptfs/issues/165
|
|
Protects findLongnameParent against symlink races.
Also add comments to several functions along the way.
Reported at https://github.com/rfjakob/gocryptfs/issues/165
|
|
gocryptfs.longname.XXX files were considered magic in PlaintextNames
mode, which was wrong.
Fix that and add tests.
Fixes https://github.com/rfjakob/gocryptfs/issues/174
|
|
|
|
|
|
In PlaintextNames mode the "gocryptfs.longname." prefix does not have any
special meaning.
https://github.com/rfjakob/gocryptfs/issues/174
|
|
Also get rid of the defer - it is not really necessary here.
|
|
Steps to reproduce:
* Create a regular reverse mount point
* Create a file "test" in the original directory
* Access the corresponding encrypted directory in the mount point (ls <encrypted dir>)
* Quickly delete the file in the original data - instead create a device node
* Access the file again, it will access the device node and attempt to read from it
Fixes https://github.com/rfjakob/gocryptfs/issues/187
|
|
Also fixes 48bd59f38843e5ebd4e4c9f666f1aea1c9990803 - the directory FD should
also be closed in case of an error.
|
|
|
|
Fixes https://github.com/rfjakob/gocryptfs/issues/184
|
|
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
|
|
Add faccessat(2) with a hack for symlink, because the
kernel does not actually looks at the passed flags.
From man 2 faccessat:
C library/kernel differences
The raw faccessat() system call takes only the first three argu‐
ments. The AT_EACCESS and AT_SYMLINK_NOFOLLOW flags are actually
implemented within the glibc wrapper function for faccessat().
|
|
...by using Readlinkat.
Tracking ticket: https://github.com/rfjakob/gocryptfs/issues/165
|
|
We need readlinkat to implement Readlink
symlink-race-free.
|
|
The "Atim" field is called "Atimespec" on Darwin,
same for Mtim and Ctim.
|
|
...by using the OpenNofollow helper & Fstatat.
Also introduce a helper to convert from unix.Stat_t to
syscall.Stat_t.
Tracking ticket: https://github.com/rfjakob/gocryptfs/issues/165
|
|
...when opening intermedia directories to give us an
extra layer of safety.
From the FreeBSD man page:
This flag can be used to prevent applications with elevated
privileges from opening files which are even unsafe to open with O_RDONLY,
such as device nodes.
|
|
...by using the new OpenNofollow helper.
The benchmark shows a small but acceptable performance loss:
$ ./benchmark-reverse.bash
LS: 2.182
CAT: 18.221
Tracking ticket: https://github.com/rfjakob/gocryptfs/issues/165
|
|
Sometimes want to open baseDir itself. This case
was broken, fix it.
|
|
Now that we have Fstatat we can use it in Getdents to
get rid of the path name.
Also, add an emulated version of getdents for MacOS. This allows
to drop the !HaveGetdents special cases from fusefrontend.
Modify the getdents test to test both native getdents and the emulated
version.
|
|
Fstatat has recently been added to x/sys/unix. Make
it available for use in gocryptfs.
|
|
...by ignoring the path that was passed in.
https://github.com/rfjakob/gocryptfs/issues/165
|
|
...using the new syscallcompat.OpenNofollow helper.
This change secures Open() against symlink race attacks
as described in https://github.com/rfjakob/gocryptfs/issues/165
|
|
OpenNofollow = symlink-race-safe Open
Prepares fixing https://github.com/rfjakob/gocryptfs/issues/165
|
|
The infrastructure will also be used by the upcoming
OpenNofollow tests.
|
|
I'm unsure why I did not notice this earlier, but the
syscall wrappers provided by x/sys/unix seem to do just
fine.
Drop our own version.
|
|
This avoids the conversion to an absolute path.
|
|
For absolute paths, the file descriptor should be ignored. In such a case
there is also no need to hold the lock or change the working directory.
|
|
As requested in https://github.com/rfjakob/gocryptfs/pull/179
|
|
...and fix the instances where the AT_SYMLINK_NOFOLLOW /
O_NOFOLLOW / O_EXCL flag was missing.
|
|
Also fix the bug in emulateFchmodat that was found by the tests.
|
|
This will allow to test them under linux as well.
|
|
Fixes the same problem as described in 72b975867a3b9bdf53fc2da62e2ba4a328d7e4ab,
except for directories instead of device nodes.
|
|
|
|
|
|
|
|
|
|
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
|
|
Mknod and Symlink
|
|
Fixes the same problem as described in 72b975867a3b9bdf53fc2da62e2ba4a328d7e4ab,
except for symlinks instead of device nodes.
|
|
|
|
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
|
|
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
|
|
* Acquire the lock before reading the current directory
* Fix a file descriptor leak
|
|
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.
|
|
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
|
|
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.
|