diff mbox series

[Openvpn-devel] Fix error detection / abort in --inetd corner case.

Message ID 20200908105130.24171-1-gert@greenie.muc.de
State Accepted
Headers
Series [Openvpn-devel] Fix error detection / abort in --inetd corner case. |

Commit Message

Gert Doering Sept. 8, 2020, 12:51 a.m. UTC 
  Calling "openvpn --inetd" from the CLI (= no socket on stdin) will
lead to endless looping in the accept(4) loop.

Instead of cluttering that function further, detect failure to call
getsockame() in phase2_inetd() already, and trigger a M_FATAL abort
on "errno == ENOTSOCK" ("The argument s is a file, not a socket").

While at it, uncrustify the --bind-dev code (whitespace only).

Trac: #350

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/socket.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Comments

Arne Schwabe Sept. 8, 2020, 1:16 a.m. UTC | #1 
Am 08.09.20 um 12:51 schrieb Gert Doering:
> Calling "openvpn --inetd" from the CLI (= no socket on stdin) will
> lead to endless looping in the accept(4) loop.
> 
> Instead of cluttering that function further, detect failure to call
> getsockame() in phase2_inetd() already, and trigger a M_FATAL abort
> on "errno == ENOTSOCK" ("The argument s is a file, not a socket").
> 
> While at it, uncrustify the --bind-dev code (whitespace only).

Yes. Ack. I am happy when we can finally get rid of --inetd.

Acked-By: Arne Schwabe <arne@rfc2549.org>
Gert Doering Sept. 8, 2020, 9:55 p.m. UTC | #2 
Patch has been applied to the master, release/2.5 and release/2.4 branch.

The 2.4 patch only has the actual "--inetd" fix, not the uncrustify whitespace
change for --bind-dev (code not in 2.4).

(As a side note for the protocol: it turns out this only fixes "--inetd nowait",
aka "--proto tcp-server".  Running "openvpn --dev tap --inetd" + pressing return
will still lead to busy spinning in the UDP receive path... next fix coming)

commit a09a2fadbadb5dc435f6fccc581163e1f637f43f (master)
commit 05050e7d089e382e0e8bc7565f30e02e2246d569 (release/2.5)
commit 769c894db29f76f6510569fc20633fb6f0c357f9 (release/2.4)
Author: Gert Doering
Date:   Tue Sep 8 12:51:30 2020 +0200

     Fix error detection / abort in --inetd corner case.

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20200908105130.24171-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20897.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index c486327b..76bdbfc5 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1141,8 +1141,8 @@  create_socket(struct link_socket *sock, struct addrinfo *addr)
 #if defined(TARGET_LINUX)
     if (sock->bind_dev)
     {
-        msg (M_INFO, "Using bind-dev %s", sock->bind_dev);
-        if (setsockopt (sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, strlen (sock->bind_dev) + 1) != 0)
+        msg(M_INFO, "Using bind-dev %s", sock->bind_dev);
+        if (setsockopt(sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, strlen(sock->bind_dev) + 1) != 0)
         {
             msg(M_WARN|M_ERRNO, "WARN: setsockopt SO_BINDTODEVICE=%s failed", sock->bind_dev);
         }
@@ -2030,8 +2030,14 @@  phase2_inetd(struct link_socket *sock, const struct frame *frame,
             }
             else
             {
-                msg(M_WARN, "inetd(%s): getsockname(%d) failed, using AF_INET",
+                int saved_errno = errno;
+                msg(M_WARN|M_ERRNO, "inetd(%s): getsockname(%d) failed, using AF_INET",
                     proto2ascii(sock->info.proto, sock->info.af, false), (int)sock->sd);
+                /* if not called with a socket on stdin, --inetd cannot work */
+                if (saved_errno == ENOTSOCK)
+                {
+                    msg(M_FATAL, "ERROR: socket required for --inetd operation");
+                }
             }
         }
 #else  /* ifdef HAVE_GETSOCKNAME */
@@ -2047,7 +2053,6 @@  phase2_inetd(struct link_socket *sock, const struct frame *frame,
                                  false,
                                  sock->inetd == INETD_NOWAIT,
                                  signal_received);
-
     }
     ASSERT(!remote_changed);
 }