interfaces/apparmor: allow reading from ecryptfs #2837

Closed
wants to merge 12 commits into
from
@@ -49,6 +49,7 @@ import (
"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/interfaces"
+ "github.com/snapcore/snapd/interfaces/mount"
"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/osutil"
"github.com/snapcore/snapd/release"
@@ -245,6 +246,14 @@ func addContent(securityTag string, snapInfo *snap.Info, opts interfaces.Confine
} else {
tagSnippets = snippetForTag
}
+ if opts.TryMode && anyEncryptedDirectory() {
+ // Add a special internal snippet for snaps that are in try
+ // mode on an encrypted home directory. This snippet provides
+ // access to the encrypted files for users other than the owner
+ // and thus allows daemons, hooks and snap commands to run
+ // under sudo.
+ tagSnippets += encryptedHomeTrySnippet
+ }
return tagSnippets
}
return ""
@@ -256,6 +265,27 @@ func addContent(securityTag string, snapInfo *snap.Info, opts interfaces.Confine
}
}
+// procSelfMountInfo is exposed as a variable for testing anyEncryptedDirectory.
+var procSelfMountInfo = mount.ProcSelfMountInfo
+
+// anyEncryptedDirectory returns true if ecryptfs is mounted anywhere.
+// TODO: Make this look for the place where the snap is mounted from, once that
+// information becomes available.
+func anyEncryptedDirectory() bool {
+ entries, err := mount.LoadMountInfo(procSelfMountInfo)
+ if err != nil {
+ logger.Noticef("cannot check if any directories are encrypted: %s", err)
+ return false
+ }
+ for _, mie := range entries {
+ // TODO: detect Ubuntu 14.04-style encrypted home directories.
+ if mie.FsType == "ecryptfs" {
+ return true
+ }
+ }
+ return false
+}
+
func reloadProfiles(profiles []string) error {
for _, profile := range profiles {
fname := filepath.Join(dirs.SnapAppArmorDir, profile)
@@ -486,3 +486,49 @@ func (s *backendSuite) TestSetupHostSnapConfineApparmorForReexecWritesNew(c *C)
})
}
+
+func (s *backendSuite) TestAnyEncryptedDirectory(c *C) {
+ // Mock the location of /proc/self/mountinfo for testing.
+ fname := filepath.Join(c.MkDir(), "mountinfo")
+ restorer := apparmor.MockProcSelfMountInfo(fname)
+ defer restorer()
+
+ // With non-existent mountinfo we say nothing is encrypted.
+ c.Assert(apparmor.AnyEncryptedDirectory(), Equals, false)
+
+ // With empty mountinfo we say nothing is encrypted.
+ c.Assert(ioutil.WriteFile(fname, nil, 0644), IsNil)
+ c.Assert(apparmor.AnyEncryptedDirectory(), Equals, false)
+
+ // Whenever we see ecryptfs filesystem we know something is encrypted.
+ err := ioutil.WriteFile(fname, []byte(`668 25 0:52 /test-encrypted /snap/test-encrypted/x1 rw,nosuid,nodev,relatime shared:177 - ecryptfs /home/foo/.Private rw,ecryptfs_fnek_sig=987ee2ca60efd36f,ecryptfs_sig=0a250b04159c25f9,ecryptfs_cipher=aes,ecryptfs_key_bytes=16,ecryptfs_unlink_sigs`), 0644)
+ c.Assert(err, IsNil)
+ c.Assert(apparmor.AnyEncryptedDirectory(), Equals, true)
+}
+
+func (s *backendSuite) TestTryModeSnapsAndEncryptionGetExtraSnippet(c *C) {
+ // Mock the location of /proc/self/mountinfo and place a mount entry
+ // where the filesystem type is "ecryptfs".
+ fname := filepath.Join(c.MkDir(), "mountinfo")
+ restorer := apparmor.MockProcSelfMountInfo(fname)
+ defer restorer()
+ err := ioutil.WriteFile(fname, []byte(`668 25 0:52 /test-encrypted /snap/test-encrypted/x1 rw,nosuid,nodev,relatime shared:177 - ecryptfs /home/foo/.Private rw,ecryptfs_fnek_sig=987ee2ca60efd36f,ecryptfs_sig=0a250b04159c25f9,ecryptfs_cipher=aes,ecryptfs_key_bytes=16,ecryptfs_unlink_sigs`), 0644)
+ c.Assert(err, IsNil)
+
+ // Mock a classic system that is not in forced devmode.
+ restorer = release.MockOnClassic(true)
+ defer restorer()
+ restorer = release.MockForcedDevmode(false)
+ defer restorer()
+
+ // Install a demo snap with confinement options that say it is in try mode.
+ opts := interfaces.ConfinementOptions{TryMode: true}
+ s.InstallSnap(c, opts, ifacetest.SambaYamlV1, 1)
+
+ // Check that the generated apparmor profile allows for access to the
+ // encrypted home directory.
+ profile := filepath.Join(dirs.SnapAppArmorDir, "snap.samba.smbd")
+ data, err := ioutil.ReadFile(profile)
+ c.Assert(err, IsNil)
+ c.Assert(string(data), testutil.Contains, "@{HOMEDIRS}/.ecryptfs/*/.Private/** mrixwlk,")
+}
@@ -23,6 +23,10 @@ import (
"github.com/snapcore/snapd/testutil"
)
+var (
+ AnyEncryptedDirectory = anyEncryptedDirectory
+)
+
// MockProfilesPath mocks the file read by LoadedProfiles()
func MockProfilesPath(t *testutil.BaseTest, profiles string) {
profilesPath = profiles
@@ -47,3 +51,10 @@ func MockClassicTemplate(fakeTemplate string) (restore func()) {
classicTemplate = fakeTemplate
return func() { classicTemplate = orig }
}
+
+// MockProcSelfMountInfo replaces the location of /proc/self/mountinfo.
+func MockProcSelfMountInfo(fakePath string) (restore func()) {
+ orig := procSelfMountInfo
+ procSelfMountInfo = fakePath
+ return func() { procSelfMountInfo = orig }
+}
@@ -408,6 +408,21 @@ var defaultTemplate = `
}
`
+// encryptedHomeTrySnippet contains apparmor snippet that allows for "snap try"
+// operations to work even if the home directory is encrypted.
+var encryptedHomeTrySnippet = `
+ # Workaround policy added by 'snap try' to support ecryptfs and
+ # /var/lib/snapd/snaps/<name>_xN.snap pointing inside user's directory
+ capability dac_override,
+ capability dac_read_search,
+ # encrypted ~/.Private and old-style encrypted $HOME
+ @{HOME}/.Private/ r,
+ @{HOME}/.Private/** mrixwlk,
+ # new-style encrypted $HOME
+ @{HOMEDIRS}/.ecryptfs/*/.Private/ r,
+ @{HOMEDIRS}/.ecryptfs/*/.Private/** mrixwlk,
@jdstrand

jdstrand Feb 13, 2017

Contributor

Looking at the bug, it has:

audit: type=1400 audit(1477673355.022:268): apparmor="DENIED" operation="open" profile="snap.snapctl-hooks.hook.configure" name="/home/.ecryptfs/kyrofa/.Private/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-3yXXfzRq3GXkhDoVeoqkJlBFyivegA--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-7yXXtzRh82OVjVmaBO0JsdHlqK7iEk--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-3yXXfzRq3GXkhDoVeoqkJlBFyivegA--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-7yXXtzRhhrBZUIEB0brHV1omVyCU.k--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-3yXXfzRq3GXkhDoVeoqkJlBFyivegA--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-7yXRhhrBA.pAGc4OcPwbPu1IZnkngk--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-3yXXfzRq3GXkhDoVeoqkJlBFyivegA.Wak--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-7yXXtzRhU.DKB3vJS4NnysPO9.RZ1U--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-3yXXtzRhCckA-2z2BEUruD7rA-mb0U--/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-7yXXtzRhx3A

This output appears truncated, but this path is important: /home/.ecryptfs/kyrofa/.Private/ECRYPTFS_FNEK_ENCRYPTED.FBaOD17IzZ9EsEQj239cjqBho2j-3yXXfzRq3GXkhDoVeoqkJlBFyivegA--/ECRYPTFS. /etc/apparmor.d/abstractions/base has a rule that matches this path:

  # new-style encrypted $HOME
  owner @{HOMEDIRS}/.ecryptfs/*/.Private/** mrixwlk,

Since you said that this is the configure hook, we know that the configure hook runs as root, we see that the rules added to this PR effectively only remove 'owner', and we see that the root process is trying to access encrypted files in /home/.ecryptfs/kyrofa/.Private/.... I'm not sure of the cause, but it seems like the snap in the bug is trying to have a 'daemon' access files in the user's directory. We don't allow non-owner match in the default policy or the home interface now, and we shouldn't allow that in the ecryptfs policy.

@kyrofa - I expect this configure hook will fail on non-encrypted HOME systems. Does it? If not, can you strace the process and see what file it is accessing?

-1 as is without more information.

@kyrofa

kyrofa Feb 13, 2017

Member

@jdstrand the snap in question is contained in the snapd codebase. You can see the configure hook doesn't do anything other than run snapctl a few times. Note also that this works fine if the snap is installed-- the problems come when the snap is being tried. I assume that's where the access to my $HOME is coming from (since the snap actually resides there).

@kyrofa

kyrofa Feb 13, 2017

Member

@jdstrand to answer your question: this works fine on a non-encrypted $HOME. However, things have changed a little since that bug was logged: since the configure hook now runs upon initial install, I get these errors when running snap try instead of snap set. straceing just straces the snap client, where we'd actually want to strace snapd itself. I'm not sure how to do that.

@jdstrand

jdstrand May 4, 2017

Contributor

Please use this:

# Workaround policy added by 'snap try' to support ecryptfs and
# /var/lib/snapd/snaps/<name>_xN.snap pointing inside user's directory
capability dac_override,
capability dac_read_search,
# encrypted ~/.Private and old-style encrypted $HOME
@{HOME}/.Private/ r,
@{HOME}/.Private/** mrixwlk,
# new-style encrypted $HOME
@{HOMEDIRS}/.ecryptfs/*/.Private/ r,
@{HOMEDIRS}/.ecryptfs/*/.Private/** mrixwlk,
@zyga

zyga May 4, 2017

Contributor

Ooops, thanks! Corrected

+`
+
// classicTemplate contains apparmor template used for snaps with classic
// confinement. This template was Designed by jdstrand:
// https://github.com/snapcore/snapd/pull/2366#discussion_r90101320
View
@@ -55,13 +55,18 @@ import (
//
// The Classic flag switches the layout of the mount namespace so that there's
// no "chroot" to the core snap.
+//
+// The TryMode flag indicates that the snap is in try mode and may need extra
+// confinement permissions.
type ConfinementOptions struct {
// DevMode flag switches confinement to non-enforcing mode.
DevMode bool
// JailMode flag switches confinement to enforcing mode.
JailMode bool
// Classic flag switches the core snap "chroot" off.
Classic bool
+ // TryMode means the snap is running from an unpacked directory.
+ TryMode bool
}
// SecurityBackend abstracts interactions between the interface system and the
@@ -42,6 +42,7 @@ func confinementOptions(flags snapstate.Flags) interfaces.ConfinementOptions {
DevMode: flags.DevMode,
JailMode: flags.JailMode,
Classic: flags.Classic,
+ TryMode: flags.TryMode,
}
}