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

[Michael Vogt]

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)]

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...

[Michael Vogt (mvo)]

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

[Zygmunt Krynicki (zyga)]

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

[Zygmunt Krynicki (zyga)]

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.

[Zygmunt Krynicki (zyga)]

This bug is fixed by: https://github.com/ubuntu-core/snappy/pull/1049

[Adam Conrad (adconrad)]

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!

[Launchpad Janitor (janitor)]

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

[Steve Langasek (vorlon)]

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)]

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...