Christoph Hellwig writes:
>
>Hi Matthew,
>
[...]
>
Hi Christoph,

Thanks for your input.  You have raised a number of interesting points, which
are addressed in more detail inline.  Some of these I should probably have
included in the documentation in the first place.

I should perhaps note here first that the implementation of these symlinks is
a work in progress - I have done it the way I have to get it going, and am
not entirely happy with it as it stands.  I am sure there are better ways of
implenting them, and am interested in technical Linux implementation comments
about that offline.

However, regardless of the implementation details, I think that there is a
lot to be said for the concept of these symlinks, and I am interested in
feedback on what people think of their level of usefulness and scope.  Also
of interest would be feedback about the various security implications that
relate to them.

It should be noted that the concept of symlinks with dynamically calculated
path names is not a new one - various Unices have implemented them at one
time or another with various variables and syntaxes.  What makes this version
perhaps a bit unusual is the combination of dynamically calculated path names
with automatically creating the target if it does not exist.

I have aimed with this patch to do one thing - dynamic symlinks - with the
aim of solving a particular (perhaps small in scope, but widely occuring) set
of problems, revolving around /tmp races.  There are other solutions that do
a whole lot more as well, like turning a machine into a series of independent
compartments.  This is great for those who need that, but introduces more
complexity (and potentially bugs) for those who don't.

More comments inline...

>I think your proposal is a really kludgy hack.  While the idea of
>user-specific namespaces in gerneral is a very good idea, your patch is far
>to ungeneric.

Yes, I agree.  It would be nice to be able to have a user-extensible syntax
for these symlinks.  This is one of the major problems I have with the
current implementation.  Though it does solve the /tmp race problem, you
can't add new dynamic symlink types of your own.  Perhaps some trickery with
loadable modules might help.

>From a security perspective, one could argue that the less configurable it is
the better - particularly for something like a firewall.  Nevertheless, from
an architectural standpoint I agree that there would be better ways of
implementing this.

>
>An sane implementation of the same concept can easily be done using Al Viro's
>namespace patches for Linux 2.4 (Latest version is namespaces-b-S3-pre8.gz in
>ftp.math.psu.edu:/pub/viro) - this patch allows an additional parameter
>(CLONE_NEWNS) to be passed to clone(2), Linux's syscall for the creation of
>rfork-style variable-weight processes which will setup an completly different
>mount table (copied from the parent).
>
>In this particular case the login process would create the users login
>shell's process using CLONE_NEWS and use the Linux 2.4+ of namespace
>bindings to create a private temp directory.  The following code sequence
>(untested and simplified) should give a hint how to implement the private
>tmpdir binding after the clone(..., CLONE_NEWNS):

>
>>> pw = getpwent();
>>> if (pw) {
>>>     strlcpy(tmpdir, pw->pw_dir, MAXTMPDIR);
>>>     strlcat(tmpdir, "/tmp", MAXTMPDIR);
>>>     createifdoesnotexist(tmpdir);
>>>     mount(tmpdir, "/tmp", "dontcare", MS_BIND, NULL);
>>> }
>

Yes, this is another way of doing something similar.  Notice however that it
requires the modification of programs such as login (or in.rshd or in.sshd or
X scripts or any other way that exists to get into the system, or that will
be created later to get into the system) to set up these things.  (Also, I
think you will find not many people run 2.4 kernels at the moment...)

There are plenty of solutions around which require modification of programs.
These symlinks allow programs to run as normal, making the kernel do the
work.  Once the kernel is replaced and the symlinks are created, the problem
is solved.

>Besides the general conceptual flaws your patch also has some implementation
>problems.  First your tmpdir-creation is implemented in the filesystem
>specific kernel code and not in the VFS.  What does your patch with /tmp

Yes, I should have mentioned this in the docco.  My 2.4 patches do this more
generically, but in 2.2 they are for ext2 only.  My apologies for the
omission - the docco has been updated.  The 2.4 patches are not yet ready to
be released, but I hope to get around to finishing them in the next couple of
weeks.

[Technical discussion]
The reason that the 2.2 patches are ext2-only is that under 2.2, all you get
in the do_follow_link entry point is the dentry structs.  These do not
(necessarily) include the name of the link.  Getting this generically in 2.2
does not appear to be easy - it is documented that calling readlink (which
would be the generic way of getting the symlink value out) may yield
different results from what follow_link gets.  2.4 has functions which deal
with the path names directly, which make life easier.  So I chose to just
deal with ext2 for the 2.2 case for now.

If you want to discuss this further I'd be glad to do so offline.
[End technical discussion]

>on nfs or reiserfs? - nothing.  Secondly you are checking against an
>effective userid of zero in your code - as Linux 2.2 uses an Posix 1003.1e
>(draft, whitedrawn) capability model and the old Unix model of comparing
>against the zero user id is considered legacy this is a very bad idea.

Yes, I am well aware of the capability model and the legacy zero user id.  I
assume you are referring to my "root" symbolic link type.  This is not part
of the code for the uid-based symlinks that form the basis of the /tmp race
solution.

For those who haven't had a look, the patch includes another symlink type, as
well as the uid-based one, that points to one place if the UID is 0, and
another if it is non-zero.  This was included more as a demonstration of the
sort of things that could be done with these links, rather than anything else.

The uid-based symlink code is capability-aware, and the discussion refers to
the issues involved with capabilities in the current implementation.

>
>An better implementation of context-sensitive directories is Malcolm Beattie
>mlsfs.  It's implemented as it's own filesystem so it is completly
>independand of the undelying phsical filesystem.  It's home on the web is at
>http://users.ox.ac.uk/~mbeattie/linux-kernel.html.

A most interesting product - great for people looking for B2 Linux systems,
but one that is a "very early alpha release".  I'm taking a different
approach and providing a solution for one specific problem: the /tmp race
hole and problems like it.  Because this is easier, I have something with
less features, but that is up and going today.

I did consider using a filesystem instead of symlinks to solve this problem,
but the method I chose seemed easier, simpler to use and just as useful for
most purposes. Under 2.4 this is done generically, without reference to the
underlying filesystem.  Perhaps the main advantage of using a filesystem is
to allow this to work inside filesystems that do not support symlinks.  For
the purpose in hand, I think if /tmp were mounted on a filesystem not
supporting symlinks, many things would break.

Thanks again for your comments

Cheers

                -Matthew

--
+--------------------------------------------------------------------------+
| Matthew Donaldson             http://www.datadeliverance.com             |
| Data Deliverance Pty. Ltd.    Email: [EMAIL PROTECTED]         |
| 30 Musgrave Ave.              Phone: +61 8 8265 7976            _        |
| Banksia Park                  Fax:   +61 8 8265 0032     John  / \/      |
| South Australia 5091                                     3:16  \_/\      |
+--------------------------------------------------------------------------+

Reply via email to