Add push-peer-info client identity fields (UV_UUID, IV_HWADDR) - #4
Open
IIayk122 wants to merge 2 commits into
Open
Add push-peer-info client identity fields (UV_UUID, IV_HWADDR)#4IIayk122 wants to merge 2 commits into
IIayk122 wants to merge 2 commits into
Conversation
Install a single decrypt key for static keys, tls-auth, and tls-crypt. Upstream init_key_ctx_bi derives one decrypt key from key_direction_state_init, while the extra fallback candidate accepted the local send key, so a reflected outbound packet authenticated as inbound and reached the tunnel. Act on an unmatched hard reset only when tls-auth or tls-crypt authenticated it. Upstream tls_pre_decrypt routes such a packet to a fresh TM_INITIAL session and leaves TM_ACTIVE running; without control channel protection any forged datagram terminated a live session. Resume a TCP frame that was interrupted between its length prefix and its body. Every reader drives the socket with a short deadline and retries on timeout, so discarding the consumed prefix desynchronized the frame stream permanently. Acknowledge an incoming control packet only when the receive buffer could take it. Upstream tls_pre_decrypt calls reliable_ack_acknowledge_packet_id inside the reliable_can_get and reliable_wont_break_sequentiality guards, so a dropped packet keeps being retransmitted instead of being purged from the peer's send buffer. Replays are still acknowledged. Return drained acknowledgment ids when the packet carrying them cannot be queued, both in the send window retry loop and on the standalone acknowledgment path. Reassemble control messages across TLS records. crypto/tls splits a single Write above maxPayloadSizeForWrite and never merges records on Read, so a large key-method-2 message or PUSH_REPLY was parsed truncated. Message length now comes from the key-method-2 layout or the NUL terminator, and bytes belonging to the next message stay buffered between reads. Keep control channel deadlines local to the channel. The root channel, every renegotiation child and the data path share one packetConnection, so forwarding a deadline to the socket let a failed renegotiation poison data channel writes. The reader now polls the socket on a fixed interval, and Read and Write enforce the channel deadlines themselves. Ignore a failed keepalive write in the server ping loop. tryWriteDataPacket also reports contention on the data write lock, and upstream check_ping_send_dowork never terminates the instance because a ping could not be queued. Bound pushed second counts before converting them to time.Duration. A large value wrapped into a small positive interval and made the client exit on the first keepalive tick. Handle OCC on the static key data channel. Upstream arms occ_interval only when !TLS_MODE, so OCC_REQUEST, OCC_REPLY and OCC_EXIT arrive exactly there; they previously reached the tunnel as non-IP packets and no reply was ever sent. Clear the fast retransmission counter when a packet is resent, as upstream reliable_send does, instead of retransmitting on every sender tick until the packet is acknowledged. Reject a control packet declaring more than RELIABLE_ACK_SIZE acknowledgment ids, matching reliable_ack_parse. Emit the comp-lzo OCC token for every active compression framing and mtu-dynamic for --fragment, as options_string does. Reject a stateless cookie response whose packet id or acknowledged ids exceed 1, as check_session_hmac_and_pkt_id does. Mask the IPv4 fragment offset with OPENVPN_IP_OFFMASK. The wider mask also covered the More Fragments bit, so --mssfix skipped the first fragment even though it carries the TCP header.
Some enterprise OpenVPN servers that expect --push-peer-info / OpenVPN Connect peer-info reject TLS handshakes with AUTH_FAILED when UV_UUID or IV_HWADDR is missing. Advertise both when PushPeerInfo is enabled, generating a stable per-client UUID when the caller does not supply one. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ClientOptions.PushPeerInfomirroring OpenVPN--push-peer-info.UV_UUIDandIV_HWADDRin the TLS key-method peer-info payload (afterIV_TCPNL), matching OpenVPN Connect / community client behavior that enterprise servers often require.PeerUUID/HardwareAddressare empty,NewClientgenerates a UUID v4 and best-effort fills a local MAC; callers can read them viaClient.PeerUUID()/Client.HardwareAddress()and persistPeerUUIDacross restarts.Motivation
Against some corporate OpenVPN deployments (cert auth +
push-peer-info), sing-openvpn completed TLS and then receivedAUTH_FAILEDon PUSH because peer-info lacked device identity fields that OpenVPN Connect sends (UV_UUID, and/orIV_HWADDR). Supplying either field was enough for those servers to accept the session; this change advertises both whenPushPeerInfois enabled.Notes
PushPeerInfo=falseby default) to avoid changing peer-info for existing clients.push-peer-infotoClientOptions.PushPeerInfo.e774474; this PR does not revisit that.Test plan
go test .(unit tests for omit/include identities, UUID generation, oversized rejection)PushPeerInfo: trueagainst a server that logs peer-info / requires Connect-style identityPushPeerInfo: falsekeeps peer-info unchanged vs previous behavior