Module loading generates event for devpath /module/<nameofmodule>.
Some modules and devices have the same KERNEL name. For example
rfkill. We need to ignore module insertions. Otherwise we get
error message when trying to run snap-device-helper.
* account_control: allow local user account management
This MR adds support for adding, removing and modifying the users.
It is required for Core Desktop.
* Remove peer entries
* Add peer=(label=unconfined)
It works the same.
* Add extra peer=(label=unconfined)
It looks like some changes in apparmor 3.1.4 cause issues with
the existing network-control rules. It appears the rules are
stricter now.
Thie commit updates the rules to match the new behavior, see
also https://bugs.launchpad.net/apparmor/+bug/2023025
Currently, any application that connects to the audio-playback or
the pulseaudio insterfaces, have write permissions to the pulse/
folder. This means that a snapped malicious application would be
able to delete the socket and put its own, being able to read the
audio of any other snapped application.
This patch removes those permissions.
* Remove unneeded rules
The "owner /run/pulse/native/ rwk" rule is already managed by a
similar previous one. Also, there is no need to allow to link the
pulse folder.
Tested both in Firefox and Telegram, and everything still works
fine.
We have a bugreport where even with an active gpio-control interface
the gpio devices cannot be accessed. The path in question is:
```
/sys/devices/platform/INT33FC:02/gpio/gpio346/direction
```
Hower we only allow:
```
/sys/class/gpio/gpio[0-9]*/{active_low,direction,value,edge} rw,
```
in our gpio-control policy.
To fix that issue this commit allows gpio prefixes that start
with /sys/devices/platform instead.
* interfaces: add comment about /sys/devices/platform/*/gpio/gpio[0-9]*/ in gpio-control
This allows bind for org.bluez.obex.* which is necessary for the
agents that are created as well as the send for the
org.bluez.AgentManager1 interface.
This change alphabetizes the supported kernel mount options, as well as
moves `zfs` after `xfs` in the list of default filesystems.
Filesystem-specific mount options are not alphabetized, and are instead
left in the order they are defined in their documentation or source
code in order to ease visual verification of their correctness.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: fix custom-device default udev kernel rules
The KERNEL value in udev rules must be the basename of the device path.
For devices for which there is not a matching kernel value specified in
the custom-device `udev-tagging` section, a default udev kernel rule is
generated. Previously, https://github.com/snapcore/snapd/pull/12734
(and prior) generated these default rules by using the complete device
path relative to `/dev/`. However, for device paths which are in
subdirectories of `/dev/`, this means that the kernel values were not
basenames, which violates the udev spec.
This commit changes this behavior to instead generate udev kernel rules
using the basename of each specified device.
Since ambiguity would arise if multiple devices had the same basename,
this change introduces a check to ensure that all the specified devices
have unique basenames.
Additionally, this commit introduces a check to ensure that all
specified kernel values in the `udev-tagging` section are basenames.
It is still the case that each specified kernel value must match one of
the specified devices.
There are currently problems with `vet` where it is claimed that several
of the `[]string` variables in `validateUDevDevicesUniqueBasenames()`
are unused. These variables are used in a several ways, so further
investigation is required as to why this is the case.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* prompting/storage: fixed missing variable assignment from append()
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* prompting/storage: fixed custom device duplicate basename error message
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: fixed custom-device unit tests introduced by commas in filepaths PR
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: adjusted custom-device comment for kernel not matching any devices
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: fixed unit test for when custom-device kernel does not match any device
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: moved custom-device unique basename check
This change moves the check for whether all specified devices have
unique basenames out of `validateUDevTaggingRule()` (which is called
once for each udev rule) into `BeforePrepareSlot()`, immediately after
the list of device paths is assembled and each path validated. Thus, it
is only called once, before any rule validation begins.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
---------
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/utils: allow commas in filepaths
Some device paths contain commas outside of groups (i.e. {a,b}) or
classes (i.e. [,.:;'"]). For example, `/dev/foo,bar` is a valid device
path which one might with to use with the custom-device interface.
Most filesystems allow commas in filepaths, as does apparmor:
https://gitlab.com/apparmor/apparmor/-/blob/master/parser/parser_regex.c#L340
Previously, createRegex() would throw an error if a comma was used
outside of a group or class. This commit removes that error and instead
treats commas outside of groups and classes as literal commas. The
accompanying tests are also adjusted to reflect this change.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/utils: added argument to allow commas in filepaths
Rather than allowing any caller of `NewPathPattern()` to successfully
validate paths containing commas, this change adds a boolean argument
which explicitly specifies whether commas should be allowed in the
filepath.
There are some risks involved with allowing commas in filepaths (see
discussion at https://github.com/snapcore/snapd/pull/12697), so it is
desirable to restrict when commas are allowed based on the caller. In
particular, superprivileged interfaces (such as `custom-device` and
`mount-control`) have valid needs for commas in filepaths, and users of
these interfaces are individually verified, so it is safe for them to
use `NewPathPattern()` with commas allowed. Other callers (particularly
unprivileged interfaces) should probably not allow commas.
I was unsure whether `overlord/hookstate/ctlcmd/mount.go` should call
`NewPathPattern()` with commas allowed or not, but since commas had
previously been disallowed and tests continue to pass with
`allowCommas=false`, then I decided to leave it as `false`.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/{builtin,utils}: added named variables for allowCommas
Also, switched `overlord/hookstate/ctlcmd/mount.go` to allow commas
(previously did not, but this should match what is allowed in
`interfaces/builtin/mount_control.go`.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/utils: added unit tests for commas in paths
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/utils: remove `QuoteMeta` when adding `","` to path regex
Since `,` is not a regex special character, the `QuoteMeta` call is
unnecessary.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/utils: renamed TestCommasInRegex to TestCreateRegexWithCommas
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* many: added unit tests for callers of NewPathPattern with allowCommas=true
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
---------
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Co-authored-by: Michael Vogt <mvo@ubuntu.com>
* interfaces/builtins: mount_control userspace options
Added both fs-independent mount options (ie. nofail, auto, defaults) and
fs-specific mount options. The latter are implemented on a per-FS
basis, where only options explicitly allowed by the filesystem type
specified in the mount attributes are allowed. If no filesystem type is
specified, then the allowed options are those of all the filesystems
listed in `defaultFSTypes`.
Mount options are treated as invalid if they fail to match any allowed
kernel mount options (as before), any fs-independent mount options, and
any fs-specific mount options corresponding to the specified filesystem
type.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: pass only kernel mount options to apparmor
Since only kernel mount options are validated by apparmor in production,
userspace and filesystem-specific mount options should not be included
in the apparmor profile.
This change filters the mount options to include only kernel mount
options when creating the option string to pass into apparmor.
Note: `BeforeConnectPlug()` is responsible for validating all options
beforehand, including userspace and fs-specific options, so
`AppArmorConnectedPlug()` performs no validation and merely filters the
option list as described above, ignoring userspace and fs-specific
options.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: refactored mount option error handling
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: added comment about kernel mount option filtering
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: improved mount option handling
This commit changes fs-specific option handling so that the option
validation fails if a mount option is _not_ one of:
1) a kernel mount option,
2) a userspace mount option, or
3) a fs-specific mount option which is compatible with every specified
filesystem for the given mount configuration.
Previously, mount options were allowed if they were found on the option
list for any of the specified filesystems. This was not correct, as
mount options should only be allowed if they are allowed for all the
possible filesystems for a given mount specification.
Additionally, this commit adjusts the corresponding unit tests to revert
to their original form those which existed before the userspace mount
options changes were added, and to explicitly add new tests which check
userspace mount options (`nofail` in particular) and filesystem-specific
mount options according to the rules described above.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: removed `nofail` from kernel mount options
The `nofail` mount option is a userspace option which is not passed to
the kernel, and thus should not be included in the apparmor mount rules.
Since previous commits related to userspace mount options introduced
mount option filtering to include only kernel mount options in the
apparmor mount rules (after all mount options are validated), it is
important that `nofail` not be included in the kernel mount option list.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: reverted functionfs optionsWithoutFsType workaround
This change removes unsupported kernel mount options from
`optionsWithoutFsType`. Previously, commit 1026f5cd7d added these
unsupported options as part of a change which allowed mount configs with
filesystem type `functionfs` to bypass kernel mount option validation,
and thus use any mount option, including those forbidden kernel options
which are affected by this change. This meant that if a mount config
with type `functionfs` used one of these mount options, such as `move`,
it would then be caught by the `optionIncompatibleWithFsType` check, as
a filesystem type (`functionfs`) was explicitly specified.
With more recent commits related to userspace mount options,
`functionfs` is now handled similarly to other filesystems: mount
options for `functionfs` are now validated against kernel, userspace,
and `functionfs`-specific mount options. Thus, the forbidden options
affected by this change will no longer make it past validation, even
with `functionfs` specified, and thus are removed from
optionsWithoutFsType` to increase clarity.
The test checking a mount with type `functionfs` and option
`make-private` has been adjusted according to the fact that
`make-private` is now rejected as unknown, similarly to any other
unsupported mount option, rather than rejected for a fstype being
specified.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: adjusted comments and naming for clarity
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: refactored fs mount option validation
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: fixed typo in mount-control
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: fixed aufs mount options which use : as separator
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: differentiate mount options with{,out} optargs
By splitting after the `=` in mount options such as `uid=1000`, we can
differentiate an allowed mount option which requires an optarg, such as
`uid=1000` for functionfs, from `uid`, which is not allowed. For
options with optional optargs, such as btrfs `compress`, by including
both `compress` and `compress=` in the list of allowed filesystem mount
options.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: generalize handling for filesystems with colon-separated mount options
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
---------
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Co-authored-by: Michael Vogt <mvo@ubuntu.com>
* interfaces/builtin: custom-device kernel in subdir
Devices may be in subdirectories of `/dev/`, rather than directly in
`/dev/` itself. This change allows matching the `kernel` attribute of
`udev-tagging` to the basename of a device path in `devices`, rather
than matching `/dev/${kernel}` to a device in `devices`. Snapd maps the
value of the `kernel` key to the string used as the `KERNEL==` udev
rule. To the best of my knowledge, this change makes custom-device udev
kernel validation match the behavior of the `KERNEL` udev rule.
Also adds an additional test which should fail on the `subsystem` key,
thus indicating that the `kernel` key was successfully validated.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: use filepath.Base instead of custom code
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: fixed udev kernel device matching
This change allows udev-tagging rules to have `kernel: deviceName` values
which match the end of device paths, rather than necessarily matching
either `"/dev/" + deviceName` or `deviceName` matching the basename of
the device.
Before, a basic udev `KERNEL==` rule was generated for each device and
then overwritten by the `kernel: deviceName` rules in the `udev-tagging`
section which matched `"/dev/" + deviceName` to the device path. Now,
the `kernel: deviceName` rules are converted to udev rules first, and
then any remaining devices which have not been matched by a rule are
given the basic `KERNEL==` udev rule.
If the number of devices are matched by the same `deviceName` from a
`kernel: deviceName` specification is not 1, then an error is thrown.
This is similar to previous handling of `deviceName` not matching any
devices, but now also includes checking that `deviceName` does not match
more than one device, as this is presumably not desired.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces/builtin: custom-device stricter udev kernel rules
This change restricts the `udev-tagging` `kernel` attribute matching
rules so that `kernel: deviceName` only matches a `devicePath` from the
device list of `"/dev/"+deviceName == devicePath` or `deviceName` is
equal to the basename of `devicePath`.
Previously, `deviceName` matched `devicePath` if `deviceName` was the
string suffix of `devicePath` (not suffix in the path sense). This
allowed, for example, `bar/name` to match `/dev/foo/bar/name`, where
now it would only match `/dev/bar/name`.
It is still the case that a `deviceName` might match more than one
device, in which case an error will be thrown. For example,
`kernel: name` would match every device in
`devices: [/dev/name, /dev/foo/name, /dev/bar/name]`, which would throw
an error, since only one match is allowed for a given kernel
`deviceName`.
Note: if `deviceName` contains a subdir (e.g. `foo/bar`), then it must
match a complete path relative to `/dev/`, since it will never match the
basename of a device.
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
* interfaces: make UDevConnectedPlug slightly more readable
---------
Signed-off-by: Oliver Calder <oliver.calder@canonical.com>
Co-authored-by: Michael Vogt <mvo@ubuntu.com>
