From ccc3c2cbd26d3514885bd665b0a110150a6e8c53 Mon Sep 17 00:00:00 2001 From: Peter Johnston Date: Tue, 27 Feb 2024 20:26:33 -0800 Subject: [PATCH] Skip incremental checksum update for unmodified fields This is important for correctness, because the current incremental checksum update routine does not work correctly when the field is set to the same value. See this equation from RFC 1071: C - one's complement sum of old header C' - one's complement sum of new header m - old value of a 16-bit field m' - new value of a 16-bit field C' = C + (-m) + m' = C + (m' - m) Assuming m is a 16-bit field, when m == m', then (m' - m) == (m + ~m) will always equal -0, or 0xFFFF in one's complement arithmetic, rather than 0x0000. The additive identity is typically 0, but since -0 has a different representation, it is *not* an identity and therefore the equation produces a different value. PiperOrigin-RevId: 610963455 --- pkg/tcpip/header/checksum.go | 3 +++ pkg/tcpip/header/checksum_test.go | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/pkg/tcpip/header/checksum.go b/pkg/tcpip/header/checksum.go index 1cdda6b90..060b4a86c 100644 --- a/pkg/tcpip/header/checksum.go +++ b/pkg/tcpip/header/checksum.go @@ -57,6 +57,9 @@ func checksumUpdate2ByteAlignedUint16(xsum, old, new uint16) uint16 { // checksum C, the new checksum C' is: // // C' = C + (-m) + m' = C + (m' - m) + if old == new { + return xsum + } return checksum.Combine(xsum, checksum.Combine(new, ^old)) } diff --git a/pkg/tcpip/header/checksum_test.go b/pkg/tcpip/header/checksum_test.go index 984508d32..0776f58b1 100644 --- a/pkg/tcpip/header/checksum_test.go +++ b/pkg/tcpip/header/checksum_test.go @@ -17,6 +17,7 @@ package header_test import ( + "bytes" "fmt" "math/rand" "sync" @@ -89,6 +90,24 @@ func TestICMPv4Checksum(t *testing.T) { }, want, fmt.Sprintf("header: {% x} data {% x}", h, b.Flatten())) } +func TestICMPv4ChecksumUpdate(t *testing.T) { + const icmpIdent = 0 + + data := make([]byte, header.ICMPv4MinimumSize) + h := header.ICMPv4(data) + h.SetType(header.ICMPv4EchoReply) + h.SetCode(header.ICMPv4UnusedCode) + h.SetIdent(icmpIdent) + h.SetChecksum(^checksum.Checksum(data, 0)) + + updated := header.ICMPv4(bytes.Clone(data)) + // Perform an incremental checksum update where we aren't actually changing the ID. + updated.SetIdentWithChecksumUpdate(icmpIdent) + if updated.Checksum() != h.Checksum() { + t.Errorf("got updated.Checksum() = %x, want = %x", updated.Checksum(), h.Checksum()) + } +} + func TestICMPv6Checksum(t *testing.T) { rnd := rand.New(rand.NewSource(42))