Skip to contentSkip to conversation

Reworking the interface for querying users

PATCH In review David Sommerseth 2 participants 9 messages Aug 11, 2015 – Jan 21, 2016

lore
Share & tools
lore

David Sommerseth

author
#0 lore raw

v3 cover Reworking the interface for querying users

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

This is the third round of improving the API for querying users for information, after it was discussed in the developer meeting August 10 2015.

The previous patch set can be found here:
<http://thread.gmane.org/gmane.network.openvpn.devel/9657>

Most of the changes was in patch 1/4, where the most important changes are replacing QUERY_USER_FOREACH macro with a for() loop and the query_user_init() function was changed to a void function which will do msg(M_FATAL,...) on malloc() failures directly. The QUERY_USER_FOREACH change also required a minor adoptation in patch 2/4 too.

It was also reported build issues [1] if both systemd.pc and libsystemd.pc files were installed. This has also been fixed in patch 4/4. The issue was that the previous PKG_CHECK_MODULES() check would do call
'pkg-config --modversion systemd libsystemd' which would return the mod-version twice, separated by a newline. The check for systemd, libsystemd and libsystemd-daemon have now been split into separate checks.

[1] <http://thread.gmane.org/gmane.network.openvpn.devel/9657/focus=9661>

During the review it was also discovered that the announced --icon argument was not present in patch 4/4. This had just slipped in the patches sent to the mailing list.

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                  |  13 ++-
 m4/pkg.m4                     |   7 +-
 src/openvpn/Makefile.am       |  15 ++-
 src/openvpn/console.c         | 224 ++++++--------------------------------
 src/openvpn/console.h         |  83 +++++++++++++-
 src/openvpn/console_builtin.c | 246 ++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/console_systemd.c | 119 ++++++++++++++++++++
 src/openvpn/misc.c            |  64 ++++++++---
 src/openvpn/pkcs11.c          |  12 ++-
 9 files changed, 562 insertions(+), 221 deletions(-)
 create mode 100644 src/openvpn/console_builtin.c
 create mode 100644 src/openvpn/console_systemd.c
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH v3 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.

Signed-off-by David Sommerseth <[email protected]...>

[v2 - Removed the QUERY_USER_FOREACH macro
- Avoided using underscore prefix in function names
- Make query_user_init() do M_FATAL and become a void function instead of returning false in these unlikely situations ]

---
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.c         | 224 ++++++--------------------------------
 src/openvpn/console.h         |  83 +++++++++++++-
 src/openvpn/console_builtin.c | 246 ++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/misc.c            |  64 ++++++++---
 src/openvpn/pkcs11.c          |  12 ++-
 6 files changed, 417 insertions(+), 214 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 +32 -192

252 lines not on this page · raw message

src/openvpn/console.h +80 -3

95 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 +49 -15

113 lines not on this page · raw message

src/openvpn/pkcs11.c +9 -3
index 3a15ef6..5a33646 100644
@@ -5,7 +5,8 @@
  *             packet encryption, packet authentication, and
  *             packet compression.
  *
- *  Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <[email protected]...>
+ *  Copyright (C) 2002-2015 OpenVPN Technologies, Inc. <[email protected]...>
+ *  Copyright (C) 2014-2015 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,14 @@ _pkcs11_openvpn_show_pkcs11_ids_pin_prompt (
 
 	ASSERT (token!=NULL);
 
+	query_user_init (&gc, &qucfg);
 	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

Re: [Openvpn-devel] [PATCH v3 1/4] Rework the user input interface to make it more modular

Hi,

so, I finally found time to review this - apologies.

We need a rebase (because of the isatty() change, sorry for that) - and a v3, though, as I found a few minor things...

Quoted 4 lines · David Sommerseth

index d66d408..212a0a8 100644

src/openvpn/console.c +0 -0
@@ -5,7 +5,8 @@

[..]

src/openvpn/console.c line 39

Quoted 8 lines
src/openvpn/console.c +2 -5
-static bool
-get_console_input_win32 (const char *prompt, const bool echo, char *input, const int capacity)
+void query_user_init(struct gc_arena *gc, struct query_user ** setup)
 {
-  HANDLE in = INVALID_HANDLE_VALUE;
-  HANDLE err = INVALID_HANDLE_VALUE;
-  DWORD len = 0;
+    ASSERT (&gc != NULL);

This needs to be "ASSERT (gc != NULL)" or just "ASSERT (gc)"... &gc will always be != NULL.

(Theoretically it could just go, as gc_malloc() is safe to be used with gc being NULL... so it would allocate global memory, but since this is just called once, even *that* would not be actually harmful)

Quoted 1 line

+ *setup = (struct query_user *) gc_malloc (sizeof(**setup)+2, true, gc);

Why "+2"? This is a structure, it has a well defined size, no "extra 0 bytes" to be expected.

Quoted 2 lines

+ if( unlikely(*setup == NULL) )

     {

gc_malloc() guarantees that the resulting pointer is not NULL, as it will always call check_malloc_return() and that will exit(1) on NULL (out_of_memory() in error.c, which is safer than calling msg() in an out of memory situation anyway)

[..]

Quoted 5 lines
src/openvpn/console.c +4 -0
+bool query_user_add(struct query_user * setup,
+                    char *prompt, size_t prompt_len,
+                    char *resp, size_t resp_len,
+                    bool query, bool echo)
 {

[..]

Quoted 9 lines
src/openvpn/console.c +9 -0
+    /* Save the information needed for the user interaction */
+    p->_gc = setup->_gc;
+    p->prompt = prompt;
+    p->prompt_len = prompt_len;
+    p->response = resp;
+    p->response_len = resp_len;
+    p->query = query;
+    p->echo = echo;
+    query_user_init(p->_gc, &p->next);

[..]

Quoted 2 lines

+ return (p->next != NULL) ? true : false;
}

I don't think this final check makes sense. First of all, no caller of query_user_add() ever *cares*, and second, it just cannot happen given that query_user_init() exit()s anyway if malloc fails.

So just make this "void query_user_add(...)" :-)

Quoted 5 lines
src/openvpn/console.h +0 -0
index 268f3fe..4d0a57f 100644
@@ -5,7 +5,8 @@

[..]

src/openvpn/console.h line 44

Quoted 12 lines

+ * Configuration setup for declaring what kind of information to ask a user for
+ */

src/openvpn/console.h +10 -0
+struct query_user {
+    struct gc_arena *_gc;     /**< Local pointer to the gc_arena maintaining this chained list */
+    char *prompt;             /**< Prompt to present to the user */
+    size_t prompt_len;        /**< Lenght of the prompt string */
+    char *response;           /**< The user's response */
+    size_t response_len;      /**< Lenght the of the user reposone */
+    bool query;               /**< True: Expect user to provide a response, False: only print  */
+    bool echo;                /**< True: The user should see what is being typed, otherwise mask it */
+    struct query_user *next;  /**<  Next information to query for, NULL indicates end of list */
+};

Is there anything in OpenVPN that would print out a message to console but not prompt for a response? The "query" field adds a bit of carry-on to structure and code, but all current uses of query_user_add() set it to "true"...

[..]

Quoted 6 lines
src/openvpn/console_builtin.c new file +0 -0
new file mode 100644
index 0000000..b8d6430
@@ -0,0 +1,246 @@

[..]

Quoted 2 lines
src/openvpn/console_builtin.c new file +2 -0
+static bool get_console_input_win32 (const char *prompt, const bool echo, char *input, const int capacity)
+{

[..]

src/openvpn/console_builtin.c line 92

Quoted 5 lines
src/openvpn/console_builtin.c new file +5 -0
+                if (echo)
+                    flags |= ENABLE_ECHO_INPUT;
+                SetConsoleMode (in, flags);
+	    } else
+                is_console = 0;

When moving code, please do not change coding style in-progress - this used to have an "else" on its own line, which makes comparing "has this just be moved or what was changed" more complicated...

src/openvpn/console_builtin.c line 105

Quoted 3 lines
src/openvpn/console_builtin.c new file +3 -0
+            free (winput);
+        } else
+            status = ReadFile (in, input, capacity, &len, NULL);

Same here.

[..]

Quoted 4 lines

+ * If no alternative implementation is declared, a wrapper in console.h will ensure
+ * query_user_exec() will call this function instead.
+ *
+ */

src/openvpn/console_builtin.c line 234

Quoted 10 lines
src/openvpn/console_builtin.c new file +10 -0
+bool query_user_exec_builtin(struct query_user * setup)
+{
+    struct query_user *p;
+    bool ret = true;
+
+    ASSERT( setup != NULL );
+
+    /* Loop through the complete query setup and when needed, collect the information */
+    for (p = setup; p->next != NULL; p = p->next)
+    {

I wonder why you coose to implement the linked list this way, instead of the more common "for (p=first; p != NULL; p = p->next)" approach?

Yours will work fine, but will always have fully-empty structure dangling at the end of the list, serving only to have an "p->next" of NULL... or am I misreading this?

(Of course the query_user_add() function would need to be changed accordingly - right now it's symmetric and should work fine, it just makes me curious what the underlying trade-off is)

src/openvpn/console_builtin.c line 243

Quoted 9 lines
src/openvpn/console_builtin.c new file +9 -0
+        if( p->query )
+        {
+            /* If the user is expected to provide some data */
+            ret &= get_console_input(p->prompt, p->echo, p->response, p->response_len);
+        } else {
+            /* If the prompt should only be presented to the user with no further action */
+            msg(M_INFO|M_NOPREFIX, "%s", p->prompt);
+        }
+    }

See above, I'm wondering about the use case of having a mechanism for "print stuff only" which we then never *use* (... there's way too much "we might eventually do that!" code in OpenVPN today).

Quoted 15 lines
src/openvpn/misc.c +2 -1
index 5627cb9..cc1dc7b 100644
@@ -5,7 +5,8 @@
  *             packet encryption, packet authentication, and
  *             packet compression.
  *
- *  Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <[email protected]...>
+ *  Copyright (C) 2002-2015 OpenVPN Technologies, Inc. <[email protected]...>
+ *  Copyright (C) 2014-2015 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

src/openvpn/misc.c get_user_pass_cr (struct user_pass *up, · line 1056

Quoted 7 lines
src/openvpn/misc.c +2 -0
@@ -1049,11 +1050,15 @@ get_user_pass_cr (struct user_pass *up,
        */
       if (flags & GET_USER_PASS_NEED_OK)
 	{
+	  struct query_user *qucfg = NULL;
 	  struct buffer user_prompt = alloc_buf_gc (128, &gc);
 
+	  query_user_init (&gc, &qucfg);

After the change from "return false to M_FATAL", we could keep this more in line with the rest of the code - which avoids double pointers for return values - and make it

src/openvpn/misc.c get_user_pass_cr (struct user_pass *up, · line 1057

Quoted 1 line
src/openvpn/misc.c +1 -0
+	  struct query_user *qucfg = query_user_init (&gc);

(adjusting query_user_add() appropriately, of course).

But the way it is works, of course.

src/openvpn/misc.c get_user_pass_cr (struct user_pass *up, · line 1062

Quoted 7 lines
src/openvpn/misc.c +3 -1
 	  buf_printf (&user_prompt, "NEED-OK|%s|%s:", prefix, up->username);
+	  query_user_add (qucfg, BSTR(&user_prompt), BLEN(&user_prompt), up->password, USER_PASS_LEN,
+                          true, true);
 
-	  if (!get_console_input (BSTR (&user_prompt), true, up->password, USER_PASS_LEN))
+	  if (!query_user_exec (qucfg))
 	    msg (M_FATAL, "ERROR: could not read %s ok-confirmation from stdin", prefix);

... also, all but one of these invocations use the same sequence

query_user_init()
query_user_add() (only a single time)
query_user_exec()

but use quite a bit of "screen real estate" of the three independent steps
- while I value the flexibility (and it's used when querying for usename *and* password), for single-shot calls, wouldn't a wrapper that just does all in one go make the code more readable?

So you'd call something like

src/openvpn/misc.c get_user_pass_cr (struct user_pass *up,

Quoted 2 lines
src/openvpn/misc.c +2 -0
+	  query_user_single_exec (BSTR(&user_prompt), BLEN(&user_prompt), up->password, USER_PASS_LEN,
+                          true, true);

which would expand to init()/add()/exec() calls inside...

(I'm sure I suggested this already for v1)

src/openvpn/pkcs11.c line 9

Quoted 9 lines
src/openvpn/pkcs11.c +2 -1
@@ -5,7 +5,8 @@
  *             packet encryption, packet authentication, and
  *             packet compression.
  *
- *  Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <[email protected]...>
+ *  Copyright (C) 2002-2015 OpenVPN Technologies, Inc. <[email protected]...>
+ *  Copyright (C) 2014-2015 David Sommerseth <[email protected]...>

I'm not sure if this is really "our style" - I can see adding a copyright notice if a module is basically rewritten (console.c, console.h), but for a 3-line change I find this surprising, as it is not done elsewhere - most of our modules only have the generic OpenVPN Technologies, just a very small set carry other copyright notices (and these usually stem from "we wrote this from scratch" or "this was taken from elsewhere").

Just thinking out loud, though. Not considering this a showstopper.

Quoted 15 lines
src/openvpn/pkcs11.c +2 -0
  *  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,14 @@ _pkcs11_openvpn_show_pkcs11_ids_pin_prompt (
 
 	ASSERT (token!=NULL);
 
+	query_user_init (&gc, &qucfg);

src/openvpn/pkcs11.c _pkcs11_openvpn_show_pkcs11_ids_pin_prompt ( · line 761

Quoted 10 lines
src/openvpn/pkcs11.c +5 -2
 	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");
 	}

...this would be another good candidate for "query_user_single_exec()"... :-)

So - in principle feature-ACK, but I think the code can be improved (and it needs a rebase due to the changes in console.c and misc.c anyway
- please do not forget to move the isatty() bits to console_builtin.c, git might not be smart enough to get *that* right. But maybe it is...)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#3 lore raw

PATCH v3 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.

[v2 - Removed the QUERY_USER_FOREACH macro]

Signed-off-by David Sommerseth <[email protected]...>
---
 configure.ac                  |   2 +-
 src/openvpn/Makefile.am       |  13 +++++
 src/openvpn/console_systemd.c | 112 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+), 1 deletion(-)
 create mode 100644 src/openvpn/console_systemd.c
configure.ac +1 -1
index 5dccfbb..182e116 100644
@@ -993,7 +993,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 +112 -0

112 lines not on this page · raw message

Signature · 1 line
1.8.3.1
#4 lore raw

Re: [Openvpn-devel] [PATCH v3 2/4] Re-implement the systemd support using the new query user API

Hi,

Quoted 6 lines · David Sommerseth

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

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

[v2 - Removed the QUERY_USER_FOREACH macro]

A few nags...

src/openvpn/Makefile.am openvpn_SOURCES = \ · line 130

Quoted 11 lines
src/openvpn/Makefile.am +11 -0
+
+
+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

Why make it so complicated? We have #ifdef ENABLE_SYSTEMD already, so we could just do this in the console.h header file...

#if defined(ENABLE_SYSTEMD) || defined(MAGIC_OTHER_CONSOLE) || ...

# define QUERY_USER_EXEC_ALTERNATIVE
#endif

IOW, we have way too many corners that spring C #define's into life, and I don't think we should invent yet a new one without very good reason
- especially if that needs a long comment to explain why it's there.

Thinking more about it - why not just always add console_systemd.c to openvpn_SOURCES (which, I think, is also relevant for "make dist", no?) and *inside* console_systemd.c wrap it with

  #if defined(ENABLE_SYSTEMD)
  ..
  #endif

so it gets compiled into an empty .o - like we do with win32.c or the ssl_$library.c variants today, so it seems to work on enough platforms to not worry about that. Plus, it's "what we do elsewhere"...

[..]

src/openvpn/console_systemd.c

Quoted 6 lines
src/openvpn/console_systemd.c new file +0 -0
new file mode 100644
index 0000000..a9cfbe6
@@ -0,0 +1,112 @@

This looks good to me, except for the #ifdef :-) - and...

src/openvpn/console_systemd.c line 42

Quoted 6 lines
src/openvpn/console_systemd.c new file +6 -0
+/*
+ * is systemd running
+ */
+
+static bool
+check_systemd_running ()

... any particular reason for not having a (void) in there? Technically, it's "moving code without changes" (good!) but while at it, you could add this in 4/4 to be proper ANSI C :-)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#5 lore raw

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

In reply to David Sommerseth

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

Signed-off-by 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
#6 lore raw

Re: [Openvpn-devel] [PATCH v3 3/4] autotools: Add support for extracting version information from pkg-config modules

Hi,

Quoted 12 lines · David Sommerseth

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

Signed-off-by David Sommerseth <[email protected]...>
---
 m4/pkg.m4 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
m4/pkg.m4 +0 -0
index 9a71878..f82496b 100644

m4/pkg.m4 AC_DEFUN([PKG_CHECK_MODULES], · line 110

Quoted 4 lines
m4/pkg.m4 +1 -0
@@ -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

Not commenting on these changes, as I have no idea what they do, and no way to test... (no system with systemd)

In doubt, buildbot will tell us if it breaks stuff somewhere... so not opposing this, at seems needed for 4/4.

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#7 lore raw

PATCH v3 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.

The patch also fixes an issue with pkg-config if both systemd.pc and libsystemd.pc is installed, which would result in the systemd version number being reported twice. The autotools check for sd_booted() is also removed, as this check doesn't really provide much of value.

Signed-off-by David Sommerseth <[email protected]...>
---
 configure.ac                  | 11 +++++++----
 src/openvpn/console_systemd.c |  7 +++++++
 2 files changed, 14 insertions(+), 4 deletions(-)
configure.ac +7 -4
index 182e116..133d35e 100644
@@ -995,10 +995,13 @@ 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],
+    PKG_CHECK_MODULES([libsystemd], [systemd],
                       [],
-                      [PKG_CHECK_MODULES([libsystemd], [libsystemd-daemon])]
-                      )
+                      [PKG_CHECK_MODULES([libsystemd], [libsystemd],
+                                         [],
+                                         [PKG_CHECK_MODULES([libsystemd], [libsystemd-daemon])]
+                                         )]
+                      ])
     AC_CHECK_HEADERS(systemd/sd-daemon.h,
        ,
        [

@@ -1007,9 +1010,9 @@ if test "$enable_systemd" = "yes" ; then
 
     saved_LIBS="${LIBS}"
     LIBS="${LIBS} ${libsystemd_LIBS}"
-    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 +7 -0
index a9cfbe6..5a85e38 100644
@@ -61,6 +61,13 @@ 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, "--icon network-vpn");
     argv_printf_cat (&argv, "%s", prompt);
 
     if ((std_out = openvpn_popen (&argv, NULL)) < 0) {
Signature · 1 line
1.8.3.1
#8 lore raw

Re: [Openvpn-devel] [PATCH v3 4/4] systemd: Do not mask usernames when querying for it via systemd-ask-password

Hi,

Quoted 6 lines · 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.

Well, this one is easy :-) - "ACK!"

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...