Skip to content
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

Merged
merged 9 commits into from
Mar 26, 2018
Merged

Cygwin support #499

merged 9 commits into from
Mar 26, 2018

Conversation

benrubson
Copy link
Contributor

@benrubson benrubson commented Mar 25, 2018

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 :

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

sc

@benrubson benrubson requested a review from rfjakob March 25, 2018 15:58
@benrubson benrubson added this to the 1.9.5 milestone Mar 25, 2018
Copy link
Collaborator

@rfjakob rfjakob left a 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

int res = 0;
if ((ctx != nullptr) && ctx->lookupNode(plaintextName)) {
if ((ctx != nullptr) && (ctx->numberOfNode(plaintextName) > maxopen)) {
Copy link
Collaborator

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)) {

Copy link
Contributor Author

@benrubson benrubson Mar 26, 2018

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 ?

Copy link
Collaborator

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.

@@ -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());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shell injection vulnerabilty!

Copy link
Contributor Author

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 ?

Copy link
Collaborator

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

benrubson added 4 commits March 26, 2018 22:09
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.
@benrubson
Copy link
Contributor Author

Thank you for your review & approval @rfjakob 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

encfs in cygwin?
2 participants