2008-03-21 14:59:59

by Miklos Szeredi

[permalink] [raw]
Subject: r-o bind in nfsd

Why is it that in fs/nfsd/vfs.c only vfs_mknod() and vfs_rename() are
surrounded by mnt_want_write/mnt_drop_write, and not the other
operations (vfs_create, vfs_mkdir, vfs_symlink, ...)?

I noticed this while looking at the AppArmor patches, which need to
pass the vfsmount down to the security module. And I'm wondering, why
can't mnt_want_write() and mnt_drop_write() be done _inside_ vfs_foo()?

I know there are a few cases, where filesystems call vfs_foo()
internally, where the vfsmount isn't available, but I think the proper
solution is just to fix those places, and not recurse back into the
VFS (which is AFAICS in all those cases totally unnecessary anyway).
This would make everybody happy, no?

Miklos


2008-03-21 15:55:12

by Al Viro

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Fri, Mar 21, 2008 at 03:59:44PM +0100, Miklos Szeredi wrote:
> Why is it that in fs/nfsd/vfs.c only vfs_mknod() and vfs_rename() are
> surrounded by mnt_want_write/mnt_drop_write, and not the other
> operations (vfs_create, vfs_mkdir, vfs_symlink, ...)?
>
> I noticed this while looking at the AppArmor patches, which need to
> pass the vfsmount down to the security module. And I'm wondering, why
> can't mnt_want_write() and mnt_drop_write() be done _inside_ vfs_foo()?
>
> I know there are a few cases, where filesystems call vfs_foo()
> internally, where the vfsmount isn't available, but I think the proper
> solution is just to fix those places, and not recurse back into the
> VFS (which is AFAICS in all those cases totally unnecessary anyway).
> This would make everybody happy, no?

Apparmor can go play with itself. The proper fix is to lift the LSM nonsense
into callers and leave vfs_...() alone; vfsmounts should *not* be passed
there at all, with the exception of vfs_follow_link() which gets the full
nameidata.

2008-03-21 16:24:30

by Miklos Szeredi

[permalink] [raw]
Subject: Re: r-o bind in nfsd

> > I know there are a few cases, where filesystems call vfs_foo()
> > internally, where the vfsmount isn't available, but I think the proper
> > solution is just to fix those places, and not recurse back into the
> > VFS (which is AFAICS in all those cases totally unnecessary anyway).
> > This would make everybody happy, no?
>
> Apparmor can go play with itself. The proper fix is to lift the LSM nonsense
> into callers and leave vfs_...() alone;

Maybe. I know precious little about this security thing, so I won't
argue about it's merits or faults. But:

a) I have a hunch that the security guys wouldn't like to see the
order between permission() and security_foo() changed.

b) I fail to see how moving functionality to callers would improve
things

> vfsmounts should *not* be passed there at all, with the exception of
> vfs_follow_link() which gets the full nameidata.

Why?

Miklos

2008-03-21 16:35:33

by Al Viro

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Fri, Mar 21, 2008 at 05:24:11PM +0100, Miklos Szeredi wrote:
> > > I know there are a few cases, where filesystems call vfs_foo()
> > > internally, where the vfsmount isn't available, but I think the proper
> > > solution is just to fix those places, and not recurse back into the
> > > VFS (which is AFAICS in all those cases totally unnecessary anyway).
> > > This would make everybody happy, no?
> >
> > Apparmor can go play with itself. The proper fix is to lift the LSM nonsense
> > into callers and leave vfs_...() alone;
>
> Maybe. I know precious little about this security thing, so I won't
> argue about it's merits or faults. But:
>
> a) I have a hunch that the security guys wouldn't like to see the
> order between permission() and security_foo() changed.

It's their problem. Adjusting LSM methods, if needed, is up to LSM
maintainers, whenever moving the hooks or code around those become
convenient for kernel proper. According to Linus, IIRC.

Especially since in this case they want to change prototypes anyway, so the
churn is not an issue and having the hook called earlier is very unlikely to
cause any problems.

> b) I fail to see how moving functionality to callers would improve
> things
>
> > vfsmounts should *not* be passed there at all, with the exception of
> > vfs_follow_link() which gets the full nameidata.
>
> Why?

Because filesystem shouldn't _care_ where it is mounted. Anything
vfsmount-dependent belongs to upper layers. The same goes for passing
nameidata to fs methods, BTW - again, ->follow_link() is an obvious
legitimate exception.

2008-03-21 16:56:20

by Miklos Szeredi

[permalink] [raw]
Subject: Re: r-o bind in nfsd

> > > > I know there are a few cases, where filesystems call vfs_foo()
> > > > internally, where the vfsmount isn't available, but I think the proper
> > > > solution is just to fix those places, and not recurse back into the
> > > > VFS (which is AFAICS in all those cases totally unnecessary anyway).
> > > > This would make everybody happy, no?
> > >
> > > Apparmor can go play with itself. The proper fix is to lift the LSM nonsense
> > > into callers and leave vfs_...() alone;
> >
> > Maybe. I know precious little about this security thing, so I won't
> > argue about it's merits or faults. But:
> >
> > a) I have a hunch that the security guys wouldn't like to see the
> > order between permission() and security_foo() changed.
>
> It's their problem. Adjusting LSM methods, if needed, is up to LSM
> maintainers, whenever moving the hooks or code around those become
> convenient for kernel proper. According to Linus, IIRC.
>
> Especially since in this case they want to change prototypes anyway, so the
> churn is not an issue and having the hook called earlier is very unlikely to
> cause any problems.

CC-d linux-security-module, James Morris.

>
> > b) I fail to see how moving functionality to callers would improve
> > things
> >
> > > vfsmounts should *not* be passed there at all, with the exception of
> > > vfs_follow_link() which gets the full nameidata.
> >
> > Why?
>
> Because filesystem shouldn't _care_ where it is mounted. Anything
> vfsmount-dependent belongs to upper layers. The same goes for passing
> nameidata to fs methods, BTW - again, ->follow_link() is an obvious
> legitimate exception.

Nobody wants to send vfsmounts to the filesystem. But vfs_...() are
still part of the "upper layer", not the filesystem, so I'm not
convinced yet. For example:

-extern int vfs_mkdir(struct inode *, struct dentry *, int);
+extern int vfs_mkdir(const struct path *, struct dentry *, int);

There's one caller of vfs_mkdir that can't do this: cgroup_clone().
But that can call cgroup_mkdir() instead.

And having the vfsmount available within vfs_...() functions means,
that the mnt_want_write() check can be moved inside, which means that
callers get simpler and less likely to be buggy. Those are all
advantages IMO, regardless of any security module issues.

Miklos

2008-03-21 17:09:20

by Miklos Szeredi

[permalink] [raw]
Subject: Re: r-o bind in nfsd

> Nobody wants to send vfsmounts to the filesystem. But vfs_...() are
> still part of the "upper layer", not the filesystem, so I'm not
> convinced yet. For example:
>
> -extern int vfs_mkdir(struct inode *, struct dentry *, int);
> +extern int vfs_mkdir(const struct path *, struct dentry *, int);
>
> There's one caller of vfs_mkdir that can't do this: cgroup_clone().
> But that can call cgroup_mkdir() instead.
>
> And having the vfsmount available within vfs_...() functions means,
> that the mnt_want_write() check can be moved inside, which means that
> callers get simpler and less likely to be buggy. Those are all
> advantages IMO, regardless of any security module issues.

Or we can introduce another set of exported functions (path_mkdir(),
...), and leave vfs_...() alone. And then the only question is if
LSM's can live with ordering change.

Miklos

2008-03-21 18:11:22

by Al Viro

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Fri, Mar 21, 2008 at 06:08:53PM +0100, Miklos Szeredi wrote:

> > And having the vfsmount available within vfs_...() functions means,
> > that the mnt_want_write() check can be moved inside, which means that
> > callers get simpler and less likely to be buggy. Those are all
> > advantages IMO, regardless of any security module issues.
>
> Or we can introduce another set of exported functions (path_mkdir(),
> ...), and leave vfs_...() alone. And then the only question is if
> LSM's can live with ordering change.

I really don't see the point of new helpers; especially since one doesn't
have to _have_ vfsmount to use the old ones and since we don't have a lot
of users of each of those to start with.

As for the apparmor and friends... I'm far past the point of trying to
give them feedback, seeing how any such feedback is ignored, if not worse
(the ugliness of some of the suggested kludges is bloody astonishing - e.g.
some guy proposed to stuff a reference to vfsmount into task_struct, etc.)

2008-03-21 18:53:41

by Miklos Szeredi

[permalink] [raw]
Subject: Re: r-o bind in nfsd

> > > And having the vfsmount available within vfs_...() functions means,
> > > that the mnt_want_write() check can be moved inside, which means that
> > > callers get simpler and less likely to be buggy. Those are all
> > > advantages IMO, regardless of any security module issues.
> >
> > Or we can introduce another set of exported functions (path_mkdir(),
> > ...), and leave vfs_...() alone. And then the only question is if
> > LSM's can live with ordering change.
>
> I really don't see the point of new helpers; especially since one doesn't
> have to _have_ vfsmount to use the old ones and since we don't have a lot
> of users of each of those to start with.

Traditionally we have syscalls, and nfsd. Both of them want the
security checks, and I think nfsd wants the read-only mount checking
as well, but I'm not entirely sure. Maybe we can handle that by just
making nfsd acquire a write-ref on the mount and keep it while it's
exported.

Then there's ecryptfs and unionfs, which probably need neither, but it
wouldn't hurt to do them anyway.

Still, even if there are only two callers, then moving stuff to up
doesn't make any sense. Passing down a struct path is free for the
syscall case, it doesn't consume any stack space or extra CPU. Do
please tell, why would that be such a bad thing?

Miklos

2008-03-21 19:49:46

by Al Viro

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Fri, Mar 21, 2008 at 07:52:35PM +0100, Miklos Szeredi wrote:
> > > > And having the vfsmount available within vfs_...() functions means,
> > > > that the mnt_want_write() check can be moved inside, which means that
> > > > callers get simpler and less likely to be buggy. Those are all
> > > > advantages IMO, regardless of any security module issues.
> > >
> > > Or we can introduce another set of exported functions (path_mkdir(),
> > > ...), and leave vfs_...() alone. And then the only question is if
> > > LSM's can live with ordering change.
> >
> > I really don't see the point of new helpers; especially since one doesn't
> > have to _have_ vfsmount to use the old ones and since we don't have a lot
> > of users of each of those to start with.
>
> Traditionally we have syscalls, and nfsd. Both of them want the
> security checks, and I think nfsd wants the read-only mount checking
> as well, but I'm not entirely sure. Maybe we can handle that by just
> making nfsd acquire a write-ref on the mount and keep it while it's
> exported.
>
> Then there's ecryptfs and unionfs, which probably need neither, but it
> wouldn't hurt to do them anyway.
>
> Still, even if there are only two callers, then moving stuff to up
> doesn't make any sense. Passing down a struct path is free for the
> syscall case, it doesn't consume any stack space or extra CPU. Do
> please tell, why would that be such a bad thing?

Because we'd been that way before; see the shitpiles around ->lookup()
getting nameidata, etc. You'll end up with some callers passing NULL
as ->mnt since they don't have anything better to pass, some stuff
called *from* the damn thing caring to check for ->mnt being NULL,
some stuff not caring about what ->mnt is at all and some assuming
that it's not NULL. Which will lead to exploding combinations that
won't be noticed until somebody steps into such config.

As for the vfsmount-dependent checks (and any kind of MAC, while we are
at it)... They belong to callers, exactly because different callers may
want different (amount of) checks.

2008-03-21 20:24:21

by Miklos Szeredi

[permalink] [raw]
Subject: Re: r-o bind in nfsd

> > > > > And having the vfsmount available within vfs_...() functions means,
> > > > > that the mnt_want_write() check can be moved inside, which means that
> > > > > callers get simpler and less likely to be buggy. Those are all
> > > > > advantages IMO, regardless of any security module issues.
> > > >
> > > > Or we can introduce another set of exported functions (path_mkdir(),
> > > > ...), and leave vfs_...() alone. And then the only question is if
> > > > LSM's can live with ordering change.
> > >
> > > I really don't see the point of new helpers; especially since one doesn't
> > > have to _have_ vfsmount to use the old ones and since we don't have a lot
> > > of users of each of those to start with.
> >
> > Traditionally we have syscalls, and nfsd. Both of them want the
> > security checks, and I think nfsd wants the read-only mount checking
> > as well, but I'm not entirely sure. Maybe we can handle that by just
> > making nfsd acquire a write-ref on the mount and keep it while it's
> > exported.
> >
> > Then there's ecryptfs and unionfs, which probably need neither, but it
> > wouldn't hurt to do them anyway.
> >
> > Still, even if there are only two callers, then moving stuff to up
> > doesn't make any sense. Passing down a struct path is free for the
> > syscall case, it doesn't consume any stack space or extra CPU. Do
> > please tell, why would that be such a bad thing?
>
> Because we'd been that way before; see the shitpiles around ->lookup()
> getting nameidata, etc. You'll end up with some callers passing NULL
> as ->mnt since they don't have anything better to pass, some stuff
> called *from* the damn thing caring to check for ->mnt being NULL,
> some stuff not caring about what ->mnt is at all and some assuming
> that it's not NULL. Which will lead to exploding combinations that
> won't be noticed until somebody steps into such config.

Right, we do want to prevent that happening.

And for example moving read-only mount checks inside vfs_...() would
ensure that.

> As for the vfsmount-dependent checks (and any kind of MAC, while we are
> at it)... They belong to callers, exactly because different callers may
> want different (amount of) checks.

And we end up random callers forgetting some of the checks, like we
have now with nfsd. Not good at all.

I think it's still a lot better all the checks are always done, even
if not strictly necessary for a certain caller, than if the caller has
to make sure the necessary ones do get done.

Assuming of course, that all valid users _do_ have the vfsmount
available, which I think is true. If you have a counterexample,
please let us know.

If not all (but most) callers have the vfsmount available, then a new
helper makes sense.

If there was only one caller which needed a certain check, then moving
that into the caller would be the right thing of course. But that's
not the case here.

Miklos

2008-03-21 21:08:26

by Dave Hansen

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Fri, 2008-03-21 at 19:52 +0100, Miklos Szeredi wrote:
> Traditionally we have syscalls, and nfsd. Both of them want the
> security checks, and I think nfsd wants the read-only mount checking
> as well, but I'm not entirely sure. Maybe we can handle that by just
> making nfsd acquire a write-ref on the mount and keep it while it's
> exported.

The only question for me would be where the current r/o checks are
happening (IS_RDONLY()). I generally based my patches on replacing
those calls.

-- Dave

2008-03-21 21:17:40

by Miklos Szeredi

[permalink] [raw]
Subject: Re: r-o bind in nfsd

> On Fri, 2008-03-21 at 19:52 +0100, Miklos Szeredi wrote:
> > Traditionally we have syscalls, and nfsd. Both of them want the
> > security checks, and I think nfsd wants the read-only mount checking
> > as well, but I'm not entirely sure. Maybe we can handle that by just
> > making nfsd acquire a write-ref on the mount and keep it while it's
> > exported.
>
> The only question for me would be where the current r/o checks are
> happening (IS_RDONLY()). I generally based my patches on replacing
> those calls.

In may_create()/may_delete() on parent directory. So that one needs
audit of all callers, unless Al can be convinced that moving those
checks into the VFS makes sense.

Miklos

2008-03-22 02:20:39

by Tetsuo Handa

[permalink] [raw]
Subject: Re: r-o bind in nfsd

Hello.

> As for the apparmor and friends... I'm far past the point of trying to
> give them feedback, seeing how any such feedback is ignored, if not worse
> (the ugliness of some of the suggested kludges is bloody astonishing - e.g.
> some guy proposed to stuff a reference to vfsmount into task_struct, etc.)
TOMOYO is one of AppArmor's friends, and I am the guy who proposed to pass
a reference to vfsmount via task_struct as if that reference is passed via stack memory.

> > Because filesystem shouldn't _care_ where it is mounted. Anything
> > vfsmount-dependent belongs to upper layers. The same goes for passing
> > nameidata to fs methods, BTW - again, ->follow_link() is an obvious
> > legitimate exception.
>
> Nobody wants to send vfsmounts to the filesystem. But vfs_...() are
> still part of the "upper layer", not the filesystem, so I'm not
> convinced yet. For example:
I'm not asking to pass the vfsmount parameter to individual filesystem's "vfs functions".
I'm asking to pass the vfsmount parameter to the "vfs *helper* functions".
So, filesytem will not care where it is mounted
even if the vfsmount is passed to "vfs *helper* functions".
The vfs helper functions are designed to aggregate common checks (permission checks,
inode_operations->foo existence checks etc.) to avoid scattering same code everywhere
in the kernel, didn't they?

> Assuming of course, that all valid users _do_ have the vfsmount
> available, which I think is true. If you have a counterexample,
> please let us know.
At least, calls to vfs helper functions from userland code
_do_ have the vfsmount available because they are called immediately after the name resolution.

Calls to vfs helper functions from kernel code does not always have the vfsmount available,
but that's beyond what the LSM can do.
We must trust kernel code, because kernel code can bypass the LSM check
if the kernel code is malicious enough to directly call "vfs functions" instead of
calling "vfs helper functions".
(Or, more simply, kernel code can rewrite the call to the LSM check to no-op
like funny workaround for vmsplice's vulnerability).

I think attempt to receive the vfsmount from kernel code won't help guaranteeing that
LSM's security checks are always performed.
Routes to access "vfs functions" from kernel code is undeterminable.
In other words, LSM can't guarantee that LSM's security checks are always performed
against the kernel code regardless of the security model.

But, at least, LSM can guarantee that LSM's security checks are always performed
against the userland code regardless of the security model.
Routes to access "vfs functions" from userland code is determinable.

So, making the vfsmount available to LSM make sense.
I don't care if the vfsmount is unavailable when the vfs helper function call was
issued from kernel code.

2008-03-25 02:53:01

by NeilBrown

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Friday March 21, [email protected] wrote:
>
> Nobody wants to send vfsmounts to the filesystem. But vfs_...() are
> still part of the "upper layer", not the filesystem, so I'm not
> convinced yet. For example:

The layers within the VFS are probably not very clearly defined, but I
think one can fairly clearly see a difference between a "filesystem"
layer and a "namespace" layer.

The vfs_XX function are (in my mind) the top of the filesystem layer.
They primarily call the relevant xxx_operation and just add minimal
support code to enforce standard policy, callouts, etc.

vfsmnts are very much part of the "namespace" layer. The heart of
this layer is probably link_path_walk. It allows mounts to tie
filesystems together in all sorts of interesting knots :-)

The readonly-bind-mount concept adds new functionality in the
namespace layer.
The filesystem layer already has a concept of readonly mounts, and
this doesn't change (I hope). The superblock still has a readonly
flag and the vfs_XXX operations must (possibly indirectly) still test
this flag.

readonly-bind-mounts adds a new 'readonly' flag in the namespace
layer.

So if you want to centralise the code for implementing this
functionality (which seems like a good idea), it should probably
go in link_path_walk, or one of it's friends, rather than in vfs_XXX.

Maybe a new LOOKUP_WRITEACCESS flag which arranges that mnt_want_write
gets called as appropriate, and that path_put will call mnt_drop_write
if needed.

Maybe some enhancement to the 'intent' structure with a similar
effect could be done instead.

Then you could, presumably, put a security hook somewhere in
link_path_walk for those modules (like AppArmor) which want to do
checks based on the namespace.

=========

I just had a look at the places where mnt_want_write is currently
called. There are quite a few of them.

Two that surprised me were touch_atime and file_update_time.

It is not clear to me that we should avoid updating 'atime' if the
bindmount is marked readonly. The file is still being accessed, so
updating the atime could still be appropriate.
Possibly a "noatime" per-vfsmnt flag would be appropriate. But I'm
not convinced that interpreting per-vfsmnt 'readonly' as "don't update
atime" is correct.

And the mnt_want_write call in file_update_time seems superfluous.
This only gets called on files that were already opened for write
..... except for named pipes. We don't call mnt_want_write when we
open those, but we do call file_update_time whenever we write to them.
That is an awkward asymmetry. I suspect that mnt_want_write *Should*
be called when a named pipe is opened for write.

I think all other calls to mnt_want_write could be avoided by passing
an appropriate flag to the relevant lookup routine, but I haven't
checked thoroughly.

NeilBrown


(for those who are interested, the places I found that call
mnt_want_write are:

update atime
update mtime
create file
mknod/mkdir/rmdir/unlink/symlink/link/rename
truncate/fchmod/chown
get_file_write_access
utimes
setxattr
'init_file' (which is only used for sockets and shared mem
segments. As the comment near init_file says, it
doesn't really need mnt_want_write, it is just there
for balance).

)

2008-03-25 11:46:14

by Tetsuo Handa

[permalink] [raw]
Subject: Re: r-o bind in nfsd

Hello.

> Maybe some enhancement to the 'intent' structure with a similar
> effect could be done instead.
>
> Then you could, presumably, put a security hook somewhere in
> link_path_walk for those modules (like AppArmor) which want to do
> checks based on the namespace.

I think link_path_walk() is not a good place to insert new LSM hooks
for pathname based access control (AppArmor and TOMOYO) purpose because

(1) The kernel don't know what operation (open/create/truncate etc.) will be
done at the moment of link_path_walk().

(2) Not all operations call link_path_walk() before these operations
are done. For example, ftruncate() doesn't call link_path_walk().

(3) The rename() and link() operations handle two pathnames.
But, it is not possible to know both pathnames at the moment of
link_path_walk().

I think we need to introduce new LSM hooks outside link_path_walk().
http://kerneltrap.org/mailarchive/linux-fsdevel/2008/2/17/882024

2008-03-25 22:32:30

by NeilBrown

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Tue, March 25, 2008 10:45 pm, Tetsuo Handa wrote:
> Hello.
>
>> Maybe some enhancement to the 'intent' structure with a similar
>> effect could be done instead.
>>
>> Then you could, presumably, put a security hook somewhere in
>> link_path_walk for those modules (like AppArmor) which want to do
>> checks based on the namespace.
>
> I think link_path_walk() is not a good place to insert new LSM hooks
> for pathname based access control (AppArmor and TOMOYO) purpose because
>
> (1) The kernel don't know what operation (open/create/truncate etc.)
> will be
> done at the moment of link_path_walk().

Though the 'indent' data structure could be used to carry this information.

>
> (2) Not all operations call link_path_walk() before these operations
> are done. For example, ftruncate() doesn't call link_path_walk().

But do you want to impose path-name based controls to ftruncate?
Surely once you have a file open for write (not O_APPEND), then no
other permission is required to truncate the file, is it?
If it is, then maybe the 'struct file' should be tagged at open time
to say whether 'truncate' is allowed.

>
> (3) The rename() and link() operations handle two pathnames.
> But, it is not possible to know both pathnames at the moment of
> link_path_walk().

Not an insolvable problem.
One could imagine an implementation where a TYPE_RENAME_FROM security
check produced a cookie that was consumed by a TYPE_RENAME_TO security
check. The cookie could then be used by the security module to
make any connection between the two names that might be appropriate.

>
> I think we need to introduce new LSM hooks outside link_path_walk().
> http://kerneltrap.org/mailarchive/linux-fsdevel/2008/2/17/882024
>
<rant>
I suspect we would be much better off removing all the security hooks.
Security done at that level seems to be way too complex such that most
people don't really understand it. And people who don't understand
security don't use it.
We'd be much better off getting rid of the whole "micro-manage security"
concept and provide isolation via some sort of high level container
approach.
</rant>

NeilBrown

2008-03-25 23:29:47

by NeilBrown

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Wed, March 26, 2008 9:49 am, Al Viro wrote:
> On Wed, Mar 26, 2008 at 09:32:08AM +1100, NeilBrown wrote:
>
>> > (1) The kernel don't know what operation (open/create/truncate etc.)
>> > will be
>> > done at the moment of link_path_walk().
>>
>> Though the 'indent' data structure could be used to carry this
>> information.
>
> If it's 'intent',

Yes, sorry.

> that mess will be gone next cycle.

Cool. Any chance of a preview? Is it in -next or -mm ??

>> > (3) The rename() and link() operations handle two pathnames.
>> > But, it is not possible to know both pathnames at the moment of
>> > link_path_walk().
>>
>> Not an insolvable problem.
>> One could imagine an implementation where a TYPE_RENAME_FROM security
>> check produced a cookie that was consumed by a TYPE_RENAME_TO security
>> check. The cookie could then be used by the security module to
>> make any connection between the two names that might be appropriate.
>
> alt.tasteless.software is that -> way...
>

While I have no desire to defend that particular design, saying "tasteless"
without suggesting an alternate approach does appear somewhat unhelpful.

NeilBrown

2008-03-26 12:34:54

by Stephen Smalley

[permalink] [raw]
Subject: Re: r-o bind in nfsd


On Wed, 2008-03-26 at 09:32 +1100, NeilBrown wrote:
> On Tue, March 25, 2008 10:45 pm, Tetsuo Handa wrote:
> > Hello.
> >
> >> Maybe some enhancement to the 'intent' structure with a similar
> >> effect could be done instead.
> >>
> >> Then you could, presumably, put a security hook somewhere in
> >> link_path_walk for those modules (like AppArmor) which want to do
> >> checks based on the namespace.
> >
> > I think link_path_walk() is not a good place to insert new LSM hooks
> > for pathname based access control (AppArmor and TOMOYO) purpose because
> >
> > (1) The kernel don't know what operation (open/create/truncate etc.)
> > will be
> > done at the moment of link_path_walk().
>
> Though the 'indent' data structure could be used to carry this information.
>
> >
> > (2) Not all operations call link_path_walk() before these operations
> > are done. For example, ftruncate() doesn't call link_path_walk().
>
> But do you want to impose path-name based controls to ftruncate?
> Surely once you have a file open for write (not O_APPEND), then no
> other permission is required to truncate the file, is it?
> If it is, then maybe the 'struct file' should be tagged at open time
> to say whether 'truncate' is allowed.
>
> >
> > (3) The rename() and link() operations handle two pathnames.
> > But, it is not possible to know both pathnames at the moment of
> > link_path_walk().
>
> Not an insolvable problem.
> One could imagine an implementation where a TYPE_RENAME_FROM security
> check produced a cookie that was consumed by a TYPE_RENAME_TO security
> check. The cookie could then be used by the security module to
> make any connection between the two names that might be appropriate.
>
> >
> > I think we need to introduce new LSM hooks outside link_path_walk().
> > http://kerneltrap.org/mailarchive/linux-fsdevel/2008/2/17/882024
> >
> <rant>
> I suspect we would be much better off removing all the security hooks.
> Security done at that level seems to be way too complex such that most
> people don't really understand it. And people who don't understand
> security don't use it.
> We'd be much better off getting rid of the whole "micro-manage security"
> concept and provide isolation via some sort of high level container
> approach.
> </rant>

Containers can be useful, but they aren't a substitute for access
control, and they don't solve the same problem.

And SELinux does get used, and recent stats on Fedora 8 suggest that few
people disable it anymore. Advances in the SELinux tools (loadable
modules, semanage, system-config-selinux, setroubleshoot, etc) have gone
a long way to enabling users to solve problems they encounter.

--
Stephen Smalley
National Security Agency

2008-03-26 16:48:28

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: r-o bind in nfsd

Quoting Stephen Smalley ([email protected]):
>
> On Wed, 2008-03-26 at 09:32 +1100, NeilBrown wrote:
> > On Tue, March 25, 2008 10:45 pm, Tetsuo Handa wrote:
> > > Hello.
> > >
> > >> Maybe some enhancement to the 'intent' structure with a similar
> > >> effect could be done instead.
> > >>
> > >> Then you could, presumably, put a security hook somewhere in
> > >> link_path_walk for those modules (like AppArmor) which want to do
> > >> checks based on the namespace.
> > >
> > > I think link_path_walk() is not a good place to insert new LSM hooks
> > > for pathname based access control (AppArmor and TOMOYO) purpose because
> > >
> > > (1) The kernel don't know what operation (open/create/truncate etc.)
> > > will be
> > > done at the moment of link_path_walk().
> >
> > Though the 'indent' data structure could be used to carry this information.
> >
> > >
> > > (2) Not all operations call link_path_walk() before these operations
> > > are done. For example, ftruncate() doesn't call link_path_walk().
> >
> > But do you want to impose path-name based controls to ftruncate?
> > Surely once you have a file open for write (not O_APPEND), then no
> > other permission is required to truncate the file, is it?
> > If it is, then maybe the 'struct file' should be tagged at open time
> > to say whether 'truncate' is allowed.
> >
> > >
> > > (3) The rename() and link() operations handle two pathnames.
> > > But, it is not possible to know both pathnames at the moment of
> > > link_path_walk().
> >
> > Not an insolvable problem.
> > One could imagine an implementation where a TYPE_RENAME_FROM security
> > check produced a cookie that was consumed by a TYPE_RENAME_TO security
> > check. The cookie could then be used by the security module to
> > make any connection between the two names that might be appropriate.
> >
> > >
> > > I think we need to introduce new LSM hooks outside link_path_walk().
> > > http://kerneltrap.org/mailarchive/linux-fsdevel/2008/2/17/882024
> > >
> > <rant>
> > I suspect we would be much better off removing all the security hooks.
> > Security done at that level seems to be way too complex such that most
> > people don't really understand it. And people who don't understand
> > security don't use it.
> > We'd be much better off getting rid of the whole "micro-manage security"
> > concept and provide isolation via some sort of high level container
> > approach.
> > </rant>
>
> Containers can be useful, but they aren't a substitute for access
> control, and they don't solve the same problem.

Not only that, but containers require an LSM to provide user isolation
and root containment.

Hopefully at some point we'll be able to get around that for most VPSs,
but that's a long ways off and personally I suspect I would always want
a policy locking VPSs up nice and tight.

> And SELinux does get used, and recent stats on Fedora 8 suggest that few
> people disable it anymore. Advances in the SELinux tools (loadable
> modules, semanage, system-config-selinux, setroubleshoot, etc) have gone
> a long way to enabling users to solve problems they encounter.
>
> --
> Stephen Smalley
> National Security Agency
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2008-03-26 21:43:19

by James Morris

[permalink] [raw]
Subject: Re: r-o bind in nfsd

On Wed, 26 Mar 2008, Serge E. Hallyn wrote:

> Not only that, but containers require an LSM to provide user isolation
> and root containment.

You mean LSM hooks, which various LSM could utilize if desired, right?


- James
--
James Morris
<[email protected]>

2008-03-27 01:40:34

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: r-o bind in nfsd

Quoting James Morris ([email protected]):
> On Wed, 26 Mar 2008, Serge E. Hallyn wrote:
>
> > Not only that, but containers require an LSM to provide user isolation
> > and root containment.
>
> You mean LSM hooks, which various LSM could utilize if desired, right?

Yes.

thanks,
-serge