* 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
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>
* snap-exec: fix detection if `cups` interface is connected
The cups interface needs a way to set the environment variable
`CUPS_SERVER` if the `cups` interface is connected. However we
have no mechanism for setting environment vars based on interface
connections currently. To workaround this, the cups work added
a workaround in `snap-exec` that tried to detect if `cups` is
connected and when it is set the environment. Unfortunately the
code in there was too simplistic because it just checked if
the directory `/var/cups` exists. However ths dir now always
exists because it's needed as the mount point.
This commit fixes the detection by checking if `/var/cups` is
a bind mount. This is checked by looking at the `stat()` data
and the `dev_t` field in there. If they differ it means the
bind mount exists and the only thing that creates this bind
mount is the `cups` `MountConnectedPlug()` code.
This is not great but it fixes the spread failure we see
in the `cups-control` test (which is a real bug) and should
be good enough until we have a proper interface backend that
can set environment variables.
* tests: re-enable interfaces-cups-control test
For unclear reasons the error message on unplug changes when I
run this on my 20.04 system so I updated the tests - it most likely
because the test-snapd-cups-control-consumer snap moved from
core16 to core20 a week ago but that is not 100% confirmed.
* snap-exec: fix unit test for CUPS_SERVER workaround
interfaces/cups: add cups-socket-directory attr, use to specify mount rules in backend
Make the cups interface slot have an optional attribute, cups-socket-directory, which
is used to specify the directory where the cups socket that the slotting snap
is going to share with client snaps.
This directory then is mounted into client snap's mount namespaces via the
mount backend at /var/cups/, such that client applications wishing to
print need to adjust the location of where they expect the cups socket to
be at all, but then will always end up sending print requests to the cups snap,
which will mediate requests to print based on whether or not the client snap
has a connected cups plug or not.
The primary advantage of this is that it means we don't need to make the cups
interface implictly slotted by the system snap, and it can always be provided
by the cups snap, and we can then make any snap with a cups interface plug
auto-connect to the slot from the cups snap unambiguously, providing all snap
clients the ability to print without any extra connections necessary,
regardless of their distro or what the client snap is.
gci needed cmd/snap-seccomp/main.go import comment to made one long line
otherwise it keeps only the last line :(
cmd/snap/color_test.go need some manual cleanup of commented out imports
This will run a snap app with an attached gdbserver from the host.
The app can run inside the confinement as a regular user and it can be
debugged by running a gdb (or frontend) and attaching it to the
running gdbserver.
The app is run as the normal user and only the gdbserver is run
as root. This allows debugging confined apps as well and it
avoids the downside of the current "snap run --gdb" that requires
to be run as root. It also allows attaching gdb frontends that
can talk gdbserver.
The setup is slightly involved:
1. snap run/snap-confine/snap-exec run normally
2. a new snap-gdbserver-shim is run that triggers "SIGSTOP"
3. gdbserver is attached once `snap run` got the SIGSTOP
from the app. Note that at this point no application code
was run yet so it does not matter if the app is using any
of these signals
4. The attached gdbserver will interrupt the running app now.
We can not do anything smart inside snap-gdbserver-shim.c
(like waiting for an attached ptracer) because that would
leak into the debug session.
5. Now gdb can get attached must send "cont", "signal SIGCONT"
6. With that setup gdb is right before the real app is exec()ed
(just like with `snap run --gdb` today)
There are some XXX inside this commit with ideas/questions about
the UX but it should be usable and an improvement over the current
`snap run --gdb`. If this looks good we could deprecate, remove
or replace the existing `snap run --gdbserver`.
The approach of attaching gdbserver from the outside instead of
running gdbserver from snap-exec was taken to avoid having to
open the ptrace syscall inside the confined app. This is what
we would have to do if we just run gdbserver inside the sandbox
that snap-confine setup for us.
* strutil: add Environment, EnvironmentDelta and RawEnvironment
This patch adds three new types and supporting methods.
Environment is an unordered map representing environment entries that is
convenient to modify. EnvironmentDelta is an ordered map representing
changes to an environment that also supports variable substitution and
expansion. Finally RawEnvironment is a slice representing key=value
pairs that is useful for low-level system interfaces.
Environment and EnvironmentDelta can be modified with simple map-like
methods Get, Set, Del. RawEnvironment can only be parsed to an
Environment.
The patch also contains Environment.ApplyDelta function, which correctly
applies a single delta to a given environment.
The types are designed to avoid the mistake of appending two slices
representing environment strings together. This was the original cause
of bug https://bugs.launchpad.net/snapd/+bug/1860369
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: use Environment, EnvironmentDelta and RawEnvironment
This patch replaces all ad-hoc environment handling with the three
aforementioned types. Environment is used whenever we are working
with the real complete environment (starting from OSEnvironment).
EnvironmentDelta is used to modify it according to snap-level and
application-level or hook-level overrides. Finally RawEnvironment
is only used by snap run and snap-exec, immediately before calling
execve.
This fixes incorrect handling of environment in snap-exec and also
avoids this class of bugs by making it hard, due to type system,
to express mistakes again.
Fixes: https://bugs.launchpad.net/snapd/+bug/1860369
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* osutil/env: import all symbols from check.v1
This style is shorter and we use it nearly everywhere else.
I'm about to combine more tests into this file and I don't want
to invert their style over to the verbose one.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: move new environment code to osutil
There's a lot of churn but that's unavoidable in such a move.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: remove RawEnvironment type
The raw environment is necessary only for executing programs. We can
remove the type and replace it with []string in tests (where it has most
of the usage) and a few call sites.
The corresponding method from Environment was renamed to ForExec().
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: make Environment a map[string]string
Some extra methods such as Get, Set and Delete are retained but they
are all arguably unnecessarily and will be removed in another pass.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: remove most map-like methods from Environment
This includes Get, Delete and Contains but not Set. Set has extra
semantics that will be removed in another pass.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: remove Environment.Set
This patch removes the leftover map-like method from the Environment
type. Some extra care was applied to ensure we don't try to write
through a nil map, something that Set protected against.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* cmd/snap-exec: use NewEnvironment in test code
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: rename EnvironmentDelta to ExpandableEnv
The main feature of what was called EnvironmentDelta is the fact it
could expand expressions in variable definitions. This makes it more
obvious.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: make ExpandableEnv an immutable *strutil.OrderedMap
There's a lot of extra changes to make the journey more complete.
First of all both snap.AppInfo and snap.HookInfo provide a method
EnvStack() []osutil.ExpandableEnv which replaces the earlier
EnvironmentOverrides. Instead of merging environment entries the
entire chain is modeled. This will be relevant when we want to expand
values.
This also means that support code to make EnvStack mutable is gone.
Second of all, snap/snapenv.ExecEnv has reverted to using plain
Environment (aka map[string]string) for basicEnv and userEnv. The
values never had expressions so using the more sophisticated code
there is not really necessary.
Delta from master could be reduced further but I went for clarity
of the progression over the clarity of the end-to-end diff.
Lastly, a small but important change, snap-exec now expands each
ExpandableEnv in sequence. First for the snap-level and then for the
app or hook level. This should fix the issue that each level wanted
to expand but not completely erase the value set previously.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* osutil: remove NewEnvironment
With the current type it's not any better than just defining a map
locally.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* osutil: make ParseRawEnvironment private
It is only required for OSEnvironment and is not used anywhere outside
of the package.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: rename ApplyDelta to SetExpandableEnv
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* osutil: update stale documentation referring to delta
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* osutil: weaken SetExpandableEnv
When initially implemented as a part of this branch, SetExpandableEnv
(nee ApplyDelta) behaved in somewhat unusual and "strong" way, where
forward references _were_ expanded. This was not previously so and
arguably is not needed.
As the code stands now, we have proper chaining starting from system
environment, to snap-level environment declarations (that can expand
variables) and all the way down to application or hook-level environment
declarations (also with variable expansions). Since such declarations
are ordered this provides the equivalence of shell execution performing
the same expansions.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: special-case Environment.Transform
Instead of offering specialized environment transformation functions
offer just the things we need: escaping and un-escaping of unsafe
variables removed by the dynamic linker when loading binaries with
AT_SECURE flag.
Various bits move from snap/snapenv to osutil.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
* many: EnvStack to EnvChain, SetExpandableEnv to ExtendWithExpanded
also adjust doc comments and cover ExtendWithExpanded for the nil
initial env case
* cmd/snap,snap/snapenv: change snapenv.ExecEnv into ExtendEnvForRun
* many: make preserving ld.so setuid unsafe vars a boundary concern
support the escaping/unescaping that we use for preserving vars
stripped out by ld.so for snap-confine, via variants of
OSEnvironment/ForExec so that is happens at the process boundaries,
loading os env and setting env for exec
add/change some tests related to this
adjust comments/doc comments or add
Co-authored-by: Samuele Pedroni <pedronis@lucediurna.net>
Co-authored-by: Michael Vogt <mvo@ubuntu.com>
It was noticed that snap-exec's handling of environment is incorrect.
Under certain circumstances the environment block contains more than one
entry for the given key (environment variable). In addition, though this
is not illustrated in this patch, the internal expansion code (built
into parts of snapd) uses only one of those values when expanding
environment variables ahead of forwarding their values to snap
applications.
This patch contains only the test case showing the problem.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Implement a finer lookup for the completion helper, under the assumption that
the helper always exists in the same directory as snap-exec.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Fix completion for classic snaps on systems where snapd libexecdir is not
/usr/lib/snapd. These are for instance Fedora and CentOS, where libexecdir is
set to /usr/libexec/snapd.
When run for completions, snap-exec would always attempt to run the completer
from the path which is correct given the snap gets a mount namespace (i.e. is
not classic).
This worked by accident on systems that use /usr/lib/snapd (eg. Ubuntu, Debian,
Arch), but failed on those using /usr/libexec/snapd.
For instance, on CentOS, one would see:
$ cmake -G Ni<tab>
/bin/bash: /usr/lib/snapd/etelpmoc.sh: No such file or directory
Fixes: https://bugs.launchpad.net/snapd/+bug/1833043
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Since we can os.Lstat any symlink safely we can determine that it is
actually a symbolic link and then decide not to follow it or use any
information about it.
This simplifies the API, keeping the fix to LP: #1800004 fixed.
Thanks to Chipaca for the idea.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
A try-mode snap is represented by a bind mount from whatever the original
location was, to /snap/$SNAP_NAME/$SNAP_REVISION as well as a symlink
from /var/lib/snapd/snap/$SNAP_NAME_$SNAP_REVISION.snap to whatever
the original location was.
In the execution environment the symlink cannot work, in general. The
code loading snap.Info was using the fully generic snap.ReadInfo which,
as one of the features, tried to stat and compute the size of the snap.
Since reading the size of a snap is not very useful for snap-exec and
since it can fail if the symlink is not reachable in the execution
environment, switch to the more limited helper.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
When a snap is created by the snapcraft CLI, each hook potentially has
an associated chunk of environment variables defined as well as scripts
that may be run before the hook itself. Currently, this is done by
generating a shell script wrapper for each hook, but this is opaque and
confusing.
Snapd supports an `environment` keyword for apps and hooks as well as a
`command-chain` for apps. Add the last item needed before the shell
wrappers can go away completely: `command-chain` support in hooks.
See forum topic https://forum.snapcraft.io/t/6112 for more details.
Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
When a snap is created by the snapcraft CLI, each app has an associated
chunk of environment variables defined as well as scripts that may be
run before the app's command. Currently, this is done by generating a
shell script wrapper for each app, but this is opaque and confusing, and
makes `snap run --shell` not actually provide a representative
environment for the app.
Snapd supports an `environment` keyword for apps and hooks, but the last
item needed before the shell wrappers can go away is the ability to run
scripts before the app command. Add support for this to apps. Hook
support will be a follow-up.
See forum topic https://forum.snapcraft.io/t/6112 for more details.
Signed-off-by: Kyle Fazzari <kyrofa@ubuntu.com>
