Skip to content

fix(openvpn): survive server-initiated soft reset and rekey - #3109

Open
ayanami-desu wants to merge 2 commits into
MetaCubeX:Alphafrom
ayanami-desu:fix/openvpn-rekey
Open

fix(openvpn): survive server-initiated soft reset and rekey#3109
ayanami-desu wants to merge 2 commits into
MetaCubeX:Alphafrom
ayanami-desu:fix/openvpn-rekey

Conversation

@ayanami-desu

Copy link
Copy Markdown

Fixes #3085.

Supersedes the closed review iteration in #3107. This version is rebased onto the current Alpha branch and incorporates the complete follow-up audit.

Summary

  • Treat P_CONTROL_SOFT_RESET_V1 as a fresh TLS/key epoch and follow OpenVPN's 0 → 1 → … → 7 → 1 key-ID sequence.
  • Preserve reliable control ordering, ACK state, replay protection, retransmission, session identity, and retiring data epochs across rekeys.
  • Make UDP control writes resilient to packet loss and temporary socket failures while surfacing permanent retransmission failures.
  • Anchor transition and outbound-promotion deadlines to soft-reset acceptance instead of delayed TLS/KM2 completion.
  • Preserve AUTH_PENDING, token refresh, push continuation, and shortened KM2 records across arbitrary TLS read boundaries.
  • Distinguish omitted tran-window from explicit zero; reject negative and overflowing values.
  • Fail closed before control/data packet-ID rollover and prevent AEAD nonce reuse.
  • Make control/TCP I/O cancellation-safe: deadlines and Close interrupt in-flight operations without dropping partial TCP frames or emitting queued payloads afterward.
  • Split TLS ciphertext into OpenVPN-compatible control datagrams and serialize complete TCP frames across partial writes.

Compatibility and safety details

  • tls-auth/tls-crypt packet IDs and replay state remain session-wide across soft resets.
  • Protected control timestamps remain stable until packet-ID rollover, matching OpenVPN 2.6 long-form packet IDs.
  • The previous data key remains available only for its configured transition window; expired writes pause until the replacement epoch is installed.
  • AUTH_FAILED, RESTART, HALT, and EXIT take precedence over coalesced push data without leaking adjacent auth tokens.
  • Malformed auth-token-user, invalid push-continuation, oversized control buffers, stale sessions, and invalid ACK arrays are rejected.

Tests

  • go test ./transport/openvpn ./adapter/outbound -count=1
  • go test -race ./transport/openvpn -count=1
  • go vet ./...
  • SKIP_INTEROP_TEST=1 SKIP_CONCURRENT_TEST=1 go test ./... -count=1
  • SKIP_INTEROP_TEST=1 SKIP_CONCURRENT_TEST=1 go test ./... -tags with_gvisor -count=1
  • Targeted deadline, Close, replay, parser, retransmission, and epoch-race regressions were stress-run repeatedly.

The branch is a single commit directly on the current upstream Alpha head.

@wwqgtxx

wwqgtxx commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

The PR does not fully implement the fault-tolerance semantics of the original tran-window. In the original implementation, if a new handshake fails, the old "lame-duck key" is retained and forwarding continues within the window; however, in the current code, if rekeying fails, failControl() is called, shutting down the entire client. That said, this "immediate shutdown upon rekey failure" behavior already existed in the PR base and is not a regression introduced by this specific PR. Perhaps we can revise it in a future PR.

@wwqgtxx

wwqgtxx commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

However, you need to fix the Go 1.20 compatibility issue; do not use context.AfterFunc directly—use our contextutils.AfterFunc instead.

@wwqgtxx

wwqgtxx commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Also need fix

=== RUN   TestControlWriteDeadlineExtensionIgnoresOldTimer
    control_test.go:1263: superseded deadline canceled write: context deadline exceeded
--- FAIL: TestControlWriteDeadlineExtensionIgnoresOldTimer (0.06s)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants