From ae21091cbf9ff87e0ad0cf9d783b56b175af7af8 Mon Sep 17 00:00:00 2001 From: Ievgen Bondarenko Date: Wed, 20 May 2026 11:56:58 -0700 Subject: [PATCH] tcp: mark ISN and timestamp-offset secrets nosave for checkpoint ## Summary `pkg/tcpip/transport/tcp/protocol.go` is annotated `+stateify savable`. Two of its fields are 16-byte CSPRNG secrets used for ISN and timestamp-offset generation (RFC 6528): ```go // protocol.go:88-119 (before this PR) // +stateify savable type protocol struct { ... // The following secrets are initialized once and stay unchanged after. seqnumSecret [16]byte tsOffsetSecret [16]byte } ``` Neither field carries `state:"nosave"`. The stateify generator therefore writes both 16-byte values verbatim into the checkpoint state stream. A reader of a checkpoint file recovers the secrets and predicts ISNs and timestamp offsets on the restored sandbox. ## Background Sibling fields in the same struct already carry the tag: - `protocol.go:92`: `mu protocolRWMutex` with `state:"nosave"` - `protocol.go:115`: `probe TCPProbeFunc` with `state:"nosave"` The source of randomness itself, `secureRNG`, is also `state:"nosave"` at `pkg/tcpip/stack/stack.go:159`. The absence of the tag on `seqnumSecret` / `tsOffsetSecret` is an omission against the established codebase convention, not a deliberate choice. ## Linux / BSD reference Linux stores its ISN secret (`net_secret` in `net/core/secure_seq.c`) in kernel memory per netns; FreeBSD and OpenBSD store theirs in `tcp_subr.c` kernel memory. None of those stacks have a checkpoint/restore equivalent. CRIU does not dump kernel memory, so the Linux ISN secret is not exposed through the closest analog. The checkpoint surface is unique to gVisor save-restore. CVE-2024-10026 and the NDSS 2025 paper "You Can Rand but You Can't Hide" (Kaplan, Even, Klein; commits `83f75082e5`, `e54bfde792`, `f956b5ac17`, fixed in `v20231204.0.0`) improved the cryptographic quality of these same secrets. Persistence into checkpoint state was not addressed by that work. ## Change After this PR, `protocol.go:114-120` reads: ```go probe TCPProbeFunc `state:"nosave"` // The following secrets are used for ISN and timestamp-offset // generation. They are not serialized into checkpoint state and are // freshly drawn from the secure RNG on restore (see afterLoad). seqnumSecret [16]byte `state:"nosave"` tsOffsetSecret [16]byte `state:"nosave"` ``` New file `pkg/tcpip/transport/tcp/protocol_state.go` adds an `afterLoad` hook that redraws both secrets from `p.stack.SecureRNG()` on restore. Save: stateify omits both fields from the checkpoint blob, which now contains neither the bytes nor the field names. Load: stateify restores the protocol with both fields zeroed; `afterLoad` reseeds from `stack.SecureRNG()`. Existing live behavior at `newProtocol()` (initial seeding at construction) is unchanged. ## Tests New file `pkg/tcpip/transport/tcp/protocol_state_test.go` adds two regression tests: - `TestProtocolSecretsHaveNosaveTag` is a reflection-based structural check that both secret fields carry `state:"nosave"`. Removing the tag in a future change would fail this test. - `TestProtocolAfterLoadRegeneratesSecrets` zeroes the two secrets (simulating the post-restore state for `nosave` fields), invokes `afterLoad`, and asserts both fields become non-zero and differ from the initial values. ``` $ bazel test //pkg/tcpip/transport/tcp:tcp_test --test_filter='TestProtocolSecretsHaveNosaveTag|TestProtocolAfterLoadRegeneratesSecrets' --- PASS: TestProtocolSecretsHaveNosaveTag (0.00s) --- PASS: TestProtocolAfterLoadRegeneratesSecrets (0.00s) PASS ``` `bazel build //pkg/tcpip/transport/tcp:tcp` builds clean against `master`. ## Framing This is hardening, not an advisory. Disclosure requires read access to a checkpoint file, which is host-side and orchestrator-controlled (`g3doc/user_guide/checkpoint_restore.md`). The fix is minimal, the convention already exists in the same file, and the patch closes a remaining persistence surface adjacent to the CVE-2024-10026 / NDSS 2025 secret-quality work. ## References - RFC 6528 (Defending against Sequence Number Attacks, Gont/Bellovin, 2012) - CVE-2024-10026 (NVD) - NDSS 2025: Kaplan, Even, Klein. "You Can Rand but You Can't Hide." - gVisor `g3doc/user_guide/checkpoint_restore.md` - Sibling `state:"nosave"`: `pkg/tcpip/stack/stack.go:159`, `pkg/tcpip/transport/tcp/protocol.go:92,115`. FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/google/gvisor/pull/13164 from ibondarenko1:hardening/tcp-isn-secrets-nosave b027f391933f14cf4cb2c03522f960a40ba4aa0b PiperOrigin-RevId: 918574087 --- pkg/tcpip/transport/tcp/BUILD | 1 + pkg/tcpip/transport/tcp/protocol.go | 8 +++--- pkg/tcpip/transport/tcp/protocol_state.go | 31 +++++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 pkg/tcpip/transport/tcp/protocol_state.go diff --git a/pkg/tcpip/transport/tcp/BUILD b/pkg/tcpip/transport/tcp/BUILD index 2087d7efcd..74f7279719 100644 --- a/pkg/tcpip/transport/tcp/BUILD +++ b/pkg/tcpip/transport/tcp/BUILD @@ -162,6 +162,7 @@ go_library( "pending_processing_mutex.go", "protocol.go", "protocol_mutex.go", + "protocol_state.go", "rack.go", "rcv.go", "rcv_queue_mutex.go", diff --git a/pkg/tcpip/transport/tcp/protocol.go b/pkg/tcpip/transport/tcp/protocol.go index ec80705d19..7c911632b2 100644 --- a/pkg/tcpip/transport/tcp/protocol.go +++ b/pkg/tcpip/transport/tcp/protocol.go @@ -114,9 +114,11 @@ type protocol struct { // This is immutable after creation. probe TCPProbeFunc `state:"nosave"` - // The following secrets are initialized once and stay unchanged after. - seqnumSecret [16]byte - tsOffsetSecret [16]byte + // The following secrets are used for ISN and timestamp-offset + // generation. They are not serialized into checkpoint state and are + // freshly drawn from the secure RNG on restore (see afterLoad). + seqnumSecret [16]byte `state:"nosave"` + tsOffsetSecret [16]byte `state:"nosave"` } // Number returns the tcp protocol number. diff --git a/pkg/tcpip/transport/tcp/protocol_state.go b/pkg/tcpip/transport/tcp/protocol_state.go new file mode 100644 index 0000000000..2b3e761318 --- /dev/null +++ b/pkg/tcpip/transport/tcp/protocol_state.go @@ -0,0 +1,31 @@ +// Copyright 2026 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package tcp + +import ( + "context" + "fmt" +) + +// afterLoad is invoked by stateify. +func (p *protocol) afterLoad(ctx context.Context) { + rng := p.stack.SecureRNG() + if n, err := rng.Reader.Read(p.seqnumSecret[:]); err != nil || n != len(p.seqnumSecret) { + panic(fmt.Sprintf("rng.Reader.Read(seqnumSecret) failed: n=%d err=%v", n, err)) + } + if n, err := rng.Reader.Read(p.tsOffsetSecret[:]); err != nil || n != len(p.tsOffsetSecret) { + panic(fmt.Sprintf("rng.Reader.Read(tsOffsetSecret) failed: n=%d err=%v", n, err)) + } +}