tcp: mark ISN and timestamp-offset secrets nosave for checkpoint#13213
Open
copybara-service[bot] wants to merge 1 commit into
Open
tcp: mark ISN and timestamp-offset secrets nosave for checkpoint#13213copybara-service[bot] wants to merge 1 commit into
copybara-service[bot] wants to merge 1 commit into
Conversation
ceed354 to
8de2fe6
Compare
## 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=#13164 from ibondarenko1:hardening/tcp-isn-secrets-nosave b027f39
PiperOrigin-RevId: 918043127
8de2fe6 to
54e51b5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tcp: mark ISN and timestamp-offset secrets nosave for checkpoint
Summary
pkg/tcpip/transport/tcp/protocol.gois annotated+stateify savable. Two of its fields are 16-byte CSPRNG secrets used for ISN and timestamp-offset generation (RFC 6528):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 protocolRWMutexwithstate:"nosave"protocol.go:115:probe TCPProbeFuncwithstate:"nosave"The source of randomness itself,
secureRNG, is alsostate:"nosave"atpkg/tcpip/stack/stack.go:159. The absence of the tag onseqnumSecret/tsOffsetSecretis an omission against the established codebase convention, not a deliberate choice.Linux / BSD reference
Linux stores its ISN secret (
net_secretinnet/core/secure_seq.c) in kernel memory per netns; FreeBSD and OpenBSD store theirs intcp_subr.ckernel 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 inv20231204.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-120reads:New file
pkg/tcpip/transport/tcp/protocol_state.goadds anafterLoadhook that redraws both secrets fromp.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;
afterLoadreseeds fromstack.SecureRNG().Existing live behavior at
newProtocol()(initial seeding at construction) is unchanged.Tests
New file
pkg/tcpip/transport/tcp/protocol_state_test.goadds two regression tests:TestProtocolSecretsHaveNosaveTagis a reflection-based structural check that both secret fields carrystate:"nosave". Removing the tag in a future change would fail this test.TestProtocolAfterLoadRegeneratesSecretszeroes the two secrets (simulating the post-restore state fornosavefields), invokesafterLoad, and asserts both fields become non-zero and differ from the initial values.bazel build //pkg/tcpip/transport/tcp:tcpbuilds clean againstmaster.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
g3doc/user_guide/checkpoint_restore.mdstate:"nosave":pkg/tcpip/stack/stack.go:159,pkg/tcpip/transport/tcp/protocol.go:92,115.FUTURE_COPYBARA_INTEGRATE_REVIEW=#13164 from ibondarenko1:hardening/tcp-isn-secrets-nosave b027f39