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 5: (3 comments) Patchset: PS5: The change makes sense to me, just some small nitpicks. If you strongly prefer doing it this way, I'm willing to approve. File src/openvpn/mudp.c: http://gerrit.openvpn.net/c/openvpn/+/1067/comment/b8f6989e_8a8a01d2 : 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. File src/openvpn/ssl_pkt.c: http://gerrit.openvpn.net/c/openvpn/+/1067/comment/3d6e4bec_68a33598 : 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. Now check_session_id_hmac() does more than checking the hmac. I'd prefer to extract this check to its own function, or rename this one. -- 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: 5 Gerrit-Owner: plaisthos <arne-open...@rfc2549.org> Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com> Gerrit-CC: MaxF <m...@max-fillinger.net> Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net> Gerrit-Attention: plaisthos <arne-open...@rfc2549.org> Gerrit-Attention: flichtenheld <fr...@lichtenheld.com> Gerrit-Comment-Date: Wed, 13 Aug 2025 14:01:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
