The Security Parameter Index value signals to the recipient which key to
decrypt the packet with. The overlay driver derives the SPI value for a
flow from a hash digest of the source and destination IP addresses. The
source and destination need to derive the same digest given the same
information as the SPI values are not signaled over the overlay driver's
control plane. Refactoring the overlay driver to use netip types
accidentally changed the hash function to digest IPv4 addresses in
4-byte form, causing newer engines to calculate a different SPI value
for a flow than older engines would. Restore the original calculation
by hashing IPv4 addresses in their 16-byte form, and refactor the
buildSPI function to take netip.Addr parameters to prevent 16-byte vs
4-byte mixups from being possible in the future.
Signed-off-by: Cory Snider <csnider@mirantis.com>
| ... | ... |
@@ -135,7 +135,7 @@ func (d *driver) setupEncryption(remoteIP netip.Addr) error {
|
| 135 | 135 |
indices := make([]spi, 0, len(d.keys)) |
| 136 | 136 |
|
| 137 | 137 |
for i, k := range d.keys {
|
| 138 |
- spis := spi{buildSPI(advIP.AsSlice(), remoteIP.AsSlice(), k.tag), buildSPI(remoteIP.AsSlice(), advIP.AsSlice(), k.tag)}
|
|
| 138 |
+ spis := spi{buildSPI(advIP, remoteIP, k.tag), buildSPI(remoteIP, advIP, k.tag)}
|
|
| 139 | 139 |
dir := reverse |
| 140 | 140 |
if i == 0 {
|
| 141 | 141 |
dir = bidir |
| ... | ... |
@@ -430,14 +430,14 @@ func spExists(sp *netlink.XfrmPolicy) (bool, error) {
|
| 430 | 430 |
} |
| 431 | 431 |
} |
| 432 | 432 |
|
| 433 |
-func buildSPI(src, dst net.IP, st uint32) int {
|
|
| 434 |
- b := make([]byte, 4) |
|
| 435 |
- binary.BigEndian.PutUint32(b, st) |
|
| 433 |
+func buildSPI(src, dst netip.Addr, st uint32) int {
|
|
| 436 | 434 |
h := fnv.New32a() |
| 437 |
- h.Write(src) |
|
| 438 |
- h.Write(b) |
|
| 439 |
- h.Write(dst) |
|
| 440 |
- return int(binary.BigEndian.Uint32(h.Sum(nil))) |
|
| 435 |
+ v := src.As16() |
|
| 436 |
+ h.Write(v[:]) |
|
| 437 |
+ binary.Write(h, binary.BigEndian, st) |
|
| 438 |
+ v = dst.As16() |
|
| 439 |
+ h.Write(v[:]) |
|
| 440 |
+ return int(h.Sum32()) |
|
| 441 | 441 |
} |
| 442 | 442 |
|
| 443 | 443 |
func buildAeadAlgo(k *key, s int) *netlink.XfrmStateAlgo {
|
| ... | ... |
@@ -542,7 +542,7 @@ func (d *driver) updateKeys(ctx context.Context, encrData discoverapi.DriverEncr |
| 542 | 542 |
} |
| 543 | 543 |
|
| 544 | 544 |
for rIP, node := range d.secMap {
|
| 545 |
- idxs := updateNodeKey(lIP.AsSlice(), aIP.AsSlice(), rIP.AsSlice(), node.spi, d.keys, newIdx, priIdx, delIdx) |
|
| 545 |
+ idxs := updateNodeKey(lIP, aIP, rIP, node.spi, d.keys, newIdx, priIdx, delIdx) |
|
| 546 | 546 |
if idxs != nil {
|
| 547 | 547 |
d.secMap[rIP] = encrNode{idxs, node.count}
|
| 548 | 548 |
} |
| ... | ... |
@@ -574,7 +574,7 @@ func (d *driver) updateKeys(ctx context.Context, encrData discoverapi.DriverEncr |
| 574 | 574 |
*********************************************************/ |
| 575 | 575 |
|
| 576 | 576 |
// Spis and keys are sorted in such away the one in position 0 is the primary |
| 577 |
-func updateNodeKey(lIP, aIP, rIP net.IP, idxs []spi, curKeys []*key, newIdx, priIdx, delIdx int) []spi {
|
|
| 577 |
+func updateNodeKey(lIP, aIP, rIP netip.Addr, idxs []spi, curKeys []*key, newIdx, priIdx, delIdx int) []spi {
|
|
| 578 | 578 |
log.G(context.TODO()).Debugf("Updating keys for node: %s (%d,%d,%d)", rIP, newIdx, priIdx, delIdx)
|
| 579 | 579 |
|
| 580 | 580 |
spis := idxs |
| ... | ... |
@@ -590,17 +590,17 @@ func updateNodeKey(lIP, aIP, rIP net.IP, idxs []spi, curKeys []*key, newIdx, pri |
| 590 | 590 |
|
| 591 | 591 |
if delIdx != -1 {
|
| 592 | 592 |
// -rSA0 |
| 593 |
- programSA(lIP, rIP, spis[delIdx], nil, reverse, false) |
|
| 593 |
+ programSA(lIP.AsSlice(), rIP.AsSlice(), spis[delIdx], nil, reverse, false) |
|
| 594 | 594 |
} |
| 595 | 595 |
|
| 596 | 596 |
if newIdx > -1 {
|
| 597 | 597 |
// +rSA2 |
| 598 |
- programSA(lIP, rIP, spis[newIdx], curKeys[newIdx], reverse, true) |
|
| 598 |
+ programSA(lIP.AsSlice(), rIP.AsSlice(), spis[newIdx], curKeys[newIdx], reverse, true) |
|
| 599 | 599 |
} |
| 600 | 600 |
|
| 601 | 601 |
if priIdx > 0 {
|
| 602 | 602 |
// +fSA2 |
| 603 |
- fSA2, _, _ := programSA(lIP, rIP, spis[priIdx], curKeys[priIdx], forward, true) |
|
| 603 |
+ fSA2, _, _ := programSA(lIP.AsSlice(), rIP.AsSlice(), spis[priIdx], curKeys[priIdx], forward, true) |
|
| 604 | 604 |
|
| 605 | 605 |
// +fSP2, -fSP1 |
| 606 | 606 |
s := getMinimalIP(fSA2.Src) |
| ... | ... |
@@ -631,7 +631,7 @@ func updateNodeKey(lIP, aIP, rIP net.IP, idxs []spi, curKeys []*key, newIdx, pri |
| 631 | 631 |
} |
| 632 | 632 |
|
| 633 | 633 |
// -fSA1 |
| 634 |
- programSA(lIP, rIP, spis[0], nil, forward, false) |
|
| 634 |
+ programSA(lIP.AsSlice(), rIP.AsSlice(), spis[0], nil, forward, false) |
|
| 635 | 635 |
} |
| 636 | 636 |
|
| 637 | 637 |
// swap |
| 638 | 638 |
new file mode 100644 |
| ... | ... |
@@ -0,0 +1,51 @@ |
| 0 |
+//go:build linux |
|
| 1 |
+ |
|
| 2 |
+package overlay |
|
| 3 |
+ |
|
| 4 |
+import ( |
|
| 5 |
+ "encoding/binary" |
|
| 6 |
+ "hash/fnv" |
|
| 7 |
+ "net" |
|
| 8 |
+ "net/netip" |
|
| 9 |
+ "testing" |
|
| 10 |
+) |
|
| 11 |
+ |
|
| 12 |
+func legacyBuildSPI(src, dst net.IP, st uint32) int {
|
|
| 13 |
+ b := make([]byte, 4) |
|
| 14 |
+ binary.BigEndian.PutUint32(b, st) |
|
| 15 |
+ h := fnv.New32a() |
|
| 16 |
+ h.Write(src) |
|
| 17 |
+ h.Write(b) |
|
| 18 |
+ h.Write(dst) |
|
| 19 |
+ return int(binary.BigEndian.Uint32(h.Sum(nil))) |
|
| 20 |
+} |
|
| 21 |
+ |
|
| 22 |
+func TestBuildSPI(t *testing.T) {
|
|
| 23 |
+ cases := []struct {
|
|
| 24 |
+ src, dst string |
|
| 25 |
+ st uint32 |
|
| 26 |
+ }{
|
|
| 27 |
+ {"1.2.3.4", "5.6.7.8", 1234},
|
|
| 28 |
+ {"::ffff:1.2.3.4", "::ffff:5.6.7.8", 1234},
|
|
| 29 |
+ {"10.0.0.1", "2001:db8::1", 5678},
|
|
| 30 |
+ {"2002::abcd:1", "172.15.14.13", 54321},
|
|
| 31 |
+ {"2002:db8::42", "2002:db8::69", 9999},
|
|
| 32 |
+ } |
|
| 33 |
+ for _, tc := range cases {
|
|
| 34 |
+ // The legacy buildSPI function is sensitive to whether src and |
|
| 35 |
+ // dst are in 4-byte or 16-byte form. Versions of the driver |
|
| 36 |
+ // using this function always pass the results of net.ParseIP(), |
|
| 37 |
+ // which always parses the address into 16-byte form. |
|
| 38 |
+ expected := legacyBuildSPI(net.ParseIP(tc.src), net.ParseIP(tc.dst), tc.st) |
|
| 39 |
+ |
|
| 40 |
+ src, dst := netip.MustParseAddr(tc.src), netip.MustParseAddr(tc.dst) |
|
| 41 |
+ actual := buildSPI(src, dst, tc.st) |
|
| 42 |
+ if expected != actual {
|
|
| 43 |
+ t.Errorf("buildSPI(%v, %v, %v) = %v; want %v", src, dst, tc.st, actual, expected)
|
|
| 44 |
+ } |
|
| 45 |
+ actual = buildSPI(src.Unmap(), dst.Unmap(), tc.st) |
|
| 46 |
+ if expected != actual {
|
|
| 47 |
+ t.Errorf("buildSPI(%v, %v, %v) = %v; want %v", src.Unmap(), dst.Unmap(), tc.st, actual, expected)
|
|
| 48 |
+ } |
|
| 49 |
+ } |
|
| 50 |
+} |