Skip to content

Commit

Permalink
Refactor IPv6 subnet validation
Browse files Browse the repository at this point in the history
- Remove package variable bridge.bridgeIPv6
- Use netip in more places
- Improve error messages from fixed-cidr-v6 checks

Signed-off-by: Rob Murray <rob.murray@docker.com>
  • Loading branch information
robmry committed May 1, 2024
1 parent f46473b commit aa3a86c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 24 deletions.
31 changes: 18 additions & 13 deletions libnetwork/drivers/bridge/bridge_linux.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
package bridge

import (
"bytes"
"context"
"fmt"
"net"
"net/netip"
"os"
"os/exec"
"strconv"
Expand All @@ -14,6 +14,7 @@ import (
"github.com/docker/docker/errdefs"
"github.com/docker/docker/libnetwork/datastore"
"github.com/docker/docker/libnetwork/driverapi"
"github.com/docker/docker/libnetwork/internal/netiputil"
"github.com/docker/docker/libnetwork/iptables"
"github.com/docker/docker/libnetwork/netlabel"
"github.com/docker/docker/libnetwork/netutils"
Expand Down Expand Up @@ -188,9 +189,12 @@ func Register(r driverapi.Registerer, config map[string]interface{}) error {
// added to the interface's Prefix List (RFC-5942), differing prefix lengths
// would be confusing and have been disallowed by earlier implementations of
// bridge address assignment.
func validateIPv6Subnet(addr *net.IPNet) error {
if addr != nil && bridgeIPv6.Contains(addr.IP) && !bytes.Equal(bridgeIPv6.Mask, addr.Mask) {
return errdefs.InvalidParameter(errors.New("clash with the Link-Local prefix 'fe80::/64'"))
func validateIPv6Subnet(addr netip.Prefix) error {
if !addr.Addr().Is6() || addr.Addr().Is4In6() {
return fmt.Errorf("'%s' is not a valid IPv6 subnet", addr)
}
if addr.Masked() != linkLocalPrefix && linkLocalPrefix.Overlaps(addr) {
return fmt.Errorf("'%s' clashes with the Link-Local prefix 'fe80::/64'", addr)
}
return nil
}
Expand All @@ -201,14 +205,11 @@ func ValidateFixedCIDRV6(val string) error {
if val == "" {
return nil
}
ip, ipNet, err := net.ParseCIDR(val)
if err != nil {
return errdefs.InvalidParameter(err)
}
if ip.To4() != nil {
return errdefs.InvalidParameter(errors.New("fixed-cidr-v6 is not an IPv6 subnet"))
prefix, err := netip.ParsePrefix(val)
if err == nil {
err = validateIPv6Subnet(prefix)
}
return validateIPv6Subnet(ipNet)
return errdefs.InvalidParameter(errors.Wrap(err, "invalid fixed-cidr-v6"))
}

// Validate performs a static validation on the network configuration parameters.
Expand All @@ -234,8 +235,12 @@ func (c *networkConfiguration) Validate() error {
return errdefs.System(errors.New("no IPv6 address was allocated for the bridge"))
}
// AddressIPv6 must be IPv6, and not overlap with the LL subnet prefix.
if err := validateIPv6Subnet(c.AddressIPv6); err != nil {
return err
addr, ok := netiputil.ToPrefix(c.AddressIPv6)
if !ok {
return errdefs.InvalidParameter(fmt.Errorf("invalid IPv6 address '%s'", c.AddressIPv6))
}
if err := validateIPv6Subnet(addr); err != nil {
return errdefs.InvalidParameter(err)
}
// If a default gw is specified, it must belong to AddressIPv6's subnet
if c.DefaultGatewayIPv6 != nil && !c.AddressIPv6.Contains(c.DefaultGatewayIPv6) {
Expand Down
14 changes: 7 additions & 7 deletions libnetwork/drivers/bridge/bridge_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1012,39 +1012,39 @@ func TestValidateFixedCIDRV6(t *testing.T) {
// Overlapping with the standard LL prefix isn't allowed.
doc: "overlapping link-local prefix fe80::/63",
input: "fe80::/63",
expectedErr: "clash with the Link-Local prefix 'fe80::/64'",
expectedErr: "invalid fixed-cidr-v6: 'fe80::/63' clashes with the Link-Local prefix 'fe80::/64'",
},
{
// Overlapping with the standard LL prefix isn't allowed.
doc: "overlapping link-local subnet fe80::/65",
input: "fe80::/65",
expectedErr: "clash with the Link-Local prefix 'fe80::/64'",
expectedErr: "invalid fixed-cidr-v6: 'fe80::/65' clashes with the Link-Local prefix 'fe80::/64'",
},
{
// The address has to be valid IPv6 subnet.
doc: "invalid IPv6 subnet",
input: "2000:db8::",
expectedErr: "invalid CIDR address: 2000:db8::",
expectedErr: "invalid fixed-cidr-v6: netip.ParsePrefix(\"2000:db8::\"): no '/'",
},
{
doc: "non-IPv6 subnet",
input: "10.3.4.5/24",
expectedErr: "fixed-cidr-v6 is not an IPv6 subnet",
expectedErr: "invalid fixed-cidr-v6: '10.3.4.5/24' is not a valid IPv6 subnet",
},
{
doc: "IPv4-mapped subnet 1",
input: "::ffff:10.2.4.0/24",
expectedErr: "fixed-cidr-v6 is not an IPv6 subnet",
expectedErr: "invalid fixed-cidr-v6: '::ffff:10.2.4.0/24' is not a valid IPv6 subnet",
},
{
doc: "IPv4-mapped subnet 2",
input: "::ffff:a01:203/24",
expectedErr: "fixed-cidr-v6 is not an IPv6 subnet",
expectedErr: "invalid fixed-cidr-v6: '::ffff:10.1.2.3/24' is not a valid IPv6 subnet",
},
{
doc: "invalid subnet",
input: "nonsense",
expectedErr: "invalid CIDR address: nonsense",
expectedErr: "invalid fixed-cidr-v6: netip.ParsePrefix(\"nonsense\"): no '/'",
},
}
for _, tc := range tests {
Expand Down
4 changes: 0 additions & 4 deletions libnetwork/drivers/bridge/setup_ipv6_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@ package bridge
import (
"context"
"fmt"
"net"
"net/netip"
"os"

"github.com/containerd/log"
"github.com/vishvananda/netlink"
)

// bridgeIPv6 is the default, link-local IPv6 address for the bridge (fe80::1/64)
var bridgeIPv6 = &net.IPNet{IP: net.ParseIP("fe80::1"), Mask: net.CIDRMask(64, 128)}

// Standard link local prefix
var linkLocalPrefix = netip.MustParsePrefix("fe80::/64")

Expand Down

0 comments on commit aa3a86c

Please sign in to comment.