Make sure that snap refresh/install does not crash if a change has no
api data. This is not happening in usual interactions with the store,
but it does in some cases when we use the fakestore, which is breaking
some integration tests.
Add a debug command to execute raw queries to the snapd API. This is useful when
trying to query snapd endpoints directly, but curl is other similar tool is not
available.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* snap, s/snaptest: add function for reading the ComponentInfo of the current revision of a component for a snap revision
* s/snapenv, c/snap: add support for component hooks to ExtendEnvForRun
* c/snap: update run to be able to run component hooks
* c/snap: refactor runSnapConfine to operate on a runnable that can represent snap hooks, component hooks, and apps
This commit doesn't need to be here, and things will work without it.
But things were getting a bit complicated in runSnapConfine with
arguments that represented different things based on what we were
running.
* c/snap-exec: handle running component hooks in snap-exec
* c/snap-exec: move parsing of snap-exec target into execHook and execApp
* snap: make error message when failing to parse current component revision a bit better
* c/snap: add IsHook method to runnable type for easier checking
* s/snaptest: use os.Symlink rather than atomic variant in test code
* snap, s/snapdir, c/snap: fix import cycle issue with hook from snapdir into snap
* c/snap, c/snap-exec: docs and panicking default for NewContainerFromDir
* c/snap, c/snap-exec: set up hook for snap.NewContainerFromDir
* c/snap: remove TODO about getting component revision
* c/snap, c/snap-exec: use _ imports rather than initializing hook manually
* s/naming: add ParseComponentRef function
* snap, o/s/backend, daemon: replace ComponentLinkPath and ComponentInstallDate param with naming.ComponentRef
* snap: use ComponentLinkPath helper in ComponentLinkPath
* s/snapdir: add doc comment for NewContainerForDir
* Revert "snap: use ComponentLinkPath helper in ComponentLinkPath"
This reverts commit 9a56c379779490f798613db31aa66b2b177ddd3d.
* Revert "snap, o/s/backend, daemon: replace ComponentLinkPath and ComponentInstallDate param with naming.ComponentRef"
This reverts commit ca39dc1e60174d769ef2345f1e4b58d63f0f7528.
* Revert "s/naming: add ParseComponentRef function"
This reverts commit a3a9130f6d617bc817a76d884a84c1b83282bb46.
* snap: use ComponentLinkPath helper in ComponentLinkPath
* snap: remove whitespace
* snap: update doc comment on ComponentLinkPath to mention usage constraints of the ContainerPlaceInfo param
* snap: replace NOTE with TODO
This changes the naming of the aspects feature to be "registry" instead
of bundle (i.e., a configuration space backed with its own storage) and
"view" instead of aspect. Once this lands, anyone that has this enabled
needs to unset the experimental flag and rename the state entry before
refreshing snapd and then re-enable.
Signed-off-by: Miguel Pires <miguel.pires@canonical.com>
* 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>
Running a non-existing snap was showing a confusing message about
race condition detection.
This fixes confusing error by differentiating between snap not
existing and a missing current symlink due to ongoing refresh.
Signed-off-by: Zeyad Gouda <zeyad.gouda@canonical.com>
Similarly to the issue fixed in
https://github.com/snapcore/snapd/pull/13783, the test was opening a
socket just to get a valid port number and then immediately closing it.
Unfortunately, the port was being reused by another test which then
received the request from this test. Fixed by sending the request to
port 0 which cannot be connected to.
Signed-off-by: Miguel Pires <miguel.pires@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: 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>
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.
* 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>
* o/devicestate: use default recovery system that is independent of seeded systems
* o/devicestate: use default recovery system from state when rebooting without a specified label
This change has the side effect of enabling users to reboot into any
recovery system with "snap reboot --recover", regardless if it is the
default or the most recently seeded system.
* o/devicestate: expose default-recovery-system via DeviceManager.Systems()
* daemon, client: expose default-recovery-system via the systems api
* c/snap: update help text for reboot to match new default-recovery-system behavior
* c/snap: add default-recovery-system to notes section in recovery cli output
* o/devicestate: prevent removal of default recovery system
* o/devicestate, daemon: use default-recovery-system for factory-reset mode as well
* c/snap: update help text to mention that default recovery system is used for factory-reset
* o/devicestate: make a newly seeded system the default-recovery-system
* o/devicestate: add helper to DeviceManager for getting the default-recovery-system
* o/devicestate: use default-recovery-system for install mode as well
* c/snap: correct "snap reboot" help text
* o/devicestate, daemon: store more information for default-recovery-system in state to match seeded systems
* cmd/snap: tweak default recovery system notes
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* o/devicestate: make sure that we're storing correct type when setting default-recovery-system, extend tests
Make sure we're storing the right type when setting default-recovery-system.
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
---------
Signed-off-by: Maciej Borzecki <maciej.borzecki@canonical.com>
Co-authored-by: Maciej Borzecki <maciej.borzecki@canonical.com>
* many: allow building without bolt using nobolt tag
Bolt is an abandoned project. Upstream is not accepting any patches.
Debian has three versions of bolt, none of which build on riscv64 due to
the lack of appropriate architecture tags.
In Debian, bolt is not built from the vendored package but instead from
the distribution package, one of three in the archive, each capturing a
different, outdated and differently patched fork.
Fortunately, bolt is not really heavily used in snapd. While we try to
address the issue regarding bolt and riscv64, we can continue to build
snapd on riscv64 without the ability to store information only used by
command-not-found helpers.
Allow advisor to fail with advisor.ErrNotSupported when snapd is built without
bolt support. Tests for the store, overlord package and the "snap advise"
command are adjusted to skip certain tests when that error is returned.
Debian-Bug: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1010686
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* advisor: adjust error message
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* tests: run unit tests for nobolt build tag
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* packaging: drop bolt from debian dependencies
With the ability to build snapd without bolt support, we can drop the patch
responsible for using non-forked bolt. Bolt is really only needed by the
advisor package, which is used and consumed as a command-not-found data set.
None of that is effective on Debian, making maintenance easier.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* packaging: do not use bolt on Debian
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
---------
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
