Skip to content

Commit 428f10f

Browse files
crosbymichaelsamuelkarp
authored and
dmcgowan
committed
Use path based unix socket for shims
This allows filesystem based ACLs for configuring access to the socket of a shim. Co-authored-by: Samuel Karp <[email protected]> Signed-off-by: Samuel Karp <[email protected]> Signed-off-by: Michael Crosby <[email protected]> Signed-off-by: Michael Crosby <[email protected]>
1 parent b321d35 commit 428f10f

8 files changed

Lines changed: 155 additions & 37 deletions

File tree

‎cmd/ctr/commands/shim/shim.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"io/ioutil"
2525
"net"
2626
"path/filepath"
27+
"strings"
2728

2829
"github.com/containerd/console"
2930
"github.com/containerd/containerd/cmd/ctr/commands"
@@ -240,10 +241,11 @@ func getTaskService(context *cli.Context) (task.TaskService, error) {
240241
s1 := filepath.Join(string(filepath.Separator), "containerd-shim", ns, id, "shim.sock")
241242
// this should not error, ctr always get a default ns
242243
ctx := namespaces.WithNamespace(gocontext.Background(), ns)
243-
s2, _ := shim.SocketAddress(ctx, id)
244+
s2, _ := shim.SocketAddress(ctx, context.GlobalString("address"), id)
245+
s2 = strings.TrimPrefix(s2, "unix://")
244246

245-
for _, socket := range []string{s1, s2} {
246-
conn, err := net.Dial("unix", "\x00"+socket)
247+
for _, socket := range []string{s2, "\x00" + s1} {
248+
conn, err := net.Dial("unix", socket)
247249
if err == nil {
248250
client := ttrpc.NewClient(conn)
249251

‎runtime/v2/runc/v1/service.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,20 +131,26 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
131131
if err != nil {
132132
return "", err
133133
}
134-
address, err := shim.SocketAddress(ctx, id)
134+
address, err := shim.SocketAddress(ctx, containerdAddress, id)
135135
if err != nil {
136136
return "", err
137137
}
138138
socket, err := shim.NewSocket(address)
139139
if err != nil {
140-
return "", err
140+
if !shim.SocketEaddrinuse(err) {
141+
return "", err
142+
}
143+
if err := shim.RemoveSocket(address); err != nil {
144+
return "", errors.Wrap(err, "remove already used socket")
145+
}
146+
if socket, err = shim.NewSocket(address); err != nil {
147+
return "", err
148+
}
141149
}
142-
defer socket.Close()
143150
f, err := socket.File()
144151
if err != nil {
145152
return "", err
146153
}
147-
defer f.Close()
148154

149155
cmd.ExtraFiles = append(cmd.ExtraFiles, f)
150156

@@ -153,6 +159,7 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
153159
}
154160
defer func() {
155161
if err != nil {
162+
_ = shim.RemoveSocket(address)
156163
cmd.Process.Kill()
157164
}
158165
}()
@@ -551,6 +558,9 @@ func (s *service) Connect(ctx context.Context, r *taskAPI.ConnectRequest) (*task
551558
func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (*ptypes.Empty, error) {
552559
s.cancel()
553560
close(s.events)
561+
if address, err := shim.ReadAddress("address"); err == nil {
562+
_ = shim.RemoveSocket(address)
563+
}
554564
return empty, nil
555565
}
556566

‎runtime/v2/runc/v2/service.go‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"os"
2626
"os/exec"
2727
"path/filepath"
28-
"strings"
2928
"sync"
3029
"syscall"
3130
"time"
@@ -105,6 +104,10 @@ func New(ctx context.Context, id string, publisher shim.Publisher, shutdown func
105104
return nil, errors.Wrap(err, "failed to initialized platform behavior")
106105
}
107106
go s.forward(ctx, publisher)
107+
108+
if address, err := shim.ReadAddress("address"); err == nil {
109+
s.shimAddress = address
110+
}
108111
return s, nil
109112
}
110113

@@ -124,7 +127,8 @@ type service struct {
124127

125128
containers map[string]*runc.Container
126129

127-
cancel func()
130+
shimAddress string
131+
cancel func()
128132
}
129133

130134
func newCommand(ctx context.Context, id, containerdBinary, containerdAddress, containerdTTRPCAddress string) (*exec.Cmd, error) {
@@ -183,30 +187,48 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
183187
break
184188
}
185189
}
186-
address, err := shim.SocketAddress(ctx, grouping)
190+
address, err := shim.SocketAddress(ctx, containerdAddress, grouping)
187191
if err != nil {
188192
return "", err
189193
}
194+
190195
socket, err := shim.NewSocket(address)
191196
if err != nil {
192-
if strings.Contains(err.Error(), "address already in use") {
197+
// the only time where this would happen is if there is a bug and the socket
198+
// was not cleaned up in the cleanup method of the shim or we are using the
199+
// grouping functionality where the new process should be run with the same
200+
// shim as an existing container
201+
if !shim.SocketEaddrinuse(err) {
202+
return "", errors.Wrap(err, "create new shim socket")
203+
}
204+
if shim.CanConnect(address) {
193205
if err := shim.WriteAddress("address", address); err != nil {
194-
return "", err
206+
return "", errors.Wrap(err, "write existing socket for shim")
195207
}
196208
return address, nil
197209
}
198-
return "", err
210+
if err := shim.RemoveSocket(address); err != nil {
211+
return "", errors.Wrap(err, "remove pre-existing socket")
212+
}
213+
if socket, err = shim.NewSocket(address); err != nil {
214+
return "", errors.Wrap(err, "try create new shim socket 2x")
215+
}
199216
}
200-
defer socket.Close()
217+
defer func() {
218+
if retErr != nil {
219+
socket.Close()
220+
_ = shim.RemoveSocket(address)
221+
}
222+
}()
201223
f, err := socket.File()
202224
if err != nil {
203225
return "", err
204226
}
205-
defer f.Close()
206227

207228
cmd.ExtraFiles = append(cmd.ExtraFiles, f)
208229

209230
if err := cmd.Start(); err != nil {
231+
f.Close()
210232
return "", err
211233
}
212234
defer func() {
@@ -273,7 +295,6 @@ func (s *service) Cleanup(ctx context.Context) (*taskAPI.DeleteResponse, error)
273295
if err != nil {
274296
return nil, err
275297
}
276-
277298
runtime, err := runc.ReadRuntime(path)
278299
if err != nil {
279300
return nil, err
@@ -652,7 +673,9 @@ func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (*pt
652673
if s.platform != nil {
653674
s.platform.Close()
654675
}
655-
676+
if s.shimAddress != "" {
677+
_ = shim.RemoveSocket(s.shimAddress)
678+
}
656679
return empty, nil
657680
}
658681

‎runtime/v2/shim/shim.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func parseFlags() {
104104
flag.BoolVar(&versionFlag, "v", false, "show the shim version and exit")
105105
flag.StringVar(&namespaceFlag, "namespace", "", "namespace that owns the shim")
106106
flag.StringVar(&idFlag, "id", "", "id of the task")
107-
flag.StringVar(&socketFlag, "socket", "", "abstract socket path to serve")
107+
flag.StringVar(&socketFlag, "socket", "", "socket path to serve")
108108
flag.StringVar(&bundlePath, "bundle", "", "path to the bundle if not workdir")
109109

110110
flag.StringVar(&addressFlag, "address", "", "grpc address back to main containerd")
@@ -195,7 +195,6 @@ func run(id string, initFunc Init, config Config) error {
195195
ctx = context.WithValue(ctx, OptsKey{}, Opts{BundlePath: bundlePath, Debug: debugFlag})
196196
ctx = log.WithLogger(ctx, log.G(ctx).WithField("runtime", id))
197197
ctx, cancel := context.WithCancel(ctx)
198-
199198
service, err := initFunc(ctx, idFlag, publisher, cancel)
200199
if err != nil {
201200
return err
@@ -300,11 +299,15 @@ func serve(ctx context.Context, server *ttrpc.Server, path string) error {
300299
return err
301300
}
302301
go func() {
303-
defer l.Close()
304302
if err := server.Serve(ctx, l); err != nil &&
305303
!strings.Contains(err.Error(), "use of closed network connection") {
306304
logrus.WithError(err).Fatal("containerd-shim: ttrpc server failure")
307305
}
306+
l.Close()
307+
if address, err := ReadAddress("address"); err == nil {
308+
_ = RemoveSocket(address)
309+
}
310+
308311
}()
309312
return nil
310313
}

‎runtime/v2/shim/shim_unix.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ func serveListener(path string) (net.Listener, error) {
5858
l, err = net.FileListener(os.NewFile(3, "socket"))
5959
path = "[inherited from parent]"
6060
} else {
61-
if len(path) > 106 {
62-
return nil, errors.Errorf("%q: unix socket path too long (> 106)", path)
61+
if len(path) > socketPathLimit {
62+
return nil, errors.Errorf("%q: unix socket path too long (> %d)", path, socketPathLimit)
6363
}
64-
l, err = net.Listen("unix", "\x00"+path)
64+
l, err = net.Listen("unix", path)
6565
}
6666
if err != nil {
6767
return nil, err
6868
}
69-
logrus.WithField("socket", path).Debug("serving api on abstract socket")
69+
logrus.WithField("socket", path).Debug("serving api on socket")
7070
return l, nil
7171
}
7272

‎runtime/v2/shim/util.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func WriteAddress(path, address string) error {
169169
// ErrNoAddress is returned when the address file has no content
170170
var ErrNoAddress = errors.New("no shim address")
171171

172-
// ReadAddress returns the shim's abstract socket address from the path
172+
// ReadAddress returns the shim's socket address from the path
173173
func ReadAddress(path string) (string, error) {
174174
path, err := filepath.Abs(path)
175175
if err != nil {

‎runtime/v2/shim/util_unix.go‎

Lines changed: 86 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ import (
3535
"github.com/pkg/errors"
3636
)
3737

38-
const shimBinaryFormat = "containerd-shim-%s-%s"
38+
const (
39+
shimBinaryFormat = "containerd-shim-%s-%s"
40+
socketPathLimit = 106
41+
)
3942

4043
func getSysProcAttr() *syscall.SysProcAttr {
4144
return &syscall.SysProcAttr{
@@ -63,20 +66,21 @@ func AdjustOOMScore(pid int) error {
6366
return nil
6467
}
6568

66-
// SocketAddress returns an abstract socket address
67-
func SocketAddress(ctx context.Context, id string) (string, error) {
69+
const socketRoot = "/run/containerd"
70+
71+
// SocketAddress returns a socket address
72+
func SocketAddress(ctx context.Context, socketPath, id string) (string, error) {
6873
ns, err := namespaces.NamespaceRequired(ctx)
6974
if err != nil {
7075
return "", err
7176
}
72-
d := sha256.Sum256([]byte(filepath.Join(ns, id)))
73-
return filepath.Join(string(filepath.Separator), "containerd-shim", fmt.Sprintf("%x.sock", d)), nil
77+
d := sha256.Sum256([]byte(filepath.Join(socketPath, ns, id)))
78+
return fmt.Sprintf("unix://%s/%x", filepath.Join(socketRoot, "s"), d), nil
7479
}
7580

76-
// AnonDialer returns a dialer for an abstract socket
81+
// AnonDialer returns a dialer for a socket
7782
func AnonDialer(address string, timeout time.Duration) (net.Conn, error) {
78-
address = strings.TrimPrefix(address, "unix://")
79-
return dialer.Dialer("\x00"+address, timeout)
83+
return dialer.Dialer(socket(address).path(), timeout)
8084
}
8185

8286
func AnonReconnectDialer(address string, timeout time.Duration) (net.Conn, error) {
@@ -85,12 +89,82 @@ func AnonReconnectDialer(address string, timeout time.Duration) (net.Conn, error
8589

8690
// NewSocket returns a new socket
8791
func NewSocket(address string) (*net.UnixListener, error) {
88-
if len(address) > 106 {
89-
return nil, errors.Errorf("%q: unix socket path too long (> 106)", address)
92+
var (
93+
sock = socket(address)
94+
path = sock.path()
95+
)
96+
if !sock.isAbstract() {
97+
if err := os.MkdirAll(filepath.Dir(path), 0600); err != nil {
98+
return nil, errors.Wrapf(err, "%s", path)
99+
}
90100
}
91-
l, err := net.Listen("unix", "\x00"+address)
101+
l, err := net.Listen("unix", path)
92102
if err != nil {
93-
return nil, errors.Wrapf(err, "failed to listen to abstract unix socket %q", address)
103+
return nil, err
104+
}
105+
if err := os.Chmod(path, 0600); err != nil {
106+
os.Remove(sock.path())
107+
l.Close()
108+
return nil, err
94109
}
95110
return l.(*net.UnixListener), nil
96111
}
112+
113+
const abstractSocketPrefix = "\x00"
114+
115+
type socket string
116+
117+
func (s socket) isAbstract() bool {
118+
return !strings.HasPrefix(string(s), "unix://")
119+
}
120+
121+
func (s socket) path() string {
122+
path := strings.TrimPrefix(string(s), "unix://")
123+
// if there was no trim performed, we assume an abstract socket
124+
if len(path) == len(s) {
125+
path = abstractSocketPrefix + path
126+
}
127+
return path
128+
}
129+
130+
// RemoveSocket removes the socket at the specified address if
131+
// it exists on the filesystem
132+
func RemoveSocket(address string) error {
133+
sock := socket(address)
134+
if !sock.isAbstract() {
135+
return os.Remove(sock.path())
136+
}
137+
return nil
138+
}
139+
140+
// SocketEaddrinuse returns true if the provided error is caused by the
141+
// EADDRINUSE error number
142+
func SocketEaddrinuse(err error) bool {
143+
netErr, ok := err.(*net.OpError)
144+
if !ok {
145+
return false
146+
}
147+
if netErr.Op != "listen" {
148+
return false
149+
}
150+
syscallErr, ok := netErr.Err.(*os.SyscallError)
151+
if !ok {
152+
return false
153+
}
154+
errno, ok := syscallErr.Err.(syscall.Errno)
155+
if !ok {
156+
return false
157+
}
158+
return errno == syscall.EADDRINUSE
159+
}
160+
161+
// CanConnect returns true if the socket provided at the address
162+
// is accepting new connections
163+
func CanConnect(address string) bool {
164+
conn, err := AnonDialer(address, 100*time.Millisecond)
165+
if err != nil {
166+
return false
167+
}
168+
conn.Close()
169+
return true
170+
}

‎runtime/v2/shim/util_windows.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,9 @@ func AnonDialer(address string, timeout time.Duration) (net.Conn, error) {
7979
return c, nil
8080
}
8181
}
82+
83+
// RemoveSocket removes the socket at the specified address if
84+
// it exists on the filesystem
85+
func RemoveSocket(address string) error {
86+
return nil
87+
}

0 commit comments

Comments
 (0)