Skip to contentSkip to conversation

tcp-server: ensure AF family is propagated to child context

PATCH In review Antonio Quartulli 3 participants 3 messages Sep 7, 2017 – Sep 21, 2017

lore
Share & tools
lore

Antonio Quartulli

author
#0 lore raw

PATCH 1/1 tcp-server: ensure AF family is propagated to child context

From: Antonio Quartulli <[email protected]...>

Commit 23d61c56 introduced the AF_UNSPEC socket family to be used when we don't know the actual one until the local socket binding is performed.

In such case AF_UNSPEC is stored in the ce.af member of the c->options object, indicating that the family has to be determined at runtime.

However, the determined value is never propagated back to the options object, which remains AF_UNSPEC and that is later used to initialize the TCP children contexts (UDP children contexts are unaffected).

This unexpected setting can trigger weird behaviours, like the one reported in ticket #933.
In this case the value AF_UNSPEC in combination with the changes implemented in 2bed089d are leading to a TCP server quitting with M_FATAL upon client connection.

Note that the misbehaviour described in #933 can only be triggered when running a TCP server with mtu-disc set in the config (no matter the value).

Fix this inconsistency by always propagating the AF family from the top to the child context when running in TCP server mode.

As a direct consequence, this patch fixes Trac #933.

Signed-off-by Antonio Quartulli <[email protected]...>
---
 src/openvpn/socket.c | 2 ++
 1 file changed, 2 insertions(+)
src/openvpn/socket.c +2 -0
index 3d4f881e..0fc91f21 100644
@@ -1786,6 +1786,8 @@ link_socket_init_phase1(struct link_socket *sock,
         ASSERT(sock->info.proto == PROTO_TCP_SERVER);
         ASSERT(!sock->inetd);
         sock->sd = accept_from->sd;
+        /* inherit (possibly guessed) info AF from parent context */
+        sock->info.af = accept_from->info.af;
     }
 
     /* are we running in HTTP proxy mode? */
Signature · 1 line
2.14.1
#1 lore raw

Am 07.09.17 um 11:55 schrieb Antonio Quartulli:

Quoted 21 lines

From: Antonio Quartulli <[email protected]...>

Commit 23d61c56 introduced the AF_UNSPEC socket family to be used when we don't know the actual one until the local socket binding is performed.

In such case AF_UNSPEC is stored in the ce.af member of the c->options object, indicating that the family has to be determined at runtime.

However, the determined value is never propagated back to the options object, which remains AF_UNSPEC and that is later used to initialize the TCP children contexts (UDP children contexts are unaffected).

This unexpected setting can trigger weird behaviours, like the one reported in ticket #933.
In this case the value AF_UNSPEC in combination with the changes implemented in 2bed089d are leading to a TCP server quitting with M_FATAL upon client connection.

Quoted 8 lines

Note that the misbehaviour described in #933 can only be triggered when running a TCP server with mtu-disc set in the config (no matter the value).

Fix this inconsistency by always propagating the AF family from the top to the child context when running in TCP server mode.

ACK. That is a really weird corner case that I must have missed :)

Arne

#2 lore raw
In reply to Antonio Quartulli

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Your patch has been applied to the following branches

commit 682e7feac3bd57e6ce7e60504cb4da5c894d0e18 (master) commit b3b7d073ce05fa6b11a28f9e70d66c4907274db5 (release/2.4)

Author: Antonio Quartulli
Date:   Thu Sep 7 17:55:30 2017 +0800
     tcp-server: ensure AF family is propagated to child context

     Trac: 933
     Signed-off-by: Antonio Quartulli <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg15380.html
     Signed-off-by: David Sommerseth <[email protected]...>

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCgAGBQJZxFAuAAoJEIbPlEyWcf3yLi0QAKqWdmij1JLt2kA7Ww0Qcgkc pixB3s0Ir5J51ZIOodYlcN8hyoRXCdLe6EX3mjumZEMvwJ7XWDa7yrr3t9WZBgLN lpW1M8jCvlevBc0sLNguFmQn++3ZDYhXN1HiDLep7hdIQrHaNGf4n4VRMtZjlITz h2xyVUDTFCuCXFDAUFIxhYngdfdn3ySWXu6RmgK4MewMGoZ8qIPrVHgjsqvBpFUr /tTnfMxZQ+fdmSth2MfWbGRapW5E7LXv2EdRiqPnhG9mhZABUHKWbL74MdUaU2yV h2dW209/ufQ9GfviRy2pLLc7XsE5LljoRlkYsB9NlDw7dA1NyVYTq6XrgcZcngbg pSFllWqVFPBvt5l7nBLY8ZcaCezFV56QaOmor8moDiKAEty0cPlweqIjCFIQakap POcBcDrzX1i5ZDC7/njCeri/KHqhhGI1irWCzJCCqdHhG0zOOCfLyruaeINqoD3A Uke/QmTzO5C6opIbGgIzalcF94BucgKFeLxsAfLuvknZxmIfS46u/OWUxUKTDzaN 8322lkeTSntCldxY0mL9b3mfhkcUsycFl88QAALIfda5Aea5aTvfYuIrzkC1D22N /yoVZ0S2+tLnU607PTYxkqGNLbxGaovN/hUfoXU9DIP4alnsPrR30ZrPVCXxM9By Ul8auE5SiGSDBdPctczF
=8SRp
-----END PGP SIGNATURE-----