[apparmor] [PATCH 5/5] Revise dconf
John Johansen
john.johansen at canonical.com
Fri Dec 16 18:01:47 UTC 2016
---
libraries/libapparmor/include/sys/apparmor.h | 16 ++---
libraries/libapparmor/src/kernel.c | 95 +++++++++++++---------------
parser/dconf.cc | 78 +++++++++++------------
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/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 +++
parser/tst/simple_tests/dconf/ok_key_01.sd | 2 +-
16 files changed, 160 insertions(+), 131 deletions(-)
create mode 100644 parser/tst/simple_tests/dconf/ok_audit_02.sd
create mode 100644 parser/tst/simple_tests/dconf/ok_dir_02.sd
diff --git a/libraries/libapparmor/include/sys/apparmor.h b/libraries/libapparmor/include/sys/apparmor.h
index 6f79eff..ac4e2c0 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 */
@@ -145,14 +145,8 @@ typedef struct {
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 */
+ size_t n; /* number of paths */
+ const char **paths; /* paths in data */
} aa_dconf_info;
extern int aa_query_label_data(const char *label, const char *key,
diff --git a/libraries/libapparmor/src/kernel.c b/libraries/libapparmor/src/kernel.c
index 349290d..88fd692 100644
--- a/libraries/libapparmor/src/kernel.c
+++ b/libraries/libapparmor/src/kernel.c
@@ -1260,70 +1260,64 @@ int aa_query_dconf_info(const char *label, aa_dconf_info *info)
{
aa_label_data_info data_info;
const char *p;
- uint32_t tmp;
- int i;
+ uint32_t tmp, size;
+ 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)
- return -1;
-
+ /* transfer the data blob */
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);
- }
+ data_info.data = NULL;
- for (i = 0; i < info->rwn; i++) {
- memcpy(&tmp, p, sizeof(tmp));
- p += sizeof(tmp);
- info->rwpaths[i] = p;
- p += le32toh(tmp);
+ /* Find how many paths there are, one data ent per profile, each
+ * profile can have multiple paths
+ */
+ for (info->n = i = 0; i < data_info.n; i++) {
+ p = data_info.ents[i].entry;
+ while (p < data_info.ents[i].entry + data_info.ents[i].size) {
+ memcpy(&tmp, p, sizeof(tmp));
+ size = le32toh(tmp);
+ p += size + sizeof(tmp);
+ /* must be null terminated */
+ if (*(p-1) != 0) {
+ errno = -EPROTO;
+ goto out;
+ }
+ info->n++;
+ }
+ /* verify set of strings is exactly the size of the data */
+ if (p - data_info.ents[i].entry != data_info.ents[i].size) {
+ errno = -ERANGE;
+ goto out;
+ }
}
- for (i = 0; i < info->arn; i++) {
- memcpy(&tmp, p, sizeof(tmp));
- p += sizeof(tmp);
- info->arpaths[i] = p;
- p += le32toh(tmp);
+ info->paths = malloc(info->n * sizeof(*info->paths));
+ if (!info->paths) {
+ errno = -ENOMEM;
+ goto out;
}
- for (i = 0; i < info->arwn; i++) {
- memcpy(&tmp, p, sizeof(tmp));
- p += sizeof(tmp);
- info->arwpaths[i] = p;
- p += le32toh(tmp);
+ /* setups paths */
+ for (n = i = 0; i < data_info.n; i++) {
+ p = data_info.ents[i].entry;
+ while (p < data_info.ents[i].entry + data_info.ents[i].size) {
+ memcpy(&tmp, p, sizeof(tmp));
+ size = le32toh(tmp);
+ p += sizeof(tmp);
+ info->paths[n++] = p;
+ p += size;
+ }
}
+ res = 0;
- data_info.data = NULL;
+out:
aa_clear_label_data(&data_info);
- return 0;
+ return res;
}
/**
@@ -1334,10 +1328,7 @@ 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->paths);
free(info->data);
memset(info, 0, sizeof(*info));
diff --git a/parser/dconf.cc b/parser/dconf.cc
index 669bf80..06afd02 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,43 @@ 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);
- }
+ /* TODO: generate an approximate watch point from the rule.
+ * ATM just use a global root
+ */
+ //dconf->paths.insert(path);
+ dconf->paths.insert("/");
}
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;
- if (path[strlen(path) - 1] == '/')
- rule << path << "[^\\x00]*";
+ rule << "\\x" << std::setfill('0') << std::setw(2) << std::hex << AA_CLASS_DCONF;
+
+ /* TODO: determine whether we really want to auto convert trailing
+ * / into subtree match. It fits with dconf style but not really
+ * with apparmor.
+ */
+ 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..7e6c054 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..3b7cfa0 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/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_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,
}
--
2.9.3
More information about the AppArmor
mailing list