Re: [Openvpn-devel] [PATCH v3] Document common uses of 'echo' directive, re-enable logging for 'echo'.

2021-01-18 Thread Selva Nair
Hi.

Looks good to me.

The white- "space police" may object to the formatting in the logging
code (see below). Could be fixed at commit time.

On Mon, Jan 18, 2021 at 11:31 AM Gert Doering  wrote:
>
> The 'echo' command can be used to signal information to an OpenVPN
> GUI driving the openvpn core via management interface.  Which commands
> exists and their syntax has so far been mostly undocumented.
>
> Condense the long and good discussion between Selva Nair and
> Jonathan K. Bullard into doc/gui-notes.txt (initial draft from
> Jonathan, comments from Selva and Arne), with a pointer added
> to doc/management-notes.txt.
>
> See:
>
> https://sourceforge.net/p/openvpn/mailman/openvpn-users/thread/CAEsd45T%2Bd6FUJ9Po0KHwtHjfuL9Q2D-poG8yFtY45Qyh%2BtHjkg%40mail.gmail.com/#msg36136236
>
> and
>
> https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/CAKuzo_jPThhvXTJAtzhqVUVOLPW1VGu6h2jQhVsHicY8P2WRqA%40mail.gmail.com/#msg36141193
>
> for the details.
>
> Re-enable logging of 'echo' statements, but only for the particular
> class of messages starting with 'echo msg...'.
>
> v2:
>   incorporate feedback from Selva Nair, correct >ECHO examples
>
> v3:
>   add "msg*" support status for Windows GUI (11.22.0) and Android (Planned)
>
> Signed-off-by: Gert Doering 
> ---
>  doc/Makefile.am  |   2 +-
>  doc/gui-notes.txt| 370 +++
>  doc/management-notes.txt |  10 ++
>  src/openvpn/options.c|  15 +-
>  4 files changed, 389 insertions(+), 8 deletions(-)
>  create mode 100644 doc/gui-notes.txt
>
> diff --git a/doc/Makefile.am b/doc/Makefile.am
> index df2f54a3..e411f5f9 100644
> --- a/doc/Makefile.am
> +++ b/doc/Makefile.am
> @@ -15,7 +15,7 @@ MAINTAINERCLEANFILES = \
>  SUBDIRS = doxygen
>
>  dist_doc_DATA = \
> -   management-notes.txt
> +   management-notes.txt gui-notes.txt
>
>  dist_noinst_DATA = \
> README.plugins interactive-service-notes.rst \
> diff --git a/doc/gui-notes.txt b/doc/gui-notes.txt
> new file mode 100644
> index ..08270f07
> --- /dev/null
> +++ b/doc/gui-notes.txt
> @@ -0,0 +1,370 @@
> +Management Interface "echo" protocol
> +
>

snipped...

+COMMAND -- setenv
+-
+
+Syntax: setenv name value
+
+Sets an environment variable that will be available to the scripts run
+by the GUI.
+
+This will set environment variable "OPENVPN_name" to value "value" for
+the scripts run by the GUI. "name" is changed to "OPENVPN_name" to
+prevent overwriting sensitive variables such as PATH. Variables are
+set in the order received, with later values replacing earlier ones
+for the same "name".
+
+Names may include only alphanumeric characters and underscores. A
+"setenv" command with an invalid name will be ignored.
+
+Android: ??
+
+Tunnelblick: Planned.
+
+Windows OpenVPN GUI: supported since release v2.4.7 (GUI version v11.12.0)
+The variables set by "setenv" are merged with those for the process
+environment.  In case of duplicate names the one in the setenv list is
+chosen.
+

The extra blank line at the end of file is unnecessary (whitespace
error according to git).

> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index ff3954d5..36009f4f 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -5306,13 +5306,14 @@ add_option(struct options *options,
>  }
>  if (good)
>  {
> -#if 0
> -/* removed for now since ECHO can potentially include
> - * security-sensitive strings */
> -msg(M_INFO, "%s:%s",
> -pull_mode ? "ECHO-PULL" : "ECHO",
> -BSTR(&string));
> -#endif
> +/* only message-related ECHO are logged, since other ECHOs
> + * can potentially include security-sensitive strings */
> +if (strncmp(p[1],"msg",3) == 0)

Missing space between function arguments.

> +{
> +msg(M_INFO, "%s:%s",
> +pull_mode ? "ECHO-PULL" : "ECHO",
> +BSTR(&string));
> +}
>  #ifdef ENABLE_MANAGEMENT
>  if (management)
>  {
> --
> 2.26.2

I have only compile tested as the code change looks sane and simple.

Acked by: selva.n...@gmail.com

Thanks,

Selva


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH v3] Document common uses of 'echo' directive, re-enable logging for 'echo'.

2021-01-18 Thread Gert Doering
The 'echo' command can be used to signal information to an OpenVPN
GUI driving the openvpn core via management interface.  Which commands
exists and their syntax has so far been mostly undocumented.

Condense the long and good discussion between Selva Nair and
Jonathan K. Bullard into doc/gui-notes.txt (initial draft from
Jonathan, comments from Selva and Arne), with a pointer added
to doc/management-notes.txt.

See:

https://sourceforge.net/p/openvpn/mailman/openvpn-users/thread/CAEsd45T%2Bd6FUJ9Po0KHwtHjfuL9Q2D-poG8yFtY45Qyh%2BtHjkg%40mail.gmail.com/#msg36136236

and

https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/CAKuzo_jPThhvXTJAtzhqVUVOLPW1VGu6h2jQhVsHicY8P2WRqA%40mail.gmail.com/#msg36141193

for the details.

Re-enable logging of 'echo' statements, but only for the particular
class of messages starting with 'echo msg...'.

v2:
  incorporate feedback from Selva Nair, correct >ECHO examples

v3:
  add "msg*" support status for Windows GUI (11.22.0) and Android (Planned)

Signed-off-by: Gert Doering 
---
 doc/Makefile.am  |   2 +-
 doc/gui-notes.txt| 370 +++
 doc/management-notes.txt |  10 ++
 src/openvpn/options.c|  15 +-
 4 files changed, 389 insertions(+), 8 deletions(-)
 create mode 100644 doc/gui-notes.txt

diff --git a/doc/Makefile.am b/doc/Makefile.am
index df2f54a3..e411f5f9 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -15,7 +15,7 @@ MAINTAINERCLEANFILES = \
 SUBDIRS = doxygen
 
 dist_doc_DATA = \
-   management-notes.txt
+   management-notes.txt gui-notes.txt
 
 dist_noinst_DATA = \
README.plugins interactive-service-notes.rst \
diff --git a/doc/gui-notes.txt b/doc/gui-notes.txt
new file mode 100644
index ..08270f07
--- /dev/null
+++ b/doc/gui-notes.txt
@@ -0,0 +1,370 @@
+Management Interface "echo" protocol
+
+
+THIS IS A PRELIMINARY VERSION OF THIS DOCUMENT. ALL INFORMATION IN IT
+IS SUBJECT TO CHANGE.
+
+
+
+CONTENTS
+THE OPENVPN --ECHO OPTION
+ENVIRONMENT COMMAND
+MESSSAGE COMMANDS
+PASSWORD COMMANDS
+QUOTING
+COMMMAND DETAILS
+
+
+=
+THE OPENVPN --ECHO OPTION
+=
+
+The OpenVPN --echo option causes commands to be sent out through the
+management interface, typically to a Graphic User Interface (GUI) such
+as "OpenVPN for Android", "Tunnelblick" (for macOS), or "Windows
+OpenVPN GUI". It can be included in a configuration file or on a
+command line, or can be pushed from the server.
+
+This document describes the commands that can be sent and how they are
+interpreted by various GUIs.
+
+ * OpenVPN does not process the commands in an --echo option; it only
+sends them out through the management interface.
+
+ * "echo" commands are processed by the GUI if, as, when, and in the
+order they are received. If no GUI is present the processing of
+commands may be delayed, the commands may never be processed, or only
+some commands may be processed. (That can happen if OpenVPN discards
+commands because its buffer for the commands fills up.)
+
+ * There is no mechanism for the GUI to acknowledge the receipt,
+success, or failure of a command.
+
+ * "echo" commands are stored by OpenVPN (within limits, see the next
+point) and sent only when the GUI requests them through the management
+interface. "echo" commands in the configuration file or the command
+line are typically requested and processed at the start of a
+connection attempt. "echo" commands that are pushed by the server are
+also typically asked for at the start of a connection attempt but can
+be sent at any time. They are processed in the middle of a connection
+attempt or after a connection is established, as the "push" options
+are received by the client from the server.
+
+  * OpenVPN's storage for echo commands is limited in size, so a large
+number of commands or commands with long messages may require that
+some commands be removed from the storage. If that happens, some of
+the commands may not be sent through the management interface when a
+GUI does connect to it or asks for the "echo" commands.
+
+ * On SIGUSR1 and SIGHUP connection restarts, "echo" commands that
+were sent through the management interface and have been saved by
+OpenVPN are sent again and will be re-processed by the GUI. (The
+message commands include a mechanism for muting (skipping) duplicate
+messages, see MESSAGE COMMANDS, below.)
+
+ * OpenVPN limits the number of separate arguments in each line of a
+configuration file. Arguments may be quoted to work around this
+limitation, see QUOTING, below.
+
+ * OpenVPN limits the size of each "echo" command sent over the
+management interface to 255 bytes, including overhead characters. To
+allow messages of arbitrary length, several message commands can be