userd: add an OpenFile method for launching local files with xdg-open (…

…#4766)

* userd: add OpenFile D-Bus method for opening local files

* interfaces: allow OpenFile method in cases where OpenURL was allowed

* userd: make changes requested by zyga

* userd: add a parentWindow argument, as requested by mvo

* userd: stat the filename first, as requested by jdstrand

* userd: check that the sender is still connected to the bus after looking
up process information

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

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'

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
+}

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

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 = `
 	<method name='OpenURL'>
 		<arg type='s' name='url' direction='in'/>
 	</method>
+	<method name="OpenFile">
+		<arg type="s" name="parent_window" direction="in"/>
+		<arg type="h" name="fd" direction="in"/>
+	</method>
 </interface>`
 
 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/<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
+}

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,18 +40,23 @@ func Test(t *testing.T) { TestingT(t) }
 type launcherSuite struct {
 	launcher *userd.Launcher
 
-	mockXdgOpen *testutil.MockCmd
+	mockXdgOpen           *testutil.MockCmd
+	restoreSnapFromSender func()
 }
 
 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)
+}