-
Notifications
You must be signed in to change notification settings - Fork 286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cygwin support #499
Cygwin support #499
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! However, please at least fix the shell injection
encfs/DirNode.cpp
Outdated
int res = 0; | ||
if ((ctx != nullptr) && ctx->lookupNode(plaintextName)) { | ||
if ((ctx != nullptr) && (ctx->numberOfNode(plaintextName) > maxopen)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
encfs/FileUtils.cpp
Outdated
@@ -1739,6 +1739,9 @@ void unmountFS(const char *mountPoint) { | |||
// fuse_unmount does not work on Mac OS, see #428 | |||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shell injection vulnerabilty!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best way is to avoid the shell by using something like execl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
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 :
winsymlinks
inCYGWIN
environment variable ;-o rellinks
option (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
readdir
API.Thank you 👍
Ben