[apparmor] [RFC v2] "features" directory for version/capability information
AppArmor: implement static feature reporting interface This adds the ability to query the internal versions of the various policy features of AppArmor. Signed-off-by: Kees Cook --- v2: - start using enum/union for display values. v1: - initial patch. --- diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 0848292..9f54625 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "include/apparmor.h" #include "include/apparmorfs.h" @@ -146,11 +147,104 @@ static const struct file_operations aa_fs_profile_remove = { static struct dentry *aa_fs_dentry __initdata; -static void __init aafs_remove(const char *name) +enum aa_fs_value { + AA_FS_STRING, + AA_FS_BOOLEAN, + AA_FS_U64, +}; + +struct aa_fs_file { + char *name; + enum aa_fs_value v_type; + union { + char *string; + unsigned long u64; + bool boolean; + } v; + const struct file_operations *file_ops; +}; + +struct aa_fs_dir { + const char *name; + struct dentry *dentry; + struct aa_fs_file *files; +}; + +static int aa_fs_seq_show(struct seq_file *seq, void *v) +{ + struct aa_fs_file *fs_file = seq->private; + + if (!fs_file) + return 0; + + switch (fs_file->v_type) { + case AA_FS_STRING: + seq_printf(seq, "%s\n", fs_file->v.string); + break; + case AA_FS_U64: + seq_printf(seq, "%#08lx\n", fs_file->v.u64); + break; + case AA_FS_BOOLEAN: + seq_printf(seq, "%s\n", fs_file->v.boolean ? + "yes" : "no"); + break; + } + + return 0; +} + +static int aa_fs_seq_open(struct inode *inode, struct file *file) +{ + return single_open(file, aa_fs_seq_show, inode->i_private); +} + +static const struct file_operations aa_fs_seq_file_ops = { + .owner = THIS_MODULE, + .open = aa_fs_seq_open, + .read = seq_read, + .llseek = seq_lseek, + .release= single_release, +}; + +#define AA_FS_STRING_FILE(name, value) \ + { name, \ + .v_type = AA_FS_STRING, .v.string = value, \ + &aa_fs_seq_file_ops } +#define AA_FS_BOOLEAN_FILE(name, value) \ + { name, \ + .v_type = AA_FS_BOOLEAN, .v.boolean = value, \ + &aa_fs_seq_file_ops } +#define AA_FS_U64_FILE(name, value) \ + { name, \ + .v_type = AA_FS_U64, .v.u64 = value, \ + &aa_fs_seq_file_ops } +static struct aa_fs_file aa_fs_features_files[] = { + AA_FS_BOOLEAN_FILE("change_hat",1), + AA_FS_BOOLEAN_FILE("change_hatv", 1), + AA_FS_BOOLEAN_FILE("change_onexec", 1), + AA_FS_BOOLEAN_FILE("change_profile",1), + AA_FS_BOOLEAN_FILE("namespaces",1), + AA_FS_BOOLEAN_FILE("network", 0), + AA_FS_STRING_FILE("file", "3.1"), + AA_FS_STRING_FILE("rlimit", "1.1"), + AA_FS_U64_FILE("capability",VFS_CAP_FLAGS_MASK), + { } +}; +#undef AA_FS_STRING_FILE + +static struct aa_fs_dir aa_fs_dirs[] = { + { "features", NULL, aa_fs_features_files }, + { } +}; + +static void __init aafs_remove(const char *name, struct dentry *dir_dentry) { struct dentry *dentry; - dentry = lookup_one_len(name, aa_fs_dentry, strlen(name)); + if (!dir_dentry) + return; + + dentry = lookup_one_len(name, dir_dentry, strlen(name)); if (!IS_ERR(dentry)) { securityfs_remove(dentry); dput(dentry); @@ -166,12 +260,14 @@ static void __init aafs_remove(const char *name) * Used aafs_remove to remove entries created with this fn. */ static int __init aafs_create(const char *name, int mask, + struct dentry *dir_dentry, + void *data, const struct file_operations *fops) { struct dentry *dentry; - dentry = securityfs_create_file(name, S_IFREG | mask, aa_fs_dentry, - NULL, fops); + dentry = securityfs_create_file(name, S_IFREG | mask, dir_dentry, + data, fops); return IS_ERR(dentry) ? PTR_ERR(dentry) : 0; } @@ -184,9 +280,21 @@ static int __init aafs_create(const char *name, int mask, void __init aa_destroy_aafs(void) { if (aa_fs_dentry) { - aafs_remove(".remove"); - aafs_remove(".replace"); - aafs_remove(".load"); + struct aa_fs_dir *fs_dir; + + aafs_remove(".remove", aa_fs_dentry); + aafs_remove(".replace", aa_fs_dentry); +
[apparmor] [RFC] "features" directory for version/capability information
Based on some of the initial design[1] discussions, here is a stab at the simple static portion of the new apparmorfs interface. [1] https://lists.ubuntu.com/archives/apparmor/2010-November/000491.html Signed-off-by: Kees Cook --- security/apparmor/apparmorfs.c | 114 + 1 file changed, 104 insertions(+), 10 deletions(-) --- diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 0848292..8990316 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -146,11 +146,67 @@ static const struct file_operations aa_fs_profile_remove = { static struct dentry *aa_fs_dentry __initdata; -static void __init aafs_remove(const char *name) +struct aa_fs_file { + char *name; + char *value; + const struct file_operations *file_ops; +}; + +struct aa_fs_dir { + const char *name; + struct dentry *dentry; + struct aa_fs_file *files; +}; + +static int aa_fs_seq_show(struct seq_file *seq, void *v) +{ + struct aa_fs_file *fs_file = seq->private; + + if (fs_file) + seq_printf(seq, "%s\n", fs_file->value); + + return 0; +} + +static int aa_fs_seq_open(struct inode *inode, struct file *file) +{ + return single_open(file, aa_fs_seq_show, inode->i_private); +} + +static const struct file_operations aa_fs_seq_file_ops = { + .owner = THIS_MODULE, + .open = aa_fs_seq_open, + .read = seq_read, + .llseek = seq_lseek, + .release= single_release, +}; + +static struct aa_fs_file aa_fs_features_files[] = { + { "capability", "2.0", &aa_fs_seq_file_ops }, + { "change_hat", "1.5", &aa_fs_seq_file_ops }, + { "change_hatv","1.0", &aa_fs_seq_file_ops }, + { "change_onexec", "1.0", &aa_fs_seq_file_ops }, + { "change_profile", "1.1", &aa_fs_seq_file_ops }, + { "file", "3.1", &aa_fs_seq_file_ops }, + { "namespaces", "1.1", &aa_fs_seq_file_ops }, + { "network","1.0", &aa_fs_seq_file_ops }, + { "rlimit", "1.1", &aa_fs_seq_file_ops }, + { NULL, NULL, NULL }, +}; + +static struct aa_fs_dir aa_fs_dirs[] = { + { "features", NULL, aa_fs_features_files }, + { NULL, NULL, NULL } +}; + +static void __init aafs_remove(const char *name, struct dentry *dir_dentry) { struct dentry *dentry; - dentry = lookup_one_len(name, aa_fs_dentry, strlen(name)); + if (!dir_dentry) + return; + + dentry = lookup_one_len(name, dir_dentry, strlen(name)); if (!IS_ERR(dentry)) { securityfs_remove(dentry); dput(dentry); @@ -166,12 +222,14 @@ static void __init aafs_remove(const char *name) * Used aafs_remove to remove entries created with this fn. */ static int __init aafs_create(const char *name, int mask, + struct dentry *dir_dentry, + void *data, const struct file_operations *fops) { struct dentry *dentry; - dentry = securityfs_create_file(name, S_IFREG | mask, aa_fs_dentry, - NULL, fops); + dentry = securityfs_create_file(name, S_IFREG | mask, dir_dentry, + data, fops); return IS_ERR(dentry) ? PTR_ERR(dentry) : 0; } @@ -184,9 +242,21 @@ static int __init aafs_create(const char *name, int mask, void __init aa_destroy_aafs(void) { if (aa_fs_dentry) { - aafs_remove(".remove"); - aafs_remove(".replace"); - aafs_remove(".load"); + struct aa_fs_dir *fs_dir; + + aafs_remove(".remove", aa_fs_dentry); + aafs_remove(".replace", aa_fs_dentry); + aafs_remove(".load", aa_fs_dentry); + + for (fs_dir = aa_fs_dirs; fs_dir->name; ++fs_dir) { + struct aa_fs_file *fs_file; + + for (fs_file = fs_dir->files; fs_file->name; ++fs_file) + aafs_remove(fs_file->name, fs_dir->dentry); + + aafs_remove(fs_dir->name, aa_fs_dentry); + fs_dir->dentry = NULL; + } securityfs_remove(aa_fs_dentry); aa_fs_dentry = NULL; @@ -203,6 +273,7 @@ void __init aa_destroy_aafs(void) int __init aa_create_aafs(void) { int error; + struct aa_fs_dir *fs_dir; if (!apparmor_initialized) return 0; @@ -219,16 +290,39 @@ int __init aa_create_aafs(void) goto error; } - error = aafs_create(".load", 0640, &aa_fs_profile_load); + error = aafs_create(".load", 0640, aa_fs_dentry, NULL, + &aa_fs_profile_load); if (error) goto erro
Re: [apparmor] [patch] split off apache permissions to abstractions/apache2-common
On 12/29/2011 09:50 AM, Christian Boltz wrote: Hello, Am Mittwoch, 28. Dezember 2011 schrieb John Johansen: On 12/21/2011 04:17 PM, Christian Boltz wrote: Note: My version of abstractions/apache2-common does not allow to read /.htaccess (I changed /**.htaccess -> /**/.htaccess) which slightly reduces permissions for ^HANDLING_UNTRUSTED_INPUT. However I doubt someone has a .htaccess in / ;-) Ugh, tbh I don't even like /**/.htaccess can we perhaps add a tunable for this, even if the base value used is just /**/ Basically I really don't like letting .htaccess reside just about anywhere, and maybe a tunable would make this more palatable I agree that .htaccess everywhere doesn't really make sense, and sane apache configurations have "AllowOverride none" for / and only allow AllowOverride (aka using a .htaccess file) in the docroot. Nevertheless, there is a big problem - if apache finds a .htaccess file and can't read it (after chmod 000 or because AppArmor blocks it), you get a nice log message: [Thu Dec 29 18:34:41 2011] [crit] [client 127.0.0.1] (13)Permission denied: /home/cb/public_html/.htaccess pcfg_openfile: unable to check htaccess file, ensure it is readable The real problem is how apache handles this situation - basically it assumes a "deny from all". This is of course the safe way (better than data disclosure or unauthorized access to $whatever), but it blocks everything inside $directory_with_unreadable_.htaccess. OTOH, a .htaccess file doesn't contain really secret content IMHO, so I don't see /**/.htaccess as a real problem. hrmmm, okay I guess I am okay with this then you can put it in both dev and the 2.7 branches -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] smbd - /etc/netgroup
On 12/29/2011 08:38 AM, Christian Boltz wrote: Hello, Am Mittwoch, 28. Dezember 2011 schrieb John Johansen: On 12/21/2011 10:06 AM, Christian Boltz wrote: smbd needs read access to /etc/netgroup. References: https://bugzilla.novell.com/show_bug.cgi?id=738041 I propose the patch for trunk and the 2.7 branch. ^ === modified file 'profiles/apparmor.d/usr.sbin.smbd' --- profiles/apparmor.d/usr.sbin.smbd 2011-11-01 17:28:49 + +++ profiles/apparmor.d/usr.sbin.smbd 2011-12-21 17:52:32 + @@ -21,6 +21,7 @@ capability sys_tty_config, /etc/mtab r, + /etc/netgroup r, /etc/printcap r, /proc/*/mounts r, /proc/sys/kernel/core_pattern r, This looks reasonable to me Acked-by: John Johansen Thanks, commited. What about backporting[1] this the 2.7 branch? ;-) sorry yes, if you want you can check it in to the 2.7 branch, or if you'd rather I will. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] split off apache permissions to abstractions/apache2-common
Hello, Am Mittwoch, 28. Dezember 2011 schrieb John Johansen: > On 12/21/2011 04:17 PM, Christian Boltz wrote: > > Note: My version of abstractions/apache2-common does not allow to > > read /.htaccess (I changed /**.htaccess -> /**/.htaccess) which > > slightly reduces permissions for ^HANDLING_UNTRUSTED_INPUT. > > However I doubt someone has a .htaccess in / ;-) > > Ugh, tbh I don't even like /**/.htaccess can we perhaps add a tunable > for this, even if the base value used is just /**/ > > Basically I really don't like letting .htaccess reside just about > anywhere, and maybe a tunable would make this more palatable I agree that .htaccess everywhere doesn't really make sense, and sane apache configurations have "AllowOverride none" for / and only allow AllowOverride (aka using a .htaccess file) in the docroot. Nevertheless, there is a big problem - if apache finds a .htaccess file and can't read it (after chmod 000 or because AppArmor blocks it), you get a nice log message: [Thu Dec 29 18:34:41 2011] [crit] [client 127.0.0.1] (13)Permission denied: /home/cb/public_html/.htaccess pcfg_openfile: unable to check htaccess file, ensure it is readable The real problem is how apache handles this situation - basically it assumes a "deny from all". This is of course the safe way (better than data disclosure or unauthorized access to $whatever), but it blocks everything inside $directory_with_unreadable_.htaccess. OTOH, a .htaccess file doesn't contain really secret content IMHO, so I don't see /**/.htaccess as a real problem. Regards, Christian Boltz -- Arroganz? Klar, ein pikfeines Restaurant mit Kleiderordnung ist aus arrogant, Du wirst nicht gezwungen dort essen zu gehen, wenn Du keine Krawatte tragen willst. Das ist mit dieser Liste ganz genauso: Willst Du hier teilnehmen? Dann bind Dir die Krawatte um... [Andreas Loesch in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] smbd - /etc/netgroup
Hello, Am Mittwoch, 28. Dezember 2011 schrieb John Johansen: > On 12/21/2011 10:06 AM, Christian Boltz wrote: > > smbd needs read access to /etc/netgroup. > > > > References: https://bugzilla.novell.com/show_bug.cgi?id=738041 > > I propose the patch for trunk and the 2.7 branch. ^ > > === modified file 'profiles/apparmor.d/usr.sbin.smbd' > > --- profiles/apparmor.d/usr.sbin.smbd 2011-11-01 17:28:49 + > > +++ profiles/apparmor.d/usr.sbin.smbd 2011-12-21 17:52:32 + > > @@ -21,6 +21,7 @@ > > > > capability sys_tty_config, > > > > /etc/mtab r, > > > > + /etc/netgroup r, > > > > /etc/printcap r, > > /proc/*/mounts r, > > /proc/sys/kernel/core_pattern r, > This looks reasonable to me > > Acked-by: John Johansen Thanks, commited. What about backporting[1] this the 2.7 branch? ;-) Regards, Christian Boltz [1] I know that's a big word for a one-line addition ;-) -- Meine allerste Festplatte hatte 30 MB, und ich war der King, weil alle anderen 20 MB hatten. Sie fragten, was ich mit 30 MB wolle, die bekomme ich doch nie voll. ;) Meine jetzige Graphikkarte hat mehr. ;)) [Bernd Brodesser in suse-linux] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor