fix(openvpn): survive server-initiated soft reset / rekey

[Lanlan13-14]

Fixes #3085.

OpenVPN 2.6 starts a new SSL session and a new key epoch on P_CONTROL_SOFT_RESET_V1. It does not renegotiate TLS on the existing SSL object. Upstream OpenVPN allocates a new SSL object per key state and has TLS-layer renegotiation disabled.

The previous mihomo client treated soft reset as TLS renegotiation on the already-completed tls.Conn. HandshakeContext returns immediately when the handshake is already done, so no ClientHello is sent. It then continued the key-method exchange on the old TLS state. Combined with a data-channel header that always used key ID 0, and a control-channel key ID that XOR-toggled 0 ↔ 1 instead of following OpenVPN’s 0 → 1 → 2 → 3 → 4 → 5 → 6 → 7 → 1, the first server-initiated rekey tore the tunnel down.

On OpenVPN 2.6.14 with reneg-sec 10 the server log is:

TLS: soft reset sec=10/10 ...
Sent fatal SSL alert: unexpected message
OpenSSL: error:0A0000F4:SSL routines::unexpected message
TLS handshake failed

The mihomo process stays up. Only the OpenVPN transport closes (use of closed network connection). Active connections drop until a full hard reset / new handshake. The original rekey error was discarded in watchControl(), so production logs never showed whether TLS, key-method, PUSH, or token refresh failed.

What each soft reset does now

1. Treat the server’s P_CONTROL_SOFT_RESET_V1 as a new key epoch. Adopt the key ID from that packet (0 → 1 → … → 7 → 1). Do not invent 0 ↔ 1.
2. Advance the reliable receive sequence past the consumed soft-reset message (MarkReceived). The server reset is new-epoch message 0; without this, ServerHello (message 1) sits in recvPending forever and the TLS handshake times out.
3. Install a fresh tls.Conn over the same control channel and run a full initial handshake. Do not call HandshakeContext on the old conn. Do not send TLS close_notify on the old epoch (that would go out under the new key ID and pollute it).
4. Run key-method 2 on the new TLS session. Derive new data-channel keys. Encode the active key ID on every P_DATA_V1 / P_DATA_V2 header.
5. Keep the retiring data channel indexed by key ID so packets still labeled with the previous epoch decrypt during the transition.
6. Parse shortened server key-method-2 records (options mandatory; username / password / peer-info optional). Preserve leftover TLS bytes so a coalesced PUSH_REPLY or AUTH_FAILED is not discarded.
7. Carry a pushed auth-token / auth-token-user into the next epoch. Capture a refreshed token when the server sends one; keep the previous token when it does not.
8. On an already-authenticated rekey, reuse the previous PUSH_REPLY (ifconfig / peer-id / cipher). OpenVPN 2.6 often does not send a second push. Waiting for one hung the second rekey for 30s.
9. If the next server soft reset arrives while TLS/key-method is still running, stash it. Do not let ControlConn.Read swallow it as an ordinary control payload.
10. Preserve the original rekey error and surface it on ReadIPPacket, instead of only use of closed network connection.

Test

Unit tests in transport/openvpn:

• TestNextKeyIDFollowsOpenVPNSequence — 0 → 1 → … → 7 → 1
• TestSoftResetAdvancesOpenVPNKeyID — second reset is key ID 2, not 0
• TestRekeyDataHeaderUsesActiveKeyID — rekeyed data packet key ID is 1, not 0
• TestParseServerKeyMethod2RecordShortenedPreservesTail — leftover PUSH_REPLY kept
• TestParsePushReplyAuthToken / TestCaptureAuthTokenUsedOnNextKeyMethod
• TestMarkReceivedUnblocksNextEpochControl — ServerHello after soft reset is delivered
• TestStashedSoftResetIsNotSwallowedByControlRead — next-epoch reset is not eaten by Read

Live check, same minimal config on both binaries:

• OpenVPN 2.6.14, reneg-sec 10, AES-128-GCM, username/password, bind 127.0.0.1:1194
• mihomo mixed-port: 17890, MATCH → OpenVPN outbound
• HTTP through the tunnel to 10.8.0.1:8080

binary	result
official alpha-7ee0b05	first soft reset: unexpected message, tunnel dies
this commit	3 consecutive soft resets, HTTP 36/36, process never restarts

Refs: MetaCubeX/mihomo#3085

[wwqgtxx]

The current revision still has a deterministic blocker that can reproduce almost exactly the production timing reported in #3085. I do not think the PR is ready to merge yet.

1. Blocker: the 30-second rekey deadline is never cleared

renegotiate() creates a context with renegotiateTimeout and passes it to startTLSEpoch():

mihomo/transport/openvpn/client.go

Lines 459 to 494 in cede0b8

	func (c *Client) renegotiate(serverReset *ControlPacket) error {
	if c.tlsConn == nil && c.controlConn == nil {
	return errRenegotiateNoTLS
	}
	renegCtx, cancel := context.WithTimeout(c.runCtx, renegotiateTimeout)
	defer cancel()
	keyID := NextKeyID(c.control.KeyID())
	if serverReset != nil {
	keyID = serverReset.KeyID & KeyIDMask
	}
	c.dataLock.Lock()
	c.lastSoftResetKey = keyID
	c.dataLock.Unlock()
	// Adopt first so QueueAck lands on the new epoch; AdoptKeyID clears acks.
	c.control.AdoptKeyID(keyID)
	if serverReset != nil {
	// The watcher already consumed the server soft reset (new-epoch
	// message 0). Advance recvMessage or ControlConn.Read will park
	// ServerHello in recvPending forever.
	c.control.MarkReceived(serverReset.MessageID)
	c.control.QueueAck(serverReset.MessageID)
	}
	if err := c.control.SendSoftReset(renegCtx); err != nil {
	return fmt.Errorf("send soft reset: %w", err)
	}
	if err := c.startTLSEpoch(renegCtx); err != nil {
	return fmt.Errorf("tls epoch handshake: %w", err)
	}
	if _, err := c.doKeyExchange(renegCtx); err != nil {
	return fmt.Errorf("rekey exchange: %w", err)
	}
	return nil

startTLSEpoch() then installs that deadline on the new TLS connection:

mihomo/transport/openvpn/client.go

Lines 153 to 176 in cede0b8

	func (c *Client) startTLSEpoch(ctx context.Context) error {
	tlsConfig, err := c.tlsConfig()
	if err != nil {
	return err
	}
	if c.controlConn == nil {
	c.controlConn = NewControlConn(c.control)
	}
	if c.tlsConn != nil {
	// Drop the old epoch without writing close_notify. Close() would send
	// it on whatever key ID is current and pollute the new control epoch.
	c.tlsConn = nil
	}
	c.controlConn.Reset()
	c.leftoverTLS = nil
	c.tlsConn = tls.Client(c.controlConn, tlsConfig)
	c.tlsEpoch++
	if deadline, ok := ctx.Deadline(); ok {
	_ = c.tlsConn.SetDeadline(deadline)
	}
	if err := c.tlsConn.HandshakeContext(ctx); err != nil {
	return fmt.Errorf("openvpn tls handshake: %w", err)
	}
	return nil

The deadline is propagated through tls.Conn -> ControlConn -> ControlChannel and stored in ControlChannel.readDeadline and writeDeadline:

mihomo/transport/openvpn/control.go

Lines 402 to 435 in cede0b8

	func (c *ControlChannel) readRawControlPacket(ctx context.Context) ([]byte, error) {
	c.mu.Lock()
	deadline := c.readDeadline
	c.mu.Unlock()
	if !deadline.IsZero() {
	var cancel context.CancelFunc
	ctx, cancel = context.WithDeadline(ctx, deadline)
	defer cancel()
	}
	return c.io.ReadPacket(ctx)
	}
	func (c *ControlChannel) SetDeadline(t time.Time) error {
	c.mu.Lock()
	c.readDeadline = t
	c.writeDeadline = t
	c.mu.Unlock()
	return nil
	}
	func (c *ControlChannel) SetReadDeadline(t time.Time) error {
	c.mu.Lock()
	c.readDeadline = t
	c.mu.Unlock()
	return nil
	}
	func (c *ControlChannel) SetWriteDeadline(t time.Time) error {
	c.mu.Lock()
	c.writeDeadline = t
	c.mu.Unlock()
	return nil

The initial handshake clears its deadline, but the rekey path does not. After a successful rekey, watchControl() waits for the next soft reset. readRawControlPacket() reapplies the stale absolute deadline even though c.runCtx itself has no deadline.

The resulting sequence is:

t=3600  Server sends the first soft reset
t~3600  The new TLS/key epoch completes successfully
t=3630  The stale rekey deadline expires
        waitForSoftReset() returns a timeout
        failControl() closes the tunnel

This is effectively the same reneg-sec + 30 seconds failure signature as #3085.

The live test with reneg-sec 10 masks this bug because every subsequent rekey moves the stored deadline forward before the previous 30-second deadline expires. The test also ends before the final deadline can expire.

At minimum, clear the deadline after a successful rekey, as is already done after the initial handshake:

if _, err := c.doKeyExchange(renegCtx); err != nil {
    return fmt.Errorf("rekey exchange: %w", err)
}
_ = c.tlsConn.SetDeadline(time.Time{})
return nil

Please also add a regression test that verifies the control-channel deadline is zero after rekey, or keeps the client idle for longer than renegotiateTimeout after a successful in-memory rekey.

2. Rekey auth-token refresh is described but not implemented

The rekey branch in doKeyExchange() returns immediately after installing the new data channel:

mihomo/transport/openvpn/client.go

Lines 220 to 241 in cede0b8

	if c.push != nil {
	// Authenticated rekeys keep the previous ifconfig / peer-id.
	// OpenVPN 2.6 often does not send another PUSH_REPLY here.
	push := mergePushReply(c.push, &PushReply{})
	c.push = push
	negotiatedCipher, err := c.config.NegotiateCipher(push.DataCiphers, push.Cipher)
	if err != nil {
	return nil, fmt.Errorf("negotiate data cipher: %w", err)
	}
	c.negotiatedCipher = negotiatedCipher
	cipherKeyLen := CipherKeyLength(negotiatedCipher)
	keys.SendCipherKey = keys.SendCipherKey[:cipherKeyLen]
	keys.RecvCipherKey = keys.RecvCipherKey[:cipherKeyLen]
	keyID := c.control.KeyID()
	newData, err := NewDataChannel(keys, negotiatedCipher, c.config.Auth, push.PeerID, keyID)
	if err != nil {
	return nil, err
	}
	c.installDataChannel(newData)
	c.markSend()
	c.markReceive()
	return push, nil

It does not call readPushReply() or captureAuthToken(). This means the initially pushed token can be used for the first rekey, but a refreshed token sent during that rekey is not consumed. A later epoch will reuse the stale token.

OpenVPN explicitly sends a renewed token as an additional minimal PUSH_REPLY,auth-token ... control message after renegotiation:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/auth_token.c#L463-L482

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/push.c#L754-L771

After doKeyExchange() returns, watchControl() reads packets directly through ControlChannel. A TLS-encrypted P_CONTROL_V1 token update is therefore acknowledged and discarded rather than passed through the active tls.Conn.

The PR description should not claim refreshed-token support until the active TLS stream remains consumable after key exchange and the token-only push is parsed and installed. If auth-token support is intentionally out of scope, it would be cleaner to remove those unrelated changes and claims from this PR.

3. The shortened key-method-2 parser can truncate a standard fragmented record

ParseServerKeyMethod2RecordConsumed() treats ioStringEOF while reading username, password, or peer-info as a successful end of record:

mihomo/transport/openvpn/keymethod.go

Lines 95 to 145 in cede0b8

	// ParseServerKeyMethod2RecordConsumed parses a server key-method-2 record and
	// reports how many bytes were consumed so following TLS control data
	// (PUSH_REPLY / AUTH_FAILED) can be preserved.
	//
	// OpenVPN 2.6 may omit the optional username, password and peer-info strings
	// after the mandatory options string.
	func ParseServerKeyMethod2RecordConsumed(packet []byte) (*KeyMethod2Record, int, error) {
	if len(packet) < 4+1+keySourceRandomSize*2 {
	return nil, 0, errors.New("key method 2 packet too short")
	}
	if binary.BigEndian.Uint32(packet[:4]) != 0 {
	return nil, 0, errors.New("invalid key method 2 prefix")
	}
	if packet[4]&0x0f != KeyMethod2 {
	return nil, 0, fmt.Errorf("unsupported key method %d", packet[4])
	}
	offset := 5
	record := &KeyMethod2Record{}
	copy(record.Sources.Server.Random1[:], packet[offset:offset+keySourceRandomSize])
	offset += keySourceRandomSize
	copy(record.Sources.Server.Random2[:], packet[offset:offset+keySourceRandomSize])
	offset += keySourceRandomSize
	var err error
	record.Options, offset, err = readOpenVPNString(packet, offset)
	if err != nil {
	return nil, 0, fmt.Errorf("read options: %w", err)
	}
	// Trailing username / password / peer-info are optional. A truncated
	// length prefix is treated as "not present" rather than a hard error so
	// a shortened 2.6 record still parses.
	if record.Username, offset, err = readOpenVPNString(packet, offset); err != nil {
	if errors.Is(err, ioStringEOF) {
	return record, offset, nil
	}
	return nil, 0, err
	}
	if record.Password, offset, err = readOpenVPNString(packet, offset); err != nil {
	if errors.Is(err, ioStringEOF) {
	return record, offset, nil
	}
	return nil, 0, err
	}
	if record.PeerInfo, offset, err = readOpenVPNString(packet, offset); err != nil {
	if errors.Is(err, ioStringEOF) {
	return record, offset, nil
	}
	return nil, 0, err
	}
	return record, offset, nil
	}

TLS is a byte stream. ioStringEOF cannot distinguish these two cases:

1. A non-standard peer omitted the trailing fields.
2. A standard record was split across two TLS Read calls.

OpenVPN 2.6.20's key_method_2_write() writes empty username/password strings when credentials are absent and then writes the peer-info field:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.c#L2263-L2320

The corresponding reader also always extracts username, password, and peer-info:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.c#L2419-L2435

Consequently, a normal record fragmented immediately after the options string can be accepted prematurely. Its remaining bytes will then be treated as following TLS control data and can make PUSH parsing wait until timeout.

If compatibility with a proven shortened record is required, only terminate early when the remaining bytes are positively identified as a following message such as PUSH_REPLY or AUTH_FAILED. Otherwise, keep reading. This behavior needs a test where each optional length/value is split across separate reads.

4. Soft-reset key ID validation accepts stale or invalid epochs

classifyWatchPacketLocked() accepts every soft reset whose key ID differs from the current one:

mihomo/transport/openvpn/control.go

Lines 337 to 349 in cede0b8

	// classifyWatchPacketLocked keeps packets from another session or key epoch
	// out of the established control channel. c.mu must be held by the caller.
	func (c *ControlChannel) classifyWatchPacketLocked(packet *ControlPacket) (softReset bool, valid bool) {
	if c.remote == (SessionID{}) || packet.LocalSession != c.remote {
	return false, false
	}
	if packet.Opcode == PControlSoftResetV1 {
	// Accept a soft reset from the same session on a different key epoch.
	// OpenVPN advances 0->1->...->7->1; do not invent the next ID locally.
	return packet.KeyID != c.keyID, packet.KeyID != c.keyID
	}
	return false, packet.KeyID == c.keyID
	}

That allows a delayed reset from a retiring epoch to move the client backwards. It also accepts key ID 0 after the initial epoch, although OpenVPN reserves it for the initial key state.

The server supplies the new key ID, but the client should still validate it:

expected := NextKeyID(c.keyID)
return packet.KeyID == expected, packet.KeyID == expected

The current test that expects key ID 0 after key ID 1 should be corrected. The expected sequence is 0 -> 1 -> 2 -> ... -> 7 -> 1.

5. UDP control packets are not retransmitted during rekey

The client soft reset, TLS ClientHello, and subsequent TLS control records are inserted into pending, but RetransmitPending() is only driven while waiting for the initial hard-reset response:

mihomo/transport/openvpn/client.go

Lines 531 to 557 in cede0b8

	func (c *Client) waitServerReset(ctx context.Context) error {
	retransmits := 0
	for {
	readCtx := ctx
	cancel := func() {}
	if c.config.Proto == ProtoUDP {
	readCtx, cancel = context.WithTimeout(ctx, ControlRetransmitDelay)
	}
	packet, err := c.control.Read(readCtx)
	cancel()
	if err != nil {
	if c.config.Proto == ProtoUDP && errors.Is(err, context.DeadlineExceeded) && ctx.Err() == nil {
	if err := c.control.RetransmitPending(ctx); err != nil {
	return fmt.Errorf("retransmit hard reset: %w", err)
	}
	retransmits++
	continue
	}
	return fmt.Errorf("read hard reset response after %d retransmits: %w", retransmits, err)
	}
	switch packet.Opcode {
	case PControlHardResetServerV2:
	return c.control.SendAck(ctx)
	case PControlHardResetServerV1:
	return fmt.Errorf("openvpn server replied with unsupported key method 1 reset")
	}
	}

There is no retransmission loop during renegotiate(). Losing any one of those UDP control packets can therefore stall the rekey until its 30-second timeout.

This may be separated from the minimal no-loss fix, but the PR should not claim reliable UDP rekey handling without a retransmission path and a packet-loss regression test.

[olicesx]

Review: fix(openvpn): survive server-initiated soft reset / rekey

Thanks for the PR — I verified the changes against the OpenVPN 2.6 reference source (ssl.c / push.c / auth_token.c) and by running the package tests locally (all pass; go vet clean).

Correct: key ID adoption with the 0→1→…→7→1 wrap (skipping 0), replying with a soft reset as the first packet of the new epoch, retiring/lame-duck data-channel routing by key ID, a fresh TLS session per epoch, and standard base64 decoding of auth-token-user all match the reference client. The retiring-channel design is compatible with the server's transition window.

Blocking

1. Control-channel deadline is never cleared after a rekey — client.go:171
startTLSEpoch sets a deadline on the shared control channel, but unlike the initial handshake (which clears it at client.go:148 with SetDeadline(time.Time{})), the renegotiation path never clears it. waitForSoftReset then blocks on that stale deadline, so the tunnel is torn down ~30s after every successful rekey whenever the reneg interval is > 30s (the default is 3600s). Your reneg-sec 10 test hides this because the next soft reset always arrives before the stale deadline fires. Please clear the deadline when renegotiate() finishes (ideally via defer).

High

2. Auth-token renewal during renegotiation is dropped — client.go:220
With auth-gen-token, OpenVPN servers push a fresh token-only PUSH_REPLY after every renegotiation (resend_auth_token_renegotiation → send_push_reply_auth_token, no client PUSH_REQUEST). This rekey branch never reads it, and while idle the watcher ack-and-discards those P_CONTROL_V1 records. With a token lifetime configured, the second rekey's key-method-2 auth fails (AUTH_TOKEN_EXPIRED) and the tunnel dies silently — no error surfaced, no fallback. takePushReply/ParsePushReplyFlexible are already shaped for exactly this message; they're just not called here.

3. No client→server retransmission during rekey (UDP only) — client.go:459
The soft-reset reply, ClientHello and client key-method-2 record are each sent once; RetransmitPending is only invoked during the initial hard reset. Over UDP, losing any one of those packets stalls readServerKeyMethod until the 30s timeout kills the tunnel. The reference client retransmits pending reliable messages on a schedule regardless of state.

Medium

4. peer-id is not carried across a rekey — client.go:223
mergePushReply(c.push, &PushReply{}) cannot inherit PeerID because the zero value is 0, not the PeerIDUnset (0xffffff) sentinel the merge checks for. The outgoing data header therefore flips from peer-id N to peer-id 0 after the first rekey (see dataHeader). Please inherit the previous peer-id explicitly in the rekey branch.

5. Data races — client.go:440
c.lastRekeyErr is written here on the watchControl goroutine and read unsynchronized from ReadIPPacket/LastRekeyError on the packet-reader goroutine; c.tlsConn is also replaced on watchControl while Client.Close() may read/close it. A -race CI run would likely flag both. Consider atomic.Pointer[error] for the error and protecting the tlsConn swap.

Minor

6. Error wrapping pollution — client.go:382
ReadIPPacket wraps unrelated read errors (e.g. context.Canceled on close) with the rekey error. Consider only appending lastRekeyErr when the underlying error is net.ErrClosed or similar.

7. Dead code — several fields/accessors have no production callers: lastDataKey, tlsEpoch, lastAuthToken, ActiveDataKeyID, LastSoftResetKeyID, LastRekeyError, and the no-op rekeyLogOnce.Do.

Happy to re-review once the deadline issue (and ideally the auth-token / UDP gaps) are addressed.

[wwqgtxx]

The original reneg-sec + 30 seconds blocker is therefore fixed. However, I still would not merge the current revision because several deterministic edge cases remain in the new rekey path.

1. an ordinary control read can park a stale soft reset without validating the next key ID

The watcher path now correctly requires a soft reset to use NextKeyID(c.keyID), but the ordinary TLS/control read bypasses that validation:

mihomo/transport/openvpn/control.go

Lines 303 to 315 in bee4126

	if !watchSoftReset {
	c.mu.Lock()
	curKey := c.keyID
	sameSession := c.remote == (SessionID{}) || packet.LocalSession == c.remote
	if packet.Opcode == PControlSoftResetV1 && packet.KeyID != curKey && sameSession {
	if c.pendingSoftReset == nil {
	c.pendingSoftReset = packet
	}
	for _, ackID := range packet.AckIDs {
	delete(c.pending, ackID)
	}
	c.mu.Unlock()
	continue

This branch parks every same-session soft reset whose key ID merely differs from the current key ID. waitForSoftReset() later returns the parked packet without revalidating it:

mihomo/transport/openvpn/control.go

Lines 205 to 212 in bee4126

	func (c *ControlChannel) waitForSoftReset(ctx context.Context) (*ControlPacket, error) {
	c.mu.Lock()
	if c.pendingSoftReset != nil {
	packet := c.pendingSoftReset
	c.pendingSoftReset = nil
	c.mu.Unlock()
	return packet, nil
	}

A focused in-memory test consistently reproduces the following sequence:

current client epoch: 2
delayed same-session soft reset arrives for epoch: 1
ControlChannel.Read parks it in pendingSoftReset
waitForSoftReset accepts epoch 1

This can move the client backwards to a retiring key epoch. It does not require an invalid key ID or a broken server; a delayed or retransmitted UDP packet is sufficient.

The minimal fix is to apply the same next-key check before parking the packet:

expected := NextKeyID(curKey)
if packet.Opcode == PControlSoftResetV1 &&
    packet.KeyID == expected && sameSession {
    // park the reset
}

An invalid soft reset should not mutate ACK, pending-message, receive-sequence, or epoch state.

2. a token-only PUSH_REPLY split across two TLS reads is discarded

readTokenPushReply() parses the buffer before each read, but after the second read it exits the loop without parsing the bytes that were just appended:

mihomo/transport/openvpn/client.go

Lines 712 to 733 in bee4126

	func readTokenPushReply(conn *tls.Conn, leftover []byte) (*PushReply, []byte, bool) {
	buf := append([]byte(nil), leftover...)
	tmp := make([]byte, 4096)
	// Try leftover first, then read with a short deadline so a record
	// arriving right after the key-method-2 record is still captured.
	for attempt := 0; attempt < 2; attempt++ {
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, true
	}
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, false
	}
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	if err != nil {
	// Timeout (or no more data): keep whatever was buffered.
	_ = conn.SetReadDeadline(time.Time{})
	return nil, buf, false
	}
	buf = append(buf, tmp[:n]...)
	}
	return nil, buf, false

Additionally, consumeRekeyPush() only stores the returned rest when ok is true:

mihomo/transport/openvpn/client.go

Lines 308 to 315 in bee4126

	if reply, rest, ok := takePushReply(c.leftoverTLS); ok {
	c.leftoverTLS = rest
	push = mergePushReply(push, reply)
	} else if conn := c.tlsConn.Load(); conn != nil {
	if reply, rest, ok := readTokenPushReply(conn, c.leftoverTLS); ok {
	c.leftoverTLS = rest
	push = mergePushReply(push, reply)
	}

I reproduced this with an in-memory TLS connection by writing the message in two parts:

read 1: PUSH_REPLY,auth-token 
read 2: SESS_ID_split\0

The helper returns ok=false even though its returned buffer contains the complete message:

PUSH_REPLY,auth-token SESS_ID_split\0

The caller then discards that buffer and continues using the old token. TLS exposes a byte stream, so this fragmentation is valid and must not depend on packet or record boundaries.

The minimal fix is to parse after every successful read, including the final allowed read, and always preserve buffered bytes when no complete message has been found. A cleaner interface would return (*PushReply, []byte, error) so absence, incomplete input, and protocol failure are distinct states.

3. AUTH_FAILED during a rekey is treated as an absent token

The token helper explicitly converts AUTH_FAILED into ok=false:

mihomo/transport/openvpn/client.go

Lines 721 to 723 in bee4126

	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, false
	}

takePushReply() does the same:

mihomo/transport/openvpn/client.go

Lines 736 to 747 in bee4126

	func takePushReply(buf []byte) (*PushReply, []byte, bool) {
	if len(buf) == 0 {
	return nil, nil, false
	}
	// AUTH_FAILED may arrive instead of PUSH_REPLY.
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	msg := string(buf)
	if idx := strings.IndexByte(msg, 0); idx >= 0 {
	msg = msg[:idx]
	}
	return nil, nil, false
	}

Because consumeRekeyPush() cannot return an error, doKeyExchange() proceeds with the previous push state and installs a new data channel even though the server rejected authentication:

mihomo/transport/openvpn/client.go

Lines 232 to 255 in bee4126

	if c.push != nil {
	// Authenticated rekeys keep the previous ifconfig / peer-id.
	// OpenVPN 2.6 often does not send another PUSH_REPLY here, but may
	// push a fresh auth-token (send_push_reply_auth_token) that must be
	// consumed to keep the next key-method-2 auth from expiring.
	c.consumeRekeyPush()
	push := c.push
	negotiatedCipher, err := c.config.NegotiateCipher(push.DataCiphers, push.Cipher)
	if err != nil {
	return nil, fmt.Errorf("negotiate data cipher: %w", err)
	}
	c.negotiatedCipher = negotiatedCipher
	cipherKeyLen := CipherKeyLength(negotiatedCipher)
	keys.SendCipherKey = keys.SendCipherKey[:cipherKeyLen]
	keys.RecvCipherKey = keys.RecvCipherKey[:cipherKeyLen]
	keyID := c.control.KeyID()
	newData, err := NewDataChannel(keys, negotiatedCipher, c.config.Auth, push.PeerID, keyID)
	if err != nil {
	return nil, err
	}
	c.installDataChannel(newData)
	c.markSend()
	c.markReceive()
	return push, nil

A focused in-memory test with:

AUTH_FAILED,SESSION: auth-token expired\0

consistently leaves the old token installed and reports no rekey error. This turns an explicit authentication failure into an apparently successful rekey followed by unexplained data loss or a later control-channel failure.

The minimal fix is for the rekey push-consumption path to return an error when AUTH_FAILED is present and for doKeyExchange() to abort before installing the new data channel.

4. retransmitting the client soft reset removes its original reset ACK

The first client soft reset carries the ACK for server soft-reset message 0. RetransmitPending() replaces the stored packet's ACK list with the current ackPending list:

mihomo/transport/openvpn/control.go

Lines 404 to 421 in bee4126

	func (c *ControlChannel) RetransmitPending(ctx context.Context) error {
	c.mu.Lock()
	packets := make([]*ControlPacket, 0, len(c.pending))
	for _, packet := range c.pending {
	cp := *packet
	cp.AckIDs = append([]uint32(nil), c.ackPending...)
	cp.AckRemoteSession = c.remote
	packets = append(packets, &cp)
	}
	c.ackPending = nil
	c.mu.Unlock()
	for _, packet := range packets {
	if err := c.writeControlPacket(ctx, packet); err != nil {
	return err
	}
	}
	return nil

The original send has already cleared ackPending, so the retransmitted packet contains no ACK. A focused in-memory packet test produces:

original client soft reset AckIDs:      [0]
retransmitted client soft reset AckIDs: []

OpenVPN keeps recently transmitted ACKs in ack_mru and includes them again in later control packets:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/reliable.c#L252-L304

The peer can sometimes recover by retransmitting its reset and causing another ACK, so this is less direct than the failures above. It nevertheless weakens the reliability mechanism added by this PR and can leave the server repeatedly retransmitting the reset.

At minimum, retransmission should merge the stored packet's ACKs with current pending ACKs instead of replacing the stored ACK list.

5. the ClientHello loss test does not actually require a retransmission

TestRekeyRetransmitsLostClientHello says that the first ClientHello is dropped, but the loop succeeds as soon as it sees the first ClientHello packet:

mihomo/transport/openvpn/rekey_test.go

Lines 402 to 473 in bee4126

	func TestRekeyRetransmitsLostClientHello(t *testing.T) {
	clientIO, serverIO := newMemoryPacketPair()
	serverCrypt, err := NewTLSCrypt(testStaticKey(), false)
	if err != nil {
	t.Fatal(err)
	}
	var serverID SessionID
	copy(serverID[:], []byte("server01"))
	client, err := NewClient(&ClientConfig{
	Proto: ProtoUDP,
	TLSCryptKey: testStaticKey(),
	}, clientIO)
	if err != nil {
	t.Fatal(err)
	}
	defer client.Close()
	client.control.SetRemoteSessionID(serverID)
	server := NewControlChannel(serverIO, serverCrypt, serverID)
	server.SetRemoteSessionID(client.control.LocalSessionID())
	client.control.clock = func() time.Time { return time.Unix(1714567890, 0) }
	server.clock = func() time.Time { return time.Unix(1714567891, 0) }
	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
	defer cancel()
	// Server starts epoch 1 with a soft reset, exactly like a real rekey.
	server.AdoptKeyID(1)
	if _, err := server.Send(ctx, PControlSoftResetV1, nil); err != nil {
	t.Fatal(err)
	}
	soft, err := client.control.waitForSoftReset(ctx)
	if err != nil {
	t.Fatal(err)
	}
	client.control.AdoptKeyID(1)
	client.control.MarkReceived(soft.MessageID)
	client.control.QueueAck(soft.MessageID)
	if err := client.control.SendSoftReset(ctx); err != nil {
	t.Fatal(err)
	}
	// Start the UDP rekey retransmission loop exactly like renegotiate().
	stop := client.retransmitRekey(ctx)
	defer stop()
	// Simulate the TLS epoch: a ClientHello record on epoch 1.
	if _, err := client.control.Send(ctx, PControlV1, []byte("client-hello")); err != nil {
	t.Fatal(err)
	}
	// Drop the client-hello the first time it is sent; it must be
	// retransmitted by the loop.
	gotHello := false
	for i := 0; i < 3; i++ {
	raw, err := serverIO.ReadPacket(ctx)
	if err != nil {
	t.Fatal(err)
	}
	pkt, _, _, err := DecodeControlPacket(serverCrypt, raw)
	if err != nil {
	t.Fatal(err)
	}
	if pkt.Opcode == PControlV1 && string(pkt.Payload) == "client-hello" {
	gotHello = true
	break
	}
	}
	if !gotHello {
	t.Fatal("client-hello never retransmitted after loss")
	}
	}

It never records and ignores the first occurrence, nor does it require a second packet with the same message ID and payload. The test therefore passes even if the retransmission loop never retransmits ClientHello.

The test should explicitly consume and discard the first ClientHello, then pass only after receiving the same reliable control message a second time. A second test should drop the first client soft reset and verify that its retransmission still acknowledges the server reset.

[wwqgtxx]

Two error-handling issues remain in the new token-consumption path.

1. readTokenPushReply suppresses every TLS read error and can discard data returned with EOF

mihomo/transport/openvpn/client.go

Lines 751 to 758 in aadc60a

	for attempt := 0; attempt < 2; attempt++ {
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	if err != nil {
	// Timeout (or no more data): keep whatever was buffered.
	_ = conn.SetReadDeadline(time.Time{})
	return nil, buf, nil
	}

The helper currently handles every Read error as if it were the expected short timeout:

n, err := conn.Read(tmp)
if err != nil {
    _ = conn.SetReadDeadline(time.Time{})
    return nil, buf, nil
}
buf = append(buf, tmp[:n]...)

This has two problems:

1. io.EOF, io.ErrUnexpectedEOF, closed-connection errors, and TLS protocol errors are all converted into a successful "no token" result.
2. The io.Reader contract permits n > 0 and err != nil in the same call, but these bytes are discarded because err is checked before tmp[:n] is appended.

The TLS implementation used by this package has exactly such a path: when application data is immediately followed by close_notify, tls.Conn.Read can return (n, io.EOF):

https://github.com/metacubex/tls/blob/v0.1.7/conn.go#L1409-L1422

A deterministic in-memory reader test returning:

data = AUTH_FAILED,SESSION: auth-token expired\0
err  = io.EOF

reproduces the problem five out of five times: readTokenPushReply returns a nil error and loses the authentication failure. doKeyExchange() can then continue and install a new data channel after the control TLS stream has failed.

The minimal fix is:

• Process n > 0 before handling err.
• Parse the appended bytes even when err is also non-nil.
• Only treat a real timeout as the optional "no token available yet" case.
• Propagate EOF, unexpected EOF, and other TLS errors if no complete control message was obtained.

Conceptually:

n, err := conn.Read(tmp)
if n > 0 {
    buf = append(buf, tmp[:n]...)
    if reply, rest, ok := takePushReply(buf); ok {
        return reply, rest, nil
    }
    if bytes.Contains(buf, []byte("AUTH_FAILED")) {
        return nil, buf, authFailedError(buf)
    }
}
if err != nil {
    _ = conn.SetReadDeadline(time.Time{})
    var netErr net.Error
    if errors.As(err, &netErr) && netErr.Timeout() {
        return nil, buf, nil
    }
    return nil, buf, err
}

2. the watcher ignores a parked or deferred AUTH_FAILED

consumeRekeyPush() now returns an error, and doKeyExchange() checks it. The earlier watcher call still ignores that return value:

mihomo/transport/openvpn/client.go

Lines 475 to 481 in aadc60a

	// Token-only PUSH_REPLY parked since the last rekey must land in
	// authPass before this key-method-2 exchange, otherwise the server
	// rejects the expired token.
	c.consumeQueuedControl()
	if c.push != nil {
	c.consumeRekeyPush()
	}

c.consumeQueuedControl()
if c.push != nil {
    c.consumeRekeyPush()
}
if err := c.renegotiate(packet); err != nil {
    // ...
}

This path handles TLS control messages parked after the previous rekey. If one of those messages is AUTH_FAILED, consumeRekeyPush() correctly creates errAuthFailed, but the watcher discards it and proceeds into another renegotiation.

This is not only a synthetic ordering. OpenVPN supports deferred authentication, and when authentication later fails it sends AUTH_FAILED to the active and initial TLS sessions:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/push.c#L391-L429

A focused in-memory watcher test consistently shows that the authentication error is ignored and replaced by the subsequent renegotiation result.

The minimal fix is to propagate the existing error before starting the next epoch:

c.consumeQueuedControl()
if c.push != nil {
    if err := c.consumeRekeyPush(); err != nil {
        c.failControl(fmt.Errorf("consume queued rekey push: %w", err))
        return
    }
}

[Lanlan13-14]

Two error-handling issues remain in the new token-consumption path.

1. readTokenPushReply suppresses every TLS read error and can discard data returned with EOF

mihomo/transport/openvpn/client.go

Lines 751 to 758 in aadc60a

	for attempt := 0; attempt < 2; attempt++ {
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	if err != nil {
	// Timeout (or no more data): keep whatever was buffered.
	_ = conn.SetReadDeadline(time.Time{})
	return nil, buf, nil
	}

The helper currently handles every Read error as if it were the expected short timeout:

n, err := conn.Read(tmp)
if err != nil {
    _ = conn.SetReadDeadline(time.Time{})
    return nil, buf, nil
}
buf = append(buf, tmp[:n]...)

This has two problems:

1. io.EOF, io.ErrUnexpectedEOF, closed-connection errors, and TLS protocol errors are all converted into a successful "no token" result.
2. The io.Reader contract permits n > 0 and err != nil in the same call, but these bytes are discarded because err is checked before tmp[:n] is appended.

The TLS implementation used by this package has exactly such a path: when application data is immediately followed by close_notify, tls.Conn.Read can return (n, io.EOF):

https://github.com/metacubex/tls/blob/v0.1.7/conn.go#L1409-L1422

A deterministic in-memory reader test returning:

data = AUTH_FAILED,SESSION: auth-token expired\0
err  = io.EOF

reproduces the problem five out of five times: readTokenPushReply returns a nil error and loses the authentication failure. doKeyExchange() can then continue and install a new data channel after the control TLS stream has failed.

The minimal fix is:

• Process n > 0 before handling err.
• Parse the appended bytes even when err is also non-nil.
• Only treat a real timeout as the optional "no token available yet" case.
• Propagate EOF, unexpected EOF, and other TLS errors if no complete control message was obtained.

Conceptually:

n, err := conn.Read(tmp)
if n > 0 {
    buf = append(buf, tmp[:n]...)
    if reply, rest, ok := takePushReply(buf); ok {
        return reply, rest, nil
    }
    if bytes.Contains(buf, []byte("AUTH_FAILED")) {
        return nil, buf, authFailedError(buf)
    }
}
if err != nil {
    _ = conn.SetReadDeadline(time.Time{})
    var netErr net.Error
    if errors.As(err, &netErr) && netErr.Timeout() {
        return nil, buf, nil
    }
    return nil, buf, err
}

2. the watcher ignores a parked or deferred AUTH_FAILED

consumeRekeyPush() now returns an error, and doKeyExchange() checks it. The earlier watcher call still ignores that return value:

mihomo/transport/openvpn/client.go

Lines 475 to 481 in aadc60a

	// Token-only PUSH_REPLY parked since the last rekey must land in
	// authPass before this key-method-2 exchange, otherwise the server
	// rejects the expired token.
	c.consumeQueuedControl()
	if c.push != nil {
	c.consumeRekeyPush()
	}

c.consumeQueuedControl()
if c.push != nil {
    c.consumeRekeyPush()
}
if err := c.renegotiate(packet); err != nil {
    // ...
}

This path handles TLS control messages parked after the previous rekey. If one of those messages is AUTH_FAILED, consumeRekeyPush() correctly creates errAuthFailed, but the watcher discards it and proceeds into another renegotiation.

This is not only a synthetic ordering. OpenVPN supports deferred authentication, and when authentication later fails it sends AUTH_FAILED to the active and initial TLS sessions:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/push.c#L391-L429

A focused in-memory watcher test consistently shows that the authentication error is ignored and replaced by the subsequent renegotiation result.

The minimal fix is to propagate the existing error before starting the next epoch:

c.consumeQueuedControl()
if c.push != nil {
    if err := c.consumeRekeyPush(); err != nil {
        c.failControl(fmt.Errorf("consume queued rekey push: %w", err))
        return
    }
}

Thanks for the detailed review — all five points are real and I've fixed them. Round 3 of fixes is in aadc60a, plus 8e9b728 for the EOF/AUTH_FAILED handling in the token path and the ACK MRU.

1. Ordinary control read parks a stale soft reset

Fixed in control.go (read(), non-watch path): a soft reset is only parked when packet.KeyID == NextKeyID(curKey) and the session matches. A delayed reset from a retiring epoch is dropped without touching ACK / pending-message / receive-sequence state. waitForSoftReset re-validates nothing because it can only ever see a reset that already passed the check.

Test: TestStaleSoftResetNotParked — parks a reset at epoch 2, then confirms a retiring-epoch reset (key 1) is rejected and only key 3 is accepted.

2. Token-only PUSH_REPLY split across two TLS reads is discarded

Fixed in readTokenPushReply:

• parse after every read, including the final allowed one,
• preserve buffered bytes even when no complete message is found,
• and now also process bytes before handling err (the io.Reader contract allows n>0 with err != nil; tls.Conn returns (n, io.EOF) when app data is immediately followed by close_notify — that no longer drops the bytes).

consumeRekeyPush always stores the returned rest.

Tests: TestReadTokenPushReplySplitAcrossReads, TestReadTokenPushReplyPartialPreserved, TestReadTokenPushReplyAUTHFailedWithEOF.

3. AUTH_FAILED during a rekey treated as absent token

consumeRekeyPush now returns an error (errAuthFailed), doKeyExchange aborts before installing a new data channel, and watchControl propagates it too — so a parked/deferred AUTH_FAILED fails the tunnel instead of being swallowed by the next renegotiation.

Tests: TestConsumeRekeyPushAUTHFailed, TestWatchControlSurfacesParkedAUTHFailed.

4. Retransmitting the client soft reset loses its original reset ACK

RetransmitPending now emits one merged ACK set pulled through the MRU (takeAcksLocked), so a retransmitted soft reset still acknowledges the server's reset.

Test: TestRekeyRetransmitKeepsResetACK.

5. ClientHello loss test didn't actually require a retransmission

TestRekeyRetransmitsLostClientHello now consumes the first ClientHello, records its message ID, and only passes once a second packet with the same message ID and payload arrives.

Reliability layer alignment

ControlChannel now keeps an ACK MRU (lruAcks) that mirrors OpenVPN's reliable_ack_write / copy_acks_to_mru (reliable.c:208-297): queued ACKs are copied into the MRU when sent and ride on this and subsequent control packets until replaced, capped at RELIABLE_ACK_SIZE=8. TestMRUCarriesAckOnSubsequentSends verifies an ACK appears on two consecutive outbound packets.

The one deliberate difference: retransmission interval is 1s here vs the reference's packet_timeout (default 60s). The protocol does not mandate the interval; 1s simply recovers faster on lossy UDP rekeys and the peer tolerates duplicate control packets. I kept that rather than shipping a slower default.

Verification

• go test -race ./transport/openvpn/ clean
• Live: OpenVPN 2.6.14, reneg-sec 45, auth-gen-token 0 180 — 2 soft resets, 2× Username/auth-token authentication succeeded, 101/101 HTTP probes, no TLS Error / link closed.

[wwqgtxx]

The earlier rekey failure chain therefore appears to be fixed. However, one error-handling edge case remains, and the newly added ACK MRU introduces two protocol-compatibility issues.

1. a complete PUSH_REPLY still suppresses an EOF returned by the same read

mihomo/transport/openvpn/client.go

Lines 762 to 770 in 8e9b728

	if n > 0 {
	buf = append(buf, tmp[:n]...)
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	}

The helper now appends n > 0 bytes before handling err, but it returns immediately when those bytes contain a complete reply:

if n > 0 {
    buf = append(buf, tmp[:n]...)
    if reply, rest, ok := takePushReply(buf); ok {
        return reply, rest, nil
    }
    // ...
}
if err != nil {
    // ...
}

Consequently, a read returning both a complete token update and EOF still converts the closed TLS stream into success. A deterministic reader returning:

data = PUSH_REPLY,auth-token SESS_ID_new\0
err  = io.EOF

reproduced this five out of five times: readTokenPushReply returned the parsed reply and a nil error. doKeyExchange can therefore install a new data channel even though the control TLS stream has already closed.

The minimal fix is to retain the parsed result until err has been classified. A real timeout may still return the complete reply, but EOF and other TLS errors should be propagated rather than hidden. AUTH_FAILED should continue to take precedence so its useful diagnostic is preserved.

2. the ACK MRU exceeds OpenVPN's four-ACK compatibility limit

mihomo/transport/openvpn/control.go

Lines 194 to 235 in 8e9b728

	func (c *ControlChannel) takeAcksLocked() []uint32 {
	// Move ackPending to the front of the MRU (newest first), dedup.
	for i := len(c.ackPending) - 1; i >= 0; i-- {
	id := c.ackPending[i]
	if !containsUint32(c.lruAcks, id) {
	c.lruAcks = append([]uint32{id}, c.lruAcks...)
	}
	}
	c.ackPending = nil
	// Cap at RELIABLE_ACK_SIZE.
	if len(c.lruAcks) > reliableAckSize {
	c.lruAcks = c.lruAcks[:reliableAckSize]
	}
	return append([]uint32(nil), c.lruAcks...)
	}
	func containsUint32(s []uint32, v uint32) bool {
	for _, x := range s {
	if x == v {
	return true
	}
	}
	return false
	}
	// reliableAckSize mirrors RELIABLE_ACK_SIZE in OpenVPN reliable.h.
	const reliableAckSize = 8
	func (c *ControlChannel) SendAck(ctx context.Context) error {
	c.mu.Lock()
	if len(c.ackPending) == 0 {
	c.mu.Unlock()
	return nil
	}
	ackIDs := c.takeAcksLocked()
	packet := &ControlPacket{
	Opcode: PAckV1,
	KeyID: c.keyID,
	LocalSession: c.local,
	AckIDs: ackIDs,
	AckRemoteSession: c.remote,
	}

takeAcksLocked always returns up to RELIABLE_ACK_SIZE (8) entries, and the result is used for both reliable control packets and dedicated ACK packets.

This does not match the reference client:

• Reliable control packets use CONTROL_SEND_ACK_MAX, which is 4:
  https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.h#L55
• For an unprotected client control channel, OpenVPN also limits dedicated ACK packets to 4. The source explicitly documents that SoftEther drops packets containing more than four ACKs:
  https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl_pkt.c#L178-L185

An in-memory test queued and sent five ACKs through a plain ControlChannel. The fifth dedicated ACK packet contained:

[4 3 2 1 0]

This creates a compatibility regression for configurations without tls-auth or tls-crypt, which mihomo supports. Once the MRU grows beyond four entries, a SoftEther peer can discard every such packet.

The minimal fix is to make ACK extraction accept a per-packet maximum:

• Use 4 for reliable control packets.
• Use 8 for a dedicated ACK packet when the protected control-channel mode permits it.
• Use 4 for dedicated ACK packets when c.crypt == nil, matching the reference client's compatibility behavior.

The MRU itself may still retain eight entries; only the number serialized into a particular packet needs to be limited.

3. existing ACKs are not moved to the front of the MRU

mihomo/transport/openvpn/control.go

Lines 194 to 207 in 8e9b728

	func (c *ControlChannel) takeAcksLocked() []uint32 {
	// Move ackPending to the front of the MRU (newest first), dedup.
	for i := len(c.ackPending) - 1; i >= 0; i-- {
	id := c.ackPending[i]
	if !containsUint32(c.lruAcks, id) {
	c.lruAcks = append([]uint32{id}, c.lruAcks...)
	}
	}
	c.ackPending = nil
	// Cap at RELIABLE_ACK_SIZE.
	if len(c.lruAcks) > reliableAckSize {
	c.lruAcks = c.lruAcks[:reliableAckSize]
	}
	return append([]uint32(nil), c.lruAcks...)

The implementation skips an ACK ID when it already exists in lruAcks:

if !containsUint32(c.lruAcks, id) {
    c.lruAcks = append([]uint32{id}, c.lruAcks...)
}

OpenVPN's copy_acks_to_mru instead moves an existing ID back to the front:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/reliable.c#L211-L249

This difference matters when the MRU is full. For example:

existing MRU: [1 2 3 4 5 6 7 8]
pending ACKs: [8 9]

The reference algorithm produces:

[8 9 1 2 3 4 5 6]

The PR produces:

[9 1 2 3 4 5 6 7]

ACK 8 was just requested again, commonly because the peer retransmitted that reliable packet, but it is nevertheless evicted. This weakens recovery precisely for the packet whose previous ACK may have been lost.

The minimal fix is to implement the same move-to-front behavior as copy_acks_to_mru, rather than treating the MRU as a simple deduplicated list.

[Lanlan13-14]

The earlier rekey failure chain therefore appears to be fixed. However, one error-handling edge case remains, and the newly added ACK MRU introduces two protocol-compatibility issues.

1. a complete PUSH_REPLY still suppresses an EOF returned by the same read

mihomo/transport/openvpn/client.go

Lines 762 to 770 in 8e9b728

	if n > 0 {
	buf = append(buf, tmp[:n]...)
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	}

The helper now appends n > 0 bytes before handling err, but it returns immediately when those bytes contain a complete reply:

if n > 0 {
    buf = append(buf, tmp[:n]...)
    if reply, rest, ok := takePushReply(buf); ok {
        return reply, rest, nil
    }
    // ...
}
if err != nil {
    // ...
}

Consequently, a read returning both a complete token update and EOF still converts the closed TLS stream into success. A deterministic reader returning:

data = PUSH_REPLY,auth-token SESS_ID_new\0
err  = io.EOF

reproduced this five out of five times: readTokenPushReply returned the parsed reply and a nil error. doKeyExchange can therefore install a new data channel even though the control TLS stream has already closed.

The minimal fix is to retain the parsed result until err has been classified. A real timeout may still return the complete reply, but EOF and other TLS errors should be propagated rather than hidden. AUTH_FAILED should continue to take precedence so its useful diagnostic is preserved.

2. the ACK MRU exceeds OpenVPN's four-ACK compatibility limit

mihomo/transport/openvpn/control.go

Lines 194 to 235 in 8e9b728

	func (c *ControlChannel) takeAcksLocked() []uint32 {
	// Move ackPending to the front of the MRU (newest first), dedup.
	for i := len(c.ackPending) - 1; i >= 0; i-- {
	id := c.ackPending[i]
	if !containsUint32(c.lruAcks, id) {
	c.lruAcks = append([]uint32{id}, c.lruAcks...)
	}
	}
	c.ackPending = nil
	// Cap at RELIABLE_ACK_SIZE.
	if len(c.lruAcks) > reliableAckSize {
	c.lruAcks = c.lruAcks[:reliableAckSize]
	}
	return append([]uint32(nil), c.lruAcks...)
	}
	func containsUint32(s []uint32, v uint32) bool {
	for _, x := range s {
	if x == v {
	return true
	}
	}
	return false
	}
	// reliableAckSize mirrors RELIABLE_ACK_SIZE in OpenVPN reliable.h.
	const reliableAckSize = 8
	func (c *ControlChannel) SendAck(ctx context.Context) error {
	c.mu.Lock()
	if len(c.ackPending) == 0 {
	c.mu.Unlock()
	return nil
	}
	ackIDs := c.takeAcksLocked()
	packet := &ControlPacket{
	Opcode: PAckV1,
	KeyID: c.keyID,
	LocalSession: c.local,
	AckIDs: ackIDs,
	AckRemoteSession: c.remote,
	}

takeAcksLocked always returns up to RELIABLE_ACK_SIZE (8) entries, and the result is used for both reliable control packets and dedicated ACK packets.

This does not match the reference client:

• Reliable control packets use CONTROL_SEND_ACK_MAX, which is 4:
  https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.h#L55
• For an unprotected client control channel, OpenVPN also limits dedicated ACK packets to 4. The source explicitly documents that SoftEther drops packets containing more than four ACKs:
  https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl_pkt.c#L178-L185

An in-memory test queued and sent five ACKs through a plain ControlChannel. The fifth dedicated ACK packet contained:

[4 3 2 1 0]

This creates a compatibility regression for configurations without tls-auth or tls-crypt, which mihomo supports. Once the MRU grows beyond four entries, a SoftEther peer can discard every such packet.

The minimal fix is to make ACK extraction accept a per-packet maximum:

• Use 4 for reliable control packets.
• Use 8 for a dedicated ACK packet when the protected control-channel mode permits it.
• Use 4 for dedicated ACK packets when c.crypt == nil, matching the reference client's compatibility behavior.

The MRU itself may still retain eight entries; only the number serialized into a particular packet needs to be limited.

3. existing ACKs are not moved to the front of the MRU

mihomo/transport/openvpn/control.go

Lines 194 to 207 in 8e9b728

	func (c *ControlChannel) takeAcksLocked() []uint32 {
	// Move ackPending to the front of the MRU (newest first), dedup.
	for i := len(c.ackPending) - 1; i >= 0; i-- {
	id := c.ackPending[i]
	if !containsUint32(c.lruAcks, id) {
	c.lruAcks = append([]uint32{id}, c.lruAcks...)
	}
	}
	c.ackPending = nil
	// Cap at RELIABLE_ACK_SIZE.
	if len(c.lruAcks) > reliableAckSize {
	c.lruAcks = c.lruAcks[:reliableAckSize]
	}
	return append([]uint32(nil), c.lruAcks...)

The implementation skips an ACK ID when it already exists in lruAcks:

if !containsUint32(c.lruAcks, id) {
    c.lruAcks = append([]uint32{id}, c.lruAcks...)
}

OpenVPN's copy_acks_to_mru instead moves an existing ID back to the front:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/reliable.c#L211-L249

This difference matters when the MRU is full. For example:

existing MRU: [1 2 3 4 5 6 7 8]
pending ACKs: [8 9]

The reference algorithm produces:

[8 9 1 2 3 4 5 6]

The PR produces:

[9 1 2 3 4 5 6 7]

ACK 8 was just requested again, commonly because the peer retransmitted that reliable packet, but it is nevertheless evicted. This weakens recovery precisely for the packet whose previous ACK may have been lost.

The minimal fix is to implement the same move-to-front behavior as copy_acks_to_mru, rather than treating the MRU as a simple deduplicated list.

MRU move-to-front. You're right — takeAcksLocked treated the MRU as a deduplicated list and could evict a re-acked ID when full. Reimplemented as copy_acks_to_mru: every pending ID is unconditionally moved to the front, shifting existing entries right and dropping the duplicate. TestMRUMoveToFrontMatchesReference pins your exact example (MRU [1..8], pending [8 9] → [8 9 1 2 3 4 5 6]). Fix is in 55744f6.

[wwqgtxx]

One substantive compatibility issue remains in the ACK MRU implementation.

mihomo/transport/openvpn/control.go

Lines 161 to 177 in 55744f6

	func (c *ControlChannel) Send(ctx context.Context, opcode Opcode, payload []byte) (uint32, error) {
	if !opcode.HasMessageID() {
	return 0, fmt.Errorf("opcode %s cannot carry a reliable message", opcode)
	}
	c.mu.Lock()
	messageID := c.sendMessage
	c.sendMessage++
	// Copy pending acks into the MRU and take the ack list from the MRU,
	// exactly like OpenVPN reliable_ack_write: recently acked IDs ride on
	// this and subsequent packets until replaced.
	ackIDs := c.takeAcksLocked()
	packet := &ControlPacket{
	Opcode: opcode,
	KeyID: c.keyID,
	LocalSession: c.local,
	AckIDs: ackIDs,

mihomo/transport/openvpn/control.go

Lines 196 to 239 in 55744f6

	func (c *ControlChannel) takeAcksLocked() []uint32 {
	// Move ackPending (newest last) into the MRU front, preserving their
	// relative order, exactly like copy_acks_to_mru's backward loop.
	for i := len(c.ackPending) - 1; i >= 0; i-- {
	id := c.ackPending[i]
	move := id
	found := false
	for j := 0; j < len(c.lruAcks); j++ {
	tmp := c.lruAcks[j]
	c.lruAcks[j] = move
	move = tmp
	if move == id {
	found = true
	break
	}
	}
	if !found && len(c.lruAcks) < reliableAckSize {
	c.lruAcks = append(c.lruAcks, move)
	}
	}
	c.ackPending = nil
	// Cap at RELIABLE_ACK_SIZE (move-to-front never grows past it).
	if len(c.lruAcks) > reliableAckSize {
	c.lruAcks = c.lruAcks[:reliableAckSize]
	}
	return append([]uint32(nil), c.lruAcks...)
	}
	// reliableAckSize mirrors RELIABLE_ACK_SIZE in OpenVPN reliable.h.
	const reliableAckSize = 8
	func (c *ControlChannel) SendAck(ctx context.Context) error {
	c.mu.Lock()
	if len(c.ackPending) == 0 {
	c.mu.Unlock()
	return nil
	}
	ackIDs := c.takeAcksLocked()
	packet := &ControlPacket{
	Opcode: PAckV1,
	KeyID: c.keyID,
	LocalSession: c.local,
	AckIDs: ackIDs,
	AckRemoteSession: c.remote,

takeAcksLocked always returns up to RELIABLE_ACK_SIZE (8) entries. That same result is serialized into both reliable control packets and dedicated ACK packets.

This does not preserve the reference client's per-packet limits:

• Reliable control packets use CONTROL_SEND_ACK_MAX, which is 4:
  https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.h#L55
• For an unprotected client control channel, OpenVPN also limits dedicated ACK packets to 4. The source explicitly documents that SoftEther drops packets containing more than four ACKs:
  https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl_pkt.c#L178-L185

mihomo supports configurations without tls-auth or tls-crypt, and its fixed IV_PROTO=6 does not advertise TLS key material export. The compatibility condition therefore applies whenever c.crypt == nil.

A deterministic in-memory test queued and sent five ACKs through a plain ControlChannel. The fifth dedicated ACK packet contained:

[4 3 2 1 0]

The test reproduced this for ten consecutive runs. A SoftEther peer can discard that packet. When it retransmits the same reliable message, the MRU causes the client to send another packet containing more than four ACKs, so the control channel can remain stuck.

The minimal fix is to separate MRU capacity from the number of ACKs serialized into a packet:

• Keep the MRU capacity at 8.
• Serialize at most 4 ACKs into reliable control packets, including retransmissions.
• Serialize at most 8 ACKs into an ordinary dedicated ACK packet.
• Limit a dedicated ACK packet to 4 when c.crypt == nil.

[Lanlan13-14]

One substantive compatibility issue remains in the ACK MRU implementation.

mihomo/transport/openvpn/control.go

Lines 161 to 177 in 55744f6

	func (c *ControlChannel) Send(ctx context.Context, opcode Opcode, payload []byte) (uint32, error) {
	if !opcode.HasMessageID() {
	return 0, fmt.Errorf("opcode %s cannot carry a reliable message", opcode)
	}
	c.mu.Lock()
	messageID := c.sendMessage
	c.sendMessage++
	// Copy pending acks into the MRU and take the ack list from the MRU,
	// exactly like OpenVPN reliable_ack_write: recently acked IDs ride on
	// this and subsequent packets until replaced.
	ackIDs := c.takeAcksLocked()
	packet := &ControlPacket{
	Opcode: opcode,
	KeyID: c.keyID,
	LocalSession: c.local,
	AckIDs: ackIDs,

mihomo/transport/openvpn/control.go

Lines 196 to 239 in 55744f6

	func (c *ControlChannel) takeAcksLocked() []uint32 {
	// Move ackPending (newest last) into the MRU front, preserving their
	// relative order, exactly like copy_acks_to_mru's backward loop.
	for i := len(c.ackPending) - 1; i >= 0; i-- {
	id := c.ackPending[i]
	move := id
	found := false
	for j := 0; j < len(c.lruAcks); j++ {
	tmp := c.lruAcks[j]
	c.lruAcks[j] = move
	move = tmp
	if move == id {
	found = true
	break
	}
	}
	if !found && len(c.lruAcks) < reliableAckSize {
	c.lruAcks = append(c.lruAcks, move)
	}
	}
	c.ackPending = nil
	// Cap at RELIABLE_ACK_SIZE (move-to-front never grows past it).
	if len(c.lruAcks) > reliableAckSize {
	c.lruAcks = c.lruAcks[:reliableAckSize]
	}
	return append([]uint32(nil), c.lruAcks...)
	}
	// reliableAckSize mirrors RELIABLE_ACK_SIZE in OpenVPN reliable.h.
	const reliableAckSize = 8
	func (c *ControlChannel) SendAck(ctx context.Context) error {
	c.mu.Lock()
	if len(c.ackPending) == 0 {
	c.mu.Unlock()
	return nil
	}
	ackIDs := c.takeAcksLocked()
	packet := &ControlPacket{
	Opcode: PAckV1,
	KeyID: c.keyID,
	LocalSession: c.local,
	AckIDs: ackIDs,
	AckRemoteSession: c.remote,

takeAcksLocked always returns up to RELIABLE_ACK_SIZE (8) entries. That same result is serialized into both reliable control packets and dedicated ACK packets.

This does not preserve the reference client's per-packet limits:

• Reliable control packets use CONTROL_SEND_ACK_MAX, which is 4:
  https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.h#L55
• For an unprotected client control channel, OpenVPN also limits dedicated ACK packets to 4. The source explicitly documents that SoftEther drops packets containing more than four ACKs:
  https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl_pkt.c#L178-L185

mihomo supports configurations without tls-auth or tls-crypt, and its fixed IV_PROTO=6 does not advertise TLS key material export. The compatibility condition therefore applies whenever c.crypt == nil.

A deterministic in-memory test queued and sent five ACKs through a plain ControlChannel. The fifth dedicated ACK packet contained:

[4 3 2 1 0]

The test reproduced this for ten consecutive runs. A SoftEther peer can discard that packet. When it retransmits the same reliable message, the MRU causes the client to send another packet containing more than four ACKs, so the control channel can remain stuck.

The minimal fix is to separate MRU capacity from the number of ACKs serialized into a packet:

• Keep the MRU capacity at 8.
• Serialize at most 4 ACKs into reliable control packets, including retransmissions.
• Serialize at most 8 ACKs into an ordinary dedicated ACK packet.
• Limit a dedicated ACK packet to 4 when c.crypt == nil.

All three are real — addressed in a14e01d (items 2, 3) and 28c2622 (item 1).

1. Complete PUSH_REPLY suppresses EOF. readTokenPushReply returned success the moment the buffer parsed into a reply, even when the same read returned io.EOF. Now a complete reply is retained but only returned on a real timeout; EOF / unexpected EOF / TLS errors are propagated (AUTH_FAILED still takes precedence for its diagnostic). TestReadTokenPushReplyCompleteReplyWithEOF pins data=PUSH_REPLY,auth-token SESS_ID_new\0 + err=io.EOF → error surfaced.

2. ACK MRU exceeds the four-ACK limit. Separated MRU capacity (8) from per-packet serialization: reliable control packets (incl. retransmits) carry ≤ CONTROL_SEND_ACK_MAX (4); dedicated ACKs carry ≤ RELIABLE_ACK_SIZE (8), and ≤ 4 when c.crypt == nil (mihomo's fixed IV_PROTO=6 doesn't advertise TLS key-material export, so the SoftEther-compat cap applies). TestAckSerializationCaps covers reliable/dedicated/retransmit × protected/plain.

3. Existing ACKs not moved to the front. takeAcksLocked now implements copy_acks_to_mru's move-to-front: each pending ID is unconditionally shifted to the front, existing entries right, duplicate dropped — so a re-acked ID is not evicted on a full MRU. TestMRUMoveToFrontMatchesReference pins your exact example (MRU [1..8], pending [8 9] → [8 9 1 2 3 4 5 6]).

Verification: go test -race ./transport/openvpn/ clean; live OpenVPN 2.6.14 reneg-sec 45 auth-gen-token 0 180 — 2 soft resets, 2× auth-token authentication succeeded, 101/101 probes, no TLS Error / link closed.

[wwqgtxx]

The original reneg-sec + 30 seconds failure chain remains fixed. However, the latest revision still has one ACK reliability issue, and the new EOF commit introduces a lower-impact read-boundary inconsistency.

1. capped ACK writes discard pending IDs that were not serialized

mihomo/transport/openvpn/control.go

Lines 197 to 226 in 28c2622

	func (c *ControlChannel) takeAcksLocked(max int) []uint32 {
	// Move ackPending (newest last) into the MRU front, preserving their
	// relative order, exactly like copy_acks_to_mru's backward loop.
	for i := len(c.ackPending) - 1; i >= 0; i-- {
	id := c.ackPending[i]
	move := id
	found := false
	for j := 0; j < len(c.lruAcks); j++ {
	tmp := c.lruAcks[j]
	c.lruAcks[j] = move
	move = tmp
	if move == id {
	found = true
	break
	}
	}
	if !found && len(c.lruAcks) < reliableAckSize {
	c.lruAcks = append(c.lruAcks, move)
	}
	}
	c.ackPending = nil
	// Cap the MRU at RELIABLE_ACK_SIZE (move-to-front never grows past it).
	if len(c.lruAcks) > reliableAckSize {
	c.lruAcks = c.lruAcks[:reliableAckSize]
	}
	n := len(c.lruAcks)
	if n > max {
	n = max
	}
	return append([]uint32(nil), c.lruAcks[:n]...)

takeAcksLocked(max) moves every entry in ackPending into the MRU and then clears the entire pending list. It only applies max when slicing the MRU for serialization.

For example:

pending ACKs: [1 2 3 4 5]
max:          4
serialized:   [1 2 3 4]
remaining:    []

OpenVPN's reliable_ack_write only consumes the pending ACKs actually copied into the packet and retains the remainder:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/reliable.c#L252-L304

The expected state after the first packet is therefore:

remaining: [5]

Without that retention, ACK 5 is not sent on the next packet unless the peer retransmits message 5 and causes it to be queued again. This creates an unnecessary retransmission and does not match the reference reliability behavior.

The new serialization-cap test queues five ACKs but only verifies that the emitted count is between one and the configured maximum:

mihomo/transport/openvpn/rekey_test.go

Lines 912 to 1052 in 28c2622

	func TestAckSerializationCaps(t *testing.T) {
	cases := []struct {
	name string
	crypt ControlCryptor
	path func(ctx context.Context, c *ControlChannel) error
	reads int
	wantMax int
	}{
	{
	name: "reliable-control-with-tls",
	crypt: mustClientCrypt(t),
	path: func(ctx context.Context, c *ControlChannel) error {
	c.QueueAck(1)
	c.QueueAck(2)
	c.QueueAck(3)
	c.QueueAck(4)
	c.QueueAck(5)
	_, err := c.Send(ctx, PControlV1, []byte("data"))
	return err
	},
	wantMax: 4,
	},
	{
	name: "reliable-control-plain",
	crypt: nil,
	path: func(ctx context.Context, c *ControlChannel) error {
	c.QueueAck(1)
	c.QueueAck(2)
	c.QueueAck(3)
	c.QueueAck(4)
	c.QueueAck(5)
	_, err := c.Send(ctx, PControlV1, []byte("data"))
	return err
	},
	wantMax: 4,
	},
	{
	name: "dedicated-ack-with-tls",
	crypt: mustClientCrypt(t),
	path: func(ctx context.Context, c *ControlChannel) error {
	for i := 0; i < 5; i++ {
	c.QueueAck(uint32(i))
	}
	return c.SendAck(ctx)
	},
	wantMax: 5,
	},
	{
	name: "dedicated-ack-plain",
	crypt: nil,
	path: func(ctx context.Context, c *ControlChannel) error {
	for i := 0; i < 5; i++ {
	c.QueueAck(uint32(i))
	}
	return c.SendAck(ctx)
	},
	wantMax: 4,
	},
	{
	name: "retransmit-with-tls",
	crypt: mustClientCrypt(t),
	path: func(ctx context.Context, c *ControlChannel) error {
	// Prime a pending reliable message, queue 5 acks, then
	// retransmit: the retransmitted reliable packet must carry
	// at most CONTROL_SEND_ACK_MAX acks.
	if _, err := c.Send(ctx, PControlV1, []byte("data")); err != nil {
	return err
	}
	for i := 0; i < 5; i++ {
	c.QueueAck(uint32(i))
	}
	return c.RetransmitPending(ctx)
	},
	reads: 2,
	wantMax: 4,
	},
	{
	name: "retransmit-plain",
	crypt: nil,
	path: func(ctx context.Context, c *ControlChannel) error {
	if _, err := c.Send(ctx, PControlV1, []byte("data")); err != nil {
	return err
	}
	for i := 0; i < 5; i++ {
	c.QueueAck(uint32(i))
	}
	return c.RetransmitPending(ctx)
	},
	reads: 2,
	wantMax: 4,
	},
	}
	for _, tc := range cases {
	t.Run(tc.name, func(t *testing.T) {
	clientIO, serverIO := newMemoryPacketPair()
	var clientID SessionID
	copy(clientID[:], []byte("client01"))
	var serverID SessionID
	copy(serverID[:], []byte("server01"))
	client := NewControlChannel(clientIO, tc.crypt, clientID)
	client.SetRemoteSessionID(serverID)
	client.clock = func() time.Time { return time.Unix(1714567890, 0) }
	// The peer decodes client->server with the server-direction crypt.
	var peerCrypt ControlCryptor
	if tc.crypt != nil {
	var err error
	peerCrypt, err = NewTLSCrypt(testStaticKey(), false)
	if err != nil {
	t.Fatal(err)
	}
	}
	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
	defer cancel()
	if err := tc.path(ctx, client); err != nil {
	t.Fatal(err)
	}
	reads := tc.reads
	if reads == 0 {
	reads = 1
	}
	var pkt *ControlPacket
	for i := 0; i < reads; i++ {
	raw, err := serverIO.ReadPacket(ctx)
	if err != nil {
	t.Fatal(err)
	}
	pkt, _, _, err = DecodeControlPacket(peerCrypt, raw)
	if err != nil {
	t.Fatalf("decode: %v", err)
	}
	}
	if len(pkt.AckIDs) > tc.wantMax {
	t.Fatalf("serialized %d acks, want <= %d: %v", len(pkt.AckIDs), tc.wantMax, pkt.AckIDs)
	}
	if len(pkt.AckIDs) == 0 {
	t.Fatal("expected at least one ack")
	}

It does not verify that the fifth ACK remains pending and appears in a subsequent packet. A deterministic package-level regression test for that state failed ten consecutive runs on the current head.

The minimal fix is to consume only the prefix that can be serialized:

n := min(len(c.ackPending), max)
// Move only c.ackPending[:n] into the MRU.
c.ackPending = c.ackPending[n:]

The test should assert the exact first packet and then verify that the next packet contains ACK 5.

2. complete token handling now depends on read-ahead boundaries

mihomo/transport/openvpn/client.go

Lines 746 to 790 in 28c2622

	func readTokenPushReply(conn pushReadConn, leftover []byte) (*PushReply, []byte, error) {
	buf := append([]byte(nil), leftover...)
	tmp := make([]byte, 4096)
	// Parse the already-buffered bytes first; a reply may be fully present.
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	for attempt := 0; attempt < 2; attempt++ {
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	// Process bytes before handling err: the io.Reader contract permits
	// n > 0 with err != nil (e.g. tls.Conn returns (n, io.EOF) when app
	// data is immediately followed by close_notify).
	if n > 0 {
	buf = append(buf, tmp[:n]...)
	}
	// AUTH_FAILED always takes precedence so its diagnostic is kept.
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	// Retain a complete reply but still classify err before returning:
	// a real timeout may carry the reply; EOF / other TLS errors must
	// be propagated, not hidden by a successful parse.
	var reply *PushReply
	var rest []byte
	if parsed, parsedRest, ok := takePushReply(buf); ok {
	reply, rest = parsed, parsedRest
	}
	if err != nil {
	_ = conn.SetReadDeadline(time.Time{})
	var netErr net.Error
	if errors.As(err, &netErr) && netErr.Timeout() {
	if reply != nil {
	return reply, rest, nil
	}
	return nil, buf, nil
	}
	return nil, buf, err
	}
	if reply != nil {
	return reply, rest, nil
	}

After reconsidering the io.Reader contract, the previously reported complete-reply-plus-EOF case should not have been treated as a defect. A reader may validly return n > 0 together with io.EOF; the returned bytes remain valid and must be processed before the terminal error.

The latest change now produces two different results for the same complete logical message:

complete token already in leftoverTLS       -> success
same token returned by Read with io.EOF     -> error

The fast path parses leftoverTLS and returns immediately without observing the stream state, while the read path parses the same message and then rejects it because EOF was returned. TLS is a byte stream, so success should not depend on whether an earlier read happened to read ahead into the token message. A deterministic comparison of these two paths failed ten consecutive runs.

AUTH_FAILED should continue to take precedence. For an ordinary complete PUSH_REPLY, the parsed reply should be accepted; EOF should be propagated only when no complete logical message was obtained.

The practical impact is lower because OpenVPN 2.6.20 frees an old OpenSSL key state with SSL_free and does not call SSL_shutdown, so the reference peer normally does not send close_notify at this point:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl_openssl.c#L1963-L1975

Nevertheless, the current behavior is read-boundary-dependent and can abort an otherwise complete rekey exchange with a peer that closes the inner TLS epoch after sending the token.

[Lanlan13-14]

The original reneg-sec + 30 seconds failure chain remains fixed. However, the latest revision still has one ACK reliability issue, and the new EOF commit introduces a lower-impact read-boundary inconsistency.

1. capped ACK writes discard pending IDs that were not serialized

mihomo/transport/openvpn/control.go

Lines 197 to 226 in 28c2622

	func (c *ControlChannel) takeAcksLocked(max int) []uint32 {
	// Move ackPending (newest last) into the MRU front, preserving their
	// relative order, exactly like copy_acks_to_mru's backward loop.
	for i := len(c.ackPending) - 1; i >= 0; i-- {
	id := c.ackPending[i]
	move := id
	found := false
	for j := 0; j < len(c.lruAcks); j++ {
	tmp := c.lruAcks[j]
	c.lruAcks[j] = move
	move = tmp
	if move == id {
	found = true
	break
	}
	}
	if !found && len(c.lruAcks) < reliableAckSize {
	c.lruAcks = append(c.lruAcks, move)
	}
	}
	c.ackPending = nil
	// Cap the MRU at RELIABLE_ACK_SIZE (move-to-front never grows past it).
	if len(c.lruAcks) > reliableAckSize {
	c.lruAcks = c.lruAcks[:reliableAckSize]
	}
	n := len(c.lruAcks)
	if n > max {
	n = max
	}
	return append([]uint32(nil), c.lruAcks[:n]...)

takeAcksLocked(max) moves every entry in ackPending into the MRU and then clears the entire pending list. It only applies max when slicing the MRU for serialization.

For example:

pending ACKs: [1 2 3 4 5]
max:          4
serialized:   [1 2 3 4]
remaining:    []

OpenVPN's reliable_ack_write only consumes the pending ACKs actually copied into the packet and retains the remainder:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/reliable.c#L252-L304

The expected state after the first packet is therefore:

remaining: [5]

Without that retention, ACK 5 is not sent on the next packet unless the peer retransmits message 5 and causes it to be queued again. This creates an unnecessary retransmission and does not match the reference reliability behavior.

The new serialization-cap test queues five ACKs but only verifies that the emitted count is between one and the configured maximum:

mihomo/transport/openvpn/rekey_test.go

Lines 912 to 1052 in 28c2622

	func TestAckSerializationCaps(t *testing.T) {
	cases := []struct {
	name string
	crypt ControlCryptor
	path func(ctx context.Context, c *ControlChannel) error
	reads int
	wantMax int
	}{
	{
	name: "reliable-control-with-tls",
	crypt: mustClientCrypt(t),
	path: func(ctx context.Context, c *ControlChannel) error {
	c.QueueAck(1)
	c.QueueAck(2)
	c.QueueAck(3)
	c.QueueAck(4)
	c.QueueAck(5)
	_, err := c.Send(ctx, PControlV1, []byte("data"))
	return err
	},
	wantMax: 4,
	},
	{
	name: "reliable-control-plain",
	crypt: nil,
	path: func(ctx context.Context, c *ControlChannel) error {
	c.QueueAck(1)
	c.QueueAck(2)
	c.QueueAck(3)
	c.QueueAck(4)
	c.QueueAck(5)
	_, err := c.Send(ctx, PControlV1, []byte("data"))
	return err
	},
	wantMax: 4,
	},
	{
	name: "dedicated-ack-with-tls",
	crypt: mustClientCrypt(t),
	path: func(ctx context.Context, c *ControlChannel) error {
	for i := 0; i < 5; i++ {
	c.QueueAck(uint32(i))
	}
	return c.SendAck(ctx)
	},
	wantMax: 5,
	},
	{
	name: "dedicated-ack-plain",
	crypt: nil,
	path: func(ctx context.Context, c *ControlChannel) error {
	for i := 0; i < 5; i++ {
	c.QueueAck(uint32(i))
	}
	return c.SendAck(ctx)
	},
	wantMax: 4,
	},
	{
	name: "retransmit-with-tls",
	crypt: mustClientCrypt(t),
	path: func(ctx context.Context, c *ControlChannel) error {
	// Prime a pending reliable message, queue 5 acks, then
	// retransmit: the retransmitted reliable packet must carry
	// at most CONTROL_SEND_ACK_MAX acks.
	if _, err := c.Send(ctx, PControlV1, []byte("data")); err != nil {
	return err
	}
	for i := 0; i < 5; i++ {
	c.QueueAck(uint32(i))
	}
	return c.RetransmitPending(ctx)
	},
	reads: 2,
	wantMax: 4,
	},
	{
	name: "retransmit-plain",
	crypt: nil,
	path: func(ctx context.Context, c *ControlChannel) error {
	if _, err := c.Send(ctx, PControlV1, []byte("data")); err != nil {
	return err
	}
	for i := 0; i < 5; i++ {
	c.QueueAck(uint32(i))
	}
	return c.RetransmitPending(ctx)
	},
	reads: 2,
	wantMax: 4,
	},
	}
	for _, tc := range cases {
	t.Run(tc.name, func(t *testing.T) {
	clientIO, serverIO := newMemoryPacketPair()
	var clientID SessionID
	copy(clientID[:], []byte("client01"))
	var serverID SessionID
	copy(serverID[:], []byte("server01"))
	client := NewControlChannel(clientIO, tc.crypt, clientID)
	client.SetRemoteSessionID(serverID)
	client.clock = func() time.Time { return time.Unix(1714567890, 0) }
	// The peer decodes client->server with the server-direction crypt.
	var peerCrypt ControlCryptor
	if tc.crypt != nil {
	var err error
	peerCrypt, err = NewTLSCrypt(testStaticKey(), false)
	if err != nil {
	t.Fatal(err)
	}
	}
	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
	defer cancel()
	if err := tc.path(ctx, client); err != nil {
	t.Fatal(err)
	}
	reads := tc.reads
	if reads == 0 {
	reads = 1
	}
	var pkt *ControlPacket
	for i := 0; i < reads; i++ {
	raw, err := serverIO.ReadPacket(ctx)
	if err != nil {
	t.Fatal(err)
	}
	pkt, _, _, err = DecodeControlPacket(peerCrypt, raw)
	if err != nil {
	t.Fatalf("decode: %v", err)
	}
	}
	if len(pkt.AckIDs) > tc.wantMax {
	t.Fatalf("serialized %d acks, want <= %d: %v", len(pkt.AckIDs), tc.wantMax, pkt.AckIDs)
	}
	if len(pkt.AckIDs) == 0 {
	t.Fatal("expected at least one ack")
	}

It does not verify that the fifth ACK remains pending and appears in a subsequent packet. A deterministic package-level regression test for that state failed ten consecutive runs on the current head.

The minimal fix is to consume only the prefix that can be serialized:

n := min(len(c.ackPending), max)
// Move only c.ackPending[:n] into the MRU.
c.ackPending = c.ackPending[n:]

The test should assert the exact first packet and then verify that the next packet contains ACK 5.

2. complete token handling now depends on read-ahead boundaries

mihomo/transport/openvpn/client.go

Lines 746 to 790 in 28c2622

	func readTokenPushReply(conn pushReadConn, leftover []byte) (*PushReply, []byte, error) {
	buf := append([]byte(nil), leftover...)
	tmp := make([]byte, 4096)
	// Parse the already-buffered bytes first; a reply may be fully present.
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	for attempt := 0; attempt < 2; attempt++ {
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	// Process bytes before handling err: the io.Reader contract permits
	// n > 0 with err != nil (e.g. tls.Conn returns (n, io.EOF) when app
	// data is immediately followed by close_notify).
	if n > 0 {
	buf = append(buf, tmp[:n]...)
	}
	// AUTH_FAILED always takes precedence so its diagnostic is kept.
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	// Retain a complete reply but still classify err before returning:
	// a real timeout may carry the reply; EOF / other TLS errors must
	// be propagated, not hidden by a successful parse.
	var reply *PushReply
	var rest []byte
	if parsed, parsedRest, ok := takePushReply(buf); ok {
	reply, rest = parsed, parsedRest
	}
	if err != nil {
	_ = conn.SetReadDeadline(time.Time{})
	var netErr net.Error
	if errors.As(err, &netErr) && netErr.Timeout() {
	if reply != nil {
	return reply, rest, nil
	}
	return nil, buf, nil
	}
	return nil, buf, err
	}
	if reply != nil {
	return reply, rest, nil
	}

After reconsidering the io.Reader contract, the previously reported complete-reply-plus-EOF case should not have been treated as a defect. A reader may validly return n > 0 together with io.EOF; the returned bytes remain valid and must be processed before the terminal error.

The latest change now produces two different results for the same complete logical message:

complete token already in leftoverTLS       -> success
same token returned by Read with io.EOF     -> error

The fast path parses leftoverTLS and returns immediately without observing the stream state, while the read path parses the same message and then rejects it because EOF was returned. TLS is a byte stream, so success should not depend on whether an earlier read happened to read ahead into the token message. A deterministic comparison of these two paths failed ten consecutive runs.

AUTH_FAILED should continue to take precedence. For an ordinary complete PUSH_REPLY, the parsed reply should be accepted; EOF should be propagated only when no complete logical message was obtained.

The practical impact is lower because OpenVPN 2.6.20 frees an old OpenSSL key state with SSL_free and does not call SSL_shutdown, so the reference peer normally does not send close_notify at this point:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl_openssl.c#L1963-L1975

Nevertheless, the current behavior is read-boundary-dependent and can abort an otherwise complete rekey exchange with a peer that closes the inner TLS epoch after sending the token.

I went through the OpenVPN 2.6.20 reference client (ssl.c, reliable.c, ssl_pkt.c, auth_token.c, push.c) against the current head (7db3d19) and changed the control channel to match those functions, not a private interpretation of the protocol.

What was copied from the reference

Key id. ssl.c:994-1002 increments session->key_id, masks with P_KEY_ID_MASK, and wraps 0 back to 1. NextKeyID does the same. A parked soft reset is accepted only when packet.KeyID == NextKeyID(curKey), matching the ks->key_id != key_id error in ssl.c:3974-3980.

ACK MRU. takeAcksLocked is copy_acks_to_mru (reliable.c:211-249) plus reliable_ack_write (reliable.c:252-304):

• pending IDs are walked backward and unconditionally moved to the front of the MRU
• only n = min(ackPending.len, max) IDs are consumed; ackPending[n:] stays pending
• the packet is filled from the MRU, up to max

TestMRUMoveToFrontMatchesReference pins the reference example: MRU [1..8], pending [8 9] → [8 9 1 2 3 4 5 6]. TestAckCapRetainsUnsentPending pins the consume-prefix case: pending [1 2 3 4 5], max 4 → first packet [1 2 3 4], remainder [5] on the next packet.

Per-packet ACK caps. From ssl.h:55 (CONTROL_SEND_ACK_MAX = 4) and ssl_pkt.c:178-185 (SoftEther: unprotected client dedicated ACKs capped at 4):

• Send / RetransmitPending → 4
• SendAck → 8, or 4 when c.crypt == nil (this client does not advertise TLS key-material export, so the TLS_WRAP_NONE branch applies)

Dedicated ACK trigger. ssl.c:3152 sends a dedicated ACK only when !reliable_ack_empty(rec_ack). SendAck returns if ackPending is empty. The MRU alone does not emit a packet.

Token / AUTH_FAILED. auth_token.c:460-481 / push.c:755 push a token-only PUSH_REPLY after renegotiation (send_push_reply_auth_token). consumeRekeyPush plus parkedTLS consume that message. AUTH_FAILED aborts the rekey before a new data channel is installed.

Complete reply + EOF. tls.Conn may return (n, io.EOF). The bytes are valid (io.Reader contract). A complete PUSH_REPLY is accepted; EOF is propagated only when no complete message was obtained. AUTH_FAILED still takes precedence. This matches leftover-TLS and Read producing the same result for the same logical message.

Immediate dedicated ACK. read() sends a dedicated P_ACK_V1 as soon as a reliable packet is received. That is the same branch the reference takes when there is no outgoing packet: ssl.c:3152-3176 (!to_link->len && !reliable_ack_empty(rec_ack) → write_control_auth(..., P_ACK_V1, ...) logged as "Dedicated ACK -> TCP/UDP"). This client is a synchronous reader and has no independent outbound scheduler, so that is the branch it always takes. It is not a private ACK policy.

What was left as-is, and why

These are not specified by those functions:

• Retransmit interval is 1s here vs packet_timeout (default 60s, exponential backoff) in the reference. The protocol does not mandate the interval; the peer accepts duplicates.
• No N_ACK_RETRANSMIT fast path (reliable.h:53). That is a reference performance optimization.

Verification

• go test -race ./transport/openvpn/ clean
• Live: OpenVPN 2.6.14, reneg-sec 45, auth-gen-token 0 180 — 2 soft resets, 2× Username/auth-token authentication succeeded, 101/101 HTTP probes, no TLS Error / link closed

[wwqgtxx]

I re-reviewed the current head from the base commit, including the control-channel state machine, ACK handling, TLS/key-method framing, authentication-token flow, and data-channel epoch selection.

The original reneg-sec + 30 seconds failure chain from #3085 is fixed: the code now creates a fresh TLS connection for each epoch, adopts the server key ID, labels data packets with that key ID, clears the rekey deadline, and carries authentication-token state. However, I found two remaining issues introduced by this PR that should be addressed before merge.

1. Late AUTH_FAILED can remain hidden indefinitely

readTokenPushReply waits for at most two 300 ms reads. If no complete reply is available, the rekey proceeds and installs the new data channel:

mihomo/transport/openvpn/client.go

Lines 737 to 786 in 7db3d19

	// readTokenPushReply tries to consume a token-only PUSH_REPLY (an
	// auth-token renewal pushed by send_push_reply_auth_token) from the TLS
	// stream, without stalling a rekey. leftover holds bytes already read past
	// the server key-method-2 record.
	//
	// TLS is a byte stream: the reply may be split across reads, so the buffer
	// is parsed after every read including the final one. On timeout the
	// buffered bytes are preserved and a nil reply (no error) is returned, so a
	// partially-received reply is not lost. AUTH_FAILED is a hard error.
	func readTokenPushReply(conn pushReadConn, leftover []byte) (*PushReply, []byte, error) {
	buf := append([]byte(nil), leftover...)
	tmp := make([]byte, 4096)
	// Parse the already-buffered bytes first; a reply may be fully present.
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	for attempt := 0; attempt < 2; attempt++ {
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	// Process bytes before handling err: the io.Reader contract permits
	// n > 0 with err != nil (e.g. tls.Conn returns (n, io.EOF) when app
	// data is immediately followed by close_notify). The bytes are valid
	// and must be processed before the terminal error.
	if n > 0 {
	buf = append(buf, tmp[:n]...)
	}
	// AUTH_FAILED always takes precedence so its diagnostic is kept.
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	// A complete reply is accepted regardless of a bundled err: the
	// logical message is present, so success must not depend on whether
	// an earlier read happened to read ahead into it (TLS is a byte
	// stream). EOF is propagated only when no complete message exists.
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if err != nil {
	_ = conn.SetReadDeadline(time.Time{})
	var netErr net.Error
	if errors.As(err, &netErr) && netErr.Timeout() {
	return nil, buf, nil
	}
	return nil, buf, err
	}
	}
	return nil, buf, nil

A later same-epoch P_CONTROL_V1 is parked by waitForSoftReset, but that function continues waiting instead of notifying the client that TLS input is available:

mihomo/transport/openvpn/control.go

Lines 283 to 312 in 7db3d19

	func (c *ControlChannel) waitForSoftReset(ctx context.Context) (*ControlPacket, error) {
	c.mu.Lock()
	if c.pendingSoftReset != nil {
	packet := c.pendingSoftReset
	c.pendingSoftReset = nil
	c.mu.Unlock()
	return packet, nil
	}
	c.mu.Unlock()
	for {
	packet, err := c.read(ctx, true)
	if err != nil {
	return nil, err
	}
	if packet.Opcode == PControlSoftResetV1 {
	return packet, nil
	}
	// Same-epoch P_CONTROL_V1 after a rekey is typically a token-only
	// PUSH_REPLY (send_push_reply_auth_token). Park the TLS payload for
	// ReadAll instead of ACK-and-dropping it. read() already ACKed.
	if packet.Opcode == PControlV1 && len(packet.Payload) > 0 {
	c.mu.Lock()
	c.parkedTLS = append(c.parkedTLS, append([]byte(nil), packet.Payload...))
	c.mu.Unlock()
	continue
	}
	if err := c.SendAck(ctx); err != nil {
	return nil, err
	}
	}

The parked TLS data is not fed into the active tls.Conn until a subsequent soft reset has already arrived:

mihomo/transport/openvpn/client.go

Lines 468 to 487 in 7db3d19

	func (c *Client) watchControl() {
	for {
	packet, err := c.control.waitForSoftReset(c.runCtx)
	if err != nil {
	c.failControl(fmt.Errorf("wait for soft reset: %w", err))
	return
	}
	// Token-only PUSH_REPLY parked since the last rekey must land in
	// authPass before this key-method-2 exchange, otherwise the server
	// rejects the expired token. A parked AUTH_FAILED (deferred auth) is
	// a hard failure: surface it before starting the next epoch instead
	// of replacing it with the next renegotiation result.
	c.consumeQueuedControl()
	if c.push != nil {
	if err := c.consumeRekeyPush(); err != nil {
	c.failControl(fmt.Errorf("consume queued rekey push: %w", err))
	return
	}
	}
	if err := c.renegotiate(packet); err != nil {

This matters because OpenVPN explicitly permits the post-renegotiation token reply to be delayed by a few seconds:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/auth_token.c#L464-L481

If deferred authentication later fails, the server can send AUTH_FAILED after mihomo's 600 ms probe has ended. On UDP there is no transport EOF, and an authentication failure normally prevents another soft reset. The result is:

rekey treated as successful
new data channel installed
late AUTH_FAILED acknowledged and parked
no subsequent soft reset
AUTH_FAILED never parsed and LastRekeyError remains empty

A deterministic in-memory test confirmed that a same-epoch control payload remains parked until either a soft reset arrives or the wait context expires. The existing TestWatchControlSurfacesParkedAUTHFailed does not cover this sequence because it pre-populates a soft reset before starting the watcher.

The established-state watcher needs to process available TLS control input immediately, independently of the next soft reset.

2. The retiring data key never expires

installDataChannel retains the previous epoch and only removes it when another data channel is installed:

mihomo/transport/openvpn/client.go

Lines 334 to 352 in 7db3d19

	func (c *Client) installDataChannel(newData *DataChannel) {
	c.dataLock.Lock()
	old := c.data
	c.retiring = old
	c.data = newData
	if c.dataByKey == nil {
	c.dataByKey = make(map[uint8]*DataChannel)
	}
	if old != nil && old.keyID != newData.keyID {
	c.dataByKey[old.keyID] = old
	}
	c.dataByKey[newData.keyID] = newData
	// Keep at most the current and previous epoch.
	for id := range c.dataByKey {
	if id != newData.keyID && (old == nil || id != old.keyID) {
	delete(c.dataByKey, id)
	}
	}
	c.dataLock.Unlock()

Incoming packets continue selecting that key without any time check:

mihomo/transport/openvpn/client.go

Lines 444 to 460 in 7db3d19

	func (c *Client) decryptDataPacket(packet []byte) ([]byte, error) {
	if len(packet) == 0 {
	return nil, errors.New("empty openvpn data packet")
	}
	_, keyID := parseOpcodeKeyID(packet[0])
	c.dataLock.RLock()
	data := c.data
	if alt, ok := c.dataByKey[keyID]; ok {
	data = alt
	} else if c.retiring != nil && c.retiring.keyID == keyID {
	data = c.retiring
	}
	c.dataLock.RUnlock()
	if data == nil {
	return nil, errors.New("openvpn data channel is not ready")
	}
	return data.Decrypt(packet)

OpenVPN assigns every lame-duck key an expiration of now + transition_window and frees it when that deadline expires:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.c#L1927-L1938

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.c#L3116-L3121

The default transition window is 3600 seconds. In the current PR, if the next rekey is delayed or disabled, the previous receive key remains usable indefinitely. This extends acceptance of packets authenticated with a retired key beyond the reference lifetime.

Existing issues retained or amplified by this PR

These are not clean regressions from the base commit, so they should not be attributed entirely to this PR. They are still relevant to the new epoch implementation.

Unknown data key IDs fall back to the current key

The new dataByKey lookup leaves data initialized to c.data when the key ID is unknown. The base implementation also always tried the current data channel, so the underlying behavior is pre-existing, but the new key-indexed routing does not actually reject unknown epochs.

For AEAD this normally fails authentication. For CBC, the HMAC excludes the outer opcode/key-ID header. A focused in-memory test changed a valid CBC packet's key ID from 1 to unknown ID 7; decryptDataPacket still accepted and decrypted it in 100/100 runs. A map miss should return an unknown-key error.

Empty retransmission consumes pending ACKs

RetransmitPending moves ACKs into the MRU before checking whether there are packets to retransmit:

mihomo/transport/openvpn/control.go

Lines 488 to 509 in 7db3d19

	func (c *ControlChannel) RetransmitPending(ctx context.Context) error {
	c.mu.Lock()
	packets := make([]*ControlPacket, 0, len(c.pending))
	// Pull current acks into the MRU once, so every retransmitted packet
	// carries the same ack set (matching OpenVPN: retransmitted reliable
	// packets reuse the original ack header, and the MRU keeps recently
	// acked IDs alive across sends).
	ackIDs := c.takeAcksLocked(controlSendAckMax)
	for _, packet := range c.pending {
	cp := *packet
	cp.AckIDs = ackIDs
	cp.AckRemoteSession = c.remote
	packets = append(packets, &cp)
	}
	c.mu.Unlock()
	for _, packet := range packets {
	if err := c.writeControlPacket(ctx, packet); err != nil {
	return err
	}
	}
	return nil

The base implementation already cleared ackPending in this case. This PR does not originate the defect, but its new periodic UDP rekey retransmission goroutine makes the race with the receive/ACK path more reachable. Returning early when len(c.pending) == 0 avoids consuming an ACK without emitting a packet.

Pre-existing non-blocking observations

• Soft resets were already accepted without requiring new-epoch reliable message ID 0. The PR's new MarkReceived(serverReset.MessageID) makes the consequence more visible because an invalid ID can now advance the receive sequence.
• The initial PUSH parser already attempted to accept an unterminated reply once ifconfig was present. The new takePushReply preserves that behavior; a TLS split after ifconfig can commit the reply before later route, DNS, cipher, or token options arrive.

[Lanlan13-14]

I re-reviewed the current head from the base commit, including the control-channel state machine, ACK handling, TLS/key-method framing, authentication-token flow, and data-channel epoch selection.

The original reneg-sec + 30 seconds failure chain from #3085 is fixed: the code now creates a fresh TLS connection for each epoch, adopts the server key ID, labels data packets with that key ID, clears the rekey deadline, and carries authentication-token state. However, I found two remaining issues introduced by this PR that should be addressed before merge.

1. Late AUTH_FAILED can remain hidden indefinitely

readTokenPushReply waits for at most two 300 ms reads. If no complete reply is available, the rekey proceeds and installs the new data channel:

mihomo/transport/openvpn/client.go

Lines 737 to 786 in 7db3d19

	// readTokenPushReply tries to consume a token-only PUSH_REPLY (an
	// auth-token renewal pushed by send_push_reply_auth_token) from the TLS
	// stream, without stalling a rekey. leftover holds bytes already read past
	// the server key-method-2 record.
	//
	// TLS is a byte stream: the reply may be split across reads, so the buffer
	// is parsed after every read including the final one. On timeout the
	// buffered bytes are preserved and a nil reply (no error) is returned, so a
	// partially-received reply is not lost. AUTH_FAILED is a hard error.
	func readTokenPushReply(conn pushReadConn, leftover []byte) (*PushReply, []byte, error) {
	buf := append([]byte(nil), leftover...)
	tmp := make([]byte, 4096)
	// Parse the already-buffered bytes first; a reply may be fully present.
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	for attempt := 0; attempt < 2; attempt++ {
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	// Process bytes before handling err: the io.Reader contract permits
	// n > 0 with err != nil (e.g. tls.Conn returns (n, io.EOF) when app
	// data is immediately followed by close_notify). The bytes are valid
	// and must be processed before the terminal error.
	if n > 0 {
	buf = append(buf, tmp[:n]...)
	}
	// AUTH_FAILED always takes precedence so its diagnostic is kept.
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	// A complete reply is accepted regardless of a bundled err: the
	// logical message is present, so success must not depend on whether
	// an earlier read happened to read ahead into it (TLS is a byte
	// stream). EOF is propagated only when no complete message exists.
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if err != nil {
	_ = conn.SetReadDeadline(time.Time{})
	var netErr net.Error
	if errors.As(err, &netErr) && netErr.Timeout() {
	return nil, buf, nil
	}
	return nil, buf, err
	}
	}
	return nil, buf, nil

A later same-epoch P_CONTROL_V1 is parked by waitForSoftReset, but that function continues waiting instead of notifying the client that TLS input is available:

mihomo/transport/openvpn/control.go

Lines 283 to 312 in 7db3d19

	func (c *ControlChannel) waitForSoftReset(ctx context.Context) (*ControlPacket, error) {
	c.mu.Lock()
	if c.pendingSoftReset != nil {
	packet := c.pendingSoftReset
	c.pendingSoftReset = nil
	c.mu.Unlock()
	return packet, nil
	}
	c.mu.Unlock()
	for {
	packet, err := c.read(ctx, true)
	if err != nil {
	return nil, err
	}
	if packet.Opcode == PControlSoftResetV1 {
	return packet, nil
	}
	// Same-epoch P_CONTROL_V1 after a rekey is typically a token-only
	// PUSH_REPLY (send_push_reply_auth_token). Park the TLS payload for
	// ReadAll instead of ACK-and-dropping it. read() already ACKed.
	if packet.Opcode == PControlV1 && len(packet.Payload) > 0 {
	c.mu.Lock()
	c.parkedTLS = append(c.parkedTLS, append([]byte(nil), packet.Payload...))
	c.mu.Unlock()
	continue
	}
	if err := c.SendAck(ctx); err != nil {
	return nil, err
	}
	}

The parked TLS data is not fed into the active tls.Conn until a subsequent soft reset has already arrived:

mihomo/transport/openvpn/client.go

Lines 468 to 487 in 7db3d19

	func (c *Client) watchControl() {
	for {
	packet, err := c.control.waitForSoftReset(c.runCtx)
	if err != nil {
	c.failControl(fmt.Errorf("wait for soft reset: %w", err))
	return
	}
	// Token-only PUSH_REPLY parked since the last rekey must land in
	// authPass before this key-method-2 exchange, otherwise the server
	// rejects the expired token. A parked AUTH_FAILED (deferred auth) is
	// a hard failure: surface it before starting the next epoch instead
	// of replacing it with the next renegotiation result.
	c.consumeQueuedControl()
	if c.push != nil {
	if err := c.consumeRekeyPush(); err != nil {
	c.failControl(fmt.Errorf("consume queued rekey push: %w", err))
	return
	}
	}
	if err := c.renegotiate(packet); err != nil {

This matters because OpenVPN explicitly permits the post-renegotiation token reply to be delayed by a few seconds:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/auth_token.c#L464-L481

If deferred authentication later fails, the server can send AUTH_FAILED after mihomo's 600 ms probe has ended. On UDP there is no transport EOF, and an authentication failure normally prevents another soft reset. The result is:

rekey treated as successful
new data channel installed
late AUTH_FAILED acknowledged and parked
no subsequent soft reset
AUTH_FAILED never parsed and LastRekeyError remains empty

A deterministic in-memory test confirmed that a same-epoch control payload remains parked until either a soft reset arrives or the wait context expires. The existing TestWatchControlSurfacesParkedAUTHFailed does not cover this sequence because it pre-populates a soft reset before starting the watcher.

The established-state watcher needs to process available TLS control input immediately, independently of the next soft reset.

2. The retiring data key never expires

installDataChannel retains the previous epoch and only removes it when another data channel is installed:

mihomo/transport/openvpn/client.go

Lines 334 to 352 in 7db3d19

	func (c *Client) installDataChannel(newData *DataChannel) {
	c.dataLock.Lock()
	old := c.data
	c.retiring = old
	c.data = newData
	if c.dataByKey == nil {
	c.dataByKey = make(map[uint8]*DataChannel)
	}
	if old != nil && old.keyID != newData.keyID {
	c.dataByKey[old.keyID] = old
	}
	c.dataByKey[newData.keyID] = newData
	// Keep at most the current and previous epoch.
	for id := range c.dataByKey {
	if id != newData.keyID && (old == nil || id != old.keyID) {
	delete(c.dataByKey, id)
	}
	}
	c.dataLock.Unlock()

Incoming packets continue selecting that key without any time check:

mihomo/transport/openvpn/client.go

Lines 444 to 460 in 7db3d19

	func (c *Client) decryptDataPacket(packet []byte) ([]byte, error) {
	if len(packet) == 0 {
	return nil, errors.New("empty openvpn data packet")
	}
	_, keyID := parseOpcodeKeyID(packet[0])
	c.dataLock.RLock()
	data := c.data
	if alt, ok := c.dataByKey[keyID]; ok {
	data = alt
	} else if c.retiring != nil && c.retiring.keyID == keyID {
	data = c.retiring
	}
	c.dataLock.RUnlock()
	if data == nil {
	return nil, errors.New("openvpn data channel is not ready")
	}
	return data.Decrypt(packet)

OpenVPN assigns every lame-duck key an expiration of now + transition_window and frees it when that deadline expires:

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.c#L1927-L1938

https://github.com/OpenVPN/openvpn/blob/v2.6.20/src/openvpn/ssl.c#L3116-L3121

The default transition window is 3600 seconds. In the current PR, if the next rekey is delayed or disabled, the previous receive key remains usable indefinitely. This extends acceptance of packets authenticated with a retired key beyond the reference lifetime.

Existing issues retained or amplified by this PR

These are not clean regressions from the base commit, so they should not be attributed entirely to this PR. They are still relevant to the new epoch implementation.

Unknown data key IDs fall back to the current key

The new dataByKey lookup leaves data initialized to c.data when the key ID is unknown. The base implementation also always tried the current data channel, so the underlying behavior is pre-existing, but the new key-indexed routing does not actually reject unknown epochs.

For AEAD this normally fails authentication. For CBC, the HMAC excludes the outer opcode/key-ID header. A focused in-memory test changed a valid CBC packet's key ID from 1 to unknown ID 7; decryptDataPacket still accepted and decrypted it in 100/100 runs. A map miss should return an unknown-key error.

Empty retransmission consumes pending ACKs

RetransmitPending moves ACKs into the MRU before checking whether there are packets to retransmit:

mihomo/transport/openvpn/control.go

Lines 488 to 509 in 7db3d19

	func (c *ControlChannel) RetransmitPending(ctx context.Context) error {
	c.mu.Lock()
	packets := make([]*ControlPacket, 0, len(c.pending))
	// Pull current acks into the MRU once, so every retransmitted packet
	// carries the same ack set (matching OpenVPN: retransmitted reliable
	// packets reuse the original ack header, and the MRU keeps recently
	// acked IDs alive across sends).
	ackIDs := c.takeAcksLocked(controlSendAckMax)
	for _, packet := range c.pending {
	cp := *packet
	cp.AckIDs = ackIDs
	cp.AckRemoteSession = c.remote
	packets = append(packets, &cp)
	}
	c.mu.Unlock()
	for _, packet := range packets {
	if err := c.writeControlPacket(ctx, packet); err != nil {
	return err
	}
	}
	return nil

The base implementation already cleared ackPending in this case. This PR does not originate the defect, but its new periodic UDP rekey retransmission goroutine makes the race with the receive/ACK path more reachable. Returning early when len(c.pending) == 0 avoids consuming an ACK without emitting a packet.

Pre-existing non-blocking observations

• Soft resets were already accepted without requiring new-epoch reliable message ID 0. The PR's new MarkReceived(serverReset.MessageID) makes the consequence more visible because an invalid ID can now advance the receive sequence.
• The initial PUSH parser already attempted to accept an unterminated reply once ifconfig was present. The new takePushReply preserves that behavior; a TLS split after ifconfig can commit the reply before later route, DNS, cipher, or token options arrive.

All six points in your review are addressed — the four merge-blockers in a209399, and the two pre-existing observations in 4cbff69.

Merge-blockers (a209399)

1. Late AUTH_FAILED can remain hidden indefinitely. waitForSoftReset now returns errParkedTLS as soon as it parks a same-epoch P_CONTROL_V1 (token update / deferred AUTH_FAILED), and watchControl consumes it immediately — it no longer waits for the next soft reset, which may never come after an auth failure. TestWaitForSoftResetParksLateControlPayload now expects the parked payload to be surfaced before the soft reset.

2. The retiring data key never expires. installDataChannel records the previous epoch's expiry at now + transitionWindow (3600s, matching OpenVPN's default lame-duck must_die in ssl.c:1927-1938). decryptDataPacket rejects packets labeled with an expired retiring epoch. Tests: TestDecryptRejectsExpiredRetiringKey, TestDecryptAcceptsRetiringKeyBeforeExpiry.

3. Unknown data key IDs fall back to the current key. decryptDataPacket now routes strictly by key ID: a map miss (and not the retiring epoch) returns an unknown-key error instead of falling back to c.data. This closes the CBC case where the HMAC excludes the outer opcode/key-ID header. Test: TestDecryptRejectsUnknownKeyID.

4. Empty retransmission consumes pending ACKs. RetransmitPending returns early when len(pending) == 0, so an empty retransmit no longer moves queued ACKs into the MRU without emitting a packet. Test: TestRetransmitPendingEmptyDoesNotConsumeAcks.

Pre-existing observations (4cbff69)

5. Soft resets accepted without new-epoch message ID 0. A new-epoch soft reset is always message 0 — the fresh reliable layer starts at 0. classifyWatchPacketLocked now rejects a next-epoch reset whose MessageID != 0, so an invalid ID can no longer advance the receive sequence through MarkReceived. TestClassifyWatchAcceptsNextEpochSoftReset now asserts a reset with message id 5 is rejected.

6. PUSH parser commits an unterminated reply after ifconfig. takePushReply now requires the NUL terminator before accepting a PUSH_REPLY. A reply fragmented across TLS reads is parsed only once complete, so later route / DNS / cipher / token options are not lost. This matches the reference client, which parses the full PUSH message after it is fully received. Test: TestTakePushReplyRequiresTerminator.

Verification

• go test -race ./transport/openvpn/ clean
• Live: OpenVPN 2.6.14, reneg-sec 45, auth-gen-token 0 180 — 2 soft resets, 2× Username/auth-token authentication succeeded, 101/101 HTTP probes, no TLS Error / link closed / AUTH_FAILED

[wwqgtxx]

I found three remaining regressions in the new rekey implementation and one incomplete validation fix. I do not think the PR is ready to merge yet.

1. A successful late token read leaves a 300 ms read deadline active

The new errParkedTLS path immediately feeds a delayed same-epoch TLS payload into the active TLS connection and calls consumeRekeyPush:

mihomo/transport/openvpn/client.go

Lines 491 to 507 in 4cbff69

	func (c *Client) watchControl() {
	for {
	packet, err := c.control.waitForSoftReset(c.runCtx)
	if err != nil {
	if errors.Is(err, errParkedTLS) {
	// A same-epoch TLS payload (token update / late AUTH_FAILED)
	// was parked. Consume it now so a deferred authentication
	// failure is surfaced immediately instead of waiting for the
	// next soft reset (which may never come).
	c.consumeQueuedControl()
	if c.push != nil {
	if err := c.consumeRekeyPush(); err != nil {
	c.failControl(fmt.Errorf("consume parked rekey push: %w", err))
	return
	}
	}
	continue

readTokenPushReply sets a 300 ms read deadline before reading, but a successfully parsed reply returns before that deadline is cleared:

mihomo/transport/openvpn/client.go

Lines 793 to 823 in 4cbff69

	for attempt := 0; attempt < 2; attempt++ {
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	// Process bytes before handling err: the io.Reader contract permits
	// n > 0 with err != nil (e.g. tls.Conn returns (n, io.EOF) when app
	// data is immediately followed by close_notify). The bytes are valid
	// and must be processed before the terminal error.
	if n > 0 {
	buf = append(buf, tmp[:n]...)
	}
	// AUTH_FAILED always takes precedence so its diagnostic is kept.
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	// A complete reply is accepted regardless of a bundled err: the
	// logical message is present, so success must not depend on whether
	// an earlier read happened to read ahead into it (TLS is a byte
	// stream). EOF is propagated only when no complete message exists.
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if err != nil {
	_ = conn.SetReadDeadline(time.Time{})
	var netErr net.Error
	if errors.As(err, &netErr) && netErr.Timeout() {
	return nil, buf, nil
	}
	return nil, buf, err
	}
	}
	return nil, buf, nil

This did not normally break the in-renegotiation call because renegotiate has a deferred deadline reset. The new immediate parked-TLS path runs outside that cleanup scope. After applying the token, the watcher continues to waitForSoftReset, whose next raw read reapplies the stale absolute deadline:

mihomo/transport/openvpn/control.go

Lines 550 to 561 in 4cbff69

	return c.io.WritePacket(ctx, encoded)
	}
	func (c *ControlChannel) readRawControlPacket(ctx context.Context) ([]byte, error) {
	c.mu.Lock()
	deadline := c.readDeadline
	c.mu.Unlock()
	if !deadline.IsZero() {
	var cancel context.CancelFunc
	ctx, cancel = context.WithDeadline(ctx, deadline)
	defer cancel()

The resulting sequence is:

late token-only PUSH_REPLY arrives
readTokenPushReply sets now + 300 ms
the complete token is parsed and returned successfully
watchControl resumes waiting for the next soft reset
the stale deadline expires
waitForSoftReset fails and failControl closes the tunnel

A deterministic in-memory reader test confirmed that the last installed deadline remains non-zero after a successful token read. The helper, or the new watcher branch, must clear the temporary deadline on every return path. A regression test should cover a successful delayed token followed by an idle established connection.

2. ReadAll can reverse TLS control bytes after UDP reordering

ReadAll drains contiguous recvPending packets first and appends parkedTLS afterward:

mihomo/transport/openvpn/control.go

Lines 328 to 349 in 4cbff69

	func (c *ControlChannel) ReadAll() []*ControlPacket {
	c.mu.Lock()
	defer c.mu.Unlock()
	n := len(c.recvPending) + len(c.parkedTLS)
	if n == 0 {
	return nil
	}
	out := make([]*ControlPacket, 0, n)
	for id := c.recvMessage; ; id++ {
	pkt, ok := c.recvPending[id]
	if !ok {
	break
	}
	delete(c.recvPending, id)
	c.recvMessage = id + 1
	out = append(out, pkt)
	}
	for _, payload := range c.parkedTLS {
	out = append(out, &ControlPacket{Opcode: PControlV1, Payload: payload})
	}
	c.parkedTLS = nil
	return out

That order is incorrect when reliable control packets arrive out of order:

expected message n+1 arrives first -> stored in recvPending
expected message n arrives          -> delivered, then stored in parkedTLS
ReadAll output                       -> n+1, then n
UnsafeFeed/TLS input                 -> later ciphertext before earlier ciphertext

The parked payload has already advanced recvMessage, so it logically precedes every newly contiguous entry in recvPending. Feeding these bytes in the current order corrupts the TLS byte stream and can terminate an otherwise recoverable UDP session.

A deterministic state-machine test produced "later" followed by "earlier". parkedTLS must be emitted before the subsequently contiguous recvPending packets, with a regression test that models this reordering.

3. The retiring-key expiration check is bypassed by the normal map entry

installDataChannel stores the retiring channel in dataByKey:

mihomo/transport/openvpn/client.go

Lines 341 to 366 in 4cbff69

	func (c *Client) installDataChannel(newData *DataChannel) {
	c.dataLock.Lock()
	old := c.data
	c.retiring = old
	c.data = newData
	if c.dataByKey == nil {
	c.dataByKey = make(map[uint8]*DataChannel)
	}
	if old != nil && old.keyID != newData.keyID {
	c.dataByKey[old.keyID] = old
	}
	c.dataByKey[newData.keyID] = newData
	// The previous epoch is a lame-duck key: keep it only for the OpenVPN
	// transition_window (default 3600s), then it must not be accepted.
	if old != nil {
	c.retiringExpiry = time.Now().Add(transitionWindow)
	} else {
	c.retiringExpiry = time.Time{}
	}
	// Keep at most the current and previous epoch.
	for id := range c.dataByKey {
	if id != newData.keyID && (old == nil || id != old.keyID) {
	delete(c.dataByKey, id)
	}
	}
	c.dataLock.Unlock()

decryptDataPacket checks retiringExpiry only in the else if branch after a dataByKey miss:

mihomo/transport/openvpn/client.go

Lines 458 to 483 in 4cbff69

	func (c *Client) decryptDataPacket(packet []byte) ([]byte, error) {
	if len(packet) == 0 {
	return nil, errors.New("empty openvpn data packet")
	}
	_, keyID := parseOpcodeKeyID(packet[0])
	c.dataLock.RLock()
	// Route strictly by key ID. A packet labeled with an unknown epoch must
	// be rejected, not silently decrypted with the current key (the CBC HMAC
	// excludes the outer opcode/key-ID header, so a wrong key would still
	// "authenticate").
	var data *DataChannel
	if alt, ok := c.dataByKey[keyID]; ok {
	data = alt
	} else if c.retiring != nil && c.retiring.keyID == keyID {
	if !c.retiringExpiry.IsZero() && time.Now().After(c.retiringExpiry) {
	// Lame-duck key expired (transition_window elapsed): reject.
	c.dataLock.RUnlock()
	return nil, errors.New("openvpn data packet from expired retiring epoch")
	}
	data = c.retiring
	}
	c.dataLock.RUnlock()
	if data == nil {
	return nil, errors.New("openvpn data packet with unknown key id")
	}
	return data.Decrypt(packet)

In a normally installed client, the retiring key always hits the map first, so the expiration branch is unreachable. A valid AEAD packet encrypted with an expired retiring key still decrypted successfully in a focused in-memory test.

The added expiration test does not catch this. It uses an uninitialized DataChannel and a one-byte P_DATA_V2 packet, then accepts any non-nil error. The malformed packet fails header parsing before expiration can be distinguished from an ordinary decryption error.

The expiration check needs to run for a map hit that selects c.retiring, or the expired map entry must be removed before lookup. The test should use a valid packet and require the specific expired-key rejection.

4. The message-ID-0 reset validation is bypassed by pendingSoftReset

The latest commit requires message ID 0 in classifyWatchPacketLocked, which fixes the direct watcher path. The ordinary control-read path parks a next-key soft reset before calling that classifier and still checks only key ID and session:

mihomo/transport/openvpn/control.go

Lines 388 to 406 in 4cbff69

	if !watchSoftReset {
	c.mu.Lock()
	curKey := c.keyID
	sameSession := c.remote == (SessionID{}) || packet.LocalSession == c.remote
	if packet.Opcode == PControlSoftResetV1 {
	// Only park a soft reset for the strictly-next epoch. A
	// delayed reset from a retiring epoch must not move the
	// client backwards, and an invalid one must not mutate ACK /
	// pending-message state.
	if packet.KeyID == NextKeyID(curKey) && sameSession {
	if c.pendingSoftReset == nil {
	c.pendingSoftReset = packet
	}
	for _, ackID := range packet.AckIDs {
	delete(c.pending, ackID)
	}
	}
	c.mu.Unlock()
	continue

waitForSoftReset later returns the parked packet without revalidation:

mihomo/transport/openvpn/control.go

Lines 288 to 296 in 4cbff69

	func (c *ControlChannel) waitForSoftReset(ctx context.Context) (*ControlPacket, error) {
	c.mu.Lock()
	if c.pendingSoftReset != nil {
	packet := c.pendingSoftReset
	c.pendingSoftReset = nil
	c.mu.Unlock()
	return packet, nil
	}
	c.mu.Unlock()

A focused control-channel test sent a next-key soft reset with message ID 5 through the ordinary-read path; it was parked and then accepted by waitForSoftReset.

Acceptance of non-zero reset IDs was pre-existing, so this is not a clean regression by itself. However, pendingSoftReset is introduced by this PR, and the latest commit's stated message-ID validation remains incomplete. The check should be applied before parking and defensively when consuming a parked reset.

[Lanlan13-14]

I found three remaining regressions in the new rekey implementation and one incomplete validation fix. I do not think the PR is ready to merge yet.

1. A successful late token read leaves a 300 ms read deadline active

The new errParkedTLS path immediately feeds a delayed same-epoch TLS payload into the active TLS connection and calls consumeRekeyPush:

mihomo/transport/openvpn/client.go

Lines 491 to 507 in 4cbff69

	func (c *Client) watchControl() {
	for {
	packet, err := c.control.waitForSoftReset(c.runCtx)
	if err != nil {
	if errors.Is(err, errParkedTLS) {
	// A same-epoch TLS payload (token update / late AUTH_FAILED)
	// was parked. Consume it now so a deferred authentication
	// failure is surfaced immediately instead of waiting for the
	// next soft reset (which may never come).
	c.consumeQueuedControl()
	if c.push != nil {
	if err := c.consumeRekeyPush(); err != nil {
	c.failControl(fmt.Errorf("consume parked rekey push: %w", err))
	return
	}
	}
	continue

readTokenPushReply sets a 300 ms read deadline before reading, but a successfully parsed reply returns before that deadline is cleared:

mihomo/transport/openvpn/client.go

Lines 793 to 823 in 4cbff69

	for attempt := 0; attempt < 2; attempt++ {
	_ = conn.SetReadDeadline(time.Now().Add(tokenPushReadTimeout))
	n, err := conn.Read(tmp)
	// Process bytes before handling err: the io.Reader contract permits
	// n > 0 with err != nil (e.g. tls.Conn returns (n, io.EOF) when app
	// data is immediately followed by close_notify). The bytes are valid
	// and must be processed before the terminal error.
	if n > 0 {
	buf = append(buf, tmp[:n]...)
	}
	// AUTH_FAILED always takes precedence so its diagnostic is kept.
	if bytes.Contains(buf, []byte("AUTH_FAILED")) {
	return nil, buf, authFailedError(buf)
	}
	// A complete reply is accepted regardless of a bundled err: the
	// logical message is present, so success must not depend on whether
	// an earlier read happened to read ahead into it (TLS is a byte
	// stream). EOF is propagated only when no complete message exists.
	if reply, rest, ok := takePushReply(buf); ok {
	return reply, rest, nil
	}
	if err != nil {
	_ = conn.SetReadDeadline(time.Time{})
	var netErr net.Error
	if errors.As(err, &netErr) && netErr.Timeout() {
	return nil, buf, nil
	}
	return nil, buf, err
	}
	}
	return nil, buf, nil

This did not normally break the in-renegotiation call because renegotiate has a deferred deadline reset. The new immediate parked-TLS path runs outside that cleanup scope. After applying the token, the watcher continues to waitForSoftReset, whose next raw read reapplies the stale absolute deadline:

mihomo/transport/openvpn/control.go

Lines 550 to 561 in 4cbff69

	return c.io.WritePacket(ctx, encoded)
	}
	func (c *ControlChannel) readRawControlPacket(ctx context.Context) ([]byte, error) {
	c.mu.Lock()
	deadline := c.readDeadline
	c.mu.Unlock()
	if !deadline.IsZero() {
	var cancel context.CancelFunc
	ctx, cancel = context.WithDeadline(ctx, deadline)
	defer cancel()

The resulting sequence is:

late token-only PUSH_REPLY arrives
readTokenPushReply sets now + 300 ms
the complete token is parsed and returned successfully
watchControl resumes waiting for the next soft reset
the stale deadline expires
waitForSoftReset fails and failControl closes the tunnel

A deterministic in-memory reader test confirmed that the last installed deadline remains non-zero after a successful token read. The helper, or the new watcher branch, must clear the temporary deadline on every return path. A regression test should cover a successful delayed token followed by an idle established connection.

2. ReadAll can reverse TLS control bytes after UDP reordering

ReadAll drains contiguous recvPending packets first and appends parkedTLS afterward:

mihomo/transport/openvpn/control.go

Lines 328 to 349 in 4cbff69

	func (c *ControlChannel) ReadAll() []*ControlPacket {
	c.mu.Lock()
	defer c.mu.Unlock()
	n := len(c.recvPending) + len(c.parkedTLS)
	if n == 0 {
	return nil
	}
	out := make([]*ControlPacket, 0, n)
	for id := c.recvMessage; ; id++ {
	pkt, ok := c.recvPending[id]
	if !ok {
	break
	}
	delete(c.recvPending, id)
	c.recvMessage = id + 1
	out = append(out, pkt)
	}
	for _, payload := range c.parkedTLS {
	out = append(out, &ControlPacket{Opcode: PControlV1, Payload: payload})
	}
	c.parkedTLS = nil
	return out

That order is incorrect when reliable control packets arrive out of order:

expected message n+1 arrives first -> stored in recvPending
expected message n arrives          -> delivered, then stored in parkedTLS
ReadAll output                       -> n+1, then n
UnsafeFeed/TLS input                 -> later ciphertext before earlier ciphertext

The parked payload has already advanced recvMessage, so it logically precedes every newly contiguous entry in recvPending. Feeding these bytes in the current order corrupts the TLS byte stream and can terminate an otherwise recoverable UDP session.

A deterministic state-machine test produced "later" followed by "earlier". parkedTLS must be emitted before the subsequently contiguous recvPending packets, with a regression test that models this reordering.

3. The retiring-key expiration check is bypassed by the normal map entry

installDataChannel stores the retiring channel in dataByKey:

mihomo/transport/openvpn/client.go

Lines 341 to 366 in 4cbff69

	func (c *Client) installDataChannel(newData *DataChannel) {
	c.dataLock.Lock()
	old := c.data
	c.retiring = old
	c.data = newData
	if c.dataByKey == nil {
	c.dataByKey = make(map[uint8]*DataChannel)
	}
	if old != nil && old.keyID != newData.keyID {
	c.dataByKey[old.keyID] = old
	}
	c.dataByKey[newData.keyID] = newData
	// The previous epoch is a lame-duck key: keep it only for the OpenVPN
	// transition_window (default 3600s), then it must not be accepted.
	if old != nil {
	c.retiringExpiry = time.Now().Add(transitionWindow)
	} else {
	c.retiringExpiry = time.Time{}
	}
	// Keep at most the current and previous epoch.
	for id := range c.dataByKey {
	if id != newData.keyID && (old == nil || id != old.keyID) {
	delete(c.dataByKey, id)
	}
	}
	c.dataLock.Unlock()

decryptDataPacket checks retiringExpiry only in the else if branch after a dataByKey miss:

mihomo/transport/openvpn/client.go

Lines 458 to 483 in 4cbff69

	func (c *Client) decryptDataPacket(packet []byte) ([]byte, error) {
	if len(packet) == 0 {
	return nil, errors.New("empty openvpn data packet")
	}
	_, keyID := parseOpcodeKeyID(packet[0])
	c.dataLock.RLock()
	// Route strictly by key ID. A packet labeled with an unknown epoch must
	// be rejected, not silently decrypted with the current key (the CBC HMAC
	// excludes the outer opcode/key-ID header, so a wrong key would still
	// "authenticate").
	var data *DataChannel
	if alt, ok := c.dataByKey[keyID]; ok {
	data = alt
	} else if c.retiring != nil && c.retiring.keyID == keyID {
	if !c.retiringExpiry.IsZero() && time.Now().After(c.retiringExpiry) {
	// Lame-duck key expired (transition_window elapsed): reject.
	c.dataLock.RUnlock()
	return nil, errors.New("openvpn data packet from expired retiring epoch")
	}
	data = c.retiring
	}
	c.dataLock.RUnlock()
	if data == nil {
	return nil, errors.New("openvpn data packet with unknown key id")
	}
	return data.Decrypt(packet)

In a normally installed client, the retiring key always hits the map first, so the expiration branch is unreachable. A valid AEAD packet encrypted with an expired retiring key still decrypted successfully in a focused in-memory test.

The added expiration test does not catch this. It uses an uninitialized DataChannel and a one-byte P_DATA_V2 packet, then accepts any non-nil error. The malformed packet fails header parsing before expiration can be distinguished from an ordinary decryption error.

The expiration check needs to run for a map hit that selects c.retiring, or the expired map entry must be removed before lookup. The test should use a valid packet and require the specific expired-key rejection.

4. The message-ID-0 reset validation is bypassed by pendingSoftReset

The latest commit requires message ID 0 in classifyWatchPacketLocked, which fixes the direct watcher path. The ordinary control-read path parks a next-key soft reset before calling that classifier and still checks only key ID and session:

mihomo/transport/openvpn/control.go

Lines 388 to 406 in 4cbff69

	if !watchSoftReset {
	c.mu.Lock()
	curKey := c.keyID
	sameSession := c.remote == (SessionID{}) || packet.LocalSession == c.remote
	if packet.Opcode == PControlSoftResetV1 {
	// Only park a soft reset for the strictly-next epoch. A
	// delayed reset from a retiring epoch must not move the
	// client backwards, and an invalid one must not mutate ACK /
	// pending-message state.
	if packet.KeyID == NextKeyID(curKey) && sameSession {
	if c.pendingSoftReset == nil {
	c.pendingSoftReset = packet
	}
	for _, ackID := range packet.AckIDs {
	delete(c.pending, ackID)
	}
	}
	c.mu.Unlock()
	continue

waitForSoftReset later returns the parked packet without revalidation:

mihomo/transport/openvpn/control.go

Lines 288 to 296 in 4cbff69

	func (c *ControlChannel) waitForSoftReset(ctx context.Context) (*ControlPacket, error) {
	c.mu.Lock()
	if c.pendingSoftReset != nil {
	packet := c.pendingSoftReset
	c.pendingSoftReset = nil
	c.mu.Unlock()
	return packet, nil
	}
	c.mu.Unlock()

A focused control-channel test sent a next-key soft reset with message ID 5 through the ordinary-read path; it was parked and then accepted by waitForSoftReset.

Acceptance of non-zero reset IDs was pre-existing, so this is not a clean regression by itself. However, pendingSoftReset is introduced by this PR, and the latest commit's stated message-ID validation remains incomplete. The check should be applied before parking and defensively when consuming a parked reset.

All four regressions are addressed in 8842c18.

1. Successful late token read leaves a 300 ms read deadline active

readTokenPushReply now clears the temporary read deadline on every return path via defer conn.SetReadDeadline(time.Time{}). A successful parse can no longer leave a stale absolute deadline, so the errParkedTLS path (which runs outside renegotiate's deadline cleanup) does not tear down the idle established connection on the next waitForSoftReset read. Tests: TestReadTokenPushReplyClearsDeadlineOnSuccess, TestReadTokenPushReplyClearsDeadlineOnError.

2. ReadAll can reverse TLS control bytes after UDP reordering

ReadAll now emits parkedTLS before subsequently contiguous recvPending entries. Parked payloads were received out of order and already advanced recvMessage, so they logically precede every newly contiguous entry; emitting them first keeps the TLS byte stream in order after UDP reordering. Test: TestReadAllEmitsParkedBeforeContiguous (asserts "earlier" then "later").

3. The retiring-key expiration check is bypassed by the normal map entry

decryptDataPacket now checks the retiring-epoch expiry before the dataByKey lookup, so a map hit on the retiring key cannot bypass the transition-window expiration. This closes the case where a valid AEAD packet encrypted with an expired retiring key was still decrypted. Tests: TestDecryptRejectsExpiredRetiringKey (now routes the expiry check before lookup), TestDecryptAcceptsRetiringKeyBeforeExpiry.

4. The message-ID-0 reset validation is bypassed by pendingSoftReset

The ordinary control-read park path now also requires MessageID == 0 before parking a next-key soft reset, and waitForSoftReset defensively revalidates a parked reset (next-epoch + message 0) when consuming it — so a non-zero reset ID can no longer be parked and then accepted. Tests: TestWaitForSoftResetRejectsInvalidParkedReset, and the message-id case in TestClassifyWatchAcceptsNextEpochSoftReset.

Verification

• go test -race ./transport/openvpn/ clean
• Live: OpenVPN 2.6.14, reneg-sec 45, auth-gen-token 0 180 — 2 soft resets, 2× Username/auth-token authentication succeeded, 101/101 HTTP probes, no TLS Error / link closed / AUTH_FAILED / expired-key errors

[wwqgtxx]

I reviewed the current PR head from the issue report and the OpenVPN 2.6.20 state machine, without carrying forward earlier review conclusions. The core correction is valid: an OpenVPN soft reset starts a fresh SSL/key state, and the PR now uses the peer's next key ID in both control and data headers. However, I still see two blocking issues in the delayed-authentication path.

1. Keep the retiring key active for outbound data during deferred authentication

The rekey branch installs the new data channel immediately after receiving the server key-method-2 record:

• client.go:246-263
• client.go:341-356

After installDataChannel, every outbound packet uses c.data, so it is labeled with the new key ID immediately. That is not equivalent to OpenVPN's key selection during deferred authentication.

OpenVPN assigns every new key state an auth_deferred_expire window, delays server-side data-key generation until authentication is actually KS_AUTH_TRUE, and deliberately continues selecting the lame-duck key for outbound traffic while the new key is inside that window:

• auth_deferred_expire_window
• server key generation after authentication
• tls_select_encryption_key

With a management or plugin-based deferred authenticator, the server can send its KM2 record while the new key is still unauthenticated. Mihomo then starts sending new-key data after at most the short token probe, but the server has not generated that data key yet and drops those packets. This creates an asymmetric outage during each rekey and can break active connections when authentication takes more than a trivial amount of time.

Please separate "new key available for eventual activation/decryption" from "key selected for outbound encryption." The old key must remain the outbound key for the OpenVPN deferred-auth grace period, or until there is reliable evidence that the peer has activated the new key.

2. Parse the TLS control stream as NUL-delimited messages before looking for a token reply

takePushReply checks whether the buffer contains PUSH_REPLY, but it always parses only the bytes before the first NUL in the entire buffer:

• client.go:841-871

OpenVPN deferred authentication sends AUTH_PENDING\0 first (and can also send INFO_PRE\0), then sends the token-only PUSH_REPLY\0 after authentication succeeds:

• send_auth_pending_messages
• control-message dispatch

For this valid stream:

AUTH_PENDING\0PUSH_REPLY,auth-token SESS_ID_fresh\0

the current code finds PUSH_REPLY, takes the first NUL, tries to parse AUTH_PENDING as a push reply, and leaves the complete token inaccessible forever. I confirmed this with a pure in-memory regression test: readTokenPushReply returned reply == nil and preserved the whole two-message buffer. The next rekey therefore still uses the stale token and can fail authentication.

Please extract and consume complete NUL-delimited control messages in order. Handle AUTH_FAILED, accept a complete PUSH_REPLY, skip or dispatch other complete commands, and retain only the final incomplete message. Also, TestWatchControlSurfacesParkedAUTHFailed currently never injects AUTH_FAILED; it only checks the no-TLS renegotiation error, so it does not cover the behavior named by the test.

[wwqgtxx]

The NUL-delimited control-message problem from the previous review is fixed. However, I still do not think this revision is ready to merge. The new outbound-key selection has three correctness problems, and the shortened key-method-2 path still cannot enter the newly supported deferred-auth control flow.

1. A packet that fails decryption still promotes the new outbound key

decryptDataPacket sets newKeyEvidence = true before calling data.Decrypt(packet).

Therefore, the code does not implement the condition stated by its comment ("decrypted successfully"). Any malformed packet carrying the current key ID sets the latch even when header validation, AEAD authentication, CBC HMAC verification, or replay validation fails. The next outbound write then switches away from the lame-duck key.

This is especially concrete for CBC because the outer opcode/key-ID byte is not covered by its HMAC: changing an old packet's header to the new key ID makes decryption fail under the new key, but still promotes that key. During deferred authentication, that recreates the behavior this commit is intended to prevent: the server has not generated/authorized the new data key and drops the client's new-key packets.

A pure in-memory regression test using a one-byte P_DATA_V2 packet with the new key ID failed as follows:

failed decryption promoted the newest outbound key

The promotion must happen only after cryptographic validation succeeds. It must also re-check c.data == data while holding dataLock; otherwise a packet selected just before another rekey can set evidence for the following epoch after the lock is reacquired.

2. A valid old epoch is discarded for outbound selection if it has not sent a packet

installDataChannel retains the old outbound key only when old.Started() is true. Started merely means that mihomo has encrypted at least one outbound packet with that key; it says nothing about whether the epoch is authenticated or valid.

The only initial-handshake case is old == nil. If a server-initiated rekey occurs on a quiet or receive-only tunnel with client ping disabled, old is non-nil but Started() is false, so this code selects the new key immediately. A deferred-auth server has not generated that key yet and drops the traffic.

The minimal in-memory sequence install(old) followed by install(new) reproduces this without any network dependency:

rekey selected the new outbound key solely because the old key had not sent data

An existing old data channel should remain the outbound candidate regardless of its packet counter. old == nil is sufficient to distinguish the first handshake.

3. The asymmetric-traffic backstop uses the wrong OpenVPN window

writeDataPacket promotes without peer evidence only when fewer than 60 seconds remain in the hard-coded 3600-second retiring-key lifetime. With no inbound data on the new epoch, mihomo therefore keeps encrypting with the old key for about 3540 seconds.

That is not how OpenVPN selects an encryption key. OpenVPN assigns the new key an auth_deferred_expire deadline based on the handshake window (capped by half of reneg-sec) and selects the lame-duck only until that deadline:

• auth_deferred_expire_window
• tls_select_encryption_key

With defaults, the selection transition is around 60 seconds after the new epoch starts, not 60 seconds before the old key's 3600-second destruction time. The current policy makes a one-way tunnel use the retired key for almost the entire transition window, defeats the expected outbound key rotation, and can black-hole traffic much earlier when the peer uses a shorter transition-window.

Peer evidence can be an early positive signal, but the no-evidence deadline needs to model auth_deferred_expire, not retiringExpiry - 60s.

4. A shortened KM2 record followed by AUTH_PENDING still times out

The PR supports a shortened server key-method-2 record by recognizing the following control command as the record boundary. However, looksLikeFollowingTLSControl recognizes only PUSH_REPLY, AUTH_FAILED, and PUSH_REQUEST.

For the shortened-record compatibility mode introduced by this PR, the following control-command ordering is possible:

<shortened server KM2><AUTH_PENDING\0><INFO_PRE,...\0><PUSH_REPLY,...\0>

the parser treats the leading AU bytes as an incomplete OpenVPN string length. readServerKeyMethod keeps reading and never reaches takePushReply; appending the later messages cannot help because the unrecognized tail still begins with AUTH_PENDING. The rekey eventually hits its 30-second deadline.

If shortened KM2 records are intentionally supported, every valid server-to-client command that can immediately follow one must be recognized at this boundary, including at least AUTH_PENDING and INFO_PRE for the deferred-auth path added by this revision.

[wwqgtxx]

The latest commit correctly fixes two previously reported points: an existing but unused old epoch is now retained as the outbound candidate, and the shortened-KM2 boundary recognizes the additional control commands. The core fix for #3085 also remains directionally correct: each soft reset creates a fresh TLS/key epoch, the peer's key ID is adopted, data headers use that key ID, and the original rekey error is preserved.

However, I still do not think the PR is ready to merge.

1. the outbound-key backstop ignores AUTH_PENDING,timeout N

The latest commit adds a fixed 60-second promotion deadline:

• authDeferredExpire
• writeDataPacket

This models only the default initial deferred-auth window. It does not model OpenVPN's dynamic deferred-auth protocol. When the server sends AUTH_PENDING,timeout N, OpenVPN both advertises that timeout and moves the key state's deadline to now + timeout:

• send_auth_pending_messages

The reference client also parses the advertised value instead of assuming 60 seconds:

• parse_auth_pending_keywords / receive_auth_pending

Mihomo now recognizes AUTH_PENDING as a possible message after shortened KM2, but takePushReply consumes it as an ignored non-push message. The timeout is never parsed or stored.

A valid failure sequence is therefore:

server sends AUTH_PENDING,timeout 300
new key remains KS_AUTH_DEFERRED
61 seconds elapse without new-key receive evidence
mihomo promotes outbound to the new key because of the fixed backstop
server drops those packets as "not authorized (deferred)"

OpenVPN explicitly rejects data for an active key while its authentication state is still deferred:

• tls_pre_decrypt

A focused pure in-memory test reproduced the premature switch: after observing AUTH_PENDING,timeout 300 and simulating 61 elapsed seconds, the outgoing packet used key ID 1 instead of retaining old key ID 0.

The minimal fix is to parse the timeout into per-epoch state and ensure the no-evidence promotion deadline never precedes the explicitly advertised deferred-auth deadline. The existing test only moves outboundStart past the hard-coded constant, so it validates the implementation assumption rather than the protocol behavior.

This issue is specific to the new fixed-60-second implementation in 1ae007f5; the previous retiring-expiry fallback was also incorrect, but replacing one hard-coded timer with another does not make deferred authentication protocol-correct.

2. PUSH continuation handling is still incomplete

takePushReply returns success after parsing any complete PUSH_REPLY. It does not distinguish an intermediate push-continuation 2 segment from the final segment.

OpenVPN emits intermediate segments with push-continuation 2 and only marks the final segment with push-continuation 1:

• send_push_options
• send_push_reply

If a TLS read ends after an intermediate segment, readPushReply returns before the remaining segments arrive. A focused test failed with:

intermediate push-continuation segment was treated as a complete reply

There is a second deterministic loss mode when multiple segments are coalesced in one read. mergePushReply inherits a slice only when the later slice is empty. If both segments contain routes or DNS servers, the later segment replaces the earlier values. A two-segment test produced:

continuation values lost: routes=[10.2.0.0/16] dns=[8.8.8.8]

The parser needs to retain continuation state, wait for the final segment, and append repeatable fields in wire order. The PR's current continuation test calls mergePushReply directly and puts each slice field in only one segment, so it does not cover either failure.

The underlying lack of continuation support predates this PR, but commit 371c89e8 explicitly attempts to fix and test it. That attempted fix remains incomplete in the current PR.

3. dropped out-of-window reliable packets are ACKed

The receive-window bound in ControlChannel.read is applied only when inserting into recvPending. The message ID is added to ackPending before the window decision, and the out-of-order branch sets sendAck even when recvWindowOK rejects the packet.

Consequently, a packet can be dropped locally but acknowledged to the peer. A focused in-memory test sent message ID 12 while the receiver expected 0. The packet was correctly excluded from recvPending, but mihomo still emitted an ACK for ID 12:

dropped out-of-window message 12 was ACKed

The sender may then delete message 12 and never retransmit it. Once messages 0 through 11 are delivered, the reliable stream has a permanent hole at 12.

OpenVPN acknowledges the message only inside the successful reliable_wont_break_sequentiality branch. It still ACKs a valid in-window replay, but it does not ACK a packet rejected for breaking the receive window:

• tls_pre_decrypt

ACK insertion and sending should occur only after the packet is known to be an accepted in-window packet or an in-window replay. This regression was introduced by 9f3546f7, which added the recvPending bound.

[wwqgtxx]

I re-reviewed the PR from its base through the current head, including the latest commit (fix(openvpn): honor AUTH_PENDING timeout, complete push-continuation, don't ACK dropped packets). The previous out-of-window ACK issue is fixed: a packet outside the receive window is now dropped without being acknowledged.

I still do not think the PR is ready to merge. The latest implementation leaves the following protocol issues.

1. AUTH_PENDING still does not protect the deferred epoch

There are three independent problems in the current path:

1. A standalone AUTH_PENDING,timeout 300\0 is treated as a complete push reply. takePushReply sets parsed = true for AUTH_PENDING, and therefore returns ok = true when no PUSH_REPLY has arrived. During initial authentication, readPushReply returns immediately, after which the handshake fails with openvpn push reply missing ifconfig address instead of waiting for the real push reply.

2. If AUTH_PENDING and PUSH_REPLY are coalesced in the same TLS plaintext buffer, mergePushReply does not preserve AuthPendingTimeout. The merged reply consequently has a zero timeout.

3. Even if the timeout reaches applyAuthPendingTimeout, production code applies it before installing the new data channel, while installDataChannel unconditionally clears deferredUntil. The new test uses the reverse order (install first, then manually apply the timeout), so it does not exercise the actual handshake/rekey sequence. In addition, applyAuthPendingTimeout writes deferredUntil without dataLock, whereas packet writes read it while holding that lock.

Pure in-memory reproductions fail deterministically with:

AUTH_PENDING alone completed PUSH parsing
AUTH_PENDING timeout lost during merge: got 0s, want 5m
epoch install discarded AUTH_PENDING deadline: outbound key=1 after 61s

Completeness needs to be based on receiving a final PUSH_REPLY, not merely parsing AUTH_PENDING. The timeout also needs to survive merging and be associated with the newly installed epoch under the same synchronization used by the data path.

2. push continuation is still reordered or lost

The merge helpers claim to preserve wire order, but appendUniquePrefixes, appendUniqueAddrs, and appendUniqueStrings initialize their output from next and append prev, producing next + prev. This reverses continuation segments. It is not only cosmetic for routes: reversing DataCiphers can change the selected cipher because NegotiateCipher uses the first mutually supported server cipher.

Continuation state is also lost across calls. In consumeRekeyPush, when a final segment arrives after an earlier call stored an intermediate segment in pushPending, the ok branch clears pushPending before merging it. The existing test only checks slice lengths for segments supplied in one buffer, so neither order nor cross-call accumulation is covered.

Pure in-memory reproductions fail with:

route wire order changed: [10.2.0.0/16 10.1.0.0/16]
intermediate continuation was discarded across calls: [10.2.0.0/16]

3. an already-buffered in-window duplicate is not re-ACKed

The new receive-window logic correctly avoids ACKing rejected packets, but ControlChannel.read ACKs an out-of-order packet only when inserting it into recvPending. If that packet is retransmitted because its first ACK was lost, exists == true suppresses both insertion and ACK.

OpenVPN queues the ACK whenever reliable_wont_break_sequentiality accepts the packet, including when reliable_not_replay subsequently identifies it as an already-buffered replay. Mihomo should likewise re-ACK an existing in-window entry without re-inserting or delivering it. The current behavior causes unnecessary retransmissions and keeps the sender's reliable slot occupied.

The focused reproduction fails with:

buffered in-window duplicate was not re-ACKed: context deadline exceeded

[wwqgtxx]

The latest commit does fix the previously reported out-of-window ACK behavior, buffered duplicate re-ACK behavior, continuation wire order/cross-call accumulation, and the pre-buffered AUTH_PENDING merge/install ordering. However, the normal OpenVPN deferred-auth path is still not complete, so I do not think this is ready to merge.

1. mihomo does not advertise the AUTH_PENDING keyword capability and does not handle the required bare fallback

InstallScriptPeerInfo still emits IV_PROTO=6. OpenVPN defines IV_PROTO_AUTH_PENDING_KW as bit 4, so supporting AUTH_PENDING,timeout N requires that bit to be present (6 | 16 = 22). The server explicitly checks this bit: send_auth_pending_messages sends AUTH_PENDING,timeout N only when the client advertises the capability; otherwise it sends a bare AUTH_PENDING.

This means a standard OpenVPN server will not send the timeout form to the current mihomo client. The fallback is also missing: parseAuthPendingTimeout gives bare AUTH_PENDING a zero timeout, applyAuthPendingTimeout ignores it, and the incomplete-reply check only treats AuthPendingTimeout > 0 as pending. In contrast, OpenVPN's receive_auth_pending uses handshake_window when no timeout keyword is supplied.

The focused protocol checks fail with:

IV_PROTO=6 does not advertise AUTH_PENDING keyword support
bare AUTH_PENDING did not receive a default timeout: AuthPendingTimeout=0

At minimum, the peer-info capability and parser behavior must agree: advertise bit 4 now that the keyword is implemented, while retaining the OpenVPN-compatible default for a bare message.

2. deferred/continued state discovered inside readTokenPushReply still returns after the short probe

readTokenPushReply initializes waitUntil only from the optional deadline known before the call. When the function itself reads an AUTH_PENDING,timeout 300 message, it merges that message into acc but never updates waitUntil. If the next 300 ms read times out, it returns the incomplete accumulator immediately. consumeRekeyPush applies the advertised deadline only after the reader has returned, then immediately reports openvpn deferred/continued push reply incomplete. Thus the timeout is honored only when AUTH_PENDING happened to be read ahead into leftoverTLS; behavior still depends on TLS read boundaries.

The same state propagation gap affects continuation. If push-continuation 2 is first discovered inside readTokenPushReply, the reader returns the intermediate reply after the next short timeout. The caller sets pushContinuationPending for replies parsed before the call, but does not set it for the more reply returned by the reader, so the rekey can continue with an incomplete push.

Deterministic in-memory readers reproducing message -> short timeout -> final PUSH_REPLY fail with:

AUTH_PENDING: reader returned at step 2 before the final PUSH_REPLY at step 3
push-continuation 2: reader returned the intermediate segment at step 2 before the final segment at step 3

The reader needs to promote newly parsed AUTH_PENDING/continuation state into its active wait policy, or return explicit incomplete-state metadata that makes the caller continue waiting. It cannot decide the deadline solely from state available before the read.

[wwqgtxx]

I re-reviewed the PR at the current head, including fix(openvpn): advertise and dynamically honor AUTH_PENDING.

The latest commit fixes the two issues from my previous review at the parser/helper level: mihomo now advertises IV_PROTO=22, bare AUTH_PENDING receives a default timeout, and readTokenPushReply promotes its local wait policy when it discovers AUTH_PENDING or push-continuation 2 during a read. The earlier ACK-window and continuation-order findings also remain fixed.

There is still one blocking deadline-propagation problem, plus a related timeout-anchor problem.

1. dynamically extending only the read deadline leaves reliable ACK writes on the expired 30-second deadline

startTLSEpoch sets both sides of the TLS connection deadline to the rekey context deadline (30 seconds). If AUTH_PENDING,timeout 300 was already buffered before readTokenPushReply is called, applyAuthPendingTimeout extends the complete ControlConn deadline and the path works. However, when AUTH_PENDING is first discovered inside readTokenPushReply, promoteWaitPolicy only updates a local waitUntil, and the loop only calls SetReadDeadline. applyAuthPendingTimeout, which updates both read and write deadlines, is not called until the helper returns after the final push.

That distinction matters because this is a reliable control channel. ControlChannel.read sends an ACK before delivering an accepted control packet, and writeControlPacket applies the still-existing write deadline. Once the original 30 seconds have elapsed, a final P_CONTROL_V1 packet can be read but its ACK fails with context deadline exceeded, so the payload is never delivered to TLS despite the newly extended read wait.

A deterministic in-memory ControlConn reproduction does the same operations as the current dynamic path (SetDeadline(expired), then only SetReadDeadline(future)) and fails with:

extended read could not deliver a packet because its ACK used the stale write deadline: context deadline exceeded

The new regression test uses AUTH_PENDING,timeout 1, which completes before the original 30-second write deadline and therefore cannot catch this. State discovered inside the helper needs to extend the underlying control operation immediately, including the write side required for reliable ACKs.

2. the advertised timeout is restarted when the final push arrives

The helper retains AuthPendingTimeout as a duration. After it may have waited for part of that duration, consumeRekeyPush passes the merged reply to applyAuthPendingTimeout, which computes a new deadline as time.Now().Add(reply.AuthPendingTimeout). The timeout is consequently anchored to the final PUSH_REPLY, not to the AUTH_PENDING observation that started the wait.

For example, an in-memory reader receiving AUTH_PENDING,timeout 1, waiting 300 ms, and then receiving the final push produces a staged deadline approximately 1.3 seconds after the pending message, rather than 1 second:

AUTH_PENDING timeout restarted after final PUSH_REPLY: deadline=observed+1.30s

OpenVPN stores an absolute push timeout for the current key state when it processes AUTH_PENDING; it does not restart the full interval after authentication completes. Restarting it can keep mihomo transmitting with the lame-duck key longer than the server-advertised window, particularly on a one-way tunnel where no new-key peer packet arrives to trigger early promotion.

[wwqgtxx]

The latest commit correctly fixes both findings from my previous review: dynamically observed AUTH_PENDING/continuation state now extends both deadline directions so reliable ACKs can be written, and the deferred-auth deadline is anchored when AUTH_PENDING is parsed instead of being restarted by the final push. The earlier capability, parser, receive-window, and continuation fixes also remain intact.

I found one remaining lifecycle bug and one timeout-update issue.

1. the parked-TLS success path leaves the operation deadline on the established connection

applyAuthPendingTimeout deliberately applies the absolute deferred-auth deadline to both tlsConn and controlConn. During a normal rekey, renegotiate has a defer that clears the ControlConn deadline when the operation finishes.

However, consumeRekeyPush is also called independently by the errParkedTLS branch in watchControl. That branch consumes the parked push and immediately continues the established-channel loop; it never runs renegotiate and has no equivalent deadline cleanup. A late AUTH_PENDING followed by its final push therefore succeeds but leaves both the read and write deadlines set to authPendingUntil. The next waitForSoftReset inherits that operation deadline and tears down an otherwise established tunnel when it expires.

A deterministic in-memory reproduction invokes the same successful parked consume with:

AUTH_PENDING,timeout 1\0PUSH_REPLY,auth-token SESS_ID_parked\0

and observes:

successful parked push consume leaked operation deadline: read=observed+1s write=observed+1s

The parked branch needs the same clear-on-operation-exit ownership as renegotiate (on both success and error paths where the connection remains usable), or consumeRekeyPush must not leave transport deadlines behind when used standalone.

2. a later AUTH_PENDING cannot shorten the current deadline

OpenVPN's management protocol explicitly states that receiving AUTH_PENDING changes the timeout proposed by the server even when the new timeout is shorter (management notes).

The current implementation only accepts later deadlines. applyAuthPendingTimeout updates deferredUntil/pendingDeferredUntil only when deadline.After(...), while promoteWaitPolicy also ignores every AUTH_PENDING after the first observation.

Applying timeout 60 and then timeout 1 for the same key deterministically leaves the staged deadline at 60 seconds:

later AUTH_PENDING did not shorten deadline: got=observed+60s want=observed+1s

Deadline replacement should be tied to the same key/epoch and the latest AUTH_PENDING message, rather than using a monotonic maximum.

[wwqgtxx]

The latest commit fixes the previously reported deferred-auth deadline lifecycle issues, but one correctness issue remains at head 04a624f4fce8bfc40af8eb845a52b2f22f401865.

Preserve a continuation discovered by the final probe

transport/openvpn/client.go:381-394

When push-continuation 2 is already present in leftoverTLS, this function sets c.pushContinuationPending = true. The equivalent state is not recorded when the continuation is first discovered inside readTokenPushReply, however. If that reader reaches its continuation deadline before receiving the final segment, it returns the accumulated PushReply with HasPushReply == true and PushContinuation == 2. The caller sets complete to false, but only clears c.pushContinuationPending on success and never sets it on this path.

Consequently, if no AUTH_PENDING was also present, the incomplete guard at lines 392-394 is false and consumeRekeyPush returns success. doKeyExchange can then install the new data channel while the partial reply remains only in c.pushPending; options or a renewed auth token in the unfinished continuation are not committed. A subsequent rekey can therefore use stale state.

I reproduced this deterministically with an in-memory reader result equivalent to:

&PushReply{
    HasPushReply:     true,
    PushContinuation: 2,
}

consumeRekeyPush returned nil instead of openvpn deferred/continued push reply incomplete.

The minimal fix is to update the persistent continuation state whenever more contains a PUSH reply, for example:

if more.HasPushReply {
    c.pushContinuationPending = more.PushContinuation == 2
}

This should be covered by a regression test where the intermediate continuation is discovered by the reader and the final segment never arrives. The existing dynamic-continuation test always supplies a final segment, so it does not exercise this exit path.

[wwqgtxx]

Do not require a final PUSH_REPLY after rekey AUTH_PENDING

mihomo/transport/openvpn/client.go

Lines 404 to 406 in 05cea9b

	if attemptedFinalRead && (c.pushContinuationPending ||
	(c.pushPending != nil && !complete && c.pushPending.AuthPendingTimeout > 0)) {
	return errors.New("openvpn deferred/continued push reply incomplete")

This rejects a valid OpenVPN deferred-auth rekey when the server does not use auth-gen-token. readTokenPushReply waits until the advertised AUTH_PENDING deadline and returns the accumulated pending metadata when no final push arrives. This guard then turns that result into openvpn deferred/continued push reply incomplete, so doKeyExchange aborts and watchControl closes the tunnel.

A final PUSH_REPLY is not a required deferred-auth success marker during a soft reset. OpenVPN can enter KS_AUTH_DEFERRED and send AUTH_PENDING for the new key state:

https://github.com/OpenVPN/openvpn/blob/c9b790f5b9e8ebca5da38c22f479c31bb8d33686/src/openvpn/ssl_verify.c#L900-L952

After authentication succeeds, OpenVPN generates the new data keys and calls resend_auth_token_renegotiation:

https://github.com/OpenVPN/openvpn/blob/c9b790f5b9e8ebca5da38c22f479c31bb8d33686/src/openvpn/ssl.c#L3407-L3432

That helper sends the minimal token-only PUSH_REPLY only when multi->auth_token_initial exists. Without auth-gen-token, it sends nothing:

https://github.com/OpenVPN/openvpn/blob/c9b790f5b9e8ebca5da38c22f479c31bb8d33686/src/openvpn/auth_token.c#L483-L501

Therefore this legal sequence has no final push:

soft reset -> KM2 -> AUTH_PENDING -> authentication succeeds -> data key becomes usable

The rekey path should distinguish an unfinished push-continuation 2 from standalone deferred-auth metadata. Absence of a token push must not be a hard error: retain the cached push state and the epoch-scoped deferred deadline, install/retain the derived data epoch, and continue to surface a later AUTH_FAILED if one arrives.

I reproduced the current failure with a package-local, in-memory test that supplied an existing push state followed by AUTH_PENDING,timeout 1 and no token push. consumeRekeyPushFrom deterministically returned:

openvpn deferred/continued push reply incomplete

This path and completeness rule are introduced by this PR; they are not a pre-existing baseline defect.

[wwqgtxx]

Install the new receive epoch before waiting out a standalone AUTH_PENDING

consumeRekeyPushFrom passes the staged deferred-auth deadline to readTokenPushReply. Once that reader sees AUTH_PENDING, it keeps retrying until the full advertised timeout (lines 1088-1106, lines 1162-1178). doKeyExchange therefore cannot install the already-derived data channel until that timeout expires (line 283).

That wait is not compatible with every valid OpenVPN configuration. AUTH_PENDING,timeout N controls how long the client waits for authentication/push completion; it does not extend the old data key's lifetime. OpenVPN starts the old key's independent transition_window when the soft reset occurs (ssl.c) and deletes it when that window expires (ssl.c). The advertised deferred-auth timeout is calculated separately and is not capped by transition_window (push.c); --tran-window is explicitly configurable (documentation).

For example, with tran-window 10 and AUTH_PENDING,timeout 60, authentication may succeed before 10 seconds without producing a token-only PUSH_REPLY when auth-gen-token is not enabled. At 10 seconds the server deletes the old key and can start using the authenticated new key. Mihomo is still blocked in readTokenPushReply, so it has neither installed the new receive key nor stopped transmitting with the old key. Both directions can then lose traffic until the 60-second pending deadline.

The new test does not exercise this production timing: it injects a reader that returns the pending reply immediately (rekey_test.go).

The minimal fix is to retain the AUTH_PENDING epoch deadline but use only the existing short token probe unless a push-continuation 2 is outstanding. Then install the new data channel promptly for receive, keep outbound traffic on the old epoch until peer evidence or the deferred deadline, and let the parked-TLS watcher consume any later token update or AUTH_FAILED message.

[wwqgtxx]

Do not keep transmitting with a retiring key past its usable lifetime

installDataChannel gives the retiring epoch a finite transition lifetime, but writeDataPacket does not include that lifetime in its promotion decision. When AUTH_PENDING,timeout N was received, deferredUntil completely replaces the normal 60-second outbound fallback. If there is no packet from the peer carrying the new key ID, mihomo therefore continues encrypting with outboundKey even after that key is no longer usable by the peer.

This is observable with a valid deferred-auth configuration:

1. A soft reset installs the new epoch for receive and retains the old epoch for transmit.
2. The server advertises an auth-pending timeout longer than its tran-window.
3. Authentication succeeds before tran-window expires, without a token-only PUSH_REPLY (auth-gen-token is not enabled).
4. The tunnel is one-way, so no new-key server packet arrives to set PeerActive.
5. The server deletes the old key at tran-window, while mihomo keeps sending packets labeled with the old key ID until the full advertised pending deadline.

Those packets are dropped for the interval between old-key deletion and deferredUntil. A focused in-memory package reproduction set the retiring expiry in the past while keeping the advertised pending deadline in the future; the next outbound packet still used key ID 0 instead of key ID 1. The existing TestAuthPendingTimeoutRespected verifies retention before the pending deadline, but never crosses the retiring-key lifetime.

The two deadlines are separate in upstream OpenVPN. receive_auth_pending changes the client push-request timeout; it does not replace the client's data-key selection deadline. The lame-duck key is independently deleted at the transition deadline (ssl.c), after which it cannot be selected by tls_select_encryption_key. --tran-window is explicitly configurable.

Please keep AUTH_PENDING as control/deferred-auth metadata without allowing it to extend old-key transmission beyond the retiring key's lifetime. The closer match to upstream is to model the outbound auth-deferred selection deadline independently; at minimum, old-key expiry must participate in the promotion decision.

This behavior is introduced by this PR: the base version had no split data/outboundKey selection or deferredUntil override.

[wwqgtxx]

Start the retiring-key window when the soft reset is accepted

mihomo/transport/openvpn/client.go

Lines 848 to 851 in 5f2f57f

	// OpenVPN starts the lame-duck transition window at soft reset, before
	// TLS and KM2 processing. Anchor it here so data-channel installation
	// cannot restart the old key's usable lifetime.
	c.beginRetiringWindow()

beginRetiringWindow() is still called too late to represent the soft-reset time. After waitForSoftReset() has already accepted the server reset, watchControl() first calls consumeQueuedControl() and consumeRekeyPush() on the previous TLS epoch, and only then enters renegotiate():

mihomo/transport/openvpn/client.go

Lines 800 to 812 in 5f2f57f

	// Token-only PUSH_REPLY parked since the last rekey must land in
	// authPass before this key-method-2 exchange, otherwise the server
	// rejects the expired token. A parked AUTH_FAILED (deferred auth) is
	// a hard failure: surface it before starting the next epoch instead
	// of replacing it with the next renegotiation result.
	c.consumeQueuedControl()
	if c.push != nil {
	if err := c.consumeRekeyPush(); err != nil {
	c.failControl(fmt.Errorf("consume queued rekey push: %w", err))
	return
	}
	}
	if err := c.renegotiate(packet); err != nil {

When no late token is available, that probe may wait for the full tokenPushReadTimeout (300 ms). The resulting local deadline is therefore approximately:

soft-reset observation + token probe + tran-window

rather than:

soft-reset observation + tran-window

Upstream OpenVPN sets must_die inside key_state_soft_reset(), immediately when the old primary is moved to the lame-duck slot:

https://github.com/OpenVPN/openvpn/blob/c9b790f5b9e8ebca5da38c22f479c31bb8d33686/src/openvpn/ssl.c#L1927-L1936

This matters now that the PR exposes short tran-window values. With matching client/server windows, a one-way tunnel, and no new-key peer packet, the server can delete the old key at its reset-anchored deadline while mihomo continues selecting it until the locally delayed deadline. Packets sent during that extra probe interval are labeled with a key the server has already removed.

A deterministic in-memory reproduction used a no-data net.Pipe for the pre-rekey token probe and measured the deadline being shifted by about 300.7 ms, matching tokenPushReadTimeout. The temporary test was removed after verification.

Please capture or install the retiring deadline immediately after waitForSoftReset() accepts the reset, before probing the previous TLS stream, and pass that absolute deadline through to installDataChannel() without restarting it in renegotiate().

This timing defect is introduced by the PR's split outbound/retiring-key lifecycle; it is not a pre-existing Alpha behavior.