Web lists-archives.com

Re: [Samba] Feedback request on a tentative proposal to enhance smb.conf symlink-related params




Hsnks Jeremy. I'm a bit puzzled, because from your reply this doesn't actually sound like it would be expensive or complex to do.

It then does a realpath() and checks if it is underneath
the share path= parameter, and if "wide links = no" and it
is not, refuses any further processing of the pathname.

Currently it does one chdir() + realdir() to find where the client is actually requesting, and then (it seems) a quick strcmp() or regex compare against the share path string to see if that's a parent.

If the comparison is made by strcmp(substr()) type functionality then this adds one strcmp(substr()) per valid/invalid path in the share params, which is minimal processing unless a user specifies very many paths indeed (not very likely and if they do then that's down to them). If it's done by regex then it means prestoring the regex match string and replacing a pattern like "^dir1/" by "^(dir1|dir2|..)/" (and a second regex for invalids). It doesn't lead to a large multiplicity of regex, if regex is used. But whether strcmp or regex, both seem quite minor/trivial in a computational "expense" sense, and fit into the existing approach with almost no change to the existing codebase.

Pseudocode for validating a path using the strcmp() method:

Inputs are the requested path and a presorted array containing all symlink param paths for the share (each entry contains a path + a boolean for whether it's from a valid/invalid symlinks param.) Array is presorted to put invalid entries first, as they override valid ones.

chdir(requested_path);
strRealpath = realdir();
boolStatus=false;    /* default if no match */
for (i=0; i < count(pathlist); i++) {
if (pathlist(i)('strPath') == substr(strRealpath,0,len(pathlist(i)('path')))) {
   /* we have a match. pathlist() entries can only be valid or invalid
        and invalid are presorted to the start of the list for efficiency
        when config initially read in.  So a match can only mean one of:
        <invalid>, or <valid having not matched any invalid>.
        Therefore the matched item itself gives us our answer. */
   boolStatus = pathlist(i)('valid');
      /* true if matched item came from a "valid symlinks" entry,
	          false if it came from an "invalid symlinks" entry */
   break;
 }
}    /* boolStatus now true/false indicates whether or not permissible */

Although not familiar with the codebase, from yiur description this might be all that's needed to validate on this schema. It doesn't seem to need more than this code snippet, and doesn't look likely to be expensive.

It is helpful to be able to specify a list of specific dirs that can/can't be followed to or links created to, and I'm not seeing expense involved. Are you sure it needs much more than this sort of thing?

Stilez




On 1 March 2018 7:15:39 pm Jeremy Allison <jra@xxxxxxxxx> wrote:

On Thu, Mar 01, 2018 at 10:48:02AM +0000, Stilez Stilezy wrote:
As mentioned in another thread, I notice that the params used to control
symlinks feel a bit inefficient and inelegant, and quite limited.  I think
there might be a good opportunity to simplify and also make their management in
Samba more powerful and adaptable to use-cases.  I'm guessing this list is a
good enough starting point to propose a smb.conf param change to this area and
see what reactions it gets.
 
 
The issue:
 
Currently symlink handling uses 3 parameters:
 
 - "follow symlinks" (are they followed at all?)
 - "wide links" (if so, can they be traversed to a location that isn't a child
of the share root dir? Effectively "read/traverse but not write" restriction)
 - "allow insecure links" (effectively "non-write restriction" removed, user
can write wide links to arbitrary dirs. as well as follow them)
 
Limitations/opacity caused by the current options:
 
1. These three params have slightly opaque names ("wide links" doesn't say
*what* about wide links is/isn't altered by the option), and only certain
combinations make sense. Implication:  smb.conf could combine them to good
effect, simplifying the config, and also being clearer about how they relate to
each other.
 
2. There is no granularity about where symlinks can link to. The only
granularity within Samba is to restrict read/traversal/create to either * a
descendant of /dir/share, or * a descendant of / (=universal access). There's
no other restriction setup possible within smb.conf. An implication is that
it's impossible to include a directive that restricts symlink use+creation to
specified locations outside the share root, other than by allowing them to have
scope over the entire of /. For example, a if the file system separates user
home and "generally available files", or has a virtual file system made up of
dynamically updated symlinks, the user can't create symlinks unless these are
all under the same share root, in which case the file system is forced to allow
symlinks to dirs. that might not be desirable. Alternatively if they aren't
under a common share root the user can't maintain symlinks as the only
directive available gives permission to create links to anywhere. It's very
crude.
 
 
Tentative ideas for discussion:
 
Replace the current symlink handling directives by directives modelled on valid
/invalid hosts+users:

    valid symlink targets = follow:<mask>, <mask>, <mask> ... [create:] <mask>,
    <mask>, <mask>, ...
    invalid symlink targets = follow:<mask>, <mask>, <mask> ... [create:]
    <mask>, <mask>, <mask>, ...
     
    <mask> is built as usual from % substitutions,
    default for each share is   valid symlink targets = follow:%P, create:%P

This would be too expensive to implement in the server
and is too much complexity for very little gain.

The reason is as follows. The server doesn't actually
read symlinks component by component - what it does is
find the last component in a client requested path, and
chdir() into that.

It then does a realpath() and checks if it is underneath
the share path= parameter, and if "wide links = no" and it
is not, refuses any further processing of the pathname.



--
To unsubscribe from this list go to the following URL and read the
instructions:  https://lists.samba.org/mailman/options/samba