diff mbox series

[Openvpn-devel] Include CE_DISABLED status of remote in "remote-entry-get" response

Message ID 20230111062910.1846688-1-selva.nair@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] Include CE_DISABLED status of remote in "remote-entry-get" response | expand

Commit Message

Selva Nair Jan. 11, 2023, 6:29 a.m. UTC 
From: Selva Nair <selva.nair@gmail.com>

- The response to the management command "remote-entry-get" is
  amended to include the status of the remote entry. The status
  reads "disabled" if (ce->flag & DISABLED) is true, "enabled"
  otherwise.

- Update and correct the description of this option in
  management-notes.txt

  Example responses:
  In response to "remote-entry-get 0"

  0,vpn.example.com,udp,enabled
  END

  Or, in response to "remote-entry-get all"

  0,vpn.example.org,udp,enabled
  2,vpn.example.net,tcp-client,disabled
  1,vpn.example.com,udp,enabled
  END

This helps the management client to show only enabled remotes
to the user.
An alternative would require the  UI/GUI to have knowledge of
what makes the daemon set CE_DISABLED (--proto-force,
--htttp-proxy-override etc.).

Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 doc/management-notes.txt | 23 +++++++++++++----------
 src/openvpn/init.c       |  8 +++++---
 2 files changed, 18 insertions(+), 13 deletions(-)

Comments

Selva Nair Jan. 11, 2023, 6:41 a.m. UTC | #1 
Error in commit message:

  0,vpn.example.org,udp,enabled
>   2,vpn.example.net,tcp-client,disabled
>   1,vpn.example.com,udp,enabled
>

That should have been

  0,vpn.example.org,udp,enabled
  1,vpn.example.net,tcp-client,disabled
  2,vpn.example.com,udp,enabled

with indices 0, 1, 2 ordered.
Gert Doering Jan. 11, 2023, 7:32 a.m. UTC | #2 
Acked-by: Gert Doering <gert@greenie.muc.de>

This is really straightforward.  Tested with my .ovpn full with generated
"remote" lines, some of them changed to "tcp", and "--proto-force tcp-client"

..
190,1185.server.org,1185,udp,disabled
191,1186.server.org,1186,udp,disabled
192,1187.server.org,1187,udp,disabled
193,1188.server.org,1188,tcp-client,enabled
194,1189.server.org,1189,tcp-client,enabled
195,1190.server.org,1190,tcp-client,enabled

(NOTE: "--proto-force tcp" will surprisingly - to me - disable *everything*,
because "client side TCP profiles are 'tcp-client', not 'tcp'")


This said, the code with all these strlen() and malloc() calls for every
single remote looks fumbly...  given that it's not exactly geared for
maxmimum memory efficientcy, I'm tempted to suggest just having an
on-the-stack "char remote[1000];" in man_remote_entry_get(), and pass
a pointer to that + sizeof() to "...callback.remote_entry_count()".

But this is something for the next round of cleanup in early 2.7.


I have reordered the example lines in the commit message.

Your patch has been applied to the master and release/2.6 branch.

commit eafbedc583c48fd46405fa0d635c688ce59c3733 (master)
commit 69383817b9805137460c0e25ede4b18573cac01d (release/2.6)
Author: Selva Nair
Date:   Wed Jan 11 01:29:10 2023 -0500

     Include CE_DISABLED status of remote in remote-entry-get response

     Signed-off-by: Selva Nair <selva.nair@gmail.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20230111062910.1846688-1-selva.nair@gmail.com>
     URL: https://www.mail-archive.com/search?l=mid&q=20230111062910.1846688-1-selva.nair@gmail.com
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 6daa811a..34f301db 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -806,9 +806,12 @@  COMMAND -- remote-entry-get (OpenVPN 2.6+ management version > 3)
 
   remote-entry-get <start> [<end>]
 
-Retrieve remote entry (host, port and protocol) for index
-<start> or indices from <start> to <end>+1. Alternatively
-<start> = "all" retrieves all remote entries.
+Retrieve remote entry (host, port, protocol, and status) for index
+<start> or indices from <start> to <end>-1. Alternatively
+<start> = "all" retrieves all remote entries. The index is 0-based.
+If the entry is disabled due to protocol or proxy restrictions
+(i.e., ce->flag & CE_DISABLED == 1), the status is returned as "disabled",
+otherwise it reads "enabled" without quotes.
 
 Example 1:
 
@@ -818,8 +821,8 @@  Example 1:
 
   OpenVPN daemon responds with
 
-  1,vpn.example.com,1194,udp
-  END
+    1,vpn.example.com,1194,udp,enabled
+    END
 
 Example 2:
 
@@ -829,8 +832,8 @@  Example 2:
 
   OpenVPN daemon responds with
 
-    1,vpn.example.com,1194,udp
-    2,vpn.example.net,443,tcp-client
+    1,vpn.example.com,1194,udp,enabled
+    2,vpn.example.net,443,tcp-client,disabled
     END
 
 Example 3:
@@ -840,9 +843,9 @@  Example 3:
 
   OpenVPN daemon with 3 connection entries responds with
 
-    1,vpn.example.com,1194,udp
-    2,vpn.example.com,443,tcp-client
-    3,vpn.example.net,443,udp
+    0,vpn.example.com,1194,udp,enabled
+    1,vpn.example.com,443,tcp-client,enabled
+    2,vpn.example.net,443,udp,enabled
     END
 
 COMMAND -- remote  (OpenVPN AS 2.1.5/OpenVPN 2.3 or higher)
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index fc1943bc..c8651232 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -353,13 +353,15 @@  management_callback_remote_entry_get(void *arg, unsigned int index, char **remot
     {
         struct connection_entry *ce = l->array[index];
         const char *proto = proto2ascii(ce->proto, ce->af, false);
+        const char *status = (ce->flags & CE_DISABLED) ? "disabled" : "enabled";
 
-        /* space for output including 2 commas and a nul */
-        int len = strlen(ce->remote) + strlen(ce->remote_port) + strlen(proto) + 2 + 1;
+        /* space for output including 3 commas and a nul */
+        int len = strlen(ce->remote) + strlen(ce->remote_port) + strlen(proto)
+                  + strlen(status) + 3 + 1;
         char *out = malloc(len);
         check_malloc_return(out);
 
-        openvpn_snprintf(out, len, "%s,%s,%s", ce->remote, ce->remote_port, proto);
+        openvpn_snprintf(out, len, "%s,%s,%s,%s", ce->remote, ce->remote_port, proto, status);
         *remote = out;
     }
     else