runtime: harden Docker 26+ networking fix

- Replace sandbox ID denylist with positive regex (^[0-9a-f]{64}$)
- Rollback partially-added endpoints on scan failure in scanEndpointsInNs

Signed-off-by: llink5 <llink5@users.noreply.github.com>

src/runtime/virtcontainers/network_linux.go

@@ -46,6 +46,11 @@ type LinuxNetwork struct {
 	interworkingModel NetInterworkingModel
 	netNSCreated      bool
 	danConfigPath     string
+	// placeholderNetNS holds the path to a placeholder network namespace
+	// that we created but later abandoned in favour of the hypervisor's
+	// netns. If best-effort deletion in addAllEndpoints fails, teardown
+	// retries the cleanup via RemoveEndpoints.
+	placeholderNetNS string
 }
 
 // NewNetwork creates a new Linux Network from a NetworkConfig.
@@ -69,11 +74,11 @@ func NewNetwork(configs ...*NetworkConfig) (Network, error) {
 	}
 
 	return &LinuxNetwork{
-		config.NetworkID,
-		[]Endpoint{},
-		config.InterworkingModel,
-		config.NetworkCreated,
-		config.DanConfigPath,
+		netNSPath:         config.NetworkID,
+		eps:               []Endpoint{},
+		interworkingModel: config.InterworkingModel,
+		netNSCreated:      config.NetworkCreated,
+		danConfigPath:     config.DanConfigPath,
 	}, nil
 }
 
@@ -350,8 +355,6 @@ func (n *LinuxNetwork) addAllEndpoints(ctx context.Context, s *Sandbox, hotplug
 			origPath := n.netNSPath
 			origCreated := n.netNSCreated
 			n.netNSPath = hypervisorNs
-			// The hypervisor's namespace was not created by us.
-			n.netNSCreated = false
 
 			_, err = n.scanEndpointsInNs(ctx, s, n.netNSPath, hotplug)
 			if err != nil {
@@ -362,11 +365,16 @@ func (n *LinuxNetwork) addAllEndpoints(ctx context.Context, s *Sandbox, hotplug
 
 			// Clean up the placeholder namespace we created — we're now
 			// using the hypervisor's namespace and the placeholder is empty.
+			// Only clear netNSCreated once deletion succeeds; on failure,
+			// stash the path so RemoveEndpoints can retry during teardown.
 			if origCreated {
 				if delErr := deleteNetNS(origPath); delErr != nil {
-					networkLogger().WithField("netns", origPath).WithError(delErr).Warn("failed to delete placeholder netns")
+					networkLogger().WithField("netns", origPath).WithError(delErr).Warn("failed to delete placeholder netns, will retry during teardown")
+					n.placeholderNetNS = origPath
 				}
 			}
+			// The hypervisor's namespace was not created by us.
+			n.netNSCreated = false
 		}
 	}
 
@@ -400,7 +408,9 @@ func (n *LinuxNetwork) scanEndpointsInNs(ctx context.Context, s *Sandbox, nsPath
 		return nil, err
 	}
 
+	epsBefore := len(n.eps)
 	var added []Endpoint
+
 	for _, link := range linkList {
 		netInfo, err := networkInfoFromLink(netlinkHandle, link)
 		if err != nil {
@@ -433,6 +443,9 @@ func (n *LinuxNetwork) scanEndpointsInNs(ctx context.Context, s *Sandbox, nsPath
 			}
 			return addErr
 		}); err != nil {
+			// Rollback: remove any endpoints added during this scan
+			// so that a failed scan does not leave partial side effects.
+			n.eps = n.eps[:epsBefore]
 			return nil, err
 		}
 	}
@@ -666,6 +679,17 @@ func (n *LinuxNetwork) RemoveEndpoints(ctx context.Context, s *Sandbox, endpoint
 		return deleteNetNS(n.netNSPath)
 	}
 
+	// Retry cleanup of a placeholder namespace whose earlier deletion
+	// failed in addAllEndpoints.
+	if n.placeholderNetNS != "" && endpoints == nil {
+		if delErr := deleteNetNS(n.placeholderNetNS); delErr != nil {
+			networkLogger().WithField("netns", n.placeholderNetNS).WithError(delErr).Warn("failed to delete placeholder netns during teardown")
+		} else {
+			networkLogger().Infof("Placeholder network namespace %q deleted", n.placeholderNetNS)
+			n.placeholderNetNS = ""
+		}
+	}
+
 	return nil
 }
 

src/runtime/virtcontainers/network_linux_test.go

@@ -363,11 +363,11 @@ func TestConvertDanDeviceToNetworkInfo(t *testing.T) {
 func TestAddEndpoints_Dan(t *testing.T) {
 
 	network := &LinuxNetwork{
-		"net-123",
-		[]Endpoint{},
-		NetXConnectDefaultModel,
-		true,
-		"testdata/dan-config.json",
+		netNSPath:         "net-123",
+		eps:               []Endpoint{},
+		interworkingModel: NetXConnectDefaultModel,
+		netNSCreated:      true,
+		danConfigPath:     "testdata/dan-config.json",
 	}
 
 	ctx := context.TODO()

src/runtime/virtcontainers/sandbox.go

@@ -352,11 +352,14 @@ func (s *Sandbox) RescanNetwork(ctx context.Context) error {
 
 	const maxWait = 5 * time.Second
 	const pollInterval = 50 * time.Millisecond
-	deadline := time.Now().Add(maxWait)
+	deadline := time.NewTimer(maxWait)
+	defer deadline.Stop()
+	ticker := time.NewTicker(pollInterval)
+	defer ticker.Stop()
 
 	s.Logger().Info("waiting for network interfaces in namespace")
 
-	for time.Now().Before(deadline) {
+	for {
 		if _, err := s.network.AddEndpoints(ctx, s, nil, true); err != nil {
 			return err
 		}
@@ -367,12 +370,12 @@ func (s *Sandbox) RescanNetwork(ctx context.Context) error {
 		select {
 		case <-ctx.Done():
 			return ctx.Err()
-		case <-time.After(pollInterval):
+		case <-deadline.C:
+			s.Logger().Warn("no network interfaces found after timeout")
+			return nil
+		case <-ticker.C:
 		}
 	}
-
-	s.Logger().Warn("no network interfaces found after timeout")
-	return nil
 }
 
 // configureGuestNetwork informs the guest agent about discovered network

src/runtime/virtcontainers/utils/utils.go

@@ -12,6 +12,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"syscall"
 	"time"
@@ -503,6 +504,9 @@ const (
 // daemon bind-mounts container network namespaces.
 var dockerNetnsPrefixes = []string{"/var/run/docker/netns/", "/run/docker/netns/"}
 
+// validSandboxID matches Docker sandbox IDs: exactly 64 lowercase hex characters.
+var validSandboxID = regexp.MustCompile(`^[0-9a-f]{64}$`)
+
 // IsDockerContainer returns if the container is managed by docker
 // This is done by checking the prestart and createRuntime hooks for
 // `libnetwork` arguments. Docker 26+ may use CreateRuntime hooks
@@ -551,21 +555,19 @@ func DockerNetnsPath(spec *specs.Spec) string {
 			for i, arg := range hook.Args {
 				if arg == dockerLibnetworkSetkey && i+1 < len(hook.Args) {
 					sandboxID := hook.Args[i+1]
-					// Validate sandbox ID to prevent path traversal.
-					// Docker sandbox IDs are always hex strings.
-					if sandboxID == "" ||
-						strings.Contains(sandboxID, "/") ||
-						strings.Contains(sandboxID, "\\") ||
-						strings.Contains(sandboxID, "..") ||
-						strings.ContainsRune(sandboxID, 0) {
+					// Docker sandbox IDs are exactly 64 lowercase hex
+					// characters. Reject anything else to prevent path
+					// traversal and unexpected input.
+					if !validSandboxID.MatchString(sandboxID) {
 						continue
 					}
 					// Docker stores netns under well-known paths.
-					// Use Lstat to reject symlinks that could point
-					// outside the Docker netns directory.
+					// Use Lstat to reject symlinks (which could point
+					// outside the Docker netns directory) and non-regular
+					// files such as directories.
 					for _, prefix := range dockerNetnsPrefixes {
 						nsPath := prefix + sandboxID
-						if fi, err := os.Lstat(nsPath); err == nil && fi.Mode().Type()&os.ModeSymlink == 0 {
+						if fi, err := os.Lstat(nsPath); err == nil && fi.Mode().IsRegular() {
 							return nsPath
 						}
 					}

src/runtime/virtcontainers/utils/utils_test.go

@@ -628,6 +628,12 @@ func TestIsDockerContainer(t *testing.T) {
 func TestDockerNetnsPath(t *testing.T) {
 	assert := assert.New(t)
 
+	// Valid 64-char hex sandbox IDs for test cases.
+	validID := strings.Repeat("ab", 32)   // 64 hex chars
+	validID2 := strings.Repeat("cd", 32)  // another 64 hex chars
+	invalidShortID := "abc123"             // too short
+	invalidUpperID := strings.Repeat("AB", 32) // uppercase rejected
+
 	// nil spec
 	assert.Equal("", DockerNetnsPath(nil))
 
@@ -644,43 +650,79 @@ func TestDockerNetnsPath(t *testing.T) {
 	}
 	assert.Equal("", DockerNetnsPath(spec))
 
-	// Prestart hook with libnetwork-setkey but netns file doesn't exist
+	// Prestart hook with libnetwork-setkey but sandbox ID too short (rejected by regex)
 	spec = &specs.Spec{
 		Hooks: &specs.Hooks{
 			Prestart: []specs.Hook{ //nolint:all
-				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", "nonexistent999", "ctrl"}},
+				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", invalidShortID, "ctrl"}},
 			},
 		},
 	}
 	assert.Equal("", DockerNetnsPath(spec))
 
-	// Prestart hook with libnetwork-setkey and existing netns file
+	// Prestart hook with libnetwork-setkey but uppercase hex (rejected by regex)
+	spec = &specs.Spec{
+		Hooks: &specs.Hooks{
+			Prestart: []specs.Hook{ //nolint:all
+				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", invalidUpperID, "ctrl"}},
+			},
+		},
+	}
+	assert.Equal("", DockerNetnsPath(spec))
+
+	// Prestart hook with valid sandbox ID but netns file doesn't exist on disk
+	spec = &specs.Spec{
+		Hooks: &specs.Hooks{
+			Prestart: []specs.Hook{ //nolint:all
+				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", validID, "ctrl"}},
+			},
+		},
+	}
+	assert.Equal("", DockerNetnsPath(spec))
+
+	// Prestart hook with libnetwork-setkey and existing netns file — success path
 	tmpDir := t.TempDir()
-	// Simulate /var/run/docker/netns/<id> by creating the file
 	fakeNsDir := filepath.Join(tmpDir, "netns")
 	err := os.MkdirAll(fakeNsDir, 0755)
 	assert.NoError(err)
-	fakeNsFile := filepath.Join(fakeNsDir, "abc123def456")
+	fakeNsFile := filepath.Join(fakeNsDir, validID)
 	err = os.WriteFile(fakeNsFile, []byte{}, 0644)
 	assert.NoError(err)
 
-	// DockerNetnsPath checks hardcoded paths (/var/run/docker/netns/, /run/docker/netns/)
-	// so with a temp dir it won't find it. Verify that non-existent paths return "".
+	// Temporarily override dockerNetnsPrefixes so DockerNetnsPath can find
+	// the netns file we created under the temp directory.
+	origPrefixes := dockerNetnsPrefixes
+	dockerNetnsPrefixes = []string{fakeNsDir + "/"}
+	defer func() { dockerNetnsPrefixes = origPrefixes }()
+
 	spec = &specs.Spec{
 		Hooks: &specs.Hooks{
 			Prestart: []specs.Hook{ //nolint:all
-				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", "abc123def456", "ctrl"}},
+				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", validID, "ctrl"}},
+			},
+		},
+	}
+	assert.Equal(fakeNsFile, DockerNetnsPath(spec))
+
+	// Sandbox ID that is a directory rather than a regular file — must be rejected
+	dirID := validID2
+	err = os.MkdirAll(filepath.Join(fakeNsDir, dirID), 0755)
+	assert.NoError(err)
+	spec = &specs.Spec{
+		Hooks: &specs.Hooks{
+			Prestart: []specs.Hook{ //nolint:all
+				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", dirID, "ctrl"}},
 			},
 		},
 	}
-	// The sandbox ID "abc123def456" won't exist under /var/run/docker/netns/
 	assert.Equal("", DockerNetnsPath(spec))
 
-	// CreateRuntime hook with libnetwork-setkey — same behavior, file doesn't exist
+	// CreateRuntime hook with valid sandbox ID — file doesn't exist
+	validID3 := strings.Repeat("ef", 32)
 	spec = &specs.Spec{
 		Hooks: &specs.Hooks{
 			CreateRuntime: []specs.Hook{
-				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", "xyz789", "ctrl"}},
+				{Args: []string{"/usr/bin/proxy", "libnetwork-setkey", validID3, "ctrl"}},
 			},
 		},
 	}