Skip to content

Commit 17618db

Browse files
authored
run-checks: check priority override on ifaces w/ mountinfo rules (#17340)
* run-checks: check prio override on ifaces w/ mountinfo rules Interfaces that grant access to /proc/self/mountinfo (explicitly or not) must have a prioritized override (see basePrioritizedSnippets in interfaces/apparmor/template.go) which swaps out the deny rule. Otherwise, the deny rule wins out and the interface does not work as intended. This adds a static check to run-checks that tries to maintain this invariant by looking for interfaces that grant /proc/self/mountinfo, even if by granting broader access (/proc/**, allow file, etc), and checking whether the file also contains the corresponding override. Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
1 parent ab7429c commit 17618db

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

run-checks

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,58 @@ missing_interface_spread_test() {
182182
done
183183
}
184184

185+
check_mountinfo_override() {
186+
# This check verifies that interfaces that grant access to /proc/self/mountinfo
187+
# (directly or not) also have a prioritized override (see basePrioritizedSnippets
188+
# in interfaces/apparmor/template.go for why this is necessary). We do this
189+
# by looking for a "AddPrioritizedSnippet(*, apparmor.MountInfoKey, ...)" call
190+
# for any such interfaces.
191+
192+
local missing_override=""
193+
local f
194+
for f in interfaces/builtin/*.go; do
195+
if [[ "$f" == *"_test.go" ]]; then
196+
continue
197+
fi
198+
199+
local out
200+
out=$(awk '
201+
# if a prioritized override is present, we can stop early
202+
/AddPrioritizedSnippet\(.* apparmor\.MountInfoKey/ { m=""; exit }
203+
204+
# We look for the following types of rules:
205+
# * Explicit mountinfo rules. For example, any combination of:
206+
# owner /proc/self/mountinfo r,
207+
# @{PROC}/@{pid} rw,
208+
# /proc/1234/mountinfo
209+
# * References to the "mountInfoSnippet" variable which contains
210+
# the same rules as the previous item
211+
# * Broad /proc grants:
212+
# owner @{PROC}/** r,
213+
# /proc/** rw,
214+
# * Relevant "allow" rules:
215+
# allow file
216+
# allow all
217+
218+
/^[[:space:]]*(owner[[:space:]]+)?(\/proc|@\{PROC\})\/(self|@\{pid\}|[0-9]+)\/mountinfo[[:space:]][a-z]*r[a-z]*,/ ||
219+
/^[[:space:]]*(owner[[:space:]]+)?(\/proc|@\{PROC\})\/\*\*[[:space:]][a-z]*r[a-z]*,/ ||
220+
/^[[:space:]]*allow[[:space:]]+(file|all),[[:space:]]*$/ ||
221+
/mountInfoSnippet/ { m = m " " NR ":" $0 ORS }
222+
END { if (m != "") printf " - %s:\n%s", FILENAME, m }
223+
' "$f")
224+
[ -n "$out" ] && missing_override+="${out}"$'\n'
225+
done
226+
227+
if [ -n "$missing_override" ]; then
228+
echo "" >&2
229+
echo "These interfaces allow access to /proc/*/mountinfo (specifically or not)" >&2
230+
echo "without adding the allow rule as a prioritized snippet" >&2
231+
echo "(see basePrioritizedSnippets in interfaces/apparmor/template.go):" >&2
232+
printf "%s" "$missing_override" >&2
233+
return 1
234+
fi
235+
}
236+
185237
forbidden_cmd_go_deps() {
186238
local -a forbidden_deps=(
187239
"github.com/snapcore/snapd/testutil"
@@ -389,6 +441,9 @@ if [ "$STATIC" = 1 ]; then
389441
echo ">> [Go] Checking cmd binaries for forbidden dependencies"
390442
forbidden_cmd_go_deps
391443

444+
echo ">> [Go] Checking interfaces that grant mountinfo access use a prioritized override"
445+
check_mountinfo_override
446+
392447
if command -v shellcheck >/dev/null; then
393448
exclude_tools_path=tests/lib/external/snapd-testing-tools
394449
echo ">> [Bash] Checking shell scripts"

0 commit comments

Comments
 (0)