Merge commit from fork

[26.0] AuthZ plugin security fixes

.github/workflows/.windows.yml

@@ -19,7 +19,7 @@ on:
         default: false
 
 env:
-  GO_VERSION: "1.21.9"
+  GO_VERSION: "1.21.10"
   GOTESTLIST_VERSION: v0.3.1
   TESTSTAT_VERSION: v0.1.25
   WINDOWS_BASE_IMAGE: mcr.microsoft.com/windows/servercore

.github/workflows/buildkit.yml

@@ -13,7 +13,7 @@ on:
   pull_request:
 
 env:
-  GO_VERSION: "1.21.9"
+  GO_VERSION: "1.21.10"
   DESTDIR: ./build
 
 jobs:

.github/workflows/test.yml

@@ -13,7 +13,7 @@ on:
   pull_request:
 
 env:
-  GO_VERSION: "1.21.9"
+  GO_VERSION: "1.21.10"
   GIT_PAGER: "cat"
   PAGER: "cat"
 

Dockerfile

@@ -1,6 +1,6 @@
 # syntax=docker/dockerfile:1
 
-ARG GO_VERSION=1.21.9
+ARG GO_VERSION=1.21.10
 ARG BASE_DEBIAN_DISTRO="bookworm"
 ARG GOLANG_IMAGE="golang:${GO_VERSION}-${BASE_DEBIAN_DISTRO}"
 ARG XX_VERSION=1.4.0

Dockerfile.simple

@@ -5,7 +5,7 @@
 
 # This represents the bare minimum required to build and test Docker.
 
-ARG GO_VERSION=1.21.9
+ARG GO_VERSION=1.21.10
 
 ARG BASE_DEBIAN_DISTRO="bookworm"
 ARG GOLANG_IMAGE="golang:${GO_VERSION}-${BASE_DEBIAN_DISTRO}"

Dockerfile.windows

@@ -161,7 +161,7 @@ FROM ${WINDOWS_BASE_IMAGE}:${WINDOWS_BASE_IMAGE_TAG}
 # Use PowerShell as the default shell
 SHELL ["powershell", "-Command", "$ErrorActionPreference = 'Stop'; $ProgressPreference = 'SilentlyContinue';"]
 
-ARG GO_VERSION=1.21.9
+ARG GO_VERSION=1.21.10
 ARG GOTESTSUM_VERSION=v1.8.2
 ARG GOWINRES_VERSION=v0.3.1
 ARG CONTAINERD_VERSION=v1.7.15

daemon/images/image_history.go

@@ -43,9 +43,14 @@ func (i *ImageService) ImageHistory(ctx context.Context, name string) ([]*image.
 			layerCounter++
 		}
 
+		var created int64
+		if h.Created != nil {
+			created = h.Created.Unix()
+		}
+
 		history = append([]*image.HistoryResponseItem{{
 			ID:        "<missing>",
-			Created:   h.Created.Unix(),
+			Created:   created,
 			CreatedBy: h.CreatedBy,
 			Comment:   h.Comment,
 			Size:      layerSize,

hack/dockerfiles/generate-files.Dockerfile

@@ -1,6 +1,6 @@
 # syntax=docker/dockerfile:1
 
-ARG GO_VERSION=1.21.9
+ARG GO_VERSION=1.21.10
 ARG BASE_DEBIAN_DISTRO="bookworm"
 ARG PROTOC_VERSION=3.11.4
 

integration/network/ipvlan/ipvlan_test.go

@@ -94,6 +94,9 @@ func TestDockerNetworkIpvlan(t *testing.T) {
 		}, {
 			name: "L3Addressing",
 			test: testIpvlanL3Addressing,
+		}, {
+			name: "NoIPv6",
+			test: testIpvlanNoIPv6,
 		},
 	} {
 
@@ -441,6 +444,28 @@ func testIpvlanL3Addressing(t *testing.T, ctx context.Context, client dclient.AP
 	assert.Check(t, is.Contains(result.Combined(), "default dev eth0"))
 }
 
+// Check that an ipvlan interface with '--ipv6=false' doesn't get kernel-assigned
+// IPv6 addresses, but the loopback interface does still have an IPv6 address ('::1').
+func testIpvlanNoIPv6(t *testing.T, ctx context.Context, client dclient.APIClient) {
+	const netName = "ipvlannet"
+	net.CreateNoError(ctx, t, client, netName, net.WithIPvlan("", "l3"))
+	assert.Check(t, n.IsNetworkAvailable(ctx, client, netName))
+
+	id := container.Run(ctx, t, client, container.WithNetworkMode(netName))
+
+	loRes := container.ExecT(ctx, t, client, id, []string{"ip", "a", "show", "dev", "lo"})
+	assert.Check(t, is.Contains(loRes.Combined(), " inet "))
+	assert.Check(t, is.Contains(loRes.Combined(), " inet6 "))
+
+	eth0Res := container.ExecT(ctx, t, client, id, []string{"ip", "a", "show", "dev", "eth0"})
+	assert.Check(t, is.Contains(eth0Res.Combined(), " inet "))
+	assert.Check(t, !strings.Contains(eth0Res.Combined(), " inet6 "),
+		"result.Combined(): %s", eth0Res.Combined())
+
+	sysctlRes := container.ExecT(ctx, t, client, id, []string{"sysctl", "-n", "net.ipv6.conf.eth0.disable_ipv6"})
+	assert.Check(t, is.Equal(strings.TrimSpace(sysctlRes.Combined()), "1"))
+}
+
 // TestIPVlanDNS checks whether DNS is forwarded, for combinations of l2/l3 mode,
 // with/without a parent interface, and with '--internal'. Note that, there's no
 // attempt here to give the ipvlan network external connectivity - when this test
@@ -452,7 +477,6 @@ func testIpvlanL3Addressing(t *testing.T, ctx context.Context, client dclient.AP
 // https://github.com/moby/moby/issues/47662
 func TestIPVlanDNS(t *testing.T) {
 	skip.If(t, testEnv.IsRootless, "rootless mode has different view of network")
-
 	ctx := testutil.StartSpan(baseContext, t)
 
 	net.StartDaftDNS(t, "127.0.0.1")

integration/network/macvlan/macvlan_test.go

@@ -77,6 +77,9 @@ func TestDockerNetworkMacvlan(t *testing.T) {
 		}, {
 			name: "Addressing",
 			test: testMacvlanAddressing,
+		}, {
+			name: "NoIPv6",
+			test: testMacvlanNoIPv6,
 		},
 	} {
 		tc := tc
@@ -298,6 +301,32 @@ func testMacvlanAddressing(t *testing.T, ctx context.Context, client client.APIC
 	assert.Check(t, strings.Contains(result.Combined(), "default via 2001:db8:abca::254 dev eth0"))
 }
 
+// Check that a macvlan interface with '--ipv6=false' doesn't get kernel-assigned
+// IPv6 addresses, but the loopback interface does still have an IPv6 address ('::1').
+func testMacvlanNoIPv6(t *testing.T, ctx context.Context, client client.APIClient) {
+	const netName = "macvlannet"
+
+	net.CreateNoError(ctx, t, client, netName,
+		net.WithMacvlan(""),
+		net.WithOption("macvlan_mode", "bridge"),
+	)
+	assert.Check(t, n.IsNetworkAvailable(ctx, client, netName))
+
+	id := container.Run(ctx, t, client, container.WithNetworkMode(netName))
+
+	loRes := container.ExecT(ctx, t, client, id, []string{"ip", "a", "show", "dev", "lo"})
+	assert.Check(t, is.Contains(loRes.Combined(), " inet "))
+	assert.Check(t, is.Contains(loRes.Combined(), " inet6 "))
+
+	eth0Res := container.ExecT(ctx, t, client, id, []string{"ip", "a", "show", "dev", "eth0"})
+	assert.Check(t, is.Contains(eth0Res.Combined(), " inet "))
+	assert.Check(t, !strings.Contains(eth0Res.Combined(), " inet6 "),
+		"result.Combined(): %s", eth0Res.Combined())
+
+	sysctlRes := container.ExecT(ctx, t, client, id, []string{"sysctl", "-n", "net.ipv6.conf.eth0.disable_ipv6"})
+	assert.Check(t, is.Equal(strings.TrimSpace(sysctlRes.Combined()), "1"))
+}
+
 // TestMACVlanDNS checks whether DNS is forwarded, with/without a parent
 // interface, and with '--internal'. Note that there's no attempt here to give
 // the macvlan network external connectivity - when this test supplies a parent

integration/networking/bridge_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"regexp"
+	"strings"
 	"testing"
 	"time"
 
@@ -611,8 +612,8 @@ func TestInternalNwConnectivity(t *testing.T) {
 	assert.Check(t, is.Contains(res.Stderr(), "Network is unreachable"))
 }
 
-// Check that the container's interface has no IPv6 address when IPv6 is
-// disabled in a container via sysctl.
+// Check that the container's interfaces have no IPv6 address when IPv6 is
+// disabled in a container via sysctl (including 'lo').
 func TestDisableIPv6Addrs(t *testing.T) {
 	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
 
@@ -676,6 +677,40 @@ func TestDisableIPv6Addrs(t *testing.T) {
 	}
 }
 
+// Check that an interface to an '--ipv6=false' network has no IPv6
+// address - either IPAM assigned, or kernel-assigned LL, but the loopback
+// interface does still have an IPv6 address ('::1').
+func TestNonIPv6Network(t *testing.T) {
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
+
+	ctx := setupTest(t)
+	d := daemon.New(t)
+	d.StartWithBusybox(ctx, t)
+	defer d.Stop(t)
+
+	c := d.NewClientT(t)
+	defer c.Close()
+
+	const netName = "testnet"
+	network.CreateNoError(ctx, t, c, netName)
+	defer network.RemoveNoError(ctx, t, c, netName)
+
+	id := container.Run(ctx, t, c, container.WithNetworkMode(netName))
+	defer c.ContainerRemove(ctx, id, containertypes.RemoveOptions{Force: true})
+
+	loRes := container.ExecT(ctx, t, c, id, []string{"ip", "a", "show", "dev", "lo"})
+	assert.Check(t, is.Contains(loRes.Combined(), " inet "))
+	assert.Check(t, is.Contains(loRes.Combined(), " inet6 "))
+
+	eth0Res := container.ExecT(ctx, t, c, id, []string{"ip", "a", "show", "dev", "eth0"})
+	assert.Check(t, is.Contains(eth0Res.Combined(), " inet "))
+	assert.Check(t, !strings.Contains(eth0Res.Combined(), " inet6 "),
+		"result.Combined(): %s", eth0Res.Combined())
+
+	sysctlRes := container.ExecT(ctx, t, c, id, []string{"sysctl", "-n", "net.ipv6.conf.eth0.disable_ipv6"})
+	assert.Check(t, is.Equal(strings.TrimSpace(sysctlRes.Combined()), "1"))
+}
+
 // Test that it's possible to set a sysctl on an interface in the container.
 // Regression test for https://github.com/moby/moby/issues/47619
 func TestSetInterfaceSysctl(t *testing.T) {

libnetwork/osl/interface_linux.go

@@ -363,17 +363,24 @@ func setInterfaceIP(nlh *netlink.Handle, iface netlink.Link, i *Interface) error
 }
 
 func setInterfaceIPv6(nlh *netlink.Handle, iface netlink.Link, i *Interface) error {
-	if i.AddressIPv6() == nil {
+	addr := i.AddressIPv6()
+	// IPv6 must be enabled on the interface if and only if the network is
+	// IPv6-enabled. For an interface on an IPv4-only network, if IPv6 isn't
+	// disabled, the interface will be put into IPv6 multicast groups making
+	// it unexpectedly susceptible to NDP cache poisoning, route injection, etc.
+	// (At present, there will always be a pre-configured IPv6 address if the
+	// network is IPv6-enabled.)
+	if err := setIPv6(i.ns.path, i.DstName(), addr != nil); err != nil {
+		return fmt.Errorf("failed to configure ipv6: %v", err)
+	}
+	if addr == nil {
 		return nil
 	}
-	if err := checkRouteConflict(nlh, i.AddressIPv6(), netlink.FAMILY_V6); err != nil {
+	if err := checkRouteConflict(nlh, addr, netlink.FAMILY_V6); err != nil {
 		return err
 	}
-	if err := setIPv6(i.ns.path, i.DstName(), true); err != nil {
-		return fmt.Errorf("failed to enable ipv6: %v", err)
-	}
-	ipAddr := &netlink.Addr{IPNet: i.AddressIPv6(), Label: "", Flags: syscall.IFA_F_NODAD}
-	return nlh.AddrAdd(iface, ipAddr)
+	nlAddr := &netlink.Addr{IPNet: addr, Label: "", Flags: syscall.IFA_F_NODAD}
+	return nlh.AddrAdd(iface, nlAddr)
 }
 
 func setInterfaceLinkLocalIPs(nlh *netlink.Handle, iface netlink.Link, i *Interface) error {

pkg/authorization/authz.go

@@ -8,6 +8,8 @@ import (
 	"io"
 	"mime"
 	"net/http"
+	"net/url"
+	"regexp"
 	"strings"
 
 	"github.com/containerd/log"
@@ -53,10 +55,23 @@ type Ctx struct {
 	authReq *Request
 }
 
+func isChunked(r *http.Request) bool {
+	// RFC 7230 specifies that content length is to be ignored if Transfer-Encoding is chunked
+	if strings.EqualFold(r.Header.Get("Transfer-Encoding"), "chunked") {
+		return true
+	}
+	for _, v := range r.TransferEncoding {
+		if strings.EqualFold(v, "chunked") {
+			return true
+		}
+	}
+	return false
+}
+
 // AuthZRequest authorized the request to the docker daemon using authZ plugins
 func (ctx *Ctx) AuthZRequest(w http.ResponseWriter, r *http.Request) error {
 	var body []byte
-	if sendBody(ctx.requestURI, r.Header) && r.ContentLength > 0 && r.ContentLength < maxBodySize {
+	if sendBody(ctx.requestURI, r.Header) && (r.ContentLength > 0 || isChunked(r)) && r.ContentLength < maxBodySize {
 		var err error
 		body, r.Body, err = drainBody(r.Body)
 		if err != nil {
@@ -109,7 +124,6 @@ func (ctx *Ctx) AuthZResponse(rm ResponseModifier, r *http.Request) error {
 	if sendBody(ctx.requestURI, rm.Header()) {
 		ctx.authReq.ResponseBody = rm.RawBody()
 	}
-
 	for _, plugin := range ctx.plugins {
 		log.G(context.TODO()).Debugf("AuthZ response using plugin %s", plugin.Name())
 
@@ -147,10 +161,26 @@ func drainBody(body io.ReadCloser) ([]byte, io.ReadCloser, error) {
 	return nil, newBody, err
 }
 
+func isAuthEndpoint(urlPath string) (bool, error) {
+	// eg www.test.com/v1.24/auth/optional?optional1=something&optional2=something (version optional)
+	matched, err := regexp.MatchString(`^[^\/]*\/(v\d[\d\.]*\/)?auth.*`, urlPath)
+	if err != nil {
+		return false, err
+	}
+	return matched, nil
+}
+
 // sendBody returns true when request/response body should be sent to AuthZPlugin
-func sendBody(url string, header http.Header) bool {
+func sendBody(inURL string, header http.Header) bool {
+	u, err := url.Parse(inURL)
+	// Assume no if the URL cannot be parsed - an empty request will still be forwarded to the plugin and should be rejected
+	if err != nil {
+		return false
+	}
+
 	// Skip body for auth endpoint
-	if strings.HasSuffix(url, "/auth") {
+	isAuth, err := isAuthEndpoint(u.Path)
+	if isAuth || err != nil {
 		return false
 	}
 

pkg/authorization/authz_unix_test.go

@@ -174,8 +174,8 @@ func TestDrainBody(t *testing.T) {
 
 func TestSendBody(t *testing.T) {
 	var (
-		url       = "nothing.com"
 		testcases = []struct {
+			url         string
 			contentType string
 			expected    bool
 		}{
@@ -219,15 +219,93 @@ func TestSendBody(t *testing.T) {
 				contentType: "",
 				expected:    false,
 			},
+			{
+				url:         "nothing.com/auth",
+				contentType: "",
+				expected:    false,
+			},
+			{
+				url:         "nothing.com/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "nothing.com/auth?p1=test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "nothing.com/test?p1=/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
+			{
+				url:         "nothing.com/something/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
+			{
+				url:         "nothing.com/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "nothing.com/v1.24/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "nothing.com/v1/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "www.nothing.com/v1.24/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "https://www.nothing.com/v1.24/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "http://nothing.com/v1.24/auth/test",
+				contentType: "application/json;charset=UTF8",
+				expected:    false,
+			},
+			{
+				url:         "www.nothing.com/test?p1=/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
+			{
+				url:         "http://www.nothing.com/test?p1=/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
+			{
+				url:         "www.nothing.com/something/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
+			{
+				url:         "https://www.nothing.com/something/auth",
+				contentType: "application/json;charset=UTF8",
+				expected:    true,
+			},
 		}
 	)
 
 	for _, testcase := range testcases {
 		header := http.Header{}
 		header.Set("Content-Type", testcase.contentType)
+		if testcase.url == "" {
+			testcase.url = "nothing.com"
+		}
 
-		if b := sendBody(url, header); b != testcase.expected {
-			t.Fatalf("Unexpected Content-Type; Expected: %t, Actual: %t", testcase.expected, b)
+		if b := sendBody(testcase.url, header); b != testcase.expected {
+			t.Fatalf("sendBody failed: url: %s, content-type: %s; Expected: %t, Actual: %t", testcase.url, testcase.contentType, testcase.expected, b)
 		}
 	}
 }