From 0b52b0eae6d31fa3fcfaa02dda4efb82fc3e42c1 Mon Sep 17 00:00:00 2001 From: Zygmunt Bazyli Krynicki Date: Thu, 11 Jul 2024 23:55:44 +0200 Subject: [PATCH] many: update apparmor to 4.0.1 (#14150) * build-aux: update vendored apparmor to 4.0.1 release Signed-off-by: Alex Murray * build-aux: add autoconf-archive to apparmor/build-packages Unlike the Launchpad tarball, the one from apparmor gitlab tarball requires this to be present as it is just a snapshot of the git tree, not a release tarball like those provided by Launchpad. Signed-off-by: Alex Murray * build-aux: remove apparmor parser performance patch This was already included upstream as part of the 3.1.0 release and hence is included in the 4.0.1 release which we are now vendoring. Signed-off-by: Alex Murray * build-aux: remove remote patch application logic They are already included in apparmor 4.x release. Signed-off-by: Zygmunt Krynicki * build-aux: remove local patch application logic All local patches are now merged in the 4.x release. Signed-off-by: Zygmunt Krynicki * cmd/configure.ac: expect apparmor 4.0.1 when building as a snap Signed-off-by: Alex Murray * sandbox/apparmor: use apparmor 4.0 abi with vendored parser Signed-off-by: Alex Murray * sandbox/apparmor: add debug logging when probing parser features Signed-off-by: Alex Murray * sandbox/apparmor: log apparmor_parser version when probing features This is helpful when trying to debug why certain features may not be supported. Signed-off-by: Alex Murray * tests/main: update for new vendored apparmor 4.0 Signed-off-by: Alex Murray * Reapply "i/builtin: allow docker-support to use mqueue (#13738)" (#13765) This reverts commit ce298864e33f8179182e0b1b1ad183f5985cf25a. * interfaces: adjust docker-support test to handle mqueue Signed-off-by: Zygmunt Krynicki * sandbox/apparmor: mask mqueue feature until apparmor 4.0.1 It seems that mediation of mqueue is miscompiled by apparmor_parser 4.0.0~beta3 that was present in Ubuntu 24.04 until the 10th of July 2024. Detect this and mask the presence of mqueue unless apparmor parser 4.0.1, or newer, is used. Signed-off-by: Zygmunt Krynicki * sandbox/apparmor: support bundled 3.0 or 4.0 (preferred) abi Mirror the logic used in apparmor-from-the-host to apparmor-from-snapd-snap. This mainly fixes tests that repackage old snapd snap without touching apparmor, but in general seems like the right thing to do. The logic is such, that abi 4 is preferred. Signed-off-by: Zygmunt Krynicki * sandbox/apparmor: unify test mocking logic Signed-off-by: Zygmunt Krynicki * sandbox/apparmor: refactor appArmorParserVersion not to clobber cmd Signed-off-by: Zygmunt Krynicki * sandbox/apparmor: fix pair of typos Signed-off-by: Zygmunt Krynicki --------- Signed-off-by: Alex Murray Signed-off-by: Zygmunt Krynicki Co-authored-by: Alex Murray --- build-aux/snap/local/apparmor/af_names.h | 15 +- ...ace-dynamic_cast-with-is_type-method.patch | 791 ------------------ build-aux/snap/snapcraft.yaml | 15 +- cmd/configure.ac | 8 +- interfaces/builtin/docker_support.go | 8 + interfaces/builtin/docker_support_test.go | 12 +- sandbox/apparmor/apparmor.go | 75 +- sandbox/apparmor/apparmor_test.go | 133 ++- .../task.yaml | 2 +- .../security-device-cgroups-helper/task.yaml | 2 +- tests/main/security-seccomp/task.yaml | 4 +- tests/main/security-setuid-root/task.yaml | 4 +- tests/main/snapd-snap/task.yaml | 2 +- 13 files changed, 213 insertions(+), 858 deletions(-) delete mode 100644 build-aux/snap/local/patches/apparmor/parser-replace-dynamic_cast-with-is_type-method.patch diff --git a/build-aux/snap/local/apparmor/af_names.h b/build-aux/snap/local/apparmor/af_names.h index 27ad9784585..70d40898c7c 100644 --- a/build-aux/snap/local/apparmor/af_names.h +++ b/build-aux/snap/local/apparmor/af_names.h @@ -1,13 +1,16 @@ /* - this file was generated on a Ubuntu kinetic install from the upstream - apparmor-3.0.7 release tarball as follows: + this file was generated on a Ubuntu mantic install from the upstream + apparmor-4.0.1 release tarball as follows: - AA_VER=3.0.7 + AA_VER=4.0.1 + TARBALL_NAME="apparmor-v${AA_VER}" wget \ - "https://launchpad.net/apparmor/3.0/${AA_VER}/+download/apparmor-${AA_VER}.tar.gz" - tar xf "apparmor-${AA_VER}.tar.gz" - cd "apparmor-${AA_VER}" + "https://gitlab.com/apparmor/apparmor/-/archive/v${AA_VER}/${TARBALL_NAME}.tar.gz" + tar xf "${TARBALL_NAME}.tar.gz" + cd "${TARBALL_NAME}" make -C parser af_names.h + cp ./parser/af_names.h + # manually append this header */ #ifndef AF_UNSPEC diff --git a/build-aux/snap/local/patches/apparmor/parser-replace-dynamic_cast-with-is_type-method.patch b/build-aux/snap/local/patches/apparmor/parser-replace-dynamic_cast-with-is_type-method.patch deleted file mode 100644 index 159e8deb66a..00000000000 --- a/build-aux/snap/local/patches/apparmor/parser-replace-dynamic_cast-with-is_type-method.patch +++ /dev/null @@ -1,791 +0,0 @@ -From 5aab543a3b03ecaea356a02928e5bb5b7a0d8fa5 Mon Sep 17 00:00:00 2001 -From: =?UTF-8?q?Alfonso=20S=C3=A1nchez-Beato?= - -Date: Mon, 15 Feb 2021 16:26:18 +0100 -Subject: [PATCH] parser: replace dynamic_cast with is_type method - -The dynamic_cast operator is slow as it needs to look at RTTI -information and even does some string comparisons, especially in deep -hierarchies like the one for Node. Profiling with callgrind showed -that dynamic_cast can eat a huge portion of the running time, as it -takes most of the time that is spent in the simplify_tree() -function. For some complex profiles, the number of calls to -dynamic_cast can be in the range of millions. - -This commit replaces the use of dynamic_cast in the Node hierarchy -with a method called is_type(), which returns true if the pointer can -be casted to the specified type. It works by looking at a Node object -field that is an integer with bits set for each type up in the -hierarchy. Therefore, dynamic_cast is replaced by a simple bits -operation. - -This change can reduce the compilation times for some profiles more -that 50%, especially in arm/arm64 arch. This opens the door to maybe -avoid "-O no-expr-simplify" in the snapd daemon, as now that option -would make the compilation slower in almost all cases. - -This is the example profile used in some of my tests, with this change -the run-time is around 1/3 of what it was before on an x86 laptop: - -profile "test" (attach_disconnected,mediate_deleted) { -dbus send - bus={fcitx,session} - path=/inputcontext_[0-9]* - interface=org.fcitx.Fcitx.InputContext - member="{Close,Destroy,Enable}IC" - peer=(label=unconfined), -dbus send - bus={fcitx,session} - path=/inputcontext_[0-9]* - interface=org.fcitx.Fcitx.InputContext - member=Reset - peer=(label=unconfined), -dbus receive - bus=fcitx - peer=(label=unconfined), -dbus receive - bus=session - interface=org.fcitx.Fcitx.* - peer=(label=unconfined), -dbus send - bus={fcitx,session} - path=/inputcontext_[0-9]* - interface=org.fcitx.Fcitx.InputContext - member="Focus{In,Out}" - peer=(label=unconfined), -dbus send - bus={fcitx,session} - path=/inputcontext_[0-9]* - interface=org.fcitx.Fcitx.InputContext - member="{CommitPreedit,Set*}" - peer=(label=unconfined), -dbus send - bus={fcitx,session} - path=/inputcontext_[0-9]* - interface=org.fcitx.Fcitx.InputContext - member="{MouseEvent,ProcessKeyEvent}" - peer=(label=unconfined), -dbus send - bus={fcitx,session} - path=/inputcontext_[0-9]* - interface=org.freedesktop.DBus.Properties - member=GetAll - peer=(label=unconfined), -dbus (send) - bus=session - path=/org/a11y/bus - interface=org.a11y.Bus - member=GetAddress - peer=(label=unconfined), -dbus (send) - bus=session - path=/org/a11y/bus - interface=org.freedesktop.DBus.Properties - member=Get{,All} - peer=(label=unconfined), -dbus (receive, send) - bus=accessibility - path=/org/a11y/atspi/** - peer=(label=unconfined), -dbus (send) - bus=system - path=/org/freedesktop/Accounts - interface=org.freedesktop.DBus.Introspectable - member=Introspect - peer=(label=unconfined), -dbus (send) - bus=system - path=/org/freedesktop/Accounts - interface=org.freedesktop.Accounts - member=FindUserById - peer=(label=unconfined), -dbus (receive, send) - bus=system - path=/org/freedesktop/Accounts/User[0-9]* - interface=org.freedesktop.DBus.Properties - member={Get,PropertiesChanged} - peer=(label=unconfined), -dbus (send) - bus=session - interface=org.gtk.Actions - member=Changed - peer=(name=org.freedesktop.DBus, label=unconfined), -dbus (receive) - bus=session - interface=org.gtk.Actions - member={Activate,DescribeAll,SetState} - peer=(label=unconfined), -dbus (receive) - bus=session - interface=org.gtk.Menus - member={Start,End} - peer=(label=unconfined), -dbus (send) - bus=session - interface=org.gtk.Menus - member=Changed - peer=(name=org.freedesktop.DBus, label=unconfined), -dbus (send) - bus=session - path="/com/ubuntu/MenuRegistrar" - interface="com.ubuntu.MenuRegistrar" - member="{Register,Unregister}{App,Surface}Menu" - peer=(label=unconfined), -} ---- - parser/libapparmor_re/aare_rules.cc | 10 +- - parser/libapparmor_re/expr-tree.cc | 63 +++++------ - parser/libapparmor_re/expr-tree.h | 162 +++++++++++++++++++++------- - parser/libapparmor_re/hfa.cc | 9 +- - 4 files changed, 165 insertions(+), 79 deletions(-) - -diff --git a/parser/libapparmor_re/aare_rules.cc b/parser/libapparmor_re/aare_rules.cc -index 1d56b3cb0..b250e1013 100644 ---- a/parser/libapparmor_re/aare_rules.cc -+++ b/parser/libapparmor_re/aare_rules.cc -@@ -97,11 +97,11 @@ bool aare_rules::add_rule_vec(int deny, uint32_t perms, uint32_t audit, - */ - exact_match = 1; - for (depth_first_traversal i(tree); i && exact_match; i++) { -- if (dynamic_cast(*i) || -- dynamic_cast(*i) || -- dynamic_cast(*i) || -- dynamic_cast(*i) || -- dynamic_cast(*i)) -+ if ((*i)->is_type(NODE_TYPE_STAR) || -+ (*i)->is_type(NODE_TYPE_PLUS) || -+ (*i)->is_type(NODE_TYPE_ANYCHAR) || -+ (*i)->is_type(NODE_TYPE_CHARSET) || -+ (*i)->is_type(NODE_TYPE_NOTCHARSET)) - exact_match = 0; - } - -diff --git a/parser/libapparmor_re/expr-tree.cc b/parser/libapparmor_re/expr-tree.cc -index 28aa35000..7dc18b041 100644 ---- a/parser/libapparmor_re/expr-tree.cc -+++ b/parser/libapparmor_re/expr-tree.cc -@@ -210,7 +210,7 @@ int TwoChildNode::normalize_eps(int dir) - // Test for E | (E | E) and E . (E . E) which will - // result in an infinite loop - Node *c = child[!dir]; -- if (dynamic_cast(c) && -+ if (c->is_type(NODE_TYPE_TWOCHILD) && - &epsnode == c->child[dir] && - &epsnode == c->child[!dir]) { - c->release(); -@@ -229,7 +229,7 @@ void CatNode::normalize(int dir) - for (;;) { - if (normalize_eps(dir)) { - continue; -- } else if (dynamic_cast(child[dir])) { -+ } else if (child[dir]->is_type(NODE_TYPE_CAT)) { - // (ab)c -> a(bc) - rotate_node(this, dir); - } else { -@@ -248,11 +248,11 @@ void AltNode::normalize(int dir) - for (;;) { - if (normalize_eps(dir)) { - continue; -- } else if (dynamic_cast(child[dir])) { -+ } else if (child[dir]->is_type(NODE_TYPE_ALT)) { - // (a | b) | c -> a | (b | c) - rotate_node(this, dir); -- } else if (dynamic_cast(child[dir]) && -- dynamic_cast(child[!dir])) { -+ } else if (child[dir]->is_type(NODE_TYPE_CHARSET) && -+ child[!dir]->is_type(NODE_TYPE_CHAR)) { - // [a] | b -> b | [a] - Node *c = child[dir]; - child[dir] = child[!dir]; -@@ -344,7 +344,7 @@ static Node *alt_to_charsets(Node *t, int dir) - - static Node *basic_alt_factor(Node *t, int dir) - { -- if (!dynamic_cast(t)) -+ if (!t->is_type(NODE_TYPE_ALT)) - return t; - - if (t->child[dir]->eq(t->child[!dir])) { -@@ -355,8 +355,8 @@ static Node *basic_alt_factor(Node *t, int dir) - return tmp; - } - // (ab) | (ac) -> a(b|c) -- if (dynamic_cast(t->child[dir]) && -- dynamic_cast(t->child[!dir]) && -+ if (t->child[dir]->is_type(NODE_TYPE_CAT) && -+ t->child[!dir]->is_type(NODE_TYPE_CAT) && - t->child[dir]->child[dir]->eq(t->child[!dir]->child[dir])) { - // (ab) | (ac) -> a(b|c) - Node *left = t->child[dir]; -@@ -369,7 +369,7 @@ static Node *basic_alt_factor(Node *t, int dir) - return left; - } - // a | (ab) -> a (E | b) -> a (b | E) -- if (dynamic_cast(t->child[!dir]) && -+ if (t->child[!dir]->is_type(NODE_TYPE_CAT) && - t->child[dir]->eq(t->child[!dir]->child[dir])) { - Node *c = t->child[!dir]; - t->child[dir]->release(); -@@ -379,7 +379,7 @@ static Node *basic_alt_factor(Node *t, int dir) - return c; - } - // ab | (a) -> a (b | E) -- if (dynamic_cast(t->child[dir]) && -+ if (t->child[dir]->is_type(NODE_TYPE_CAT) && - t->child[dir]->child[dir]->eq(t->child[!dir])) { - Node *c = t->child[dir]; - t->child[!dir]->release(); -@@ -394,7 +394,7 @@ static Node *basic_alt_factor(Node *t, int dir) - - static Node *basic_simplify(Node *t, int dir) - { -- if (dynamic_cast(t) && &epsnode == t->child[!dir]) { -+ if (t->is_type(NODE_TYPE_CAT) && &epsnode == t->child[!dir]) { - // aE -> a - Node *tmp = t->child[dir]; - t->child[dir] = NULL; -@@ -419,7 +419,7 @@ static Node *basic_simplify(Node *t, int dir) - */ - Node *simplify_tree_base(Node *t, int dir, bool &mod) - { -- if (dynamic_cast(t)) -+ if (t->is_type(NODE_TYPE_IMPORTANT)) - return t; - - for (int i = 0; i < 2; i++) { -@@ -442,15 +442,15 @@ Node *simplify_tree_base(Node *t, int dir, bool &mod) - } - - /* all tests after this must meet 2 alt node condition */ -- if (!dynamic_cast(t) || -- !dynamic_cast(t->child[!dir])) -+ if (!t->is_type(NODE_TYPE_ALT) || -+ !t->child[!dir]->is_type(NODE_TYPE_ALT)) - break; - - // a | (a | b) -> (a | b) - // a | (b | (c | a)) -> (b | (c | a)) - Node *p = t; - Node *i = t->child[!dir]; -- for (; dynamic_cast(i); p = i, i = i->child[!dir]) { -+ for (; i->is_type(NODE_TYPE_ALT); p = i, i = i->child[!dir]) { - if (t->child[dir]->eq(i->child[dir])) { - Node *tmp = t->child[!dir]; - t->child[!dir] = NULL; -@@ -475,19 +475,19 @@ Node *simplify_tree_base(Node *t, int dir, bool &mod) - int count = 0; - Node *subject = t->child[dir]; - Node *a = subject; -- if (dynamic_cast(subject)) -+ if (subject->is_type(NODE_TYPE_CAT)) - a = subject->child[dir]; - - for (pp = p = t, i = t->child[!dir]; -- dynamic_cast(i);) { -- if ((dynamic_cast(i->child[dir]) && -+ i->is_type(NODE_TYPE_ALT);) { -+ if ((i->child[dir]->is_type(NODE_TYPE_CAT) && - a->eq(i->child[dir]->child[dir])) || - (a->eq(i->child[dir]))) { - // extract matching alt node - p->child[!dir] = i->child[!dir]; - i->child[!dir] = subject; - subject = basic_simplify(i, dir); -- if (dynamic_cast(subject)) -+ if (subject->is_type(NODE_TYPE_CAT)) - a = subject->child[dir]; - else - a = subject; -@@ -502,7 +502,7 @@ Node *simplify_tree_base(Node *t, int dir, bool &mod) - } - - // last altnode in chain check other dir as well -- if ((dynamic_cast(i) && -+ if ((i->is_type(NODE_TYPE_CAT) && - a->eq(i->child[dir])) || (a->eq(i))) { - count++; - if (t == p) { -@@ -528,7 +528,7 @@ int debug_tree(Node *t) - { - int nodes = 1; - -- if (!dynamic_cast(t)) { -+ if (!t->is_type(NODE_TYPE_IMPORTANT)) { - if (t->child[0]) - nodes += debug_tree(t->child[0]); - if (t->child[1]) -@@ -539,30 +539,30 @@ int debug_tree(Node *t) - - static void count_tree_nodes(Node *t, struct node_counts *counts) - { -- if (dynamic_cast(t)) { -+ if (t->is_type(NODE_TYPE_ALT)) { - counts->alt++; - count_tree_nodes(t->child[0], counts); - count_tree_nodes(t->child[1], counts); -- } else if (dynamic_cast(t)) { -+ } else if (t->is_type(NODE_TYPE_CAT)) { - counts->cat++; - count_tree_nodes(t->child[0], counts); - count_tree_nodes(t->child[1], counts); -- } else if (dynamic_cast(t)) { -+ } else if (t->is_type(NODE_TYPE_PLUS)) { - counts->plus++; - count_tree_nodes(t->child[0], counts); -- } else if (dynamic_cast(t)) { -+ } else if (t->is_type(NODE_TYPE_STAR)) { - counts->star++; - count_tree_nodes(t->child[0], counts); -- } else if (dynamic_cast(t)) { -+ } else if (t->is_type(NODE_TYPE_OPTIONAL)) { - counts->optional++; - count_tree_nodes(t->child[0], counts); -- } else if (dynamic_cast(t)) { -+ } else if (t->is_type(NODE_TYPE_CHAR)) { - counts->charnode++; -- } else if (dynamic_cast(t)) { -+ } else if (t->is_type(NODE_TYPE_ANYCHAR)) { - counts->any++; -- } else if (dynamic_cast(t)) { -+ } else if (t->is_type(NODE_TYPE_CHARSET)) { - counts->charset++; -- } else if (dynamic_cast(t)) { -+ } else if (t->is_type(NODE_TYPE_NOTCHARSET)) { - counts->notcharset++; - } - } -@@ -635,7 +635,8 @@ Node *simplify_tree(Node *t, dfaflags_t flags) - void flip_tree(Node *node) - { - for (depth_first_traversal i(node); i; i++) { -- if (CatNode *cat = dynamic_cast(*i)) { -+ if ((*i)->is_type(NODE_TYPE_CAT)) { -+ CatNode *cat = static_cast(*i); - swap(cat->child[0], cat->child[1]); - } - } -diff --git a/parser/libapparmor_re/expr-tree.h b/parser/libapparmor_re/expr-tree.h -index 551dd0eeb..8ada4a4a7 100644 ---- a/parser/libapparmor_re/expr-tree.h -+++ b/parser/libapparmor_re/expr-tree.h -@@ -222,16 +222,43 @@ typedef struct Cases { - - ostream &operator<<(ostream &os, Node &node); - -+#define NODE_TYPE_NODE 0 -+#define NODE_TYPE_INNER (1 << 0) -+#define NODE_TYPE_ONECHILD (1 << 1) -+#define NODE_TYPE_TWOCHILD (1 << 2) -+#define NODE_TYPE_LEAF (1 << 3) -+#define NODE_TYPE_EPS (1 << 4) -+#define NODE_TYPE_IMPORTANT (1 << 5) -+#define NODE_TYPE_C (1 << 6) -+#define NODE_TYPE_CHAR (1 << 7) -+#define NODE_TYPE_CHARSET (1 << 8) -+#define NODE_TYPE_NOTCHARSET (1 << 9) -+#define NODE_TYPE_ANYCHAR (1 << 10) -+#define NODE_TYPE_STAR (1 << 11) -+#define NODE_TYPE_OPTIONAL (1 << 12) -+#define NODE_TYPE_PLUS (1 << 13) -+#define NODE_TYPE_CAT (1 << 14) -+#define NODE_TYPE_ALT (1 << 15) -+#define NODE_TYPE_SHARED (1 << 16) -+#define NODE_TYPE_ACCEPT (1 << 17) -+#define NODE_TYPE_MATCHFLAG (1 << 18) -+#define NODE_TYPE_EXACTMATCHFLAG (1 << 19) -+#define NODE_TYPE_DENYMATCHFLAG (1 << 20) -+ - /* An abstract node in the syntax tree. */ - class Node { - public: -- Node(): nullable(false), label(0) { child[0] = child[1] = 0; } -- Node(Node *left): nullable(false), label(0) -+ Node(): nullable(false), type_flags(NODE_TYPE_NODE), label(0) -+ { -+ child[0] = child[1] = 0; -+ } -+ Node(Node *left): nullable(false), type_flags(NODE_TYPE_NODE), label(0) - { - child[0] = left; - child[1] = 0; - } -- Node(Node *left, Node *right): nullable(false), label(0) -+ Node(Node *left, Node *right): nullable(false), -+ type_flags(NODE_TYPE_NODE), label(0) - { - child[0] = left; - child[1] = right; -@@ -302,6 +329,13 @@ public: - NodeSet firstpos, lastpos, followpos; - /* child 0 is left, child 1 is right */ - Node *child[2]; -+ /* -+ * Bitmap that stores supported pointer casts for the Node, composed -+ * by the NODE_TYPE_* flags. This is used by is_type() as a substitute -+ * of costly dynamic_cast calls. -+ */ -+ unsigned type_flags; -+ bool is_type(unsigned type) { return type_flags & type; } - - unsigned int label; /* unique number for debug etc */ - /** -@@ -315,25 +349,34 @@ public: - - class InnerNode: public Node { - public: -- InnerNode(): Node() { }; -- InnerNode(Node *left): Node(left) { }; -- InnerNode(Node *left, Node *right): Node(left, right) { }; -+ InnerNode(): Node() { type_flags |= NODE_TYPE_INNER; }; -+ InnerNode(Node *left): Node(left) { type_flags |= NODE_TYPE_INNER; }; -+ InnerNode(Node *left, Node *right): Node(left, right) -+ { -+ type_flags |= NODE_TYPE_INNER; -+ }; - }; - - class OneChildNode: public InnerNode { - public: -- OneChildNode(Node *left): InnerNode(left) { }; -+ OneChildNode(Node *left): InnerNode(left) -+ { -+ type_flags |= NODE_TYPE_ONECHILD; -+ }; - }; - - class TwoChildNode: public InnerNode { - public: -- TwoChildNode(Node *left, Node *right): InnerNode(left, right) { }; -+ TwoChildNode(Node *left, Node *right): InnerNode(left, right) -+ { -+ type_flags |= NODE_TYPE_TWOCHILD; -+ }; - virtual int normalize_eps(int dir); - }; - - class LeafNode: public Node { - public: -- LeafNode(): Node() { }; -+ LeafNode(): Node() { type_flags |= NODE_TYPE_LEAF; }; - virtual void normalize(int dir __attribute__((unused))) { return; } - }; - -@@ -342,6 +385,7 @@ class EpsNode: public LeafNode { - public: - EpsNode(): LeafNode() - { -+ type_flags |= NODE_TYPE_EPS; - nullable = true; - label = 0; - } -@@ -356,7 +400,7 @@ public: - void compute_lastpos() { } - int eq(Node *other) - { -- if (dynamic_cast(other)) -+ if (other->is_type(NODE_TYPE_EPS)) - return 1; - return 0; - } -@@ -373,7 +417,7 @@ public: - */ - class ImportantNode: public LeafNode { - public: -- ImportantNode(): LeafNode() { } -+ ImportantNode(): LeafNode() { type_flags |= NODE_TYPE_IMPORTANT; } - void compute_firstpos() { firstpos.insert(this); } - void compute_lastpos() { lastpos.insert(this); } - virtual void follow(Cases &cases) = 0; -@@ -386,7 +430,7 @@ public: - */ - class CNode: public ImportantNode { - public: -- CNode(): ImportantNode() { } -+ CNode(): ImportantNode() { type_flags |= NODE_TYPE_C; } - int is_accept(void) { return false; } - int is_postprocess(void) { return false; } - }; -@@ -394,7 +438,7 @@ public: - /* Match one specific character (/c/). */ - class CharNode: public CNode { - public: -- CharNode(transchar c): c(c) { } -+ CharNode(transchar c): c(c) { type_flags |= NODE_TYPE_CHAR; } - void follow(Cases &cases) - { - NodeSet **x = &cases.cases[c]; -@@ -408,8 +452,8 @@ public: - } - int eq(Node *other) - { -- CharNode *o = dynamic_cast(other); -- if (o) { -+ if (other->is_type(NODE_TYPE_CHAR)) { -+ CharNode *o = static_cast(other); - return c == o->c; - } - return 0; -@@ -439,7 +483,10 @@ public: - /* Match a set of characters (/[abc]/). */ - class CharSetNode: public CNode { - public: -- CharSetNode(Chars &chars): chars(chars) { } -+ CharSetNode(Chars &chars): chars(chars) -+ { -+ type_flags |= NODE_TYPE_CHARSET; -+ } - void follow(Cases &cases) - { - for (Chars::iterator i = chars.begin(); i != chars.end(); i++) { -@@ -455,8 +502,11 @@ public: - } - int eq(Node *other) - { -- CharSetNode *o = dynamic_cast(other); -- if (!o || chars.size() != o->chars.size()) -+ if (!other->is_type(NODE_TYPE_CHARSET)) -+ return 0; -+ -+ CharSetNode *o = static_cast(other); -+ if (chars.size() != o->chars.size()) - return 0; - - for (Chars::iterator i = chars.begin(), j = o->chars.begin(); -@@ -498,7 +548,10 @@ public: - /* Match all except one character (/[^abc]/). */ - class NotCharSetNode: public CNode { - public: -- NotCharSetNode(Chars &chars): chars(chars) { } -+ NotCharSetNode(Chars &chars): chars(chars) -+ { -+ type_flags |= NODE_TYPE_NOTCHARSET; -+ } - void follow(Cases &cases) - { - if (!cases.otherwise) -@@ -522,8 +575,11 @@ public: - } - int eq(Node *other) - { -- NotCharSetNode *o = dynamic_cast(other); -- if (!o || chars.size() != o->chars.size()) -+ if (!other->is_type(NODE_TYPE_NOTCHARSET)) -+ return 0; -+ -+ NotCharSetNode *o = static_cast(other); -+ if (chars.size() != o->chars.size()) - return 0; - - for (Chars::iterator i = chars.begin(), j = o->chars.begin(); -@@ -565,7 +621,7 @@ public: - /* Match any character (/./). */ - class AnyCharNode: public CNode { - public: -- AnyCharNode() { } -+ AnyCharNode() { type_flags |= NODE_TYPE_ANYCHAR; } - void follow(Cases &cases) - { - if (!cases.otherwise) -@@ -579,7 +635,7 @@ public: - } - int eq(Node *other) - { -- if (dynamic_cast(other)) -+ if (other->is_type(NODE_TYPE_ANYCHAR)) - return 1; - return 0; - } -@@ -589,7 +645,11 @@ public: - /* Match a node zero or more times. (This is a unary operator.) */ - class StarNode: public OneChildNode { - public: -- StarNode(Node *left): OneChildNode(left) { nullable = true; } -+ StarNode(Node *left): OneChildNode(left) -+ { -+ type_flags |= NODE_TYPE_STAR; -+ nullable = true; -+ } - void compute_firstpos() { firstpos = child[0]->firstpos; } - void compute_lastpos() { lastpos = child[0]->lastpos; } - void compute_followpos() -@@ -601,7 +661,7 @@ public: - } - int eq(Node *other) - { -- if (dynamic_cast(other)) -+ if (other->is_type(NODE_TYPE_STAR)) - return child[0]->eq(other->child[0]); - return 0; - } -@@ -618,12 +678,16 @@ public: - /* Match a node zero or one times. */ - class OptionalNode: public OneChildNode { - public: -- OptionalNode(Node *left): OneChildNode(left) { nullable = true; } -+ OptionalNode(Node *left): OneChildNode(left) -+ { -+ type_flags |= NODE_TYPE_OPTIONAL; -+ nullable = true; -+ } - void compute_firstpos() { firstpos = child[0]->firstpos; } - void compute_lastpos() { lastpos = child[0]->lastpos; } - int eq(Node *other) - { -- if (dynamic_cast(other)) -+ if (other->is_type(NODE_TYPE_OPTIONAL)) - return child[0]->eq(other->child[0]); - return 0; - } -@@ -638,7 +702,9 @@ public: - /* Match a node one or more times. (This is a unary operator.) */ - class PlusNode: public OneChildNode { - public: -- PlusNode(Node *left): OneChildNode(left) { -+ PlusNode(Node *left): OneChildNode(left) -+ { -+ type_flags |= NODE_TYPE_PLUS; - } - void compute_nullable() { nullable = child[0]->nullable; } - void compute_firstpos() { firstpos = child[0]->firstpos; } -@@ -651,7 +717,7 @@ public: - } - } - int eq(Node *other) { -- if (dynamic_cast(other)) -+ if (other->is_type(NODE_TYPE_PLUS)) - return child[0]->eq(other->child[0]); - return 0; - } -@@ -667,7 +733,10 @@ public: - /* Match a pair of consecutive nodes. */ - class CatNode: public TwoChildNode { - public: -- CatNode(Node *left, Node *right): TwoChildNode(left, right) { } -+ CatNode(Node *left, Node *right): TwoChildNode(left, right) -+ { -+ type_flags |= NODE_TYPE_CAT; -+ } - void compute_nullable() - { - nullable = child[0]->nullable && child[1]->nullable; -@@ -695,7 +764,7 @@ public: - } - int eq(Node *other) - { -- if (dynamic_cast(other)) { -+ if (other->is_type(NODE_TYPE_CAT)) { - if (!child[0]->eq(other->child[0])) - return 0; - return child[1]->eq(other->child[1]); -@@ -730,7 +799,10 @@ public: - /* Match one of two alternative nodes. */ - class AltNode: public TwoChildNode { - public: -- AltNode(Node *left, Node *right): TwoChildNode(left, right) { } -+ AltNode(Node *left, Node *right): TwoChildNode(left, right) -+ { -+ type_flags |= NODE_TYPE_ALT; -+ } - void compute_nullable() - { - nullable = child[0]->nullable || child[1]->nullable; -@@ -745,7 +817,7 @@ public: - } - int eq(Node *other) - { -- if (dynamic_cast(other)) { -+ if (other->is_type(NODE_TYPE_ALT)) { - if (!child[0]->eq(other->child[0])) - return 0; - return child[1]->eq(other->child[1]); -@@ -780,7 +852,10 @@ public: - - class SharedNode: public ImportantNode { - public: -- SharedNode() { } -+ SharedNode() -+ { -+ type_flags |= NODE_TYPE_SHARED; -+ } - void release(void) - { - /* don't delete SharedNodes via release as they are shared, and -@@ -803,14 +878,17 @@ public: - */ - class AcceptNode: public SharedNode { - public: -- AcceptNode() { } -+ AcceptNode() { type_flags |= NODE_TYPE_ACCEPT; } - int is_accept(void) { return true; } - int is_postprocess(void) { return false; } - }; - - class MatchFlag: public AcceptNode { - public: -- MatchFlag(uint32_t flag, uint32_t audit): flag(flag), audit(audit) { } -+ MatchFlag(uint32_t flag, uint32_t audit): flag(flag), audit(audit) -+ { -+ type_flags |= NODE_TYPE_MATCHFLAG; -+ } - ostream &dump(ostream &os) { return os << "< 0x" << hex << flag << '>'; } - - uint32_t flag; -@@ -819,12 +897,18 @@ public: - - class ExactMatchFlag: public MatchFlag { - public: -- ExactMatchFlag(uint32_t flag, uint32_t audit): MatchFlag(flag, audit) {} -+ ExactMatchFlag(uint32_t flag, uint32_t audit): MatchFlag(flag, audit) -+ { -+ type_flags |= NODE_TYPE_EXACTMATCHFLAG; -+ } - }; - - class DenyMatchFlag: public MatchFlag { - public: -- DenyMatchFlag(uint32_t flag, uint32_t quiet): MatchFlag(flag, quiet) {} -+ DenyMatchFlag(uint32_t flag, uint32_t quiet): MatchFlag(flag, quiet) -+ { -+ type_flags |= NODE_TYPE_DENYMATCHFLAG; -+ } - }; - - /* Traverse the syntax tree depth-first in an iterator-like manner. */ -@@ -833,7 +917,7 @@ class depth_first_traversal { - void push_left(Node *node) { - pos.push(node); - -- while (dynamic_cast(node)) { -+ while (node->is_type(NODE_TYPE_INNER)) { - pos.push(node->child[0]); - node = node->child[0]; - } -diff --git a/parser/libapparmor_re/hfa.cc b/parser/libapparmor_re/hfa.cc -index 9cea4c3fc..e1ef1803b 100644 ---- a/parser/libapparmor_re/hfa.cc -+++ b/parser/libapparmor_re/hfa.cc -@@ -1352,17 +1352,18 @@ int accept_perms(NodeSet *state, perms_t &perms, bool filedfa) - return error; - - for (NodeSet::iterator i = state->begin(); i != state->end(); i++) { -- MatchFlag *match; -- if (!(match = dynamic_cast(*i))) -+ if (!(*i)->is_type(NODE_TYPE_MATCHFLAG)) - continue; -- if (dynamic_cast(match)) { -+ -+ MatchFlag *match = static_cast(*i); -+ if (match->is_type(NODE_TYPE_EXACTMATCHFLAG)) { - /* exact match only ever happens with x */ - if (filedfa && !is_merged_x_consistent(exact_match_allow, - match->flag)) - error = 1;; - exact_match_allow |= match->flag; - exact_audit |= match->audit; -- } else if (dynamic_cast(match)) { -+ } else if (match->is_type(NODE_TYPE_DENYMATCHFLAG)) { - perms.deny |= match->flag; - perms.quiet |= match->audit; - } else { --- -2.34.1 diff --git a/build-aux/snap/snapcraft.yaml b/build-aux/snap/snapcraft.yaml index 6b98eabb70b..a09685a9865 100644 --- a/build-aux/snap/snapcraft.yaml +++ b/build-aux/snap/snapcraft.yaml @@ -128,25 +128,14 @@ parts: apparmor: plugin: autotools build-packages: + - autoconf-archive - bison - flex - gettext - g++ - pkg-config - wget - source: https://launchpad.net/apparmor/3.0/3.0.8/+download/apparmor-3.0.8.tar.gz - override-pull: | - craftctl default - # add support for mediating posix mqueue's and userns - these come from - # the ubuntu source package for lunar - # https://git.launchpad.net/ubuntu/+source/apparmor/tree/debian/patches/ubuntu?h=ubuntu/lunar - for feature in mqueue userns; do - wget https://git.launchpad.net/ubuntu/+source/apparmor/plain/debian/patches/ubuntu/add-${feature}-support.patch?h=ubuntu/lunar -O - | patch -p1 - done - # apply local apparmor patches - for patch in "${CRAFT_PROJECT_DIR}"/build-aux/snap/local/patches/apparmor/*; do - patch -p1 < $patch - done + source: https://gitlab.com/apparmor/apparmor/-/archive/v4.0.1/apparmor-v4.0.1.tar.gz override-build: | cd "${CRAFT_PART_BUILD}"/libraries/libapparmor ./autogen.sh diff --git a/cmd/configure.ac b/cmd/configure.ac index 11337454826..8a3d1e586e8 100644 --- a/cmd/configure.ac +++ b/cmd/configure.ac @@ -82,11 +82,11 @@ AS_IF([test "x$with_unit_tests" = "xyes"], [ # Check if apparmor userspace library is available. AS_IF([test "x$enable_apparmor" = "xyes"], [ - # Expect AppArmor3 when building as a snap under snapcraft + # Expect AppArmor4 when building as a snap under snapcraft AS_IF([test "x$SNAPCRAFT_PROJECT_NAME" = "xsnapd"], [ - PKG_CHECK_MODULES([APPARMOR3], [libapparmor = 3.0.8], [ - AC_DEFINE([HAVE_APPARMOR], [1], [Build with apparmor3 support])], [ - AC_MSG_ERROR([unable to find apparmor3 for snap build of snapd])])], [ + PKG_CHECK_MODULES([APPARMOR4], [libapparmor = 4.0.1], [ + AC_DEFINE([HAVE_APPARMOR], [1], [Build with apparmor4 support])], [ + AC_MSG_ERROR([unable to find apparmor4 for snap build of snapd])])], [ PKG_CHECK_MODULES([APPARMOR], [libapparmor], [ AC_DEFINE([HAVE_APPARMOR], [1], [Build with apparmor support])])]) ], [ diff --git a/interfaces/builtin/docker_support.go b/interfaces/builtin/docker_support.go index e6712c41858..259369fa0a8 100644 --- a/interfaces/builtin/docker_support.go +++ b/interfaces/builtin/docker_support.go @@ -62,6 +62,11 @@ const dockerSupportConnectedPlugAppArmorUserNS = ` userns, ` +const dockerSupportConnectedPlugAppArmorMqueue = ` +# allow unrestricted use of posix message queues +mqueue, +` + const dockerSupportConnectedPlugAppArmor = ` # Description: allow operating as the Docker daemon/containerd. This policy is # intentionally not restrictive and is here to help guard against programming @@ -831,6 +836,9 @@ func (iface *dockerSupportInterface) AppArmorConnectedPlug(spec *apparmor.Specif if strutil.ListContains(features, "userns") { spec.AddSnippet(dockerSupportConnectedPlugAppArmorUserNS) } + if strutil.ListContains(features, "mqueue") { + spec.AddSnippet(dockerSupportConnectedPlugAppArmorMqueue) + } } spec.SetUsesPtraceTrace() diff --git a/interfaces/builtin/docker_support_test.go b/interfaces/builtin/docker_support_test.go index c84370009df..413a2382868 100644 --- a/interfaces/builtin/docker_support_test.go +++ b/interfaces/builtin/docker_support_test.go @@ -320,6 +320,11 @@ func (s *DockerSupportInterfaceSuite) TestGenerateAAREExclusionPatterns(c *C) { const dockerSupportConnectedPlugAppArmorUserNS = ` # allow use of user namespaces userns, +` + + const dockerSupportConnectedPlugAppArmorMqueue = ` +# allow unrestricted use of posix message queues +mqueue, ` const dockerSupportConnectedPlugAppArmor = ` @@ -845,17 +850,22 @@ ptrace (read, trace) peer=unconfined, // Generate profile to compare with privilegedProfile := dockerSupportPrivilegedAppArmor + dockerSupportConnectedPlugAppArmor - // if apparmor supports userns mediation then add this too if (apparmor_sandbox.ProbedLevel() != apparmor_sandbox.Partial) && (apparmor_sandbox.ProbedLevel() != apparmor_sandbox.Full) { c.Skip(apparmor_sandbox.Summary()) } + // if apparmor supports userns mediation then add this too features, err := apparmor_sandbox.ParserFeatures() c.Assert(err, IsNil) if strutil.ListContains(features, "userns") { privilegedProfile += dockerSupportConnectedPlugAppArmorUserNS } + // if apparmor supports mqueue mediation then add this too + if strutil.ListContains(features, "mqueue") { + privilegedProfile += dockerSupportConnectedPlugAppArmorMqueue + } + // Profile existing profile expectedHash, err := testutil.AppArmorParseAndHashHelper("#include \nprofile docker_support {" + privilegedProfile + "}") c.Assert(err, IsNil) diff --git a/sandbox/apparmor/apparmor.go b/sandbox/apparmor/apparmor.go index 45b1b5a9941..92deea6f82e 100644 --- a/sandbox/apparmor/apparmor.go +++ b/sandbox/apparmor/apparmor.go @@ -654,6 +654,7 @@ func probeParserFeatures() ([]string, error) { feature string flags []string probe string + minVer string }{ { feature: "unsafe", @@ -670,6 +671,7 @@ func probeParserFeatures() ([]string, error) { { feature: "mqueue", probe: "mqueue,", + minVer: "4.0.1", }, { feature: "cap-bpf", @@ -701,11 +703,29 @@ func probeParserFeatures() ([]string, error) { if err != nil { return []string{}, err } + + aaVer := appArmorParserVersion() + logger.Debugf("apparmor parser version: %q", aaVer) + features := make([]string, 0, len(featureProbes)+1) for _, fp := range featureProbes { + if minVer := fp.minVer; minVer != "" { + res, err := strutil.VersionCompare(aaVer, minVer) + if err != nil { + logger.Noticef("cannot compare versions: %s", err) + continue + } + if res < 0 { + logger.Debugf("skipping apparmor feature check for %s due to insufficient version %s", fp.feature, aaVer) + continue + } + } // recreate the Cmd each time so we can exec it each time cmd, _, _ := AppArmorParser() - if tryAppArmorParserFeature(cmd, fp.flags, fp.probe) { + err := tryAppArmorParserFeature(cmd, fp.flags, fp.probe) + if err != nil { + logger.Debugf("cannot probe apparmor feature %q: %v", fp.feature, err) + } else { features = append(features, fp.feature) } } @@ -713,6 +733,7 @@ func probeParserFeatures() ([]string, error) { features = append(features, "snapd-internal") } sort.Strings(features) + logger.Debugf("probed apparmor parser features for version %s (internal=%v): %v", aaVer, internal, features) return features, nil } @@ -761,16 +782,35 @@ func AppArmorParser() (cmd *exec.Cmd, internal bool, err error) { if path, err := snapdtool.InternalToolPath("apparmor_parser"); err == nil { if osutil.IsExecutable(path) && snapdAppArmorSupportsReexec() && !systemAppArmorLoadsSnapPolicy() { prefix := strings.TrimSuffix(path, "apparmor_parser") - // when using the internal apparmor_parser also use - // its own configuration and includes etc plus - // also ensure we use the 3.0 feature ABI to get - // the widest array of policy features across the - // widest array of kernel versions + snapdAbi30File := filepath.Join(prefix, "/apparmor.d/abi/3.0") + snapdAbi40File := filepath.Join(prefix, "/apparmor.d/abi/4.0") + + // When using the internal apparmor_parser also use its own + // configuration and includes etc plus also ensure we use the 4.0 + // feature ABI to get the widest array of policy features across + // the widest array of kernel versions. + // + // In case snapd is injected into snapd snap, with + // older apparmor, use that instead so that things + // don't generally fail. + abiFile := "" + fi40, err40 := os.Lstat(snapdAbi40File) + fi30, err30 := os.Lstat(snapdAbi30File) + switch { + case err40 == nil && !fi40.IsDir(): + abiFile = snapdAbi40File + case err30 == nil && !fi30.IsDir(): + abiFile = snapdAbi30File + default: + return nil, false, fmt.Errorf("internal snapd apparmor_parser but no abi files") + } + args := []string{ "--config-file", filepath.Join(prefix, "/apparmor/parser.conf"), "--base", filepath.Join(prefix, "/apparmor.d"), - "--policy-features", filepath.Join(prefix, "/apparmor.d/abi/3.0"), + "--policy-features", abiFile, } + return exec.Command(path, args...), true, nil } } @@ -803,8 +843,23 @@ func AppArmorParser() (cmd *exec.Cmd, internal bool, err error) { return nil, false, os.ErrNotExist } +func appArmorParserVersion() string { + cmd, _, _ := AppArmorParser() + cmd.Args = append(cmd.Args, "--version") + output, err := cmd.CombinedOutput() + if err != nil { + return "" + } + logger.Debugf("apparmor_parser --version\n%s", output) + // output is like "AppArmor parser version 2.13.4\n" + // "Copyright ..." + // get the version number from the first line + parts := strings.Split(strings.Split(string(output), "\n")[0], " ") + return parts[len(parts)-1] +} + // tryAppArmorParserFeature attempts to pre-process a bit of apparmor syntax with a given parser. -func tryAppArmorParserFeature(cmd *exec.Cmd, flags []string, rule string) bool { +func tryAppArmorParserFeature(cmd *exec.Cmd, flags []string, rule string) error { cmd.Args = append(cmd.Args, "--preprocess") flagSnippet := "" if len(flags) > 0 { @@ -816,9 +871,9 @@ func tryAppArmorParserFeature(cmd *exec.Cmd, flags []string, rule string) bool { // older versions of apparmor_parser can exit with success even // though they fail to parse if err != nil || strings.Contains(string(output), "parser error") { - return false + return fmt.Errorf("apparmor_parser failed: %v: %s", err, output) } - return true + return nil } // UpdateHomedirsTunable sets the AppArmor HOMEDIRS tunable to the list of the diff --git a/sandbox/apparmor/apparmor_test.go b/sandbox/apparmor/apparmor_test.go index 7332ea36ba5..1ffcb1ce0dc 100644 --- a/sandbox/apparmor/apparmor_test.go +++ b/sandbox/apparmor/apparmor_test.go @@ -75,17 +75,20 @@ func (*apparmorSuite) TestAppArmorParser(c *C) { c.Check(err, Equals, nil) } -func (*apparmorSuite) TestAppArmorInternalAppArmorParser(c *C) { +func (*apparmorSuite) TestAppArmorInternalAppArmorParserAbi3(c *C) { fakeroot := c.MkDir() dirs.SetRootDir(fakeroot) - d := filepath.Join(dirs.SnapMountDir, "/snapd/42", "/usr/lib/snapd") - c.Assert(os.MkdirAll(d, 0755), IsNil) - p := filepath.Join(d, "apparmor_parser") - c.Assert(os.WriteFile(p, nil, 0755), IsNil) + libSnapdDir := filepath.Join(dirs.SnapMountDir, "/snapd/42/usr/lib/snapd") + parser := filepath.Join(libSnapdDir, "apparmor_parser") + c.Assert(os.MkdirAll(libSnapdDir, 0755), IsNil) + c.Assert(os.WriteFile(parser, nil, 0755), IsNil) + c.Assert(os.MkdirAll(filepath.Join(libSnapdDir, "apparmor.d/abi"), 755), IsNil) + c.Assert(os.WriteFile(filepath.Join(libSnapdDir, "apparmor.d/abi/3.0"), nil, 0644), IsNil) + restore := snapdtool.MockOsReadlink(func(path string) (string, error) { c.Assert(path, Equals, "/proc/self/exe") - return filepath.Join(d, "snapd"), nil + return filepath.Join(libSnapdDir, "snapd"), nil }) defer restore() restore = apparmor.MockSnapdAppArmorSupportsReexec(func() bool { return true }) @@ -93,12 +96,45 @@ func (*apparmorSuite) TestAppArmorInternalAppArmorParser(c *C) { cmd, internal, err := apparmor.AppArmorParser() c.Check(err, IsNil) - c.Check(cmd.Path, Equals, p) + c.Check(cmd.Path, Equals, parser) c.Check(cmd.Args, DeepEquals, []string{ - p, - "--config-file", filepath.Join(d, "/apparmor/parser.conf"), - "--base", filepath.Join(d, "/apparmor.d"), - "--policy-features", filepath.Join(d, "/apparmor.d/abi/3.0"), + parser, + "--config-file", filepath.Join(libSnapdDir, "/apparmor/parser.conf"), + "--base", filepath.Join(libSnapdDir, "/apparmor.d"), + "--policy-features", filepath.Join(libSnapdDir, "/apparmor.d/abi/3.0"), + }) + c.Check(internal, Equals, true) +} + +func (*apparmorSuite) TestAppArmorInternalAppArmorParserAbi4(c *C) { + fakeroot := c.MkDir() + dirs.SetRootDir(fakeroot) + + libSnapdDir := filepath.Join(dirs.SnapMountDir, "/snapd/42/usr/lib/snapd") + parser := filepath.Join(libSnapdDir, "apparmor_parser") + c.Assert(os.MkdirAll(libSnapdDir, 0755), IsNil) + c.Assert(os.WriteFile(parser, nil, 0755), IsNil) + c.Assert(os.MkdirAll(filepath.Join(libSnapdDir, "apparmor.d/abi"), 755), IsNil) + c.Assert(os.WriteFile(filepath.Join(libSnapdDir, "apparmor.d/abi/3.0"), nil, 0644), IsNil) + c.Assert(os.WriteFile(filepath.Join(libSnapdDir, "apparmor.d/abi/4.0"), nil, 0644), IsNil) + + restore := snapdtool.MockOsReadlink(func(path string) (string, error) { + c.Assert(path, Equals, "/proc/self/exe") + return filepath.Join(libSnapdDir, "snapd"), nil + }) + defer restore() + restore = apparmor.MockSnapdAppArmorSupportsReexec(func() bool { return true }) + defer restore() + + cmd, internal, err := apparmor.AppArmorParser() + c.Check(err, IsNil) + c.Check(cmd.Path, Equals, parser) + c.Check(cmd.Args, DeepEquals, []string{ + parser, + "--config-file", filepath.Join(libSnapdDir, "/apparmor/parser.conf"), + "--base", filepath.Join(libSnapdDir, "/apparmor.d"), + // 4.0 was preferred. + "--policy-features", filepath.Join(libSnapdDir, "/apparmor.d/abi/4.0"), }) c.Check(internal, Equals, true) } @@ -313,11 +349,36 @@ func (s *apparmorSuite) TestProbeAppArmorKernelFeatures(c *C) { } func probeOneParserFeature(c *C, known *[]string, parserPath, featureName, profileText string) { + probeOneVersionDependentParserFeature(c, known, parserPath, "0.0.0", featureName, profileText) +} + +// fakeParserScript returns a shell script mimicking apparmor_parser. +// +// The returned script is prepared such, that additional logic may be safely +// appended to it. +func fakeParserScript(parserVersion string) string { const script = `#!/bin/sh set -e -test "$(cat | tr -d '\n')" = '%s' +if [ "${1:-}" = "--version" ]; then + cat <<__VERSION__ +AppArmor parser version %s +Copyright (C) 1999-2008 Novell Inc. +Copyright 2009-2018 Canonical Ltd. +__VERSION__ + exit 0 +fi ` - err := os.WriteFile(parserPath, []byte(fmt.Sprintf(script, profileText)), 0o755) + return fmt.Sprintf(script, parserVersion) +} + +func fakeParserAnticipatingProfileScript(parserVersion, profileText string) string { + textCompareLogic := fmt.Sprintf(`test "$(cat | tr -d '\n')" = '%s'`, profileText) + + return fakeParserScript(parserVersion) + textCompareLogic +} + +func probeOneVersionDependentParserFeature(c *C, known *[]string, parserPath, parserVersion, featureName, profileText string) { + err := os.WriteFile(parserPath, []byte(fakeParserAnticipatingProfileScript(parserVersion, profileText)), 0o700) c.Assert(err, IsNil) cmd, _, _ := apparmor.AppArmorParser() @@ -353,6 +414,23 @@ func (s *parserFeatureTestSuite) SetUpTest(c *C) { s.AddCleanup(restore) } +func (s *parserFeatureTestSuite) TestProbeMqueueWith4Beta(c *C) { + const parserVersion = "4.0.0~beta3" + const profileText = `profile snap-test { mqueue,}` + + parserPath := filepath.Join(s.binDir, "apparmor_parser") + + err := os.WriteFile(parserPath, []byte(fakeParserAnticipatingProfileScript(parserVersion, profileText)), 0o700) + c.Assert(err, IsNil) + + cmd, _, _ := apparmor.AppArmorParser() + c.Assert(cmd.Path, Equals, parserPath, Commentf("Unexpectedly using apparmor parser from %s", cmd.Path)) + + features, err := apparmor.ProbeParserFeatures() + c.Assert(err, IsNil) + c.Check(features, HasLen, 0, Commentf("Mqueue feature unexpectedly enabled by fake 4.0.0~beta3 parser")) +} + func (s *parserFeatureTestSuite) TestProbeFeature(c *C) { // Pretend we can only support one feature at a time. var knownProbes []string @@ -362,7 +440,7 @@ func (s *parserFeatureTestSuite) TestProbeFeature(c *C) { probeOneParserFeature(c, &knownProbes, parserPath, "cap-audit-read", `profile snap-test { capability audit_read,}`) probeOneParserFeature(c, &knownProbes, parserPath, "cap-bpf", `profile snap-test { capability bpf,}`) probeOneParserFeature(c, &knownProbes, parserPath, "include-if-exists", `profile snap-test { #include if exists "/foo"}`) - probeOneParserFeature(c, &knownProbes, parserPath, "mqueue", `profile snap-test { mqueue,}`) + probeOneVersionDependentParserFeature(c, &knownProbes, parserPath, "4.0.1", "mqueue", `profile snap-test { mqueue,}`) probeOneParserFeature(c, &knownProbes, parserPath, "prompt", `profile snap-test { prompt /foo r,}`) probeOneParserFeature(c, &knownProbes, parserPath, "qipcrtr-socket", `profile snap-test { network qipcrtr dgram,}`) probeOneParserFeature(c, &knownProbes, parserPath, "unconfined", `profile snap-test flags=(unconfined) { # test unconfined}`) @@ -371,7 +449,7 @@ func (s *parserFeatureTestSuite) TestProbeFeature(c *C) { probeOneParserFeature(c, &knownProbes, parserPath, "xdp", `profile snap-test { network xdp,}`) // Pretend we have all the features. - err := os.WriteFile(parserPath, []byte("#!/bin/sh\nexit 0\n"), 0o755) + err := os.WriteFile(parserPath, []byte(fakeParserScript("4.0.1")), 0o755) c.Assert(err, IsNil) // Did any feature probes got added to non-test code? @@ -393,10 +471,11 @@ func (s *parserFeatureTestSuite) TestNoParser(c *C) { func (s *parserFeatureTestSuite) TestInternalParser(c *C) { // Put a fake parser at $SNAP_MOUNT_DIR/snapd/42/usr/lib/snapd/apparmor_parser libSnapdDir := filepath.Join(dirs.SnapMountDir, "/snapd/42/usr/lib/snapd") - err := os.MkdirAll(libSnapdDir, 0755) - c.Assert(err, IsNil) - err = os.WriteFile(filepath.Join(libSnapdDir, "apparmor_parser"), nil, 0755) - c.Assert(err, IsNil) + parser := filepath.Join(libSnapdDir, "apparmor_parser") + c.Assert(os.MkdirAll(libSnapdDir, 0755), IsNil) + c.Assert(os.WriteFile(parser, nil, 0755), IsNil) + c.Assert(os.MkdirAll(filepath.Join(libSnapdDir, "apparmor.d/abi"), 755), IsNil) + c.Assert(os.WriteFile(filepath.Join(libSnapdDir, "apparmor.d/abi/4.0"), nil, 0644), IsNil) // Pretend that we are running snapd from that snap location. restore := snapdtool.MockOsReadlink(func(path string) (string, error) { @@ -427,7 +506,7 @@ func (s *apparmorSuite) TestInterfaceSystemKey(c *C) { c.Assert(os.MkdirAll(filepath.Join(d, featuresSysPath, "policy"), 0755), IsNil) c.Assert(os.MkdirAll(filepath.Join(d, featuresSysPath, "network"), 0755), IsNil) - mockParserCmd := testutil.MockCommand(c, "apparmor_parser", "") + mockParserCmd := testutil.MockCommand(c, "apparmor_parser", fakeParserScript("4.0.1")) defer mockParserCmd.Restore() restore = apparmor.MockParserSearchPath(mockParserCmd.BinDir()) defer restore() @@ -469,7 +548,7 @@ func (s *apparmorSuite) TestFeaturesProbedOnce(c *C) { c.Assert(os.MkdirAll(filepath.Join(d, featuresSysPath, "policy"), 0755), IsNil) c.Assert(os.MkdirAll(filepath.Join(d, featuresSysPath, "network"), 0755), IsNil) - mockParserCmd := testutil.MockCommand(c, "apparmor_parser", "") + mockParserCmd := testutil.MockCommand(c, "apparmor_parser", fakeParserScript("4.0.1")) defer mockParserCmd.Restore() restore = apparmor.MockParserSearchPath(mockParserCmd.BinDir()) defer restore() @@ -644,13 +723,15 @@ func (s *apparmorSuite) TestSetupConfCacheDirsWithInternalApparmor(c *C) { fakeroot := c.MkDir() dirs.SetRootDir(fakeroot) - d := filepath.Join(dirs.SnapMountDir, "/snapd/42", "/usr/lib/snapd") - c.Assert(os.MkdirAll(d, 0755), IsNil) - p := filepath.Join(d, "apparmor_parser") - c.Assert(os.WriteFile(p, nil, 0755), IsNil) + libSnapdDir := filepath.Join(dirs.SnapMountDir, "/snapd/42/usr/lib/snapd") + parser := filepath.Join(libSnapdDir, "apparmor_parser") + c.Assert(os.MkdirAll(libSnapdDir, 0755), IsNil) + c.Assert(os.WriteFile(parser, nil, 0755), IsNil) + c.Assert(os.MkdirAll(filepath.Join(libSnapdDir, "apparmor.d/abi"), 755), IsNil) + c.Assert(os.WriteFile(filepath.Join(libSnapdDir, "apparmor.d/abi/4.0"), nil, 0644), IsNil) restore := snapdtool.MockOsReadlink(func(path string) (string, error) { c.Assert(path, Equals, "/proc/self/exe") - return filepath.Join(d, "snapd"), nil + return filepath.Join(libSnapdDir, "snapd"), nil }) defer restore() restore = apparmor.MockSnapdAppArmorSupportsReexec(func() bool { return true }) diff --git a/tests/main/interfaces-network-control-ip-netns/task.yaml b/tests/main/interfaces-network-control-ip-netns/task.yaml index 26c934f7dd3..d254d80fb2f 100644 --- a/tests/main/interfaces-network-control-ip-netns/task.yaml +++ b/tests/main/interfaces-network-control-ip-netns/task.yaml @@ -13,7 +13,7 @@ prepare: | # ensure the right apparmor_parser is used APPARMOR_PARSER="apparmor_parser" if snap debug sandbox-features --required apparmor:parser:snapd-internal; then - APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/3.0" + APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/4.0" fi $APPARMOR_PARSER -r /var/lib/snapd/apparmor/profiles/snap.snapd-hacker-toolbelt.busybox fi diff --git a/tests/main/security-device-cgroups-helper/task.yaml b/tests/main/security-device-cgroups-helper/task.yaml index 10c8dab1946..ad6321a5a7f 100644 --- a/tests/main/security-device-cgroups-helper/task.yaml +++ b/tests/main/security-device-cgroups-helper/task.yaml @@ -55,7 +55,7 @@ execute: | # ensure the right apparmor_parser is used APPARMOR_PARSER="apparmor_parser" if snap debug sandbox-features --required apparmor:parser:snapd-internal; then - APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/3.0" + APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/4.0" fi $APPARMOR_PARSER -r /var/lib/snapd/apparmor/profiles/snap.test-strict-cgroup-helper.sh fi diff --git a/tests/main/security-seccomp/task.yaml b/tests/main/security-seccomp/task.yaml index 762c5c561cb..2ef31fee51b 100644 --- a/tests/main/security-seccomp/task.yaml +++ b/tests/main/security-seccomp/task.yaml @@ -42,7 +42,7 @@ prepare: | # ensure the right apparmor_parser is used APPARMOR_PARSER="apparmor_parser" if snap debug sandbox-features --required apparmor:parser:snapd-internal; then - APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/3.0" + APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/4.0" fi $APPARMOR_PARSER -K -r "$AAP" fi @@ -57,7 +57,7 @@ restore: | # ensure the right apparmor_parser is used APPARMOR_PARSER="apparmor_parser" if snap debug sandbox-features --required apparmor:parser:snapd-internal; then - APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/3.0" + APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/4.0" fi $APPARMOR_PARSER -K -r "$AAP" fi diff --git a/tests/main/security-setuid-root/task.yaml b/tests/main/security-setuid-root/task.yaml index 68620de70c0..999af8631ec 100644 --- a/tests/main/security-setuid-root/task.yaml +++ b/tests/main/security-setuid-root/task.yaml @@ -15,7 +15,7 @@ prepare: | # ensure the right apparmor_parser is used APPARMOR_PARSER="apparmor_parser" if snap debug sandbox-features --required apparmor:parser:snapd-internal; then - APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/3.0" + APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/4.0" fi for p in /var/lib/snapd/apparmor/profiles/snap-confine.*; do $APPARMOR_PARSER -R "$p" @@ -26,7 +26,7 @@ restore: | # ensure the right apparmor_parser is used APPARMOR_PARSER="apparmor_parser" if snap debug sandbox-features --required apparmor:parser:snapd-internal; then - APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/3.0" + APPARMOR_PARSER="/snap/snapd/current/usr/lib/snapd/apparmor_parser --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf -b /snap/snapd/current/usr/lib/snapd/apparmor.d --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/4.0" fi for p in /var/lib/snapd/apparmor/profiles/snap-confine.*; do $APPARMOR_PARSER -r "$p" diff --git a/tests/main/snapd-snap/task.yaml b/tests/main/snapd-snap/task.yaml index da5fba3e212..5f2a3e2bf74 100644 --- a/tests/main/snapd-snap/task.yaml +++ b/tests/main/snapd-snap/task.yaml @@ -249,7 +249,7 @@ execute: | /snap/snapd/current/usr/lib/snapd/apparmor_parser \ --config-file /snap/snapd/current/usr/lib/snapd/apparmor/parser.conf \ -b /snap/snapd/current/usr/lib/snapd/apparmor.d \ - --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/3.0 \ + --policy-features /snap/snapd/current/usr/lib/snapd/apparmor.d/abi/4.0 \ -r /var/lib/snapd/apparmor/profiles/snap.snapcraft.snapcraft echo "Then we should be able to successfully install a snap"