[apparmor] [PATCH 7/7] Revise dconf

Seth Arnold seth.arnold at canonical.com
Fri Feb 17 01:58:25 UTC 2017


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.johansen at canonical.com>

Acked-by: Seth Arnold <seth.arnold at 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,
>  }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20170216/35a243bb/attachment-0001.pgp>


More information about the AppArmor mailing list