[Openvpn-devel] [M] Change in openvpn[master]: Check message id/acked ids too when doing sessionid cookie checks

[MaxF (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

MaxF has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1067?usp=email )

Change subject: Check message id/acked ids too when doing sessionid cookie checks
......................................................................


Patch Set 6: Code-Review+2

(2 comments)

File src/openvpn/mudp.c:

http://gerrit.openvpn.net/c/openvpn/+/1067/comment/a89a1941_de7cae02 :
PS5, Line 163:             msg(D_MULTI_MEDIUM, "Packet (%s) with invalid or missing SID from %s",
             :                 packet_opcode_name(op), peer);
> This debug message may now be incorrect. The packet might have a valid SID, but a wrong packet ID.
Done


File src/openvpn/ssl_pkt.c:

http://gerrit.openvpn.net/c/openvpn/+/1067/comment/333f0fff_e4904c7d :
PS5, Line 518:     /* Check if the packet ID of the packet or ACKED packet  is <= 1 */
             :     for (int i = 0; i < ack.len; i++)
             :     {
             :         /* This packet ACKs a packet that has a higher packet id than the
             :          * ones expected in the three-way handshake, consider it as invalid
             :          * for the session */
             :         if (ack.packet_id[i] > 1)
             :         {
             :             return false;
             :         }
             :     }
             : 
             :     if (!pkt_is_ack)
             :     {
             :         packet_id_type message_id;
             :         /* Extract the packet ID from the packet */
             :         if (!reliable_ack_read_packet_id(&buf, &message_id))
             :         {
             :             return false;
             :         }
             : 
             :         /* similar check. Anything larger than 1 is not considered part of the
             :          * three-way handshake */
             :         if (message_id > 1)
             :         {
             :             return false;
             :         }
             :     }
> Maybe nitpicky, but this seems like scope creep for this function. […]
Done



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1067?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I6752dcd5aff3e5cea2b439366479e86751a1c403
Gerrit-Change-Number: 1067
Gerrit-PatchSet: 6
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: MaxF <ma...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Tue, 19 Aug 2025 13:59:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: MaxF <ma...@ma...>
Gerrit-MessageType: comment