235 Commits

Author SHA1 Message Date
Michael Vogt
f43583ca37 snap-{seccomp,confine}: replace global seccomp filter with template
The global.bin seccomp filter was written before we had support for
explicit deny rules in snap-seccomp. With these rules we can replace
the hard to followup logic of the global filter and just make the
rules part of the standard seccomp template.

The global rules are best summarized in this comment:
```
struct scmp_arg_cmp no_tty_inject = {
    /* We learned that existing programs make legitimate requests with all
     * bits set in the more significant 32bit word of the 64 bit double
     * word. While this kernel behavior remains suspect and presumably
     * undesired it is unlikely to change for backwards compatibility
     * reasons. As such we cannot block all requests with high-bits set.
     *
     * When faced with ioctl(fd, request); refuse to proceed when
     * request&0xffffffff == TIOCSTI. This specific way to encode the
     * filter has the following important properties:
     *
     * - it blocks ioctl(fd, TIOCSTI, ptr).
     * - it also blocks ioctl(fd, (1UL<<32) | TIOCSTI, ptr).
     * - it doesn't block ioctl(fd, (1UL<<32) | (request not equal to TIOCSTI), ptr); */
    .arg = 1,
    .op = SCMP_CMP_MASKED_EQ,
    .datum_a = 0xffffffffUL,
    .datum_b = TIOCSTI,
};
sc_err = seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), sys_ioctl_nr, 1, no_tty_inject);
```
and the same for `TIOCLINUX`.
2024-06-19 08:28:39 +02:00
Andrew Phelps
71d22420df many: add a *SnapAppSet to ConnectedPlug/Slot types and use it to build label expressions in interfaces (#13773)
Now that we have app sets in the interfaces repo, keep a pointer to them in ConnectedPlug/Slot types. Use this to build label expressions in the interfaces. 

* many: add a pointer to a SnapAppSet into Connected(Plug|Slot) to that interfaces can build a complete label expression, including component hooks

* interfaces: update doc comments on ConnectedPlug/Slot.AppSet

* interfaces: remove TODO that has been addressed

* interfaces: use app set pointer for instance name check

* snap: add Runnable type that represents the runnable parts of a snap

* interfaces, o/ifacestate: use snap.Runnable rather than interfaces.Runnable

* interfaces, i/builtin, o/ifacestate: panic on failed invariant check in NewConnectedPlug/Slot

* interfaces: add methods to app set for getting runnables that can connect to plug/slot

* interfaces: build label expressions using runnables

* interfaces: doc comment for SlotRunnables

* interfaces: implement Slot/PlugRunnables with shared helper

* interfaces: log and skip security tags that do not match expected pattern

* snap, interfaces: move runnable constructors to methods on AppInfo and HookInfo

* interfaces: refactor to allow labelExpr to operate directly on a ConnectedPlug/Slot

* snap: move around Runnable methods
2024-06-14 18:37:26 +02:00
Oliver Calder
0ff642e82e many: include prompt prefix in apparmor rules (#13822)
* features,i/{apparmor,builtin}: include prompt prefix in home interface

If prompting is supported and enabled, include the prompt prefix in
AppArmor rules for the home interface, which will cause AppArmor to send
a prompt when accessing any file in $HOME.

In the future, if other interfaces include the ###PROMPT### prefix in
their rule snippets, this will also be handled accordingly.

At the moment, the status of prompting support is checked whenever the
AppArmor backend prepares profiles. This is okay, since AppArmor support
for prompting depends on kernel and parser features, which are only
probed once after snapd starts. However, to ensure that the same
supported value is used even if that were not the case, and in case we
wish to only use the prompt prefix for some snaps or interfaces, we may
wish to embed whether to use the prompt prefix in the AppArmor
Specification instead.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* features: adjust unsupported messages when checking apparmor features errors

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* interfaces: add prompting status to system key

Include whether prompting is supported and enabled as a single field in
the system key. This way, if `(supported && enabled)` changes, security
profiles will be regenerated when snapd starts up.

Currently, prompting support only changes when the AppArmor kernel or
parser features change, and profile regeneration is the only other place
where it is checked whether AppArmor prompting is supported and enabled.
Thus, including whether prompting is supported and enabled in the system
key ensures that security profiles are regenerated when necessary during
snapd startup, and only when necessary (e.g. not if support changed but
prompting flag remained disabled nor if flag changed but prompting
remained unsupported).

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* o/c/configcore: restart snapd when prompting value changes

When the prompting flag changes and the result entails that security
profiles should be regenerated, restart snapd to do so.

This is required iff prompting is supported and the experimental
apparmor-prompting flag changes -- if prompting is not supported,
prompting can't be used, so no need to regenerate profiles. Importantly,
prompting support is based entirely on the available AppArmor kernel and
parser features, and these are only probed once during snapd startup, so
prompting support cannot change (under the current implementation)
except when snapd restarts.

Since `(supported && enabled)` is part of the system key, and a restart
is only triggered if prompting is supported and the flag value changes
(which is equivalent to `(supported && enabled)`, since the supported
value cannot change while snapd is running), restarting after the flag
has changed causes the system key to be different, and thus to trigger a
security profile regeneration, as desired.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* interfaces,o/ifacestate: set prompting in apparmor spec

Rather than checking whether AppArmor prompting is supported and enabled
whenever the AppArmor backend is processing a snippet, instead include
that precomputed value in the Specification itself, and place it there
via `buildConfinementOptions`. This way, any spec created with the same
`confinementOptions` will make the same decision as to whether to
include prompt prefixes on relevant rules.

Currently, `buildConfinementOptions` simply checks whether prompting is
supported and enabled via the methods on `features.AppArmorPrompting`,
but ideally, this value would be looked up from either the system key
or by checking whether the prompting listener is running. It remains to
be seen how the value computed as part of the system key can be
guaranteed to be the same as that used elsewhere, either in
`buildConfinementOptions` or when deciding whether to start the
listener.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* o/c/configcore: request snapd restart whenever prompting flag changes

Previously, a snapd restart was only requested when the status of the
"apparmor-prompting" experimental feature flag changed and prompting was
supported. However, since prompting support is dependent on AppArmor
kernel and parser features which are probed only once during startup,
and systems which do not use vendored AppArmor may have had an update to
the system AppArmor package which newly supports AppArmor prompting, it
is safer to request a restart of snapd to re-check for prompting
support.

This way, if one is enabling prompting for the first time on a system
without prompting support, they can have snapd installed, update their
kernel or apparmor installation to support prompting, and then set the
prompting flag to enable prompting without needing to manually restart
snapd.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* interfaces: support optional trailing space after ###PROMPT###

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

i/apparmor: move promptReplacer definition to before its use

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: add test of restart behavior when setting experimental.apparmor-prompting

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: refactor prompting test to reset failed count and safely check for restarts

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

tests: add shellcheck exception for apparmor prompting flag restart test

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: check that snapd PID != 0 and use snap changes to wait for feature change to complete

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: check for start-limit-hit before calling reset-failed

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: add ubuntu core to apparmor prompting flag restart test

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: check apparmor-prompting value after setting it unchanged

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* Revert "tests: check for start-limit-hit before calling reset-failed"

This reverts commit bea68516c3287fa44d6718f0794484746ae99ac5.

* tests: check systemd start-limit-hit when apparmor-prompting flag changed

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* o/configstate/configcore: add unit tests for doExperimentalApparmorPromptingDaemonRestart

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* i/builtin: add missing prompt prefix and adjust test

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* i/apparmor: add test for prompt prefix substitution

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* many: pass prompting value into system key functions

As such, we now precompute whether prompting is supported and enabled in
`InterfaceManager.StartUp()`, store it in the `InterfaceManager`
instance, and pass it into the call to `WriteSystemKey()`.

Additionally, we make `buildConfinementOptions` a method of
`InterfaceManager`, thus eliminating the need to check within the system
key functions whether prompting is supported and enabled.

However, there remains a problem that `snap run` calls
`SystemKeyMismatch()`, which previously invoked
`apparmor.ParserFeatures()` via `AppArmorPrompting.IsSupported()`, and
now calls `AppArmorPrompting.IsSupported()` directly and passes the
result into `SystemKeyMismatch()`. In either case, we really want this
to be avoided if at all possible, since `snap run` does not have access
to the cached value from the `InterfaceManager`, and thus must invoke
the `apparmor_parser` binary to check parser features whenever we want
to run any snap.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* many: don't probe parser features when checking system key mismatch

Since `snap run` calls `SystemKeyMismatch()`, we want to avoid needing
to call `AppArmorPrompting.IsSupported()` if at all possible, since this
calls `apparmor.ParserFeatures()`, which executes the `apparmor_parser`
binary. We can and should call `AppArmorPrompting.IsSupported()` when
writing the system key, but not when checking for a mismatch.

The system key written to disk should correctly hold the list of kernel
and parser features, the parser mtime, and whether or not prompting was
previously supported and enabled. We can check whether apparmor parser
features have changed by checking the parser mtime, without needing to
probe parser features -- this optimization is already used by
`SystemKeyMismatch()`. If we knew whether prompting was previously
supported (regardless of whether it was enabled), then so long as the
parser and kernel features are unchanged, we know that prompting support
is also unchanged.

Thus, if we add a second prompting-related field to the system key, this
one storing whether prompting is supported (ignoring enabled), we can
check if prompting support is unchanged without needing to call
`AppArmorPrompting.IsSupported()`.

Furthermore, `SystemKeyMismatch()` is the function in question, and if
there is any mismatch detected, it should return such as soon as
possible, regardless of what the mismatch is. Therefore, if we know that
either kernel or parser features have changed, then we can immediately
return that there is a mismatch, and we don't need to check whether
those feature changes affect prompting support.

Therefore, the new cases which we must worry about when checking for a
system key mismatch are the following, when all other system key fields
are unchanged (note that prompting must be supported in order to be
supported&&enabled):

1. supported: F, supported&&enabled: F, newFlag: F, mismatch: F
2. supported: F, supported&&enabled: F, newFlag: T, mismatch: F
3. supported: T, supported&&enabled: F, newFlag: F, mismatch: F
4. supported: T, supported&&enabled: F, newFlag: T, mismatch: T
5. supported: T, supported&&enabled: T, newFlag: F, mismatch: T
6. supported: T, supported&&enabled: T, newFlag: T, mismatch: F

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* interfaces: fix test string formatting

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* o/configstate/configcore: adjust prompting-related comments

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: increase prompting check_snapd_restarted timeout and add systemd show

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: reset start limit when checking if snapd restarted after prompting change

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* many: add system key extra data to hold prompting enabled status

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* many: only store one apparmor prompting value in system key

When checking for a system key mismatch, use the stored AppArmor
parser features from the system key on disk (along with the kernel
features from the newly-generated key) to check whether prompting is
supported, and AND that with the `AppArmorPrompting` value passed in
with the `SystemKeyExtraData`. If the kernel or parser features have
changed, the system key will be a mismatch anyway, so it is perfectly
safe to use the existing parser features to check for prompting support.

As such, the functions to check for prompting support have been moved
from `features` to `sandbox/apparmor`, and the support check has been
separated from the call to get `ParserFeatures()` and
`KernelFeatures()`, so that the values from the system key can be passed
in instead of invoking those functions.

Using the system key's stored parser and kernel features, there is no
need to save whether prompting is supported as part of the system key,
simplifying the key and the logic used to set the prompting value.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: explicitly install jq in apparmor-prompting-flag-restart test

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* many: consolidate checks for apparmor prompting support

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* interfaces,s/apparmor: use features struct when checking prompting support

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: improve logging in apparmor-prompting-flag-restart test

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

* tests: fix prompting flag restart test on core18

For some reason, when snapd fails due to start-limit-hit on core18, the
snapd.failure.service starts and acquires the state lock, thus
preventing snapd from successfully becoming "active" again and leaving
it retrying at "activating". It is unclear why this happens on core18
and not elsewhere.

As a fix, when resetting the start limit, stop snapd.failure.service
manually to ensure that snapd can successfully start.

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>

---------

Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
2024-06-11 18:13:00 +01:00
Andrew Phelps
c1cf798549 many: generate security profiles from component hooks (#13686)
* interfaces: add method to SnapAppSet for getting list of runnable entities

* interfaces: replace references to hooks/apps in backends with usage of SnapAppSet.Runnables

* interfaces: rename SecurityTagGlob to SecurityTagGlobs and make it handle component hooks

* snap: add method for getting component hooks for a specific plug

* interfaces: add component hooks to output of SecurityTagsForConnectedPlug

* snapstate: add methods for getting components installed for the current and arbitrary revisions of a snap

* o/ifacestate: properly set up SnapAppSets with components prior to passing them off to security backends

* o/snapstate: create setup-profiles task when installing a component

* many: add side info param to snaptest.MockComponent

* many: fix failing tests caused by changes in rebase

* snap: add ComponentHookSecurityTag for getting a component hook's security tag

* interfaces: implement SecurityTagGlobs with snap.ComponentHookSecurityTag

* interfaces: move some helper functions to helpers.go

* o/snapstate: add functions that are useful when operating on component-related tasks

* o/ifacestate: use functions from snapstate rather than local functions

* i/apparmor: cleanup comment and whitespace

* o/snapstate: replace some speculative code with TODOs for now

* interfaces, o/ifacestate: remove Type from interfaces.Runnable and do not sort the result of SnapAppSet.Runnables()

* o/snapstate: remove unused variable
2024-05-08 11:30:03 -04:00
Andrew Phelps
277fbc266e many: add components to interfaces.SnapAppSet (#13837) 2024-05-07 09:53:59 -04:00
Jorge Sancho Larraz
182b8ae699 snap-seccomp, snap-confine, i/seccomp, tests: rework seccomp denylist (#13443)
* snap-{seccomp,confine}: rework seccomp denylist

When a denylist was introduced in PR#12849 we reached the limits
of the API of libseccomp and some things are now hard to reason
about [1]. This is mostly because what we try to do does not
match the libseccomp API very well and a slightly different
approach is probably more aligned with it's design (see also
this libseccomp issue [2] that is related to our issue).

So this commit changes the approach and instead of trying to use
a single filter the work is split into two filters:
1. explicit allow list
2. explicit deny list

and then load both filters into the kernel. The way the kernel
works is that it selects the most restrictive action.

So in the case of PR#12849:
```
~ioctl - TIOCSTI
~ioctl - TIOCLINUX
ioctl
```
For `ioctl(TIOCLINUX)` the first allow filter would pass `ioctl`
but the second deny filter would correctly deny the TIOCLINUX.

The file format of the `snap.snap.app.bin` changes to `.bin2`
and includes a proper header that would allow us to extend more
easily in the future.

The exiting tests for negative filtering got also updated so that
the deny/allow is tested via different errno numbers to ensure that
the expected filter denies the access.

The `snap-seccomp` spread test now also runs on all ubuntu releases.

This work will also allow us to remove the `global.bin` seccomp
filter in a followup PR.

[1] https://github.com/snapcore/snapd/pull/12849#discussion_r1206855700
[2] https://github.com/seccomp/libseccomp/issues/44

* snap-confine: tweak sc_seccomp_file_header struct (thanks Philip!)

* snap-confine: tweak struct init workaround in sc_apply_seccomp_profile_for_security_tag (thanks Philip)

* snap-seccomp: remove outdated comment about big endian

* snap-confine: rename sc_must_read_header_from_file->sc_must_read_and_validate_header_from_file

* snap-seccomp: rework exportBPF() to not require a temp file

Thanks to Valentin for the suggestion. Also reverts the change to
the `install-store-laaaarge` tests because there is no need for
space in /tmp anymore.

* tests: improve messae in security-seccomp deny test

* snap-confine: rename "struct stat buf" -> "struct stat stat_buf"

* snap-confine: check that filer size if multiple of sock_filter

Thanks to Valentin for the suggestion. Also adds a bunch of
C unit tests to check that the code works correctly. Sadly
C makes it hard to write this in a concise way so there is
a bit of repetition.

* snap-confine: extract must_read_and_validate_header_from_file_dies_with() helper

* snap-confine: workaround bug in gcc from 14.04

The gcc (4.8.4) in 14.04 will not compile the following code:
```
	struct sc_seccomp_file_header hdr = {0};
```
and will error with:
```
snap-confine/seccomp-support.c: In function ‘sc_apply_seccomp_profile_for_security_tag’:
snap-confine/seccomp-support.c:246:9: error: missing braces around initializer [-Werror=missing-braces]
  struct sc_seccomp_file_header hdr = {0};
         ^
snap-confine/seccomp-support.c:246:9: error: (near initialization for ‘hdr.header’) [-Werror=missing-braces]
```

to workaround this a pragma is added.

* snap-confine: check filters are not empty and keep read access to global.bin file

* tests: add details field to security-profiles and snap-seccomp spread tests

* snap-confine: move empty filter validation to sc_must_read_filter_from_file to avoid conflicts with classic snaps

* snap-{seccomp,confine}: add tests for missing seccomp profile and explicit deny has precedence to to explicit allow

* snap-confine: run make fmt

* cmd/snap-confine: make fmt again with indent 2.2.13+

Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>

* snap-confine: Several code improvements

* snap-confine: Fix format

* snap-confine: Test fix and update deprecated SEEK_CUR

* snap-confine: Fix test

* snap-confine: Use inclusive language where possible

* snap-confine: Make woke happy until we can remove cmd/snap-seccomp-blacklist

---------

Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Co-authored-by: Michael Vogt <mvo@ubuntu.com>
Co-authored-by: Maciej Borzecki <maciej.borzecki@canonical.com>
2024-04-10 17:50:09 +02:00
Miguel Pires
79c5ac14b2 many: remove usages of deprecated io/ioutil package (#13768)
* many: remove usages of deprecated io/ioutil package

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

* .golangci.yml: remove errcheck ignore rule for io/ioutil

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

* run-checks: prevent new usages of io/ioutil

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>

---------

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
2024-04-03 23:23:24 +02:00
Andrew Phelps
069b7f684e many: use interfaces.SnapAppSet in security backends (#13587) 2024-02-27 14:52:02 -05:00
Andrew Phelps
d8a2e847e5 many: introduce SnapAppSet for use in security backends (#13574)
* interfaces: introduce SnapAppSet and forward one to the security backends

* snap: implement methods on snap.Info for getting apps and hooks for slots and plugs

This will enable us to remove the Hooks fields from the SlotInfo and
PlugInfo structs.

* interfaces: implement methods on SnapAppSet in terms of methods on snap.Info

* snap, interfaces: replace usage of {Plug,Slot}Info.SecurityTags with methods on SnapAppSet

* i/builtin: replace slotAppLabelExpr and plugAppLabelExpr with corresponding methods on SnapAppSet

* snap, o/snapstate, interfaces: remove Hooks field on snap.PlugInfo and snap.SlotInfo

* builtin, interfaces: fix tests that use Specification that now have a SnapAppSet

* snap: add tests for new methods on Info

* interfaces, i/builtin: port over some tests for SnapAppSet methods {Plug,Slot}LabelExpression

* interfaces: test PlugSecurityTags and SlotSecurityTags methods

* interfaces: add doc comments to SnapAppSet and methods

* i/builtin: remove ported over tests

* interfaces, many: require that SnapAppSet methods for getting security tags are called with plug/slot that comes from correct snap

Many tests did not properly adhere to this requirment, so they had to be
modifed to modify this rule.

Additionally, a hack was inroduced in the methods for getting label
expressions on the SnapAppSet. If a plug/slot did not originate from the
same snap that the SnapAppSet was created from, then we will use the
snap.Info that the plug/slot carries in the method instead. This will
fail to work once component hooks are introduced, so this will need to
be resolved by then.

* interfaces: test fallback for using LabelExpr methods with mismatch plug/slot

* snap: correct placement of TODOs to preserve doc comments

* snap: add doc comments for Plug/Slot.Unscoped

* interfaces: test for using SecurityTagsForPlug and SecurityTagsForSlot with wrong snap

* interfaces: tweak error messages in SnapAppSet SecurityTags methods

* i/builtin: fix missed conflict

* i/apparmor: add doc comment to Specification.appSet

* snap: fix doc coment on PlugInfo.Unscoped
2024-02-19 11:57:42 +01:00
Miguel Pires
29c9752d66 many: s/ioutil.WriteFile/os.WriteFile (#13217)
Replace ioutil.WriteFile with os.WriteFile since the former has been
deprecated since go1.16 and simply calls the latter.

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
2023-09-26 11:38:46 +01:00
Michael Vogt
9c72433ef0 arch: add new arch.Endian() helper and use it in seccomp (#13028)
* arch: add new `arch.Endian()` helper and use it in seccomp

Go has no good way to get the native endianness of a system.
However for certain use-cases (like seccomp) this is quite
important. We already have a (hackish) `isBigEndian()` helper
in our code. However this will also be needed in snap-seccomp
for PR#13014 this will also be needed so moving the helper
to the `arch` package seems to be prudent.

* arch: fix usage of runtimeGOARCH

Co-authored-by: Miguel Pires <miguelpires94@gmail.com>

---------

Co-authored-by: Miguel Pires <miguelpires94@gmail.com>
2023-07-31 09:44:11 +02:00
Alexandre Lissy
bfce6e9bdf interfaces/seccomp/template: Adding kcmp to allow Mesa usecases
For fixing https://bugs.launchpad.net/snapd/+bug/1998980 implement the
suggested fix of allowing kcmp in the base template.
2023-05-29 11:22:42 +02:00
Michael Vogt
d7b49dd6f5 many: add a bunch of TODO/FIXME for a followup :) 2023-05-26 18:32:26 +02:00
Alex Murray
453f9e8395 interfaces/seccomp: explicitly disallow the use of ioctl + TIOCLINUX
Fixes CVE-2023-1523

Signed-off-by: Alex Murray <alex.murray@canonical.com>
2023-05-26 18:32:26 +02:00
Miguel Pires
d097436c1c many: fix formatting w/ gofmt 1.19
Go 1.19 includes some changes to gofmt which intend to make lists and
heading clearer when rendered (https://go.dev/doc/go1.19). This commit
is the result of running the new gofmt and manually fixing some of it.
This was necessary because the new gofmt assumed lines beginning w/ tabs
to start lists or examples. While this is often true in our codebase,
we occasionally also use tabs to indent the lines after a TODO or FIXME
prefix or in yaml (e.g., excerpts of a snap.yaml). This meant that a lot of the
reformatted comments were broken and had to be fixed manually.

Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
2023-01-16 14:23:11 +01:00
Alberto Mardegan
ecb4ba8247 seccomp: allow opening XDP sockets
This is the first step to allow applications to open AF_XDP sockets.
This change is limited to SecComp and is not enough to deliver the XDP
functionality. Two more obstables remain:

1) AppArmor: profiles need to have the rule `network xdp,`
2) Capabilities: the net_raw capability is needed to use XDP

This change basically just registers the XDP protocol as allowed in the
BPF profile of a snap.
2022-10-05 09:12:14 +03:00
Isaac True
15e53b3712 interfaces: posix-mq: add new interface
* interfaces: posix-mq: add new interface

- Add support for a new posix-mq interface, including AppArmor and seccomp
  rules. This allows creating, sending, and receiving IPC messages over POSIX
  message queues between snaps.
- Remove commented out seccomp rules for POSIX message queues in template.go

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: updated static information

- Added a declaration for plugs to allow snaps from the same publisher to automatically connect
- Added slot declaration to static information
- Changed slot to disallow auto connections by default
- Allow slot installation for all snap types
- Remove implicit interfaces

Signed-off-by: Isaac True <isaac.true@canonical.com>

* apparmor: add feature detection for POSIX message queues

Check if the AppArmor implementation supports the "mqueue" keyword.

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: extend validation and testing

- Permissions are now also validated in the BeforePrepare* functions
- Check if the AppArmor system supports the mqueue feature
- Extend unit tests to also check that unwanted permissions are not included
- Additionally change mq_notify syscall to be included when the read permission is used

Signed-off-by: Isaac True <isaac.true@canonical.com>

* apparmor: update unit tests to include mqueue feature detection

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: extend MQ path validation

- Ensure the given path is not an AppArmor regex and is a clean path
- Surround the path with quotes in the AppArmor rule
- Update unit tests to with the new validation

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: remove unneeded aliases

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: mark slot as super-privileged

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: update path handling

- If the path does not begin with '/', add a '/'
- Use the name of the slot as the path if no path has been given

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: remove get/setattr

These permissions are not supported by AppArmor

* interfaces: posix-mq: remove redundant connected slot rule

Additionally added indenting the AppArmor rules

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: add open permission

* interfaces: posix-mq: update AppArmor snippet generation

- The permanent slot snippet is now generated from the complete list of
  available permissions, rather than hard-coded specific permissions.
- Append open to the connected plug permissions array rather than directly
  entering it into the snippet.
- Update unit tests to reflect new changes.

Signed-off-by: Isaac True <isaac.true@canonical.com>

* tests: add posix-mq to interfaces-many-snap-provided

* interfaces: posix-mq: replace function with strutil.ListContains

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: mock AppArmor feature in unit tests

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: add posix-mq test cases to base declaration tests

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: change test variable names to reflect test cases

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: improve comments and error messages

- Remove slot name from error messages
- Standardise comment format
- Additionally remove permission validation from BeforePreparePlug as the permissions are configured in the slot

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: add additional unit tests and rework permission parsing

- Test to ensure that the path attribute is a string
- Test that the permissions attribute only contains valid permissions
- Add functionality and unit test to ensure that the permissions attribute is a list of strings

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: rename path attribute to "posix-mq"

This change brings the interface in line with other interfaces which
also use paths.

Signed-off-by: Isaac True <isaac.true@canonical.com>

* Revert "interfaces: posix-mq: rename path attribute to "posix-mq""

This reverts commit 47b9e5f72a84b085784c6e21eeadf4adb26978b5.

* interfaces: posix-mq: add "posix-mq" label attribute

This adds an additional attribute called "posix-mq" which can be used to
help identify which plugs should connect to which slots, similar to the
`shared-memory` interface.

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: validate posix-mq attribute in BeforePreparePlug

Additionally add more unit tests to validate posix-mq label handling.

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: improve unit tests by checking for explicit errors

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: various code cleanups to improve readability

Signed-off-by: Isaac True <isaac.true@canonical.com>

* interfaces: posix-mq: fix unit test not being run

- Additionally check for an explicit error

Signed-off-by: Isaac True <isaac.true@canonical.com>

Co-authored-by: Michael Vogt <mvo@ubuntu.com>
2022-05-06 08:07:22 +02:00
James Henstridge
eaad8a23df interfaces: add a steam-support interface (#11708)
This interface is intended to provide some additional permissions needed by the steam snap.

At present, this is primarily AppArmor and seccomp rules to allow Steam to launch pressure-vessel containers, which it uses to provide a consistent runtime environment to some games (at the moment mainly Windows games it runs under Proton/Wine). PV is based on Bubblewrap, as used by Flatpak and various other process sandboxes on GNOME systems.

Related to getting Steam games to run, I've added the futex_waitv syscall to the base template. Although the Ubuntu kernels don't yet support this syscall, we want to let Proton try to call it so it will fall back to the old futex API. As this has essentially the same security concerns as the existing futex syscalls, it seemed sensible to add it to the base template rather than the steam-support interface.

snap-seccomp knows about this syscall as of 15th April, when PR #11674 was merged.

* interfaces: add a steam-support interface with permissions needed to set up pressure-vessel containers

* interfaces/seccomp: add futex_waitv to the base template

This is a new syscall used to wait on multiple futexes at once, and
Wine/Proton will attempt to use it if the kernel supports it. Blocking
access prevents it from falling back to the other futex related
syscalls.

* tests: add steam-support to policy snap

* interfaces: limit proc access to same owner in steam interface

* interfaces: lock down the remount AppArmor rules for steam-support

* interfaces: allow pressure-vessel to mount tmpfs to mask certain directories

* interfaces/policy: add base declaration tests for steam-support
2022-04-29 20:29:10 +02:00
Alfonso Sánchez-Beato
9379cf376f i/seccomp/template.go: add close_range to the allowed syscalls
close_range was added to kernel 5.9. For snap-seccomp to actually use
this information, libseccomp 2.5.2 or later is needed.
2022-03-11 12:02:14 +01:00
Ian Johnson
f3f669d720 Merge remote-tracking branch 'mvo5-private/release-2.54.3'
Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
2022-02-17 11:13:00 -06:00
Ian Johnson
0947b3d51d interfaces, o/ifacestate: generate profile transition rules for devmode snaps
These additional accesses allow a snap that is in devmode confinement to
execute other snaps via `snap run ...` while snap-confine still ends up being
confined with a strict apparmor profile. Previously what would happen is that
snap-confine when executed from a devmode snap would only be in complain mode,
which is the same as the devmode confinement of the calling snap. This is not
ideal because it exposes snap-confine to attacks that may lead to privilege
escalation because unprivileged users can execute commands from inside a
devmode snap.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>
2022-02-09 18:08:46 +01:00
Alex Murray
80c54bc84a interfaces/seccomp: Add rseq to base seccomp template
glibc 2.35 calls rseq() for all threads so allow this by default

Signed-off-by: Alex Murray <alex.murray@canonical.com>
2022-02-08 15:36:15 +10:30
Maciej Borzecki
999c2e61f0 interfaces/seccomp: add clone3 to default template
Recent combinations of Go 1.17, glibc 2.34 and Linux 5.14 ended up triggering
pthread_create() code paths that try to use clone3() syscall when executing
snap-exec. Since snap-exec runs under the seccomp profile of the application,
make sure that clone3 is allowed in the default template. Also, applications may
trigger this code path themselves anyway.

The strace output when this fails looks like this:

mprotect(0x7f4ad3ea2000, 8388608, PROT_READ|PROT_WRITE) = 0
rt_sigprocmask(SIG_BLOCK, ~[], ~[KILL STOP RTMIN RT_1], 8) = 0
syscall_435(0x7ffc466b4c60, 0x58, 0x58b300, 0x8, 0x7f4ad46a1640, 0x7ffc466b4d4f) = -1 (errno 1)
rt_sigprocmask(SIG_SETMASK, ~[KILL STOP RTMIN RT_1], NULL, 8) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
write(2, "runtime/cgo: ", 13runtime/cgo: )           = 13
write(2, "pthread_create failed: Operation not permitted", 46pthread_create
failed: Operation not permitted) = 46

Where syscall 435 is also known as clone3:

$ scmp_sys_resolver 435
clone3

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
2021-09-27 12:00:53 +02:00
Tsunghan Liu (Robert Liu)
925f1a779c interface/builtin: add qualcomm-ipc-router interface for AF_QIPCRTR socket protocol
* interface/seccomp: add socket AF_QIPCRTR

AF_QIPCRTR (Qualcomm IPC router protocol) is used to communicate
with services provided by other hardware blocks in the system.

Snaps to access some Qualcomm hardware components need this protocol.

Signed-off-by: Robert Liu <robert.liu@canonical.com>

* snap-seccomp: add AF_QIPCRTR and PF_QIPCRTR

Signed-off-by: Robert Liu <robert.liu@canonical.com>

* interfaces/builtin: add qrtr

Signed-off-by: Robert Liu <robert.liu@canonical.com>

* interfaces/builtin/qrtr: limit type to sock_dgram only

Signed-off-by: Robert Liu <robert.liu@canonical.com>

* interfaces/builtin/qualcomm-ipc-router: rename from qrtr and add more details

Signed-off-by: Robert Liu <robert.liu@canonical.com>

* interfaces/builtin/qualcomm-ipc-router: update tests

Signed-off-by: Robert Liu <robert.liu@canonical.com>

* sandbox/apparmor: support checking for network qipcrtr dgram parser feature

This is not a required or even preferred feature at this time, it will just be
used by one specific interface for checking. Eventually it should become a
proper feature that is queried / included in the system-key perhaps, etc. but
the rest of the machinery for this is not available yet.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* interfaces/qualcomm-ipc-router: only perform the conn if the parser supports it

If the apparmor_parser on the system doesn't support the qipcrtr-socket
feature, then we shouldn't proceed with the connection of the apparmor plug.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* interfaces/apparmor: expose the apparmor sandbox features through Specification

This allows interfaces to specialize their policy or behavior based on what
features are available in both the parser and the kernel.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* interfaces/qualcomm-ipc-router: adjust implementation to use spec.Features()

This is the better way where the individual interface doesn't need to import
the sandbox directly and can instead get the features from the specification.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* cmd/snap-seccomp: address gofmt for 1.13

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* tests/interfaces-many-core-provided: check on xenial, qualcomm-ipc-router fails

This interface does not work on xenial, so we should get an error message
trying to connect it.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* interfaces: rename MockSetFeatures -> MockFeatures

Thanks to Samuele for the suggestion.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* interfaces/qualcomm-ipc-router: drop redundant dgram from rule

Thanks to Alex for pointing this out.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* snap-seccomp: import "github.com/mvo5/libseccomp-golang" without the "seccomp" prefix to avoid breaking the debian-sid patch

* tests: fix skip on 16.04 for qualcomm-ipc-router

* interfaces/repo: add comment about issue with AppArmorConnectedPlug failures

Explain a potential issue we are running into with the current state of the
qualcomm-ipc-router interface.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* interfaces/qualcomm-ipc-router: switch to BeforePreparePlug based impl

Use BeforePreparePlug instead of AppArmorConnectedPlug since
AppArmorConnectedPlug returning non-nil error leads to an inability to process
other connection changes for that snap until snapd is restarted.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* Revert "interfaces/apparmor: expose the apparmor sandbox features through Specification"

This reverts commit bff6b6b2b5c62349e2605c199241c97a61ba6cb3.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* interfaces/qualcomm-ipc-router: switch to using BeforeConnectPlug

BeforePreparePlug is actually run just when a plug is declared, not necessarily
when the plug is going to be connected. For qualcomm-ipc-router, we want to
reject the connection, not necessarily the plug by itself.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* interfaces/builtin/qualcomm-ipc-router: fix method args to match interface

Also need to adjust the new interfaces.BeforeConnectPlug helper which tests
this as it was using the wrong type as well.

Thanks to Samuele for finding this.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

* tests/main/interfaces-many-core-provided: fix if check for xenial to add UC16

Xenial and Ubuntu Core 16 suffer from the same problem so they both need to be
considered in this check.

Signed-off-by: Ian Johnson <ian.johnson@canonical.com>

Co-authored-by: Ian Johnson <ian.johnson@canonical.com>
Co-authored-by: Michael Vogt <mvo@ubuntu.com>
2021-09-03 21:22:14 +02:00
Alberto Mardegan
9826edb4d9 interfaces: avoid unnecessary for loops
We can use the `...` syntax to append an entire array.
2021-05-12 10:32:24 +03:00