interfaces/apparmor: expand @{APP_PKGNAME} to just snap name #840

Merged
merged 1 commit into from Apr 8, 2016

Conversation

Projects
None yet
4 participants
Contributor

zyga commented Apr 8, 2016

This patch drops the .developer suffix from APP_PKGNAME since this is
the current filesystem layout.

NOTE: this was tested with actual device. :-)

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

interfaces/apparmor: expand @{APP_PKGNAME} to just snap name
This patch drops the .developer suffix from APP_PKGNAME since this is
the current filesystem layout.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Collaborator

mvo5 commented Apr 8, 2016

👍 makes me wonder a bit why old-security did not break. but maybe we updated that already and I forgot :)

Contributor

zyga commented Apr 8, 2016

@mvo5 old-security is not a real interface and I suspect that we did change the old code at the time

Contributor

zyga commented Apr 8, 2016

Pinging @jdstrand for review

Contributor

niemeyer commented Apr 8, 2016

LGTM

@zyga zyga merged commit aa28532 into snapcore:master Apr 8, 2016

2 of 4 checks passed

Integration tests
Details
autopkgtest
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.007%) to 76.451%
Details
@@ -280,7 +280,7 @@ const commonPrefix = `
@{APP_APPNAME}="smbd"
@{APP_ID_DBUS}="samba_2eacme_5fsmbd_5f1"
@jdstrand

jdstrand Apr 8, 2016

Contributor

This should be changed too.

@@ -280,7 +280,7 @@ const commonPrefix = `
@{APP_APPNAME}="smbd"
@{APP_ID_DBUS}="samba_2eacme_5fsmbd_5f1"
@{APP_PKGNAME_DBUS}="samba_2eacme"
@jdstrand

jdstrand Apr 8, 2016

Contributor

ditto

@@ -54,7 +54,7 @@ func legacyVariables(appInfo *snap.AppInfo) []byte {
fmt.Fprintf(&buf, "@{APP_PKGNAME_DBUS}=\"%s\"\n",
@jdstrand

jdstrand Apr 8, 2016

Contributor

APP_PKGNAME_DBUS and APP_ID_DBUS should be changed as well.

Contributor

jdstrand commented Apr 8, 2016

I see that snap/info.go has a modified SecurityTag() that does:
return fmt.Sprintf("snap.%s.%s", app.Snap.Name(), app.Name)

As such, PROFILEATTACH is going to be snap.%s.%s. That means that any apparmor rules that use the profile attach label (ie, signal, ptrace, unix, dbus) are now broken between that change and this change. Ie, in template.go:

unix peer=(label=@{APP_PKGNAME}_*),
signal peer=@{APP_PKGNAME}_*,

These need to be changed to:

unix peer=(label=snap.@{APP_PKGNAME}.*),
signal peer=snap.@{APP_PKGNAME}.*,

Note I both prepended 'snap.' and changed the '_' to '.'

The current builtins are all ok, but future policy like bluez will need to use similar rules.

Contributor

zyga commented Apr 8, 2016

@jdstrand thanks for pointing out the subtle tweak in apparmor rules. I'll keep an eye out for this in bluez reviews. I'll iterate with updates to variables but I'd like to actually drop existing variables and replace them with just SNAP_NAME SNAP_REVISION and not much else. Is that doable?

Contributor

jdstrand commented Apr 8, 2016

With modern variables use:

unix peer=(label=snap.@{SNAP_NAME}.*),
signal peer=snap.@{SNAP_NAME}.*,
Contributor

jdstrand commented Apr 8, 2016

In fact, with that change you can remove legacyVariables altogether.

Contributor

jdstrand commented Apr 8, 2016

Oh wait, I spoke too soon. Don't remove legacyVariables yet-- the file rules still need to be updated for modern variables.

@zyga zyga deleted the zyga:ifaces-apparmor-template branch Dec 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment