Skip to contentSkip to conversation

Reworking the interface for querying users

PATCH Superseded David Sommerseth 1 participant 5 messages Nov 16, 2014

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover Reworking the interface for querying users

From: David Sommerseth <[email protected]...>

These patches enhances the mechanism in OpenVPN which queries the user for username, passwords, PIN codes, etc. When the management interface is not in use, the current behaviour is to ask for this information sequentially via the console OpenVPN is running on.

The first patch will introduce a more modular approach which also standardizes an API for bulk operations. This means that it is possible to implement more advanced mechanisms asking for both username and passwords at the same time through a single call in the core OpenVPN code. This patch also temporarily removes the systemd implementation.

The second patch will re-implements systemd support using this new API with the same functionality as before this patch set.

The third patch is a re-submission of an earlier patch, which is needed by the fourth patch. It is sent separately, as it provides extra version information on a general basis and is not strictly connected to systemd.

The fourth and last patch is a rebased patch of an earlier submission, removing the masking of usernames. This only happens when the proper systemd version which supports an --echo argument is found.

All patches has been tested locally with different configurations, requiring username, password and passphrases to PKCS#12 files. The challenge/response interface has not been tested, as well as PKCS#11. All runs via valgrind shows no memory leaks with this new implementation.

David Sommerseth (4):
Rework the user input interface to make it more modular Re-implement the systemd support using the new query user API
autotools: Add support for extracting version information from
pkg-config modules
systemd: Do not mask usernames when querying for it via
systemd-ask-password

 configure.ac                  |   3 +-
 m4/pkg.m4                     |   7 +-
 src/openvpn/Makefile.am       |  15 ++-
 src/openvpn/console.c         | 228 ++++++---------------------------------
 src/openvpn/console.h         |  92 +++++++++++++++-
 src/openvpn/console_builtin.c | 246 ++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/console_systemd.c |  89 +++++++++++++++
 src/openvpn/misc.c            |  79 +++++++++++---
 src/openvpn/pkcs11.c          |  14 ++-
 9 files changed, 558 insertions(+), 215 deletions(-)
 create mode 100644 src/openvpn/console_builtin.c
 create mode 100644 src/openvpn/console_systemd.c
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH 1/4 Rework the user input interface to make it more modular

From: David Sommerseth <[email protected]...>

This is will provide an interface for other mechanisms to be used to query the user for information, such as usernames, passwords, etc.

It has also been a goal to make it possible to query for all the information in one call and not do it sequencially as before.

---
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.c         | 228 ++++++---------------------------------
 src/openvpn/console.h         |  92 +++++++++++++++-
 src/openvpn/console_builtin.c | 246 ++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/misc.c            |  79 +++++++++++---
 src/openvpn/pkcs11.c          |  14 ++-
 6 files changed, 449 insertions(+), 212 deletions(-)
 create mode 100644 src/openvpn/console_builtin.c
src/openvpn/Makefile.am +1 -1
index d089f50..e07389f 100644
@@ -69,7 +69,7 @@ openvpn_SOURCES = \
 	memdbg.h \
 	misc.c misc.h \
 	platform.c platform.h \
-	console.c console.h \
+	console.c console.h console_builtin.c \
 	mroute.c mroute.h \
 	mss.c mss.h \
 	mstats.c mstats.h \
src/openvpn/console.c +35 -193

254 lines not on this page · raw message

src/openvpn/console.h +90 -2

104 lines not on this page · raw message

src/openvpn/console_builtin.c new file +246 -0

246 lines not on this page · raw message

src/openvpn/misc.c +65 -14

128 lines not on this page · raw message

src/openvpn/pkcs11.c +12 -2
index 3a15ef6..75ac392 100644
@@ -6,6 +6,7 @@
  *             packet compression.
  *
  *  Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <[email protected]...>
+ *  Copyright (C) 2014      David Sommerseth <[email protected]...>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2
@@ -740,6 +741,7 @@ _pkcs11_openvpn_show_pkcs11_ids_pin_prompt (
 	const size_t pin_max
 ) {
 	struct gc_arena gc = gc_new ();
+	struct query_user *qucfg = NULL;
 	struct buffer pass_prompt = alloc_buf_gc (128, &gc);
 
 	(void)global_data;
@@ -748,10 +750,18 @@ _pkcs11_openvpn_show_pkcs11_ids_pin_prompt (
 
 	ASSERT (token!=NULL);
 
+	if( !query_user_init (&gc, &qucfg) )
+	{
+	    msg (M_FATAL, "ERROR: Could not prepare for token PIN dialogue");
+	}
+
 	buf_printf (&pass_prompt, "Please enter '%s' token PIN or 'cancel': ", token->display);
+	query_user_add(qucfg, BSTR(&pass_prompt), BLEN(&pass_prompt),
+		       pin, pin_max, true, false);
 
-	if (!get_console_input (BSTR (&pass_prompt), false, pin, pin_max)) {
-		msg (M_FATAL, "Cannot read password from stdin");
+	if (!query_user_exec (qucfg))
+	{
+	    msg (M_FATAL, "Could not retrieve the PIN");
 	}
 
 	gc_free (&gc);
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH 2/4 Re-implement the systemd support using the new query user API

In reply to David Sommerseth

From: David Sommerseth <[email protected]...>

This provides exactly the same systemd functionality which existed before the query user infrastructure got implemented.

---
 configure.ac                  |  2 +-
 src/openvpn/Makefile.am       | 13 +++++++
 src/openvpn/console_systemd.c | 83 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 src/openvpn/console_systemd.c
configure.ac +1 -1
index 608ab6d..0b8e111 100644
@@ -1000,7 +1000,7 @@ fi
 dnl
 dnl Check for systemd
 dnl
-
+AM_CONDITIONAL([ENABLE_SYSTEMD], [test "${enable_systemd}" = "yes"])
 if test "$enable_systemd" = "yes" ; then
     PKG_CHECK_MODULES([libsystemd], [systemd libsystemd],
                       [],
src/openvpn/Makefile.am +13 -0
index e07389f..29cafd7 100644
@@ -117,6 +117,19 @@ openvpn_SOURCES = \
 	tun.c tun.h \
 	win32.h win32.c \
 	cryptoapi.h cryptoapi.c
+
+
+if ENABLE_SYSTEMD
+#
+# If --enable-systemd have been used, build the alternative systemd mechanism
+# The CFLAGS macro set here ensures the alternative mechanism is being used.
+# See console.h for more information.
+#
+AM_CFLAGS += -DQUERY_USER_EXEC_ALTERNATIVE
+openvpn_SOURCES += console_systemd.c
+endif
+
+
 openvpn_LDADD = \
 	$(top_builddir)/src/compat/libcompat.la \
 	$(SOCKETS_LIBS) \
src/openvpn/console_systemd.c new file +83 -0

83 lines not on this page · raw message

Signature · 1 line
1.8.3.1
#3 lore raw

PATCH 3/4 autotools: Add support for extracting version information from pkg-config modules

In reply to David Sommerseth

From: David Sommerseth <[email protected]...>

---
 m4/pkg.m4 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
m4/pkg.m4 +5 -2
index 9a71878..f82496b 100644
@@ -107,16 +107,18 @@ AC_DEFUN([PKG_CHECK_MODULES],
 [AC_REQUIRE([PKG_PROG_PKG_CONFIG])dnl
 AC_ARG_VAR([$1][_CFLAGS], [C compiler flags for $1, overriding pkg-config])dnl
 AC_ARG_VAR([$1][_LIBS], [linker flags for $1, overriding pkg-config])dnl
+AC_ARG_VAR([$1][_MODVERSION], [module version])dnl
 
 pkg_failed=no
 AC_MSG_CHECKING([for $1])
 
 _PKG_CONFIG([$1][_CFLAGS], [cflags], [$2])
 _PKG_CONFIG([$1][_LIBS], [libs], [$2])
+_PKG_CONFIG([$1][_MODVERSION], [modversion], [$2])
 
 m4_define([_PKG_TEXT], [Alternatively, you may set the environment variables $1[]_CFLAGS
-and $1[]_LIBS to avoid the need to call pkg-config.
-See the pkg-config man page for more details.])
+and $1[]_LIBS to avoid the need to call pkg-config.  The $1[]_MODVERSION provides version
+information.  See the pkg-config man page for more details.])
 
 if test $pkg_failed = yes; then
    	AC_MSG_RESULT([no])
@@ -153,6 +155,7 @@ To get pkg-config, see <http://pkg-config.freedesktop.org/>.])[]dnl
 else
 	$1[]_CFLAGS=$pkg_cv_[]$1[]_CFLAGS
 	$1[]_LIBS=$pkg_cv_[]$1[]_LIBS
+	$1[]_MODVERSION=$pkg_cv_[]$1[]_MODVERSION
         AC_MSG_RESULT([yes])
 	$3
 fi[]dnl
Signature · 1 line
1.8.3.1
#4 lore raw

PATCH 4/4 systemd: Do not mask usernames when querying for it via systemd-ask-password

In reply to David Sommerseth

From: David Sommerseth <[email protected]...>

In systemd after version 216, systemd-ask-password will support --echo which will avoid masking the user input. As OpenVPN uses this mechanism collecting usernames when systemd is available, this will avoid the input of usernames to be masked.

This patch also adds the --icon argument, which is aimed at graphical inputs. For example when OpenVPN is started at system boot-time using a graphical boot interface such as Plymouth.

---
 configure.ac                  | 1 +
 src/openvpn/console_systemd.c | 6 ++++++
 2 files changed, 7 insertions(+)
configure.ac +1 -0
index 0b8e111..6e9f45c 100644
@@ -1017,6 +1017,7 @@ if test "$enable_systemd" = "yes" ; then
     AC_CHECK_FUNCS([sd_booted], [], [AC_MSG_ERROR([systemd library is missing sd_booted()])])
     OPTIONAL_SYSTEMD_LIBS="${libsystemd_LIBS}"
     AC_DEFINE(ENABLE_SYSTEMD, 1, [Enable systemd integration])
+    AC_DEFINE_UNQUOTED(SYSTEMD_VERSION, [`echo ${libsystemd_MODVERSION}`], [systemd version installed])
     LIBS="${saved_LIBS}"
 fi
 
src/openvpn/console_systemd.c +6 -0
index 63d0399..9ba582a 100644
@@ -32,6 +32,12 @@ get_console_input_systemd (const char *prompt, const bool echo, char *input, con
 
   argv_init (&argv);
   argv_printf (&argv, SYSTEMD_ASK_PASSWORD_PATH);
+#if SYSTEMD_VERSION > 216
+  if( echo )
+  {
+      argv_printf_cat(&argv, "--echo");
+  }
+#endif
   argv_printf_cat (&argv, "%s", prompt);
 
   if ((std_out = openvpn_popen (&argv, NULL)) < 0) {
Signature · 1 line
1.8.3.1