Skip to content

Commit 689d92c

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 ea04599 commit 689d92c

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
@@ -134,20 +134,26 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
134134
if err != nil {
135135
return "", err
136136
}
137-
address, err := shim.SocketAddress(ctx, id)
137+
address, err := shim.SocketAddress(ctx, containerdAddress, id)
138138
if err != nil {
139139
return "", err
140140
}
141141
socket, err := shim.NewSocket(address)
142142
if err != nil {
143-
return "", err
143+
if !shim.SocketEaddrinuse(err) {
144+
return "", err
145+
}
146+
if err := shim.RemoveSocket(address); err != nil {
147+
return "", errors.Wrap(err, "remove already used socket")
148+
}
149+
if socket, err = shim.NewSocket(address); err != nil {
150+
return "", err
151+
}
144152
}
145-
defer socket.Close()
146153
f, err := socket.File()
147154
if err != nil {
148155
return "", err
149156
}
150-
defer f.Close()
151157

152158
cmd.ExtraFiles = append(cmd.ExtraFiles, f)
153159

@@ -156,6 +162,7 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
156162
}
157163
defer func() {
158164
if err != nil {
165+
_ = shim.RemoveSocket(address)
159166
cmd.Process.Kill()
160167
}
161168
}()
@@ -550,6 +557,9 @@ func (s *service) Connect(ctx context.Context, r *taskAPI.ConnectRequest) (*task
550557
func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (*ptypes.Empty, error) {
551558
s.cancel()
552559
close(s.events)
560+
if address, err := shim.ReadAddress("address"); err == nil {
561+
_ = shim.RemoveSocket(address)
562+
}
553563
return empty, nil
554564
}
555565

‎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"
@@ -96,6 +95,10 @@ func New(ctx context.Context, id string, publisher shim.Publisher, shutdown func
9695
return nil, errors.Wrap(err, "failed to initialized platform behavior")
9796
}
9897
go s.forward(ctx, publisher)
98+
99+
if address, err := shim.ReadAddress("address"); err == nil {
100+
s.shimAddress = address
101+
}
99102
return s, nil
100103
}
101104

@@ -115,7 +118,8 @@ type service struct {
115118

116119
containers map[string]*runc.Container
117120

118-
cancel func()
121+
shimAddress string
122+
cancel func()
119123
}
120124

121125
func newCommand(ctx context.Context, id, containerdBinary, containerdAddress, containerdTTRPCAddress string) (*exec.Cmd, error) {
@@ -174,30 +178,48 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container
174178
break
175179
}
176180
}
177-
address, err := shim.SocketAddress(ctx, grouping)
181+
address, err := shim.SocketAddress(ctx, containerdAddress, grouping)
178182
if err != nil {
179183
return "", err
180184
}
185+
181186
socket, err := shim.NewSocket(address)
182187
if err != nil {
183-
if strings.Contains(err.Error(), "address already in use") {
188+
// the only time where this would happen is if there is a bug and the socket
189+
// was not cleaned up in the cleanup method of the shim or we are using the
190+
// grouping functionality where the new process should be run with the same
191+
// shim as an existing container
192+
if !shim.SocketEaddrinuse(err) {
193+
return "", errors.Wrap(err, "create new shim socket")
194+
}
195+
if shim.CanConnect(address) {
184196
if err := shim.WriteAddress("address", address); err != nil {
185-
return "", err
197+
return "", errors.Wrap(err, "write existing socket for shim")
186198
}
187199
return address, nil
188200
}
189-
return "", err
201+
if err := shim.RemoveSocket(address); err != nil {
202+
return "", errors.Wrap(err, "remove pre-existing socket")
203+
}
204+
if socket, err = shim.NewSocket(address); err != nil {
205+
return "", errors.Wrap(err, "try create new shim socket 2x")
206+
}
190207
}
191-
defer socket.Close()
208+
defer func() {
209+
if retErr != nil {
210+
socket.Close()
211+
_ = shim.RemoveSocket(address)
212+
}
213+
}()
192214
f, err := socket.File()
193215
if err != nil {
194216
return "", err
195217
}
196-
defer f.Close()
197218

198219
cmd.ExtraFiles = append(cmd.ExtraFiles, f)
199220

200221
if err := cmd.Start(); err != nil {
222+
f.Close()
201223
return "", err
202224
}
203225
defer func() {
@@ -251,7 +273,6 @@ func (s *service) Cleanup(ctx context.Context) (*taskAPI.DeleteResponse, error)
251273
if err != nil {
252274
return nil, err
253275
}
254-
255276
runtime, err := runc.ReadRuntime(path)
256277
if err != nil {
257278
return nil, err
@@ -610,7 +631,9 @@ func (s *service) Shutdown(ctx context.Context, r *taskAPI.ShutdownRequest) (*pt
610631
if s.platform != nil {
611632
s.platform.Close()
612633
}
613-
634+
if s.shimAddress != "" {
635+
_ = shim.RemoveSocket(s.shimAddress)
636+
}
614637
return empty, nil
615638
}
616639

‎runtime/v2/shim/shim.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func parseFlags() {
101101
flag.BoolVar(&debugFlag, "debug", false, "enable debug output in logs")
102102
flag.StringVar(&namespaceFlag, "namespace", "", "namespace that owns the shim")
103103
flag.StringVar(&idFlag, "id", "", "id of the task")
104-
flag.StringVar(&socketFlag, "socket", "", "abstract socket path to serve")
104+
flag.StringVar(&socketFlag, "socket", "", "socket path to serve")
105105
flag.StringVar(&bundlePath, "bundle", "", "path to the bundle if not workdir")
106106

107107
flag.StringVar(&addressFlag, "address", "", "grpc address back to main containerd")
@@ -183,7 +183,6 @@ func run(id string, initFunc Init, config Config) error {
183183
ctx = context.WithValue(ctx, OptsKey{}, Opts{BundlePath: bundlePath, Debug: debugFlag})
184184
ctx = log.WithLogger(ctx, log.G(ctx).WithField("runtime", id))
185185
ctx, cancel := context.WithCancel(ctx)
186-
187186
service, err := initFunc(ctx, idFlag, publisher, cancel)
188187
if err != nil {
189188
return err
@@ -288,11 +287,15 @@ func serve(ctx context.Context, server *ttrpc.Server, path string) error {
288287
return err
289288
}
290289
go func() {
291-
defer l.Close()
292290
if err := server.Serve(ctx, l); err != nil &&
293291
!strings.Contains(err.Error(), "use of closed network connection") {
294292
logrus.WithError(err).Fatal("containerd-shim: ttrpc server failure")
295293
}
294+
l.Close()
295+
if address, err := ReadAddress("address"); err == nil {
296+
_ = RemoveSocket(address)
297+
}
298+
296299
}()
297300
return nil
298301
}

‎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
@@ -34,7 +34,10 @@ import (
3434
"github.com/pkg/errors"
3535
)
3636

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

3942
func getSysProcAttr() *syscall.SysProcAttr {
4043
return &syscall.SysProcAttr{
@@ -62,20 +65,21 @@ func AdjustOOMScore(pid int) error {
6265
return nil
6366
}
6467

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

75-
// AnonDialer returns a dialer for an abstract socket
80+
// AnonDialer returns a dialer for a socket
7681
func AnonDialer(address string, timeout time.Duration) (net.Conn, error) {
77-
address = strings.TrimPrefix(address, "unix://")
78-
return net.DialTimeout("unix", "\x00"+address, timeout)
82+
return net.DialTimeout("unix", socket(address).path(), timeout)
7983
}
8084

8185
func AnonReconnectDialer(address string, timeout time.Duration) (net.Conn, error) {
@@ -84,12 +88,82 @@ func AnonReconnectDialer(address string, timeout time.Duration) (net.Conn, error
8488

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

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