From f956b5ac17ae1f60a4d21999b59ba18c55f86d56 Mon Sep 17 00:00:00 2001
From: Zeling Feng <zeling@google.com>
Date: Mon, 20 Nov 2023 21:43:33 -0800
Subject: [PATCH] Use cryptographic hash functions for TCP

gVisor used to use jenkins for generating initial sequence number and timestamp
offset. Since it is not a cryptographic hash function and easily reversible, it
can cause security concerns. Replaced it with sha256 which is a secure hash
function.

Discovered by Inon Kaplan (PhD candidate in the Hebrew University School of
Computer Science and Engineering), Ron Even (BSc graduate of Bar Ilan
University) and Amit Klein (faculty member in the Hebrew University School of
Computer Science and Engineering). Details will be provided in their paper, to
be presented in a forthcoming academic conference.

Fixes #6473

PiperOrigin-RevId: 584211221
---
 pkg/tcpip/transport/tcp/connect.go  | 11 ++++++++---
 pkg/tcpip/transport/tcp/protocol.go | 14 ++++++++------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/pkg/tcpip/transport/tcp/connect.go b/pkg/tcpip/transport/tcp/connect.go
index 5daf30d10..67c69f6ef 100644
--- a/pkg/tcpip/transport/tcp/connect.go
+++ b/pkg/tcpip/transport/tcp/connect.go
@@ -15,6 +15,7 @@
 package tcp
 
 import (
+	"crypto/sha256"
 	"encoding/binary"
 	"fmt"
 	"math"
@@ -23,7 +24,6 @@ import (
 	"gvisor.dev/gvisor/pkg/sync"
 	"gvisor.dev/gvisor/pkg/tcpip"
 	"gvisor.dev/gvisor/pkg/tcpip/checksum"
-	"gvisor.dev/gvisor/pkg/tcpip/hash/jenkins"
 	"gvisor.dev/gvisor/pkg/tcpip/header"
 	"gvisor.dev/gvisor/pkg/tcpip/seqnum"
 	"gvisor.dev/gvisor/pkg/tcpip/stack"
@@ -236,10 +236,14 @@ func (h *handshake) resetState() {
 // generateSecureISN generates a secure Initial Sequence number based on the
 // recommendation here https://tools.ietf.org/html/rfc6528#page-3.
 func generateSecureISN(id stack.TransportEndpointID, clock tcpip.Clock, seed uint32) seqnum.Value {
-	isnHasher := jenkins.Sum32(seed)
+	isnHasher := sha256.New()
+
+	seedBuf := make([]byte, 4)
+	binary.LittleEndian.PutUint32(seedBuf, seed)
 	// Per hash.Hash.Writer:
 	//
 	// It never returns an error.
+	_, _ = isnHasher.Write(seedBuf)
 	_, _ = isnHasher.Write(id.LocalAddress.AsSlice())
 	_, _ = isnHasher.Write(id.RemoteAddress.AsSlice())
 	portBuf := make([]byte, 2)
@@ -257,7 +261,8 @@ func generateSecureISN(id stack.TransportEndpointID, clock tcpip.Clock, seed uin
 	//
 	// Which sort of guarantees that we won't reuse the ISN for a new
 	// connection for the same tuple for at least 274s.
-	isn := isnHasher.Sum32() + uint32(clock.NowMonotonic().Sub(tcpip.MonotonicTime{}).Nanoseconds()>>6)
+	hash := binary.LittleEndian.Uint32(isnHasher.Sum(nil)[:4])
+	isn := hash + uint32(clock.NowMonotonic().Sub(tcpip.MonotonicTime{}).Nanoseconds()>>6)
 	return seqnum.Value(isn)
 }
 
diff --git a/pkg/tcpip/transport/tcp/protocol.go b/pkg/tcpip/transport/tcp/protocol.go
index 555951488..090fc6e9d 100644
--- a/pkg/tcpip/transport/tcp/protocol.go
+++ b/pkg/tcpip/transport/tcp/protocol.go
@@ -16,13 +16,14 @@
 package tcp
 
 import (
+	"crypto/sha256"
+	"encoding/binary"
 	"runtime"
 	"strings"
 	"time"
 
 	"gvisor.dev/gvisor/pkg/sync"
 	"gvisor.dev/gvisor/pkg/tcpip"
-	"gvisor.dev/gvisor/pkg/tcpip/hash/jenkins"
 	"gvisor.dev/gvisor/pkg/tcpip/header"
 	"gvisor.dev/gvisor/pkg/tcpip/header/parse"
 	"gvisor.dev/gvisor/pkg/tcpip/internal/tcp"
@@ -178,16 +179,17 @@ func (p *protocol) tsOffset(src, dst tcpip.Address) tcp.TSOffset {
 	//
 	// See https://tools.ietf.org/html/rfc7323#section-5.4 for details on
 	// why this is required.
-	//
-	// TODO(https://gvisor.dev/issues/6473): This is not really secure as
-	// it does not use the recommended algorithm linked above.
-	h := jenkins.Sum32(p.tsOffsetSecret)
+	h := sha256.New()
+
+	secretBuf := make([]byte, 4)
+	binary.LittleEndian.PutUint32(secretBuf, p.tsOffsetSecret)
 	// Per hash.Hash.Writer:
 	//
 	// It never returns an error.
+	_, _ = h.Write(secretBuf)
 	_, _ = h.Write(src.AsSlice())
 	_, _ = h.Write(dst.AsSlice())
-	return tcp.NewTSOffset(h.Sum32())
+	return tcp.NewTSOffset(binary.LittleEndian.Uint32(h.Sum(nil)[:4]))
 }
 
 // replyWithReset replies to the given segment with a reset segment.