setup-profile configures security based on snap.Info from DisconnectSnap, which still sees older revision

Bug #1572463 reported by Michael Vogt on 2016-04-20
24
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Snappy
High
Zygmunt Krynicki
snapd (Ubuntu)
High
Zygmunt Krynicki
Xenial
High
Zygmunt Krynicki

Bug Description

When the same snap is sideloaded two times the security profile stops working:

```
$ sudo snap install youtube-dl_2016.03.27_amd64.snap
[\] Setup snap "youtube-dl" security profiles
$ youtube-dl.run
WARNING: Assuming --restrict-filenames since file system encoding cannot encode all characters. Set the LC_ALL environment variable to fix this.
Usage: youtube-dl [OPTIONS] URL [URL...]

youtube-dl: error: You must provide at least one URL.
Type youtube-dl --help to see a list of all options.

$ sudo snap install youtube-dl_2016.03.27_amd64.snap
[-] Copy snap "youtube-dl" data
$ youtube-dl.run
/bin/sh: 0: Can't open /snap/youtube-dl/100002/command-run.wrapper

$ dmesg|tail -n1
[13348.347319] audit: type=1400 audit(1461143833.011:132): apparmor="DENIED" operation="open" profile="snap.youtube-dl.run" name="/snap/youtube-dl/100002/command-run.wrapper" pid=28849 comm="command-run.wra" requested_mask="r" denied_mask="r" fsuid=1000 ouid=0

$ grep SNAP_REVISION}= /var/lib/snapd/apparmor/profiles/snap.youtube-dl.run
@{SNAP_REVISION}="100001"
```

$ snap changes|grep sideload
6 Done 2016-04-20T09:17:02Z 2016-04-20T09:17:03Z Install "/tmp/snapd-sideload-pkg-620395148" snap file
7 Done 2016-04-20T09:17:10Z 2016-04-20T09:17:11Z Install "/tmp/snapd-sideload-pkg-340731359" snap file

Michael Vogt (mvo) wrote :
Download full text (3.4 KiB)

Here is the output of syslog:

```
Apr 20 11:17:02 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 28 on Do: Mount snap "youtube-dl"
Apr 20 11:17:02 top systemd[1]: Mounting Squashfs mount unit for youtube-dl...
Apr 20 11:17:03 top systemd[1]: Mounted Squashfs mount unit for youtube-dl.
Apr 20 11:17:03 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 29 on Do: Copy snap "youtube-dl" data
Apr 20 11:17:03 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 30 on Do: Setup snap "youtube-dl" security profiles
Apr 20 11:17:03 top kernel: [13338.436765] audit: type=1400 audit(1461143823.099:127): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="snap.youtube-dl.run" pid=28776 comm="apparmor_parser"
Apr 20 11:17:03 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 31 on Do: Make snap "youtube-dl" available to the system
Apr 20 11:17:03 top snap[28710]: main.go:151: DEBUG: cannot parse arguments: [snap install youtube-dl_2016.03.27_amd64.snap]: <nil>
Apr 20 11:17:06 top kernel: [13342.020641] audit: type=1400 audit(1461143826.683:128): apparmor="DENIED" operation="open" profile="snap.youtube-dl.run" name="/proc/28781/mounts" pid=28781 comm="python3" requested_mask="r" denied_mask="r" fsuid=1000 ouid=1000
Apr 20 11:17:06 top kernel: [13342.178885] audit: type=1400 audit(1461143826.839:129): apparmor="DENIED" operation="exec" profile="snap.youtube-dl.run" name="/sbin/ldconfig" pid=28782 comm="python3" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0
Apr 20 11:17:06 top kernel: [13342.183377] audit: type=1400 audit(1461143826.847:130): apparmor="DENIED" operation="exec" profile="snap.youtube-dl.run" name="/sbin/ldconfig" pid=28784 comm="python3" requested_mask="x" denied_mask="x" fsuid=1000 ouid=0
Apr 20 11:17:10 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 33 on Do: Mount snap "youtube-dl"
Apr 20 11:17:11 top systemd[1]: Mounting Squashfs mount unit for youtube-dl...
Apr 20 11:17:11 top systemd[1]: Mounted Squashfs mount unit for youtube-dl.
Apr 20 11:17:11 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 34 on Do: Make current revision for snap "youtube-dl" unavailable
Apr 20 11:17:11 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 35 on Do: Copy snap "youtube-dl" data
Apr 20 11:17:11 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 36 on Do: Setup snap "youtube-dl" security profiles
Apr 20 11:17:11 top kernel: [13346.496807] audit: type=1400 audit(1461143831.159:131): apparmor="STATUS" operation="profile_replace" profile="unconfined" name="snap.youtube-dl.run" pid=28846 comm="apparmor_parser"
Apr 20 11:17:11 top /usr/lib/snapd/snapd[2951]: taskrunner.go:234: DEBUG: Running task 37 on Do: Make snap "youtube-dl" available to the system
Apr 20 11:17:11 top snap[28789]: main.go:151: DEBUG: cannot parse arguments: [snap install youtube-dl_2016.03.27_amd64.snap]: <nil>
Apr 20 11:17:13 top kernel: [13348.347319] audit: type=1400 audit(1461143833.011:132): apparmor="DENIED" operation="open" profile="snap.youtube-dl.run" name="/snap/youtube-dl/100002/command...

Read more...

description: updated
Michael Vogt (mvo) wrote :

Ups, sorry, the second run does also show the apparmor_parser

Zygmunt Krynicki (zyga) wrote :

I cannot reproduce this with side-loaded hello-world. I get the correct revision each time (100..1, then ...2, then ...3)

summary: - Installing a new two times leaves it with no security profiles
+ Installing a new two times leaves it with profiles referring to older
+ revision
Zygmunt Krynicki (zyga) wrote :

Ok, so this is what this bug is about:

When setup-profile runs it does the following sequence of actions:

 - disconnect the snap in question, remember the list of snaps that were affected by this disconnect
 - remove the snap from Repository (the one with the older Revision)
 - add the snap to the repository (the one with the newer Revision)
 - auto connect the new snap
 - reload connections for the new snap
 - setup the security of all affected snaps

The last step is where the bug shows up. The list of affected snaps is remembered as a list of snap.Info objects. Those keep the old .Revision around.

summary: - Installing a new two times leaves it with profiles referring to older
- revision
+ setup-profile configures security based on snap.Info from
+ DisconnectSnap, which still sees older revision
Zygmunt Krynicki (zyga) on 2016-04-20
Changed in snapd (Ubuntu):
status: New → In Progress
importance: Undecided → High
assignee: nobody → Zygmunt Krynicki (zyga)
Michael Vogt (mvo) on 2016-04-26
Changed in snappy:
milestone: none → sru-1
status: New → Triaged
importance: Undecided → High
Zygmunt Krynicki (zyga) on 2016-04-26
Changed in snappy:
status: Triaged → Fix Committed
assignee: nobody → Zygmunt Krynicki (zyga)
Michael Vogt (mvo) on 2016-04-29
Changed in snapd (Ubuntu):
status: In Progress → Fix Committed
Changed in snapd (Ubuntu Xenial):
status: In Progress → Fix Committed

Hello Michael, or anyone else affected,

Accepted snapd into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/snapd/2.0.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-needed
John Lenton (chipaca) on 2016-05-12
tags: added: verification-done
removed: verification-needed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package snapd - 2.0.3

---------------
snapd (2.0.3) xenial; urgency=medium

  * New upstream micro release:
    - integration-tests, debian/tests: add unity snap autopkg test
    - snappy: introduce first feature flag for assumes: common-data-dir
    - timeout,snap: add YAML unmarshal function for timeout.Timeout
    - many: go into state.Retry state when unmounting a snap fails.
      (LP: #1571721, #1575399)
    - daemon,client,cmd/snap: improve output after snap
      install/refresh/remove (LP: #1574830)
    - integration-tests, debian/tests: add test for home interface
    - interfaces,overlord: support unversioned data
    - interfaces/builtin: improve the bluez interface
    - cmd: don't include the unit tests when building with go test -c
      for integration tests
    - integration-tests: teach some new trick to the fake store,
      reenable the app refresh test
    - many: move with some simplifications test snap building to
      snap/snaptest
    - asserts: define type for revision related errors
    - snap/snaptest,daemon,overlord/ifacestate,overlord/snapstate: unify
      mocking snaps behind MockSnap
    - snappy: fix openSnapFile's handling of sideInfo
    - daemon: improve snap sideload form handling
    - snap: add short and long description to the man-page
      (LP: #1570280)
    - snappy: remove unused SetProperty
    - snappy: use more accurate test data
    - integration-tests: add a integration test about remove removing
      all revisions
    - overlord/snapstate: make "snap remove" remove all revisions of a
      snap (LP: #1571710)
    - integration-tests: re-enable a bunch of integration tests
    - snappy: remove unused dbus code
    - overlord/ifacestate: fix setup-profiles to use new snap revision
      for setup (LP: #1572463)
    - integration-tests: add regression test for auth bug LP:#1571491
    - client, snap: remove obsolete TypeCore which was used in the old
      SystemImage days
    - integration-tests: add apparmor test
    - cmd: don't perform type assertion when we know error to be nil
    - client: list correct snap types
    - intefaces/builtin: allow getsockname on connected x11 plugs
      (LP: #1574526)
    - daemon,overlord/snapstate: read name out of sideloaded snap early,
      improved change summary
    - overlord: keep tasks unlinked from a change hidden, prune them
    - integration-tests: snap list on fresh boot is good again
    - integration-tests: add partial term to the find test
    - integration-tests: changed default release to 16
    - integration-tests: add regression test for snaps not present after
      reboot
    - integration-tests: network interface
    - integration-tests: add proxy related environment variables to
      snapd env file
    - README.md: snappy => snap
    - etc: trivial typo fix (LP:#1569892)
    - debian: remove unneeded /var/lib/snapd/apparmor/additional
      directory (LP: #1569577)

 -- Michael Vogt <email address hidden> Tue, 03 May 2016 07:51:57 +0200

Changed in snapd (Ubuntu Xenial):
status: Fix Committed → Fix Released

The verification of the Stable Release Update for snapd has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Launchpad Janitor (janitor) wrote :
Download full text (6.5 KiB)

This bug was fixed in the package snapd - 2.11+16.10

---------------
snapd (2.11+16.10) yakkety; urgency=medium

  * New upstream release: LP: #1605303
    - increase version number to reflect the nature of the update
      better
    - store, daemon, client, cmd/snap, docs/rest.md: adieu search
      grammar
    - debian: move snapd.refresh.timer into timers.target
    - snapstate: add daemon-reload to fix autopkgtest on yakkety
    - Interfaces: hardware-observe
    - snap: rework the output after a snap operation
    - daemon, cmd/snap: refresh --devmode
    - store, daemon, client, cmd/snap: implement `snap find --private`
    - tests: add network-observe interface spread test
    - interfaces/builtin: allow getsockopt for connected x11 plugs
    - osutil: check for nogrup instead of adm
    - store: small cleanups (more needed)
    - snap/squashfs: fix test not to hardcode snap size
    - client,cmd/snap: cleanup cmd/snap test suite, add extra args
      testThis cleans up the cmd/snap test suite:
    - wrappers: map "never" restart condition to "no."
    - wrappers: run update-desktop-database after add/remove of desktop
      files
    - release: work around elementary mistake
    - many: remove all traces of channel from the buying codepath
    - store: kill setUbuntuStoreHeaders
    - docs: add payment methods documentation
    - many: present user with a choice of payment backends
    - asserts: add cross checks for snap asserts
    - cmd/snap,cmd/snap-exec: support running hooks via snap-exec.
    - tests: improve snap run symlink tests
    - tests: add content sharing interface spread test
    - store & many: a mechanical branch shortening store names
    - snappy: remove old snappy pkg
    - overlord/snapstate: kill flagscompat
    - overlord/snapstate, daemon, client, cmd/snap: devmode override
      (aka confined)
    - tests: extend refresh test to talk to the staging and production
      stores
    - asserts,daemon: cross checks for account and account-key
      assertions
    - client: existing JSON fixtures uses tabs for indentation
    - snap-exec: add proper integration test for snap-exec
    - spread.yaml, tests: replace hello-world with test-snapd-tools
    - tests: add locale-control interface spread test
    - tests: add mount-observe interface spread test
    - tests: add system-observe interface spread test
    - many: add AuthContext to mediate user updates to the state
    - store/auth: add helper for the macaroon refresh endpoint
    - cmd: add buy command
    - overlord: switch snapstate.Update to use ListRefresh (aka
      /snaps/metadata)
    - snap-exec: fix silly off-by-one error
    - tests: stop using hello-world.echo in the tests
    - tests: add env command to test-snapd-tools
    - classic: remove (most of) "classic" mode, this is implemented as a
      snap now
    - many: remove snapstate.Candidate and other cleanups
    - many: removed authenticator, store gets a user instead
    - asserts: fix minor doc comment typo
    - snap: ensure unknown arguments to `snap run` are ignored
    - overlord/auth: add Device/SetDevice to persist device identity in
      state
    - overlord: make SyncBoot work aga...

Read more...

Changed in snapd (Ubuntu):
status: Fix Committed → Fix Released
Michael Vogt (mvo) on 2016-11-29
Changed in snappy:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers