瀏覽代碼

api: Validate IPAM config before creating a network

Currently, IPAM config is never validated by the API. Some checks
are done by the CLI, but they're not exhaustive. And some of these
misconfigurations might be caught early by libnetwork (ie. when the
network is created), and others only surface when connecting a container
to a misconfigured network. In both cases, the API would return a 500.

Although the `NetworkCreate` endpoint might already return warnings,
these are never displayed by the CLI. As such, it was decided during a
maintainer's call to return validation errors _for all API versions_.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Albin Kerouanton 2 年之前
父節點
當前提交
4f47013feb

+ 4 - 0
api/swagger.yaml

@@ -9929,6 +9929,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: |
             Forbidden operation. This happens when trying to create a network named after a pre-defined network,

+ 116 - 0
api/types/network/ipam.go

@@ -1,5 +1,13 @@
 package network
 
+import (
+	"errors"
+	"fmt"
+	"net/netip"
+
+	"github.com/docker/docker/internal/multierror"
+)
+
 // IPAM represents IP Address Management
 type IPAM struct {
 	Driver  string
@@ -14,3 +22,111 @@ type IPAMConfig struct {
 	Gateway    string            `json:",omitempty"`
 	AuxAddress map[string]string `json:"AuxiliaryAddresses,omitempty"`
 }
+
+type ipFamily string
+
+const (
+	ip4 ipFamily = "IPv4"
+	ip6 ipFamily = "IPv6"
+)
+
+func ValidateIPAM(ipam *IPAM, enableIPv6 bool) error {
+	if ipam == nil {
+		return nil
+	}
+
+	var errs []error
+	for _, cfg := range ipam.Config {
+		subnet, err := netip.ParsePrefix(cfg.Subnet)
+		if err != nil {
+			errs = append(errs, fmt.Errorf("invalid subnet %s: invalid CIDR block notation", cfg.Subnet))
+			continue
+		}
+		subnetFamily := ip4
+		if subnet.Addr().Is6() {
+			subnetFamily = ip6
+		}
+
+		if subnet != subnet.Masked() {
+			errs = append(errs, fmt.Errorf("invalid subnet %s: it should be %s", subnet, subnet.Masked()))
+		}
+
+		if !enableIPv6 && subnetFamily == ip6 {
+			errs = append(errs, fmt.Errorf("invalid subnet %s: IPv6 has not been enabled for this network", subnet))
+		}
+
+		if ipRangeErrs := validateIPRange(cfg.IPRange, subnet, subnetFamily); len(ipRangeErrs) > 0 {
+			errs = append(errs, ipRangeErrs...)
+		}
+
+		if err := validateAddress(cfg.Gateway, subnet, subnetFamily); err != nil {
+			errs = append(errs, fmt.Errorf("invalid gateway %s: %w", cfg.Gateway, err))
+		}
+
+		for auxName, aux := range cfg.AuxAddress {
+			if err := validateAddress(aux, subnet, subnetFamily); err != nil {
+				errs = append(errs, fmt.Errorf("invalid auxiliary address %s: %w", auxName, err))
+			}
+		}
+	}
+
+	if err := multierror.Join(errs...); err != nil {
+		return fmt.Errorf("invalid network config:\n%w", err)
+	}
+
+	return nil
+}
+
+func validateIPRange(ipRange string, subnet netip.Prefix, subnetFamily ipFamily) []error {
+	if ipRange == "" {
+		return nil
+	}
+	prefix, err := netip.ParsePrefix(ipRange)
+	if err != nil {
+		return []error{fmt.Errorf("invalid ip-range %s: invalid CIDR block notation", ipRange)}
+	}
+	family := ip4
+	if prefix.Addr().Is6() {
+		family = ip6
+	}
+
+	if family != subnetFamily {
+		return []error{fmt.Errorf("invalid ip-range %s: parent subnet is an %s block", ipRange, subnetFamily)}
+	}
+
+	var errs []error
+	if prefix.Bits() < subnet.Bits() {
+		errs = append(errs, fmt.Errorf("invalid ip-range %s: CIDR block is bigger than its parent subnet %s", ipRange, subnet))
+	}
+	if prefix != prefix.Masked() {
+		errs = append(errs, fmt.Errorf("invalid ip-range %s: it should be %s", prefix, prefix.Masked()))
+	}
+	if !subnet.Overlaps(prefix) {
+		errs = append(errs, fmt.Errorf("invalid ip-range %s: parent subnet %s doesn't contain ip-range", ipRange, subnet))
+	}
+
+	return errs
+}
+
+func validateAddress(address string, subnet netip.Prefix, subnetFamily ipFamily) error {
+	if address == "" {
+		return nil
+	}
+	addr, err := netip.ParseAddr(address)
+	if err != nil {
+		return errors.New("invalid address")
+	}
+	family := ip4
+	if addr.Is6() {
+		family = ip6
+	}
+
+	if family != subnetFamily {
+		return fmt.Errorf("parent subnet is an %s block", subnetFamily)
+	}
+	if !subnet.Contains(addr) {
+		return fmt.Errorf("parent subnet %s doesn't contain this address", subnet)
+	}
+
+	return nil
+}

+ 143 - 0
api/types/network/ipam_test.go

@@ -0,0 +1,143 @@
+package network
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+)
+
+func TestNetworkWithInvalidIPAM(t *testing.T) {
+	testcases := []struct {
+		name           string
+		ipam           IPAM
+		ipv6           bool
+		expectedErrors []string
+	}{
+		{
+			name: "IP version mismatch",
+			ipam: IPAM{
+				Config: []IPAMConfig{{
+					Subnet:     "10.10.10.0/24",
+					IPRange:    "2001:db8::/32",
+					Gateway:    "2001:db8::1",
+					AuxAddress: map[string]string{"DefaultGatewayIPv4": "2001:db8::1"},
+				}},
+			},
+			expectedErrors: []string{
+				"invalid ip-range 2001:db8::/32: parent subnet is an IPv4 block",
+				"invalid gateway 2001:db8::1: parent subnet is an IPv4 block",
+				"invalid auxiliary address DefaultGatewayIPv4: parent subnet is an IPv4 block",
+			},
+		},
+		{
+			name:           "IPv6 subnet is discarded when IPv6 is disabled",
+			ipam:           IPAM{Config: []IPAMConfig{{Subnet: "2001:db8::/32"}}},
+			ipv6:           false,
+			expectedErrors: []string{"invalid subnet 2001:db8::/32: IPv6 has not been enabled for this network"},
+		},
+		{
+			name: "Invalid data - Subnet",
+			ipam: IPAM{Config: []IPAMConfig{{Subnet: "foobar"}}},
+			expectedErrors: []string{
+				`invalid subnet foobar: invalid CIDR block notation`,
+			},
+		},
+		{
+			name: "Invalid data",
+			ipam: IPAM{
+				Config: []IPAMConfig{{
+					Subnet:     "10.10.10.0/24",
+					IPRange:    "foobar",
+					Gateway:    "1001.10.5.3",
+					AuxAddress: map[string]string{"DefaultGatewayIPv4": "dummy"},
+				}},
+			},
+			expectedErrors: []string{
+				"invalid ip-range foobar: invalid CIDR block notation",
+				"invalid gateway 1001.10.5.3: invalid address",
+				"invalid auxiliary address DefaultGatewayIPv4: invalid address",
+			},
+		},
+		{
+			name: "IPRange bigger than its subnet",
+			ipam: IPAM{
+				Config: []IPAMConfig{
+					{Subnet: "10.10.10.0/24", IPRange: "10.0.0.0/8"},
+				},
+			},
+			expectedErrors: []string{
+				"invalid ip-range 10.0.0.0/8: CIDR block is bigger than its parent subnet 10.10.10.0/24",
+			},
+		},
+		{
+			name: "Out of range prefix & addresses",
+			ipam: IPAM{
+				Config: []IPAMConfig{{
+					Subnet:     "10.0.0.0/8",
+					IPRange:    "192.168.0.1/24",
+					Gateway:    "192.168.0.1",
+					AuxAddress: map[string]string{"DefaultGatewayIPv4": "192.168.0.1"},
+				}},
+			},
+			expectedErrors: []string{
+				"invalid ip-range 192.168.0.1/24: it should be 192.168.0.0/24",
+				"invalid ip-range 192.168.0.1/24: parent subnet 10.0.0.0/8 doesn't contain ip-range",
+				"invalid gateway 192.168.0.1: parent subnet 10.0.0.0/8 doesn't contain this address",
+				"invalid auxiliary address DefaultGatewayIPv4: parent subnet 10.0.0.0/8 doesn't contain this address",
+			},
+		},
+		{
+			name: "Subnet with host fragment set",
+			ipam: IPAM{
+				Config: []IPAMConfig{{
+					Subnet: "10.10.10.0/8",
+				}},
+			},
+			expectedErrors: []string{"invalid subnet 10.10.10.0/8: it should be 10.0.0.0/8"},
+		},
+		{
+			name: "IPRange with host fragment set",
+			ipam: IPAM{
+				Config: []IPAMConfig{{
+					Subnet:  "10.0.0.0/8",
+					IPRange: "10.10.10.0/16",
+				}},
+			},
+			expectedErrors: []string{"invalid ip-range 10.10.10.0/16: it should be 10.10.0.0/16"},
+		},
+		{
+			name: "Empty IPAM is valid",
+			ipam: IPAM{},
+		},
+		{
+			name: "Valid IPAM",
+			ipam: IPAM{
+				Config: []IPAMConfig{{
+					Subnet:     "10.0.0.0/8",
+					IPRange:    "10.10.0.0/16",
+					Gateway:    "10.10.0.1",
+					AuxAddress: map[string]string{"DefaultGatewayIPv4": "10.10.0.1"},
+				}},
+			},
+		},
+	}
+
+	for _, tc := range testcases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			errs := ValidateIPAM(&tc.ipam, tc.ipv6)
+			if tc.expectedErrors == nil {
+				assert.NilError(t, errs)
+				return
+			}
+
+			assert.Check(t, is.ErrorContains(errs, "invalid network config"))
+			for _, expected := range tc.expectedErrors {
+				assert.Check(t, is.ErrorContains(errs, expected))
+			}
+		})
+	}
+}

+ 3 - 0
daemon/network.go

@@ -346,6 +346,9 @@ func (daemon *Daemon) createNetwork(cfg *config.Config, create types.NetworkCrea
 		nwOptions = append(nwOptions, libnetwork.NetworkOptionConfigOnly())
 	}
 
+	if err := network.ValidateIPAM(create.IPAM, create.EnableIPv6); err != nil {
+		return nil, errdefs.InvalidParameter(err)
+	}
 	if create.IPAM != nil {
 		ipam := create.IPAM
 		v4Conf, v6Conf, err := getIpamConfig(ipam.Config)

+ 4 - 0
docs/api/v1.25.yaml

@@ -6149,6 +6149,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.26.yaml

@@ -6158,6 +6158,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.27.yaml

@@ -6229,6 +6229,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.28.yaml

@@ -6358,6 +6358,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.29.yaml

@@ -6396,6 +6396,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.30.yaml

@@ -6659,6 +6659,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.31.yaml

@@ -6757,6 +6757,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.32.yaml

@@ -7798,6 +7798,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.33.yaml

@@ -7807,6 +7807,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.34.yaml

@@ -7848,6 +7848,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.35.yaml

@@ -7860,6 +7860,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.36.yaml

@@ -7902,6 +7902,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.37.yaml

@@ -7945,6 +7945,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.38.yaml

@@ -8006,6 +8006,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.39.yaml

@@ -8955,6 +8955,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.40.yaml

@@ -9292,6 +9292,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.41.yaml

@@ -9499,6 +9499,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.42.yaml

@@ -9877,6 +9877,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 4 - 0
docs/api/v1.43.yaml

@@ -9895,6 +9895,10 @@ paths:
             example:
               Id: "22be93d5babb089c5aab8dbc369042fad48ff791584ca2da2100db837a1c7c30"
               Warning: ""
+        400:
+          description: "bad parameter"
+          schema:
+            $ref: "#/definitions/ErrorResponse"
         403:
           description: "operation not supported for pre-defined networks"
           schema:

+ 3 - 0
docs/api/version-history.md

@@ -38,6 +38,9 @@ keywords: "API, Docker, rcli, REST, documentation"
   specifications directories. The use of the applied setting requires the daemon
   to have expermental enabled, and for non-experimental daemons an empty list is
   always returned.
+* `POST /networks/create` now returns a 400 if the `IPAMConfig` has invalid
+  values. Note that this change is _unversioned_ and applied to all API
+  versions on daemon that support version 1.44.
 
 ## v1.43 API changes
 

+ 4 - 1
hack/make/test-docker-py

@@ -23,7 +23,10 @@ source hack/make/.integration-test-helpers
 # TODO re-enable test_attach_no_stream after https://github.com/docker/docker-py/issues/2513 is resolved
 # TODO re-enable test_create_with_device_cgroup_rules after https://github.com/docker/docker-py/issues/2939 is resolved
 # TODO re-enable test_prune_volumes after https://github.com/docker/docker-py/pull/3051 is resolved
-: "${PY_TEST_OPTIONS:=--junitxml=${DEST}/junit-report.xml --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream --deselect=tests/integration/api_container_test.py::CreateContainerTest::test_create_with_device_cgroup_rules --deselect=tests/integration/api_volume_test.py::TestVolumes::test_prune_volumes}"
+# TODO re-enable test_connect_with_ipv6_address after we updated to a version of docker-py with https://github.com/docker/docker-py/pull/3169
+# TODO re-enable test_create_network_ipv6_enabled after we updated to a version of docker-py with https://github.com/docker/docker-py/pull/3169
+# TODO re-enable test_create_with_ipv6_address after we updated to a version of docker-py with https://github.com/docker/docker-py/pull/3169
+: "${PY_TEST_OPTIONS:=--junitxml=${DEST}/junit-report.xml --deselect=tests/integration/api_container_test.py::AttachContainerTest::test_attach_no_stream --deselect=tests/integration/api_container_test.py::CreateContainerTest::test_create_with_device_cgroup_rules --deselect=tests/integration/api_volume_test.py::TestVolumes::test_prune_volumes --deselect=tests/integration/api_network_test.py::TestNetworks::test_connect_with_ipv6_address --deselect=tests/integration/api_network_test.py::TestNetworks::test_create_network_ipv6_enabled --deselect=tests/integration/api_network_test.py::TestNetworks::test_create_with_ipv6_address}"
 (
 	bundle .integration-daemon-start