Windows support

[ccleaud]

I encountered some compilation errors with the latest "windows" branch from unreleased git-crypt v0.4. It happens when I updated my forked repo with commit 19ea278a31e58dc99da2301a08d2322fdabd0bf9 (2014-06-03).

So I've made some changes and it now compiles fine on Windows:

• commit 164e0a937ef24c3d8290edcae3825b04698c92e7:
• - got the content of your repo as of 2014-06-03 (overwritten my files)
• - repository reorganisation (no source change, juste moved to 'src' directory)
• commit 3c1a3054e96344292ef70af68dcfb7f5556eed9c:
• - corrected in order to compile using mingw32

Can you merge changes to the upstream repo please? Thanks a lot.

[AGWA]

Thanks for testing out the Windows branch and reporting this.

Unfortunately, I can't figure out what you've changed because of the repository reorganization and several unrelated changes being in the same commit. At first glance, the only substantive change I see is the unlink before the rename, though that isn't fixing a compilation error. Could you please separate out the actual fixes into distinct commits?

Also, regarding the unlink, since it's unnecessary on Unix and makes the code less robust, I want to make sure it's only executed on Windows. I also want to avoid #ifdefs in the code. So the best approach would be to declare a wrapper function in util.hpp. In util-unix.cpp, it can just call rename. In util-windows.cpp, it would unlink and then rename.

[ccleaud]

Hi,

I did commit separation but it wasn’t optimal. So I’ve separated my commits in a better way.

It should be OK now.

You’re right about unlink: this is NOT a compilation error but was an execution error on Windows OS that blocked the key migration command.

The compilation error was the lack of function “umask” in Win32 environment.

In order to avoid #ifdefs, I’ve created 2 wrappers in util-xxx.cpp and util.hpp:

•      util_umask (error when compiling with mingw32)
  

•      util_rename (error when executing on Windows OS if target file already exists)
  

I hope this will help you.

If you need help to test on Windows OS, I would be pleased to help you.

Regards,

From: Andrew Ayer [mailto:[email protected]] Sent: Thursday, June 26, 2014 7:07 PM To: AGWA/git-crypt Cc: Cyril CLEAUD Subject: Re: [git-crypt] Compilation error on Windows environment using mingw32 (#22)

Thanks for testing out the Windows branch and reporting this.

Unfortunately, I can't figure out what you've changed because of the repository reorganization and several unrelated changes being in the same commit. At first glance, the only substantive change I see is the unlink before the rename, though that isn't fixing a compilation error. Could you please separate out the actual fixes into distinct commits?

Also, regarding the unlink, since it's unnecessary on Unix and makes the code less robust, I want to make sure it's only executed on Windows. I also want to avoid #ifdefs in the code. So the best approach would be to declare a wrapper function in util.hpp. In util-unix.cpp, it can just call rename. In util-windows.cpp, it would unlink and then rename.

— Reply to this email directly or view it on GitHub https://github.com/AGWA/git-crypt/issues/22#issuecomment-47252268 . https://github.com/notifications/beacon/7109720__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcxOTQyMTYwOSwiZGF0YSI6eyJpZCI6MzU2MjE0NjZ9fQ==--1f86e97b36c238d30d7b837f98b4efcb5352e6b2.gif

[AGWA]

Hi,

Sorry for the delay in getting back to you. Thanks for the util functions. They look good and I've just pushed them to the revamp branch. (Note that I've merged the windows branch into the revamp branch and deleted the windows branch. All Windows development will now be based on the revamp branch.) Could you see if the code compiles now? Also, do you know if there's a Windows equivalent to umask I should be using? Basically, I want to make sure that any secret key file that I create can't possibly be read by any other user. Thanks!

[ccleaud]

Hi,

The Windows compilation is now OK.

I think umask has no equivalent in Win32 environment. But it is possible to create a chmod-like function for Windows.

I’ll work on it and get in touch.

[ccleaud]

Hi,

I did find a Win32 equivalent to umask but it would be too hard to implement (it would require creating a Windows security descriptor to give to each Win32 file / directory creation, forcing us to use API function instead of portable C / C++ functions).

So, I forgave this way.

I’ve coded a chmod-like function for Windows and updated code to use the function.

Also, I’ve updated documentation to make use cases clear.

You’ll find 3 commits I wish you to pull:

•     1208ba1cc1aa3a31c85d93642a1c2e302b1381ed : documentation update with new file « USAGE » that explains step-by-step how to use git-crypt
  

•     afbf7f02f79f22931ee2f49060a9ce876f53ace8: chmod-like implementation for Windows
  

•     949bcef1d768e9b761da81d1e6175be9057798f3: removed suspicious unlink when creating temp file – moreover, removing the file prevents changing its access mode (because umask is no longer used)
  

@see: https://github.com/ccleaud/git-crypt/commits/master

Thanks for merging.

[AGWA]

The problem with using chmod instead of umask is that there's a small window between creating the file and chmoding it when it has insecure permissions. This can actually be exploited in practice on Linux by using inotify to monitor when the file is created and then immediately opening it.

How about we declare a function in util.hpp with the signature void create_protected_file (const char* path). If path doesn't exist yet, it would create an empty file with restrictive permissions. After calling create_protected_file to create the file, git-crypt would open and write to the file with std::fstream as usual.

The Unix implementation would simply call open with O_CREAT and restrictive permissions for the mode argument. The Windows implementation would use the security descriptor as you described to create the file. This should work as long as writing to a file with std::fstream on Windows doesn't reset its permissions.

Let me know if you think this would be feasible. I can take care of the Unix implementation if so. Thanks for helping out with this!

[ccleaud]

You’re right about the small window when the file has insecure permissions.

Another simple way is to move up the call to “protect_file” in the function generating the key file.

Thus, the file is chmoded after its creation and before it is written (tested on Delian and Windows).

@see commit 89323bfd427c61b1bfeacf794dab589b96b2d535

What do you think?

[AGWA]

Thus, the file is chmoded after its creation and before it is written

That's not sufficient. An attacker can still get a file descriptor to the file, and use it to read the file after its contents have been written.

Bottom line, the file has to have secure permissions from the moment it's created.

[ccleaud]

I was thinking access were controlled each I/O operation and not only on descriptor acquisition (open).

In such situation, the file needs to be created with correct rights.

I’ll update my code and get in touch with you next commit

[ccleaud]

Hello,

I’ve pushed a new version that should solve the problem.

Please see commit 466bd7ddef82a7ebef04b6cd67a17c87cd30aafc

[AGWA]

Hi @ccleaud, sorry I haven't had a chance to look at your latest commit yet. I think in the interest of getting 0.4 released soon, Windows support should be considered experimental in 0.4. I'll get back to this after the 0.4 release with the aim of making Windows a fully-supported platform for 0.5.

[dzungpv]

Any progress? I am using python git crypt version. I like this because performance and security. Anyone confirm it will work fine on windows?

[miso-belica]

There is now build for Windows available at the release page starting from v0.7.0 so there is no need for custom builds and downloading potentially infected EXE files from over the GitHub 🎉

I believe this issue can be closed 🙂