Compatibility improvement to reparse point handling.

Discussion:

Joe Lowe

2017-04-27 20:39:24 UTC

Greetings,

Attached is a proposed patch that improves Cygwin reparse point
handling, to be compatible with reparse points whose SubstituteName does
not begin with one of the 3 prefixes specifically recognized in the
current code.

With the patch, Cygwin validates that the SubstituteName has one of the
two prefixes that Cygwin can correctly convert to a working readlink()
result, otherwise the reparse point is treated as a normal file or
folder. The new behavior is already similar to how Cygwin handles
unknown reparse point types and volume mount points, so the required
code changes were minimal and well isolated.

Example of current Cygwin behavior:

A junction with this SubstituteName value
"\Device\HarddiskVolume4\Users\joe\Documents" works in windows
applications to open my documents folder, but in Cygwin applications it
does not work. Cygwin treats the junction as a posix symlink with a
target of "/cygdrive/c/Device/HarddiskVolume4/Users/joe/Documents",
which does not exist.

Testing:

Testing the change against the currently supported types of reparse
points is straightforward. I did basic tests with symlink to UNC, mount
point, and a relative symlink, and behavior was unchanged from current code.

I also tested against currently broken cases using Pismo File Mount
Audit Package and by using a custom tool to create arbitrary reparse
points. Other testing options include EncFSMP and VirtualSynth. I can
provide additional info for testing to whoever is interested.

Background:

There are reasons that mount points or symlinks to 3rd party file
systems may not have one of the three prefixes that Cygwin currently
requires:

1) The UNC prefix causes the reparse target to go through the MUP
(Multi-UNC-Provider) arbitration process. This process involves network
IO and timeouts that for 3rd party file systems are likely unnecessary.
This negatively effects user experience and it leaks local system
activity onto the network. Also, the UNC prefix is not appropriate for
local volumes.

2) Drive letters are limited. Junctions and symlinks to volumes without
drive letters cannot have the drive letter prefix.

3) The volume GUID prefix implies a mount point for a local volume
managed by the windows plug-and-play system and volume manager. Some
applications will interpret volumes with mount points in this format as
having certain attributes or features. At a minimum, the volume GUID
prefix is not appropriate for reparse points targeting network volumes,
and is not handled by Cygwin for symbolic link type reparse points.

License:

The patch is entirely my own work. I do not consider the patch
substantial enough for copyright to apply, but if licensing is
considered necessary then the patch is licensed as follows.

2-clause BSD:
=========================================================================
Copyright (c) 2017, Joe Lowe
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
met:

1. Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.

2. Redistributions in binary form must reproduce the above copyright
notice, this list of conditions and the following disclaimer in the
documentation and/or other materials provided with the distribution.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
NEGLIGENCE OR OTHERWISE ARISING IN ANY WAY OUT OF THE USE OF THIS
SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
=========================================================================

Feedback appreciated.

Joe L.

Corinna Vinschen

2017-04-28 10:21:22 UTC

Permalink

Hi Joe,

firt thanks for the patch. In theory patches should go to the
cygwin-patches mailing list but cygwin-developers is ok for now.

Post by Joe Lowe
A junction with this SubstituteName value
"\Device\HarddiskVolume4\Users\joe\Documents" works in windows applications
to open my documents folder, but in Cygwin applications it does not work.
Cygwin treats the junction as a posix symlink with a target of
"/cygdrive/c/Device/HarddiskVolume4/Users/joe/Documents", which does not
exist.

A "junction"? I'd just like to make sure that we're talking about the
same reparse point type. A junction is handled by an
IO_REPARSE_TAG_MOUNT_POINT.

Post by Joe Lowe
1) The UNC prefix causes the reparse target to go through the MUP
(Multi-UNC-Provider) arbitration process. This process involves network IO
and timeouts that for 3rd party file systems are likely unnecessary. This
negatively effects user experience and it leaks local system activity onto
the network. Also, the UNC prefix is not appropriate for local volumes.
2) Drive letters are limited. Junctions and symlinks to volumes without
drive letters cannot have the drive letter prefix.
3) The volume GUID prefix implies a mount point for a local volume managed
by the windows plug-and-play system and volume manager. Some applications
will interpret volumes with mount points in this format as having certain
attributes or features. At a minimum, the volume GUID prefix is not
appropriate for reparse points targeting network volumes, and is not handled
by Cygwin for symbolic link type reparse points.

As far as IO_REPARSE_TAG_MOUNT_POINT types are concerned the volume GUID
prefix is already handled, line 2321 in git HEAD.

Post by Joe Lowe
The patch is entirely my own work. I do not consider the patch substantial
enough for copyright to apply, but if licensing is considered necessary then
the patch is licensed as follows.

Thank you. Your patch is substantial and requires the 2-clause BSD waiver
per https://cygwin.com/contrib.html. See also
https://cygwin.com/git/?p=newlib-cygwin.git;f=winsup/CONTRIBUTORS;hb=HEAD

Post by Joe Lowe
Feedback appreciated.

Your patch is basically ok, I have three points, though:

- The is_absolute_reparse_target_posix_symlink_compatible function
doesn't only have a somewhat too long name, it's also too complicated
for my taste. We already handle path prefix in the code and so far
they are handled differently most of the time. For instance, see
the aforementioned line 2321 in git:
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/path.cc;h=7d1d23d72cf1d9833683e139c06e4c282363dc1c;hb=HEAD#l2321

We also have matching constants wchar_t strings for other comparison
operations, which I'd prefer to use in this code:
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/globals.cc;h=5c5d64ee44b762e6655153b7d4b651591b65c1ba;hb=HEAD#l119

The "UNC" string is always upper case.

- As far as \Device\HarddiskVolumeX paths are concerned, we might want
to handle them actually as valid symlinks. We could most easily
accomplish that by prepending "/proc/sys/" to the string when
converting them to POSIX paths.

That's not quite as easy for IO_REPARSE_TAG_MOUNT_POINT paths starting
with the GUID volume given the way the NT namespace is mapped into
/proc/sys. It depends on the fact whether the volume has been mounted
globally or if it's a user mount point. For a start it would actually
be easier to handle them just as files or dirs, but I'd appreciate
ideas as to how we could handling them as symlinks.

- Last but not least, your timing is a bit bad. I'll be offline for all
of May, starting tomorrow. We can discuss this further in June.

Thanks,
Corinna

--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat

Joe Lowe

2017-04-28 17:18:35 UTC

Permalink

Post by Corinna Vinschen
Hi Joe,
firt thanks for the patch. In theory patches should go to the
cygwin-patches mailing list but cygwin-developers is ok for now.

OK. Got it.

Post by Corinna Vinschen

A "junction"? I'd just like to make sure that we're talking about the
same reparse point type. A junction is handled by an
IO_REPARSE_TAG_MOUNT_POINT.

Agreed, "directory junction" as created with the "cmd /c mklink /j ..."
. This ends up on disk as a reparse point with tag
IO_REPARSE_TAG_MOUNT_POINT, same tag as a volume mount point.

Post by Corinna Vinschen

As far as IO_REPARSE_TAG_MOUNT_POINT types are concerned the volume GUID
prefix is already handled, line 2321 in git HEAD.

Thank you. Your patch is substantial and requires the 2-clause BSD waiver
per https://cygwin.com/contrib.html. See also
https://cygwin.com/git/?p=newlib-cygwin.git;f=winsup/CONTRIBUTORS;hb=HEAD

Post by Joe Lowe
Feedback appreciated.

- The is_absolute_reparse_target_posix_symlink_compatible function
doesn't only have a somewhat too long name, it's also too complicated
for my taste. We already handle path prefix in the code and so far
they are handled differently most of the time. For instance, see
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/path.cc;h=7d1d23d72cf1d9833683e139c06e4c282363dc1c;hb=HEAD#l2321
We also have matching constants wchar_t strings for other comparison
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/globals.cc;h=5c5d64ee44b762e6655153b7d4b651591b65c1ba;hb=HEAD#l119
The "UNC" string is always upper case.

I have no problem with any such changes. Feel free to modify the patch
as desired, or I can make changes. I avoided string compares mainly
because of the style of the logic in posixify(), and uncertainty about
performance concerns. The drive letter prefix check will still take more
that just a string compare.

Reparse point targets can point anywhere in the native NT object
namespace, so there is an unbounded set of potential target prefixes
that will not convert to posix symlinks. If the logic is changed to use
string constants, it still needs to compare against a complete list of
known convertible prefixes (currently two), as opposed to an unavoidably
incomplete list of known non-convertible prefixes.

Post by Corinna Vinschen
- As far as \Device\HarddiskVolumeX paths are concerned, we might want
to handle them actually as valid symlinks. We could most easily
accomplish that by prepending "/proc/sys/" to the string when
converting them to POSIX paths.
That's not quite as easy for IO_REPARSE_TAG_MOUNT_POINT paths starting
with the GUID volume given the way the NT namespace is mapped into
/proc/sys. It depends on the fact whether the volume has been mounted
globally or if it's a user mount point. For a start it would actually
be easier to handle them just as files or dirs, but I'd appreciate
ideas as to how we could handling them as symlinks.

IMO: I think the current Cygwin logic is good in this regard. Any
symlink created by a user with Cygwin tools or with native "cmd /c
mklink" works today as a posix symlink to Cygwin applications.

The ability for a technical user or 3rd party developer to create a
reparse point that does not get treated as a posix symlink in Cygwin
applications, is a useful feature. This would be equivalent to a linux
user choosing to use a "bind" mount point instead of a symlink.

Post by Corinna Vinschen
- Last but not least, your timing is a bit bad. I'll be offline for all
of May, starting tomorrow. We can discuss this further in June.

Timing is not an issue. Please ping me when you are back if you want me
to modify the patch. Enjoy your time away.

Post by Corinna Vinschen
Thanks,
Corinna