From 2d966df174119830697db982f3f8c4ead5e663e5 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 30 Nov 2017 17:15:28 -0800 Subject: [PATCH] cmd/containerd-shim: require unix socket credentials Signed-off-by: Stephen J Day --- cmd/containerd-shim/main_unix.go | 7 +- cmd/containerd-shim/shim_darwin.go | 30 +++++++ cmd/containerd-shim/shim_linux.go | 4 +- cmd/containerd-shim/shim_unix.go | 6 +- vendor.conf | 2 +- vendor/github.com/stevvooe/ttrpc/config.go | 23 ++++++ vendor/github.com/stevvooe/ttrpc/handshake.go | 34 ++++++++ vendor/github.com/stevvooe/ttrpc/server.go | 51 +++++++++--- vendor/github.com/stevvooe/ttrpc/unixcreds.go | 82 +++++++++++++++++++ 9 files changed, 218 insertions(+), 21 deletions(-) create mode 100644 cmd/containerd-shim/shim_darwin.go create mode 100644 vendor/github.com/stevvooe/ttrpc/config.go create mode 100644 vendor/github.com/stevvooe/ttrpc/handshake.go create mode 100644 vendor/github.com/stevvooe/ttrpc/unixcreds.go diff --git a/cmd/containerd-shim/main_unix.go b/cmd/containerd-shim/main_unix.go index c8df7a5a4..e1996fd92 100644 --- a/cmd/containerd-shim/main_unix.go +++ b/cmd/containerd-shim/main_unix.go @@ -5,7 +5,6 @@ package main import ( "bytes" "context" - "errors" "flag" "fmt" "net" @@ -25,6 +24,7 @@ import ( "github.com/containerd/containerd/reaper" "github.com/containerd/typeurl" ptypes "github.com/gogo/protobuf/types" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/stevvooe/ttrpc" "golang.org/x/sys/unix" @@ -88,7 +88,10 @@ func executeShim() error { if err != nil { return err } - server := newServer() + server, err := newServer() + if err != nil { + return errors.Wrap(err, "failed creating server") + } sv, err := shim.NewService( shim.Config{ Path: path, diff --git a/cmd/containerd-shim/shim_darwin.go b/cmd/containerd-shim/shim_darwin.go new file mode 100644 index 000000000..6f6c92103 --- /dev/null +++ b/cmd/containerd-shim/shim_darwin.go @@ -0,0 +1,30 @@ +// +build darwin + +package main + +import ( + "os" + "os/signal" + + "github.com/containerd/containerd/reaper" + runc "github.com/containerd/go-runc" + "github.com/stevvooe/ttrpc" +) + +// setupSignals creates a new signal handler for all signals and sets the shim as a +// sub-reaper so that the container processes are reparented +func setupSignals() (chan os.Signal, error) { + signals := make(chan os.Signal, 2048) + signal.Notify(signals) + // make sure runc is setup to use the monitor + // for waiting on processes + runc.Monitor = reaper.Default + return signals, nil +} + +func newServer() (*ttrpc.Server, error) { + // for darwin, we omit the socket credentials because these syscalls are + // slightly different. since we don't have darwin support yet, this can be + // implemented later and the build can continue without issue. + return ttrpc.NewServer() +} diff --git a/cmd/containerd-shim/shim_linux.go b/cmd/containerd-shim/shim_linux.go index 73c1b68f9..aa86790bf 100644 --- a/cmd/containerd-shim/shim_linux.go +++ b/cmd/containerd-shim/shim_linux.go @@ -26,6 +26,6 @@ func setupSignals() (chan os.Signal, error) { return signals, nil } -func newServer() *ttrpc.Server { - return ttrpc.NewServer() +func newServer() (*ttrpc.Server, error) { + return ttrpc.NewServer(ttrpc.WithServerHandshaker(ttrpc.UnixSocketRequireSameUser())) } diff --git a/cmd/containerd-shim/shim_unix.go b/cmd/containerd-shim/shim_unix.go index b7aa13e8f..e998ed6e2 100644 --- a/cmd/containerd-shim/shim_unix.go +++ b/cmd/containerd-shim/shim_unix.go @@ -1,4 +1,4 @@ -// +build !linux,!windows +// +build !linux,!windows,!darwin package main @@ -22,6 +22,6 @@ func setupSignals() (chan os.Signal, error) { return signals, nil } -func newServer() *ttrpc.Server { - return ttrpc.NewServer() +func newServer() (*ttrpc.Server, error) { + return ttrpc.NewServer(ttrpc.WithServerHandshaker(ttrpc.UnixSocketRequireSameUser())) } diff --git a/vendor.conf b/vendor.conf index 382aaa66b..dffc7049c 100644 --- a/vendor.conf +++ b/vendor.conf @@ -41,4 +41,4 @@ github.com/boltdb/bolt e9cf4fae01b5a8ff89d0ec6b32f0d9c9f79aefdd google.golang.org/genproto d80a6e20e776b0b17a324d0ba1ab50a39c8e8944 golang.org/x/text 19e51611da83d6be54ddafce4a4af510cb3e9ea4 github.com/dmcgowan/go-tar go1.10 -github.com/stevvooe/ttrpc 8c92e22ce0c492875ccaac3ab06143a77d8ed0c1 +github.com/stevvooe/ttrpc 45d16b41b590938186c5c7cde8088607b3933231 diff --git a/vendor/github.com/stevvooe/ttrpc/config.go b/vendor/github.com/stevvooe/ttrpc/config.go new file mode 100644 index 000000000..23bc603a3 --- /dev/null +++ b/vendor/github.com/stevvooe/ttrpc/config.go @@ -0,0 +1,23 @@ +package ttrpc + +import "github.com/pkg/errors" + +type serverConfig struct { + handshaker Handshaker +} + +type ServerOpt func(*serverConfig) error + +// WithServerHandshaker can be passed to NewServer to ensure that the +// handshaker is called before every connection attempt. +// +// Only one handshaker is allowed per server. +func WithServerHandshaker(handshaker Handshaker) ServerOpt { + return func(c *serverConfig) error { + if c.handshaker != nil { + return errors.New("only one handshaker allowed per server") + } + c.handshaker = handshaker + return nil + } +} diff --git a/vendor/github.com/stevvooe/ttrpc/handshake.go b/vendor/github.com/stevvooe/ttrpc/handshake.go new file mode 100644 index 000000000..a08ae8ee4 --- /dev/null +++ b/vendor/github.com/stevvooe/ttrpc/handshake.go @@ -0,0 +1,34 @@ +package ttrpc + +import ( + "context" + "net" +) + +// Handshaker defines the interface for connection handshakes performed on the +// server or client when first connecting. +type Handshaker interface { + // Handshake should confirm or decorate a connection that may be incoming + // to a server or outgoing from a client. + // + // If this returns without an error, the caller should use the connection + // in place of the original connection. + // + // The second return value can contain credential specific data, such as + // unix socket credentials or TLS information. + // + // While we currently only have implementations on the server-side, this + // interface should be sufficient to implement similar handshakes on the + // client-side. + Handshake(ctx context.Context, conn net.Conn) (net.Conn, interface{}, error) +} + +type handshakerFunc func(ctx context.Context, conn net.Conn) (net.Conn, interface{}, error) + +func (fn handshakerFunc) Handshake(ctx context.Context, conn net.Conn) (net.Conn, interface{}, error) { + return fn(ctx, conn) +} + +func noopHandshake(ctx context.Context, conn net.Conn) (net.Conn, interface{}, error) { + return conn, nil, nil +} diff --git a/vendor/github.com/stevvooe/ttrpc/server.go b/vendor/github.com/stevvooe/ttrpc/server.go index ed2d14cf7..edfca0c52 100644 --- a/vendor/github.com/stevvooe/ttrpc/server.go +++ b/vendor/github.com/stevvooe/ttrpc/server.go @@ -2,6 +2,7 @@ package ttrpc import ( "context" + "io" "math/rand" "net" "sync" @@ -19,6 +20,7 @@ var ( ) type Server struct { + config *serverConfig services *serviceSet codec codec @@ -28,13 +30,21 @@ type Server struct { done chan struct{} // marks point at which we stop serving requests } -func NewServer() *Server { +func NewServer(opts ...ServerOpt) (*Server, error) { + config := &serverConfig{} + for _, opt := range opts { + if err := opt(config); err != nil { + return nil, err + } + } + return &Server{ + config: config, services: newServiceSet(), done: make(chan struct{}), listeners: make(map[net.Listener]struct{}), connections: make(map[*serverConn]struct{}), - } + }, nil } func (s *Server) Register(name string, methods map[string]Method) { @@ -46,10 +56,15 @@ func (s *Server) Serve(l net.Listener) error { defer s.closeListener(l) var ( - ctx = context.Background() - backoff time.Duration + ctx = context.Background() + backoff time.Duration + handshaker = s.config.handshaker ) + if handshaker == nil { + handshaker = handshakerFunc(noopHandshake) + } + for { conn, err := l.Accept() if err != nil { @@ -82,7 +97,15 @@ func (s *Server) Serve(l net.Listener) error { } backoff = 0 - sc := s.newConn(conn) + + approved, handshake, err := handshaker.Handshake(ctx, conn) + if err != nil { + log.L.WithError(err).Errorf("ttrpc: refusing connection after handshake") + conn.Close() + continue + } + + sc := s.newConn(approved, handshake) go sc.run(ctx) } } @@ -205,11 +228,12 @@ func (cs connState) String() string { } } -func (s *Server) newConn(conn net.Conn) *serverConn { +func (s *Server) newConn(conn net.Conn, handshake interface{}) *serverConn { c := &serverConn{ - server: s, - conn: conn, - shutdown: make(chan struct{}), + server: s, + conn: conn, + handshake: handshake, + shutdown: make(chan struct{}), } c.setState(connStateIdle) s.addConnection(c) @@ -217,9 +241,10 @@ func (s *Server) newConn(conn net.Conn) *serverConn { } type serverConn struct { - server *Server - conn net.Conn - state atomic.Value + server *Server + conn net.Conn + handshake interface{} // data from handshake, not used for now + state atomic.Value shutdownOnce sync.Once shutdown chan struct{} // forced shutdown, used by close @@ -406,7 +431,7 @@ func (c *serverConn) run(sctx context.Context) { // branch. Basically, it means that we are no longer receiving // requests due to a terminal error. recvErr = nil // connection is now "closing" - if err != nil { + if err != nil && err != io.EOF { log.L.WithError(err).Error("error receiving message") } case <-shutdown: diff --git a/vendor/github.com/stevvooe/ttrpc/unixcreds.go b/vendor/github.com/stevvooe/ttrpc/unixcreds.go new file mode 100644 index 000000000..5de8b922f --- /dev/null +++ b/vendor/github.com/stevvooe/ttrpc/unixcreds.go @@ -0,0 +1,82 @@ +// +build linux freebsd solaris + +package ttrpc + +import ( + "context" + "net" + "os" + "syscall" + + "github.com/pkg/errors" + "golang.org/x/sys/unix" +) + +type UnixCredentialsFunc func(*unix.Ucred) error + +func (fn UnixCredentialsFunc) Handshake(ctx context.Context, conn net.Conn) (net.Conn, interface{}, error) { + uc, err := requireUnixSocket(conn) + if err != nil { + return nil, nil, errors.Wrap(err, "ttrpc.UnixCredentialsFunc: require unix socket") + } + + // TODO(stevvooe): Calling (*UnixConn).File causes a 5x performance + // decrease vs just accessing the fd directly. Need to do some more + // troubleshooting to isolate this to Go runtime or kernel. + fp, err := uc.File() + if err != nil { + return nil, nil, errors.Wrap(err, "ttrpc.UnixCredentialsFunc: failed to get unix file") + } + defer fp.Close() // this gets duped and must be closed when this method is complete. + + ucred, err := unix.GetsockoptUcred(int(fp.Fd()), unix.SOL_SOCKET, unix.SO_PEERCRED) + if err != nil { + return nil, nil, errors.Wrapf(err, "ttrpc.UnixCredentialsFunc: failed to retrieve socket peer credentials") + } + + if err := fn(ucred); err != nil { + return nil, nil, errors.Wrapf(err, "ttrpc.UnixCredentialsFunc: credential check failed") + } + + return uc, ucred, nil +} + +func UnixSocketRequireUidGid(uid, gid int) UnixCredentialsFunc { + return func(ucred *unix.Ucred) error { + return requireUidGid(ucred, uid, gid) + } +} + +func UnixSocketRequireRoot() UnixCredentialsFunc { + return UnixSocketRequireUidGid(0, 0) +} + +// UnixSocketRequireSameUser resolves the current unix user and returns a +// UnixCredentialsFunc that will validate incoming unix connections against the +// current credentials. +// +// This is useful when using abstract sockets that are accessible by all users. +func UnixSocketRequireSameUser() UnixCredentialsFunc { + uid, gid := os.Getuid(), os.Getgid() + return UnixSocketRequireUidGid(uid, gid) +} + +func requireRoot(ucred *unix.Ucred) error { + return requireUidGid(ucred, 0, 0) +} + +func requireUidGid(ucred *unix.Ucred, uid, gid int) error { + if (uid != -1 && uint32(uid) != ucred.Uid) || (gid != -1 && uint32(gid) != ucred.Gid) { + return errors.Wrap(syscall.EPERM, "ttrpc: invalid credentials") + } + return nil +} + +func requireUnixSocket(conn net.Conn) (*net.UnixConn, error) { + uc, ok := conn.(*net.UnixConn) + if !ok { + return nil, errors.New("a unix socket connection is required") + } + + return uc, nil +}