Re: [TOMOYO #13 (mmotm 2008-11-19-02-19) 01/11] Introduce security_path_clear() hook.

[Tetsuo Handa]

Hello.

Stephen Smalley wrote:
> On Thu, 2008-11-20 at 20:25 +0900, Tetsuo Handa wrote:
> > plain text document attachment (introduce-security_path_clear.patch)
> > To perform DAC performed in vfs_foo() before MAC, we let security_path_foo()
> > save a result into our own hash table and return 0, and let security_inode_foo()
> > return the saved result. Since security_inode_foo() is not always called after
> > security_path_foo(), we need security_path_clear() to clear the hash table.
> 
> This seems very fragile and unmaintainable to me.  The fact that you
> even need a security_path_clear() hook suggests that something is wrong
> with the other security_path* hooks. I'd suggest that you explicitly
> pass the result of the security_path* hooks down to the security_inode*
> hooks instead.  What do others think?

You are recommending us to pass variables required for security_inode_*() via
stack memory rather than private hash, aren't you?
I think there are two problems.

One is that the variable passed via stack memory won't be used by SELinux and
SMACK and "CONFIG_SECURITY=n kernels", which will be a waste of stack memory.

The other one is that TOMOYO will need another variable for telling how the
security_inode_*() are called. Passing the variable via stack memory requires
modification of all vfs_*() calls, but TOMOYO doesn't check requests issued
by (e.g.) stackable filesystems.

By the way, this security_path_clear() is intended to be able to return DAC's
error code in priority to MAC's error code, but there are two problems for
TOMOYO.
One is that pathnames which will be later denied by DAC are appended by
TOMOYO's learning mode (i.e. garbage entries appears in the learned policy).
The other is that warning messages on pathnames which will be later denied by
DAC are generated by TOMOYO's enforcing mode.

Thus, it will be preferable for TOMOYO to "do MAC checks after DAC checks"
rather than to "return DAC's error in priority to MAC's error while doing MAC
checks before DAC checks".

To do so, "security_path_*() should be replaced by security_path_set(vfsmount)"
and "let security_inode_*() do MAC checks using the result of
security_path_set()" and "let security_path_clear() clear the result of
security_path_set() in case security_inode_*() was not called".

So, I think storing the pathname of "struct vfsmount" in the form of "char *"
into private hash at security_path_set() and clearing the private hash at
security_path_clear() should be most preferable.

Regards.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/