diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 33a2aa79..d4fb011c 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -22,16 +22,22 @@ import ( const ( maxChainLength = 28 chainPrefix = "CNI-" - prefixLength = len(chainPrefix) ) -// Generates a chain name to be used with iptables. -// Ensures that the generated chain name is exactly -// maxChainLength chars in length +// FormatChainName generates a chain name to be used +// with iptables. Ensures that the generated chain +// name is exactly maxChainLength chars in length. func FormatChainName(name string, id string) string { - chainBytes := sha512.Sum512([]byte(name + id)) - chain := fmt.Sprintf("%s%x", chainPrefix, chainBytes) - return chain[:maxChainLength] + return MustFormatChainNameWithPrefix(name, id, "") +} + +// MustFormatChainNameWithPrefix generates a chain name similar +// to FormatChainName, but adds a custom prefix between +// chainPrefix and unique identifier. Ensures that the +// generated chain name is exactly maxChainLength chars in length. +// Panics if the given prefix is too long. +func MustFormatChainNameWithPrefix(name string, id string, prefix string) string { + return MustFormatHashWithPrefix(maxChainLength, chainPrefix+prefix, name+id) } // FormatComment returns a comment used for easier @@ -39,3 +45,16 @@ func FormatChainName(name string, id string) string { func FormatComment(name string, id string) string { return fmt.Sprintf("name: %q id: %q", name, id) } + +const MaxHashLen = sha512.Size * 2 + +// MustFormatHashWithPrefix returns a string of given length that begins with the +// given prefix. It is filled with entropy based on the given string toHash. +func MustFormatHashWithPrefix(length int, prefix string, toHash string) string { + if len(prefix) >= length || length > MaxHashLen { + panic("invalid length") + } + + output := sha512.Sum512([]byte(toHash)) + return fmt.Sprintf("%s%x", prefix, output)[:length] +} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index d703de42..0182c6d4 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -15,37 +15,151 @@ package utils import ( + "fmt" + "strings" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) var _ = Describe("Utils", func() { - It("must format a short name", func() { - chain := FormatChainName("test", "1234") - Expect(len(chain)).To(Equal(maxChainLength)) - Expect(chain).To(Equal("CNI-2bbe0c48b91a7d1b8a6753a8")) + Describe("FormatChainName", func() { + It("must format a short name", func() { + chain := FormatChainName("test", "1234") + Expect(len(chain)).To(Equal(maxChainLength)) + Expect(chain).To(Equal("CNI-2bbe0c48b91a7d1b8a6753a8")) + }) + + It("must truncate a long name", func() { + chain := FormatChainName("testalongnamethatdoesnotmakesense", "1234") + Expect(len(chain)).To(Equal(maxChainLength)) + Expect(chain).To(Equal("CNI-374f33fe84ab0ed84dcdebe3")) + }) + + It("must be predictable", func() { + chain1 := FormatChainName("testalongnamethatdoesnotmakesense", "1234") + chain2 := FormatChainName("testalongnamethatdoesnotmakesense", "1234") + Expect(len(chain1)).To(Equal(maxChainLength)) + Expect(len(chain2)).To(Equal(maxChainLength)) + Expect(chain1).To(Equal(chain2)) + }) + + It("must change when a character changes", func() { + chain1 := FormatChainName("testalongnamethatdoesnotmakesense", "1234") + chain2 := FormatChainName("testalongnamethatdoesnotmakesense", "1235") + Expect(len(chain1)).To(Equal(maxChainLength)) + Expect(len(chain2)).To(Equal(maxChainLength)) + Expect(chain1).To(Equal("CNI-374f33fe84ab0ed84dcdebe3")) + Expect(chain1).NotTo(Equal(chain2)) + }) }) - It("must truncate a long name", func() { - chain := FormatChainName("testalongnamethatdoesnotmakesense", "1234") - Expect(len(chain)).To(Equal(maxChainLength)) - Expect(chain).To(Equal("CNI-374f33fe84ab0ed84dcdebe3")) + Describe("MustFormatChainNameWithPrefix", func() { + It("generates a chain name with a prefix", func() { + chain := MustFormatChainNameWithPrefix("test", "1234", "PREFIX-") + Expect(len(chain)).To(Equal(maxChainLength)) + Expect(chain).To(Equal("CNI-PREFIX-2bbe0c48b91a7d1b8")) + }) + + It("must format a short name", func() { + chain := MustFormatChainNameWithPrefix("test", "1234", "PREFIX-") + Expect(len(chain)).To(Equal(maxChainLength)) + Expect(chain).To(Equal("CNI-PREFIX-2bbe0c48b91a7d1b8")) + }) + + It("must truncate a long name", func() { + chain := MustFormatChainNameWithPrefix("testalongnamethatdoesnotmakesense", "1234", "PREFIX-") + Expect(len(chain)).To(Equal(maxChainLength)) + Expect(chain).To(Equal("CNI-PREFIX-374f33fe84ab0ed84")) + }) + + It("must be predictable", func() { + chain1 := MustFormatChainNameWithPrefix("testalongnamethatdoesnotmakesense", "1234", "PREFIX-") + chain2 := MustFormatChainNameWithPrefix("testalongnamethatdoesnotmakesense", "1234", "PREFIX-") + Expect(len(chain1)).To(Equal(maxChainLength)) + Expect(len(chain2)).To(Equal(maxChainLength)) + Expect(chain1).To(Equal(chain2)) + }) + + It("must change when a character changes", func() { + chain1 := MustFormatChainNameWithPrefix("testalongnamethatdoesnotmakesense", "1234", "PREFIX-") + chain2 := MustFormatChainNameWithPrefix("testalongnamethatdoesnotmakesense", "1235", "PREFIX-") + Expect(len(chain1)).To(Equal(maxChainLength)) + Expect(len(chain2)).To(Equal(maxChainLength)) + Expect(chain1).To(Equal("CNI-PREFIX-374f33fe84ab0ed84")) + Expect(chain1).NotTo(Equal(chain2)) + }) + + It("panics when prefix is too large", func() { + longPrefix := strings.Repeat("PREFIX-", 4) + Expect(func() { + MustFormatChainNameWithPrefix("test", "1234", longPrefix) + }).To(Panic()) + }) }) - It("must be predictable", func() { - chain1 := FormatChainName("testalongnamethatdoesnotmakesense", "1234") - chain2 := FormatChainName("testalongnamethatdoesnotmakesense", "1234") - Expect(len(chain1)).To(Equal(maxChainLength)) - Expect(len(chain2)).To(Equal(maxChainLength)) - Expect(chain1).To(Equal(chain2)) + Describe("MustFormatHashWithPrefix", func() { + It("always returns a string with the given prefix", func() { + Expect(MustFormatHashWithPrefix(10, "AAA", "some string")).To(HavePrefix("AAA")) + Expect(MustFormatHashWithPrefix(10, "foo", "some string")).To(HavePrefix("foo")) + Expect(MustFormatHashWithPrefix(10, "bar", "some string")).To(HavePrefix("bar")) + }) + + It("always returns a string of the given length", func() { + Expect(MustFormatHashWithPrefix(10, "AAA", "some string")).To(HaveLen(10)) + Expect(MustFormatHashWithPrefix(15, "AAA", "some string")).To(HaveLen(15)) + Expect(MustFormatHashWithPrefix(5, "AAA", "some string")).To(HaveLen(5)) + }) + + It("is deterministic", func() { + val1 := MustFormatHashWithPrefix(10, "AAA", "some string") + val2 := MustFormatHashWithPrefix(10, "AAA", "some string") + val3 := MustFormatHashWithPrefix(10, "AAA", "some string") + Expect(val1).To(Equal(val2)) + Expect(val1).To(Equal(val3)) + }) + + It("is (nearly) perfect (injective function)", func() { + hashes := map[string]int{} + + for i := 0; i < 1000; i++ { + name := fmt.Sprintf("string %d", i) + hashes[MustFormatHashWithPrefix(8, "", name)]++ + } + + for key, count := range hashes { + Expect(count).To(Equal(1), "for key "+key+" got non-unique correspondence") + } + }) + + assertPanicWith := func(f func(), expectedErrorMessage string) { + defer func() { + Expect(recover()).To(Equal(expectedErrorMessage)) + }() + f() + Fail("function should have panicked but did not") + } + + It("panics when prefix is longer than the length", func() { + assertPanicWith( + func() { MustFormatHashWithPrefix(3, "AAA", "some string") }, + "invalid length", + ) + }) + + It("panics when length is not positive", func() { + assertPanicWith( + func() { MustFormatHashWithPrefix(0, "", "some string") }, + "invalid length", + ) + }) + + It("panics when length is larger than MaxLen", func() { + assertPanicWith( + func() { MustFormatHashWithPrefix(MaxHashLen+1, "", "some string") }, + "invalid length", + ) + }) }) - It("must change when a character changes", func() { - chain1 := FormatChainName("testalongnamethatdoesnotmakesense", "1234") - chain2 := FormatChainName("testalongnamethatdoesnotmakesense", "1235") - Expect(len(chain1)).To(Equal(maxChainLength)) - Expect(len(chain2)).To(Equal(maxChainLength)) - Expect(chain1).To(Equal("CNI-374f33fe84ab0ed84dcdebe3")) - Expect(chain1).NotTo(Equal(chain2)) - }) }) diff --git a/plugins/meta/bandwidth/bandwidth_linux_test.go b/plugins/meta/bandwidth/bandwidth_linux_test.go index 2cc0796a..2a973c57 100644 --- a/plugins/meta/bandwidth/bandwidth_linux_test.go +++ b/plugins/meta/bandwidth/bandwidth_linux_test.go @@ -105,7 +105,7 @@ var _ = Describe("bandwidth test", func() { hostIP = net.IP{169, 254, 0, 1} containerIP = net.IP{10, 254, 0, 1} hostIfaceMTU = 1024 - ifbDeviceName = "5b6c" + ifbDeviceName = "bwpa8eda89404b7" createVeth(hostNs.Path(), hostIfname, containerNs.Path(), containerIfname, hostIP, containerIP, hostIfaceMTU) }) diff --git a/plugins/meta/bandwidth/main.go b/plugins/meta/bandwidth/main.go index d967c932..bedb1bad 100644 --- a/plugins/meta/bandwidth/main.go +++ b/plugins/meta/bandwidth/main.go @@ -15,7 +15,6 @@ package main import ( - "crypto/sha1" "encoding/json" "fmt" @@ -28,9 +27,13 @@ import ( "github.com/containernetworking/plugins/pkg/ip" "github.com/containernetworking/plugins/pkg/ns" + "github.com/containernetworking/plugins/pkg/utils" bv "github.com/containernetworking/plugins/pkg/utils/buildversion" ) +const maxIfbDeviceLength = 15 +const ifbDevicePrefix = "bwp" + // BandwidthEntry corresponds to a single entry in the bandwidth argument, // see CONVENTIONS.md type BandwidthEntry struct { @@ -111,14 +114,8 @@ func validateRateAndBurst(rate int, burst int) error { return nil } -func getIfbDeviceName(networkName string, containerId string) (string, error) { - hash := sha1.New() - _, err := hash.Write([]byte(networkName + containerId)) - if err != nil { - return "", err - } - - return fmt.Sprintf("%x", hash.Sum(nil))[:4], nil +func getIfbDeviceName(networkName string, containerId string) string { + return utils.MustFormatHashWithPrefix(maxIfbDeviceLength, ifbDevicePrefix, networkName+containerId) } func getMTU(deviceName string) (int, error) { @@ -205,10 +202,7 @@ func cmdAdd(args *skel.CmdArgs) error { return err } - ifbDeviceName, err := getIfbDeviceName(conf.Name, args.ContainerID) - if err != nil { - return err - } + ifbDeviceName := getIfbDeviceName(conf.Name, args.ContainerID) err = CreateIfb(ifbDeviceName, mtu) if err != nil { @@ -239,10 +233,7 @@ func cmdDel(args *skel.CmdArgs) error { return err } - ifbDeviceName, err := getIfbDeviceName(conf.Name, args.ContainerID) - if err != nil { - return err - } + ifbDeviceName := getIfbDeviceName(conf.Name, args.ContainerID) if err := TeardownIfb(ifbDeviceName); err != nil { return err @@ -343,10 +334,7 @@ func cmdCheck(args *skel.CmdArgs) error { latency := latencyInUsec(latencyInMillis) limitInBytes := limit(uint64(rateInBytes), latency, uint32(burstInBytes)) - ifbDeviceName, err := getIfbDeviceName(bwConf.Name, args.ContainerID) - if err != nil { - return err - } + ifbDeviceName := getIfbDeviceName(bwConf.Name, args.ContainerID) ifbDevice, err := netlink.LinkByName(ifbDeviceName) if err != nil { diff --git a/plugins/meta/portmap/portmap.go b/plugins/meta/portmap/portmap.go index d73dda20..06b6d1d2 100644 --- a/plugins/meta/portmap/portmap.go +++ b/plugins/meta/portmap/portmap.go @@ -20,6 +20,7 @@ import ( "sort" "strconv" + "github.com/containernetworking/plugins/pkg/utils" "github.com/containernetworking/plugins/pkg/utils/sysctl" "github.com/coreos/go-iptables/iptables" ) @@ -172,7 +173,7 @@ func genToplevelDnatChain() chain { func genDnatChain(netName, containerID string) chain { return chain{ table: "nat", - name: formatChainName("DN-", netName, containerID), + name: utils.MustFormatChainNameWithPrefix(netName, containerID, "DN-"), entryChains: []string{TopLevelDNATChainName}, } } @@ -323,11 +324,9 @@ func enableLocalnetRouting(ifName string) error { // genOldSnatChain is no longer used, but used to be created. We'll try and // tear it down in case the plugin version changed between ADD and DEL func genOldSnatChain(netName, containerID string) chain { - name := formatChainName("SN-", netName, containerID) - return chain{ table: "nat", - name: name, + name: utils.MustFormatChainNameWithPrefix(netName, containerID, "SN-"), entryChains: []string{OldTopLevelSNATChainName}, } } diff --git a/plugins/meta/portmap/utils.go b/plugins/meta/portmap/utils.go index 6d6bc298..a733fdaa 100644 --- a/plugins/meta/portmap/utils.go +++ b/plugins/meta/portmap/utils.go @@ -15,7 +15,6 @@ package main import ( - "crypto/sha512" "fmt" "net" "strconv" @@ -24,8 +23,6 @@ import ( "github.com/vishvananda/netlink" ) -const maxChainNameLength = 28 - // fmtIpPort correctly formats ip:port literals for iptables and ip6tables - // need to wrap v6 literals in a [] func fmtIpPort(ip net.IP, port int) string { @@ -62,12 +59,6 @@ func getRoutableHostIF(containerIP net.IP) string { return "" } -func formatChainName(prefix, name, id string) string { - chainBytes := sha512.Sum512([]byte(name + id)) - chain := fmt.Sprintf("CNI-%s%x", prefix, chainBytes) - return chain[:maxChainNameLength] -} - // groupByProto groups port numbers by protocol func groupByProto(entries []PortMapEntry) map[string][]int { if len(entries) == 0 {