samba - Mar 2018 - Feedback request on a tentative proposal to enhance smb.conf symlink-related params

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*

The syntax in each param is a keyword (follow: | create:) which identifies
if the masks after it control *following* a symlink to the location, or
*creating* a symlink to the location. The type must be explicit (no
implicit "follow"). After each "(follow: | create:)", any
masks listed are
deemed to be of that type. An empty list is valid and means "none".

   - A symlink to /dir/foo/bar will be *followed *if any of /, /dir,
   /dir/foo, or /dir/foo/bar are given as "follow" masks in a
"valid symlink
   targets" (*provided that* none of these is also listed within an
   "invalid symlink targets" follow clause).
   - A symlink to /dir/foo/bar can be *created* by a user if any of /,
   /dir, /dir/foo, or /dir/foo/bar are given as "create" masks in a
"valid
   symlink targets" (*provided that* none of these is also listed within an
   "invalid symlink targets" create clause).


*Examples (the share root in each case is /dir/foo/share):*


*DEFAULT (follow:%P, create:%P)*
 - user can follow a symlink anywhere within /dir/foo/share and subdirs
with path /dir/foo/share/**, and can create symlinks that point at any
target within these. Same as current "follow symlinks"

*valid symlink targets = follow:/*
 - user can follow a symlink anywhere within file system tree, but can only
create symlinks that point to targets like /dir/foo/share/** (default for
create is %P). Same as current "follow symlinks" + "wide
links"

*valid symlink targets = follow:/, create:/*
 - user can follow a symlink anywhere within file system tree, and create
symlinks that point to targets anywhere in the file system tree. Same as
current "follow symlinks" + "wide links" + "insecure
wide links"

*valid symlink targets = follow:/, create:/*
*invalid symlink targets = create:/etc, /bin, /boot, /dev, follow:/etc,
/dev*
  - user can create symlinks that point anywhere except a descendant of
/etc, /bin, /dev, and can follow symlinks anywhere other than to
descendants of /etc and /dev

*valid symlink targets = follow:%P, %P/../bar1, %P/../bar2, create:**%P,
%P/../bar1, %P/../bar2*
  - user can follow symlinks to targets within /dir/foo/share,
/dir/foo/bar1 and /dir/foo/bar2, but not to any other locations outside the
share root, and can create symlinks to any location in these dirs. (but not
to locations in other "wide" dirs.)

*valid symlink targets = follow:%P, %P/../bar1, %P/../bar2, create:**%P,
%P/../bar1, %P/../bar2*
*invalid symlink targets = follow:%P/../bar1/private, create:*
*%P/../bar1/private*
  - as above but cannot follow or create any symlink whose destination is
within /dir/foo/bar1/private

*valid symlink targets = follow:%P, /pub, create:**%P, /pub/devel*
*invalid symlink targets = create:/pub/devel/unlinkable*
  - the user can follow and create symlinks pointing within the share root.
They can also follow any symlink to a target within /pub (a general public
dir), and can create symlinks to *most* locations within /pub, with the
exception of links to /pub/unlinkable/***

*invalid symlink targets = create:/, follow:/*
  - user cannot create or follow any symlinks

*valid symlink targets = create: , follow:*
  - overrides the defaults of %P with empty follow+allow lists, effectively
making symlinks followable to no targets and creatable to no targets,
Equivalent to previous example.


This would simplify and enhance symlink handling, by allowing a user to
specify explicitly where symlinks can/cannot be followed to, and where they
can/cannot be created to link to. So a Samba share could allow wide symlink
creation and yet restrict where they linked, either by enumerating
acceptable locations or prohibiting invalid ones - a very powerful and
flexible tool.  It also simplifies and replicates current behaviour, and
brings symlink params in line with other "valid/invalid" params, such
as
those permitting/prohibiting IPs/hosts/users.

(As an aside, I haven't assumed regex because I figure this is way powerful
enough for the majority of uses that aren't currently configurable, and
Samba needs to be fast but regex is expensive. Not much gain, lots of
potential cost.)

Feedback and suggestions/comments sought?

Thanks,

Stilez

(Add to above email in this thread:
Symlink params aren't independent of other params, although conceptually 
they could (and perhaps should?) be. Symlink handling entangles with Unix 
extensions, with possible behaviour changes. The proposal avoids this and 
gives "clean" and easily understood symlink behaviours that don't
interact
with Unix extensions or any other params. So behaviour is consistent, 
easier to be certain of implications, independent of other settings, and 
distinct from whether or not Unix extensions are set.


On 1 March 2018 10:48:02 am Stilez Stilezy <stilezy at gmail.com> 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*
>
> The syntax in each param is a keyword (follow: | create:) which identifies
> if the masks after it control *following* a symlink to the location, or
> *creating* a symlink to the location. The type must be explicit (no
> implicit "follow"). After each "(follow: | create:)",
any masks listed are
> deemed to be of that type. An empty list is valid and means
"none".
>
>    - A symlink to /dir/foo/bar will be *followed *if any of /, /dir,
>    /dir/foo, or /dir/foo/bar are given as "follow" masks in a
"valid symlink
>    targets" (*provided that* none of these is also listed within an
>    "invalid symlink targets" follow clause).
>    - A symlink to /dir/foo/bar can be *created* by a user if any of /,
>    /dir, /dir/foo, or /dir/foo/bar are given as "create" masks in
a "valid
>    symlink targets" (*provided that* none of these is also listed
within an
>    "invalid symlink targets" create clause).
>
>
> *Examples (the share root in each case is /dir/foo/share):*
>
>
> *DEFAULT (follow:%P, create:%P)*
>  - user can follow a symlink anywhere within /dir/foo/share and subdirs
> with path /dir/foo/share/**, and can create symlinks that point at any
> target within these. Same as current "follow symlinks"
>
> *valid symlink targets = follow:/*
>  - user can follow a symlink anywhere within file system tree, but can only
> create symlinks that point to targets like /dir/foo/share/** (default for
> create is %P). Same as current "follow symlinks" + "wide
links"
>
> *valid symlink targets = follow:/, create:/*
>  - user can follow a symlink anywhere within file system tree, and create
> symlinks that point to targets anywhere in the file system tree. Same as
> current "follow symlinks" + "wide links" +
"insecure wide links"
>
> *valid symlink targets = follow:/, create:/*
> *invalid symlink targets = create:/etc, /bin, /boot, /dev, follow:/etc,
> /dev*
>   - user can create symlinks that point anywhere except a descendant of
> /etc, /bin, /dev, and can follow symlinks anywhere other than to
> descendants of /etc and /dev
>
> *valid symlink targets = follow:%P, %P/../bar1, %P/../bar2, create:**%P,
> %P/../bar1, %P/../bar2*
>   - user can follow symlinks to targets within /dir/foo/share,
> /dir/foo/bar1 and /dir/foo/bar2, but not to any other locations outside the
> share root, and can create symlinks to any location in these dirs. (but not
> to locations in other "wide" dirs.)
>
> *valid symlink targets = follow:%P, %P/../bar1, %P/../bar2, create:**%P,
> %P/../bar1, %P/../bar2*
> *invalid symlink targets = follow:%P/../bar1/private, create:*
> *%P/../bar1/private*
>   - as above but cannot follow or create any symlink whose destination is
> within /dir/foo/bar1/private
>
> *valid symlink targets = follow:%P, /pub, create:**%P, /pub/devel*
> *invalid symlink targets = create:/pub/devel/unlinkable*
>   - the user can follow and create symlinks pointing within the share root.
> They can also follow any symlink to a target within /pub (a general public
> dir), and can create symlinks to *most* locations within /pub, with the
> exception of links to /pub/unlinkable/***
>
> *invalid symlink targets = create:/, follow:/*
>   - user cannot create or follow any symlinks
>
> *valid symlink targets = create: , follow:*
>   - overrides the defaults of %P with empty follow+allow lists, effectively
> making symlinks followable to no targets and creatable to no targets,
> Equivalent to previous example.
>
>
> This would simplify and enhance symlink handling, by allowing a user to
> specify explicitly where symlinks can/cannot be followed to, and where they
> can/cannot be created to link to. So a Samba share could allow wide symlink
> creation and yet restrict where they linked, either by enumerating
> acceptable locations or prohibiting invalid ones - a very powerful and
> flexible tool.  It also simplifies and replicates current behaviour, and
> brings symlink params in line with other "valid/invalid" params,
such as
> those permitting/prohibiting IPs/hosts/users.
>
> (As an aside, I haven't assumed regex because I figure this is way
powerful
> enough for the majority of uses that aren't currently configurable, and
> Samba needs to be fast but regex is expensive. Not much gain, lots of
> potential cost.)
>
> Feedback and suggestions/comments sought?
>
> Thanks,
>
> Stilez

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.

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 at samba.org> 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.