On 2013-07-29 16:36:25, Seth Arnold wrote:
> On Mon, Jul 29, 2013 at 02:24:15AM -0700, Tyler Hicks wrote:
> > Add an interface for trusted applications to use when they need to query
> > AppArmor kernel policy to determine if an action should be allowed.
> > 
> > This is a simplified interface that tries to make it as easy as possible
> > for applications to use. They provide a permissions mask and query
> > string and they get a pair of booleans back that let them know if the
> > action should be allowed and/or audited.
> > 
> > Signed-off-by: Tyler Hicks <[email protected]>
> 
> Acked-by: Seth Arnold <[email protected]>
> 
> with a few comments inline...
> 
> > ---
> >  libraries/libapparmor/src/Makefile.am         |  2 +-
> >  libraries/libapparmor/src/apparmor.h          | 41 +++++++++++
> >  libraries/libapparmor/src/kernel_interface.c  | 98 
> > +++++++++++++++++++++++++++
> >  libraries/libapparmor/src/libapparmor.map     |  7 ++
> >  libraries/libapparmor/swig/SWIG/libapparmor.i |  2 +
> >  5 files changed, 149 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libraries/libapparmor/src/Makefile.am 
> > b/libraries/libapparmor/src/Makefile.am
> > index 6507673..48bf2f4 100644
> > --- a/libraries/libapparmor/src/Makefile.am
> > +++ b/libraries/libapparmor/src/Makefile.am
> > @@ -47,7 +47,7 @@ lib_LTLIBRARIES = libapparmor.la
> >  noinst_HEADERS = grammar.h parser.h scanner.h af_protos.h
> >  
> >  libapparmor_la_SOURCES = grammar.y libaalogparse.c kernel_interface.c 
> > scanner.c
> > -libapparmor_la_LDFLAGS = -version-info 
> > $(AA_LIB_CURRENT):$(AA_LIB_REVISION):$(AA_LIB_AGE) -XCClinker -dynamic \
> > +libapparmor_la_LDFLAGS = -version-info 
> > $(AA_LIB_CURRENT):$(AA_LIB_REVISION):$(AA_LIB_AGE) -XCClinker -dynamic 
> > -pthread \
> >     -Wl,--version-script=$(top_srcdir)/src/libapparmor.map 
> > -Wl,-soname=libapparmor.so.1
> >  
> >  pkgconfigdir = $(libdir)/pkgconfig
> > diff --git a/libraries/libapparmor/src/apparmor.h 
> > b/libraries/libapparmor/src/apparmor.h
> > index 79bc69c..387e197 100644
> > --- a/libraries/libapparmor/src/apparmor.h
> > +++ b/libraries/libapparmor/src/apparmor.h
> > @@ -18,10 +18,41 @@
> >  #ifndef _SYS_APPARMOR_H_
> >  #define _SYS_APPARMOR_H    1
> >  
> > +#include <stdint.h>
> >  #include <sys/types.h>
> >  
> >  __BEGIN_DECLS
> >  
> > +/*
> > + * Class of mediation types in the AppArmor policy db
> > + */
> > +#define AA_CLASS_COND              0
> > +#define AA_CLASS_UNKNOWN   1
> > +#define AA_CLASS_FILE              2
> > +#define AA_CLASS_CAP               3
> > +#define AA_CLASS_NET               4
> > +#define AA_CLASS_RLIMITS   5
> > +#define AA_CLASS_DOMAIN            6
> > +#define AA_CLASS_MOUNT             7
> > +#define AA_CLASS_NS_DOMAIN 8
> > +#define AA_CLASS_PTRACE            9
> > +
> > +#define AA_CLASS_ENV               16
> > +
> > +#define AA_CLASS_DBUS              32
> > +#define AA_CLASS_X         33
> > +
> > +
> > +/* Permission Flags for Mediation classes */
> > +#define AA_MAY_WRITE               (1 << 1)
> > +#define AA_MAY_READ                (1 << 2)
> > +#define AA_MAY_BIND                (1 << 6)
> > +
> > +#define AA_DBUS_SEND               AA_MAY_WRITE
> > +#define AA_DBUS_RECEIVE            AA_MAY_READ
> > +#define AA_DBUS_BIND               AA_MAY_BIND
> > +
> > +
> >  /* Prototypes for apparmor state queries */
> >  extern int aa_is_enabled(void);
> >  extern int aa_find_mountpoint(char **mnt);
> > @@ -51,6 +82,16 @@ extern int aa_getcon(char **con, char **mode);
> >  extern int aa_getpeercon_raw(int fd, char *buf, int *len, char **mode);
> >  extern int aa_getpeercon(int fd, char **con, char **mode);
> >  
> > +/* A NUL character is used to separate the query command prefix string 
> > from the
> > + * rest of the query string. The query command sizes intentionally include 
> > the
> > + * NUL-terminator in their values.
> > + */
> > +#define AA_QUERY_CMD_LABEL         "label"
> > +#define AA_QUERY_CMD_LABEL_SIZE            sizeof(AA_QUERY_CMD_LABEL)
> > +
> > +extern int aa_query_label(uint32_t mask, char *query, size_t size, int 
> > *allow,
> > +                     int *audit);
> > +
> >  #define __macroarg_counter(Y...) __macroarg_count1 ( , ##Y)
> >  #define __macroarg_count1(Y...) __macroarg_count2 (Y, 
> > 16,15,14,13,12,11,10,9,8,7,6,5,4,3,2,1,0)
> >  #define 
> > __macroarg_count2(_,x0,x1,x2,x3,x4,x5,x6,x7,x8,x9,x10,x11,x12,x13,x14,x15,n,Y...)
> >  n
> > diff --git a/libraries/libapparmor/src/kernel_interface.c 
> > b/libraries/libapparmor/src/kernel_interface.c
> > index ea1659d..356307e 100644
> > --- a/libraries/libapparmor/src/kernel_interface.c
> > +++ b/libraries/libapparmor/src/kernel_interface.c
> > @@ -28,6 +28,9 @@
> >  #include <limits.h>
> >  #include <stdarg.h>
> >  #include <mntent.h>
> > +#include <inttypes.h>
> > +#include <stdint.h>
> 
> inttypes.h already includes stdint.h.

Ah, nice catch. I'll drop the stdint.h include in my local copy. (I'm
not going to send out a v2)

> 
> > +#include <pthread.h>
> >  
> >  #include "apparmor.h"
> >  
> > @@ -650,3 +653,98 @@ int aa_getpeercon(int fd, char **con, char **mode)
> >  
> >     return size;
> >  }
> > +
> > +static pthread_once_t aafs_access_control = PTHREAD_ONCE_INIT;
> > +static char *aafs_access = NULL;
> > +
> > +static void aafs_access_init_once(void)
> > +{
> > +   char *aafs;
> > +   int ret;
> > +
> > +   ret = aa_find_mountpoint(&aafs);
> > +   if (ret < 0)
> > +           return;
> > +
> > +   ret = asprintf(&aafs_access, "%s/.access", aafs);
> > +   if (ret < 0)
> > +           aafs_access = NULL;
> > +
> > +   free(aafs);
> > +}
> > +
> > +/* "allow 0x00000000\ndeny 0x00000000\naudit 0x00000000\nquiet 
> > 0x00000000\n" */
> > +#define QUERY_LABEL_REPLY_LEN      67
> > +
> > +/**
> > + * aa_query_label - query the access(es) of a label
> > + * @mask: permission bits to query
> > + * @query: binary query string, must be offset by AA_QUERY_CMD_LABEL_SIZE
> > + * @size: size of the query string must include AA_QUERY_CMD_LABEL_SIZE
> > + * @allowed: upon successful return, will be 1 if query is allowed and 0 
> > if not
> > + * @audited: upon successful return, will be 1 if query should be audited 
> > and 0
> > + *           if not
> > + *
> > + * Returns: 0 on success else -1 and sets errno
> > + */
> > +int aa_query_label(uint32_t mask, char *query, size_t size, int *allowed,
> > +              int *audited)
> > +{
> > +   char buf[QUERY_LABEL_REPLY_LEN];
> > +   uint32_t allow, deny, audit, quiet;
> > +   int fd, ret, saved;
> > +
> > +   if (!mask || size <= AA_QUERY_CMD_LABEL_SIZE) {
> > +           errno = EINVAL;
> > +           return -1;
> > +   }
> > +
> > +   ret = pthread_once(&aafs_access_control, aafs_access_init_once);
> > +   if (ret) {
> > +           errno = EINVAL;
> > +           return -1;
> > +   } else if (!aafs_access) {
> > +           errno = ENOMEM;
> > +           return -1;
> > +   }
> > +
> > +   fd = open(aafs_access, O_RDWR);
> > +   if (fd == -1)
> > +           return -1;
> > +
> > +   memcpy(query, AA_QUERY_CMD_LABEL, AA_QUERY_CMD_LABEL_SIZE);
> > +   errno = 0;
> > +   ret = write(fd, query, size);
> > +   if (ret != size) {
> > +           if (ret >= 0)
> > +                   errno = EPROTO;
> > +           return -1;
> > +   }
> > +
> > +   ret = read(fd, buf, QUERY_LABEL_REPLY_LEN);
> > +   saved = errno;
> > +   (void)close(fd);
> 
> This may not be enough to silence the new-fangled warnings.

It seems to be enough on Saucy, both with a simple make and with a PPA
build, which I assume has more hardening flags enabled.

> 
> > +   errno = saved;
> > +   if (ret != QUERY_LABEL_REPLY_LEN) {
> > +           if (ret >= 0)
> > +                   errno = EPROTO;
> > +           return -1;
> > +   }
> > +
> > +   ret = sscanf(buf, "allow 0x%8" SCNx32 "\n"
> > +                     "deny 0x%8"  SCNx32 "\n"
> > +                     "audit 0x%8" SCNx32 "\n"
> > +                     "quiet 0x%8" SCNx32 "\n",
> > +                &allow, &deny, &audit, &quiet);
> > +   if (ret != 4) {
> > +           errno = EPROTONOSUPPORT;
> > +           return -1;
> > +   }
> > +
> > +   *allowed = mask & ~(allow & ~deny) ? 0 : 1;
> > +   if (!(*allowed))
> > +           audit = 0xFFFFFFFF;
> > +   *audited = mask & ~(audit & ~quiet) ? 0 : 1;
> 
> I already broke my head thinking this bit through once -- and I came to
> the wrong conclusion last time :) -- so I hope this has the fix you
> found you needed last time through. :)

Luckily for the both of us, I kept the patch updated. :)

Thanks for the review!

Tyler

> 
> > +
> > +   return 0;
> > +}
> > diff --git a/libraries/libapparmor/src/libapparmor.map 
> > b/libraries/libapparmor/src/libapparmor.map
> > index c4fcf8f..df696a7 100644
> > --- a/libraries/libapparmor/src/libapparmor.map
> > +++ b/libraries/libapparmor/src/libapparmor.map
> > @@ -37,3 +37,10 @@ APPARMOR_1.1 {
> >    local:
> >     *;
> >  } APPARMOR_1.0;
> > +
> > +APPARMOR_3.0 {
> > +  global:
> > +   aa_query_label;
> > +  local:
> > +   *;
> > +} APPARMOR_1.1;
> > diff --git a/libraries/libapparmor/swig/SWIG/libapparmor.i 
> > b/libraries/libapparmor/swig/SWIG/libapparmor.i
> > index 1d3ca07..a94cb23 100644
> > --- a/libraries/libapparmor/swig/SWIG/libapparmor.i
> > +++ b/libraries/libapparmor/swig/SWIG/libapparmor.i
> > @@ -27,3 +27,5 @@ extern int aa_gettaskcon(pid_t target, char **con, char 
> > **mode);
> >  extern int aa_getcon(char **con, char **mode);
> >  extern int aa_getpeercon_raw(int fd, char *buf, int *len, char **mode);
> >  extern int aa_getpeercon(int fd, char **con, char **mode);
> > +extern int aa_query_label(uint32_t mask, char *query, size_t size, int 
> > *allow,
> > +                     int *audit);
> > -- 
> 
> Thanks!



> -- 
> AppArmor mailing list
> [email protected]
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to