~snappy-dev/snapd/+git/snapd-pawel:disk-space-awareness/remove-check

Branch merges

Related source package recipes

No recipes using this branch. (?)

Create packaging recipe

Related snap packages

Related charm recipes

41bcf5b... by Paweł Stołowski on 2020-08-24

Workaround for nsfs-related denials on centos8 due to snapshot size estimation for lxd snap.

044d27b... by Paweł Stołowski on 2020-08-24

Merge branch 'master' into disk-space-awareness/remove-check

0f3ac65... by Paweł Stołowski on 2020-08-24

Update SELinux security profile to allow listing of tmpfs_t directory contents for snapd.

bc21b77... by Zygmunt Krynicki on 2020-08-24

many: use transient scope for tracking apps and hooks (#7825)

This is the left-over commit after the refresh-app-awareness
application tracking branch was de-composed and merged to
master separately.

* many: use transient scope for tracking apps and hooks

This patch entirely changes how snapd tracks programs for the purpose
of refresh-app-awareness feature. The old method was using cgroup-v1
only behavior that was stealing processes from systemd's point of view.
The new behavior relies on a DBus API call to systemd, to create
a transient scope with a name that allows snapd to derive snap security
tag from. Due to the requirements of the used API the scope name
must be unique. This is enforced by systemd and handled by creating
a scope with the name "snap.${uuid}.${package}.${app}.scope" for apps
and "snap.${uuid}.${package}.hook.${hook}.scope" for hooks. Services
have a .service unit that is giving us equivalent information so they
do not have or need a scope. The UUID is generated by the kernel.

The old approach relied on privileged snap-confine to alter cgroup
configuration. The new approach requires an unprivileged DBus call and
was thus moved to snap-run. The associated C code and apparmor permissions
have been removed.

The new approach does not support systemd used on Ubuntu 14.04.
The feature is not effective there and application tracking fails
silently.

The code has several TODOs that I plan to address as follow-ups,
given that the feature is behind a feature flag anyway:

 - add additional spread test for tracking services
 - add additional spread test for tracking non-root applications

In addition I plan to make some organization changes so that large parts
of what is in overlord/snapstate/refresh is moved to sandbox/cgroup
instead. I decided to do this as a follow-up as it is easier to handle
with regards to conflicts.

* tests: correct old comment

* sandbox/cgroup: drop unused PidsInGroup

Reused some tests for PidsInFile

* overlord/snapstate: test extra cgroup path variants

* overlord/snapstate: use filepath.Base instead of Split

* overlord/snapstate: use filepath.Dir instead of Split

* overlord: fix typo 'dot'

Co-Authored-By: Ian Johnson <email address hidden>

* overlord: fix typo 'processes'

Co-Authored-By: Ian Johnson <email address hidden>

* overlord: spurious newline

Co-Authored-By: Ian Johnson <email address hidden>

* overlord: add missing dot

Co-Authored-By: Ian Johnson <email address hidden>

* tests: fix typo "systemd"

Co-Authored-By: Ian Johnson <email address hidden>

* tests: tweak wording

Co-Authored-By: Ian Johnson <email address hidden>

* tests: add missing comma

Co-Authored-By: Ian Johnson <email address hidden>

* overlord: fix typo 'harvest'

Co-Authored-By: Ian Johnson <email address hidden>

* tests: reword stale comment about /run/snapd/cgroup

* overlord/snapstate: clarify locking of refresh checks

* overlord: handle trailing slash in securityTagFromCgroupPath

It used to require a clean path but it is more robust and simple to
clean internally. This way the function is less brittle.

* cmd/snap: reformat code around dbus call

* overlord,sandbox: move pidsOfSnap to sandbox/cgroup

Some of the support code, namely securityTagFromCgroupPath and writePids
moves as well. The overlord-side tests now rely on a mock that no longer
scans cgroups and reads files. Some test data was adjusted (mainly "foo"
=> "pkg" rename of the snap name).

* sandbox: add comment about cgroup -> security tag

* sandbox/cgroup: verify that snap.*.scope is handled ok

* sandbox/cgroup: skip descending after cgorup.procs

If we find the cgroup.procs file, associate the cgroup path with a snap
security tag *and* extract PID information from it we no longer need to
recursively scan the directory.

This is a micro-optimization.

* snap/naming: add validator for snap security tag

* sandbox/cgroup: validate snap security tags

When scanning cgroup hierarchy validate all snap security tags before
handing them off to the caller.

* cgroup/sandbox: scan precisely the right cgroup path

In v1 mode scanning /sys/fs/cgroup/ is wasteful because there will be
lots of other cgroups and duplicates there. We can resort to scanning
just /sys/fs/cgroup/systemd.

In v2 mode scanning /sys/fs/cgorup is good as-is.

* cmd/snap: reword comment for clarity

* cmd/snap: add TODO about service cgroups

When systemd starts a service it places it in a .service cgroup path
already. We could verify that.

* cmd/snap: document properties and auxUnit

* cmd/snap: explain why we use uniquely named scopes

* overlord/snapstate: gofmt -s

* tests: fix cgroup-tracking test after test-snapd-sh changes

* sandbox/cgroup: unexport PidsInFile

* cmd/snap-run: document randomness for scope UUIDs

The randomness is provided by the kernel, upon reads from
/proc/sys/kernel/random/uuid, is ultimately fulled by the same source
that feeds /dev/urandom.

* cmd/snap-run: explain how process accounting works

This comment was requested by Jamie so that we have an overview of the
entire cgroup management process in one location, that is, next to the
transient scope creation logic.

* cmd/snap-run: refer to documentation of StartTransientUnit

* cmd/snap-run: return distinct error on scope clashes

This might help in debugging in case our UUID generator is unreliable.

* dirs: make CgroupDir private

It is only required internally for FreezerCgroupDir

* sandbox/cgroup: correct documentation of PidsOfSnap

The relationship between experimental tracking feature and services was
changed since the comment was originally written.

* sandbox/cgroup: tweak potentially confusing walker

The walker short-circuited an error being provided by the golang
standard library _or_ a directory being visited (where error is nil)
into one condition, returning the error (either real or nil) to quickly
express the fact that such case is not interested for analysis in the
walk function. This was correct but possibly confusing. Split the code
into explicit chunks and tests.

* sandbox/cgroup: expand documentation of the walker

* tests: spell out cgroup configurations exercised

* sandbox/cgroup: verify pid tracking and snapd.service

* sandbox/cgroup: more tests for parsing pids

* sandbox/cgroup: more tests for pid-snap association

* sandbox/cgroup: add extra test for PidsOfSnap

* sandbox/cgroup: fix typo "correctly"

* sandbox/cgroup: fix typo "encountered"

* sandbox/cgroup: fix typo "momentarily"

* cmd/snap-run: move debug line to where it should be

* cmd/snap-run: log if we cannot make transient scopes

* sandbox/cgroup: document why we scan particular dirs

* tests: run cgroup tracking test as test and root

* tests: exec commands, not just run them

* sandbox/cgroup: use put rather than bin

* sandbox/cgroup: add missing comma

* sandbox/cgroup: use "from it" instead of "from one"

* sandbox/cgroup: don't capitalize after colon

* sandbox/cgroup: among ... results, not result

* sandbox/cgroup: improve comment clarity

* sandbox/cgroup: say "for a" instead of "of a"

* sandbox/cgroup: fix incorrect wording

* cgroup/sandbox: plural processes

* sandbox/cgroup: fix some more grammar

* cmd/snap: reword comment about randomness quality

* cmd/snap: tweak comment wording

* cmd/snap: tweak more comments

* cmd/snap: fix wording and refer to upstream docs

* cmd/snap: rewrite documentation of scope placements

* cmd/snap: add TODO to invert scope names

* tests: improve quoting situation in session-tool

Using single quotes will now work. Mixing quotes will misbehave but
at least it is somewhat better than before.

* tests/session-tool: allow tracking payload pid

session-tool invokes a session and always has a parent process (session
manager) and payload process (what you wanted to run). In the case
where the payload process PID is relevant, you can use the -p option to
write the pid of the payload process to a file.

* cmd/snap: handle ChildExited erorr

When invoking a snap as a non-root user talk to the session bus
to create systemd transient scope. In the case that the session bus
DBus socket is not available, such as on Debian derivatives which package
that socket into a distinct package, we get a ChildExited error when
attempting to send a DBus message.

Since the session bus is always available in desktop sessions this
failure is deemed non-fatal. It is more important to start applications
than to track them for refresh-app-awareness.

* cmd/snap: verify effective path in the tracking cgroup

When running under systemd << v238 we may end up asking systemd to
start a transient scope with our PID in it, get no notification of error
of any kind but get a silent error.

This is caused by the fact that systemd relies on kernel policy when
moving a process within a cgroup hierarchy. In the case that the same
user owns the origin and destination, the move is allowed.

In the case that this ownership is not the same this move is denied.

Systemd v238 has grown extra features where systemd-wide systemd can
assist in this move, even if the kernel would otherwise not allow it
(root can move anything). On system running older versions of systemd
this operation can silently fail.

It is important to know that it doesn't fail in a desktop session,
because ownership is arranged such so that the running user owns both
"sides".

This particular error can happen when snaps are started having logged
in remotely, for example with ssh.

To detect this case, where eventually we would like to issue a snapd
warning, measure the effective path in the tracking cgroup and log
a debug message when this situation has occurred.

* tests: fix leftovers from bad merge

* tests: disable cgroup-tracking for core20

This test is failing on core 20 because of something being missing from
user session stack. I'm looking into this but in order to unblock
progress let's disable this test for now.

* tests: remove pid and stamp files

* testutil: add NewDBusTestConn

This patch adds support code for testing code making DBus calls.
It is similar in spirit to our HTTP testing code. A DBus connection
is provided, marshaling messages sent between application code driven
by the test and the message bus.

This allows testing any DBus interaction in isolation, with any scenario
that is desired.

* cmd/run: early work for testing dbus in snap-run

We use two DBus calls in snap run but they are entirely untested. This
patch begins the journey towards eventual full coverage of DBus
interactions.

* tests: remove, not install

* cmd/snap: document the significance of scope names

* cmd/snap: highlight reasons for choice of uniqueness

* tests: rename pidN_session to sessionN_pid

The old name read like it was the session of PID 1, which this is not.

* tests: clarify that snap run performs cgroup transition

Earlier design used snap-confine for this operation but this is now done
by snap run. The comment was just stale.

* cmd/snap: document doCreateTransientScope

* cmd/snap: de-duplicate doCreateTransientScope comment

* sandbox/cgroup: test variant with two users

The test now models two users, in two separate sessions, running the
same application twice, each.

* many: gofmt 1.6

* tests: pass -y to apt-get

* cmd/snap: ignore failures to connect session bus

There are two instances where snap run attempts to connect to session
bus, one when snap run decides to wake up the XDG document portal, the
other is where snap run decides to place the started program in a new
transient unit, a new scope.

The location of the session bus can be provided in one of several ways,
mainly with an environment variable $DBUS_SESSION_BUS_ADDRESS. Another
option is to just try to connect to /run/user/UID/bus.

When using "sudo" to switch from one non-root user to another non-root
user, unless sudo is used with -l, to create a new session, there will
be no bus to connect to.

While not great, tracking is secondary to executing. As such ignore all
errors from establishing connection to session bus.

* cmd/snap: handle session bus failure better

Work on the test exercising various failures of application tracking
feature has shown me that in some cases we can establish a connection to
the session bus but fail to invoke a DBus method to systemd --user. This
is especially true on Ubuntu 16.04, where systemd --user is just not
available.

This patch changes handling various errors so that failure to connect to
the session bus and failure to talk to systemd --user are handled the
same way. When the invoking user is root we fall back to using the
system bus.

Doing this has also allowed changing some of the error handling so that
failure to track an application is a non-fatal error that logged when
debugging is enabled.

* cgroup: use .mount unit in TestPidsOfSnapUnrelatedStuff

* tests: pass SNAPD_DEBUG SNAP_CONFINE_DEBUG via session-tool

Those variables are useful but the nature of starting programs with
session-tool makes them run with pristine environment. After pulling
hair from my head for a moment I realized this is the problem.

While passing all environment is certainly undesirable, passing the two
debug variables we use often seems worthwhile.

* tests: use pkill rather than killall

Some systems lack killall.

* cmd/snap: do not activate systemd with dbus calls

And here I thought those flags would not come in handy.

On systems that don't have systemd --user, like Ubuntu 14.04 or Ubuntu
16.04, attempting to bus-activate systemd will not work very well. In
absence of a session bus, go-dbus spawns dbus-launch, then proceeds to
activate org.freedesktop.systemd1, which times out after several
minutes.

I'm still looking at how we can avoid spawning dbus-launch, I think one
way would be to not use the helpers provided by go-dbus but instead
perform the detection ourselves, by looking for $DBUS_SESSION_BUS_ADDRESS or
accessing the socket at /run/user/<UID>/bus and failing fast if those
are not available.

* cmd/snap: handle NameHasNoOwner

On Ubuntu 14.04 systemd is not running as a session service, since we no
longer try to bus-activate it we now, quickly get a response from the
bus manager that the name org.freedesktop.systemd1 has no owner.

Treat this exactly the same as we handle other cases of old systemd, by
not creating the transient scope.

* tests: show how app tracking can fail

The interaction of cgroup-based application tracking and systemd version
is sadly complex. This test shows how the interaction looks like across
versions of the stack.

This patch also undoes the no-activate flag, as this is required to
interact with systemd --user on 16.04+.

This patch also contains a fix for dbus-launch leaking when there is no
session bus and go-dbus tries to make one.

The test expands to all core and classic Ubuntu systems.

* cmd: fix typo

* tests: measure tracking on core systems

* tests: re-enable cgroup-tracking on core20

* tests: add theory to 16vs18 mystery

* tests: fixup grep pattern

* tests: explain why cgroup-tracking test fails on core20

* tests: adjust cgroup-tracking-failure after core20 updates

Core 20 now supports DBus session bus.

* cmd/snap,cgroup: put swap UUID and security tag

Jamie asked for the systemd scope names to have the security tag up
front and the UUID at the back. This patch does just that.

* tests:adjust the cgroup-tracking-failure test

This test is impacted by what is available in test images. Since we just
released new images with additional dependencies pre-installed I
adjusted the test to check the variant without DBus session bus in a
different way.

* dbusutil: move DBus utilities to new package

The cmd/snap package is full of various test helpers and in an effort to
reduce the clutter, this patch is moving various DBus helpers to a new
top-level package.

* dbusutil: add dbustest package

This package contains the code that used to live in
testutil/dbustest2.go. At some point we should also move the
existing debustest.go that spawns real DBus daemon.

* sandbox/cgroup: move CreateTransientScope

The snap run command contains lots of low-level bits, related to
application startup. This patch moves the tracking cgroup code over to
the cgroup package.

* cgroup/sandbox: remove ControllerPathV1

With the tracking code in place, this function is no longer used.

This code has landed meanwhile, and needs to be adjusted for the generic
support for cgroup v2 for app tracking.

* cgroup: test session -> system bus fallback

When snapd wants to track an application it is launching, it is asking
systemd to create a transient scope unit. Normally the session systemd
is used (aka systemd --user) but if that bus is not available and if the
user is root, then the logic automatically falls back to the system bus.

This allows tracking applications that live outside of any session, as
long as they run as root. One concrete example is all of the snap hooks,
that are invoked by snapd.

* cgroup: test doCreateTransientScope error paths

There are several errros that are handled specially at this level. Most
of the interesting things happen higher but test coverage is improved a
little.

* tests: remove sleep 3 left over from debugging

* cgroup: drop implemented TODO

The UUID and security-tag were swapped earlier but I didn't notice the
comment.

* cgroup: typo "IDs"

* dirs: remove unused cgroup dirs

After the recent shuffles, it seems that the directory entries for
cgroups is no longer used. I will also follow up to modernize the
private variable used by sandbox/cgroup module.

* cgroup: modernize directory definition

Instead of custom mocking and a private variable FreezerCgroupDir is now
a part of the root directory callback system.

* cgroup: make doCreateTransientScope mockable

* cgroup: add more test for CreateTransientScope

Those tests exercise various interactions that fail in interesting ways.

* cgroup: swap getting uuid and connecting to dbus

There's no point in wasting entropy if DBus is unavailable.

* cgroup: fix typo "immediately"

* cmd/snap: simplify mocking of CreateTransientScope

* cgroup: add ConfirmSystemdServiceTracking

snap run needs to confirm if the current process is tracked as a systemd
service belonging to the given security tag. This patch provides a trivial
implementation of this logic.

* cmd/snap: verify existing tracking of systemd services

Tracking is now uniformly handled by both services, non-service apps
and hooks. In addition the condition indicating if tracking is required
is now more explicit.

* tests: fix emulation of disabled dbus service

* tests: do not install/remove dbus-user-session

This package is now pre-installed. In addition the restore section was
buggy by unconditionally removing the package, even though it was not
effectively installed by the test.

* snap/naming: add ParseSecurityTag and friends

We have a need to parse security tags and extract specific fields,
like snap name, in a reliable way. This patch introduces a new function,
ParseSecurityTag, along with a group of related interfaces
ParsedSecurityTag, ParsedAppSecurityTag and ParsedHookSecurityTag.

Together they piggy back on the work that was already done for
ValidateSecurityTag, to return the right data at the right time.
At the same time the aforementioned function reduces to a trivial
call to ParseSecurityTag.

* cgroup: replace pathOfPidCgroup with ProcPidPath

We had managed to collect some duplicate logic with exactly the
same semantics but different mocking helpers. Drop the one that feels
more redundant.

* cgroup: extend SnapNameFromPid to support tracking cgroup

There are two ways we can identify a process as belonging to a snap:
 - using the freezer cgroup, available to all apps and hooks
   when cgroup v1 is used.
 - using the tracking cgroup, available to all services or all
   apps and hooks when refresh-app-awareness is enabled

This patch extends the logic to cover the second case.

* cgroup: improve tests for SnapNameFromPid

Some duplicate boilerplate moved to common helper. Some new tests.

* cgroup: mock version of cgroup in tests

This went a bit unnoticed. The code that scans for pids is cgroup aware
but tests were not mocking it before.

* cgroup: make writePids aware of cgroup version

This way we can easily write tests for v2 as well as v1.

* cgroup: test PidsOfSnap in cgroup.V2 mode

All tests now run both in v2 and in v1 mode.

* cgroup: document use of SkipDir

* dbusutil: add missing unit tests

* cgroup: use dbustest.StubConnection

Removing some duplicate boilerplate.

* dbusutil: tweak name of mock functions

This makes it more clear the other bus type is not available

* cgroup: ensure unrelated files are not scanned

* sandbox/cgroup: allow discovering PIDs of given snap

The main function PidOfSnap, returns information about the processes of
a given snap, including the application or hook each process belongs to.

The returned information is inherently volatile, as processes can die
and fork at will. To use correctly it needs to be paired with
appropriate locking. The main consumer will be the snap manager, which
will use it around refreshes.

The final usage depends on the both the snap lock and the snap run
inhibition lock, which is critical for consistency of the result.
Holding the snap lock, perform the measurement, depending on the result
either release the snap lock and do nothing new or obtain the snap run
inhibition lock and release the snap lock. This method guarantees that
we can reliably make a decision not on the particular pid values but on
the set of applications and hooks that are running - exactly what
refresh app awareness was designed to need.

The actual logic is relatively simple and involves scanning cgroups for
directories named snap.$SNAP_INSTANCE_NAME.*, each directory has a
cgroup.procs file that contains a snapshot of the PIDs inhabiting that
cgroup at a given time. All cgroup directories that belong to the snap
in question are scanned and aggregated.

Both cgroup v1 and cgroup v2 modes are supported. In v1 mode we only
scan the systemd tracking cgroup. In v2 mode there is only one directory
with unified cgroup so everything is canned.

There's a collection of unit tests. Spread tests are bundled with the
complete feature, where we measure application tracking in detial.

* tests: stop root's session dbus.service

The cgroup-tracking test runs various scenarios to examine how cgroup
tracking behaves in practice. Once case involves the root user. On some
systems the root user has a session with a session bus that can be
accessed by the tracking feature. On those systems that bus is
socket-activated and stays behind after the test executes.

Due to the limitations in tests.session ability to restore the session
for the root user, handle dbus.service via a special case in those
specific tests.

* sandbox: drop unneeded test helper

* sandbox/cgroup: remove unused PidsInGroup

We no longer use this public function anywhere. Internally we use one of
the simpler versions that is not exported.

* tests: stop root session dbus when restoring

* tests: explicitly remove lxd and lxd-demo-server

* tests: explicitly stop root session dbus.service

* tests: update stale reference to version-tool

* cmd/snap: remove workaround for racy systemd

The workaround was required because both snap run and snap-confine were
writing to cgroups (indirectly via systemd and directly, respectively).
With a single writer we can restore the simpler code.

* cmd/snap: remove unused EnableFeatures test helper

The corresponding code is now in sandbox/cgroup.

Co-authored-by: Ian Johnson <email address hidden>

c61fa95... by Zygmunt Krynicki on 2020-08-24

Merge pull request #9198 from zyga/feature/hidden-snap-folder

features: add HiddenSnapFolder feature flag

0307c53... by Claudio Matsuoka on 2020-08-21

Merge pull request #9203 from anonymouse64/bugfix/code-must-match-comments

tests/lib/nested.sh: fix partition typo, unmount the image on uc20 too

410d5fd... by Ian Johnson on 2020-08-21

tests/lib/nested.sh: fix partition typo, unmount the image on uc20 too

and git blame says the culprit is ...

  .-'---`-.
,' `.
| \
| \
\ _ \
,\ _ ,'-,/-)\
( * \ \,' ,' ,'-)
 `._,) -',-')
   \/ ''/
    ) / /
   / ,'-'

Signed-off-by: Ian Johnson <email address hidden>

f964a28... by Zygmunt Krynicki on 2020-08-21

Merge pull request #9200 from zyga/tweak/snap-run-opens-read-only-lock-file

runinhibit: open the lock file in read-only mode in IsLocked

ae452df... by Ian Johnson on 2020-08-21

Merge pull request #9190 from anonymouse64/feature/recover-mode-reboot-back-run-initramfs

cmd/s-b/initramfs-mounts: make recover -> run mode transition automatic

This ensures that reboots or even hard resets that happen at any point
after we exit the initramfs will cause the system to transition out of
recover mode to run mode automatically.

5cd1446... by Ian Johnson on 2020-08-21

boot/initramfs: rename MarkRecoverModeBootSuccessful to EnsureNextBootToRunMode

Also finish the incomplete doc-comment, and combine the err statement in
cmd_initramfs_mounts.go into the if statement for a single line.

Signed-off-by: Ian Johnson <email address hidden>