interfaces: conditionally deny /proc/self/mountinfo - #16982
Conversation
|
Thu May 7 14:09:31 UTC 2026 Failures:Preparing:
Executing:
Restoring:
Skipped tests from snapd-testing-skipIf you wish to have any of the below tests run in your PR, in your PR description, add 'unskip:' followed by a copy-and-pasted list (without variants) of the below tests you wish to run (unskip plus test list must be valid yaml)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #16982 +/- ##
==========================================
- Coverage 79.06% 79.05% -0.01%
==========================================
Files 1374 1377 +3
Lines 191076 191195 +119
Branches 2465 2465
==========================================
+ Hits 151065 151157 +92
- Misses 30918 30931 +13
- Partials 9093 9107 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pedronis
left a comment
There was a problem hiding this comment.
thank you, did a pass, some small comments and some naming questions
jslarraz
left a comment
There was a problem hiding this comment.
Overall looks good to me. I left a couple of suggestions
95ce6c7 to
fd0b25d
Compare
Extends the concept of prioritized snippets to have allow base prioritized snippets not specific to any security tag that can be added to a base template and are removed if any prioritized snippet of the same key is added by a plug. This is used to add a deny rule for /proc/self/mountinfo which the Go runtime from 1.25+ tries to read, resulting in logged denials for both Go snaps and snap-exec. This rule is removed if any interface providing read access to this path is plugged. https://warthogs.atlassian.net/browse/SNAPDENG-35488 https://warthogs.atlassian.net/browse/SNAPDENG-36466 Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
fd0b25d to
c0c68bf
Compare
|
This broke my ability to run docker containers with
|
Please see https://forum.snapcraft.io/t/snapd-2-76-mountinfo-regression-affecting-steam-docker-snaps/52137 for further discussion around this issue and proposed mitigations. |
* i/builtin: fix mountinfo denial for steam/docker interfaces #16982 added a default denial for /proc/self/mountinfo due to recent changes to the Go runtime. It missed adding overrides for the docker and steam support interfaces since those granted more broad accesses (e.g., /proc/** or allow all). Signed-off-by: Miguel Pires <miguel.pires@canonical.com> * i/builtin: make tests more broad Signed-off-by: Miguel Pires <miguel.pires@canonical.com> --------- Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
…l#17321) * i/builtin: fix mountinfo denial for steam/docker interfaces canonical#16982 added a default denial for /proc/self/mountinfo due to recent changes to the Go runtime. It missed adding overrides for the docker and steam support interfaces since those granted more broad accesses (e.g., /proc/** or allow all). Signed-off-by: Miguel Pires <miguel.pires@canonical.com> * i/builtin: make tests more broad Signed-off-by: Miguel Pires <miguel.pires@canonical.com> --------- Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
* i/snap-confine: harden seccomp template (#27)
* cmd/snap-seccomp: recognize O_TMPFILE
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* cmd/snap-seccomp: recognize S_ISUID and S_ISGID
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* cmd/snap-seccomp: recognize O_CREAT
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* interfaces/seccomp: deny chmod with setuid/setgid bits set
This has been a long standing TODO that was fixable since the
introduction of the argument filtering in the seccomp compiler.
Tests check that chmod g+s and chmod u+s are rejected
Check that we cannot chmod a file to have setuid or setgid bits.
Check that we can still set the sticky bit.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* interfaces/seccomp: deny open with setuid/setgid mode
Similarly to chmod, fchmod and fchmodat, open, openat and openat2 can be
used to create setuid and setgid executables. The patch contains several
elements:
- Deny open and openat with both O_CREAT bit in flags and set-uid bit in
mode from being used.
- Deny open and openat with both O_CREAT bit in flags and set-gid bit in
mode from being used.
- Deny open and openat with both O_TMPFILE bit in flags and set-uid bit
in mode from being used.
- Deny open and openat with both O_TMPFILE bit in flags and set-gid bit
in mode from being used.
- Deny openat2 from being used (see below).
In openat2 seccomp is insufficient to inspect arguments, so until we use
an eBPF LSM or seccomp user-space notification with hand-off, we cannot
allow using that system call.
That last bit is technically a no-operation, as the seccomp filter does
not allow unknown system calls. It's left as a documentation in case
someone adds a "missing syscall" and not look deeper.
Tests now check that open g+s and open u+s are rejected.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* interfaces/seccomp: deny creat with setuid/setgid mode
Similarly to chmod and open family of system calls, the creat[e] system
call may be used to create setuid and setgid executables. The patch
contains two elements:
- Deny creat system call with set-uid bit in mode from being used.
- Deny creat system call with set-gid bit in mode from being used.
Tests now check that creat g+s and open u+s are rejected
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* interfaces/seccomp: deny mknod with setuid/setgid mode
In the common theme restrict creation of setuid/setgid files via mknod.
Tests now check that mknod g+s and mknod u+s are rejected
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* interfaces/seccomp: use symbolic S_IS{U,G}ID instead literals
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* interfaces/seccomp: use symbolic O_CREAT instead of literals
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
---------
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* interfaces/apparmor: don't grant access to systemd userdb API sockets
These sockets provide access to the varlink-based API of systemd-userdbd which
in turn exposes hashed passwords and other details of the current user account
to any snap running as that user, as it doesn't distinguish between sandboxed
applications compared to traditional applications running as the user. In turn,
if the snap runs a root daemon it can then access the account details (again
including hashed passwords) of any user on the system.
Note, the impact of this issue is limited since systemd-userdbd is not installed
by default in Ubuntu or other distributions that I am aware of and so in this
case these varlink sockets are not active and no information is exposed.
This commit fixes LP: #2067006 by removing this access entirely - note this
issue was assigned CVE-2024-5300.
Signed-off-by: Alex Murray <alex.murray@canonical.com>
* cmd/snap-confine: use O_NOFOLLOW when creating replica of base rootfs
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* cmd/snap-confine: move temporary rootfs to private tmp directory
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* sandbox/apparmor: update homedirs snippet to use private tmp path
This is a follow-up after the change in snap-confine earlier.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* dirs: add SnapPrivateTmpDir variable
Add a new dirs variable for the snap private temporary directory
(/tmp/snap-private-tmp), consistent with how other paths are managed in the
dirs package. This allows tests to mock the path without touching the real
/tmp.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* overlord: create /tmp/snap-private-tmp on ifacemgr startup
During interface manager startup (when profilesNeedRegeneration is called).
This ensures snapd creates the directory on each startup, which is relevant
to snapd.snap re-exec feature.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* cmd/snap: wake up snapd if /tmp/snap-private-tmp is missing
Update maybeCheckSystemKeyMismatch if /tmp/snap-private-tmp is missing.
This ensures that snapd creates this directory, as root, even if it was
missing for whatever reason.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* packaging: pre-create /tmp/snap-private-tmp for snap-confine
Commit 84987ca798 (cmd/snap-confine: move temporary rootfs to private tmp
directory) changed snap-confine to use
/tmp/snap-private-tmp/snap.rootfs_XXXXXX instead of
/tmp/snap.rootfs_XXXXXX. The parent /tmp/snap-private-tmp directory must
already exist with root:root 700 permissions before snap-confine can
bootstrap the mount namespace.
In all packaging maintenance scripts (postinst/%post) for every packaging
directory (debian-sid, ubuntu-16.04, ubuntu-26.04, fedora, opensuse, arch).
This ensures the directory exists immediately after package
install/upgrade, before snapd first starts.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* snap-confine: rely on tmpfiles and packaging scripts to create /tmp/snap-private-tmp
With the changes introduced in this branch (compared to release/2.76),
snap-confine no longer needs to create /tmp/snap-private-tmp itself. The
directory is now expected to be created by packaging scripts, systemd-tmpfiles,
or snapd.
Remove the sc_ensure_mkdir() call and the associated race condition
handling code (fchown/fchmod recovery) that was needed when snap-confine
created the directory with 0000 permissions and another parallel instance
could observe the intermediate state.
The open() and fstat() calls are kept to verify the directory exists and
is a directory, failing loudly if none of the above sources created it.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* snap-confine: tighten AppArmor perms for /tmp/snap-private-tmp/
The old sc_ensure_mkdir() call needed mkdirat() on /tmp/snap-private-tmp
itself, which required w permission. Since that call was removed, the
AppArmor rule can be downgraded from rw to r.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* cmd/snap-confine: check /tmp/snap-private-tmp ownership and permissions
Add a sanity check in setup_private_tmp() that verifies
/tmp/snap-private-tmp is owned by root:root with mode 0700 before
proceeding. This prevents snap execution when the directory has been
tampered with or misconfigured.
Extend the snap-confine-tmp-mount spread test to cover both error
scenarios: wrong group ownership (root:users) and wrong permissions (0755).
Both should cause snap-confine to bail out with an 'unexpected ownership'
error. The existing happy-path test remains at the end.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* security release: 2.76.1
* .github/workflows: fix permissions
* github: remove reference to repo when accessing PR data
* release: 2.76.1 (try 2)
* t/l/s/s/test-snapd-kernel-module-consumer: update test-snapd-kernel-module-consumer snap to use kmod binary for kernel modules testing (#17122)
* t/l/s/s/test-snapd-kernel-module-consumer: update test-snapd-kernel-module-consumer snap to use kmod binary for kernel modules testing
* t/l/s/s/test-snapd-kernel-module-consumer: remove manually hard linking of lsmod, insmod, rmmod to kmod
* t/l/s/s/test-snapd-kernel-module-consumer: globally define plug
* t/l/s/s/test-snapd-kernel-module-consumer: change executable path for lsmod, rmmod, insmod
* t/l/s/stest-snapd-kernel-module-consumer: fix to pack with stable snapcraft version
* tests: stabilize rpm package tracking in fedora/opensuse spread cleanup (#17136)
* tests: fix interfaces-kernel-module-control on core 18 (#17155)
* tests: fix interfaces-kernel-module-control on core 18
* tests: add comment explaining the use of the revision
* tests: update main/nvidia-files (#17154)
* tests: update main/nvidia-files
* tests: revert removing broken driver condition
* tests: add 470 as transitional on jammy
* github: use fedora 44 support commit for image garden in CI (#17175)
* github: use fedora 44 support commit for image garden in CI
* github: use --revision for checkout out fedora 44 support
* tests: fix security-logging test in opensuse (#17170)
* tests: fix security-logging test in opensuse
As auditd holds the netlink audit socket exclusively; if it is running,
journald cannot receive audit messages. The solution propossed is to
check audit messages in the audit logs instead.
* minor improvement using retry
* fix shellcheck
* move python code to .py file
* If auditctl is available, use it to enable the audit subsystem.
* pin the audit log too
* tests: migrate fips and secboot from google to openstack (#17185)
* tests: migrate fips and secboot from google to openstack
This change is migrating the secboot and fips tests from google to
openstack.
* fips repo added while image was created
* tests/nested/manual/cmdline-option: use nesting instead of dot notation (#17188)
* tests/lib/prepare: only inject core specific snapd.service addons when building for UC (#17192)
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* tests/core/auto-refresh-backoff-after-reboot: ensure fakestore is up before snapd after reboot (#17200)
* tests/release/distro-upgrade: if final upgrade unavailable, don't fail test (#17176)
* tests/release/distro-upgrade: if final upgrade unavailable, don't fail test
* tests/release/distro-upgrade: update description
* t/m/interfaces-kernel-module-control: enable interfaces-kernel-module-control spread test for systems with compressed kernel module object files (#17123)
* t/m/interfaces-kernel-module-control: enable interfaces-kernel-module-control spread test for core24 and 26
* t/m/interfaces-kernel-module-control: improve path selection
* t/m/interfaces-kernel-module-control: use file to store the path to the module
* t/m/interfaces-kernel-module-control: store uncompressed module object file path in an environment variable
* tests: move trusty to openstack and remove alternative-backend from workflows (#17226)
* tests: move trusty to openstack and remove alternative-backend from workflows
As google cannot be reached from ps7 runners, the alternative-backend
doesn't make sense any more.
This change removes the alternative-backend from the gh workflows and
migrates ubuntu trusty from google to openstack.
There is a new image ready for ubuntu trusty.
* remove curl to google bucket
* tests: add back printer-driver-cups-pdf as a dependency (#17235)
* tests: adjust dd during reflash to write faster to VMs (#17250)
* tests: cleanup redundant tests.cleanup restore (#17252)
* tests/core/update-snapd-symlink: also run on UC24 and UC26 (#17290)
Enable the test on UC24 and UC26
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* tests/main/lxd: do not refresh it to latest/stable anymore as the 4.0 can be removed fine. (#17299)
* tests/main/lxd: do not refresh it to latest/stable anymore as the 4.0 can be removed fine.
* tests/main/lxd-postrm-purge: do not refresh lxd either
* i/builtin: fix mountinfo denial for steam/docker interfaces (#17321)
* i/builtin: fix mountinfo denial for steam/docker interfaces
#16982 added a default denial for
/proc/self/mountinfo due to recent changes to the Go runtime. It missed
adding overrides for the docker and steam support interfaces since those
granted more broad accesses (e.g., /proc/** or allow all).
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
* i/builtin: make tests more broad
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
---------
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
* release: 2.76.2
* secboot: update to rev 457b03a - permit missing hw rot
* Use new secboot API for reprovision TPM (picked from PR17033)
* many: support keyboard configuration at install-time for first-boot (#17207)
On TPM-FDE Ubuntu systems where a passphrase/PIN is configured during installation,
the user must enter credentials at the plymouth prompt on first-boot - before snapd
has had a chance to detect the runtime keyboard layout. This commit extends the
"/v2/systems/{system-label}" install API so that the installer can pass the selected
keyboard layout through to first boot (written as kernel command line arguments),
ensuring plymouth detects the correct XKB configuration even on first-boot.
The mechanism for passing extra snapd kcmdline args from install-time is generic
but only currently used to pass xkb configs. The install-time extra kcmdline args
are persisted to a file under ubuntu-save to be used for initialization by snapd
on first-boot.
Relevant spec: SD272
---------
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* o/install: re-enable passphrases/PINs at install-time (#17219)
* o/install: re-enable passphrases/PINs at install-time
Now that proper keyboard configuration can be setup for
first-boot, this commits re-enables setting a PIN or
passphrase at install-time.
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* tests/lib/muinstaller: support setting pin and keyboard-config
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* tests: check support for setting PIN or passphrase at install-time
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* tests: check support for setting keyboard configuration at install-time
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
---------
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* many: require volumes authentication if HWROT is missing (#17289)
As a security requirement, volumes-auth is now mandatory if
hardware root of trust is missing.
This information is relayed through a new "requirements" field
under "GET /v2/systems/<system-label>" with an array of
requirements (e.g. ["volumes-auth"]).
This is also enforced on the "setup-storage-encryption" step
of the "install" action, where snapd will fail if volumes-auth
is not passed when it is required due to missing HWROT.
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
* data/selinux: use init_named_socket_activation() for allwoing systemd to start snapd through socket activation (#17206)
Since we've dropped RHEL7, we can update the policy to use
init_named_socket_activation() macro rather than our workarounds.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* packaging/debian-sid: make sure that usr/bin/snap is built with correct build tags (#17314)
Make sure that usr/bin/snap is built with correct build tags.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* o/ifacestate, tests: ensure profiles are setup before running prepare-{slot, plug}* hooks (#17323)
* tests: add spread test for prepare-slot-related failure
* o/ifacestate: ensure profiles are setup before running prepare-slot* hooks
* o/ifacestate: update unit tests
* release: 2.76.3
---------
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Alex Murray <alex.murray@canonical.com>
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
Co-authored-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Co-authored-by: Alex Murray <alex.murray@canonical.com>
Co-authored-by: ernestl <ernest.lotter@canonical.com>
Co-authored-by: Nathnael Bekele <135308096+natibek@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Sergio Cazzolato <sergio.cazzolato@canonical.com>
Co-authored-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Co-authored-by: Meulengracht <the_meulengracht@hotmail.com>
Co-authored-by: Miguel Pires <miguel.pires@canonical.com>
Co-authored-by: Zeyad Yasser <zeyad.gouda@canonical.com>
* i/builtin: fix mountinfo denial for steam/docker interfaces #16982 added a default denial for /proc/self/mountinfo due to recent changes to the Go runtime. It missed adding overrides for the docker and steam support interfaces since those granted more broad accesses (e.g., /proc/** or allow all). Signed-off-by: Miguel Pires <miguel.pires@canonical.com> * i/builtin: make tests more broad Signed-off-by: Miguel Pires <miguel.pires@canonical.com> --------- Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
Extends the concept of prioritized snippets to have allow base prioritized snippets not specific to any security tag that can be added to a base template and are removed if any prioritized snippet of the same key is added by a plug.
This is used to add a deny rule for /proc/self/mountinfo which the Go runtime from 1.25+ tries to read, resulting in logged denials for both Go snaps and snap-exec. This rule is removed if any interface providing read access to this path is plugged.
https://warthogs.atlassian.net/browse/SNAPDENG-35488
https://warthogs.atlassian.net/browse/SNAPDENG-36466