Skip to content

Commit

Permalink
Fix psid cookie older client failure bug [PG-93]
Browse files Browse the repository at this point in the history
Older clients, 2.5.x and below, send an ACK_V1 packet in response to
the server's HARD_RESET packet whereas 2.6.x clients send a CONTROL_V1
packet.  The code that checked the packet length of the client's
response failed to comprehend the fact the ACK_V1 packet does not
include a packet id field following the peer session id field.  So the
code rejected the ACK_V1 packet as being too short.

The fix was to require packet length only up to and including the peer
session id field.  This works to allow safe parsing for both the
ACK_V1 response and the CONTROL_V1 response.

Signed-off-by: Mark Deric <[email protected]>
  • Loading branch information
Mark Deric authored and dsommers committed Nov 24, 2023
1 parent b928585 commit ecdf041
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions openvpn/ssl/psid_cookie_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,10 @@ class PsidCookieImpl : public PsidCookie

static const size_t reqd_packet_size
// clang-format off
// [op_field] [cli_psid] [HMAC] [cli_auth_pktid] [acked] [srv_psid] [cli_pktid]
= 1 + SID_SIZE + hmac_size + long_pktid_size_ + 5 + SID_SIZE + short_pktid_size_;
// [op_field] [cli_psid] [HMAC] [cli_auth_pktid] [acked] [srv_psid]
= 1 + SID_SIZE + hmac_size + long_pktid_size_ + 5 + SID_SIZE;
// the fixed size, 5, of the [acked] field recognizes that the client's first
// response will ack exactly one packet, the server's HARD_RESET
// clang-format on
if (pkt_buf.size() < reqd_packet_size)
{
Expand Down

0 comments on commit ecdf041

Please sign in to comment.