Repository navigation
Cygwin support - #499
Cygwin support#499
Conversation
rfjakob
left a comment
rfjakob
left a comment
There was a problem hiding this comment.
Nice work! However, please at least fix the shell injection
|
|
||
| int res = 0; | ||
| if ((ctx != nullptr) && ctx->lookupNode(plaintextName)) { | ||
| if ((ctx != nullptr) && (ctx->numberOfNode(plaintextName) > maxopen)) { |
There was a problem hiding this comment.
Windows does not allow deleting open files, so we can just skip the check.
In other words:
if ( !windows && (ctx != nullptr) && (ctx->numberOfNode(plaintextName) > maxopen)) {
There was a problem hiding this comment.
I'll do this yes, thus this will be even more simple 👍
Btw, one question regarding deletion.
Why do we check that the file is no more opened before deleting it ?
fuse(8) :
hard_remove
The default behavior is that if an open file is deleted, the
file is renamed to a hidden file (.fuse_hiddenXXX), and only
removed when the file is finally released. This relieves the
filesystem implementation of having to deal with this problem.
This option disables the hiding behavior, and files are removed
immediately in an unlink operation (or in a rename operation
which overwrites an existing file).
Just to avoid weird things if hard_remove is in use ?
Can't we rely on the user he knows what he does ?
There was a problem hiding this comment.
I asked myself the same thing yesterday, but this code is even older than the git history, and I am not completely sure. It may be related to how encfs shares backing file descriptors between multiple open FUSE file descriptors. I believe they are stored in a table indexed by path.
| unmount(mountPoint, MNT_FORCE); | ||
| #endif | ||
| #ifdef __CYGWIN__ | ||
| system(string("/usr/bin/pkill -f '(^|/)encfs .*/.* ").append(mountPoint).append("?( |$)'").c_str()); |
There was a problem hiding this comment.
Shell injection vulnerabilty!
There was a problem hiding this comment.
Thank you for your review 👍
I think a regex match of mountPoint would do the trick ?
Perhaps you thought about something else ?
There was a problem hiding this comment.
The best way is to avoid the shell by using something like execl
When deleting a file, Windows first opens it, deletes it, and closes it. But Windows does not allow deleting opened files, so no need to check before deletion.
When renaming a file, Windows first opens it, renames it and then closes it. We then must decrease the target openFiles count.
|
Thank you for your review & approval @rfjakob 👍 |
Hi,
This PR adds support to compile and run EncFS on Cygwin (thus closes #66).
The FUSE layer is handled by WinFsp.
Port history can be found here.
All tests pass, but symlink ones.
The symlink tests results depend one :
winsymlinksinCYGWINenvironment variable ;-o rellinksoption (see more in Symlinks to absolute path winfsp/winfsp#153).There is no real consensus, as no combination of the 2 options make all symlink tests successful, and their configuration may depend on usage / requirements.
See the wiki for installation instructions.
WinFsp may give nice performance improvements possibilities, especially thanks to
READDIR_PLUS.Some work may however be needed to use the WIN
readdirAPI.Thank you 👍
Ben