many: split public snapd REST API into separate socket.

client/client.go

@@ -30,13 +30,28 @@ import (
 	"net/url"
 	"os"
 	"path"
+	"syscall"
 	"time"
 
 	"github.com/snapcore/snapd/dirs"
 )
 
-func unixDialer(_, _ string) (net.Conn, error) {
-	return net.Dial("unix", dirs.SnapdSocket)
+func unixDialer() func(string, string) (net.Conn, error) {
+	// We have two sockets available: the SnapdSocket (which provides
+	// administrative access), and the SnapSocket (which doesn't). Use the most
+	// powerful one available (e.g. from within snaps, SnapdSocket is hidden by
+	// apparmor unless the snap has the snapd-control interface).
+	socketPath := dirs.SnapdSocket
+	file, err := os.OpenFile(socketPath, os.O_RDWR, 0666)
+	if err == nil {
+		file.Close()
+	} else if e, ok := err.(*os.PathError); ok && (e.Err == syscall.ENOENT || e.Err == syscall.EACCES) {
+		socketPath = dirs.SnapSocket
+	}
+
+	return func(_, _ string) (net.Conn, error) {
+		return net.Dial("unix", socketPath)
+	}
 }
 
 type doer interface {
@@ -66,7 +81,7 @@ func New(config *Config) *Client {
 				Host:   "localhost",
 			},
 			doer: &http.Client{
-				Transport: &http.Transport{Dial: unixDialer},
+				Transport: &http.Transport{Dial: unixDialer()},
 			},
 		}
 	}

client/client_test.go

@@ -30,12 +30,12 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"gopkg.in/check.v1"
 
 	"github.com/snapcore/snapd/client"
 	"github.com/snapcore/snapd/dirs"
-	"time"
 )
 
 // Hook up check.v1 into the "go test" runner
@@ -175,7 +175,7 @@ func (cs *clientSuite) TestServerVersion(c *check.C) {
 	})
 }
 
-func (cs *clientSuite) TestClientIntegration(c *check.C) {
+func (cs *clientSuite) TestSnapdClientIntegration(c *check.C) {
 	c.Assert(os.MkdirAll(filepath.Dir(dirs.SnapdSocket), 0755), check.IsNil)
 	l, err := net.Listen("unix", dirs.SnapdSocket)
 	if err != nil {
@@ -202,6 +202,39 @@ func (cs *clientSuite) TestClientIntegration(c *check.C) {
 	c.Check(si.Series, check.Equals, "42")
 }
 
+func (cs *clientSuite) TestSnapClientIntegration(c *check.C) {
+	c.Assert(os.MkdirAll(filepath.Dir(dirs.SnapSocket), 0755), check.IsNil)
+	l, err := net.Listen("unix", dirs.SnapSocket)
+	if err != nil {
+		c.Fatalf("unable to listen on %q: %v", dirs.SnapSocket, err)
+	}
+
+	f := func(w http.ResponseWriter, r *http.Request) {
+		c.Check(r.URL.Path, check.Equals, "/v2/snapctl")
+		c.Check(r.URL.RawQuery, check.Equals, "")
+
+		fmt.Fprintln(w, `{"type":"sync", "result":{"stdout":"test stdout","stderr":"test stderr"}}`)
+	}
+
+	srv := &httptest.Server{
+		Listener: l,
+		Config:   &http.Server{Handler: http.HandlerFunc(f)},
+	}
+	srv.Start()
+	defer srv.Close()
+
+	cli := client.New(nil)
+	options := client.SnapCtlOptions{
+		Context: "foo",
+		Args:    []string{"bar", "--baz"},
+	}
+
+	stdout, stderr, err := cli.RunSnapctl(options)
+	c.Check(err, check.IsNil)
+	c.Check(string(stdout), check.Equals, "test stdout")
+	c.Check(string(stderr), check.Equals, "test stderr")
+}
+
 func (cs *clientSuite) TestClientReportsOpError(c *check.C) {
 	cs.rsp = `{"type": "error", "status": "potatoes"}`
 	_, err := cs.cli.SysInfo()

cmd/snap/cmd_run.go

@@ -118,7 +118,6 @@ func getSnapInfo(snapName string, revision snap.Revision) (*snap.Info, error) {
 func snapExecEnv(info *snap.Info) []string {
 	env := snapenv.Basic(info)
 	env = append(env, snapenv.User(info, os.Getenv("HOME"))...)
-	env = append(env, "PATH=${PATH}:/usr/lib/snapd")
 	return env
 }
 

cmd/snap/cmd_run_test.go

@@ -75,7 +75,6 @@ func (s *SnapSuite) TestSnapRunSnapExecEnv(c *check.C) {
 	env := snaprun.SnapExecEnv(info)
 	sort.Strings(env)
 	c.Check(env, check.DeepEquals, []string{
-		"PATH=${PATH}:/usr/lib/snapd",
 		fmt.Sprintf("SNAP=%s/snapname/42", dirs.SnapMountDir),
 		fmt.Sprintf("SNAP_ARCH=%s", arch.UbuntuArchitecture()),
 		"SNAP_COMMON=/var/snap/snapname/common",

daemon/api.go

@@ -193,7 +193,7 @@ var (
 
 	snapctlCmd = &Command{
 		Path:   "/v2/snapctl",
-		UserOK: false,
+		SnapOK: true,
 		POST:   runSnapctl,
 	}
 )

daemon/daemon.go

@@ -30,6 +30,7 @@ import (
 	"github.com/gorilla/mux"
 	"gopkg.in/tomb.v2"
 
+	"github.com/snapcore/snapd/dirs"
 	"github.com/snapcore/snapd/logger"
 	"github.com/snapcore/snapd/notifications"
 	"github.com/snapcore/snapd/overlord"
@@ -38,12 +39,13 @@ import (
 
 // A Daemon listens for requests and routes them to the right command
 type Daemon struct {
-	Version  string
-	overlord *overlord.Overlord
-	listener net.Listener
-	tomb     tomb.Tomb
-	router   *mux.Router
-	hub      *notifications.Hub
+	Version       string
+	overlord      *overlord.Overlord
+	snapdListener net.Listener
+	snapListener  net.Listener
+	tomb          tomb.Tomb
+	router        *mux.Router
+	hub           *notifications.Hub
 	// enableInternalInterfaceActions controls if adding and removing slots and plugs is allowed.
 	enableInternalInterfaceActions bool
 }
@@ -63,7 +65,9 @@ type Command struct {
 	GuestOK bool
 	// can non-admin GET?
 	UserOK bool
-	//
+	// is this path accessible on the snapd-snap socket?
+	SnapOK bool
+
 	d *Daemon
 }
 
@@ -74,13 +78,19 @@ func (c *Command) canAccess(r *http.Request, user *auth.UserState) bool {
 	}
 
 	isUser := false
-	if uid, err := ucrednetGetUID(r.RemoteAddr); err == nil {
+	uid, err := ucrednetGetUID(r.RemoteAddr)
+	if err == nil {
 		if uid == 0 {
 			// Superuser does anything.
 			return true
 		}
 
 		isUser = true
+	} else if err != errNoUID {
+		logger.Noticef("unexpected error when attempting to get UID: %s", err)
+		return false
+	} else if c.SnapOK {
+		return true
 	}
 
 	if r.Method != "GET" {
@@ -171,11 +181,21 @@ func (d *Daemon) Init() error {
 		return err
 	}
 
-	if len(listeners) != 1 {
-		return fmt.Errorf("daemon does not handle %d listeners right now, just one", len(listeners))
+	if len(listeners) != 2 {
+		return fmt.Errorf("daemon does not handle %d listeners right now, only two", len(listeners))
+	}
+
+	listenerMap := map[string]net.Listener{
+		listeners[0].Addr().String(): listeners[0],
+		listeners[1].Addr().String(): listeners[1],
 	}
 
-	d.listener = &ucrednetListener{listeners[0]}
+	d.snapdListener = &ucrednetListener{listenerMap[dirs.SnapdSocket]}
+
+	// Note that the SnapSocket listener does not use ucrednet. We use the lack
+	// of remote information as an indication that the request originated with
+	// this socket.
+	d.snapListener = listenerMap[dirs.SnapSocket]
 
 	d.addRoutes()
 
@@ -207,8 +227,17 @@ func (d *Daemon) Start() {
 
 	// the loop runs in its own goroutine
 	d.overlord.Loop()
+
 	d.tomb.Go(func() error {
-		if err := http.Serve(d.listener, logit(d.router)); err != nil && d.tomb.Err() == tomb.ErrStillAlive {
+		d.tomb.Go(func() error {
+			if err := http.Serve(d.snapListener, logit(d.router)); err != nil && d.tomb.Err() == tomb.ErrStillAlive {
+				return err
+			}
+
+			return nil
+		})
+
+		if err := http.Serve(d.snapdListener, logit(d.router)); err != nil && d.tomb.Err() == tomb.ErrStillAlive {
 			return err
 		}
 
@@ -219,8 +248,10 @@ func (d *Daemon) Start() {
 // Stop shuts down the Daemon
 func (d *Daemon) Stop() error {
 	d.tomb.Kill(nil)
-	d.listener.Close()
+	d.snapdListener.Close()
+	d.snapListener.Close()
 	d.overlord.Stop()
+
 	return d.tomb.Wait()
 }
 

daemon/daemon_test.go

@@ -135,6 +135,15 @@ func (s *daemonSuite) TestGuestAccess(c *check.C) {
 	c.Check(cmd.canAccess(put, nil), check.Equals, false)
 	c.Check(cmd.canAccess(pst, nil), check.Equals, false)
 	c.Check(cmd.canAccess(del, nil), check.Equals, false)
+
+	// Since this request has no RemoteAddr, it must be coming from the snap
+	// socket instead of the snapd one. In that case, if SnapOK is true, this
+	// command should be wide open for all HTTP methods.
+	cmd = &Command{d: newTestDaemon(c), SnapOK: true}
+	c.Check(cmd.canAccess(get, nil), check.Equals, true)
+	c.Check(cmd.canAccess(put, nil), check.Equals, true)
+	c.Check(cmd.canAccess(pst, nil), check.Equals, true)
+	c.Check(cmd.canAccess(del, nil), check.Equals, true)
 }
 
 func (s *daemonSuite) TestUserAccess(c *check.C) {
@@ -152,6 +161,13 @@ func (s *daemonSuite) TestUserAccess(c *check.C) {
 	cmd = &Command{d: newTestDaemon(c), GuestOK: true}
 	c.Check(cmd.canAccess(get, nil), check.Equals, true)
 	c.Check(cmd.canAccess(put, nil), check.Equals, false)
+
+	// Since this request has a RemoteAddr, it must be coming from the snapd
+	// socket instead of the snap one. In that case, SnapOK should have no
+	// bearing on the default behavior, which is to deny access.
+	cmd = &Command{d: newTestDaemon(c), SnapOK: true}
+	c.Check(cmd.canAccess(get, nil), check.Equals, false)
+	c.Check(cmd.canAccess(put, nil), check.Equals, false)
 }
 
 func (s *daemonSuite) TestSuperAccess(c *check.C) {
@@ -169,6 +185,10 @@ func (s *daemonSuite) TestSuperAccess(c *check.C) {
 	cmd = &Command{d: newTestDaemon(c), GuestOK: true}
 	c.Check(cmd.canAccess(get, nil), check.Equals, true)
 	c.Check(cmd.canAccess(put, nil), check.Equals, true)
+
+	cmd = &Command{d: newTestDaemon(c), SnapOK: true}
+	c.Check(cmd.canAccess(get, nil), check.Equals, true)
+	c.Check(cmd.canAccess(put, nil), check.Equals, true)
 }
 
 func (s *daemonSuite) TestAddRoutes(c *check.C) {
@@ -207,14 +227,37 @@ func (s *daemonSuite) TestStartStop(c *check.C) {
 	l, err := net.Listen("tcp", "127.0.0.1:0")
 	c.Assert(err, check.IsNil)
 
-	accept := make(chan struct{})
-	d.listener = &witnessAcceptListener{l, accept}
+	snapdAccept := make(chan struct{})
+	d.snapdListener = &witnessAcceptListener{l, snapdAccept}
+
+	snapAccept := make(chan struct{})
+	d.snapListener = &witnessAcceptListener{l, snapAccept}
+
 	d.Start()
-	select {
-	case <-accept:
-	case <-time.After(2 * time.Second):
-		c.Fatal("Accept was not called")
-	}
+
+	snapdDone := make(chan struct{})
+	go func() {
+		select {
+		case <-snapdAccept:
+		case <-time.After(2 * time.Second):
+			c.Fatal("snapd accept was not called")
+		}
+		close(snapdDone)
+	}()
+
+	snapDone := make(chan struct{})
+	go func() {
+		select {
+		case <-snapAccept:
+		case <-time.After(2 * time.Second):
+			c.Fatal("snapd accept was not called")
+		}
+		close(snapDone)
+	}()
+
+	<-snapdDone
+	<-snapDone
+
 	err = d.Stop()
 	c.Check(err, check.IsNil)
 }
@@ -224,15 +267,37 @@ func (s *daemonSuite) TestRestartWiring(c *check.C) {
 	l, err := net.Listen("tcp", "127.0.0.1:0")
 	c.Assert(err, check.IsNil)
 
-	accept := make(chan struct{})
-	d.listener = &witnessAcceptListener{l, accept}
+	snapdAccept := make(chan struct{})
+	d.snapdListener = &witnessAcceptListener{l, snapdAccept}
+
+	snapAccept := make(chan struct{})
+	d.snapListener = &witnessAcceptListener{l, snapAccept}
+
 	d.Start()
 	defer d.Stop()
-	select {
-	case <-accept:
-	case <-time.After(2 * time.Second):
-		c.Fatal("Accept was not called")
-	}
+
+	snapdDone := make(chan struct{})
+	go func() {
+		select {
+		case <-snapdAccept:
+		case <-time.After(2 * time.Second):
+			c.Fatal("snapd accept was not called")
+		}
+		close(snapdDone)
+	}()
+
+	snapDone := make(chan struct{})
+	go func() {
+		select {
+		case <-snapAccept:
+		case <-time.After(2 * time.Second):
+			c.Fatal("snap accept was not called")
+		}
+		close(snapDone)
+	}()
+
+	<-snapdDone
+	<-snapDone
 
 	d.overlord.State().RequestRestart()
 

debian/snapd.install

@@ -1,5 +1,5 @@
 /usr/bin/snap
-/usr/bin/snapctl usr/lib/snapd
+/usr/bin/snapctl
 /usr/bin/snapd usr/lib/snapd
 /usr/bin/snap-exec usr/lib/snapd
 data/completion/snap /usr/share/bash-completion/completions/

debian/snapd.socket

@@ -3,6 +3,7 @@ Description=Socket activation for snappy daemon
 
 [Socket]
 ListenStream=/run/snapd.socket
+ListenStream=/run/snapd-snap.socket
 SocketMode=0666
 # these are the defaults, but can't hurt to specify them anyway:
 SocketUser=root