diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index 7332cdf24b..2ef2040dc1 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -30,7 +30,7 @@ import ( const ( networkType = "bridge" vethPrefix = "veth" - vethLen = 7 + vethLen = len(vethPrefix) + 7 defaultContainerVethPrefix = "eth" maxAllocatePortAttempts = 10 ) diff --git a/libnetwork/drivers/ipvlan/ipvlan.go b/libnetwork/drivers/ipvlan/ipvlan.go index c0e3964c2d..89091b9cba 100644 --- a/libnetwork/drivers/ipvlan/ipvlan.go +++ b/libnetwork/drivers/ipvlan/ipvlan.go @@ -14,9 +14,9 @@ import ( ) const ( - vethLen = 7 containerVethPrefix = "eth" vethPrefix = "veth" + vethLen = len(vethPrefix) + 7 driverName = "ipvlan" // driver type name parentOpt = "parent" // parent interface -o parent diff --git a/libnetwork/drivers/macvlan/macvlan.go b/libnetwork/drivers/macvlan/macvlan.go index d455affa56..eae5bb8176 100644 --- a/libnetwork/drivers/macvlan/macvlan.go +++ b/libnetwork/drivers/macvlan/macvlan.go @@ -14,9 +14,9 @@ import ( ) const ( - vethLen = 7 containerVethPrefix = "eth" vethPrefix = "veth" + vethLen = len(vethPrefix) + 7 driverName = "macvlan" // driver type name modePrivate = "private" // macvlan mode private modeVepa = "vepa" // macvlan mode vepa diff --git a/libnetwork/drivers/overlay/overlay.go b/libnetwork/drivers/overlay/overlay.go index 07084317b7..5daf05e0b3 100644 --- a/libnetwork/drivers/overlay/overlay.go +++ b/libnetwork/drivers/overlay/overlay.go @@ -25,7 +25,7 @@ import ( const ( networkType = "overlay" vethPrefix = "veth" - vethLen = 7 + vethLen = len(vethPrefix) + 7 vxlanIDStart = 256 vxlanIDEnd = (1 << 24) - 1 vxlanEncap = 50 diff --git a/libnetwork/drivers/windows/overlay/overlay_windows.go b/libnetwork/drivers/windows/overlay/overlay_windows.go index d31370246c..727a3f1db6 100644 --- a/libnetwork/drivers/windows/overlay/overlay_windows.go +++ b/libnetwork/drivers/windows/overlay/overlay_windows.go @@ -17,10 +17,7 @@ import ( ) const ( - networkType = "overlay" - vethPrefix = "veth" - vethLen = 7 - secureOption = "encrypted" + networkType = "overlay" ) type driver struct { diff --git a/libnetwork/netutils/utils.go b/libnetwork/netutils/utils.go index 76b2478cb7..49f3c39611 100644 --- a/libnetwork/netutils/utils.go +++ b/libnetwork/netutils/utils.go @@ -121,14 +121,21 @@ func GenerateMACFromIP(ip net.IP) net.HardwareAddr { return genMAC(ip) } -// GenerateRandomName returns a new name joined with a prefix. This size -// specified is used to truncate the randomly generated value -func GenerateRandomName(prefix string, size int) (string, error) { - id := make([]byte, 32) - if _, err := io.ReadFull(rand.Reader, id); err != nil { +// GenerateRandomName returns a string of the specified length, created by joining the prefix to random hex characters. +// The length must be strictly larger than len(prefix), or an error will be returned. +func GenerateRandomName(prefix string, length int) (string, error) { + if length <= len(prefix) { + return "", fmt.Errorf("invalid length %d for prefix %s", length, prefix) + } + + // We add 1 here as integer division will round down, and we want to round up. + b := make([]byte, (length-len(prefix)+1)/2) + if _, err := io.ReadFull(rand.Reader, b); err != nil { return "", err } - return prefix + hex.EncodeToString(id)[:size], nil + + // By taking a slice here, we ensure that the string is always the correct length. + return (prefix + hex.EncodeToString(b))[:length], nil } // ReverseIP accepts a V4 or V6 IP string in the canonical form and returns a reversed IP in diff --git a/libnetwork/netutils/utils_linux.go b/libnetwork/netutils/utils_linux.go index 89aec1ac2a..761cd433a7 100644 --- a/libnetwork/netutils/utils_linux.go +++ b/libnetwork/netutils/utils_linux.go @@ -8,7 +8,6 @@ package netutils import ( "net" "os" - "strings" "github.com/docker/docker/libnetwork/ipamutils" "github.com/docker/docker/libnetwork/ns" @@ -49,11 +48,11 @@ func GenerateIfaceName(nlh *netlink.Handle, prefix string, len int) (string, err for i := 0; i < 3; i++ { name, err := GenerateRandomName(prefix, len) if err != nil { - continue + return "", err } _, err = linkByName(name) if err != nil { - if strings.Contains(err.Error(), "not found") { + if errors.As(err, &netlink.LinkNotFoundError{}) { return name, nil } return "", err diff --git a/libnetwork/netutils/utils_linux_test.go b/libnetwork/netutils/utils_linux_test.go index ae9c42bc8a..34f1ae6779 100644 --- a/libnetwork/netutils/utils_linux_test.go +++ b/libnetwork/netutils/utils_linux_test.go @@ -2,14 +2,18 @@ package netutils import ( "bytes" + "fmt" "net" "sort" + "strings" "testing" "github.com/docker/docker/libnetwork/ipamutils" "github.com/docker/docker/libnetwork/testutils" "github.com/docker/docker/libnetwork/types" "github.com/vishvananda/netlink" + "gotest.tools/v3/assert" + is "gotest.tools/v3/assert/cmp" ) func TestNonOverlappingNameservers(t *testing.T) { @@ -186,21 +190,46 @@ func TestNetworkRange(t *testing.T) { // Test veth name generation "veth"+rand (e.g.veth0f60e2c) func TestGenerateRandomName(t *testing.T) { - name1, err := GenerateRandomName("veth", 7) - if err != nil { - t.Fatal(err) + const vethPrefix = "veth" + const vethLen = len(vethPrefix) + 7 + + testCases := []struct { + prefix string + length int + error bool + }{ + {vethPrefix, -1, true}, + {vethPrefix, 0, true}, + {vethPrefix, len(vethPrefix) - 1, true}, + {vethPrefix, len(vethPrefix), true}, + {vethPrefix, len(vethPrefix) + 1, false}, + {vethPrefix, 255, false}, } - // veth plus generated append equals a len of 11 - if len(name1) != 11 { - t.Fatalf("Expected 11 characters, instead received %d characters", len(name1)) + for _, tc := range testCases { + t.Run(fmt.Sprintf("prefix=%s/length=%d", tc.prefix, tc.length), func(t *testing.T) { + name, err := GenerateRandomName(tc.prefix, tc.length) + if tc.error { + assert.Check(t, is.ErrorContains(err, "invalid length")) + } else { + assert.NilError(t, err) + assert.Check(t, strings.HasPrefix(name, tc.prefix), "Expected name to start with %s", tc.prefix) + assert.Check(t, is.Equal(len(name), tc.length), "Expected %d characters, instead received %d characters", tc.length, len(name)) + } + }) } - name2, err := GenerateRandomName("veth", 7) - if err != nil { - t.Fatal(err) - } - // Fail if the random generated names equal one another - if name1 == name2 { - t.Fatalf("Expected differing values but received %s and %s", name1, name2) + + var randomNames [16]string + for i := range randomNames { + randomName, err := GenerateRandomName(vethPrefix, vethLen) + assert.NilError(t, err) + + for _, oldName := range randomNames { + if randomName == oldName { + t.Fatalf("Duplicate random name generated: %s", randomName) + } + } + + randomNames[i] = randomName } }