From 4d27f33b09932a7f6cc5ccb03ad6f7462d497afb Mon Sep 17 00:00:00 2001
From: Julian Elischer <jrelis@google.com>
Date: Fri, 16 Oct 2020 12:28:54 -0700
Subject: Make IPv4 check the IP header checksum

The IPv4 header checksum has not been checked, at least in recent times,
so add code to do so. Fix all the tests that fail because they never
needed to set the checksum.

Fixes #4484

PiperOrigin-RevId: 337556243
---
 pkg/tcpip/network/ipv4/ipv4.go      | 26 ++++++++++++++++++++++++++
 pkg/tcpip/network/ipv4/ipv4_test.go | 18 ++++++++++++++++++
 2 files changed, 44 insertions(+)

(limited to 'pkg/tcpip/network/ipv4')

diff --git a/pkg/tcpip/network/ipv4/ipv4.go b/pkg/tcpip/network/ipv4/ipv4.go
index 743aa0575..e7c58ae0a 100644
--- a/pkg/tcpip/network/ipv4/ipv4.go
+++ b/pkg/tcpip/network/ipv4/ipv4.go
@@ -440,6 +440,32 @@ func (e *endpoint) HandlePacket(r *stack.Route, pkt *stack.PacketBuffer) {
 		return
 	}
 
+	// There has been some confusion regarding verifying checksums. We need
+	// just look for negative 0 (0xffff) as the checksum, as it's not possible to
+	// get positive 0 (0) for the checksum. Some bad implementations could get it
+	// when doing entry replacement in the early days of the Internet,
+	// however the lore that one needs to check for both persists.
+	//
+	// RFC 1624 section 1 describes the source of this confusion as:
+	//     [the partial recalculation method described in RFC 1071] computes a
+	//     result for certain cases that differs from the one obtained from
+	//     scratch (one's complement of one's complement sum of the original
+	//     fields).
+	//
+	// However RFC 1624 section 5 clarifies that if using the verification method
+	// "recommended by RFC 1071, it does not matter if an intermediate system
+	// generated a -0 instead of +0".
+	//
+	// RFC1071 page 1 specifies the verification method as:
+	//	  (3)  To check a checksum, the 1's complement sum is computed over the
+	//        same set of octets, including the checksum field.  If the result
+	//        is all 1 bits (-0 in 1's complement arithmetic), the check
+	//        succeeds.
+	if h.CalculateChecksum() != 0xffff {
+		r.Stats().IP.MalformedPacketsReceived.Increment()
+		return
+	}
+
 	// As per RFC 1122 section 3.2.1.3:
 	//   When a host sends any datagram, the IP source address MUST
 	//   be one of its own IP addresses (but not a broadcast or
diff --git a/pkg/tcpip/network/ipv4/ipv4_test.go b/pkg/tcpip/network/ipv4/ipv4_test.go
index 16c36707d..fee11bb38 100644
--- a/pkg/tcpip/network/ipv4/ipv4_test.go
+++ b/pkg/tcpip/network/ipv4/ipv4_test.go
@@ -120,6 +120,7 @@ func TestIPv4Sanity(t *testing.T) {
 	tests := []struct {
 		name              string
 		headerLength      uint8 // value of 0 means "use correct size"
+		badHeaderChecksum bool
 		maxTotalLength    uint16
 		transportProtocol uint8
 		TTL               uint8
@@ -135,6 +136,14 @@ func TestIPv4Sanity(t *testing.T) {
 			transportProtocol: uint8(header.ICMPv4ProtocolNumber),
 			TTL:               ttl,
 		},
+		{
+			name:              "bad header checksum",
+			maxTotalLength:    defaultMTU,
+			transportProtocol: uint8(header.ICMPv4ProtocolNumber),
+			TTL:               ttl,
+			badHeaderChecksum: true,
+			shouldFail:        true,
+		},
 		// The TTL tests check that we are not rejecting an incoming packet
 		// with a zero or one TTL, which has been a point of confusion in the
 		// past as RFC 791 says: "If this field contains the value zero, then the
@@ -290,6 +299,12 @@ func TestIPv4Sanity(t *testing.T) {
 			if test.headerLength != 0 {
 				ip.SetHeaderLength(test.headerLength)
 			}
+			ip.SetChecksum(0)
+			ipHeaderChecksum := ip.CalculateChecksum()
+			if test.badHeaderChecksum {
+				ipHeaderChecksum += 42
+			}
+			ip.SetChecksum(^ipHeaderChecksum)
 			requestPkt := stack.NewPacketBuffer(stack.PacketBufferOptions{
 				Data: hdr.View().ToVectorisedView(),
 			})
@@ -1427,6 +1442,7 @@ func TestReceiveFragments(t *testing.T) {
 					SrcAddr:        frag.srcAddr,
 					DstAddr:        frag.dstAddr,
 				})
+				ip.SetChecksum(^ip.CalculateChecksum())
 
 				vv := hdr.View().ToVectorisedView()
 				vv.AppendView(frag.payload)
@@ -1695,6 +1711,7 @@ func TestPacketQueing(t *testing.T) {
 					SrcAddr:     host2IPv4Addr.AddressWithPrefix.Address,
 					DstAddr:     host1IPv4Addr.AddressWithPrefix.Address,
 				})
+				ip.SetChecksum(^ip.CalculateChecksum())
 				e.InjectInbound(ipv4.ProtocolNumber, stack.NewPacketBuffer(stack.PacketBufferOptions{
 					Data: hdr.View().ToVectorisedView(),
 				}))
@@ -1738,6 +1755,7 @@ func TestPacketQueing(t *testing.T) {
 					SrcAddr:     host2IPv4Addr.AddressWithPrefix.Address,
 					DstAddr:     host1IPv4Addr.AddressWithPrefix.Address,
 				})
+				ip.SetChecksum(^ip.CalculateChecksum())
 				e.InjectInbound(header.IPv4ProtocolNumber, stack.NewPacketBuffer(stack.PacketBufferOptions{
 					Data: hdr.View().ToVectorisedView(),
 				}))
-- 
cgit v1.2.3