Re: Adding a security parameter to VFS functions
On Wednesday 15 August 2007 18:23, Casey Schaufler wrote: Hi Linus, Al, Would you object greatly to functions like vfs_mkdir() gaining a security parameter? Could you describe how this compares to the proposal that the AppArmor developers suggested recently? I expect that we can reduce the amount of discussion required, and maybe avoid some confusion if you could do that. That's from one of those patches: -int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) +int vfs_mkdir(struct inode *dir, struct dentry *dentry, struct vfsmount *mnt, + int mode) We need the vfsmount in the LSM hooks in addition to the dentry in order to figure out where in the filesystem namespace we are. The various vfs_ functions are the ones calling the LSM hooks. (The same could be achieved passing a struct path instead.) -- Andreas - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Wednesday 15 August 2007 13:40, David Howells wrote:
Hi Linus, Al,
Would you object greatly to functions like vfs_mkdir() gaining a security
parameter? What I'm thinking of is this:
int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode,
struct security *security)
Where the security context is the state of the context at the time the call
was issued:
struct security {
uid_t fsuid;
git_t fsgid;
struct group_info *group_info;
void*security;
struct key *session_keyring;
struct key *process_keyring;
struct key *thread_keyring;
And perhaps:
struct audit_context*audit_context;
seccomp_t seccomp;
};
This would, for the most part, be a temporary affair, being set up by such
as sys_mkdir()/sys_mkdirat() from data held in task_struct.
That's additional setup work unless that struct can be embedded in
task_struct. We would be complicating the common / fast / local case to
simplify the not-so-common case or cases.
-- Andreas
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Wed, 15 Aug 2007, David Howells wrote: Would you object greatly to functions like vfs_mkdir() gaining a security parameter? What I'm thinking of is this: int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode, struct security *security) I personally consider this an affront to everythign that is decent. Why the *hell* would mkdir() be so magical as to need something like that? Make it something sane, like a struct nameidata instead, and make it at least try to look like the path creation that is done by open(). Or create a struct file * or something. I can imagine having mkdir() being passed similar data as open() (ie lookup()), but I cannot _possibly_ imagine it ever being valid to pass in something totally made-up to just mkdir(), and nothing else. There's something fundamentally wrong there. What makes mkdir() so magical? Also, what about all the other ops? Why is mkdir() special, but not mknod()? Why is mkdir() special, but not rmdir()? Really, none of this seems to make any sense unless you describe what is so magical about mkdir(). Linus - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Thu, Aug 16, 2007 at 03:57:24PM -0700, Linus Torvalds wrote: I personally consider this an affront to everythign that is decent. Why the *hell* would mkdir() be so magical as to need something like that? Make it something sane, like a struct nameidata instead, and make it at least try to look like the path creation that is done by open(). Or create a struct file * or something. No. I agree that mkdir/mknod/etc. should all take the same thing. *However*, it should not be nameidata or file. Why? Because it should not contain vfsmount. Object creation should not *care* where fs is mounted. At all. The same goes for open(), BTW - letting it see the reference to vfsmount via nameidata is bloody wrong and I really couldn't care less what kinds of perversions apparmour crowd might like - pathnames simply do not belong there. Said that, I agree that we need uniform prototypes for all these guys and I can see arguments both for and against having creds passed by reference stored in whatever struct we are passing (for: copy-on-write refcounted cred objects would almost never have to be modified or copied and normally we would just pick the current creds reference from task_struct and assing it to a single field in whatever we pass instead of nameidata; against: might be conceptually simpler to have all that data directly in that struct and a helper filling it in). Which way we do that and which struct we end up passing is a very good question, but I want to make it very clear: having object creation/removal/ renaming methods[1] depend on which vfsmount we are dealing with is *wrong*. IOW, I strongly object against the use of nameidata here. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Aug 16, 2007, at 18:57:24, Linus Torvalds wrote: On Wed, 15 Aug 2007, David Howells wrote: Would you object greatly to functions like vfs_mkdir() gaining a security parameter? What I'm thinking of is this: int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode, struct security *security) I personally consider this an affront to everythign that is decent. Why the *hell* would mkdir() be so magical as to need something like that? Not speaking directly for David, but I believe the reason is for background kernel code which needs to do filesystem access during a thread's execution with *completely* different security context from that of the thread. Examples should be reasonably obvious; kNFSd is one, but it also includes anything where the kernel would poke directly into the filesystem, such as network filesystem cachefiles. Make it something sane, like a struct nameidata instead, and make it at least try to look like the path creation that is done by open (). Or create a struct file * or something. I can imagine having mkdir() being passed similar data as open () (ie lookup()), but I cannot _possibly_ imagine it ever being valid to pass in something totally made-up to just mkdir(), and nothing else. There's something fundamentally wrong there. I would offer the suggestion of using the described struct security in-place in the task struct, in place of using all those fields individually. That would be, in effect the default security context for any given task, if NULL is passed to the appropriate vfs function. For CacheFiles and kNFSd, they could each allocate their own during initialization or new-connection and pass that to any mkdir(), etc that they do on behalf of a given client. What makes mkdir() so magical? Also, what about all the other ops? Why is mkdir() special, but not mknod()? Why is mkdir() special, but not rmdir()? Really, none of this seems to make any sense unless you describe what is so magical about mkdir(). I think mkdir() was just an example he was using, probably because it was the first VFS call he needed to set a security context on. Next would come anything which CacheFiles or NFSd call on the underlying filesystem. Cheers, Kyle Moffett - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
--- David Howells [EMAIL PROTECTED] wrote:
Hi Linus, Al,
Would you object greatly to functions like vfs_mkdir() gaining a security
parameter?
Could you describe how this compares to the proposal that the
AppArmor developers suggested recently? I expect that we can
reduce the amount of discussion required, and maybe avoid some
confusion if you could do that.
Thank you.
What I'm thinking of is this:
int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode,
struct security *security)
Where the security context is the state of the context at the time the call
was issued:
struct security {
uid_t fsuid;
git_t fsgid;
struct group_info *group_info;
void*security;
struct key *session_keyring;
struct key *process_keyring;
struct key *thread_keyring;
And perhaps:
struct audit_context*audit_context;
seccomp_t seccomp;
};
This would, for the most part, be a temporary affair, being set up by such as
sys_mkdir()/sys_mkdirat() from data held in task_struct.
This information would then be passed into the filesystem and LSM layers so
that files, directories, etc. can be created, opened, deleted, or otherwise
mangled based on these security items, rather than the one in whichever
task_struct is current.
The reason for doing this would be to support an act-as interface, so that
services such as nfsd and cachefiles could act with different security
details
to the ones attached to the task. This would have a couple of potential
benefits:
(1) nfsd threads don't have to keep changing their security contexts.
(2) cachefiles can act on behalf of a process without changing its security
context.
Note that I/O operations such as read, write and ioctl would *not* be passed
this data as the file struct should contain the relevant security
information.
Similarly, page I/O operations would also not need alteration as the VMA
covering the region points to a file struct, which holds the appropriate
security.
David
Casey Schaufler
[EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
Casey Schaufler [EMAIL PROTECTED] wrote: Could you describe how this compares to the proposal that the AppArmor developers suggested recently? I expect that we can reduce the amount of discussion required, and maybe avoid some confusion if you could do that. I don't know what that is. David - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
