* cmd/libsnap-confine-private: helper for detecting if executing inside a container
Add a helper which attempts to detect if the current process is executing inside
a container environment. Specifically, look for /run/systemd/container and check
whether it is non empty.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: do not setup device cgroup if running inside a container
Do not set up a device cgroup filter, if we're running inside the container. The
rationale is that the container environment has already shut down device access
sufficiently, and especially if running in unprivileged container, we may not be
able to set it up correctly anyway.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: allow reading of /run/systemd/container
Allow snap-confine to read /run/system/container to implement container
execution check.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: use strnlen for sc_is_container
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
---------
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Co-authored-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* cmd/snap: record snap-run-inhibit notice
Record a snap-run-inhibit notice when snap run is inhibited due refresh.
* cmd/snap: remove old desktop notifications (thanks @pedronis @zyga)
* cmd/snap: always send notices when snap run is inhibited
+ fallback to text if no snap has the marker snap-refresh-observe
interface connected and a terminal is detected.
* cmd/snap: send text fallback notification to stderr (thanks @bboozzoo)
* cmd/snap: initialize inhibition flow only when it is needed
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* tests/main: add test for snap run inhibition flows
* test/main/snap-run-inhibition-flow: remove text fallback check
Text fallback is inconsistent across systems due to the terminal
checks in snapd. It is hard to mock a real terminal in all systems
while redirecting output to a file for testing.
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
---------
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* many: add support for user daemons in "snapctl services" by introducing the same --user/--global switches as "snap services"
* o/hookstate/ctlcmd: remove error messages
* cmd/snap,o/hookstate/ctlcmd: correct some doc strings
* daemon,cmd/snap: support for user services in snap services
* NEWS: update news to reflect this functionality
* cmd/snap: add missing unit tests
* many: use interface instead for StatusDecorator to allow for unit testing
* daemon: fix a static check for a range loop where a variable could be omitted
* daemon,cmd/snap: support user-service status of the root user with a --user switch
* t/main/services-user: add a case for root user
* t/main/services-user: fix wrong filename
* cmd/snap: fix TestAppStatus unit test failing
* cmd/snap: extend help for "snap services" to describe the new --global and --user switches
remove errors on redundant switches, remove unneeded argument, move validation of arguments closer to entry of Execute
* cmd/snap: refer directly to fields in the help docs
* 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>
The apparmor profile can use 'kill' enforcement mode, in which case the the
process execution an action it has no permissions for will be sent SIGKILL. We
need to account for this mode explicitly, otherwise s-c will complain about
running with elevated permissions and exit with an error, which makes not sense
since the process is still properly confined.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* snap: return NotFoundError when current symlink is missing in ReadCurrentInfo
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* cmd/snap: use updated "current" revision after snap refresh run inhibition
* cmd/snap: retry snap-run when race condition is detected
* cmd/snap: remove commented out test (thanks @olivercalder)
* cmd/snap: add doc comments for {w,maybeW}aitWhileInhibited (thanks @olivercalder)
* cmd/snap: add better comments and debug logs (thanks @bboozzoo)
* cmd/snap: explain why we cannot rely on O_CLOEXEC (thanks @zyga)
* cmd/snap: simplify snap refresh conflict detection
Only check that if we start without a hint lock file and after creating
the tracking cgroup it exists then it means that a refresh was started
for the snap.
* cmd/snap: retry on failure due to missing current symlink (thanks @pedronis)
We could have started without a hint lock file and then we have
an ongoing refresh which removed current symlink.
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* tests/main/snap-run-symlink-error: fix error matching
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
---------
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
go test -race complains about data race, which isn't entirely accurate as the
goroutine which may modify the data has exited by the time checks are done.
However, should we want to enable -race as part of the test suite, this fixes a
blocker.
```
==================
WARNING: DATA RACE
Read at 0x00c0000123a8 by goroutine 16:
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch_test.(*triggerwatchSuite).TestNoDevsWaitKeyTimeout()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch_test.go:118 +0x3e5
runtime.call16()
/usr/lib/go/src/runtime/asm_amd64.s:770 +0x42
reflect.Value.Call()
/usr/lib/go/src/reflect/value.go:380 +0xb5
gopkg.in/check%2ev1.(*suiteRunner).forkTest.func1()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:775 +0x9c5
gopkg.in/check%2ev1.(*suiteRunner).forkCall.func1()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:669 +0xe9
Previous write at 0x00c0000123a8 by goroutine 18:
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch_test.(*mockTriggerDevice).WaitForTrigger()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch_test.go:48 +0x44
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch.Wait.gowrap2()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch.go:112 +0x50
Goroutine 16 (running) created at:
gopkg.in/check%2ev1.(*suiteRunner).forkCall()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:666 +0x5ba
gopkg.in/check%2ev1.(*suiteRunner).forkTest()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:757 +0x155
gopkg.in/check%2ev1.(*suiteRunner).runTest()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:812 +0x419
gopkg.in/check%2ev1.(*suiteRunner).run()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:618 +0x3c6
gopkg.in/check%2ev1.Run()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/run.go:92 +0x44
gopkg.in/check%2ev1.RunAll()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/run.go:84 +0x124
gopkg.in/check%2ev1.TestingT()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/run.go:72 +0x5d3
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch_test.Test()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch_test.go:35 +0x26
testing.tRunner()
/usr/lib/go/src/testing/testing.go:1689 +0x21e
testing.(*T).Run.gowrap1()
/usr/lib/go/src/testing/testing.go:1742 +0x44
Goroutine 18 (finished) created at:
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch.Wait()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch.go:112 +0x94b
github.com/snapcore/snapd/cmd/snap-bootstrap/triggerwatch_test.(*triggerwatchSuite).TestNoDevsWaitKeyTimeout()
/home/maciek/work/canonical/snapd/cmd/snap-bootstrap/triggerwatch/triggerwatch_test.go:115 +0x21b
runtime.call16()
/usr/lib/go/src/runtime/asm_amd64.s:770 +0x42
reflect.Value.Call()
/usr/lib/go/src/reflect/value.go:380 +0xb5
gopkg.in/check%2ev1.(*suiteRunner).forkTest.func1()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:775 +0x9c5
gopkg.in/check%2ev1.(*suiteRunner).forkCall.func1()
/home/maciek/work/canonical/snapd/vendor/gopkg.in/check.v1/check.go:669 +0xe9
==================
```
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Since we closed the server immediately, the port was being re-used by
another test. This caused this test to send its HTTP request to another
test that wasn't expecting it.
Ahead of introduction of CIFS workaround, generalize the names so that we use
more general language rather instead of focusing on NFS.
As a special exception the externally visible wording related to NFS is kept
intact in two places:
- The apparmor "nfs-support" file name
- The system key "nfs-home" key.
From points of view this is all an elaborate internal rename that should nto be
observable outside of snapd, apart from log messages that may no longer speak of
NFS but of remote file systems.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* interfaces/udev: add non-strict flag to snap cgroup device file
Add a non-strict=true flag to the snap's cgroup device file, to inform
snap-confine that the snap was indeed installed in a non-strict confinement mode
(eg. devmode, or classic). This supplements an earlier mechanism in which snapd
would not generate any rules tagging devices for a specific snap and can be used
as an explicit indicator to avoid mandatory device cgroup even when using bare
or core24 and later bases (as well as custom base snaps).
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: account for non-strict confinement when setting up device cgroup
Snaps may be installed in a non-strict confinement mode. In which case, expect
an explicit non-strict=true in the per snap /var/lib/snapd/cgroup/snap.*.file.
This replaces an earlier mechanism of implicit non-strict confinement when no
devices are assigned to the snap.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* tests/main/security-device-cgroups-required-or-optional: update to check non-strict confinement
Update the test to check that --devmode results in a non-strict confinement
device cgroup setup.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
---------
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Coverity scan indicated an issue where we iterate over the characters of snap
instance name and use their values as loop boundary but we do not directly
validate if the input is of reasonable length. Coverity tagged it as
https://cwe.mitre.org/data/definitions/606.html
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Coverity scan raised a possible TOCTOU issue with how we check SC_HOSTFS_DIR.
Specifically we did stat() followed by mkdir(). The issue wasn't grave as
mkdir() was set to fail on any error (that includes EEXIST). However, to ensure
a peace of mind, reverse the order of operation, such that we mkdir() first and
then verify ownership if a directory was indeed created. This was tagged as
https://cwe.mitre.org/data/definitions/367.html
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Since snapd carries go.mod there is no point in trying to fiddle with GOPATH and
simply let the Go toolchain figure things out.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
When using apparmor_parser from the host, pin the supported policy to ABI 3.0
if one is present on the host. This way even on distributions with Apparmor
4.0, we get the behavior of 3.0 and the meaning of our apparmor profiles stays
the same. At the same time apparmor 2.0 distributions remain unchanged.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* client,cmd/snap: introduce --user, --system and --users switches for snap service operations
* client,o/servicestate: move Scope/UserSelection to client for reuse in client
* client,cmd/snap: improve handling of user and scope args
* NEWS: update news to reflect that we now support user daemons in start/stop/restart
* cmd/snap: some review feedback on allowed input
* t/main/services-user: add additional user to verify services are correctly affected
* cmd/snap: do not allow --system --user together, do not allow --users with =all
* tests,cmd: use --users=all in test, dont mark --users optional, enforce a value for it, add case for --system --users=all in spread test
* cmd/snap: add a comment for unreachable code, and correct a couple of messages
This commit updates snapd's useage of strace to use the new
--gid/--uid cmdline options instead of -u to work around an
issue that causes issues if strace is statically linked (due
to libnss).
Signed-off-by: Tony Espy <espy@canonical.com>
* dirs: add directory location for storing cgroup policy related flags
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* interfaces/udev: introduce cgroup policy flag for self managed device cgroup
Some snaps, due to their interfaces, are allowed to self manage the device
cgroup. In this case, the assumption was to not emit any rules at all, and
instead rely on the implicit behavior that no rules means no matching devices
and hence no device cgroup filtering. However, with introduction of a device
cgroup by default for all snaps on core24 onward, regardless of any assigned
devices, we need a separate source of information to indicate that a snap can do
self management.
The patch introduces a policy flags under /var/lib/snapd/cgroup, named
snap.<name>.device, eg.
/var/lib/snapd/cgroup/snap.docker.device, which provides a hints for
snap-confine to not set up a device cgroup filtering for apps.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: support snaps which self-manage device cgroup
Support for snaps for which policy explicitly states that the device cgroup is
self-managed. The typical use case is container like technologies. In such
scenario, there will be a device cgroup configuration file at a known location
which got generated by snapd whenever the relevant interface state changed.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* tests/main/security-device-cgroups-self-manage: spread test
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: drop base from bases exempt from mandatory device cgroup
We have confirmed that there are no snaps which (ab)use system files and use
bare base to obtain access to devices. As such, the bare base can be dropped
form the list of bases exempt from mandatory device cgroup.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* interfaces/udev: remove snap devices file when removing the snap
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* interfaces/udev: consistent use of fs.ErrNotExist
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: leave comments
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* interfaces/udev: tweak return path
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* interfaces/udev: improve managed device cgroup unit tests, verify calls to udevadm
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* NEWS: leave a note about mandatory device cgroup
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* tests/main/security-device-cgroups-self-manage: tweak comments
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* interfaces/udev: always write the device file
Always write the device file which serves as a synchronization point between
snap-confine and the snapd udev backend.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/libsnap-confine-private: add helper for waiting for a file to show up
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: make cgroup device file mandatory
Make the per-snap /var/lib/snapd/cgroup/snap.*.device file mandatory, such that
it can be used as a synchronization point between snapd calling Setup() of
relevant security backends and the execution path in snap-confine.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-mgmt: do cleanup of /var/lib/snapd/cgroup
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* packaging: declare /var/lib/snapd/cgroup
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: use the file wait helper
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* packaging: create cgroup directory
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* tests/main/security-device-cgroups-self-manage: update file check
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* tests/main/security-device-cgroups-required-or-optional: update test to verify device file
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* interfaces/udev: refactor reloading
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* cmd/snap-confine: move device cgroup mode selection to a helper
Extract device cgroup mode selection into a helper function.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
---------
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
