[S] Change in openvpn[master]: ssl: Ignore hard reset packets with a non-zero packet id
"flichtenheld \(Code Review\) via Openvpn-devel" <[email protected]> Thu, 30 Jul 2026 09:03:47 +0000
| Newsgroups | gmane.network.openvpn.devel |
|---|---|
| Message-ID | <[email protected]> |
--===============6620241564073878995==
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
Content-Type: multipart/alternative; boundary="usCEnIUwvig="; charset=UTF-8
--usCEnIUwvig=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Attention is currently required from: plaisthos=2E
Hello plaisthos,
I'd l=
ike you to do a code review=2E
Please visit
http://gerrit=2Eopenvpn=2E=
net/c/openvpn/+/1832?usp=3Demail
to review the following change=2E
Chang=
e subject: ssl: Ignore hard reset packets with a non-zero packet id
=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E
ssl: Ignore hard re=
set packets with a non-zero packet id
A hard reset is always the first pac=
ket of a session, so it always
carries reliable packet id 0=2E tls_process_=
state() relies on that when it
treats a received reset as the early negotia=
tion packet only for packet
id 0, and the stateless three-way handshake rel=
ies on it as well (see
the comment in session_skip_to_pre_start())=2E
A re=
set claiming a different id is therefore bogus=2E
We had a bug that could =
cause hard reset replays with packet id 1 in
specific scenarios (P2P TCP)=
=2E In that case we accepted the packet id
at face value and then ignored t=
he control packet that actually had
id 1 as an replay=2E This caused a diff=
icult to diagnose dead connection
that was stuck just before TLS negotiatio=
n=2E The check added handles
this specific scenario well in that we just ig=
nore the bogus reset
but do not abort the connection attempt=2E Starting fr=
esh might retrigger
the bug=2E If there would be a separate bug where the c=
lient only sends
hard resets with packet id 1 we will still get logging on =
the server
side now=2E
Change-Id: I3c7d1f9e5b2a4c6d8e1f3a5b7c9d2e4f6a8b1c3=
d
Signed-off-by: Frank Lichtenheld <frank@lichtenheld=2Ecom>
---
M src/open=
vpn/ssl=2Ec
1 file changed, 15 insertions(+), 1 deletion(-)
git pull s=
sh://gerrit=2Eopenvpn=2Enet:29418/openvpn refs/changes/32/1832/1
diff --gi=
t a/src/openvpn/ssl=2Ec b/src/openvpn/ssl=2Ec
index 7f2e850=2E=2E46ccc12 10=
0644
--- a/src/openvpn/ssl=2Ec
+++ b/src/openvpn/ssl=2Ec
@@ -3903,8 +3903,2=
2 @@
/* Extract the packet ID from the packet */
if (reli=
able_ack_read_packet_id(buf, &id))
{
+ /* A hard reset =
always is the first packet of a session, so it
+ * always must =
use packet id 0=2E Ignore it if it claims another id=2E
+ * In =
a specific existing bug these packets were replays of an
+ * al=
ready handled reset, so ignoring it is better than aborting
+ *=
the connection attempt=2E
+ */
+ if (is_hard_reset_=
method2(op) && id !=3D 0)
+ {
+ msg(D_TLS_ERRORS,=
+ "TLS Error: received %s with packet id " packet_id_fo=
rmat
+ " from %s -- 0 was expected, ignoring packet",
+ =
packet_opcode_name(op), (packet_id_print_type)id,
+ =
print_link_socket_actual(from, &gc));
+ }
=
/* Avoid deadlock by rejecting packet that would de-sequentialize rec=
eive buffer */
- if (reliable_wont_break_sequentiality(ks->rec_r=
eliable, id))
+ else if (reliable_wont_break_sequentiality(ks->r=
ec_reliable, id))
{
if (reliable_not_replay(k=
s->rec_reliable, id))
{
--
To view, visit http://gerrit=
=2Eopenvpn=2Enet/c/openvpn/+/1832?usp=3Demail
To unsubscribe, or for help w=
riting mail filters, visit http://gerrit=2Eopenvpn=2Enet/settings?usp=3Dema=
il
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: ma=
ster
Gerrit-Change-Id: I3c7d1f9e5b2a4c6d8e1f3a5b7c9d2e4f6a8b1c3d
Gerrit-Cha=
nge-Number: 1832
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <frank@licht=
enheld=2Ecom>
Gerrit-Reviewer: plaisthos <arne-openvpn@rfc2549=2Eorg>
Gerri=
t-CC: openvpn-devel <openvpn-devel@lists=2Esourceforge=2Enet>
Gerrit-Attent=
ion: plaisthos <arne-openvpn@rfc2549=2Eorg>
--usCEnIUwvig=
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
<!DOCTYPE html><html><head><style></style></head><body><p> Attention is cur=
rently required from: plaisthos=2E </p>
<p>flichtenheld would like plaistho=
s to <strong>review</strong> this change=2E
</p><p><a href=3D"http://gerrit=
=2Eopenvpn=2Enet/c/openvpn/+/1832?usp=3Demail">View Change</a></p><pre clas=
s=3D"blocks" style=3D"font-family: monospace,monospace; white-space: pre-wr=
ap;">ssl: Ignore hard reset packets with a non-zero packet id<br><br>A hard=
reset is always the first packet of a session, so it always<br>carries rel=
iable packet id 0=2E tls_process_state() relies on that when it<br>treats a=
received reset as the early negotiation packet only for packet<br>id 0, an=
d the stateless three-way handshake relies on it as well (see<br>the commen=
t in session_skip_to_pre_start())=2E<br><br>A reset claiming a different id=
is therefore bogus=2E<br><br>We had a bug that could cause hard reset repl=
ays with packet id 1 in<br>specific scenarios (P2P TCP)=2E In that case we =
accepted the packet id<br>at face value and then ignored the control packet=
that actually had<br>id 1 as an replay=2E This caused a difficult to diagn=
ose dead connection<br>that was stuck just before TLS negotiation=2E The ch=
eck added handles<br>this specific scenario well in that we just ignore the=
bogus reset<br>but do not abort the connection attempt=2E Starting fresh m=
ight retrigger<br>the bug=2E If there would be a separate bug where the cli=
ent only sends<br>hard resets with packet id 1 we will still get logging on=
the server<br>side now=2E<br><br>Change-Id: I3c7d1f9e5b2a4c6d8e1f3a5b7c9d2=
e4f6a8b1c3d<br>Signed-off-by: Frank Lichtenheld <frank@lichtenheld=2Ecom=
><br>---<br>M src/openvpn/ssl=2Ec<br>1 file changed, 15 insertions(+), 1=
deletion(-)<br><br></pre><pre class=3D"blocks" style=3D"font-family: monos=
pace,monospace; white-space: pre-wrap;">git pull ssh://gerrit=2Eopenvpn=2En=
et:29418/openvpn refs/changes/32/1832/1</pre><pre style=3D"font-family: mon=
ospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/openvpn/ss=
l=2Ec b/src/openvpn/ssl=2Ec</span><br><span>index 7f2e850=2E=2E46ccc12 1006=
44</span><br><span>--- a/src/openvpn/ssl=2Ec</span><br><span>+++ b/src/open=
vpn/ssl=2Ec</span><br><span>@@ -3903,8 +3903,22 @@</span><br><span> =
/* Extract the packet ID from the packet */</span><br><span> if (r=
eliable_ack_read_packet_id(buf, &id))</span><br><span> {</span>=
<br><span style=3D"color: hsl(120, 100%, 40%);">+ /* A hard rese=
t always is the first packet of a session, so it</span><br><span style=3D"c=
olor: hsl(120, 100%, 40%);">+ * always must use packet id 0=2E =
Ignore it if it claims another id=2E</span><br><span style=3D"color: hsl(12=
0, 100%, 40%);">+ * In a specific existing bug these packets we=
re replays of an</span><br><span style=3D"color: hsl(120, 100%, 40%);">+ =
* already handled reset, so ignoring it is better than aborting</=
span><br><span style=3D"color: hsl(120, 100%, 40%);">+ * the co=
nnection attempt=2E</span><br><span style=3D"color: hsl(120, 100%, 40%);">+=
*/</span><br><span style=3D"color: hsl(120, 100%, 40%);">+ =
if (is_hard_reset_method2(op) && id !=3D 0)</span><br><span=
style=3D"color: hsl(120, 100%, 40%);">+ {</span><br><span style=
=3D"color: hsl(120, 100%, 40%);">+ msg(D_TLS_ERRORS,</span><=
br><span style=3D"color: hsl(120, 100%, 40%);">+ "T=
LS Error: received %s with packet id " packet_id_format</span><br><spa=
n style=3D"color: hsl(120, 100%, 40%);">+ " from %s=
-- 0 was expected, ignoring packet",</span><br><span style=3D"color: =
hsl(120, 100%, 40%);">+ packet_opcode_name(op), (packet_=
id_print_type)id,</span><br><span style=3D"color: hsl(120, 100%, 40%);">+ =
print_link_socket_actual(from, &gc));</span><br><span=
style=3D"color: hsl(120, 100%, 40%);">+ }</span><br><span> =
/* Avoid deadlock by rejecting packet that would de-sequentialize r=
eceive buffer */</span><br><span style=3D"color: hsl(0, 100%, 40%);">- =
if (reliable_wont_break_sequentiality(ks->rec_reliable, id))</spa=
n><br><span style=3D"color: hsl(120, 100%, 40%);">+ else if (rel=
iable_wont_break_sequentiality(ks->rec_reliable, id))</span><br><span> =
{</span><br><span> if (reliable_not_replay(ks-&g=
t;rec_reliable, id))</span><br><span> {</span><br><span></s=
pan><br></pre><p>To view, visit <a href=3D"http://gerrit=2Eopenvpn=2Enet/c/=
openvpn/+/1832?usp=3Demail">change 1832</a>=2E To unsubscribe, or for help =
writing mail filters, visit <a href=3D"http://gerrit=2Eopenvpn=2Enet/settin=
gs?usp=3Demail">settings</a>=2E</p><div itemscope itemtype=3D"http://schema=
=2Eorg/EmailMessage"><div itemscope itemprop=3D"action" itemtype=3D"http://=
schema=2Eorg/ViewAction"><link itemprop=3D"url" href=3D"http://gerrit=2Eope=
nvpn=2Enet/c/openvpn/+/1832?usp=3Demail"/><meta itemprop=3D"name" content=
=3D"View Change"/></div></div>
<div style=3D"display:none"> Gerrit-Message=
Type: newchange </div>
<div style=3D"display:none"> Gerrit-Project: openvpn=
</div>
<div style=3D"display:none"> Gerrit-Branch: master </div>
<div styl=
e=3D"display:none"> Gerrit-Change-Id: I3c7d1f9e5b2a4c6d8e1f3a5b7c9d2e4f6a8b=
1c3d </div>
<div style=3D"display:none"> Gerrit-Change-Number: 1832 </div>
=
<div style=3D"display:none"> Gerrit-PatchSet: 1 </div>
<div style=3D"displa=
y:none"> Gerrit-Owner: flichtenheld <frank@lichtenheld=2Ecom> </div>
=
<div style=3D"display:none"> Gerrit-Reviewer: plaisthos <arne-openvpn@rf=
c2549=2Eorg> </div>
<div style=3D"display:none"> Gerrit-CC: openvpn-deve=
l <openvpn-devel@lists=2Esourceforge=2Enet> </div>
<div style=3D"disp=
lay:none"> Gerrit-Attention: plaisthos <arne-openvpn@rfc2549=2Eorg> <=
/div>
</body></html>
--usCEnIUwvig=--
--===============6620241564073878995==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
--===============6620241564073878995==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
--===============6620241564073878995==--