Security hole if directory junctions are synchronized by Syncthing the same way as symlinks, on Windows:
Consider a computer A of an attacker and a computer B, to which a folder is synchronized from A by Syncthing. The target directory on B is exported as a shared folder, e.g. in order the attacker can check whether everything has been synchronized correctly.
Now, the attacker creates a directory junction and a symlink, both pointing to C:\Users. If Syncthing synchronizes both to B as directory junction/symlink, the directory junction can be used by the attacker to access the content of C:\Users, to which he has otherwise no remote access. The symlink causes no security problems.
This vulnerability is caused by the fact, that directory junctions are processed at the server side. When the attacker uses e.g. the File Explorer to access the shared folder on B, the File Explorer allows him to traverse the directory junction. On the other hand, the File Explorer (and any other means) refuses to traverse the symlink. Not only that, Syncthing probably won’t be able to write the symlink, because this requires elevated administrator account.
Hence, there are only two possibilities for Syncthing how to treat directory junctions: either ignore them (which is the current behaviour), or convert them to directories.
Just a little status update from my side. I’d continue my experiments with the code regarding symlinks and after I’ve found a way to discard incoming symlinks on linux when we are in followSymlinks mode, I had tried to get this working for symlink’ed files too.
This is very hard to figure out, so you all were right with your posts about security concerns. For example, I got it working to follow symlinked files and dirs with 2 linux machines. Machine A created a symlink to /etc/debian_version and machine B received the file correctly. Then I changed the contents on A and they were synced over to B. When I changed the regular file on B, A refused to finish the file in the puller. After I figured out how to do that, I got my contents synced over to A. But a syncthing-conflict file was also created being a symlink itself. Experimenting further with the goal A writes the contents from the regular file of B back to the symlink target, I hit the panic “error tried to version a symlink”. Uffff.
It’s very hard to get it all clean so it “just works for testing” as a starting point to know which code parts would be affected. Not even speaking of how FAR it would be to get this stable. At least, I’m having fun learning go and understanding Syncthing :-).
I have similar issue to @xarx with additional dimension - to synchronize several Win laptops to NAS. Again - each one of them has several places/dirs to sync (not necessarily the same - some for sharing, some for backup) so my first thought was to create one catalog per laptop (C:\sync) with symlinks/junctions to all the places which need to be synchronized.
I really love syncting, having used it successfully to synchronize/backup my phone (but that’s just pictures and messages - three dirs). Now, the idea of having 200+ dirs to arrange (instead of 8 – one per laptop) is really nightmare.
I am all for option for advanced users for some symlinks and/or junction handling on Windows.
In the meantime, @Catfriend1 can you please share your binary with symlinks enabled? I would gladly test drive it, but I am not quite comfortable with compiling go myself.
@Catfriend1 can you please share your binary with symlinks enabled? I would gladly test drive it, but I am not quite comfortable with compiling go myself.
I could, but please be aware it could have negative consequences in theory. I’m using the modification since end of 2016 in our company and never lost data (backups also in place!). It is advised to be very careful with the symlink mod. It’s mostly safe to run under Windows with making folders containing symlinks sendOnly. SendReceive works too, but other members may not delete folders that are symlinks on the sendreceive “host”.
The binary also proved protocol compatible to the official syncthing. It just announces directory symlinks as directories which official syncthing picks up on remotes.
Careful: do NOT create symlinks to files. I did not test this.
Then I’m setting up a new folder in Syncthing at “/view/stfolderroot” and change “markerName” in the advanced options panel to “Mount-binded-dir-pointing-elsewhere-1/.stsubfolder”.
If the mount-bind target goes unavailable (“dangling”), Syncthing will stop the folder instead of picking up deleted content to its database.
This approach assumes the whole “/view/stfolderroot” contents are pointing to content on the same target disk, of course. If you have different disks, you should setup a virtual directory tree view for each disk to ensure it is properly stopped in case the disk or disk mount point goes unavailable. (due to ISCSI, fsck or sth else)
I’ve tested that I can distinguish directory junctions from ordinary symlinks in Golang (on Windows). Using syscall to a kernel32.dll function.
Does it suffice like this?:
Intent: Treat directory junctions on Windows as directories
There are two types of symbolic links in Windows NTFS - ordinary symlinks, and directory junctions. (They both are NTFS reparse points; other reparse point types are not considered as symbolic links).
As discussed above, there is a substantial distinction between processing of symlinks and directory junctions - in particular, NTFS symlinks are similar to Linux symlinks, while directory junctions are something in between
symlinks and Linux directory mounts.
From the Syncthing perspective, the directory junctions can be safely treated as directories, in contrast symlinks can’t (this could cause security issues).
In Windows, directory junctions are used to mount directories from other parts of the filesystem. When there is a need to access a directory transparently (i.e. without knowing there’s a redirection)
from within another directory, directory junction is the right way to do that. Windows itself uses directory junctions on tens of locations exactly for this purpose, while symlink is used only at one particular place.
Syncthing currently fails to traverse directory junctions ignoring this important Windows feature. Unfortunately, there’s no workaround for this in Windows (as opposed to Linux), which causes an almost insurmountable problem
when one needs to backup many (tens) directories that are scattered on many various locations in the filesystem.
This is a proposal for implementing the directory junctions traversal in Syncthing:
Make traversal of directory junctions optional:
Historically, the usage of directory junctions evolved, and even now not everyone may want the directory junctions to be traversed. Hence, I propose a switch in Advanced configuration that would enable or disable directory
junctions traversal. The switch would be per folder, not global, enabling different folders to have a different setting.
When directory junctions traversal is enabled,
directory junctions will be treated as ordinary directories, and Syncthing will synchronize them to other devices like ordinary directories. Upon write, Syncthing won’t create directory junctions, but if they are already present,
it shall traverse them.
When directory junctions traversal is disabled,
directory junctions will be synchronized as empty directories. (Another possibility would be to ignore them, but definitely NOT sync them as symlinks. I prefer treating them as empty directories.)
Hence, when the switch allowing directory junctions traversal is turned off, the content of the directory will be synchronized as deleted. Upon write, directory junctions will fail to be traversed. (There’s no point in allowing to traversal while writing if the traversal is prohibited while reading. I haven’t checked (yet) how Syncthing currently resolves the situation when at target there’s a directory junction (or symlink), while at source there’s an ordinary directory - does the sync fail?)
Special case - invalid directory junction
This can happen when the directory junction point to a removable disk (symlink should be used instead) or if the target directory was removed/renamed. In every case, this is an error condition, the directory junction is corrupted.
In this case, either the directory junction should be treated as empty, or as unchanged. I prefer the second option, as it is fail safe, though I do not know whether this is possible in Syncthing. Of course, syncronized changes
from other devices shall fail to be written to corrupted directory junctions.
Special case - infinite recursion
Directory junctions can point to an ancestor directory, and cause infinite recursion this way. Such a situation can be either handled by Syncthing, or ignored (the user will be responsible not to allow that). Of course,
it would be better to handle such a situation by Syncthing. This would be relatively easy to implement, but it would require to pass on a context object upon a directory walk/traversal. I haven’t checked whether a suitable context
object is awailable in Syncthing code, or whether it would have to be introduced.
Sounds workable. I mean, the fact that recursive junctions are allowed seems bizarre, but can be handled by tracking which junctions we’ve traversed and bailing if we come back to one we’ve seen before. There’ll be some corner cases to handle there as we sometimes do partial scans deep in a folder (not having traversed there to begin with) but there is a symlink check so I think it could be implemented there. We might also get stuff like filesystem notifications from in there, probably. Or if those present as the non-junctioned name then we need to handle that too. Oh, and if we use notifications and the junction gets removed we probably need to handle that somehow. Junctioning folders into each other might have some special cases as well, I haven’t fully thought that through.
What are the current uses of junctions in Windows where Syncthing might run into them? I mean, by default, not because of wanting to collect multiple directories into a single shared folder.
If that’s the only real use case, it might be an alternative to allow multiple folder roots in a folder, “mounted” at different places. There’s be trickiness there too, to be sure, but it might be more generally useful.
I think this should end up as a flag, which just treats junctions as directories, nothing else.
If there is recursion, let the application fill it’s database with trash and blow up in the users face. The user must be wearing big boy pants when deciding to enable it, hence should have checked that there are no cycles.
Junction pointing at something invalid handling will depend on what the error is when you try to “stat” it. If it returns “does not exist”, then we’ll go ahead and nuke all the data behind it, if it returns something else we’ll just assume it’s a permission error and leave it alone. Again, no special handling needed I think, and we should leave the existing logic to handle.
I think notifications will simply not work for this, but again, it’s a big boy feature, let the big boy handle it. I don’t think we’ll modify the watcher to crawl folders to check for junctions to setup recursive watches in the resolved locations.
Just a digression, but Dropbox in the past used to support junctions, and their notifications did not work for them either. The junctions would only get scanned and synced when Dropbox was being started (as there is no concept of periodic scans in it, at least as far as I know).
In comparison, Syncthing has more robust scanning mechanisms, so I believe that this should not be a big deal.
Ain’t nobody so big of a boy that they won’t come here to complain when it doesn’t work though. This is the thing with any new feature. It’s not just stirring together some code, it’s maintaining and supporting it in eternity.
I am not aware of junctions having come up in the context of syncthing, except for what @xarx describes. And their use-case would be covered by the following, which is useful everywhere and doesn’t need dealing with junctions at all. Thus I would greatly prefer that.
Plus is anyone considering working on this and maintaining it after? I can see myself working on a multi-root folder, that might be useful for myself (this does not mean that I will do it). I certainly won’t do any special casing with safety-nets and so on for junctions. And I would probably not be in favor of a “big-boy” barebones version that can blow up in the users face, because users don’t respect dangerous settings and thus we get to support these blowups afterwards.
Some folders had a different name in previous versions of Windows. Then they were renamed, but the old name is still available - as a junction to the new folder. (E.g. “C:\Documents and Settings” -> “C:\Users”, “C:\Users\Default user” -> “C:\Users\Default”,…).
Another use is as various shortcuts to folders embedded deeper in the directory tree. (E.g. “\Application Data” -> “\AppData\Roaming” or “\AppData\Local\History” -> “\AppData\Local\Microsoft\Windows\History”.)
Another use is as a convenient redirection to a folder located elsewhere. (E.g. “\PrintHood” -> “\AppData\Roaming\Microsoft\Windows\Printer Shortcuts”.)
I personally have relocated various folders in user profile that contain “valuable user data” (e.g. “Documents”, “Music”, “Videos” etc.) from the system drive (C:) to a data drive, and replaced the folders at the original location with directory junctions.
I have also moved e.g. temporary folders from the system drive to another drive, so that the whole system partition can be backed up without cluttering the backup with unnecessary data. Or I used to move folders from one disk to another because of insufficient space.
These all are various usages of directory junctions. Deliberately, I have omitted the use of directory junctions to collect multiple directories into a single shared folder.
So, what next? I can implement the feature, though it’ll take me some time. I’ll have to make the build process working, I’ll be fighting with Golang that I do not know, and I do not have much time. But I do want the feature, so I’ll make sure it’s implemented in a near future. Then I’ll send a pull request. Or do you want to implement that yourself?
Do you have any expectations concerning the notifications you were discussing just now? I don’t know how they work, so you would have to advice me.
Dropbox will follow Windows junction points and sync the files or folders they link to. However, any changes to those files or folders made from the Windows operating system will not sync again until the Dropbox desktop app is restarted. To get around this, move the original folder to your Dropbox and add a junction point from its previous location to link to its new location in the Dropbox folder.
They recommend inverting the relationship instead…
Are there junctions to files as well as directories, or are they just being unnecessarily general?
I have the directory junctions already set up on the disk, because of other reasons. I now only need Syncthing to support them. Multiple roots would work too, they might have a more general usage, but they would require to set up (and maintain) all folders again, just for Syncthing. Hence I prefer implementing the directory junctions transparency.
IMO, implementation of multiple roots would require much more work. And when it will be done? Is it in YOUR road plan?
Directory junctions are just for directories, not for files.
I’d say that this is a rather simple change, and it can be written easily maintainable.
I suspect that this isn’t a simple change, and that it will carry a significant maintenance and support cost just by it’s nature, as an oddity with relatively few users and with corner cases we’ve already established will cause disaster (recursive junctions) or non-working features (notifications). You can prove me wrong on the implementation part. Even then, a simple but half assed feature might be something we regret years later. (Compare for example “ignore deletes” which I implemented, was probably ten lines of code, we still have a support issues on it every week years later…)
The alternative (adding multiple folder roots within the same folder) might have none of these drawbacks and enable more interesting usages that we haven’t considered yet, while still solving your use case. That you might have to take a one time cost to reconfigure a folder to use the new feature doesn’t really enter into the calculations.
Yes, but then they become just regular folders from the Dropbox’s point of view. One can do the same with Syncthing too. The real difference is that Dropbox does follow junctions located in its folder, while Syncthing does not.
I personally do not really need or care for this feature anyway, but just wanted to add some information from my long experience with Dropbox. I tried to experiment with junctions back then, but the lack of change detection (with no periodic scanner) ended up being a dealbreaker for me.
@Catfriend1 has already implemented that, mostly, though he did that for symlinks in general, not for directory junctions. I saw his changes, there were not many modifications. At the places where he tested for symlinks, I would call a general function “IsTraversable” instead. Implementation of this function would be encapsulated elsewhere, it’s not that difficult. The only difficulty would be only in tracking the traversed links in order to prevent infinite recursion. IMO nothing difficult and complicated.
If notifications wouldn’t work, would Syncthing stop synchronizing?
As I understood in previous discussions, advanced features often lack some functionality, so this drawback seems acceptable to me.
Thinking about that again, this is not a full-fledged replacement for directory junctions. What if you want to sync a directory that contains directory junctions inside. How would you glue up the the complete directory from individual synchronized pieces? This would help only in the simplest scenarios, like “collecting multiple directories into a single shared folder”.
Catfriend1 did something, but I don’t think it’s the right approach. If I were going to do this, I’d run with the premise that junctions aren’t symlinks and implement a hack in the fs layer. There, you can lie and say that a given junction is not in fact a junction, it’s just a normal directory. Things would then probably mostly work without butchering the symlink checks.
What happens with the notifications is another thing. Does it successfully set up notifications and everything looks fine but they just don’t work? That’s probably not acceptable. Do we throw an error and say that you can’t use notifications because there are junctions? Might be OK.
You are the ones that have to say how the feature is to be implemented. But I do think that implementing the feature by hacking the fs layer and lying there would cause many troubles in the future. In maintenance, in particular. Because that would obfuscate the purpose of the changes, and prevent developers from using the standard Golang packages, as IsDir there would return a different result than IsDir in your package (if there’s such a function in your package - this was just an example). Moreover, you still would have to make the modifications related to prevention of infinite recursion - and for that you would need to know whether the current fs object is a dir or symbolic link.
Sure, IsTraversable should be put into the fs or related package. If you want to hide the junctions from the main code, you would have to replace all uses to IsDir with IsTraversable, where appropriate. Without that you would make the code maintenance difficult.
I myself would put everywhere explicitly “if IsDir() || (IsLink() && IsTraversable()) then…”. Such an implementation would be simple, then. But again, you are the ones who says how this is to be implemented.
