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

Developer mode for symbolic links #308

Merged

Conversation

schinagl
Copy link
Contributor

@schinagl schinagl commented Mar 4, 2022

Problem

Winfile fails on copying Symbolic Links in developer mode

How to test

Download ln.exe from https://schinagl.priv.at/nt/ln/ln64.zip and place it in the same directory as the below .bat file.
Start TestCopyReparsePoint.bat from an administrative command prompt in your e.g. temp directory. Make sure you are in developer mode and do not have SE_CREATE_SYMBOLICLINK assigned to the active user.

It builds a structure like

├───1
└───sl
    ├───iLib
    │   ├───inc
    │   └───src
    ├───iLib_symlink
    │   ├───inc
    │   └───src
    └───zzz

Navigate to x:\sl in Winfile and copy it e.g. via Drag and CTRL to x:\1. You will receive no errors opposite to the original version, which fails on symbolic link creation of x:\sl\iLib_symlink.

Comments on the code

WFCopy() and MKDir() check the return code of its respective operation and if it was ERROR_PRIVILEGE_NOT_HELD, it creates the symbolic link itself.
This has the drawback, that alternative data streams and EA records are not copied.

  • For EA records I would definitley put this under known limitation, because they are legacy from OS/2
  • Alternative streams on symbolic links is deficit, yes.

ACLs are not copied, but this also applies for the CreateDirectoryEx()/CopyFile() case, so ACLs are fine, I think.
Junctions work anyhow fine.

Making this whole EA record and alternative streams copy work is quite some work, I already did it for LinkShellExentsion, so I could contribute the coding, but it is a few hundred lines of code. This then also handles weird cases like sparse alternative streams, EA records, compressed and encrypted files, but I guess we are overshooting here.

I much more hope, that a future version of CreateDirectoryEx() and CopyFileEx() in e.g. Win11 support developer mode, so that this problem is then solved.

In general this implements the simplest of all Reparse Point copy strategies, which I called the 'Splice' method. See also the explanation of Splice in the LinkShellExtension Docu.

Winfile does the 'Splice' pattern also for inner and outer reparse points. So this will only work for absolute symbolic links. Relative symbolic links will be copied but point to 'something'. This can not be solved with this PR for Winfile, because it needs more sophisticated symlink resolving.

Maybe in a later PR we could adress relative symbolic links.

In the course of action DecodeReparsepoint() was adapted a little, because the second argument was never used, and in my opinion one should pass a path to a reparse point and not path\*.* as the first argument

So ... lets see what you guys think.

@schinagl schinagl force-pushed the symbolic_link_with_developer_mode branch from f78886a to d94bc2f Compare March 14, 2022 18:55
MKDir creates the symbolic links itself if in developer mode
Adapted DecodeReparsePoint a little
* to suit it for \??\ prefix
* remove unused second parameter
* for client coding

Symbolic Link directories are copied in developer mode properly

Symbolic Link files are copied in developer mode properly
@schinagl
Copy link
Contributor Author

Maybe my description above was too long or confusing, but basically this is a bug fix:
Currently one can not copy a tree with symbolic links without error boxes, when in developer mode.

So maybe @malxau you can give it a look/try? It would bring us forward... :-)

@craigwims
Copy link
Contributor

@schinagl, I must be dense. What do you mean by "developer mode"?

@schinagl
Copy link
Contributor Author

schinagl commented Apr 4, 2022

Problem

The privilege to create Symbolic Links (SeCreateSymbolicLink) is normally only held by administrators.
But 'normal' users also want to create symbolic links, and do not want to enter the admin password each time via the UAC dialog.

So there are two ways out of this

a) Enable 'developer mode': https://www.howtogeek.com/292914/what-is-developer-mode-in-windows-10/
b) Grant the privilege to create symbolic links in the policy editor to a specific user: https://schinagl.priv.at/nt/hardlinkshellext/hardlinkshellext.html#changesymboliclinkprivilege

With a) or b) enabled a normal user can create Symbolic Links

What is the point of this fix

  1. We use CopyFileEx instead of CopyFile, and ask it to copy Symbolic links if possible. ( .... COPY_FILE_COPY_SYMLINK) ( Files are copied normally anyhow ... ) [CopyFileEx instead of CopyFile, because in the good case (== SeCreateSymbolicLink held) anyhow it copies the symlink and we are done]
  2. In case of failure CopyFileEx will return with ERROR_PRIVILEGE_NOT_HELD, which means we are not allowed to create symbolic links. ( See above for the ways out )
  3. Then we give it another try and try to create the Symbolic Link ourselves with SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE flag set to CreateSymbolicLink().
  4. Assuming the user had 'developer mode' on, we are now able to create a symbolic link and saved the day:-)

Basically we do similar things for CreateDirectoryEx() and also here provide a fallback to create symbolic links if 'developer mode' is on

@craigwims
Copy link
Contributor

thanks! Which version of Windows introduced CopyFileEx?

@schinagl
Copy link
Contributor Author

schinagl commented Apr 4, 2022

Windows XP/Windows Server 2003

@schinagl
Copy link
Contributor Author

schinagl commented Apr 6, 2022

Added Review fiindings

@craigwims
Copy link
Contributor

When I run the test bat file and follow the ctrl+drag instructions above, the copied symlink 'iLib_symlink' icon starts out plain in the left pane of the window"

image

When I refresh by expand/collapse 'sl', the icon is correct.

Separately, how do I test the case of WFCopyIfSymlink in lfn.c? The above case doesn't trigger it.

Another case I don't understand: if I create a directory '2' next to '1' and ctrl+drag 'iLib_symlink' to '2', I get a "Confirm File Replace" dialog box. Why is that?

@schinagl
Copy link
Contributor Author

schinagl commented Apr 6, 2022

A few good questions, lets start with

How do I test the case of WFCopyIfSymlink in lfn.c? ✔️

  1. Unfortunately my published testcase lacked a symbolic link file. Sorry. I have updated my testcase so that it also contains a symbolic link file

  2. The only way to test this is to

  • enable developer mode as admin
  • create a non admin user
  • log on as non-admin
  • put a breakpoint to WFCopy:548

and copy the given tree from sl/ to 1/ as described above

Then it looks like this
image

It is important, that your Winfile, does not hold SeCreateSymbolicLinkPrivilege. (Use procexp.exe!) You must not see it in the list of privileges. If you see it and it is 'Disabled' then you hold the privilege and it can not be reproduced.

When I refresh by expand/collapse 'sl', the icon is correct. ✔️

Can reproduce.
This was definitely introduced with #309 via treectl:1695 and treectl:1700. But this only means that #297 does not feed the attributes ATTR_SYMLINK/ATTR_JUNCTION in any situation.
So this has nothing to do with this PR in general, but anyhow will look into this.
Have merged master into the branch
@malxau: Since you did #297, do you have an idea?

It is a refresh problem. Attributes are fed correctly in WFFind(First|Next)

If I create a directory '2' next to '1' and ctrl+drag 'iLib_symlink' to '2 ✔️

Can reproduce.

I know the reason, and basically this PR is only the messenger, but this problem was always there.
For the interested:
WFMoveCopyDriverThread() was changed with #303 so that if symlinks are on the second level (beneath a selection) of a tree, deletion and copy of reparse points works fine. So far so good.
But this was not enough, because WFMoveCopyDriverThread() can not handle situations, where a reparse point is on level 1, which means is selected. Then it tries to follow it, which it must not, because it wants to copy the content behind a reparse point onto itself, which causes the error message, which you reported as 'confirmation'

Anyhow, I hope I can find a solution for this.

In general sorry for the inconvenience. Will do by best to get it fixed

@schinagl
Copy link
Contributor Author

schinagl commented Apr 8, 2022

Fixed 'If I create a directory '2' next to '1' and ctrl+drag 'iLib_symlink' to '2'

@schinagl
Copy link
Contributor Author

schinagl commented Apr 8, 2022

Fixed 'When I refresh by expand/collapse 'sl', the icon is correct.'

@craigwims
Copy link
Contributor

The issues are fixed; thanks!

I do have one concern about a code change; see the new comment.

@craigwims craigwims merged commit 5f517be into microsoft:master Apr 11, 2022
schinagl pushed a commit to schinagl/winfile that referenced this pull request Apr 11, 2022
@schinagl schinagl deleted the symbolic_link_with_developer_mode branch April 11, 2022 18:25
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.

2 participants