On Fri, Feb 10, 2017 at 12:56:16PM -0800, John Johansen wrote:
> Revise the dconf patch so that the returned paths will have the
> correct permissions when taking into account stacking and delegation.
> 
> This requires doing a permission query per path, (or building the
> library infrastructure to handle permission changes due to stacking
> or delegation). This is done once while building the list and will
> remain good until policy is changed.
> 
> Signed-off-by: John Johansen <john.johan...@canonical.com>

Acked-by: Seth Arnold <seth.arn...@canonical.com>

with the caveat that I'd really rather see cmalloc() used instead of
malloc() for r_paths and friends.

Thanks

> ---
>  libraries/libapparmor/include/sys/apparmor.h       |  30 +--
>  libraries/libapparmor/src/Makefile.am              |   4 +
>  libraries/libapparmor/src/kernel.c                 | 205 
> +++++++++++++++------
>  libraries/libapparmor/src/tst_dconf.c              |  59 ++++++
>  parser/dconf.cc                                    |  75 ++++----
>  parser/dconf.h                                     |   8 +-
>  parser/parser_lex.l                                |  10 +-
>  parser/parser_yacc.y                               |  33 +++-
>  parser/tst/equality.sh                             |  16 ++
>  parser/tst/simple_tests/dconf/bad_path_01.sd       |   4 +-
>  parser/tst/simple_tests/dconf/bad_path_02.sd       |   4 +-
>  parser/tst/simple_tests/dconf/bad_path_03.sd       |   2 +-
>  parser/tst/simple_tests/dconf/bad_path_04.sd       |   2 +-
>  parser/tst/simple_tests/dconf/bad_path_05.sd       |   8 +
>  parser/tst/simple_tests/dconf/bad_path_06.sd       |   8 +
>  parser/tst/simple_tests/dconf/bad_path_07.sd       |   8 +
>  parser/tst/simple_tests/dconf/bad_path_08.sd       |   8 +
>  parser/tst/simple_tests/dconf/bad_path_09.sd       |   8 +
>  parser/tst/simple_tests/dconf/ok_audit_01.sd       |   3 +-
>  parser/tst/simple_tests/dconf/ok_audit_02.sd       |   8 +
>  parser/tst/simple_tests/dconf/ok_dir_01.sd         |   2 +-
>  parser/tst/simple_tests/dconf/ok_dir_02.sd         |   8 +
>  .../tst/simple_tests/dconf/ok_escapedrechars_01.sd |   8 +
>  .../tst/simple_tests/dconf/ok_escapedrechars_02.sd |   8 +
>  .../tst/simple_tests/dconf/ok_escapedrechars_03.sd |   8 +
>  .../tst/simple_tests/dconf/ok_escapedrechars_04.sd |   8 +
>  .../tst/simple_tests/dconf/ok_escapedrechars_05.sd |   8 +
>  parser/tst/simple_tests/dconf/ok_key_01.sd         |   2 +-
>  28 files changed, 413 insertions(+), 142 deletions(-)
>  create mode 100644 libraries/libapparmor/src/tst_dconf.c
>  create mode 100644 parser/tst/simple_tests/dconf/bad_path_05.sd
>  create mode 100644 parser/tst/simple_tests/dconf/bad_path_06.sd
>  create mode 100644 parser/tst/simple_tests/dconf/bad_path_07.sd
>  create mode 100644 parser/tst/simple_tests/dconf/bad_path_08.sd
>  create mode 100644 parser/tst/simple_tests/dconf/bad_path_09.sd
>  create mode 100644 parser/tst/simple_tests/dconf/ok_audit_02.sd
>  create mode 100644 parser/tst/simple_tests/dconf/ok_dir_02.sd
>  create mode 100644 parser/tst/simple_tests/dconf/ok_escapedrechars_01.sd
>  create mode 100644 parser/tst/simple_tests/dconf/ok_escapedrechars_02.sd
>  create mode 100644 parser/tst/simple_tests/dconf/ok_escapedrechars_03.sd
>  create mode 100644 parser/tst/simple_tests/dconf/ok_escapedrechars_04.sd
>  create mode 100644 parser/tst/simple_tests/dconf/ok_escapedrechars_05.sd
> 
> diff --git a/libraries/libapparmor/include/sys/apparmor.h 
> b/libraries/libapparmor/include/sys/apparmor.h
> index 6f79eff..e94725e 100644
> --- a/libraries/libapparmor/include/sys/apparmor.h
> +++ b/libraries/libapparmor/include/sys/apparmor.h
> @@ -63,9 +63,9 @@ __BEGIN_DECLS
>  
>  
>  /* Permission flags for the AA_CLASS_DCONF mediation class */
> -#define AA_DCONF_READ                        (1 << 2)
> -#define AA_DCONF_WRITE                       (1 << 1)
> -#define AA_DCONF_READWRITE           ((AA_DCONF_READ) | (AA_DCONF_WRITE))
> +#define AA_DCONF_READ                        (AA_MAY_READ)
> +#define AA_DCONF_WRITE                       (AA_MAY_WRITE)
> +#define AA_VALID_DCONF_PERMS         ((AA_DCONF_READ) | (AA_DCONF_WRITE))
>  
>  
>  /* Prototypes for apparmor state queries */
> @@ -143,22 +143,24 @@ typedef struct {
>       aa_label_data_ent *ents;        /* free vec of entries */
>  } aa_label_data_info;
>  
> -typedef struct {
> -     char *data;                     /* free data */
> -     size_t rn;                      /* number of rpaths */
> -     size_t rwn;                     /* number of rwpaths */
> -     size_t arn;                     /* number of arpaths */
> -     size_t arwn;                    /* number of arwpaths */
> -     const char **rpaths;            /* read-only paths in data */
> -     const char **rwpaths;           /* read-write paths in data */
> -     const char **arpaths;           /* audit read-only paths in data */
> -     const char **arwpaths;          /* audit read-write paths in data */
> -} aa_dconf_info;
>  
>  extern int aa_query_label_data(const char *label, const char *key,
>                              aa_label_data_info *out);
>  extern void aa_clear_label_data(aa_label_data_info *info);
>  
> +
> +typedef struct {
> +     char *data;                     /* free data */
> +     size_t r_n;                     /* number of r_paths */
> +     size_t rw_n;                    /* number of rw_paths */
> +     size_t ar_n;                    /* number of ar_paths */
> +     size_t arw_n;                   /* number of arw_paths */
> +     const char **r_paths;           /* read-only paths in data */
> +     const char **rw_paths;          /* read-write paths in data */
> +     const char **ar_paths;          /* audit read-only paths in data */
> +     const char **arw_paths;         /* audit read-write paths in data */
> +} aa_dconf_info;
> +
>  extern int aa_query_dconf_info(const char *label, aa_dconf_info *info);
>  extern void aa_clear_dconf_info(aa_dconf_info *info);
>  
> diff --git a/libraries/libapparmor/src/Makefile.am 
> b/libraries/libapparmor/src/Makefile.am
> index dd393a9..817510f 100644
> --- a/libraries/libapparmor/src/Makefile.am
> +++ b/libraries/libapparmor/src/Makefile.am
> @@ -71,6 +71,10 @@ tst_kernel_SOURCES = tst_kernel.c
>  tst_kernel_LDADD = .libs/libapparmor.a
>  tst_kernel_LDFLAGS = -pthread
>  
> +tst_dconf_SOURCES = tst_dconf.c
> +tst_dconf_LDADD = .libs/libapparmor.a
> +tst_dconf_LDFLAGS = -pthread
> +
>  check_PROGRAMS = tst_aalogmisc tst_features tst_kernel
>  TESTS = $(check_PROGRAMS)
>  
> diff --git a/libraries/libapparmor/src/kernel.c 
> b/libraries/libapparmor/src/kernel.c
> index 7aa665d..2b94689 100644
> --- a/libraries/libapparmor/src/kernel.c
> +++ b/libraries/libapparmor/src/kernel.c
> @@ -1119,8 +1119,8 @@ static ssize_t query_dconf_setup(char **query, const 
> char *label, size_t label_l
>               return -1;
>       memcpy(*query + AA_QUERY_CMD_LABEL_SIZE, label, label_len);
>       /* null separator */
> -     *query[AA_QUERY_CMD_LABEL_SIZE + label_len] = 0;
> -     *query[AA_QUERY_CMD_LABEL_SIZE + label_len + 1] = AA_CLASS_DCONF;
> +     (*query)[AA_QUERY_CMD_LABEL_SIZE + label_len] = 0;
> +     (*query)[AA_QUERY_CMD_LABEL_SIZE + label_len + 1] = AA_CLASS_DCONF;
>       memcpy(*query + AA_QUERY_CMD_LABEL_SIZE + label_len + 2, path, 
> path_len);
>  
>       return size;
> @@ -1321,6 +1321,35 @@ void aa_clear_label_data(aa_label_data_info *info)
>       memset(info, 0, sizeof(*info));
>  }
>  
> +static uint32_t unpack32(const char *p)
> +{
> +     uint32_t tmp;
> +     memcpy(&tmp, p, sizeof(tmp));
> +     return le32toh(tmp);
> +}
> +
> +static int strcmp_cb(const void *p1, const void *p2)
> +{
> +           return strcmp(* (char * const *) p1, * (char * const *) p2);
> +}
> +
> +static int unique(const char **vec, size_t n)
> +{
> +     size_t i, dups = 0, pos = 0;
> +
> +     for (i = 1; i < n; i++) {
> +             if (strcmp(vec[pos], vec[i]) == 0) {
> +                     dups++;
> +                     continue;
> +             }
> +             pos++;
> +             if (dups)
> +                     vec[pos] = vec[i];
> +     }
> +
> +     return dups;
> +}
> +
>  /**
>   * aa_query_dconf_info - query dconf info associated with @label
>   * @label: label of the confinement context
> @@ -1332,72 +1361,128 @@ void aa_clear_label_data(aa_label_data_info *info)
>   */
>  int aa_query_dconf_info(const char *label, aa_dconf_info *info)
>  {
> -     aa_label_data_info data_info;
> +     aa_label_data_info raw;
>       const char *p;
> -     uint32_t tmp;
> -     int i;
> +     int i, n, res = -1;
>  
>       memset(info, 0, sizeof (*info));
> -
> -     if (aa_query_label_data(label, "dconf", &data_info))
> -             return -1;
> -
> -     if (data_info.n < 1)
> +     if (aa_query_label_data(label, "dconf", &raw))
>               return -1;
>  
> -     info->data = data_info.data;
> -     p = data_info.ents[0].entry;
> -
> -     memcpy(&tmp, p, sizeof(tmp));
> -     p += sizeof(tmp);
> -     info->rn = le32toh(tmp);
> -     memcpy(&tmp, p, sizeof(tmp));
> -     p += sizeof(tmp);
> -     info->rwn = le32toh(tmp);
> -     memcpy(&tmp, p, sizeof(tmp));
> -     p += sizeof(tmp);
> -     info->arn = le32toh(tmp);
> -     memcpy(&tmp, p, sizeof(tmp));
> -     p += sizeof(tmp);
> -     info->arwn = le32toh(tmp);
> -
> -     info->rpaths = malloc(info->rn * sizeof(*info->rpaths));
> -     info->rwpaths = malloc(info->rwn * sizeof(*info->rwpaths));
> -     info->arpaths = malloc(info->arn * sizeof(*info->arpaths));
> -     info->arwpaths = malloc(info->arwn * sizeof(*info->arwpaths));
> -
> -     for (i = 0; i < info->rn; i++) {
> -             memcpy(&tmp, p, sizeof(tmp));
> -             p += sizeof(tmp);
> -             info->rpaths[i] = p;
> -             p += le32toh(tmp);
> +     /* Find upper bound on how many paths there are, one data ent per
> +      * profile, each profile can have multiple paths and some maybe
> +      * the same.
> +      *
> +      * Outer loop, for each profile in label
> +      */
> +     for (n = i = 0; i < raw.n; i++) {
> +             /* find dconf ents within data blob.
> +              * <uint32_t><string><\0>
> +              */
> +             p = raw.ents[i].entry;
> +             while (p < raw.ents[i].entry + raw.ents[i].size) {
> +                     uint32_t size = unpack32(p);
> +                     p += size + sizeof(size);
> +                     /* must be null terminated */
> +                     if (*(p-1) != 0)
> +                             goto eproto;
> +                     n++;
> +             }
> +             /* verify set of strings is exactly the size of the data */
> +             if (p - raw.ents[i].entry != raw.ents[i].size)
> +                     goto erange;
>       }
>  
> -     for (i = 0; i < info->rwn; i++) {
> -             memcpy(&tmp, p, sizeof(tmp));
> -             p += sizeof(tmp);
> -             info->rwpaths[i] = p;
> -             p += le32toh(tmp);
> +     /* setups work array for paths */
> +     {
> +             const char *vec[n];
> +             aa_perms_t perms[n];
> +             int j;
> +
> +             for (i = j = 0; i < raw.n; i++) {
> +                     p = raw.ents[i].entry;
> +                     while (p < raw.ents[i].entry + raw.ents[i].size) {
> +                             uint32_t size = unpack32(p);
> +                             vec[j++] = p + sizeof(size);
> +                             p += size + sizeof(size);
> +                     }
> +             }
> +             qsort(vec, n, sizeof(const char *), strcmp_cb);
> +             n -= unique(vec, n);
> +             for (i = 0; i < n; i++) {
> +                     /* do a query to get the compound label
> +                      * perms permissions for an entry
> +                      */
> +                     if (aa_query_dconf_raw(label, strlen(label), vec[i],
> +                                            strlen(vec[i]), &perms[i]))
> +                                     goto out;
> +                     perms[i].allow &= ~perms[i].deny;
> +                     if (perms[i].allow & AA_MAY_WRITE) {
> +                             if (perms[i].audit & AA_MAY_WRITE)
> +                                     info->arw_n++;
> +                             else
> +                                     info->rw_n++;
> +                     } else if (perms[i].allow & AA_MAY_READ) {
> +                             if (perms[i].audit & AA_MAY_READ)
> +                                     info->ar_n++;
> +                             else
> +                                     info->r_n++;
> +                     } else
> +                             goto eproto;
> +             }
> +             
> +             /* allocate vecs that point into data */
> +             info->r_paths = malloc(info->r_n * sizeof(const char *));
> +             if (!info->r_paths)
> +                     goto nomem;
> +             info->rw_paths = malloc(info->rw_n * sizeof(const char *));
> +             if (!info->rw_paths)
> +                     goto nomem;
> +             info->ar_paths = malloc(info->ar_n * sizeof(const char *));
> +             if (!info->ar_paths)
> +                     goto nomem;
> +             info->arw_paths = malloc(info->arw_n * sizeof(const char *));
> +             if (!info->arw_paths)
> +                     goto nomem;
> +
> +             /* now setup the data arrays */
> +             size_t r_n = 0, rw_n = 0, ar_n = 0, arw_n = 0;
> +             for (i = 0; i < n; i++) {
> +                     if (perms[i].allow & AA_MAY_WRITE) {
> +                             if (perms[i].audit & AA_MAY_WRITE)
> +                                     info->arw_paths[arw_n++] = vec[i];
> +                             else
> +                                     info->rw_paths[rw_n++] = vec[i];
> +                     } else if (perms[i].allow & AA_MAY_READ) {
> +                             if (perms[i].audit & AA_MAY_READ)
> +                                     info->ar_paths[ar_n++] = vec[i];
> +                             else
> +                                     info->r_paths[r_n++] = vec[i];
> +                     }
> +             }
>       }
>  
> -     for (i = 0; i < info->arn; i++) {
> -             memcpy(&tmp, p, sizeof(tmp));
> -             p += sizeof(tmp);
> -             info->arpaths[i] = p;
> -             p += le32toh(tmp);
> -     }
> +     /* transfer raw data blob */
> +     info->data = raw.data;
> +     raw.data = NULL;
> +     res = 0;
> +out:
> +     aa_clear_label_data(&raw);
>  
> -     for (i = 0; i < info->arwn; i++) {
> -             memcpy(&tmp, p, sizeof(tmp));
> -             p += sizeof(tmp);
> -             info->arwpaths[i] = p;
> -             p += le32toh(tmp);
> -     }
> +     return res;
>  
> -     data_info.data = NULL;
> -     aa_clear_label_data(&data_info);
> +eproto:
> +     errno = EPROTO;
> +     goto out;
>  
> -     return 0;
> +erange:
> +     errno = ERANGE;
> +     goto out;
> +
> +nomem:
> +     aa_clear_dconf_info(info);
> +     errno = ENOMEM;
> +     goto out;
>  }
>  
>  /**
> @@ -1408,10 +1493,10 @@ int aa_query_dconf_info(const char *label, 
> aa_dconf_info *info)
>   */
>  void aa_clear_dconf_info(aa_dconf_info *info)
>  {
> -     free(info->arwpaths);
> -     free(info->arpaths);
> -     free(info->rwpaths);
> -     free(info->rpaths);
> +     free(info->r_paths);
> +     free(info->ar_paths);
> +     free(info->rw_paths);
> +     free(info->arw_paths);
>       free(info->data);
>  
>       memset(info, 0, sizeof(*info));
> diff --git a/libraries/libapparmor/src/tst_dconf.c 
> b/libraries/libapparmor/src/tst_dconf.c
> new file mode 100644
> index 0000000..97c332c
> --- /dev/null
> +++ b/libraries/libapparmor/src/tst_dconf.c
> @@ -0,0 +1,59 @@
> +/*
> + *   Copyright (c) 2017
> + *   Canonical, Ltd. (All rights reserved)
> + *
> + *   This program is free software; you can redistribute it and/or
> + *   modify it under the terms of version 2 of the GNU General Public
> + *   License published by the Free Software Foundation.
> + *
> + *   This program is distributed in the hope that it will be useful,
> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *   GNU General Public License for more details.
> + *
> + *   You should have received a copy of the GNU General Public License
> + *   along with this program; if not, contact Novell, Inc. or Canonical
> + *   Ltd.
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include "kernel.c"
> +
> +void print_strings(const char *msg, const char **paths, size_t n)
> +{
> +     size_t i;
> +
> +     printf("%s: %lu entries\n", msg, n);
> +     for (i = 0; i < n; i++)
> +             printf("\t%s\n", paths[i]);
> +}
> +
> +void usage(const char *prog) {
> +     fprintf(stderr, "Usage\n  %s <profile>\n", prog);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +     aa_dconf_info info;
> +     int retval;
> +
> +     if (argc < 2) {
> +             usage(argv[0]);
> +             exit(1);
> +     }
> +
> +     retval = aa_query_dconf_info(argv[1], &info);
> +     if (retval == -1) {
> +             fprintf(stderr, "query failed: %s\n", strerror(errno));
> +             exit(1);
> +     }
> +
> +     print_strings("read paths", info.r_paths, info.r_n);
> +     print_strings("audit read paths", info.ar_paths, info.ar_n);
> +     print_strings("read/write paths", info.rw_paths, info.rw_n);
> +     print_strings("audit read/write paths", info.arw_paths, info.arw_n);
> +
> +     return 0;
> +}
> diff --git a/parser/dconf.cc b/parser/dconf.cc
> index 669bf80..a673791 100644
> --- a/parser/dconf.cc
> +++ b/parser/dconf.cc
> @@ -24,32 +24,12 @@
>  #include <sstream>
>  #include <iomanip>
>  
> -dconfDataEnt::~dconfDataEnt()
> -{
> -}
>  
>  void dconfDataEnt::serialize(std::ostringstream &buf)
>  {
>       ostringstream tmp;
>  
> -     sd_write32(tmp, rpaths.size());
> -     sd_write32(tmp, rwpaths.size());
> -     sd_write32(tmp, arpaths.size());
> -     sd_write32(tmp, arwpaths.size());
> -
> -     for (set<std::string>::iterator i = rpaths.begin(); i != rpaths.end(); 
> i++) {
> -             sd_write32(tmp, i->size() + 1);
> -             tmp.write(i->c_str(), i->size() + 1);
> -     }
> -     for (set<std::string>::iterator i = rwpaths.begin(); i != 
> rwpaths.end(); i++) {
> -             sd_write32(tmp, i->size() + 1);
> -             tmp.write(i->c_str(), i->size() + 1);
> -     }
> -     for (set<std::string>::iterator i = arpaths.begin(); i != 
> arpaths.end(); i++) {
> -             sd_write32(tmp, i->size() + 1);
> -             tmp.write(i->c_str(), i->size() + 1);
> -     }
> -     for (set<std::string>::iterator i = arwpaths.begin(); i != 
> arwpaths.end(); i++) {
> +     for (set<std::string>::iterator i = paths.begin(); i != paths.end(); 
> i++) {
>               sd_write32(tmp, i->size() + 1);
>               tmp.write(i->c_str(), i->size() + 1);
>       }
> @@ -75,7 +55,7 @@ std::ostream &dconf_rule::dump(std::ostream &os)
>  
>       os << "dconf (";
>  
> -     switch (mode & AA_DCONF_READWRITE) {
> +     switch (mode & AA_VALID_DCONF_PERMS) {
>       case AA_DCONF_READ:
>               os << "read";
>               break;
> @@ -90,6 +70,18 @@ std::ostream &dconf_rule::dump(std::ostream &os)
>  
>  int dconf_rule::expand_variables(void)
>  {
> +     const char *pstr;
> +     int error = expand_entry_variables(&path);
> +     if (error)
> +             return error;
> +     /* do additional semantic checks. TODO these should have their own hook 
> */
> +     if (path[0] != '/')
> +             return -1;
> +     for (pstr = path; *pstr; pstr++) {
> +             if (*pstr == '/' && *(pstr + 1) == '/')
> +                     return -1;
> +     }
> +
>       return 0;
>  }
>  
> @@ -108,37 +100,40 @@ void dconf_rule::gen_data(Profile &prof)
>                       return;
>       }
>  
> -     if (mode & AA_DCONF_WRITE) {
> -             if (audit)
> -                     dconf->arwpaths.insert(path);
> -             else
> -                     dconf->rwpaths.insert(path);
> -     } else {
> -             if (audit)
> -                     dconf->arpaths.insert(path);
> -             else
> -                     dconf->rpaths.insert(path);
> -     }
> +     dconf->paths.insert(path);
>  }
>  
>  int dconf_rule::gen_policy_re(Profile &prof)
>  {
> +     std::string buf;
>       std::ostringstream rule;
> +     pattern_t ptype;
> +     int pos;
>  
> -     if ((mode & AA_DCONF_READWRITE) == 0)
> +     /* don't generate policy for rules without permissions */
> +     if ((mode & AA_VALID_DCONF_PERMS) == 0)
>               return RULE_OK;
>  
> -     rule << "\\x" << std::setfill('0') << std::setw(2) << std::hex << 
> AA_CLASS_DCONF;
> +     ptype = convert_aaregex_to_pcre(path, 0, glob_default, buf, &pos);
> +     if (ptype == ePatternInvalid)
> +             return RULE_ERROR;
> +     else if (ptype != ePatternBasic) {
> +             cerr << "pattern matching not supported in dconf rules";
> +             return RULE_ERROR;
> +     }
>  
> -     if (path[strlen(path) - 1] == '/')
> -             rule << path << "[^\\x00]*";
> +     rule << "\\x" << std::setfill('0') << std::setw(2) << std::hex << 
> AA_CLASS_DCONF;
> +     
> +     /* dconf directories are converted to tail globs for the dfa */
> +     if (buf[buf.length() - 1] == '/')
> +             rule << buf << "[^\\x00]*";
>       else
> -             rule << path;
> +             rule << buf;
>  
>       if (!prof.policy.rules->add_rule(rule.str().c_str(),
>                                        0,
> -                                      mode & AA_DCONF_READWRITE,
> -                                      audit ? mode & AA_DCONF_READWRITE : 0,
> +                                      mode & AA_VALID_DCONF_PERMS,
> +                                      audit ? mode & AA_VALID_DCONF_PERMS : 
> 0,
>                                        dfaflags))
>               return RULE_ERROR;
>  
> diff --git a/parser/dconf.h b/parser/dconf.h
> index 0477f06..c8c72c5 100644
> --- a/parser/dconf.h
> +++ b/parser/dconf.h
> @@ -24,12 +24,8 @@
>  
>  class dconfDataEnt: public DataEnt {
>  public:
> -     set<std::string> rpaths;
> -     set<std::string> rwpaths;
> -     set<std::string> arpaths;
> -     set<std::string> arwpaths;
> -
> -     virtual ~dconfDataEnt();
> +     set<std::string> paths;
> +     virtual ~dconfDataEnt() { };
>  
>       void serialize(std::ostringstream &buf);
>  };
> diff --git a/parser/parser_lex.l b/parser/parser_lex.l
> index 82cd84c..7b95d97 100644
> --- a/parser/parser_lex.l
> +++ b/parser/parser_lex.l
> @@ -215,6 +215,7 @@ ID_NOEQ           {ID_CHARS_NOEQ}|(,{ID_CHARS_NOEQ})
>  IDS_NOEQ     {LEADING_ID_CHARS_NOEQ}{ID_NOEQ}*
>  ALLOWED_QUOTED_ID    [^\0"]|\\\"
>  QUOTED_ID    \"{ALLOWED_QUOTED_ID}*\"
> +ID_CHARS_NOPAREN     [^ \t\n"!,(]
>  
>  IP           {NUMBER}\.{NUMBER}\.{NUMBER}\.{NUMBER}
>  
> @@ -229,9 +230,6 @@ BOOL_VARIABLE     $(\{{VARIABLE_NAME}\}|{VARIABLE_NAME})
>  LABEL                
> (\/|{SET_VARIABLE}{POST_VAR_ID}|{COLON}|{AMPERSAND}){ID}*
>  QUOTED_LABEL \"(\/|{SET_VAR_PREFIX}|{COLON}|{AMPERSAND})([^\0"]|\\\")*\"
>  
> -DPATHCOMP    [^[:space:]/]+
> -DPATHNAME    {SLASH}({DPATHCOMP}{SLASH})*{DPATHCOMP}?
> -
>  OPEN_PAREN   \(
>  CLOSE_PAREN  \)
>  COMMA                \,
> @@ -508,7 +506,7 @@ LT_EQUAL  <=
>       tracedby        { RETURN_TOKEN(TOK_TRACEDBY); }
>  }
>  
> -<DBUS_MODE,SIGNAL_MODE,PTRACE_MODE,UNIX_MODE>{
> +<DCONF_MODE,DBUS_MODE,SIGNAL_MODE,PTRACE_MODE,UNIX_MODE>{
>       read            { RETURN_TOKEN(TOK_READ); }
>       write           { RETURN_TOKEN(TOK_WRITE); }
>       {OPEN_PAREN}    {
> @@ -521,9 +519,7 @@ LT_EQUAL  <=
>  }
>  
>  <DCONF_MODE>{
> -     r       { RETURN_TOKEN(TOK_READ); }
> -     rw      { RETURN_TOKEN(TOK_READWRITE); }
> -     {DPATHNAME}     {
> +     ({ID_CHARS_NOPAREN}{ID}*|{QUOTED_ID}) {
>               yylval.id = processid(yytext, yyleng);
>               RETURN_TOKEN(TOK_ID);
>       }
> diff --git a/parser/parser_yacc.y b/parser/parser_yacc.y
> index 75e9387..16245d6 100644
> --- a/parser/parser_yacc.y
> +++ b/parser/parser_yacc.y
> @@ -149,7 +149,6 @@ void add_local_entry(Profile *prof);
>  %token TOK_BIND
>  %token TOK_READ
>  %token TOK_WRITE
> -%token TOK_READWRITE
>  %token TOK_EAVESDROP
>  %token TOK_PEER
>  %token TOK_TRACE
> @@ -1325,17 +1324,41 @@ dbus_rule: TOK_DBUS opt_dbus_perm opt_conds 
> opt_cond_list TOK_END_OF_RULE
>       }
>  
>  dconf_perm: TOK_READ { $$ = AA_DCONF_READ; }
> -     | TOK_READWRITE { $$ = AA_DCONF_READWRITE; }
> +     | TOK_WRITE { $$ = AA_DCONF_WRITE; }
> +     | TOK_VALUE
> +     {
> +             if (!$1)
> +                     $$ = 0;
> +             else if (strcmp($1, "read") == 0)
> +                     $$ = AA_DCONF_READ;
> +             else if (strcmp($1, "write") == 0)
> +                     $$ = AA_DCONF_WRITE;
> +             else
> +                     parse_X_mode("dconf", AA_DCONF_READ | AA_DCONF_WRITE,
> +                                  $1, &$$, 1);
> +             free($1);
> +     }
> +     | TOK_MODE
> +     {
> +             parse_X_mode("dconf", AA_DCONF_READ | AA_DCONF_WRITE, $1, &$$,
> +                          1);
> +             free($1);
> +     }
>  
>  dconf_perms: { /* nothing */ $$ = 0; }
>       | dconf_perms dconf_perm { $$ = $1 | $2; }
> +     | dconf_perms TOK_COMMA dconf_perm { $$ = $1 | $3; }
>  
> -opt_dconf_perms: { /* nothing */ $$ = AA_DCONF_READWRITE; /* default 
> read-write */ }
> +opt_dconf_perms: { /* nothing */ $$ = AA_VALID_DCONF_PERMS; }
>       | dconf_perms dconf_perm { $$ = $1 | $2; }
> +     | TOK_OPENPAREN dconf_perms TOK_CLOSEPAREN { $$ = $2; }
>  
> -dconf_rule: TOK_DCONF TOK_ID opt_dconf_perms TOK_END_OF_RULE
> +dconf_rule: TOK_DCONF opt_dconf_perms opt_id TOK_END_OF_RULE
>       {
> -             dconf_rule *ent = new dconf_rule($2, $3);
> +             dconf_rule *ent;
> +             if ((($2) & AA_DCONF_WRITE) && !(($2) & AA_DCONF_READ))
> +                     yyerror(_("dconf write permission must be accompanied 
> by read permission"));
> +             ent = new dconf_rule($3 ? $3 : strdup("/"), $2);
>               if (!ent) {
>                       yyerror(_("Memory allocation error."));
>               }
> diff --git a/parser/tst/equality.sh b/parser/tst/equality.sh
> index 029eec4..c2ac80b 100755
> --- a/parser/tst/equality.sh
> +++ b/parser/tst/equality.sh
> @@ -265,6 +265,22 @@ verify_binary_equality "dbus minimization found in dbus 
> abstractions" \
>                     peer=(name=org.freedesktop.DBus),
>             dbus send bus=session, }"
>  
> +verify_binary_equality "dconf read" \
> +     "/t { dconf r /, }" \
> +     "/t { dconf read /, }"
> +
> +verify_binary_equality "dconf read-write" \
> +     "/t { dconf /, }" \
> +     "/t { dconf rw /, }" \
> +     "/t { dconf wr /, }" \
> +     "/t { dconf (read write) /, }" \
> +     "/t { dconf (write read) /, }" \
> +     "/t { dconf (read, write) /, }"
> +
> +verify_binary_equality "dconf missing options" \
> +     "/t { dconf /, }" \
> +     "/t { dconf, }"
> +
>  # Rules compatible with audit, deny, and audit deny
>  # note: change_profile does not support audit/allow/deny atm
>  for rule in "capability" "capability mac_admin" \
> diff --git a/parser/tst/simple_tests/dconf/bad_path_01.sd 
> b/parser/tst/simple_tests/dconf/bad_path_01.sd
> index c78d407..ee703df 100644
> --- a/parser/tst/simple_tests/dconf/bad_path_01.sd
> +++ b/parser/tst/simple_tests/dconf/bad_path_01.sd
> @@ -1,8 +1,8 @@
>  #
> -#=DESCRIPTION dconf rule missing path
> +#=DESCRIPTION dconf rule path doesn't start with /
>  #=EXRESULT FAIL
>  #
>  
>  profile bad_path {
> -  dconf r,
> +  dconf r abc,
>  }
> diff --git a/parser/tst/simple_tests/dconf/bad_path_02.sd 
> b/parser/tst/simple_tests/dconf/bad_path_02.sd
> index e0ccc03..2e42492 100644
> --- a/parser/tst/simple_tests/dconf/bad_path_02.sd
> +++ b/parser/tst/simple_tests/dconf/bad_path_02.sd
> @@ -1,8 +1,8 @@
>  #
> -#=DESCRIPTION dconf rule missing leading slash
> +#=DESCRIPTION dconf  trailing perm
>  #=EXRESULT FAIL
>  #
>  
>  profile bad_path {
> -  dconf a/b/c r,
> +  dconf /a/b/c r,
>  }
> diff --git a/parser/tst/simple_tests/dconf/bad_path_03.sd 
> b/parser/tst/simple_tests/dconf/bad_path_03.sd
> index d9c93c9..daf746a 100644
> --- a/parser/tst/simple_tests/dconf/bad_path_03.sd
> +++ b/parser/tst/simple_tests/dconf/bad_path_03.sd
> @@ -4,5 +4,5 @@
>  #
>  
>  profile bad_path {
> -  dconf /a//b r,
> +  dconf r /a//b,
>  }
> diff --git a/parser/tst/simple_tests/dconf/bad_path_04.sd 
> b/parser/tst/simple_tests/dconf/bad_path_04.sd
> index a870c12..d9e086b 100644
> --- a/parser/tst/simple_tests/dconf/bad_path_04.sd
> +++ b/parser/tst/simple_tests/dconf/bad_path_04.sd
> @@ -4,5 +4,5 @@
>  #
>  
>  profile bad_path {
> -  dconf /a /b r,
> +  dconf r /a /b,
>  }
> diff --git a/parser/tst/simple_tests/dconf/bad_path_05.sd 
> b/parser/tst/simple_tests/dconf/bad_path_05.sd
> new file mode 100644
> index 0000000..38b61d2
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/bad_path_05.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with tail glob
> +#=EXRESULT FAIL
> +#
> +
> +profile bad_path {
> +  dconf r /a/b**,
> +}
> diff --git a/parser/tst/simple_tests/dconf/bad_path_06.sd 
> b/parser/tst/simple_tests/dconf/bad_path_06.sd
> new file mode 100644
> index 0000000..d73aa0c
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/bad_path_06.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with alternation
> +#=EXRESULT FAIL
> +#
> +
> +profile bad_path {
> +  dconf r /a/b{c,d},
> +}
> diff --git a/parser/tst/simple_tests/dconf/bad_path_07.sd 
> b/parser/tst/simple_tests/dconf/bad_path_07.sd
> new file mode 100644
> index 0000000..114fc61
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/bad_path_07.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with ?
> +#=EXRESULT FAIL
> +#
> +
> +profile bad_path {
> +  dconf r /a/b?,
> +}
> diff --git a/parser/tst/simple_tests/dconf/bad_path_08.sd 
> b/parser/tst/simple_tests/dconf/bad_path_08.sd
> new file mode 100644
> index 0000000..925f2c7
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/bad_path_08.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with *
> +#=EXRESULT FAIL
> +#
> +
> +profile bad_path {
> +  dconf r /a*/b,
> +}
> diff --git a/parser/tst/simple_tests/dconf/bad_path_09.sd 
> b/parser/tst/simple_tests/dconf/bad_path_09.sd
> new file mode 100644
> index 0000000..3f5b274
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/bad_path_09.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with non tail **
> +#=EXRESULT FAIL
> +#
> +
> +profile bad_path {
> +  dconf r /a**/b,
> +}
> diff --git a/parser/tst/simple_tests/dconf/ok_audit_01.sd 
> b/parser/tst/simple_tests/dconf/ok_audit_01.sd
> index d2e63f2..6b2087c 100644
> --- a/parser/tst/simple_tests/dconf/ok_audit_01.sd
> +++ b/parser/tst/simple_tests/dconf/ok_audit_01.sd
> @@ -4,6 +4,5 @@
>  #
>  
>  profile ok_audit {
> -  audit dconf /a/b/c r,
> -  audit dconf /d/e/f rw,
> +  audit dconf r /a/b/c,
>  }
> diff --git a/parser/tst/simple_tests/dconf/ok_audit_02.sd 
> b/parser/tst/simple_tests/dconf/ok_audit_02.sd
> new file mode 100644
> index 0000000..786557d
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/ok_audit_02.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with audit
> +#=EXRESULT PASS
> +#
> +
> +profile ok_audit {
> +  audit dconf rw /d/e/f,
> +}
> diff --git a/parser/tst/simple_tests/dconf/ok_dir_01.sd 
> b/parser/tst/simple_tests/dconf/ok_dir_01.sd
> index aa1f487..29c987a 100644
> --- a/parser/tst/simple_tests/dconf/ok_dir_01.sd
> +++ b/parser/tst/simple_tests/dconf/ok_dir_01.sd
> @@ -4,5 +4,5 @@
>  #
>  
>  profile ok_audit {
> -  dconf /a/b/c/ r,
> +  dconf r /a/b/c/,
>  }
> diff --git a/parser/tst/simple_tests/dconf/ok_dir_02.sd 
> b/parser/tst/simple_tests/dconf/ok_dir_02.sd
> new file mode 100644
> index 0000000..52dbe5f
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/ok_dir_02.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule for dir
> +#=EXRESULT PASS
> +#
> +
> +profile ok_audit {
> +  dconf rw /a/b/c/,
> +}
> diff --git a/parser/tst/simple_tests/dconf/ok_escapedrechars_01.sd 
> b/parser/tst/simple_tests/dconf/ok_escapedrechars_01.sd
> new file mode 100644
> index 0000000..78dbe64
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/ok_escapedrechars_01.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with \*
> +#=EXRESULT PASS
> +#
> +
> +profile ok_audit {
> +  dconf r /a/\*b/c,
> +}
> diff --git a/parser/tst/simple_tests/dconf/ok_escapedrechars_02.sd 
> b/parser/tst/simple_tests/dconf/ok_escapedrechars_02.sd
> new file mode 100644
> index 0000000..1c0604b
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/ok_escapedrechars_02.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with \*\*
> +#=EXRESULT PASS
> +#
> +
> +profile ok_audit {
> +  dconf r /a/\*\*b/c,
> +}
> diff --git a/parser/tst/simple_tests/dconf/ok_escapedrechars_03.sd 
> b/parser/tst/simple_tests/dconf/ok_escapedrechars_03.sd
> new file mode 100644
> index 0000000..044b6aa
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/ok_escapedrechars_03.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with tail \*\*
> +#=EXRESULT PASS
> +#
> +
> +profile ok_audit {
> +  dconf r /a/b/c\*\*,
> +}
> diff --git a/parser/tst/simple_tests/dconf/ok_escapedrechars_04.sd 
> b/parser/tst/simple_tests/dconf/ok_escapedrechars_04.sd
> new file mode 100644
> index 0000000..603b6ed
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/ok_escapedrechars_04.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with tail ?
> +#=EXRESULT PASS
> +#
> +
> +profile ok_audit {
> +  dconf r /a/b/c\?,
> +}
> diff --git a/parser/tst/simple_tests/dconf/ok_escapedrechars_05.sd 
> b/parser/tst/simple_tests/dconf/ok_escapedrechars_05.sd
> new file mode 100644
> index 0000000..e9c25e0
> --- /dev/null
> +++ b/parser/tst/simple_tests/dconf/ok_escapedrechars_05.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION dconf rule with tail \{
> +#=EXRESULT PASS
> +#
> +
> +profile ok_audit {
> +  dconf r /a/b/c\{c,d\},
> +}
> diff --git a/parser/tst/simple_tests/dconf/ok_key_01.sd 
> b/parser/tst/simple_tests/dconf/ok_key_01.sd
> index 5ec92fa..b6c6f3f 100644
> --- a/parser/tst/simple_tests/dconf/ok_key_01.sd
> +++ b/parser/tst/simple_tests/dconf/ok_key_01.sd
> @@ -4,5 +4,5 @@
>  #
>  
>  profile ok_audit {
> -  dconf /a/b/c r,
> +  dconf r /a/b/c,
>  }

Attachment: signature.asc
Description: PGP signature

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to