diff --git a/interfaces/builtin/desktop.go b/interfaces/builtin/desktop.go index 0a0de04cd40..d41d90eb191 100644 --- a/interfaces/builtin/desktop.go +++ b/interfaces/builtin/desktop.go @@ -139,7 +139,7 @@ dbus (send) bus=session path=/io/snapcraft/Launcher interface=io.snapcraft.Launcher - member=OpenURL + member={OpenURL,OpenFile} peer=(label=unconfined), # Allow checking status, activating and locking the screensaver diff --git a/interfaces/builtin/unity7.go b/interfaces/builtin/unity7.go index d8f5d8f23b7..b7497b20824 100644 --- a/interfaces/builtin/unity7.go +++ b/interfaces/builtin/unity7.go @@ -112,7 +112,7 @@ dbus (send) bus=session path=/io/snapcraft/Launcher interface=io.snapcraft.Launcher - member=OpenURL + member={OpenURL,OpenFile} peer=(label=unconfined), # Allow use of snapd's internal 'xdg-settings' diff --git a/osutil/sys/syscall.go b/osutil/sys/syscall.go index 23a87445983..a4b13fee18a 100644 --- a/osutil/sys/syscall.go +++ b/osutil/sys/syscall.go @@ -95,3 +95,15 @@ func FchownAt(dirfd uintptr, path string, uid UserID, gid GroupID, flags int) er } return errno } + +// As of Go 1.9, the O_PATH constant does not seem to be declared +// uniformly over all archtiectures. +const O_PATH = 0x200000 + +func FcntlGetFl(fd int) (int, error) { + flags, _, errno := syscall.Syscall(syscall.SYS_FCNTL, uintptr(fd), uintptr(syscall.F_GETFL), 0) + if errno != 0 { + return 0, errno + } + return int(flags), nil +} diff --git a/userd/helpers.go b/userd/helpers.go index bbfbc353b1d..b4de72e9692 100644 --- a/userd/helpers.go +++ b/userd/helpers.go @@ -42,6 +42,13 @@ func snapFromSenderImpl(conn *dbus.Conn, sender dbus.Sender) (string, error) { if err != nil { return "", fmt.Errorf("cannot find snap for connection: %v", err) } + // Check that the sender is still connected to the bus: if it + // has disconnected between the GetConnectionUnixProcessID + // call and when we poked around in /proc, then it is possible + // that the process ID was reused. + if !nameHasOwner(conn, sender) { + return "", fmt.Errorf("sender is no longer connected to the bus") + } return snap, nil } @@ -54,6 +61,16 @@ func connectionPid(conn *dbus.Conn, sender dbus.Sender) (pid int, err error) { return pid, nil } +func nameHasOwner(conn *dbus.Conn, sender dbus.Sender) bool { + call := conn.BusObject().Call("org.freedesktop.DBus.NameHasOwner", 0, sender) + if call.Err != nil { + return false + } + var hasOwner bool + call.Store(&hasOwner) + return hasOwner +} + // FIXME: move to osutil? func snapFromPid(pid int) (string, error) { f, err := os.Open(fmt.Sprintf("%s/proc/%d/cgroup", dirs.GlobalRootDir, pid)) diff --git a/userd/launcher.go b/userd/launcher.go index 4d4e7fc314b..13bd06e76b1 100644 --- a/userd/launcher.go +++ b/userd/launcher.go @@ -22,10 +22,16 @@ package userd import ( "fmt" "net/url" + "os" "os/exec" + "syscall" + "time" "github.com/godbus/dbus" + "github.com/snapcore/snapd/i18n" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/strutil" + "github.com/snapcore/snapd/userd/ui" ) const launcherIntrospectionXML = ` @@ -40,6 +46,10 @@ const launcherIntrospectionXML = ` + + + + ` var ( @@ -93,3 +103,85 @@ func (s *Launcher) OpenURL(addr string) *dbus.Error { return nil } + +// fdToFilename determines the path associated with an open file descriptor. +// +// The file descriptor cannot be opened using O_PATH and must refer to +// a regular file or to a directory. The symlink at /proc/self/fd/ +// is read to determine the filename. The descriptor is also fstat'ed +// and the resulting device number and inode number are compared to +// stat on the path determined earlier. The numbers must match. +func fdToFilename(fd int) (string, error) { + flags, err := sys.FcntlGetFl(fd) + if err != nil { + return "", err + } + // File descriptors opened with O_PATH do not imply access to + // the file in question. + if flags&sys.O_PATH != 0 { + return "", fmt.Errorf("cannot use file descriptors opened using O_PATH") + } + + // Determine the file name associated with the passed file descriptor. + filename, err := os.Readlink(fmt.Sprintf("/proc/self/fd/%d", fd)) + if err != nil { + return "", err + } + + var fileStat, fdStat syscall.Stat_t + if err := syscall.Stat(filename, &fileStat); err != nil { + return "", err + } + if err := syscall.Fstat(fd, &fdStat); err != nil { + return "", err + } + + // Sanity check to ensure we've got the right file + if fdStat.Dev != fileStat.Dev || fdStat.Ino != fileStat.Ino { + return "", fmt.Errorf("cannot determine file name") + } + + fileType := fileStat.Mode & syscall.S_IFMT + if fileType != syscall.S_IFREG && fileType != syscall.S_IFDIR { + return "", fmt.Errorf("cannot open anything other than regular files or directories") + } + + return filename, nil +} + +func (s *Launcher) OpenFile(parentWindow string, clientFd dbus.UnixFD, sender dbus.Sender) *dbus.Error { + // godbus transfers ownership of this file descriptor to us + fd := int(clientFd) + defer syscall.Close(fd) + + filename, err := fdToFilename(fd) + if err != nil { + return dbus.MakeFailedError(err) + } + + snap, err := snapFromSender(s.conn, sender) + if err != nil { + return dbus.MakeFailedError(err) + } + dialog, err := ui.New() + if err != nil { + return dbus.MakeFailedError(err) + } + answeredYes := dialog.YesNo( + i18n.G("Allow opening file?"), + fmt.Sprintf(i18n.G("Allow snap %q to open file %q?"), snap, filename), + &ui.DialogOptions{ + Timeout: 5 * 60 * time.Second, + Footer: i18n.G("This dialog will close automatically after 5 minutes of inactivity."), + }, + ) + if !answeredYes { + return dbus.MakeFailedError(fmt.Errorf("permission denied")) + } + + if err = exec.Command("xdg-open", filename).Run(); err != nil { + return dbus.MakeFailedError(fmt.Errorf("cannot open supplied URL")) + } + + return nil +} diff --git a/userd/launcher_test.go b/userd/launcher_test.go index 6dacea358f1..73540a5472f 100644 --- a/userd/launcher_test.go +++ b/userd/launcher_test.go @@ -20,12 +20,17 @@ package userd_test import ( + "io/ioutil" + "os" + "path/filepath" + "syscall" "testing" "github.com/godbus/dbus" . "gopkg.in/check.v1" + "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/testutil" "github.com/snapcore/snapd/userd" ) @@ -35,7 +40,8 @@ func Test(t *testing.T) { TestingT(t) } type launcherSuite struct { launcher *userd.Launcher - mockXdgOpen *testutil.MockCmd + mockXdgOpen *testutil.MockCmd + restoreSnapFromSender func() } var _ = Suite(&launcherSuite{}) @@ -43,10 +49,14 @@ var _ = Suite(&launcherSuite{}) func (s *launcherSuite) SetUpTest(c *C) { s.launcher = &userd.Launcher{} s.mockXdgOpen = testutil.MockCommand(c, "xdg-open", "") + s.restoreSnapFromSender = userd.MockSnapFromSender(func(*dbus.Conn, dbus.Sender) (string, error) { + return "some-snap", nil + }) } func (s *launcherSuite) TearDownTest(c *C) { s.mockXdgOpen.Restore() + s.restoreSnapFromSender() } func (s *launcherSuite) TestOpenURLWithNotAllowedScheme(c *C) { @@ -83,3 +93,98 @@ func (s *launcherSuite) TestOpenURLWithFailingXdgOpen(c *C) { c.Assert(err, NotNil) c.Assert(err, ErrorMatches, "cannot open supplied URL") } + +func (s *launcherSuite) TestOpenFileUserAccepts(c *C) { + mockZenity := testutil.MockCommand(c, "zenity", "true") + defer mockZenity.Restore() + + path := filepath.Join(c.MkDir(), "test.txt") + c.Assert(ioutil.WriteFile(path, []byte("Hello world"), 0644), IsNil) + + file, err := os.Open(path) + c.Assert(err, IsNil) + defer file.Close() + + dupFd, err := syscall.Dup(int(file.Fd())) + c.Assert(err, IsNil) + + err = s.launcher.OpenFile("", dbus.UnixFD(dupFd), ":some-dbus-sender") + c.Assert(err, IsNil) + c.Assert(s.mockXdgOpen.Calls(), DeepEquals, [][]string{ + {"xdg-open", path}, + }) +} + +func (s *launcherSuite) TestOpenFileUserDeclines(c *C) { + mockZenity := testutil.MockCommand(c, "zenity", "false") + defer mockZenity.Restore() + + path := filepath.Join(c.MkDir(), "test.txt") + c.Assert(ioutil.WriteFile(path, []byte("Hello world"), 0644), IsNil) + + file, err := os.Open(path) + c.Assert(err, IsNil) + defer file.Close() + + dupFd, err := syscall.Dup(int(file.Fd())) + c.Assert(err, IsNil) + + err = s.launcher.OpenFile("", dbus.UnixFD(dupFd), ":some-dbus-sender") + c.Assert(err, NotNil) + c.Assert(err, ErrorMatches, "permission denied") + c.Assert(s.mockXdgOpen.Calls(), IsNil) +} + +func (s *launcherSuite) TestOpenFileSucceedsWithDirectory(c *C) { + mockZenity := testutil.MockCommand(c, "zenity", "true") + defer mockZenity.Restore() + + dir := c.MkDir() + fd, err := syscall.Open(dir, syscall.O_RDONLY|syscall.O_DIRECTORY, 0755) + c.Assert(err, IsNil) + defer syscall.Close(fd) + + dupFd, err := syscall.Dup(fd) + c.Assert(err, IsNil) + + err = s.launcher.OpenFile("", dbus.UnixFD(dupFd), ":some-dbus-sender") + c.Assert(err, IsNil) + c.Assert(s.mockXdgOpen.Calls(), DeepEquals, [][]string{ + {"xdg-open", dir}, + }) +} + +func (s *launcherSuite) TestOpenFileFailsWithDeviceFile(c *C) { + mockZenity := testutil.MockCommand(c, "zenity", "true") + defer mockZenity.Restore() + + file, err := os.Open("/dev/null") + c.Assert(err, IsNil) + defer file.Close() + + dupFd, err := syscall.Dup(int(file.Fd())) + c.Assert(err, IsNil) + + err = s.launcher.OpenFile("", dbus.UnixFD(dupFd), ":some-dbus-sender") + c.Assert(err, NotNil) + c.Assert(err, ErrorMatches, "cannot open anything other than regular files or directories") + c.Assert(s.mockXdgOpen.Calls(), IsNil) +} + +func (s *launcherSuite) TestOpenFileFailsWithPathDescriptor(c *C) { + mockZenity := testutil.MockCommand(c, "zenity", "true") + defer mockZenity.Restore() + + dir := c.MkDir() + fd, err := syscall.Open(dir, sys.O_PATH, 0755) + c.Assert(err, IsNil) + defer syscall.Close(fd) + + dupFd, err := syscall.Dup(fd) + c.Assert(err, IsNil) + + err = s.launcher.OpenFile("", dbus.UnixFD(dupFd), ":some-dbus-sender") + c.Assert(err, NotNil) + c.Assert(err, ErrorMatches, "cannot use file descriptors opened using O_PATH") + c.Assert(s.mockXdgOpen.Calls(), IsNil) +}