diff --git a/internal/localaddr/localaddr.go b/internal/localaddr/localaddr.go new file mode 100644 index 0000000000..3bde98da07 --- /dev/null +++ b/internal/localaddr/localaddr.go @@ -0,0 +1,75 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +// Package localaddr parses the local (non-TCP) addresses containerd listens +// on and dials on Windows: AF_UNIX sockets and named pipes. It is plain string +// handling, so that it can be shared and tested on every platform. +package localaddr + +import "strings" + +// Network is the kind of local endpoint an address refers to. +type Network int + +const ( + // Unix is an AF_UNIX socket. + Unix Network = iota + // NamedPipe is a Windows named pipe. + NamedPipe +) + +// Parse classifies a local address and returns the path to give to net.Listen +// (Unix) or go-winio (NamedPipe). An address is one of: +// - "unix://" or "unix:": an AF_UNIX socket. gRPC passes a +// custom dialer the first form for a path that is absolute on the host +// and the second otherwise. +// - "npipe://": a named pipe, as "npipe:////./pipe/" or +// "npipe://./pipe/". +// - "//./pipe/" or `\\.\pipe\`: a named pipe. +// - anything else: an AF_UNIX socket. +// +// A scheme is authoritative. The path of a named pipe is always +// "//./pipe/"; that of a Unix socket is returned as written, for the +// caller to convert to the host's separators. explicit is false for a bare +// path that merely defaults to Unix. +func Parse(address string) (network Network, path string, explicit bool) { + if rest, ok := strings.CutPrefix(address, "unix:"); ok { + rest = strings.TrimPrefix(rest, "//") + // Also accept "unix:///C:/", as in a file URL, and gRPC's + // "unix:/C:/". + if len(rest) >= 3 && rest[0] == '/' && rest[2] == ':' { + rest = rest[1:] + } + return Unix, rest, true + } + slashed := strings.ReplaceAll(address, `\`, "/") + if rest, ok := strings.CutPrefix(slashed, "npipe://"); ok { + // The authority of "npipe://./pipe/" is ".". + if hasPrefixFold(rest, "./pipe/") { + rest = "//" + rest + } + return NamedPipe, rest, true + } + if hasPrefixFold(slashed, "//./pipe/") { + return NamedPipe, slashed, true + } + return Unix, address, false +} + +// hasPrefixFold is strings.HasPrefix ignoring case, as the pipe namespace is. +func hasPrefixFold(s, prefix string) bool { + return len(s) >= len(prefix) && strings.EqualFold(s[:len(prefix)], prefix) +} diff --git a/internal/localaddr/localaddr_test.go b/internal/localaddr/localaddr_test.go new file mode 100644 index 0000000000..cbc0716f79 --- /dev/null +++ b/internal/localaddr/localaddr_test.go @@ -0,0 +1,64 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package localaddr + +import "testing" + +func TestParse(t *testing.T) { + testcases := []struct { + name string + address string + wantNetwork Network + wantPath string + wantExplicit bool + }{ + // Named pipes without a scheme (backward compatibility). + {name: "pipe with slashes", address: "//./pipe/containerd", wantNetwork: NamedPipe, wantPath: "//./pipe/containerd", wantExplicit: true}, + {name: "pipe with backslashes", address: `\\.\pipe\containerd`, wantNetwork: NamedPipe, wantPath: "//./pipe/containerd", wantExplicit: true}, + {name: "pipe namespace is case insensitive", address: `\\.\PIPE\Containerd`, wantNetwork: NamedPipe, wantPath: "//./PIPE/Containerd", wantExplicit: true}, + + // Named pipes with a scheme. + {name: "npipe canonical", address: "npipe:////./pipe/containerd", wantNetwork: NamedPipe, wantPath: "//./pipe/containerd", wantExplicit: true}, + {name: "npipe short", address: "npipe://./pipe/containerd", wantNetwork: NamedPipe, wantPath: "//./pipe/containerd", wantExplicit: true}, + {name: "npipe short case insensitive", address: "npipe://./PIPE/containerd", wantNetwork: NamedPipe, wantPath: "//./PIPE/containerd", wantExplicit: true}, + {name: "npipe with backslashes", address: `npipe://\\.\pipe\containerd`, wantNetwork: NamedPipe, wantPath: "//./pipe/containerd", wantExplicit: true}, + + // AF_UNIX sockets with a scheme. + {name: "unix absolute", address: "unix:///tmp/containerd.sock", wantNetwork: Unix, wantPath: "/tmp/containerd.sock", wantExplicit: true}, + {name: "unix drive letter", address: `unix://C:\run\containerd.sock`, wantNetwork: Unix, wantPath: `C:\run\containerd.sock`, wantExplicit: true}, + {name: "unix opaque drive letter", address: "unix:C:/run/containerd.sock", wantNetwork: Unix, wantPath: "C:/run/containerd.sock", wantExplicit: true}, + {name: "unix file URL drive letter", address: "unix:///C:/run/containerd.sock", wantNetwork: Unix, wantPath: "C:/run/containerd.sock", wantExplicit: true}, + {name: "unix opaque absolute", address: "unix:/tmp/containerd.sock", wantNetwork: Unix, wantPath: "/tmp/containerd.sock", wantExplicit: true}, + // The scheme is authoritative, whatever the path looks like. + {name: "unix scheme wins over pipe path", address: "unix:////./pipe/containerd", wantNetwork: Unix, wantPath: "//./pipe/containerd", wantExplicit: true}, + + // Bare paths default to AF_UNIX. + {name: "bare absolute", address: "/tmp/containerd.sock", wantNetwork: Unix, wantPath: "/tmp/containerd.sock"}, + {name: "bare drive letter", address: `C:\run\containerd.sock`, wantNetwork: Unix, wantPath: `C:\run\containerd.sock`}, + {name: "bare relative", address: "containerd.sock", wantNetwork: Unix, wantPath: "containerd.sock"}, + {name: "not a pipe namespace", address: "//./notpipe/containerd", wantNetwork: Unix, wantPath: "//./notpipe/containerd"}, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + network, path, explicit := Parse(tc.address) + if network != tc.wantNetwork || path != tc.wantPath || explicit != tc.wantExplicit { + t.Errorf("Parse(%q) = (%v, %q, %v), want (%v, %q, %v)", + tc.address, network, path, explicit, tc.wantNetwork, tc.wantPath, tc.wantExplicit) + } + }) + } +} diff --git a/pkg/dialer/dialer_test.go b/pkg/dialer/dialer_test.go new file mode 100644 index 0000000000..1560956fe0 --- /dev/null +++ b/pkg/dialer/dialer_test.go @@ -0,0 +1,113 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dialer + +import ( + "context" + "net" + "os" + "path/filepath" + "testing" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/credentials/insecure" +) + +// acceptAndClose accepts connections on l and closes them, until l is closed. +func acceptAndClose(l net.Listener) { + for { + conn, err := l.Accept() + if err != nil { + return + } + conn.Close() + } +} + +// listenUnix listens on an AF_UNIX socket in a directory with a short path: +// t.TempDir embeds the test name, which can overflow the sun_path limit (104 +// on macOS). +func listenUnix(t *testing.T, name string) (net.Listener, string) { + t.Helper() + + dir, err := os.MkdirTemp("", "sock") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(dir) }) + + sockPath := filepath.Join(dir, name) + l, err := net.Listen("unix", sockPath) + if err != nil { + t.Fatalf("Listen(%q) failed: %v", sockPath, err) + } + t.Cleanup(func() { l.Close() }) + return l, sockPath +} + +func TestDialerUnixSocket(t *testing.T) { + for _, prefix := range []string{"", "unix://"} { + t.Run("prefix="+prefix, func(t *testing.T) { + l, sockPath := listenUnix(t, "test.sock") + go acceptAndClose(l) + + conn, err := dialer(prefix+sockPath, 5*time.Second) + if err != nil { + t.Fatalf("dialer(%q) failed: %v", prefix+sockPath, err) + } + conn.Close() + }) + } +} + +// TestDialAddressGRPC verifies that gRPC accepts the target from DialAddress +// and connects through ContextDialer, as the containerd client does. +func TestDialAddressGRPC(t *testing.T) { + l, sockPath := listenUnix(t, "test.sock") + connectGRPC(t, l, sockPath) +} + +// connectGRPC serves gRPC on l and connects to it at address. +func connectGRPC(t *testing.T, l net.Listener, address string) { + t.Helper() + + srv := grpc.NewServer() + go srv.Serve(l) + t.Cleanup(srv.Stop) + + target := DialAddress(address) + conn, err := grpc.NewClient(target, + grpc.WithContextDialer(ContextDialer), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + if err != nil { + t.Fatalf("grpc.NewClient(%q) failed: %v", target, err) + } + defer conn.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + conn.Connect() + for state := conn.GetState(); state != connectivity.Ready; state = conn.GetState() { + if !conn.WaitForStateChange(ctx, state) { + t.Fatalf("connection to %q is %v, want %v: %v", target, state, connectivity.Ready, ctx.Err()) + } + } +} diff --git a/pkg/dialer/dialer_unix.go b/pkg/dialer/dialer_unix.go index 1d98a0b444..3f8c50f7d8 100644 --- a/pkg/dialer/dialer_unix.go +++ b/pkg/dialer/dialer_unix.go @@ -27,8 +27,12 @@ import ( ) // DialAddress returns the address with unix:// prepended to the -// provided address +// provided address. An address that already carries the scheme is +// returned unchanged. func DialAddress(address string) string { + if strings.HasPrefix(address, "unix://") { + return address + } return "unix://" + address } diff --git a/pkg/dialer/dialer_unix_test.go b/pkg/dialer/dialer_unix_test.go new file mode 100644 index 0000000000..bea86c4e1f --- /dev/null +++ b/pkg/dialer/dialer_unix_test.go @@ -0,0 +1,39 @@ +//go:build !windows + +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dialer + +import "testing" + +func TestDialAddressScheme(t *testing.T) { + testcases := []struct { + name string + address string + want string + }{ + {name: "bare path gets unix", address: "/run/containerd/containerd.sock", want: "unix:///run/containerd/containerd.sock"}, + {name: "unix scheme idempotent", address: "unix:///run/containerd/containerd.sock", want: "unix:///run/containerd/containerd.sock"}, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if got := DialAddress(tc.address); got != tc.want { + t.Errorf("DialAddress(%q) = %q, want %q", tc.address, got, tc.want) + } + }) + } +} diff --git a/pkg/dialer/dialer_windows.go b/pkg/dialer/dialer_windows.go index 0fed3ba5ca..078824f3d2 100644 --- a/pkg/dialer/dialer_windows.go +++ b/pkg/dialer/dialer_windows.go @@ -18,29 +18,51 @@ package dialer import ( "net" + "net/url" "os" "path/filepath" "strings" "time" winio "github.com/Microsoft/go-winio" + + "github.com/containerd/containerd/v2/internal/localaddr" ) func isNoent(err error) bool { return os.IsNotExist(err) } +// dialer connects to a named pipe or an AF_UNIX socket, see localaddr.Parse. func dialer(address string, timeout time.Duration) (net.Conn, error) { - address = strings.TrimPrefix(filepath.ToSlash(address), "npipe://") - return winio.DialPipe(address, &timeout) + network, path, _ := localaddr.Parse(address) + if network == localaddr.NamedPipe { + return winio.DialPipe(path, &timeout) + } + return net.DialTimeout("unix", filepath.FromSlash(path), timeout) } -// DialAddress returns the dial address with npipe:// prepended to the -// provided address +// DialAddress returns address as a gRPC target: "npipe://" for a named pipe, +// "unix://" for an AF_UNIX socket. The socket path is URL escaped, as gRPC +// unescapes it before ContextDialer, so apply this only once. func DialAddress(address string) string { - address = filepath.ToSlash(address) - if !strings.HasPrefix(address, "npipe://") { - address = "npipe://" + address + network, path, _ := localaddr.Parse(address) + if network == localaddr.NamedPipe { + return "npipe://" + path } - return address + path = filepath.ToSlash(path) + // The opaque "unix:" of a relative path cannot hold '#' or '?'. + if !strings.HasPrefix(path, "/") && !filepath.IsAbs(path) { + if abs, err := filepath.Abs(path); err == nil { + path = filepath.ToSlash(abs) + } + } + // gRPC rejects "unix://C:/..." for its authority. + if len(path) >= 2 && path[1] == ':' { + path = "/" + path + } + if !strings.HasPrefix(path, "/") { + return "unix:" + path + } + return (&url.URL{Scheme: "unix", Path: path}).String() } diff --git a/pkg/dialer/dialer_windows_test.go b/pkg/dialer/dialer_windows_test.go new file mode 100644 index 0000000000..5dec7dd576 --- /dev/null +++ b/pkg/dialer/dialer_windows_test.go @@ -0,0 +1,104 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package dialer + +import ( + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Microsoft/go-winio" +) + +func TestDialAddressScheme(t *testing.T) { + testcases := []struct { + name string + address string + want string + }{ + // Bare paths. + {name: "pipe gets npipe", address: "//./pipe/docker", want: "npipe:////./pipe/docker"}, + {name: "backslash pipe gets npipe", address: `\\.\pipe\docker`, want: "npipe:////./pipe/docker"}, + {name: "unix path gets unix", address: "/tmp/docker.sock", want: "unix:///tmp/docker.sock"}, + // gRPC rejects "unix://C:/...", so a drive letter path gets a slash. + {name: "windows path gets unix", address: "C:/Users/test/docker.sock", want: "unix:///C:/Users/test/docker.sock"}, + {name: "windows backslash path gets unix", address: `C:\Users\test\docker.sock`, want: "unix:///C:/Users/test/docker.sock"}, + + // Schemed addresses are normalized. + {name: "npipe canonical", address: "npipe:////./pipe/docker", want: "npipe:////./pipe/docker"}, + {name: "npipe short", address: "npipe://./pipe/docker", want: "npipe:////./pipe/docker"}, + {name: "npipe with backslashes", address: `npipe://\\.\pipe\docker`, want: "npipe:////./pipe/docker"}, + {name: "unix absolute", address: "unix:///tmp/docker.sock", want: "unix:///tmp/docker.sock"}, + {name: "unix opaque", address: "unix:C:/Users/test/docker.sock", want: "unix:///C:/Users/test/docker.sock"}, + {name: "unix with authority", address: `unix://C:\Users\test\docker.sock`, want: "unix:///C:/Users/test/docker.sock"}, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + got := DialAddress(tc.address) + if got != tc.want { + t.Errorf("DialAddress(%q) = %q, want %q", tc.address, got, tc.want) + } + if again := DialAddress(got); again != got { + t.Errorf("DialAddress is not idempotent: DialAddress(%q) = %q", got, again) + } + }) + } +} + +// TestDialAddressEscapes verifies that '#', '%' and spaces in a socket path, +// absolute or relative, reach ContextDialer unchanged through gRPC. +func TestDialAddressEscapes(t *testing.T) { + for _, name := range []string{"a#b.sock", "a%23b.sock", "a b.sock"} { + t.Run(name, func(t *testing.T) { + l, sockPath := listenUnix(t, name) + connectGRPC(t, l, sockPath) + }) + t.Run("relative "+name, func(t *testing.T) { + l, sockPath := listenUnix(t, name) + t.Chdir(filepath.Dir(sockPath)) + connectGRPC(t, l, name) + }) + } +} + +// TestDialerNamedPipe verifies that every spelling of a pipe address reaches +// the same pipe. +func TestDialerNamedPipe(t *testing.T) { + name := "containerd-test-" + strings.ReplaceAll(t.Name(), "/", "-") + l, err := winio.ListenPipe("//./pipe/"+name, nil) + if err != nil { + t.Fatalf("ListenPipe failed: %v", err) + } + defer l.Close() + go acceptAndClose(l) + + for _, address := range []string{ + `\\.\pipe\` + name, + "//./pipe/" + name, + "npipe:////./pipe/" + name, + "npipe://./pipe/" + name, + } { + t.Run(address, func(t *testing.T) { + conn, err := dialer(address, 5*time.Second) + if err != nil { + t.Fatalf("dialer(%q) failed: %v", address, err) + } + conn.Close() + }) + } +} diff --git a/pkg/sys/socket_test.go b/pkg/sys/socket_test.go new file mode 100644 index 0000000000..5b10012380 --- /dev/null +++ b/pkg/sys/socket_test.go @@ -0,0 +1,113 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package sys + +import ( + "net" + "os" + "path/filepath" + "testing" + "time" + + // required for `-test.root` flag not to fail + _ "github.com/containerd/continuity/testutil" +) + +// shortTempDir returns a temporary directory with a short path: t.TempDir +// embeds the test name, which can overflow the sun_path limit (104 on macOS). +func shortTempDir(t *testing.T) string { + t.Helper() + dir, err := os.MkdirTemp("", "sock") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { os.RemoveAll(dir) }) + return dir +} + +// checkAccept verifies that a connection made by dial reaches l. +func checkAccept(t *testing.T, l net.Listener, dial func() (net.Conn, error)) { + t.Helper() + + type accepted struct { + conn net.Conn + err error + } + done := make(chan accepted, 1) + go func() { + conn, err := l.Accept() + done <- accepted{conn, err} + }() + + conn, err := dial() + if err != nil { + t.Fatalf("dial failed: %v", err) + } + // Stay connected until Accept returns: go-winio drops a pipe connection + // that the client closed first, and Accept then waits for another one. + defer conn.Close() + + select { + case a := <-done: + if a.err != nil { + t.Fatalf("Accept failed: %v", a.err) + } + a.conn.Close() + case <-time.After(10 * time.Second): + t.Fatal("Accept did not return") + } +} + +func TestGetLocalListenerUnixSocket(t *testing.T) { + for _, prefix := range []string{"", "unix://"} { + t.Run("prefix="+prefix, func(t *testing.T) { + sockPath := filepath.Join(shortTempDir(t), "test.sock") + + l, err := GetLocalListener(prefix+sockPath, os.Getuid(), os.Getgid()) + if err != nil { + t.Fatalf("GetLocalListener(%q) failed: %v", prefix+sockPath, err) + } + defer l.Close() + + checkAccept(t, l, func() (net.Conn, error) { return net.Dial("unix", sockPath) }) + }) + } +} + +// TestGetLocalListenerStaleSocket verifies that a socket file left by a +// previous listener does not prevent listening on the same path. +func TestGetLocalListenerStaleSocket(t *testing.T) { + sockPath := filepath.Join(shortTempDir(t), "test.sock") + + l, err := GetLocalListener(sockPath, os.Getuid(), os.Getgid()) + if err != nil { + t.Fatalf("GetLocalListener(%q) failed: %v", sockPath, err) + } + l.(*net.UnixListener).SetUnlinkOnClose(false) + l.Close() + if _, err := os.Lstat(sockPath); err != nil { + t.Fatalf("expected a stale socket at %q: %v", sockPath, err) + } + + l, err = GetLocalListener(sockPath, os.Getuid(), os.Getgid()) + if err != nil { + t.Fatalf("GetLocalListener(%q) over a stale socket failed: %v", sockPath, err) + } + defer l.Close() + + checkAccept(t, l, func() (net.Conn, error) { return net.Dial("unix", sockPath) }) +} diff --git a/pkg/sys/socket_unix.go b/pkg/sys/socket_unix.go index 91034e1718..b93554d664 100644 --- a/pkg/sys/socket_unix.go +++ b/pkg/sys/socket_unix.go @@ -23,6 +23,7 @@ import ( "net" "os" "path/filepath" + "strings" "golang.org/x/sys/unix" ) @@ -42,8 +43,11 @@ func CreateUnixSocket(path string) (net.Listener, error) { return net.Listen("unix", path) } -// GetLocalListener returns a listener out of a unix socket. +// GetLocalListener returns a listener out of a unix socket. The path may +// carry a "unix://" prefix, as in dialer.DialAddress. func GetLocalListener(path string, uid, gid int) (net.Listener, error) { + path = strings.TrimPrefix(path, "unix://") + // Ensure parent directory is created if err := mkdirAs(filepath.Dir(path), uid, gid); err != nil { return nil, err diff --git a/pkg/sys/socket_windows.go b/pkg/sys/socket_windows.go index 1ae12bc511..15c29b61ec 100644 --- a/pkg/sys/socket_windows.go +++ b/pkg/sys/socket_windows.go @@ -17,14 +17,99 @@ package sys import ( + "fmt" "net" + "os" + "path/filepath" "github.com/Microsoft/go-winio" + "golang.org/x/sys/windows" + + "github.com/containerd/containerd/v2/internal/localaddr" ) -// GetLocalListener returns a Listernet out of a named pipe. -// `path` must be of the form of `\\.\pipe\` -// (see https://msdn.microsoft.com/en-us/library/windows/desktop/aa365150) +// sddlSocketAdministratorsLocalSystem grants Builtin Administrators and Local +// System full access and nobody else. Unlike SddlAdministratorsLocalSystem it +// has no OI/CI flags, which only apply to containers. +const sddlSocketAdministratorsLocalSystem = "D:P(A;;GA;;;BA)(A;;GA;;;SY)" + +// maxUnixPathLen is the longest AF_UNIX path: sun_path is 108 bytes on Windows +// as on Linux (https://lwn.net/Articles/987098/), including the NUL. +const maxUnixPathLen = 107 + +// GetLocalListener returns a listener for a named pipe or an AF_UNIX socket +// (Windows 10 1803+, Server 2019+), see localaddr.Parse for the address forms. +// +// uid and gid are unused: an AF_UNIX socket is instead restricted by DACL to +// Builtin Administrators and Local System, and named pipes follow go-winio +// defaults. func GetLocalListener(path string, uid, gid int) (net.Listener, error) { - return winio.ListenPipe(path, nil) + network, path, _ := localaddr.Parse(path) + path = filepath.FromSlash(path) + if network == localaddr.NamedPipe { + return winio.ListenPipe(path, nil) + } + return createUnixSocket(path) +} + +// createUnixSocket listens on an AF_UNIX socket at path. os.Chmod and os.Chown +// do nothing on Windows, so the parent directory is created with +// MkdirAllWithACL and the socket gets a protected DACL. +func createUnixSocket(path string) (net.Listener, error) { + if len(path) > maxUnixPathLen { + return nil, fmt.Errorf("%q: unix socket path too long (> %d)", path, maxUnixPathLen) + } + if err := MkdirAllWithACL(filepath.Dir(path), 0750); err != nil { + return nil, err + } + if err := removeStaleSocket(path); err != nil { + return nil, err + } + l, err := net.Listen("unix", path) + if err != nil { + return nil, fmt.Errorf("failed to listen on unix socket %s: %w", path, err) + } + // The socket inherits its parent's ACEs until the DACL below replaces + // them. MkdirAllWithACL closes that window for directories created here; + // restricting a pre-existing parent is up to the deployer. + if err := setFileSecurityDescriptor(path, sddlSocketAdministratorsLocalSystem); err != nil { + l.Close() + return nil, fmt.Errorf("failed to set security descriptor on unix socket %s: %w", path, err) + } + return l, nil +} + +// removeStaleSocket removes a file left at path by a previous listener. It +// refuses a directory, which os.Remove would delete if empty on Windows. +func removeStaleSocket(path string) error { + fi, err := os.Lstat(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + if fi.IsDir() { + return fmt.Errorf("%s is a directory, not a unix socket", path) + } + return os.Remove(path) +} + +// setFileSecurityDescriptor applies the DACL of sddl to the file at path, +// protected so that it does not inherit ACEs from the parent directory. +func setFileSecurityDescriptor(path, sddl string) error { + sd, err := windows.SecurityDescriptorFromString(sddl) + if err != nil { + return err + } + dacl, _, err := sd.DACL() + if err != nil { + return err + } + return windows.SetNamedSecurityInfo( + path, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION, + nil, nil, dacl, nil, + ) } diff --git a/pkg/sys/socket_windows_test.go b/pkg/sys/socket_windows_test.go new file mode 100644 index 0000000000..db299b6548 --- /dev/null +++ b/pkg/sys/socket_windows_test.go @@ -0,0 +1,148 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package sys + +import ( + "net" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Microsoft/go-winio" + "golang.org/x/sys/windows" +) + +// TestGetLocalListenerNamedPipe verifies that every spelling of a pipe +// address listens on the same pipe. +func TestGetLocalListenerNamedPipe(t *testing.T) { + name := "containerd-test-" + strings.ReplaceAll(t.Name(), "/", "-") + for _, address := range []string{ + `\\.\pipe\` + name, + "//./pipe/" + name, + "npipe:////./pipe/" + name, + "npipe://./pipe/" + name, + } { + t.Run(address, func(t *testing.T) { + l, err := GetLocalListener(address, 0, 0) + if err != nil { + t.Fatalf("GetLocalListener(%q) failed: %v", address, err) + } + defer l.Close() + + checkAccept(t, l, func() (net.Conn, error) { return winio.DialPipe("//./pipe/"+name, nil) }) + }) + } +} + +// pathOfLen returns a path of exactly n bytes in dir. +func pathOfLen(t *testing.T, dir string, n int) string { + t.Helper() + name := n - len(dir) - 1 + if name < 1 { + t.Skipf("%q leaves no room for a file name in %d bytes", dir, n) + } + return filepath.Join(dir, strings.Repeat("a", name)) +} + +// TestCreateUnixSocketPathLength verifies that a path over the sun_path limit +// is rejected here rather than passed to the OS. +func TestCreateUnixSocketPathLength(t *testing.T) { + dir := shortTempDir(t) + + l, err := createUnixSocket(pathOfLen(t, dir, maxUnixPathLen)) + if err != nil { + t.Fatalf("a path of %d bytes should be accepted: %v", maxUnixPathLen, err) + } + l.Close() + + for _, n := range []int{maxUnixPathLen + 1, 2 * maxUnixPathLen} { + path := pathOfLen(t, dir, n) + l, err := createUnixSocket(path) + if err == nil { + l.Close() + t.Fatalf("a path of %d bytes should be rejected", n) + } + if !strings.Contains(err.Error(), "too long") { + t.Errorf("path of %d bytes: got %v, want a \"too long\" error", n, err) + } + if _, err := os.Lstat(path); !os.IsNotExist(err) { + t.Errorf("a rejected path must not be created: %v", err) + } + } +} + +// TestCreateUnixSocketOverDirectory verifies that a directory at the socket +// path is rejected and left alone, as os.Remove would delete an empty one. +func TestCreateUnixSocketOverDirectory(t *testing.T) { + path := filepath.Join(shortTempDir(t), "test.sock") + if err := os.Mkdir(path, 0o700); err != nil { + t.Fatal(err) + } + + l, err := createUnixSocket(path) + if err == nil { + l.Close() + t.Fatal("expected an error for a socket path that is a directory") + } + if fi, err := os.Lstat(path); err != nil || !fi.IsDir() { + t.Errorf("the directory at %q must be left in place: %v", path, err) + } +} + +// TestCreateUnixSocketDACL verifies that only Builtin Administrators and Local +// System can reach the socket, which is root-equivalent. +func TestCreateUnixSocketDACL(t *testing.T) { + sockPath := filepath.Join(shortTempDir(t), "test.sock") + + l, err := createUnixSocket(sockPath) + if err != nil { + t.Fatalf("createUnixSocket(%q) failed: %v", sockPath, err) + } + defer l.Close() + + sd, err := windows.GetNamedSecurityInfo(sockPath, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION) + if err != nil { + t.Fatalf("GetNamedSecurityInfo(%q) failed: %v", sockPath, err) + } + + // Protected, or ACEs inherited from the parent could widen access. + control, _, err := sd.Control() + if err != nil { + t.Fatalf("Control failed: %v", err) + } + if control&windows.SE_DACL_PROTECTED == 0 { + t.Errorf("DACL is not protected: control = %#x, sd = %q", control, sd) + } + + // Exactly the two ACEs of the SDDL. + dacl, _, err := sd.DACL() + if err != nil { + t.Fatalf("DACL failed: %v", err) + } + if dacl == nil { + t.Fatal("socket has a NULL DACL, which grants everyone full access") + } + if dacl.AceCount != 2 { + t.Errorf("DACL has %d ACEs, want 2: %q", dacl.AceCount, sd) + } + for _, sid := range []string{";;;BA)", ";;;SY)"} { + if !strings.Contains(sd.String(), sid) { + t.Errorf("DACL %q is missing an ACE for %q", sd, sid) + } + } +} diff --git a/plugins/server/internal/listen_windows.go b/plugins/server/internal/listen_windows.go index 32a7e39872..4164646b27 100644 --- a/plugins/server/internal/listen_windows.go +++ b/plugins/server/internal/listen_windows.go @@ -16,8 +16,30 @@ package internal -import "strings" +import ( + "net" + "path/filepath" + "github.com/containerd/containerd/v2/internal/localaddr" +) + +// IsLocalAddress reports whether path is a named pipe or an AF_UNIX socket +// rather than a TCP address, see localaddr.Parse. A bare path is a socket if it +// is absolute, has a drive letter or has no port, as GetLocalListener takes it. func IsLocalAddress(path string) bool { - return strings.HasPrefix(path, `\\.\pipe\`) + if _, _, explicit := localaddr.Parse(path); explicit || filepath.IsAbs(path) || hasDriveLetter(path) { + return true + } + _, _, err := net.SplitHostPort(path) + return err != nil +} + +// hasDriveLetter reports whether path starts with a drive letter, as in +// "C:debug.sock", which SplitHostPort would take for a host and a port. +func hasDriveLetter(path string) bool { + if len(path) < 2 || path[1] != ':' { + return false + } + c := path[0] + return 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z' } diff --git a/plugins/server/internal/listen_windows_test.go b/plugins/server/internal/listen_windows_test.go new file mode 100644 index 0000000000..09fb411b4a --- /dev/null +++ b/plugins/server/internal/listen_windows_test.go @@ -0,0 +1,54 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package internal + +import "testing" + +func TestIsLocalAddress(t *testing.T) { + testcases := []struct { + address string + want bool + }{ + // Named pipes. + {address: `\\.\pipe\containerd-debug`, want: true}, + {address: "//./pipe/containerd-debug", want: true}, + {address: `\\.\PIPE\containerd-debug`, want: true}, + {address: "npipe:////./pipe/containerd-debug", want: true}, + {address: "npipe://./pipe/containerd-debug", want: true}, + + // AF_UNIX sockets. + {address: "unix:///run/containerd/debug.sock", want: true}, + {address: `unix://C:\run\containerd\debug.sock`, want: true}, + {address: `C:\run\containerd\debug.sock`, want: true}, + {address: "debug.sock", want: true}, + {address: "C:debug.sock", want: true}, + {address: `run\debug.sock`, want: true}, + + // TCP. + {address: "127.0.0.1:6060", want: false}, + {address: "localhost:6060", want: false}, + {address: ":6060", want: false}, + {address: "[::1]:6060", want: false}, + } + for _, tc := range testcases { + t.Run(tc.address, func(t *testing.T) { + if got := IsLocalAddress(tc.address); got != tc.want { + t.Errorf("IsLocalAddress(%q) = %v, want %v", tc.address, got, tc.want) + } + }) + } +}