[apparmor] [RFC v2] "features" directory for version/capability information

2011-12-29 Thread Kees Cook
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

2011-12-29 Thread Kees Cook
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

2011-12-29 Thread John Johansen

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

2011-12-29 Thread John Johansen

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

2011-12-29 Thread Christian Boltz
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

2011-12-29 Thread Christian Boltz
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