diff --git a/cmd/snap-confine/snap-confine.apparmor.in b/cmd/snap-confine/snap-confine.apparmor.in index 81337b9835a..c2cf8c218b2 100644 --- a/cmd/snap-confine/snap-confine.apparmor.in +++ b/cmd/snap-confine/snap-confine.apparmor.in @@ -137,6 +137,9 @@ # LD_PRELOAD to be set to a library which is in a directory configured by # ld.so.conf, but access to those locations is mediated by this profile # (which requires rules for specific locations). + # TODO: use GenerateAAREExclusionPatterns for this, though the first + # rule and the fact that the generative aspect is not an absolute filepath + # complicate using that function directly change_profile unsafe /** -> [^u/]**, change_profile unsafe /** -> u[^n]**, change_profile unsafe /** -> un[^c]**, diff --git a/interfaces/apparmor/template.go b/interfaces/apparmor/template.go index f1e8a19fc8c..b1c7d9a8733 100644 --- a/interfaces/apparmor/template.go +++ b/interfaces/apparmor/template.go @@ -681,6 +681,7 @@ var defaultOtherBaseTemplateRules = ` # - /lib/modules # # Everything but /lib/firmware and /lib/modules + # TODO: use GenerateAAREExclusionPatterns for this /{,usr/}lib/ r, /{,usr/}lib/[^fm]** mrklix, /{,usr/}lib/{f[^i],m[^o]}** mrklix, @@ -709,6 +710,7 @@ var defaultOtherBaseTemplateRules = ` # # Everything but /usr/lib and /usr/src, which are handled elsewhere. /usr/ r, + # TODO: use GenerateAAREExclusionPatterns for this /usr/[^ls]** mrklix, /usr/{l[^i],s[^r]}** mrklix, /usr/{li[^b],sr[^c]}** mrklix, diff --git a/interfaces/builtin/docker_support.go b/interfaces/builtin/docker_support.go index 4b95c2f2250..e6712c41858 100644 --- a/interfaces/builtin/docker_support.go +++ b/interfaces/builtin/docker_support.go @@ -185,156 +185,20 @@ pivot_root, # when the AppArmor parser bugs are fixed) this can go back to the much simpler # rule: # change_profile unsafe /** -> docker-default, -# but until then we are stuck with: -change_profile unsafe /[^s]** -> docker-default, -change_profile unsafe /s[^n]** -> docker-default, -change_profile unsafe /sn[^a]** -> docker-default, -change_profile unsafe /sna[^p]** -> docker-default, -change_profile unsafe /snap[^/]** -> docker-default, -change_profile unsafe /snap/[^sc]** -> docker-default, -change_profile unsafe /snap/{s[^n],c[^o]}** -> docker-default, -change_profile unsafe /snap/{sn[^a],co[^r]}** -> docker-default, -change_profile unsafe /snap/{sna[^p],cor[^e]}** -> docker-default, - -# branch for the /snap/core/... paths -change_profile unsafe /snap/core[^/]** -> docker-default, -change_profile unsafe /snap/core/*/[^u]** -> docker-default, -change_profile unsafe /snap/core/*/u[^s]** -> docker-default, -change_profile unsafe /snap/core/*/us[^r]** -> docker-default, -change_profile unsafe /snap/core/*/usr[^/]** -> docker-default, -change_profile unsafe /snap/core/*/usr/[^l]** -> docker-default, -change_profile unsafe /snap/core/*/usr/l[^i]** -> docker-default, -change_profile unsafe /snap/core/*/usr/li[^b]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib[^/]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/[^s]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/s[^n]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/sn[^a]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/sna[^p]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snap[^d]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd[^/]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/[^s]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/s[^n]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/sn[^a]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/sna[^p]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap[^-]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-[^c]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-c[^o]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-co[^n]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-con[^f]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-conf[^i]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confi[^n]** -> docker-default, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confin[^e]** -> docker-default, - -# branch for the /snap/snapd/... paths -change_profile unsafe /snap/snap[^d]** -> docker-default, -change_profile unsafe /snap/snapd[^/]** -> docker-default, -change_profile unsafe /snap/snapd/*/[^u]** -> docker-default, -change_profile unsafe /snap/snapd/*/u[^s]** -> docker-default, -change_profile unsafe /snap/snapd/*/us[^r]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr[^/]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/[^l]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/l[^i]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/li[^b]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib[^/]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/[^s]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/s[^n]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/sn[^a]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/sna[^p]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snap[^d]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd[^/]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/[^s]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/s[^n]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/sn[^a]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/sna[^p]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap[^-]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-[^c]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-c[^o]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-co[^n]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-con[^f]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-conf[^i]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confi[^n]** -> docker-default, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confin[^e]** -> docker-default, - +# below are auto-generated rules using GenerateAAREExclusionPatterns +###EXCL{change_profile unsafe <> -> docker-default,:/snap/snapd/*/usr/lib/snapd/snap-confine,/snap/core/*/usr/lib/snapd/snap-confine}### # signal/tracing rules too signal (send) peer=docker-default, ptrace (read, trace) peer=docker-default, - # defaults for containerd # TODO: When we drop support for executing other snaps from devmode snaps (or # when the AppArmor parser bugs are fixed) this can go back to the much simpler # rule: # change_profile unsafe /** -> cri-containerd.apparmor.d, -# see above comment, we need this because we can't have nice things -change_profile unsafe /[^s]** -> cri-containerd.apparmor.d, -change_profile unsafe /s[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /sn[^a]** -> cri-containerd.apparmor.d, -change_profile unsafe /sna[^p]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/[^sc]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/{s[^n],c[^o]}** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/{sn[^a],co[^r]}** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/{sna[^p],cor[^e]}** -> cri-containerd.apparmor.d, - -# branch for the /snap/core/... paths -change_profile unsafe /snap/core[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/[^u]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/u[^s]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/us[^r]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/[^l]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/l[^i]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/li[^b]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/[^s]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/s[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/sn[^a]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/sna[^p]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snap[^d]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/[^s]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/s[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/sn[^a]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/sna[^p]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap[^-]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-[^c]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-c[^o]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-co[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-con[^f]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-conf[^i]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confi[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confin[^e]** -> cri-containerd.apparmor.d, - -# branch for the /snap/snapd/... paths -change_profile unsafe /snap/snap[^d]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/[^u]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/u[^s]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/us[^r]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/[^l]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/l[^i]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/li[^b]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/[^s]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/s[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/sn[^a]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/sna[^p]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snap[^d]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd[^/]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/[^s]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/s[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/sn[^a]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/sna[^p]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap[^-]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-[^c]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-c[^o]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-co[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-con[^f]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-conf[^i]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confi[^n]** -> cri-containerd.apparmor.d, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confin[^e]** -> cri-containerd.apparmor.d, +# below are auto-generated rules using GenerateAAREExclusionPatterns +###EXCL{change_profile unsafe <> -> cri-containerd.apparmor.d,:/snap/snapd/*/usr/lib/snapd/snap-confine,/snap/core/*/usr/lib/snapd/snap-confine}### # signal/tracing rules too signal (send) peer=cri-containerd.apparmor.d, @@ -824,74 +688,8 @@ const dockerSupportPrivilegedAppArmor = ` # but until then we need this set of rules to avoid exec transition conflicts. # See also the comment above the "change_profile unsafe /** -> docker-default," # rule for more context. -change_profile unsafe /[^s]**, -change_profile unsafe /s[^n]**, -change_profile unsafe /sn[^a]**, -change_profile unsafe /sna[^p]**, -change_profile unsafe /snap[^/]**, -change_profile unsafe /snap/[^sc]**, -change_profile unsafe /snap/{s[^n],c[^o]}**, -change_profile unsafe /snap/{sn[^a],co[^r]}**, -change_profile unsafe /snap/{sna[^p],cor[^e]}**, - -# branch for the /snap/core/... paths -change_profile unsafe /snap/core[^/]**, -change_profile unsafe /snap/core/*/[^u]**, -change_profile unsafe /snap/core/*/u[^s]**, -change_profile unsafe /snap/core/*/us[^r]**, -change_profile unsafe /snap/core/*/usr[^/]**, -change_profile unsafe /snap/core/*/usr/[^l]**, -change_profile unsafe /snap/core/*/usr/l[^i]**, -change_profile unsafe /snap/core/*/usr/li[^b]**, -change_profile unsafe /snap/core/*/usr/lib[^/]**, -change_profile unsafe /snap/core/*/usr/lib/[^s]**, -change_profile unsafe /snap/core/*/usr/lib/s[^n]**, -change_profile unsafe /snap/core/*/usr/lib/sn[^a]**, -change_profile unsafe /snap/core/*/usr/lib/sna[^p]**, -change_profile unsafe /snap/core/*/usr/lib/snap[^d]**, -change_profile unsafe /snap/core/*/usr/lib/snapd[^/]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/[^s]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/s[^n]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/sn[^a]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/sna[^p]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap[^-]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-[^c]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-c[^o]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-co[^n]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-con[^f]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-conf[^i]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confi[^n]**, -change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confin[^e]**, - -# branch for the /snap/snapd/... paths -change_profile unsafe /snap/snap[^d]**, -change_profile unsafe /snap/snapd[^/]**, -change_profile unsafe /snap/snapd/*/[^u]**, -change_profile unsafe /snap/snapd/*/u[^s]**, -change_profile unsafe /snap/snapd/*/us[^r]**, -change_profile unsafe /snap/snapd/*/usr[^/]**, -change_profile unsafe /snap/snapd/*/usr/[^l]**, -change_profile unsafe /snap/snapd/*/usr/l[^i]**, -change_profile unsafe /snap/snapd/*/usr/li[^b]**, -change_profile unsafe /snap/snapd/*/usr/lib[^/]**, -change_profile unsafe /snap/snapd/*/usr/lib/[^s]**, -change_profile unsafe /snap/snapd/*/usr/lib/s[^n]**, -change_profile unsafe /snap/snapd/*/usr/lib/sn[^a]**, -change_profile unsafe /snap/snapd/*/usr/lib/sna[^p]**, -change_profile unsafe /snap/snapd/*/usr/lib/snap[^d]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd[^/]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/[^s]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/s[^n]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/sn[^a]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/sna[^p]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap[^-]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-[^c]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-c[^o]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-co[^n]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-con[^f]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-conf[^i]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confi[^n]**, -change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confin[^e]**, +# below are auto-generated rules using GenerateAAREExclusionPatterns +###EXCL{change_profile unsafe <>,:/snap/snapd/*/usr/lib/snapd/snap-confine,/snap/core/*/usr/lib/snapd/snap-confine}### # allow signaling and tracing any unconfined process since if containers are # launched without confinement docker still needs to trace them @@ -914,73 +712,8 @@ ptrace (read, trace) peer=unconfined, # but until then we need this set of rules to avoid exec transition conflicts. # See also the comment above the "change_profile unsafe /** -> docker-default," # rule for more context. -/[^s]** rwlix, -/s[^n]** rwlix, -/sn[^a]** rwlix, -/sna[^p]** rwlix, -/snap/[^sc]** rwlix, -/snap/{s[^n],c[^o]}** rwlix, -/snap/{sn[^a],co[^r]}** rwlix, -/snap/{sna[^p],cor[^e]}** rwlix, - -# branch for the /snap/core/... paths -/snap/core[^/]** rwlix, -/snap/core/*/[^u]** rwlix, -/snap/core/*/u[^s]** rwlix, -/snap/core/*/us[^r]** rwlix, -/snap/core/*/usr[^/]** rwlix, -/snap/core/*/usr/[^l]** rwlix, -/snap/core/*/usr/l[^i]** rwlix, -/snap/core/*/usr/li[^b]** rwlix, -/snap/core/*/usr/lib[^/]** rwlix, -/snap/core/*/usr/lib/[^s]** rwlix, -/snap/core/*/usr/lib/s[^n]** rwlix, -/snap/core/*/usr/lib/sn[^a]** rwlix, -/snap/core/*/usr/lib/sna[^p]** rwlix, -/snap/core/*/usr/lib/snap[^d]** rwlix, -/snap/core/*/usr/lib/snapd[^/]** rwlix, -/snap/core/*/usr/lib/snapd/[^s]** rwlix, -/snap/core/*/usr/lib/snapd/s[^n]** rwlix, -/snap/core/*/usr/lib/snapd/sn[^a]** rwlix, -/snap/core/*/usr/lib/snapd/sna[^p]** rwlix, -/snap/core/*/usr/lib/snapd/snap[^-]** rwlix, -/snap/core/*/usr/lib/snapd/snap-[^c]** rwlix, -/snap/core/*/usr/lib/snapd/snap-c[^o]** rwlix, -/snap/core/*/usr/lib/snapd/snap-co[^n]** rwlix, -/snap/core/*/usr/lib/snapd/snap-con[^f]** rwlix, -/snap/core/*/usr/lib/snapd/snap-conf[^i]** rwlix, -/snap/core/*/usr/lib/snapd/snap-confi[^n]** rwlix, -/snap/core/*/usr/lib/snapd/snap-confin[^e]** rwlix, - -# branch for the /snap/snapd/... paths -/snap/snap[^d]** rwlix, -/snap/snapd[^/]** rwlix, -/snap/snapd/*/[^u]** rwlix, -/snap/snapd/*/u[^s]** rwlix, -/snap/snapd/*/us[^r]** rwlix, -/snap/snapd/*/usr[^/]** rwlix, -/snap/snapd/*/usr/[^l]** rwlix, -/snap/snapd/*/usr/l[^i]** rwlix, -/snap/snapd/*/usr/li[^b]** rwlix, -/snap/snapd/*/usr/lib[^/]** rwlix, -/snap/snapd/*/usr/lib/[^s]** rwlix, -/snap/snapd/*/usr/lib/s[^n]** rwlix, -/snap/snapd/*/usr/lib/sn[^a]** rwlix, -/snap/snapd/*/usr/lib/sna[^p]** rwlix, -/snap/snapd/*/usr/lib/snap[^d]** rwlix, -/snap/snapd/*/usr/lib/snapd[^/]** rwlix, -/snap/snapd/*/usr/lib/snapd/[^s]** rwlix, -/snap/snapd/*/usr/lib/snapd/s[^n]** rwlix, -/snap/snapd/*/usr/lib/snapd/sn[^a]** rwlix, -/snap/snapd/*/usr/lib/snapd/sna[^p]** rwlix, -/snap/snapd/*/usr/lib/snapd/snap[^-]** rwlix, -/snap/snapd/*/usr/lib/snapd/snap-[^c]** rwlix, -/snap/snapd/*/usr/lib/snapd/snap-c[^o]** rwlix, -/snap/snapd/*/usr/lib/snapd/snap-co[^n]** rwlix, -/snap/snapd/*/usr/lib/snapd/snap-con[^f]** rwlix, -/snap/snapd/*/usr/lib/snapd/snap-conf[^i]** rwlix, -/snap/snapd/*/usr/lib/snapd/snap-confi[^n]** rwlix, -/snap/snapd/*/usr/lib/snapd/snap-confin[^e]** rwlix, +# below are auto-generated rules using GenerateAAREExclusionPatterns +###EXCL{<> rwlix,:/snap/snapd/*/usr/lib/snapd/snap-confine,/snap/core/*/usr/lib/snapd/snap-confine}### ` const dockerSupportPrivilegedSecComp = ` @@ -1021,9 +754,70 @@ func (iface *dockerSupportInterface) AppArmorConnectedPlug(spec *apparmor.Specif // try to modify, otherwise immutable, pycache inside the // snaps. spec.SetSuppressPycacheDeny() - spec.AddSnippet(dockerSupportConnectedPlugAppArmor) + + defaultSnippet, err := apparmor_sandbox.InsertAAREExclusionPatterns( + dockerSupportConnectedPlugAppArmor, + []string{ + "/snap/snapd/*/usr/lib/snapd/snap-confine", + "/snap/core/*/usr/lib/snapd/snap-confine", + }, + &apparmor_sandbox.AAREExclusionPatternsOptions{ + Prefix: "change_profile unsafe ", + Suffix: " -> docker-default,", + }, + ) + if err != nil { + return err + } + + defaultSnippet, err = apparmor_sandbox.InsertAAREExclusionPatterns( + defaultSnippet, + []string{ + "/snap/snapd/*/usr/lib/snapd/snap-confine", + "/snap/core/*/usr/lib/snapd/snap-confine", + }, + &apparmor_sandbox.AAREExclusionPatternsOptions{ + Prefix: "change_profile unsafe ", + Suffix: " -> cri-containerd.apparmor.d,", + }, + ) + if err != nil { + return err + } + + spec.AddSnippet(defaultSnippet) if privileged { - spec.AddSnippet(dockerSupportPrivilegedAppArmor) + privilegedSnippet, err := apparmor_sandbox.InsertAAREExclusionPatterns( + dockerSupportPrivilegedAppArmor, + []string{ + "/snap/snapd/*/usr/lib/snapd/snap-confine", + "/snap/core/*/usr/lib/snapd/snap-confine", + }, + &apparmor_sandbox.AAREExclusionPatternsOptions{ + Prefix: "change_profile unsafe ", + Suffix: ",", + }, + ) + if err != nil { + return err + } + + privilegedSnippet, err = apparmor_sandbox.InsertAAREExclusionPatterns( + privilegedSnippet, + []string{ + "/snap/snapd/*/usr/lib/snapd/snap-confine", + "/snap/core/*/usr/lib/snapd/snap-confine", + }, + &apparmor_sandbox.AAREExclusionPatternsOptions{ + Prefix: "", + Suffix: " rwlix,", + }, + ) + if err != nil { + return err + } + + spec.AddSnippet(privilegedSnippet) } if !release.OnClassic { spec.AddSnippet(dockerSupportConnectedPlugAppArmorCore) diff --git a/interfaces/builtin/docker_support_test.go b/interfaces/builtin/docker_support_test.go index 35e63ea9a2c..c84370009df 100644 --- a/interfaces/builtin/docker_support_test.go +++ b/interfaces/builtin/docker_support_test.go @@ -20,6 +20,7 @@ package builtin_test import ( + "github.com/snapcore/snapd/strutil" . "gopkg.in/check.v1" "github.com/snapcore/snapd/interfaces" @@ -313,3 +314,561 @@ func (s *DockerSupportInterfaceSuite) TestServicePermanentPlugSnippets(c *C) { _, err = interfaces.PermanentPlugServiceSnippets(s.iface, s.malformedPlugInfo) c.Assert(err, ErrorMatches, "docker-support plug requires bool with 'privileged-containers'") } + +func (s *DockerSupportInterfaceSuite) TestGenerateAAREExclusionPatterns(c *C) { + + const dockerSupportConnectedPlugAppArmorUserNS = ` +# allow use of user namespaces +userns, +` + + const dockerSupportConnectedPlugAppArmor = ` +# Description: allow operating as the Docker daemon/containerd. This policy is +# intentionally not restrictive and is here to help guard against programming +# errors and not for security confinement. The Docker daemon by design requires +# extensive access to the system and cannot be effectively confined against +# malicious activity. + +#include + +# Allow sockets/etc for docker +/{,var/}run/docker.sock rw, +/{,var/}run/docker/ rw, +/{,var/}run/docker/** mrwklix, +/{,var/}run/runc/ rw, +/{,var/}run/runc/** mrwklix, + +# Allow sockets/etc for containerd +/{,var/}run/containerd/{,s/,runc/,runc/k8s.io/,runc/k8s.io/*/} rw, +/{,var/}run/containerd/runc/k8s.io/*/** rwk, +/{,var/}run/containerd/{io.containerd*/,io.containerd*/k8s.io/,io.containerd*/k8s.io/*/} rw, +/{,var/}run/containerd/io.containerd*/*/** rwk, +/{,var/}run/containerd/s/** rwk, + +# Limit ipam-state to k8s +/run/ipam-state/k8s-** rw, +/run/ipam-state/k8s-*/lock k, + +# Socket for docker-containerd-shim +unix (bind,listen) type=stream addr="@/containerd-shim/**.sock\x00", + +/{,var/}run/mount/utab r, + +# Wide read access to /proc, but somewhat limited writes for now +@{PROC}/ r, +@{PROC}/** r, +@{PROC}/[0-9]*/attr/{,apparmor/}exec w, +@{PROC}/[0-9]*/oom_score_adj w, + +# Limited read access to specific bits of /sys +/sys/kernel/mm/hugepages/ r, +/sys/kernel/mm/transparent_hugepage/{,**} r, +/sys/fs/cgroup/cpuset/cpuset.cpus r, +/sys/fs/cgroup/cpuset/cpuset.mems r, +/sys/module/apparmor/parameters/enabled r, + +# Limit cgroup writes a bit (Docker uses a "docker" sub-group) +/sys/fs/cgroup/*/docker/ rw, +/sys/fs/cgroup/*/docker/** rw, + +# Also allow cgroup writes to kubernetes pods +/sys/fs/cgroup/*/kubepods/ rw, +/sys/fs/cgroup/*/kubepods/** rw, + +# containerd can also be configured to use the systemd cgroup driver via +# plugins.cri.systemd_cgroup = true which moves container processes into +# systemd-managed cgroups. This is now the recommended configuration since it +# provides a single cgroup manager (systemd) in an effort to achieve consistent +# views of resources. +/sys/fs/cgroup/*/systemd/{,system.slice/} rw, # create missing dirs +/sys/fs/cgroup/*/systemd/system.slice/** r, +/sys/fs/cgroup/*/systemd/system.slice/cgroup.procs w, + +# Allow tracing ourself (especially the "runc" process we create) +ptrace (trace) peer=@{profile_name}, + +# Docker needs a lot of caps, but limits them in the app container +capability, + +# Docker does all kinds of mounts all over the filesystem +/dev/mapper/control rw, +/dev/mapper/docker* rw, +/dev/loop-control r, +/dev/loop[0-9]* rw, +/sys/devices/virtual/block/dm-[0-9]*/** r, +mount, +umount, + +# After doing a pivot_root using //.pivot_rootNNNNNN, +# Docker removes the leftover /.pivot_rootNNNNNN directory (which is now +# relative to "/" instead of "/" thanks to pivot_root) +pivot_root, +/.pivot_root[0-9]*/ rw, + +# file descriptors (/proc/NNN/fd/X) +# file descriptors in the container show up here due to attach_disconnected +/[0-9]* rw, + +# Docker needs to be able to create and load the profile it applies to +# containers ("docker-default") +/{,usr/}sbin/apparmor_parser ixr, +/etc/apparmor.d/cache/ r, # apparmor 2.12 and below +/etc/apparmor.d/cache/.features r, +/etc/apparmor.d/{,cache/}docker* rw, +/var/cache/apparmor/{,*/} r, # apparmor 2.13 and higher +/var/cache/apparmor/*/.features r, +/var/cache/apparmor/*/docker* rw, +/etc/apparmor.d/tunables/{,**} r, +/etc/apparmor.d/abstractions/{,**} r, +/etc/apparmor/parser.conf r, +/etc/apparmor.d/abi/{,*} r, +/etc/apparmor/subdomain.conf r, +/sys/kernel/security/apparmor/.replace rw, +/sys/kernel/security/apparmor/{,**} r, + +# use 'privileged-containers: true' to support --security-opts + +# defaults for docker-default +# Unfortunately, the docker snap is currently (by design?) setup to have both +# the privileged and unprivileged variant of the docker-support interface +# connected which means we have rules that are compatible to allow both +# transitioning to docker-default profile here AAAAAAND transitioning to any +# other profile below in the privileged snippet, BUUUUUUUT also need to be +# triply compatible with the injected compatibility snap-confine transition +# rules to temporarily support executing other snaps from devmode snaps. +# So we are left with writing out these extremely verbose regexps because AARE +# does not have a negative concept to exclude just the paths we want. +# See also https://bugs.launchpad.net/apparmor/+bug/1964853 and +# https://bugs.launchpad.net/apparmor/+bug/1964854 for more details on the +# AppArmor parser side of things. +# TODO: When we drop support for executing other snaps from devmode snaps (or +# when the AppArmor parser bugs are fixed) this can go back to the much simpler +# rule: +# change_profile unsafe /** -> docker-default, +# but until then we are stuck with: +change_profile unsafe /[^s]** -> docker-default, +change_profile unsafe /s[^n]** -> docker-default, +change_profile unsafe /sn[^a]** -> docker-default, +change_profile unsafe /sna[^p]** -> docker-default, +change_profile unsafe /snap[^/]** -> docker-default, +change_profile unsafe /snap/[^sc]** -> docker-default, +change_profile unsafe /snap/{s[^n],c[^o]}** -> docker-default, +change_profile unsafe /snap/{sn[^a],co[^r]}** -> docker-default, +change_profile unsafe /snap/{sna[^p],cor[^e]}** -> docker-default, + +# branch for the /snap/core/... paths +change_profile unsafe /snap/core[^/]** -> docker-default, +change_profile unsafe /snap/core/*/[^u]** -> docker-default, +change_profile unsafe /snap/core/*/u[^s]** -> docker-default, +change_profile unsafe /snap/core/*/us[^r]** -> docker-default, +change_profile unsafe /snap/core/*/usr[^/]** -> docker-default, +change_profile unsafe /snap/core/*/usr/[^l]** -> docker-default, +change_profile unsafe /snap/core/*/usr/l[^i]** -> docker-default, +change_profile unsafe /snap/core/*/usr/li[^b]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib[^/]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/[^s]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/s[^n]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/sn[^a]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/sna[^p]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snap[^d]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd[^/]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/[^s]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/s[^n]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/sn[^a]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/sna[^p]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap[^-]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-[^c]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-c[^o]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-co[^n]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-con[^f]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-conf[^i]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confi[^n]** -> docker-default, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confin[^e]** -> docker-default, + +# branch for the /snap/snapd/... paths +change_profile unsafe /snap/snap[^d]** -> docker-default, +change_profile unsafe /snap/snapd[^/]** -> docker-default, +change_profile unsafe /snap/snapd/*/[^u]** -> docker-default, +change_profile unsafe /snap/snapd/*/u[^s]** -> docker-default, +change_profile unsafe /snap/snapd/*/us[^r]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr[^/]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/[^l]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/l[^i]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/li[^b]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib[^/]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/[^s]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/s[^n]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/sn[^a]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/sna[^p]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snap[^d]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd[^/]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/[^s]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/s[^n]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/sn[^a]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/sna[^p]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap[^-]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-[^c]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-c[^o]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-co[^n]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-con[^f]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-conf[^i]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confi[^n]** -> docker-default, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confin[^e]** -> docker-default, + + +# signal/tracing rules too +signal (send) peer=docker-default, +ptrace (read, trace) peer=docker-default, + + +# defaults for containerd +# TODO: When we drop support for executing other snaps from devmode snaps (or +# when the AppArmor parser bugs are fixed) this can go back to the much simpler +# rule: +# change_profile unsafe /** -> cri-containerd.apparmor.d, +# see above comment, we need this because we can't have nice things +change_profile unsafe /[^s]** -> cri-containerd.apparmor.d, +change_profile unsafe /s[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /sn[^a]** -> cri-containerd.apparmor.d, +change_profile unsafe /sna[^p]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/[^sc]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/{s[^n],c[^o]}** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/{sn[^a],co[^r]}** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/{sna[^p],cor[^e]}** -> cri-containerd.apparmor.d, + +# branch for the /snap/core/... paths +change_profile unsafe /snap/core[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/[^u]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/u[^s]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/us[^r]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/[^l]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/l[^i]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/li[^b]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/[^s]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/s[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/sn[^a]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/sna[^p]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snap[^d]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/[^s]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/s[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/sn[^a]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/sna[^p]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap[^-]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-[^c]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-c[^o]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-co[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-con[^f]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-conf[^i]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confi[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confin[^e]** -> cri-containerd.apparmor.d, + +# branch for the /snap/snapd/... paths +change_profile unsafe /snap/snap[^d]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/[^u]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/u[^s]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/us[^r]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/[^l]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/l[^i]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/li[^b]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/[^s]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/s[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/sn[^a]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/sna[^p]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snap[^d]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd[^/]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/[^s]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/s[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/sn[^a]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/sna[^p]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap[^-]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-[^c]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-c[^o]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-co[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-con[^f]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-conf[^i]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confi[^n]** -> cri-containerd.apparmor.d, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confin[^e]** -> cri-containerd.apparmor.d, + +# signal/tracing rules too +signal (send) peer=cri-containerd.apparmor.d, +ptrace (read, trace) peer=cri-containerd.apparmor.d, + +# Graph (storage) driver bits +/{dev,run}/shm/aufs.xino mrw, +/proc/fs/aufs/plink_maint w, +/sys/fs/aufs/** r, + +#cf bug 1502785 +/ r, + +# recent versions of docker make a symlink from /dev/ptmx to /dev/pts/ptmx +# and so to allow allocating a new shell we need this +/dev/pts/ptmx rw, + +# needed by runc for mitigation of CVE-2019-5736 +# For details see https://bugs.launchpad.net/apparmor/+bug/1820344 +/ ix, +/bin/runc ixr, + +/pause ixr, +/bin/busybox ixr, + +# When kubernetes drives containerd, containerd needs access to CNI services, +# like flanneld's subnet.env for DNS. This would ideally be snap-specific (it +# could if the control plane was a snap), but in deployments where the control +# plane is not a snap, it will tell flannel to use this path. +/run/flannel/{,**} rk, + +# When kubernetes drives containerd, containerd needs access to various +# secrets for the pods which are overlayed at /run/secrets/.... +# This would ideally be snap-specific (it could if the control plane was a +# snap), but in deployments where the control plane is not a snap, it will tell +# containerd to use this path for various account information for pods. +/run/secrets/kubernetes.io/{,**} rk, + +# Allow using the 'autobind' feature of bind() (eg, for journald via go-systemd) +# unix (bind) type=dgram addr=auto, +# TODO: when snapd vendors in AppArmor userspace, then enable the new syntax +# above which allows only "empty"/automatic addresses, for now we simply permit +# all addresses with SOCK_DGRAM type, which leaks info for other addresses than +# what docker tries to use +# see https://bugs.launchpad.net/snapd/+bug/1867216 +unix (bind) type=dgram, + +# With cgroup v2, docker uses the systemd driver to run the containers, +# which requires dockerd to talk to systemd over system bus. +dbus (send) + bus=system + path=/org/freedesktop/systemd1 + interface=org.freedesktop.systemd1.Manager + member={StartTransientUnit,KillUnit,StopUnit,ResetFailedUnit,SetUnitProperties} + peer=(name=org.freedesktop.systemd1,label=unconfined), + +dbus (receive) + bus=system + path=/org/freedesktop/systemd1 + interface=org.freedesktop.systemd1.Manager + member=JobRemoved + peer=(label=unconfined), + +dbus (send) + bus=system + interface=org.freedesktop.DBus.Properties + path=/org/freedesktop/systemd1 + member=Get{,All} + peer=(name=org.freedesktop.systemd1,label=unconfined), + +` + + const dockerSupportPrivilegedAppArmor = ` +# Description: allow docker daemon to run privileged containers. This gives +# full access to all resources on the system and thus gives device ownership to +# connected snaps. + +# These rules are here to allow Docker to launch unconfined containers but +# allow the docker daemon itself to go unconfined. Since it runs as root, this +# grants device ownership. +# TODO: When we drop support for executing other snaps from devmode snaps (or +# when the AppArmor parser bugs are fixed) this can go back to the much simpler +# rule: +# change_profile unsafe /**, +# but until then we need this set of rules to avoid exec transition conflicts. +# See also the comment above the "change_profile unsafe /** -> docker-default," +# rule for more context. +change_profile unsafe /[^s]**, +change_profile unsafe /s[^n]**, +change_profile unsafe /sn[^a]**, +change_profile unsafe /sna[^p]**, +change_profile unsafe /snap[^/]**, +change_profile unsafe /snap/[^sc]**, +change_profile unsafe /snap/{s[^n],c[^o]}**, +change_profile unsafe /snap/{sn[^a],co[^r]}**, +change_profile unsafe /snap/{sna[^p],cor[^e]}**, + +# branch for the /snap/core/... paths +change_profile unsafe /snap/core[^/]**, +change_profile unsafe /snap/core/*/[^u]**, +change_profile unsafe /snap/core/*/u[^s]**, +change_profile unsafe /snap/core/*/us[^r]**, +change_profile unsafe /snap/core/*/usr[^/]**, +change_profile unsafe /snap/core/*/usr/[^l]**, +change_profile unsafe /snap/core/*/usr/l[^i]**, +change_profile unsafe /snap/core/*/usr/li[^b]**, +change_profile unsafe /snap/core/*/usr/lib[^/]**, +change_profile unsafe /snap/core/*/usr/lib/[^s]**, +change_profile unsafe /snap/core/*/usr/lib/s[^n]**, +change_profile unsafe /snap/core/*/usr/lib/sn[^a]**, +change_profile unsafe /snap/core/*/usr/lib/sna[^p]**, +change_profile unsafe /snap/core/*/usr/lib/snap[^d]**, +change_profile unsafe /snap/core/*/usr/lib/snapd[^/]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/[^s]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/s[^n]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/sn[^a]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/sna[^p]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap[^-]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-[^c]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-c[^o]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-co[^n]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-con[^f]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-conf[^i]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confi[^n]**, +change_profile unsafe /snap/core/*/usr/lib/snapd/snap-confin[^e]**, + +# branch for the /snap/snapd/... paths +change_profile unsafe /snap/snap[^d]**, +change_profile unsafe /snap/snapd[^/]**, +change_profile unsafe /snap/snapd/*/[^u]**, +change_profile unsafe /snap/snapd/*/u[^s]**, +change_profile unsafe /snap/snapd/*/us[^r]**, +change_profile unsafe /snap/snapd/*/usr[^/]**, +change_profile unsafe /snap/snapd/*/usr/[^l]**, +change_profile unsafe /snap/snapd/*/usr/l[^i]**, +change_profile unsafe /snap/snapd/*/usr/li[^b]**, +change_profile unsafe /snap/snapd/*/usr/lib[^/]**, +change_profile unsafe /snap/snapd/*/usr/lib/[^s]**, +change_profile unsafe /snap/snapd/*/usr/lib/s[^n]**, +change_profile unsafe /snap/snapd/*/usr/lib/sn[^a]**, +change_profile unsafe /snap/snapd/*/usr/lib/sna[^p]**, +change_profile unsafe /snap/snapd/*/usr/lib/snap[^d]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd[^/]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/[^s]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/s[^n]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/sn[^a]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/sna[^p]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap[^-]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-[^c]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-c[^o]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-co[^n]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-con[^f]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-conf[^i]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confi[^n]**, +change_profile unsafe /snap/snapd/*/usr/lib/snapd/snap-confin[^e]**, + +# allow signaling and tracing any unconfined process since if containers are +# launched without confinement docker still needs to trace them +signal (send) peer=unconfined, +ptrace (read, trace) peer=unconfined, + +# This grants raw access to device files and thus device ownership +/dev/** mrwkl, +@{PROC}/** mrwkl, + +# When kubernetes drives docker/containerd, it creates and runs files in the +# container at arbitrary locations (eg, via pivot_root). +# Allow any file except for executing /snap/{snapd,core}/*/usr/lib/snapd/snap-confine +# because in devmode confinement we will have a separate "x" transition on exec +# rule that is in the policy that will overlap and thus conflict with this rule. +# TODO: When we drop support for executing other snaps from devmode snaps (or +# when the AppArmor parser bugs are fixed) this can go back to the much simpler +# rule: +# /** rwlix, +# but until then we need this set of rules to avoid exec transition conflicts. +# See also the comment above the "change_profile unsafe /** -> docker-default," +# rule for more context. +/[^s]** rwlix, +/s[^n]** rwlix, +/sn[^a]** rwlix, +/sna[^p]** rwlix, +/snap[^/]** rwlix, +/snap/[^sc]** rwlix, +/snap/{s[^n],c[^o]}** rwlix, +/snap/{sn[^a],co[^r]}** rwlix, +/snap/{sna[^p],cor[^e]}** rwlix, + +# branch for the /snap/core/... paths +/snap/core[^/]** rwlix, +/snap/core/*/[^u]** rwlix, +/snap/core/*/u[^s]** rwlix, +/snap/core/*/us[^r]** rwlix, +/snap/core/*/usr[^/]** rwlix, +/snap/core/*/usr/[^l]** rwlix, +/snap/core/*/usr/l[^i]** rwlix, +/snap/core/*/usr/li[^b]** rwlix, +/snap/core/*/usr/lib[^/]** rwlix, +/snap/core/*/usr/lib/[^s]** rwlix, +/snap/core/*/usr/lib/s[^n]** rwlix, +/snap/core/*/usr/lib/sn[^a]** rwlix, +/snap/core/*/usr/lib/sna[^p]** rwlix, +/snap/core/*/usr/lib/snap[^d]** rwlix, +/snap/core/*/usr/lib/snapd[^/]** rwlix, +/snap/core/*/usr/lib/snapd/[^s]** rwlix, +/snap/core/*/usr/lib/snapd/s[^n]** rwlix, +/snap/core/*/usr/lib/snapd/sn[^a]** rwlix, +/snap/core/*/usr/lib/snapd/sna[^p]** rwlix, +/snap/core/*/usr/lib/snapd/snap[^-]** rwlix, +/snap/core/*/usr/lib/snapd/snap-[^c]** rwlix, +/snap/core/*/usr/lib/snapd/snap-c[^o]** rwlix, +/snap/core/*/usr/lib/snapd/snap-co[^n]** rwlix, +/snap/core/*/usr/lib/snapd/snap-con[^f]** rwlix, +/snap/core/*/usr/lib/snapd/snap-conf[^i]** rwlix, +/snap/core/*/usr/lib/snapd/snap-confi[^n]** rwlix, +/snap/core/*/usr/lib/snapd/snap-confin[^e]** rwlix, + +# branch for the /snap/snapd/... paths +/snap/snap[^d]** rwlix, +/snap/snapd[^/]** rwlix, +/snap/snapd/*/[^u]** rwlix, +/snap/snapd/*/u[^s]** rwlix, +/snap/snapd/*/us[^r]** rwlix, +/snap/snapd/*/usr[^/]** rwlix, +/snap/snapd/*/usr/[^l]** rwlix, +/snap/snapd/*/usr/l[^i]** rwlix, +/snap/snapd/*/usr/li[^b]** rwlix, +/snap/snapd/*/usr/lib[^/]** rwlix, +/snap/snapd/*/usr/lib/[^s]** rwlix, +/snap/snapd/*/usr/lib/s[^n]** rwlix, +/snap/snapd/*/usr/lib/sn[^a]** rwlix, +/snap/snapd/*/usr/lib/sna[^p]** rwlix, +/snap/snapd/*/usr/lib/snap[^d]** rwlix, +/snap/snapd/*/usr/lib/snapd[^/]** rwlix, +/snap/snapd/*/usr/lib/snapd/[^s]** rwlix, +/snap/snapd/*/usr/lib/snapd/s[^n]** rwlix, +/snap/snapd/*/usr/lib/snapd/sn[^a]** rwlix, +/snap/snapd/*/usr/lib/snapd/sna[^p]** rwlix, +/snap/snapd/*/usr/lib/snapd/snap[^-]** rwlix, +/snap/snapd/*/usr/lib/snapd/snap-[^c]** rwlix, +/snap/snapd/*/usr/lib/snapd/snap-c[^o]** rwlix, +/snap/snapd/*/usr/lib/snapd/snap-co[^n]** rwlix, +/snap/snapd/*/usr/lib/snapd/snap-con[^f]** rwlix, +/snap/snapd/*/usr/lib/snapd/snap-conf[^i]** rwlix, +/snap/snapd/*/usr/lib/snapd/snap-confi[^n]** rwlix, +/snap/snapd/*/usr/lib/snapd/snap-confin[^e]** rwlix, +` + + // 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()) + } + + features, err := apparmor_sandbox.ParserFeatures() + c.Assert(err, IsNil) + if strutil.ListContains(features, "userns") { + privilegedProfile += dockerSupportConnectedPlugAppArmorUserNS + } + + // Profile existing profile + expectedHash, err := testutil.AppArmorParseAndHashHelper("#include \nprofile docker_support {" + privilegedProfile + "}") + c.Assert(err, IsNil) + + // Profile generated using GenerateAAREExclusionPatterns + appSet, err := interfaces.NewSnapAppSet(s.privContainersPlug.Snap(), nil) + c.Assert(err, IsNil) + apparmorSpec := apparmor.NewSpecification(appSet) + c.Assert(apparmorSpec.AddConnectedPlug(s.iface, s.privContainersPlug, s.slot), IsNil) + c.Assert(apparmorSpec.SecurityTags(), DeepEquals, []string{"snap.docker.app"}) + resHash, err := testutil.AppArmorParseAndHashHelper("#include \nprofile docker_support {" + apparmorSpec.SnippetForTag("snap.docker.app") + "}") + c.Assert(err, IsNil) + comment := Commentf("Apparmor rules generated by GenerateAAREExclusionPatterns doesn't match the expected profile") + c.Assert(resHash, Equals, expectedHash, comment) + +} diff --git a/interfaces/builtin/home.go b/interfaces/builtin/home.go index 6aed3fddfa4..d6781edf487 100644 --- a/interfaces/builtin/home.go +++ b/interfaces/builtin/home.go @@ -56,6 +56,9 @@ const homeConnectedPlugAppArmor = ` # Allow read/write access to all files in @{HOME}, except snap application # data in @{HOME}/snap and toplevel hidden directories in @{HOME}. +# TODO: use GenerateAAREExclusionPatterns for this - though the first +# rule here complicates using it slightly from the inclusion of the "." to +# prevent reading dotfiles ###PROMPT### owner @{HOME}/[^s.]** rwkl###HOME_IX###, ###PROMPT### owner @{HOME}/s[^n]** rwkl###HOME_IX###, ###PROMPT### owner @{HOME}/sn[^a]** rwkl###HOME_IX###, @@ -85,6 +88,9 @@ audit deny @{HOME}/bin wl, const homeConnectedPlugAppArmorWithAllRead = ` # Allow non-owner read to non-hidden and non-snap files and directories capability dac_read_search, +# TODO: use GenerateAAREExclusionPatterns for this - though the first +# rule here complicates using it slightly from the inclusion of the "." to +# prevent reading dotfiles ###PROMPT### @{HOME}/ r, ###PROMPT### @{HOME}/[^s.]** r, ###PROMPT### @{HOME}/s[^n]** r, diff --git a/interfaces/builtin/system_backup.go b/interfaces/builtin/system_backup.go index 2805ef8c7b3..370e1cc1b70 100644 --- a/interfaces/builtin/system_backup.go +++ b/interfaces/builtin/system_backup.go @@ -34,6 +34,7 @@ const systemBackupConnectedPlugAppArmor = ` capability dac_read_search, # read access to everything except items under /dev, /sys and /proc +# TODO: use GenerateAAREExclusionPatterns for this /{,var/lib/snapd/hostfs/}[^dsp]** r, /{,var/lib/snapd/hostfs/}{d[^e],s[^y],p[^r]}** r, /{,var/lib/snapd/hostfs/}{de[^v],sy[^s],pr[^o]}** r, diff --git a/interfaces/builtin/utils.go b/interfaces/builtin/utils.go index 639bbcb9d66..93ef6b3f02e 100644 --- a/interfaces/builtin/utils.go +++ b/interfaces/builtin/utils.go @@ -85,6 +85,9 @@ func verifySlotPathAttribute(slotRef *interfaces.SlotRef, attrs interfaces.Attre // "f[^o]*", // "fo[^o]*", // } +// +// For a more generic version of this see GenerateAAREExclusionPatterns +// TODO: can this be rewritten to use/share code with that function instead? func aareExclusivePatterns(orig string) []string { // This function currently is only intended to be used with desktop // prefixes as calculated by info.DesktopPrefix (the snap name and diff --git a/sandbox/apparmor/apparmor.go b/sandbox/apparmor/apparmor.go index c7bc49e0c7e..45b1b5a9941 100644 --- a/sandbox/apparmor/apparmor.go +++ b/sandbox/apparmor/apparmor.go @@ -22,6 +22,7 @@ package apparmor import ( "bufio" "bytes" + "errors" "fmt" "os" "os/exec" @@ -56,6 +57,234 @@ func ValidateNoAppArmorRegexp(s string) error { return nil } +type AAREExclusionPatternsOptions struct { + // Prefix is a string to include on every line in the permutations before + // the exclusion permutation itself. + Prefix string + // Suffix is a string to include on every line in the permutations after + // the exclusion permutation itself. + Suffix string + + // TODO: add options for generating non-filepaths like what we need for the + // unconfined profile transition exclusion in snap-confine's profile as well + // as an option for adding an extra character to the very first rule like we + // need in the home interface +} + +// InsertAAREExclusionPatterns replaces a ###EXCL{,}### snippet +// with matching prefix and comma separated suffixes with a set of rules generated +// by GenerateAAREExclusionPatterns. +func InsertAAREExclusionPatterns(aaRules string, excludePatterns []string, opts *AAREExclusionPatternsOptions) (string, error) { + exclussionPatterns, err := GenerateAAREExclusionPatterns(excludePatterns, opts) + if err != nil { + return "", err + } + + placeHolder := fmt.Sprintf("###EXCL{%s<>%s:%s}###", opts.Prefix, opts.Suffix, strings.Join(excludePatterns[:], ",")) + + if !strings.Contains(aaRules, placeHolder) { + return "", fmt.Errorf("placeholder pattern %q not found", placeHolder) + } + return strings.Replace(aaRules, placeHolder, strings.TrimSuffix(exclussionPatterns, "\n"), -1), nil +} + +// GenerateAAREExclusionPatterns generates a series of valid AppArmor +// regular expression negation rules such that anything except the specific +// excludePatterns will match with the specified prefix and suffix rules. For +// example to allow reading any file except those matching /usr/*/foo, you would +// call this function with "/usr/*/foo" as the first argument, "" as the prefix +// and " r," as the suffix (the suffix being the main part of the read rule) and +// this function would return the following multi-line string with the relevant +// rules: +// +// /[^u]** r, +// /u[^s]** r, +// /us[^r]** r, +// /usr[^/]** r, +// /usr/*/[^f]** r, +// /usr/*/f[^o]** r, +// /usr/*/fo[^o]** r, +// +// This function only treats '*' specially in the string and does not handle any +// other alternations etc. that AARE may more generally support, and all +// patterns provided must be absolute filepaths that are at least 2 runes long. +// +// This function also works with multiple exclude patterns such as specifying to +// exclude "/usr/lib/snapd" and "/var/lib/snapd" with suffix " r," would yield: +// +// /[^uv]** r, +// /{u[^s],v[^a]}** r, +// /{us[^r],va[^r]}** r, +// /{usr[^/],var[^/]}** r, +// /{usr/[^l],var/[^l]}** r, +// /{usr/l[^i],var/l[^i]}** r, +// /{usr/li[^b],var/li[^b]}** r, +// /{usr/lib[^/],var/lib[^/]}** r, +// /{usr/lib/[^s],var/lib/[^s]}** r, +// /{usr/lib/s[^n],var/lib/s[^n]}** r, +// /{usr/lib/sn[^a],var/lib/sn[^a]}** r, +// /{usr/lib/sna[^p],var/lib/sna[^p]}** r, +// /{usr/lib/snap[^d],var/lib/snap[^d]}** r, +// +// Note that with the previous rules, /usr/lib/snapdaemon would not match any rule +// +// This function has the following limitations: +// The first character after a subpattern common to two or more excludePatterns cannot +// be '*' on any of the excludePatterns that share the common prefix. +// Eg. ["/snap/core/**", "/snap/*/core/**"] where '/snap/' would be the subpattern common +// to both excludePattern, and '*' would be the first character after the common subpattern +// in the second excludePattern. +// This is because there are no apparmor rules that can fulfill both requirements. +// For /snap/[^c]** -> It will also match /snap/a/core/** which should be excluded by +// the second pattern. +// For /snap/[^c*]** -> It will also exclude access to /snap/a/a that should be allowed +// as it is not explicitly excluded by any pattern +// +// When the '*' is used to exclude suffixes, like in ["/*.bin"], rules should be generated +// in a reverse way: +// +// /*[^n]{,/**} rw, +// /*[^i]n{,/**} rw, +// /*[^b]in{,/**} rw, +// /*[^.]bin{,/**} rw, +// +// While generating those rules is technically possible, it will make the logic way more +// complex, thus the function would just return an error if a pattern of this kind is found. +// This functionality can be added in a subsequent PR if needed in the future +func GenerateAAREExclusionPatterns(excludePatterns []string, opts *AAREExclusionPatternsOptions) (string, error) { + if len(excludePatterns) == 0 { + return "", errors.New("no patterns provided") + } + seen := map[string]bool{} + for _, patt := range excludePatterns { + // check for duplicates + if seen[patt] { + return "", errors.New("exclude patterns contain duplicates") + } + seen[patt] = true + + // check if it is at least legnth 2 + if len(patt) < 2 { + return "", errors.New("exclude patterns must be at least length 2") + } + + // check that it starts as an absolute path + if patt[0] != '/' { + return "", errors.New("exclude patterns must be absolute filepaths") + } + + // TODO: should we also validate that the only character in the pattern + // from AARE is "*" ? + } + if opts == nil { + opts = &AAREExclusionPatternsOptions{} + } + return generateAAREExclusionPatternsGenericImpl(excludePatterns, opts) + +} + +func generateAAREExclusionPatternsGenericImpl(excludePatterns []string, opts *AAREExclusionPatternsOptions) (string, error) { + // Find the length of longest pattern (size) + size := 0 + for _, pattern := range excludePatterns { + if len(pattern) > size { + size = len(pattern) + } + } + + // Find the longest prefix common to ALL patterns. + commonPrefix, _ := strutil.FindCommonPrefix(excludePatterns) + + // This loop will iterate over the length of the longest excludePattern + // (charInd = 1..size), generating an apparmor rule on each iteration + // for the corresponding subpatterns, understanding as such, the first + // (charInd+1) characters of the excludePatterns. + var builder strings.Builder + for charInd := 1; charInd < size; charInd++ { + // This loop will group the subpatterns properly, generating the subpatterns map, where: + // - the key would be the subpatternPrefix, considering as such the subpattern except + // its last character (pattern[0:charInd]). + // - the value would be the charset, which would be the subpattern last character + // (pattern[charInd]). If several subpatterns share the same subpatternPrefix, the + // charset would be a string including the last character of all those subpatterns. + subpatternPrefix := "" + subpatterns := map[string]string{} + for _, pattern := range excludePatterns { + // Handle unsupported cases + if (charInd < len(pattern)) && pattern[charInd] == '*' { + // Check if the excludePattern has a character different from '/' after a wildcard + if ((charInd + 1) < len(pattern)) && (pattern[charInd+1] != '/') { + return "", errors.New("exclude patterns does not support suffixes for now") + } + // Check if '*' is the first character after a common subpattern + for _, patt := range excludePatterns { + if (patt != pattern) && ((charInd) < len(patt)) && (pattern[:charInd] == patt[:charInd]) && (patt[charInd] != '*') { + return "", errors.New("first character after a common subpattern cannot be a wildcard") + } + } + } + + // Skip patterns that are already finished, wildcards and slashes preceded by wildcards. + if (charInd >= len(pattern)) || + (pattern[charInd] == '*') || + ((pattern[charInd] == '/') && (pattern[charInd-1] == '*')) { + continue + } + // Group subpatterns + subpatternPrefix = pattern[:charInd] + if charset, exists := subpatterns[subpatternPrefix]; !exists { + // Add the pattern if it didn't exist yet + subpatterns[subpatternPrefix] = string(pattern[charInd]) + } else { + if !strings.Contains(charset, string(pattern[charInd])) { + // Two patterns only differ on the last character + subpatterns[subpatternPrefix] = charset + string(pattern[charInd]) + } + } + } + + // Write patterns + if len(subpatterns) > 0 { + // First order keys to ensure profiles are always the same + // Sort key in map to ensure consistency in results + prefixes := make([]string, 0, len(subpatterns)) + for prefix := range subpatterns { + prefixes = append(prefixes, prefix) + } + sort.Strings(prefixes) + + // + // eg. /squashfs-root/usr/lib/[^a]** if len(subpatterns) == 1 + // eg. /squashfs-root/usr/lib/{[^a],[^b]}** if len(subpatterns) > 1 + builder.WriteString(opts.Prefix) + if charInd < len(commonPrefix) { + builder.WriteString(commonPrefix[:charInd]) + } else { + builder.WriteString(commonPrefix) + } + if len(subpatterns) > 1 { + builder.WriteRune('{') + } + for i := range prefixes { + if i > 0 { + builder.WriteRune(',') + } + if len(commonPrefix) < len(prefixes[i]) { + builder.WriteString(prefixes[i][len(commonPrefix):]) + } + builder.WriteString("[^" + subpatterns[prefixes[i]] + "]") + } + if len(subpatterns) > 1 { + builder.WriteRune('}') + } + builder.WriteString("**") + builder.WriteString(opts.Suffix) + builder.WriteRune('\n') + } + } + return builder.String(), nil +} + // LevelType encodes the kind of support for apparmor // found on this system. type LevelType int diff --git a/sandbox/apparmor/apparmor_test.go b/sandbox/apparmor/apparmor_test.go index 069a1ed9e6c..7332ea36ba5 100644 --- a/sandbox/apparmor/apparmor_test.go +++ b/sandbox/apparmor/apparmor_test.go @@ -718,3 +718,450 @@ func (s *apparmorSuite) TestSystemAppArmorLoadsSnapPolicy(c *C) { c.Check(loadsPolicy, Equals, tc.expectedResult, Commentf("%v", tc)) } } + +func (s *apparmorSuite) TestInsertAAREExclusionPatterns(c *C) { + tt := []struct { + comment string + aaRules string + excludePatterns []string + prefix string + suffix string + err string + expRule string + }{ + // simple single pattern cases + { + comment: "happy", + aaRules: ` +# Start of exclusion pattern +###EXCL{change_profile <> -> suf,:/ab}### +# End of exclusion pattern +`[1:], + excludePatterns: []string{"/ab"}, + prefix: "change_profile ", + suffix: " -> suf,", + expRule: ` +# Start of exclusion pattern +change_profile /[^a]** -> suf, +change_profile /a[^b]** -> suf, +# End of exclusion pattern +`[1:], + }, { + comment: "missing pattern", + aaRules: ` +# Start of exclusion pattern +###EXCL{change_profile <> -> wrong_suf,:/ab}### +# End of exclusion pattern +`[1:], + excludePatterns: []string{"/ab"}, + prefix: "change_profile ", + suffix: " -> suf,", + err: "placeholder pattern \\\"###EXCL{change_profile <> -> suf,:/ab}###\\\" not found", + }, + } + + for _, t := range tt { + comment := Commentf(t.comment) + opts := &apparmor.AAREExclusionPatternsOptions{ + Prefix: t.prefix, + Suffix: t.suffix, + } + res, err := apparmor.InsertAAREExclusionPatterns(t.aaRules, t.excludePatterns, opts) + if t.err != "" { + c.Assert(err, ErrorMatches, t.err, comment) + continue + } + c.Assert(err, IsNil) + + resHash, err1 := testutil.AppArmorParseAndHashHelper("profile test {" + res + "}") + expectedHash, err2 := testutil.AppArmorParseAndHashHelper("profile test {" + t.expRule + "}") + if (err1 != nil) || (err2 != nil) { + comment = Commentf(t.comment + "\n\nNote that an error occurred in AppArmorParseAndHashHelper " + + "while compiling and hashing the apparmor policy and string comparison was used as fallback.") + c.Assert(res, Equals, t.expRule, comment) + } else { + c.Assert(resHash, Equals, expectedHash, comment) + } + } +} + +func (s *apparmorSuite) TestGenerateAAREExclusionPatterns(c *C) { + tt := []struct { + comment string + excludePatterns []string + prefix string + suffix string + err string + expRule string + }{ + // simple single pattern cases + { + comment: "single shortest", + excludePatterns: []string{"/ab"}, + prefix: "change_profile ", + suffix: " -> suf,", + expRule: ` +change_profile /[^a]** -> suf, +change_profile /a[^b]** -> suf, +`[1:], + }, + { + comment: "single simple with wildcard", + excludePatterns: []string{"/a/*/bc"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^/]** r, +/a/*/[^b]** r, +/a/*/b[^c]** r, +`[1:], + }, + + // multiple pattern cases + { + comment: "same length no overlap shortest", + excludePatterns: []string{"/a", "/d"}, + suffix: " r,", + expRule: ` +/[^ad]** r, +`[1:], + }, + { + comment: "diff length no overlap shortest", + excludePatterns: []string{"/a", "/dc"}, + suffix: " r,", + expRule: ` +/[^ad]** r, +/d[^c]** r, +`[1:], + }, + { + comment: "diff length overlap", + excludePatterns: []string{"/ad", "/adc"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^d]** r, +/ad[^c]** r, +`[1:], + }, + { + comment: "same length no overlap", + excludePatterns: []string{"/ab", "/de"}, + suffix: " r,", + expRule: ` +/[^ad]** r, +/{a[^b],d[^e]}** r, +`[1:], + }, + { + comment: "same length overlap", + excludePatterns: []string{"/ab", "/ac"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^bc]** r, +`[1:], + }, + { + comment: "same length overlap", + excludePatterns: []string{"/AB", "/AC"}, + suffix: " r,", + expRule: ` +/[^A]** r, +/A[^BC]** r, +`[1:], + }, + { + comment: "same length overlap with wildcard", + excludePatterns: []string{"/ab/*/c", "/ad/*/c"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^bd]** r, +/a{b[^/],d[^/]}** r, +/a{b/*/[^c],d/*/[^c]}** r, +`[1:], + }, + { + comment: "different length same overlap with extra nonoverlapping", + excludePatterns: []string{"/a/bc/d", "/a/bc/e", "/a/fg"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^/]** r, +/a/[^bf]** r, +/a/{b[^c],f[^g]}** r, +/a/bc[^/]** r, +/a/bc/[^de]** r, +`[1:], + }, + { + comment: "diff length overlap with wildcard", + excludePatterns: []string{"/abc/*/c", "/ad/*/c"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^bd]** r, +/a{b[^c],d[^/]}** r, +/abc[^/]** r, +/ad/*/[^c]** r, +/abc/*/[^c]** r, +`[1:], + }, + { + comment: "more diff length overlap with wildcard", + excludePatterns: []string{"/abc/*/c", "/ab/*/e", "/ad/*/c"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^bd]** r, +/a{b[^c/],d[^/]}** r, +/abc[^/]** r, +/a{b/*/[^e],d/*/[^c]}** r, +/abc/*/[^c]** r, +`[1:], + }, + { + comment: "very diff length overlap with wildcard after diff length", + excludePatterns: []string{"/abc/*/c/*/e", "/ad/*/c"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^bd]** r, +/a{b[^c],d[^/]}** r, +/abc[^/]** r, +/ad/*/[^c]** r, +/abc/*/[^c]** r, +/abc/*/c[^/]** r, +/abc/*/c/*/[^e]** r, +`[1:], + }, + { + comment: "same length overlap with wildcard in overlap", + excludePatterns: []string{"/a/*/b", "/a/*/c"}, + suffix: " r,", + expRule: ` +/[^a]** r, +/a[^/]** r, +/a/*/[^bc]** r, +`[1:], + }, + + // unhappy cases + { + comment: "duplicates", + excludePatterns: []string{"/ab/*/c", "/ad/*/c", "/ad/*/c"}, + err: "exclude patterns contain duplicates", + }, + { + comment: "nothing", + excludePatterns: []string{}, + err: "no patterns provided", + }, + { + comment: "relative path", + excludePatterns: []string{"foo"}, + err: "exclude patterns must be absolute filepaths", + }, + { + comment: "pattern has suffix", + excludePatterns: []string{"/foo/*-bar"}, + err: "exclude patterns does not support suffixes for now", + }, + { + comment: "wildcard after common pattern", + excludePatterns: []string{"/foo/*/bar", "/foo/bar"}, + err: "first character after a common subpattern cannot be a wildcard", + }, + // specific cases used around codebase + { + comment: "any file inherit exec rules except snap-confine for devmode snap executing other snaps", + excludePatterns: []string{ + "/snap/core/*/usr/lib/snapd/snap-confine", + "/snap/snapd/*/usr/lib/snapd/snap-confine", + }, + prefix: "", + suffix: " rwlix,", + expRule: ` +/[^s]** rwlix, +/s[^n]** rwlix, +/sn[^a]** rwlix, +/sna[^p]** rwlix, +/snap[^/]** rwlix, +/snap/[^cs]** rwlix, +/snap/{c[^o],s[^n]}** rwlix, +/snap/{co[^r],sn[^a]}** rwlix, +/snap/{cor[^e],sna[^p]}** rwlix, +/snap/{core[^/],snap[^d]}** rwlix, +/snap/snapd[^/]** rwlix, +/snap/core/*/[^u]** rwlix, +/snap/{core/*/u[^s],snapd/*/[^u]}** rwlix, +/snap/{core/*/us[^r],snapd/*/u[^s]}** rwlix, +/snap/{core/*/usr[^/],snapd/*/us[^r]}** rwlix, +/snap/{core/*/usr/[^l],snapd/*/usr[^/]}** rwlix, +/snap/{core/*/usr/l[^i],snapd/*/usr/[^l]}** rwlix, +/snap/{core/*/usr/li[^b],snapd/*/usr/l[^i]}** rwlix, +/snap/{core/*/usr/lib[^/],snapd/*/usr/li[^b]}** rwlix, +/snap/{core/*/usr/lib/[^s],snapd/*/usr/lib[^/]}** rwlix, +/snap/{core/*/usr/lib/s[^n],snapd/*/usr/lib/[^s]}** rwlix, +/snap/{core/*/usr/lib/sn[^a],snapd/*/usr/lib/s[^n]}** rwlix, +/snap/{core/*/usr/lib/sna[^p],snapd/*/usr/lib/sn[^a]}** rwlix, +/snap/{core/*/usr/lib/snap[^d],snapd/*/usr/lib/sna[^p]}** rwlix, +/snap/{core/*/usr/lib/snapd[^/],snapd/*/usr/lib/snap[^d]}** rwlix, +/snap/{core/*/usr/lib/snapd/[^s],snapd/*/usr/lib/snapd[^/]}** rwlix, +/snap/{core/*/usr/lib/snapd/s[^n],snapd/*/usr/lib/snapd/[^s]}** rwlix, +/snap/{core/*/usr/lib/snapd/sn[^a],snapd/*/usr/lib/snapd/s[^n]}** rwlix, +/snap/{core/*/usr/lib/snapd/sna[^p],snapd/*/usr/lib/snapd/sn[^a]}** rwlix, +/snap/{core/*/usr/lib/snapd/snap[^-],snapd/*/usr/lib/snapd/sna[^p]}** rwlix, +/snap/{core/*/usr/lib/snapd/snap-[^c],snapd/*/usr/lib/snapd/snap[^-]}** rwlix, +/snap/{core/*/usr/lib/snapd/snap-c[^o],snapd/*/usr/lib/snapd/snap-[^c]}** rwlix, +/snap/{core/*/usr/lib/snapd/snap-co[^n],snapd/*/usr/lib/snapd/snap-c[^o]}** rwlix, +/snap/{core/*/usr/lib/snapd/snap-con[^f],snapd/*/usr/lib/snapd/snap-co[^n]}** rwlix, +/snap/{core/*/usr/lib/snapd/snap-conf[^i],snapd/*/usr/lib/snapd/snap-con[^f]}** rwlix, +/snap/{core/*/usr/lib/snapd/snap-confi[^n],snapd/*/usr/lib/snapd/snap-conf[^i]}** rwlix, +/snap/{core/*/usr/lib/snapd/snap-confin[^e],snapd/*/usr/lib/snapd/snap-confi[^n]}** rwlix, +/snap/snapd/*/usr/lib/snapd/snap-confin[^e]** rwlix, +`[1:], + }, + { + comment: "anything except /lib/{firmware,modules} for non-core base template", + excludePatterns: []string{"/lib/firmware", "/lib/modules"}, + suffix: " mrklix,", + expRule: ` +/[^l]** mrklix, +/l[^i]** mrklix, +/li[^b]** mrklix, +/lib[^/]** mrklix, +/lib/[^fm]** mrklix, +/lib/{f[^i],m[^o]}** mrklix, +/lib/{fi[^r],mo[^d]}** mrklix, +/lib/{fir[^m],mod[^u]}** mrklix, +/lib/{firm[^w],modu[^l]}** mrklix, +/lib/{firmw[^a],modul[^e]}** mrklix, +/lib/{firmwa[^r],module[^s]}** mrklix, +/lib/firmwar[^e]** mrklix, +`[1:], + }, + { + comment: "anything except /usr/src, /usr/lib/{firmware,snapd,modules} for non-core base template", + excludePatterns: []string{"/usr/lib/firmware", "/usr/lib/modules", "/usr/lib/snapd"}, + suffix: " mrklix,", + expRule: ` +/[^u]** mrklix, +/u[^s]** mrklix, +/us[^r]** mrklix, +/usr[^/]** mrklix, +/usr/[^l]** mrklix, +/usr/l[^i]** mrklix, +/usr/li[^b]** mrklix, +/usr/lib[^/]** mrklix, +/usr/lib/[^fms]** mrklix, +/usr/lib/{f[^i],m[^o],s[^n]}** mrklix, +/usr/lib/{fi[^r],mo[^d],sn[^a]}** mrklix, +/usr/lib/{fir[^m],mod[^u],sna[^p]}** mrklix, +/usr/lib/{firm[^w],modu[^l],snap[^d]}** mrklix, +/usr/lib/{firmw[^a],modul[^e]}** mrklix, +/usr/lib/{firmwa[^r],module[^s]}** mrklix, +/usr/lib/firmwar[^e]** mrklix, +`[1:], + }, + { + comment: "anything except /var/lib/{dhcp,extrausers,jenkins,snapd} /var/log /var/snap and /var/tmp for non-core base template", + excludePatterns: []string{ + "/var/lib/dhcp", + "/var/lib/extrausers", + "/var/lib/jenkins", + "/var/lib/snapd", + "/var/log", + "/var/snap", + "/var/tmp", + }, + suffix: " mrklix,", + expRule: ` +/[^v]** mrklix, +/v[^a]** mrklix, +/va[^r]** mrklix, +/var[^/]** mrklix, +/var/[^lst]** mrklix, +/var/{l[^io],s[^n],t[^m]}** mrklix, +/var/{li[^b],lo[^g],sn[^a],tm[^p]}** mrklix, +/var/{lib[^/],sna[^p]}** mrklix, +/var/lib/[^dejs]** mrklix, +/var/{lib/d[^h],lib/e[^x],lib/j[^e],lib/s[^n]}** mrklix, +/var/{lib/dh[^c],lib/ex[^t],lib/je[^n],lib/sn[^a]}** mrklix, +/var/{lib/dhc[^p],lib/ext[^r],lib/jen[^k],lib/sna[^p]}** mrklix, +/var/{lib/extr[^a],lib/jenk[^i],lib/snap[^d]}** mrklix, +/var/{lib/extra[^u],lib/jenki[^n]}** mrklix, +/var/{lib/extrau[^s],lib/jenkin[^s]}** mrklix, +/var/lib/extraus[^e]** mrklix, +/var/lib/extrause[^r]** mrklix, +/var/lib/extrauser[^s]** mrklix, +`[1:], + }, + { + comment: "everything except {/var/lib/snapd/hostfs,}/{dev,proc,sys} for system-backup", + excludePatterns: []string{ + "/dev/", + "/proc/", + "/sys/", + "/var/lib/snapd/hostfs/dev/", + "/var/lib/snapd/hostfs/proc/", + "/var/lib/snapd/hostfs/sys/", + }, + suffix: " r,", + expRule: ` +/[^dpsv]** r, +/{d[^e],p[^r],s[^y],v[^a]}** r, +/{de[^v],pr[^o],sy[^s],va[^r]}** r, +/{dev[^/],pro[^c],sys[^/],var[^/]}** r, +/{proc[^/],var/[^l]}** r, +/var/l[^i]** r, +/var/li[^b]** r, +/var/lib[^/]** r, +/var/lib/[^s]** r, +/var/lib/s[^n]** r, +/var/lib/sn[^a]** r, +/var/lib/sna[^p]** r, +/var/lib/snap[^d]** r, +/var/lib/snapd[^/]** r, +/var/lib/snapd/[^h]** r, +/var/lib/snapd/h[^o]** r, +/var/lib/snapd/ho[^s]** r, +/var/lib/snapd/hos[^t]** r, +/var/lib/snapd/host[^f]** r, +/var/lib/snapd/hostf[^s]** r, +/var/lib/snapd/hostfs[^/]** r, +/var/lib/snapd/hostfs/[^dps]** r, +/{var/lib/snapd/hostfs/d[^e],var/lib/snapd/hostfs/p[^r],var/lib/snapd/hostfs/s[^y]}** r, +/{var/lib/snapd/hostfs/de[^v],var/lib/snapd/hostfs/pr[^o],var/lib/snapd/hostfs/sy[^s]}** r, +/{var/lib/snapd/hostfs/dev[^/],var/lib/snapd/hostfs/pro[^c],var/lib/snapd/hostfs/sys[^/]}** r, +/var/lib/snapd/hostfs/proc[^/]** r, +`[1:], + }, + } + + for _, t := range tt { + comment := Commentf(t.comment) + opts := &apparmor.AAREExclusionPatternsOptions{ + Prefix: t.prefix, + Suffix: t.suffix, + } + res, err := apparmor.GenerateAAREExclusionPatterns(t.excludePatterns, opts) + if t.err != "" { + c.Assert(err, ErrorMatches, t.err, comment) + continue + } + c.Assert(err, IsNil) + + resHash, err1 := testutil.AppArmorParseAndHashHelper("profile test {" + res + "}") + expectedHash, err2 := testutil.AppArmorParseAndHashHelper("profile test {" + t.expRule + "}") + if (err1 != nil) || (err2 != nil) { + comment = Commentf(t.comment + "\n\nNote that an error occurred in AppArmorParseAndHashHelper " + + "while compiling and hashing the apparmor policy and string comparison was used as fallback.") + c.Assert(res, Equals, t.expRule, comment) + } else { + c.Assert(resHash, Equals, expectedHash, comment) + } + } +} diff --git a/strutil/commonprefix.go b/strutil/commonprefix.go new file mode 100644 index 00000000000..f83ca5911d5 --- /dev/null +++ b/strutil/commonprefix.go @@ -0,0 +1,53 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2024 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * 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, see . + * + */ + +package strutil + +import "errors" + +func FindCommonPrefix(patterns []string) (string, error) { + if len(patterns) == 0 { + return "", errors.New("no patterns provided") + } + if len(patterns) == 1 { + return patterns[0], nil + } + + // Find the length of shortest pattern (minSize) + minSize := len(patterns[0]) + for _, pattern := range patterns[1:] { + if len(pattern) < minSize { + minSize = len(pattern) + } + } + + // Find the longest prefix common to ALL patterns. + commonPrefix := "" +findCommonPrefix: + for charInd := 0; charInd < minSize; charInd++ { + for _, pattern := range patterns[1:] { + if pattern[charInd] != patterns[0][charInd] { + break findCommonPrefix + } + } + commonPrefix = patterns[0][:charInd+1] + } + + return commonPrefix, nil +} diff --git a/strutil/commonprefix_test.go b/strutil/commonprefix_test.go new file mode 100644 index 00000000000..1d61a4bf055 --- /dev/null +++ b/strutil/commonprefix_test.go @@ -0,0 +1,83 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2018 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * 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, see . + * + */ + +package strutil_test + +import ( + . "gopkg.in/check.v1" + + "github.com/snapcore/snapd/strutil" +) + +type commonPrefixSuite struct{} + +var _ = Suite(&commonPrefixSuite{}) + +func (s *commonPrefixSuite) TestCommonPrefix(c *C) { + tt := []struct { + patterns []string + commonPrefix string + err string + }{ + { + []string{}, + "", + "no patterns provided", + }, + { + []string{ + "/one/single/pattern", + }, + "/one/single/pattern", + "", + }, + { + []string{ + "/pattern/n/one", + "/pattern/n/two", + }, + "/pattern/n/", + "", + }, + { + []string{ + "/one/", + "/one/two/", + }, + "/one/", + "", + }, + { + []string{ + "$ONE", + "/one/two/", + }, + "", + "", + }, + } + + for _, t := range tt { + commonPrefix, err := strutil.FindCommonPrefix(t.patterns) + c.Assert(commonPrefix, Equals, t.commonPrefix) + if t.err != "" { + c.Assert(err, ErrorMatches, t.err) + } + } +} diff --git a/testutil/apparmor.go b/testutil/apparmor.go new file mode 100644 index 00000000000..c84b1058bbf --- /dev/null +++ b/testutil/apparmor.go @@ -0,0 +1,70 @@ +// -*- Mode: Go; indent-tabs-mode: t -*- + +/* + * Copyright (C) 2024 Canonical Ltd + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * 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, see . + * + */ + +package testutil + +import ( + "crypto/sha1" + "encoding/hex" + "fmt" + "io" + "os/exec" +) + +func AppArmorParseAndHashHelper(profile string) (string, error) { + // Create app_armor parser command with arguments to only return the compiled + // policy to stdout. The profile is not cached or loaded. + apparmorParser := exec.Command("apparmor_parser", "-QKS") + + // Get stdin and stdout to pipe the command + apparmorParserStdin, err := apparmorParser.StdinPipe() + if err != nil { + return "Error creating stdin pipe for apparmor_parser", err + } + apparmorParserStdout, err := apparmorParser.StdoutPipe() + if err != nil { + return "Error creating stdout pipe for apparmor_parser", err + } + + // Start apparmor_parser command + if err := apparmorParser.Start(); err != nil { + return "Error starting apparmor_parser", err + } + + // Write apparmor profile to apparmor_parser stdin + go func() { + defer apparmorParserStdin.Close() + io.WriteString(apparmorParserStdin, profile) + }() + + // Calculate the hash + h := sha1.New() + io.Copy(h, apparmorParserStdout) + + // Get apparmor_parser command output + if err := apparmorParser.Wait(); err != nil { + if exiterr, ok := err.(*exec.ExitError); ok { + return fmt.Sprintf("apparmor_parser command exited with status code %d", exiterr.ExitCode()), err + } else { + return "Error waiting for apparmor_parser command", err + } + } + + return hex.EncodeToString(h.Sum(nil)), nil +}