Use openBackingDir() and Fstatat().
High performance impact, though part of it should be
mitigated by adding DirIV caching to the new code paths.
$ ./benchmark.bash
Testing gocryptfs at /tmp/benchmark.bash.Eou: gocryptfs v1.6-37-ge3914b3-dirty; go-fuse v20170619-66-g6df8ddc; 2018-10-14 go1.11
WRITE: 262144000 bytes (262 MB, 250 MiB) copied, 1.2289 s, 213 MB/s
READ: 262144000 bytes (262 MB, 250 MiB) copied, 1.02616 s, 255 MB/s
UNTAR: 24.490
MD5: 13.120
LS: 3.368
RM: 9.232
The directory was already created, so return success even if Fchownat fails.
The same error handling is already used if fs.args.PlaintextNames is false.
Even though filesystem notifications aren't implemented for FUSE, I decided to
try my hand at implementing the autounmount feature (#128). I based it on the
EncFS autounmount code, which records filesystem accesses and checks every X
seconds whether it's idled long enough to unmount.
I've tested the feature locally, but I haven't added any tests for this flag.
I also haven't worked with Go before. So please let me know if there's
anything that should be done differently.
One particular concern: I worked from the assumption that the open files table
is unique per-filesystem. If that's not true, I'll need to add an open file
count and associated lock to the Filesystem type instead.
https://github.com/rfjakob/gocryptfs/pull/265
Error was:
# github.com/rfjakob/gocryptfs/internal/fusefrontend
internal/fusefrontend/fs.go:179: cannot use perms | 256 (type uint16) as type uint32 in argument to syscall.Fchmod
internal/fusefrontend/fs.go:185: cannot use perms (type uint16) as type uint32 in argument to syscall.Fchmod
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
As uncovered by xfstests generic/465, concurrent reads and writes
could lead to this,
doRead 3015532: corrupt block #1039: stupidgcm: message authentication failed,
as the read could pick up a block that has not yet been completely written -
write() is not atomic!
Now writes take ContentLock exclusively, while reads take it shared,
meaning that multiple reads can run in parallel with each other, but
not with a write.
This also simplifies the file header locking.
xfstests generic/083 fills the filesystem almost completely while
running fsstress in parallel. In fsck, these would show up:
readFileID 2580: incomplete file, got 18 instead of 19 bytes
This could happen when writing the file header works, but writing
the actual data fails.
Now we kill the header again by truncating the file to zero.
If the underlying filesystem is full, it is normal get ENOSPC here.
Log at Info level instead of Warning.
Fixes xfstests generic/015 and generic/027, which complained about
the extra output.
O_DIRECT accesses must be aligned in both offset and length. Due to our
crypto header, alignment will be off, even if userspace makes aligned
accesses. Running xfstests generic/013 on ext4 used to trigger lots of
EINVAL errors due to missing alignment. Just fall back to buffered IO.
"gocryptfs -fsck" will need access to helper functions,
and to get that, it will need to cast a gofuse.File to a
fusefrontend.File. Make fusefrontend.File exported to make
this work.
We are clean again.
Warnings were:
internal/fusefrontend/fs.go:443:14: should omit type string from declaration
of var cTarget; it will be inferred from the right-hand side
internal/fusefrontend/xattr.go:26:1: comment on exported method FS.GetXAttr
should be of the form "GetXAttr ..."
internal/syscallcompat/sys_common.go:9:7: exported const PATH_MAX should have
comment or be unexported
Reading system.posix_acl_access and system.posix_acl_default
should return EOPNOTSUPP to inform user-space that we do not
support ACLs.
xftestest essientially does
chacl -l | grep "Operation not supported"
to determine if the filesystem supports ACLs, and used to
wrongly believe that gocryptfs does.
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.
We previously returned EPERM to prevent the kernel from
blacklisting our xattr support once we get an unsupported
flag, but this causes lots of trouble on MacOS:
Cannot save files from GUI apps, see
https://github.com/rfjakob/gocryptfs/issues/229
Returning ENOSYS triggers the dotfiles fallback on MacOS
and fixes the issue.
OpenDir and ListXAttr skip over corrupt entries,
readFileID treats files the are too small as empty.
This improves usability in the face of corruption,
but hides the problem in a log message instead of
putting it in the return code.
Create a channel to report these corruptions to fsck
so it can report them to the user.
Also update the manpage and the changelog with the -fsck option.
Closes https://github.com/rfjakob/gocryptfs/issues/191
This should not happen via FUSE as the kernel caps the size,
but with fsck we have the first user that calls Read directly.
For symmetry, check it for Write as well.
A few places have called tlog.Warn.Print, which directly
calls into log.Logger due to embedding, losing all features
of tlog.
Stop embedding log.Logger to make sure the internal functions
cannot be called accidentially and fix (several!) instances
that did.
Both fusefrontend and fusefrontend_reverse were doing
essentially the same thing, move it into main's
initFuseFrontend.
A side-effect is that we have a reference to cryptocore
in main, which will help with wiping the keys on exit
(https://github.com/rfjakob/gocryptfs/issues/211).
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.
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
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
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.
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.
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
MacOS creates lots of these files, and if the directory is otherwise
empty, we would throw an IO error to the unsuspecting user.
With this patch, we log a warning, but otherwise pretend we did not
see it.
Mitigates https://github.com/rfjakob/gocryptfs/issues/140
...and if Getdents is not available at all.
Due to this warning I now know that SSHFS always returns DT_UNKNOWN:
gocryptfs[8129]: Getdents: convertDType: received DT_UNKNOWN, falling back to Lstat
This behavoir is confirmed at http://ahefner.livejournal.com/16875.html:
"With sshfs, I finally found that obscure case. The dtype is always set to DT_UNKNOWN [...]"
Remove the "Masterkey" field from fusefrontend.Args because it
should not be stored longer than neccessary. Instead pass the
masterkey as a separate argument to the filesystem initializers.
Then overwrite it with zeros immediately so we don't have
to wait for garbage collection.
Note that the crypto implementation still stores at least a
masterkey-derived value, so this change makes it harder, but not
impossible, to extract the encryption keys from memory.
Suggested at https://github.com/rfjakob/gocryptfs/issues/137
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
Previously we ran through the decryption steps even for an empty
ciphertext slice. The functions handle it correctly, but returning
early skips all the extra calls.
Speeds up the tar extract benchmark by about 4%.
We use two levels of buffers:
1) 4kiB+overhead for each ciphertext block
2) 128kiB+overhead for each FUSE write (32 ciphertext blocks)
This commit adds a sync.Pool for both levels.
The memory-efficiency for small writes could be improved,
as we now always use a 128kiB buffer.
This fixes a few issues I have found reviewing the code:
1) Limit the amount of data ReadLongName() will read. Previously,
you could send gocryptfs into out-of-memory by symlinking
gocryptfs.diriv to /dev/zero.
2) Handle the empty input case in unPad16() by returning an
error. Previously, it would panic with an out-of-bounds array
read. It is unclear to me if this could actually be triggered.
3) Reject empty names after base64-decoding in DecryptName().
An empty name crashes emeCipher.Decrypt().
It is unclear to me if B64.DecodeString() can actually return
a non-error empty result, but let's guard against it anyway.
