Query User Input patches

PATCH In review David Sommerseth 3 participants 20 messages Aug 8, 2016 – Oct 11, 2016 1 message not in the archive

Share & tools

David Sommerseth

author

2016-08-08 19:28 UTC #0 lore raw

v4 cover Query User Input patches

Lets see if fourth time is the charm. This patch set reworks the whole interface of quering the user for input. This will be the building block for enabling OpenVPN to gather as much information as possible at the initalization point, as well as providing an API for other more advanced mechanisms to query the user for input in the OS.

Previous attempts can be found here:
<https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/1439311312-2682-1-git-send-email-openvpn.list%40topphemmelig.net/#msg34365281>
<https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/1430942786-31419-1-git-send-email-openvpn.list%40topphemmelig.net/#msg34089983>
<https://sourceforge.net/p/openvpn/mailman/openvpn-devel/thread/1416138935-12614-1-git-send-email-openvpn.list%40topphemmelig.net/#msg33044439>

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                     |   9 +-
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.c         | 228 +++++-------------------------------
 src/openvpn/console.h         |  89 +++++++++++++-
 src/openvpn/console_builtin.c | 261 ++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/console_systemd.c | 122 ++++++++++++++++++++
 src/openvpn/misc.c            |  58 +++++++---
 src/openvpn/pkcs11.c          |   7 +-
 9 files changed, 550 insertions(+), 229 deletions(-)
 create mode 100644 src/openvpn/console_builtin.c
 create mode 100644 src/openvpn/console_systemd.c

Signature · 1 line

1.8.3.1

David Sommerseth

author

2016-08-08 19:28 UTC #1 lore raw

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

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.

[v4 - add a simple wrapper combining query_user_{init,add,exec}()
- change disapproved &= syntax ]

[v3 - Avoid the dynamic list, use a static list of QUERY_USER_NUMSLOTS
- The list of query_user data is now a global variable
- Replaced query_user_init() with query_user_clear()
- Make query_user_add() a void function
- Rebased against master/600dd9a16fc61 ]

[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 ]

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

---
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.c         | 228 +++++-------------------------------
 src/openvpn/console.h         |  89 +++++++++++++-
 src/openvpn/console_builtin.c | 261 ++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/misc.c            |  58 +++++++---
 src/openvpn/pkcs11.c          |   7 +-
 6 files changed, 420 insertions(+), 225 deletions(-)
 create mode 100644 src/openvpn/console_builtin.c

src/openvpn/Makefile.am +1 -1

index 9d4bf61..8d6d39f 100644
@@ -68,7 +68,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 +27 ~~-201~~

252 lines not on this page · raw message

src/openvpn/console.h +87 -2

100 lines not on this page · raw message

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

261 lines not on this page · raw message

src/openvpn/misc.c +40 ~~-18~~

100 lines not on this page · raw message

src/openvpn/pkcs11.c +4 -3

index a1f13c5..2621058 100644
@@ -744,9 +744,10 @@ _pkcs11_openvpn_show_pkcs11_ids_pin_prompt (
 	ASSERT (token!=NULL);
 
 	buf_printf (&pass_prompt, "Please enter '%s' token PIN or 'cancel': ", token->display);
-
-	if (!get_console_input (BSTR (&pass_prompt), false, pin, pin_max)) {
-		msg (M_FATAL, "Cannot read password from stdin");
+	if (!query_user_SINGLE(BSTR(&pass_prompt), BLEN(&pass_prompt),
+		       pin, pin_max, false))
+	{
+	    msg (M_FATAL, "Could not retrieve the PIN");
 	}
 
 	gc_free (&gc);

Signature · 1 line

1.8.3.1

Selva Nair

2016-08-13 03:48 UTC #2 lore raw

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

Hi,

Quoted 40 lines · David Sommerseth

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.
[v4 - add a simple wrapper combining query_user_{init,add,exec}()
- change disapproved &= syntax ]
[v3 - Avoid the dynamic list, use a static list of QUERY_USER_NUMSLOTS
- The list of query_user data is now a global variable
- Replaced query_user_init() with query_user_clear()
- Make query_user_add() a void function
- Rebased against master/600dd9a16fc61 ]
[v2 - Removed the QUERY_USER_FOREACH macro
src/openvpn/console.h +0 -0
index 268f3fe..44d49ef 100644
@@ -6,6 +6,8 @@
  *             packet compression.
  *
  *  Copyright (C) 2002-2010 OpenVPN Technologies, Inc. <[email protected]...
Quoted 1 line
+ * Copyright (C) 2014-2015 David Sommerseth <[email protected]...>
+ * Copyright (C) 2016 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/console.h +2 -2
@@ -27,7 +29,90 @@
 
 #include "basic.h"
 
-bool
-get_console_input (const char *prompt, const bool echo, char *input,
 const int capacity);
+/**
+ *  Configuration setup for declaring what kind of information to ask a

Quoted 3 lines

user for
+ */
+struct _query_user {

+ char *prompt; /**< Prompt to present to the user */

Quoted 11 lines

src/openvpn/console.h +4 -0

+    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 echo;                /**< True: The user should see what is

being typed, otherwise mask it */

src/openvpn/console.h +4 -0

+};
+
+#define QUERY_USER_NUMSLOTS 10
+extern struct _query_user query_user[];  /**< Global variable, declared

in console.c */

Hmm... this is wading into style territory, so I'll move on by just mumbling that leading underscore in struct names serves no useful purpose.

In misc.c:

Quoted 14 lines

src/openvpn/console.h +1 -1

@@ -1184,32 +1192,46 @@ get_user_pass_cr (struct user_pass *up,
              struct buffer user_prompt = alloc_buf_gc (128, &gc);
              struct buffer pass_prompt = alloc_buf_gc (128, &gc);
 
+             query_user_clear ();
              buf_printf (&user_prompt, "Enter %s Username:", prefix);
              buf_printf (&pass_prompt, "Enter %s Password:", prefix);
 
 
              if (username_from_stdin && !(flags &
 GET_USER_PASS_PASSWORD_ONLY))
                {
-                 if (!get_console_input (BSTR (&user_prompt), true,

src/openvpn/console.h get_user_pass_cr (struct user_pass *up, · line 1211

Quoted 10 lines

src/openvpn/console.h +6 -1

 up->username, USER_PASS_LEN))
-                   msg (M_FATAL, "ERROR: could not read %s username from
 stdin", prefix);
+                 query_user_add (BSTR(&user_prompt), BLEN(&user_prompt),
+                                  up->username, USER_PASS_LEN, true);
+               }
+
+             query_user_add (BSTR(&pass_prompt), BLEN(&pass_prompt),
+                              up->password, USER_PASS_LEN, false);

The above call should be conditional on "if (password_from_stdin)". Else it will get called even after reading password from a file. Unfortunately get_user_pass_cr() has become so fragile after repeated hacks, its so easy to break it...

The following may sound harsh, but that's not the intention. I've spent hours on these patches now, so just trying to get the best out of it:
In the end, I'm not sure what is achieved by this new framework. I see multiple queries can be combined into one exec call, but the way its used in get_user_pass_cr() does not make full use of it. Sure, username and password are prompted in one call, but static challenge is still handled by an additional call. FWIW, to be true to the spirit of the implementation, a request for username/password/static-challenge-response should result in only one call to query_user_exec, not two. Only then one can even remotely hope to have a way prompting the user for all that info in one dialog by some future ask-password agent. Unless you set a clear style and guidance here, over time this will degenerate into something logically identical to the original code except for different function names and twice the function calls.

Even with one exec call, finally systemd-ask-password has to be called multiple times and it seems currently there are no agents that could take multiple queries and prompt the user in one dialog, for example.

Its more like a solution looking for a problem. Which is not a bad thing per se..

Regards,

Selva

David Sommerseth

2016-08-15 10:49 UTC #3 lore raw

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Quoted 4 lines · Selva Nair

Hi,
On Mon, Aug 8, 2016 at 3:28 PM, David Sommerseth <[email protected]... <mailto:[email protected]...>> wrote:

[...snip...]

Quoted 6 lines

} @@ -1184,32 +1192,46 @@ get_user_pass_cr (struct user_pass *up, struct buffer user_prompt = alloc_buf_gc (128, &gc); struct buffer pass_prompt = alloc_buf_gc (128, &gc);
+ query_user_clear (); buf_printf (&user_prompt, "Enter
%s Username:",

prefix);

Quoted 1 line

buf_printf (&pass_prompt, "Enter %s Password:",

prefix);

Quoted 9 lines

if (username_from_stdin && !(flags & GET_USER_PASS_PASSWORD_ONLY))
{ - if (!get_console_input (BSTR (&user_prompt),
true, up->username, USER_PASS_LEN)) - msg
(M_FATAL, "ERROR: could not read %s username from stdin", prefix);
+ query_user_add (BSTR(&user_prompt),
BLEN(&user_prompt), +
up->username, USER_PASS_LEN,

true);

Quoted 2 lines

+ } + + query_user_add
(BSTR(&pass_prompt),

BLEN(&pass_prompt),

Quoted 8 lines

+ up->password, USER_PASS_LEN,
false);
The above call should be conditional on "if (password_from_stdin)". Else it will get called even after reading password from a file. Unfortunately get_user_pass_cr() has become so fragile after repeated hacks, its so easy to break it...

Good catch! I've noticed that in some very few configurations git master already behaves somewhat odd with systemd enabled. I think what you've spotted here is one of the issues I've seen but not had time to investigate yet. This week I'm fairly tied up in some other projects, but I'll dig into this and test these things. You've pointed me into a very sane code path, though.

Quoted 7 lines

The following may sound harsh, but that's not the intention. I've spent hours on these patches now, so just trying to get the best out of it: In the end, I'm not sure what is achieved by this new framework. I see multiple queries can be combined into one exec call, but the way its used in get_user_pass_cr() does not make full use of it. Sure, username and password are prompted in one call, but static challenge is still handled by an additional call. FWIW,

Quoted 8 lines

to be true to the spirit of the implementation, a request for username/password/static-challenge-response should result in only one call to query_user_exec, not two. Only then one can even remotely hope to have a way prompting the user for all that info in one dialog by some future ask-password agent. Unless you set a clear style and guidance here, over time this will degenerate into something logically identical to the original code except for different function names and twice the function calls.

Fair point! The thing is that *currently* systemd-ask-password isn't clever at all. I've been discussing this with a few upstream systemd developers already, and basically they are going to re-write much of the systemd-ask-password stuff to be far more intelligent. But much of that work has halted as a new approach is planned to use DBus. That again creates other challenges at boot time as DBus might not be running when systemd-ask-password is called. This will be resolved once the kdbus work gets into an upstream kernel, then things can start to move forward too.

Quoted 4 lines

Even with one exec call, finally systemd-ask-password has to be called multiple times and it seems currently there are no agents that could take multiple queries and prompt the user in one dialog, for example.

And these patches are just the first step. First we need a more sane API to work with, then we can start juggling things more around. My plan is to move as much of these user query calls as early as possible during the init phase. Then depending on the backend (standard console, systemd, dbus, whatever fancy someone else writes), the user is queried for this information as efficiently as possible.

I've also started to ponder how we can make use of this via the management interface too, for the current GUIs we have around. Which means that an end user can be asked for private key password or PKCS#11 PIN, username and password in one go - as OpenVPN can now tell the UI what it needs to establish a connection. This could also give some benefits to the OpenVPN plug-in in NetworkManager too.

There are absolutely some challenges which needs to be resolved with the challenge/response protocols. But this needs to be looked into far more closely when a reasonable framework is in place. I prefer to start at the low hanging fruits to gain experience and then start solving the harder ones, especially when they're related. For the challenge/response protocol there are actually two protocol, which works quite differently. Anyway, this is on the TODO list as well.

I am also considering if we should have some plug-in API hooks for this as well, so that a third-party can add a separate query mechanism without needing to recompile OpenVPN. But this comes much later once we know where and how things move forward. Currently the goal is to have the query-user interface somewhat similar to the SSL library, where you flip it at compile time for the default.

Quoted 2 lines

Its more like a solution looking for a problem. Which is not a bad thing per se..

As this patch-set has been discussed for a year now or so, I can understand this point of view if you've not followed the discussions over time.

My main goal is to improve the end user interaction with OpenVPN, as I find the current model less user friendly and very limited when integrating OpenVPN with today's more modern user interfaces. So these patches, are just laying a foundation for what can come next. Without a new approach to query users, it is fairly limited what you can do with the situation as it is today.

If someone sees a better approach for resolving this, then I'm willing to look into that ... but considering this process have been going on for over a year without too much fuzz around this approach, the barrier to redesign everything from scratch again is quite high.

- --
kind regards,

David Sommerseth
OpenVPN Technologies, Inc
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iEYEARECAAYFAlexnkUACgkQDC186MBRfroidQCeMMKyVVJYxdTlgQz9VvhgjQ4T 0JcAoJEqUtx9N6jbgkNzvCwY91Uhk6b5
=3wuk
-----END PGP SIGNATURE-----

Selva Nair

2016-08-15 16:19 UTC #4 lore raw

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

Hi,

Quoted 21 lines · David Sommerseth

Quoted 8 lines
if (username_from_stdin && !(flags & GET_USER_PASS_PASSWORD_ONLY))
{ - if (!get_console_input (BSTR (&user_prompt),
true, up->username, USER_PASS_LEN)) - msg
(M_FATAL, "ERROR: could not read %s username from stdin", prefix);
+ query_user_add (BSTR(&user_prompt),
BLEN(&user_prompt), +
up->username, USER_PASS_LEN,
true);
Quoted 2 lines
+ } + + query_user_add
(BSTR(&pass_prompt),
BLEN(&pass_prompt),
Quoted 8 lines
+ up->password, USER_PASS_LEN,
false);
The above call should be conditional on "if (password_from_stdin)". Else it will get called even after reading password from a file. Unfortunately get_user_pass_cr() has become so fragile after repeated hacks, its so easy to break it...

Quoted 7 lines

Good catch! I've noticed that in some very few configurations git master already behaves somewhat odd with systemd enabled. I think what you've spotted here is one of the issues I've seen but not had time to investigate yet. This week I'm fairly tied up in some other projects, but I'll dig into this and test these things. You've pointed me into a very sane code path, though.

I too saw some systemd weirdness when openvpn is started from init scripts:
the password prompt came up before username followed by password again. But that was before fixing the CLEAR(*input) bug in 2/4. Now the patchset appears to behave fine (except for the above bug) on my test setups (Debian jessie and stretch/sid).

Quoted 16 lines

Quoted 7 lines
The following may sound harsh, but that's not the intention. I've spent hours on these patches now, so just trying to get the best out of it: In the end, I'm not sure what is achieved by this new framework. I see multiple queries can be combined into one exec call, but the way its used in get_user_pass_cr() does not make full use of it. Sure, username and password are prompted in one call, but static challenge is still handled by an additional call. FWIW,
Quoted 8 lines
to be true to the spirit of the implementation, a request for username/password/static-challenge-response should result in only one call to query_user_exec, not two. Only then one can even remotely hope to have a way prompting the user for all that info in one dialog by some future ask-password agent. Unless you set a clear style and guidance here, over time this will degenerate into something logically identical to the original code except for different function names and twice the function calls.

Quoted 4 lines

Fair point! The thing is that *currently* systemd-ask-password isn't clever at all. I've been discussing this with a few upstream systemd developers already, and basically they are going to re-write much of the systemd-ask-password stuff to be far more intelligent. But much

of that work has halted as a new approach is planned to use DBus.

Quoted 10 lines

That again creates other challenges at boot time as DBus might not be running when systemd-ask-password is called. This will be resolved once the kdbus work gets into an upstream kernel, then things can start to move forward too.
Quoted 4 lines
Even with one exec call, finally systemd-ask-password has to be called multiple times and it seems currently there are no agents that could take multiple queries and prompt the user in one dialog, for example.

Quoted 7 lines

And these patches are just the first step. First we need a more sane API to work with, then we can start juggling things more around. My plan is to move as much of these user query calls as early as possible during the init phase. Then depending on the backend (standard console, systemd, dbus, whatever fancy someone else writes), the user is queried for this information as efficiently as possible.

Yes, that's all what I meant by this being ahead of its time. Which is fine..

Irrespective of future plans, its pretty easy to combine the prompt for username/password/static-challenge into one query_exec call.

Quoted 7 lines

I've also started to ponder how we can make use of this via the management interface too, for the current GUIs we have around. Which means that an end user can be asked for private key password or PKCS#11 PIN, username and password in one go - as OpenVPN can now tell the UI what it needs to establish a connection. This could also give some benefits to the OpenVPN plug-in in NetworkManager too.

Actually the management interface (MI) works pretty well as is. I think most GUI's use the MI and can, at least in principle, prompt for user-auth and challenge response together --- see the Windows GUI as an example. Dynamic challenge has to be prompted for in a follow-up dialog as the challenge is not known in advance. Add to that GUI's ability to remember passwords, all that has to be repeatedly prompted for, in many cases, is the challenge response (static or dynamic). Of course there is room for improvement along the lines of query user by having a uniform way of passing all user-input request to the management.

If someone sees a better approach for resolving this, then I'm willing

Quoted 3 lines

to look into that ... but considering this process have been going on for over a year without too much fuzz around this approach, the barrier to redesign everything from scratch again is quite high.

I have no idea for a better approach and the design of query_user looks good to me. I only wanted to point out some minor ways in which the patch set could be improved. And to indicate that only when user-input agents that can make full use of it becomes available we'll know whether the design needs further tweaks or not.

Best,

Selva

David Sommerseth

author

2016-08-26 17:48 UTC #5 lore raw

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

In reply to David Sommerseth

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.

[v5 - Ensure password prompt is only displayed if we should read
from stdin ]

[v4 - add a simple wrapper combining query_user_{init,add,exec}()
- change disapproved &= syntax ]

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

---
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.c         | 228 +++++-------------------------------
 src/openvpn/console.h         |  89 +++++++++++++-
 src/openvpn/console_builtin.c | 261 ++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/misc.c            |  61 +++++++---
 src/openvpn/pkcs11.c          |   7 +-
 6 files changed, 423 insertions(+), 225 deletions(-)
 create mode 100644 src/openvpn/console_builtin.c

src/openvpn/Makefile.am +1 -1

index 9d4bf61..8d6d39f 100644
@@ -68,7 +68,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 +27 ~~-201~~

252 lines not on this page · raw message

src/openvpn/console.h +87 -2

100 lines not on this page · raw message

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

261 lines not on this page · raw message

src/openvpn/misc.c +43 ~~-18~~

103 lines not on this page · raw message

src/openvpn/pkcs11.c +4 -3

index a1f13c5..2621058 100644
@@ -744,9 +744,10 @@ _pkcs11_openvpn_show_pkcs11_ids_pin_prompt (
 	ASSERT (token!=NULL);
 
 	buf_printf (&pass_prompt, "Please enter '%s' token PIN or 'cancel': ", token->display);
-
-	if (!get_console_input (BSTR (&pass_prompt), false, pin, pin_max)) {
-		msg (M_FATAL, "Cannot read password from stdin");
+	if (!query_user_SINGLE(BSTR(&pass_prompt), BLEN(&pass_prompt),
+		       pin, pin_max, false))
+	{
+	    msg (M_FATAL, "Could not retrieve the PIN");
 	}
 
 	gc_free (&gc);

Signature · 1 line

1.8.3.1

Selva Nair

2016-08-27 00:59 UTC #6 lore raw

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

Quoted 24 lines · David Sommerseth

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.
[v5 - Ensure password prompt is only displayed if we should read
from stdin ]
[v4 - add a simple wrapper combining query_user_{init,add,exec}()
- change disapproved &= syntax ]
[v3 - Avoid the dynamic list, use a static list of QUERY_USER_NUMSLOTS
- The list of query_user data is now a global variable
- Replaced query_user_init() with query_user_clear()
- Make query_user_add() a void function
- Rebased against master/600dd9a16fc61 ]
[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 ]
Signed-off-by David Sommerseth <[email protected]...>

Quoted 9 lines

---
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.c         | 228 +++++-------------------------------
 src/openvpn/console.h         |  89 +++++++++++++-
 src/openvpn/console_builtin.c | 261 ++++++++++++++++++++++++++++++
++++++++++++
 src/openvpn/misc.c            |  61 +++++++---
 src/openvpn/pkcs11.c          |   7 +-
 6 files changed, 423 insertions(+), 225 deletions(-)

v5 looks good.
ACK (only for git-master).

I still think username, password and static challenge should be combined into one query_user_exec() call but no need to hold this back for that.

Selva

David Sommerseth

author

2016-10-11 10:27 UTC #7 lore raw

Re: [Openvpn-devel] [PATCH applied] Rework the user input interface to make it more modular

In reply to David Sommerseth

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Your patch has been applied to the master branch.

commit 430ce8bd03b23717ec42a8a34aa66e804ca97287

Author: David Sommerseth
Date:   Fri Aug 26 19:48:52 2016 +0200

     Rework the user input interface to make it more modular

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: Selva Nair <[email protected]...>
     Message-Id: [email protected]...
     URL: https://www.mail-archive.com/[email protected]/msg00137.html

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX/L6mAAoJEIbPlEyWcf3y7C8P/ApkbdUBClSilzQ3csFpF0lL 2na8uN7HCYIYb9Baghb5raYSqkLu1+11ScSWI/7kcYmCGECpLVgd7PZbnjm+zHxU h7E00CT3GsSdYx1CBLhFBARBv8XY33o4Sxb09VqhkaXjMw8/Fv0CvAcA9JxtfxGw oB/FW+Mdg9KC2pT+gBpO9C3XyBfetKaW8ri5Ncwjj++H7LL3Gf/omVj9tgZ7E3ek icP0VN/p6X3dxiAAVSNMv4n4wzJ0ChaPV+iANFpVNiyNEAQAjh+TNXeS/ZK+rgo8 hT8RVgbAdCJV5f0Fc0tH839wPnmmvsSxgb+XDBJg+BoZkQsYJwWTmxRXEOTYFXn/ UyO9KDkXPN5/Puhwa246v2dWed1k5/8Uv5RTCBH51AsYJp/2VJOSo21EjDVtrAwd f1Xam01SJGgA/bt+GRH98vmWM3aAeQ7SUJfKQgb9/M0M5W4jfrp/9ov5ePRNTL/j FSMGxVFxrd6WvepCTfmjLTOLGFcP3wusG/uASpHZ6X0JZpzyEm1GOqPXEDTQjWnN 0nWQwmdriMJx98WVtk6iG4CPXzGHYGGNxm76UdT85Cat1VFlEQ8+5hu39pQ3p/tg snPyiOvFxZlBueDySghKiND5wBntA/IB9n6ZHH24YityMyu0kTfm0l4Q/nfT/Npj GwQYUd13LQSlHVzzSO7D
=muzV
-----END PGP SIGNATURE-----

David Sommerseth

author

2016-08-08 19:28 UTC #8 lore raw

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

In reply to David Sommerseth

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

[v4 - change disapproved &= syntax ]

[v3 - Remove QUERY_USER_EXEC_ALTERNATIVE macro, simplify
alternatives definition directly in console.h. For now only depend on ENABLE_SYSTEMD]

[v2 - Removed the QUERY_USER_FOREACH macro]

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

---
 configure.ac                  |   2 +-
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.h         |   6 +--
 src/openvpn/console_systemd.c | 115 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 5 deletions(-)
 create mode 100644 src/openvpn/console_systemd.c

configure.ac +1 -1

index 4f14ebd..c3088fd 100644
@@ -1006,7 +1006,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 +1 -1

index 8d6d39f..a306726 100644
@@ -68,7 +68,7 @@ openvpn_SOURCES = \
 	memdbg.h \
 	misc.c misc.h \
 	platform.c platform.h \
-	console.c console.h console_builtin.c \
+	console.c console.h console_builtin.c console_systemd.c \
 	mroute.c mroute.h \
 	mss.c mss.h \
 	mstats.c mstats.h \

src/openvpn/console.h +3 -3

index 44d49ef..41f2b8c 100644
@@ -76,7 +76,7 @@ void query_user_add (char *prompt, size_t prompt_len,
 bool query_user_exec_builtin ();
 
 
-#ifdef QUERY_USER_EXEC_ALTERNATIVE
+#if defined(ENABLE_SYSTEMD)
 /**
  * Executes a configured setup, using the compiled method for querying the user
  *

@@ -86,7 +86,7 @@ bool query_user_exec_builtin ();
  */
 bool query_user_exec ();
 
-#else  /* QUERY_USER_EXEC_ALTERNATIVE not defined*/
+#else  /* ENABLE_SYSTEMD not defined*/
 /**
  * Wrapper function enabling query_user_exec() if no alternative methods have
  * been enabled

@@ -96,7 +96,7 @@ static bool query_user_exec ()
 {
     return query_user_exec_builtin();
 }
-#endif  /* QUERY_USER_EXEC_ALTERNATIVE */
+#endif  /* defined(ENABLE_SYSTEMD) */
 
 
 /**

src/openvpn/console_systemd.c new file +115 -0

115 lines not on this page · raw message

Signature · 1 line

1.8.3.1

David Sommerseth

author

2016-08-12 10:57 UTC #9 lore raw

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

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

[v5 - Ensure NULL termination fix in d09fbf958f1c is included ]

[v4 - change disapproved &= syntax ]

[v3 - Remove QUERY_USER_EXEC_ALTERNATIVE macro, simplify
alternatives definition directly in console.h. For now only depend on ENABLE_SYSTEMD]

[v2 - Removed the QUERY_USER_FOREACH macro]

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

---
 configure.ac                  |   2 +-
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.h         |   6 +--
 src/openvpn/console_systemd.c | 115 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 5 deletions(-)
 create mode 100644 src/openvpn/console_systemd.c

configure.ac +1 -1

index 4f14ebd..c3088fd 100644
@@ -1006,7 +1006,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 +1 -1

index 8d6d39f..a306726 100644
@@ -68,7 +68,7 @@ openvpn_SOURCES = \
 	memdbg.h \
 	misc.c misc.h \
 	platform.c platform.h \
-	console.c console.h console_builtin.c \
+	console.c console.h console_builtin.c console_systemd.c \
 	mroute.c mroute.h \
 	mss.c mss.h \
 	mstats.c mstats.h \

src/openvpn/console.h +3 -3

index 44d49ef..41f2b8c 100644
@@ -76,7 +76,7 @@ void query_user_add (char *prompt, size_t prompt_len,
 bool query_user_exec_builtin ();
 
 
-#ifdef QUERY_USER_EXEC_ALTERNATIVE
+#if defined(ENABLE_SYSTEMD)
 /**
  * Executes a configured setup, using the compiled method for querying the user
  *

@@ -86,7 +86,7 @@ bool query_user_exec_builtin ();
  */
 bool query_user_exec ();
 
-#else  /* QUERY_USER_EXEC_ALTERNATIVE not defined*/
+#else  /* ENABLE_SYSTEMD not defined*/
 /**
  * Wrapper function enabling query_user_exec() if no alternative methods have
  * been enabled

@@ -96,7 +96,7 @@ static bool query_user_exec ()
 {
     return query_user_exec_builtin();
 }
-#endif  /* QUERY_USER_EXEC_ALTERNATIVE */
+#endif  /* defined(ENABLE_SYSTEMD) */
 
 
 /**

src/openvpn/console_systemd.c new file +115 -0

115 lines not on this page · raw message

Signature · 1 line

1.8.3.1

A message that is not in the archive (sent off-list or not yet received): [email protected]...

Selva Nair

2016-10-07 19:56 UTC #11 lore raw

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

This ACK never went to the list.. sending again..

---------- Forwarded message ----------
From: Selva Nair <[email protected]...>
Date: Fri, Aug 26, 2016 at 9:00 PM
Subject: Re: [Openvpn-devel] [PATCH v4.1 2/4] Re-implement the systemd support using the new query user API
To: David Sommerseth <[email protected]...>

Quoted 14 lines · David Sommerseth

This provides exactly the same systemd functionality which existed before the query user infrastructure got implemented.
[v5 - Ensure NULL termination fix in d09fbf958f1c is included ]
[v4 - change disapproved &= syntax ]
[v3 - Remove QUERY_USER_EXEC_ALTERNATIVE macro, simplify
alternatives definition directly in console.h. For now only depend on ENABLE_SYSTEMD]
[v2 - Removed the QUERY_USER_FOREACH macro]
Signed-off-by David Sommerseth <[email protected]...>

Quoted 9 lines

---
 configure.ac                  |   2 +-
 src/openvpn/Makefile.am       |   2 +-
 src/openvpn/console.h         |   6 +--
 src/openvpn/console_systemd.c | 115 ++++++++++++++++++++++++++++++
++++++++++++
 4 files changed, 120 insertions(+), 5 deletions(-)
 create mode 100644 src/openvpn/console_systemd.c

With the NUL termination back in place all look good. ACK (for master only)

Selva

David Sommerseth

author

2016-10-11 10:28 UTC #12 lore raw

Re: [Openvpn-devel] [PATCH applied] Re-implement the systemd support using the new query user API

In reply to David Sommerseth

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Your patch has been applied to the master branch.

commit 3280d8c8f3583d03fed923837447ef45c8f83d52

Author: David Sommerseth
Date:   Fri Aug 12 12:57:25 2016 +0200

     Re-implement the systemd support using the new query user API

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: Selva Nair <[email protected]...>
     Message-Id: [email protected]...
     URL: https://www.mail-archive.com/[email protected]/msg12424.html

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIbBAEBAgAGBQJX/L68AAoJEIbPlEyWcf3y0AUP9A5pJizU5OX08SIejHGavVEa sujLx8k09iNsxmjybqb8PeeUQPXhQ0G4QgiVPrAaU0clfgmoHGgpocFCPSFxcQc2 02XoC6/pFX5pE7QZS4WqUqEVtnCBh10z7hIg1+jUsvwWtXYHZ+sjvn3xg0Oc12hs bItJZpIiNU73Tbfgy71GQlybbmUzi9uoYHfeWHAEpnMDLjDUvZXSp0TKm83WIn3h b5PvVMg0SQWuGVyV3xevYOwP6RsMIphY/jkGcNlA8gxkmewRdWVsbgxdwrJg0zCn WpB4HRySUiQiWeyIqcS8ZmuHmzkHog7Vyi3tJRyHJBGaSR9IsCoTwa+o8TFfYUKs tJ2eilMR4Vy97JgIY1FI7GLE8kF4/YdZc+qJu1Hg7VoHxbndkER4B9v1vA0zAFjl iHQq4WJ4A6WwBW1JLrqSWw8jSeRouqExWp7QXAf88pRmbmDBVP07x1G2VdE+SkE/ ch38QN6mk/NwIzvUmCyyrqeKcjT4cpt6wA5uCAEk1ywHJTX/NFqovr78rXx/n8Er N/PM7ZC8pvZtYqQBopajRgvUHJzQxnJ3IExhXwoO2AKR0GH1V5ISmtjvaV5faoEG 5JBPx4x3eyV17Id8w9cjWBAD1XB+n2wLmIJbPClo0CKLkgF3lUUb7X+dCeLL0C6R O/R5HpmJZBo6suhHbVw=
=GdAC
-----END PGP SIGNATURE-----

David Sommerseth

author

2016-08-08 19:28 UTC #13 lore raw

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

In reply to David Sommerseth

This patch adds extracting the pkg-config --modversion output into a variable configure.ac can consume. The extracted content is put into ${pkgname}_MODVERSION. See config.log for details after ./configure have run.

The patch also modifies slightly how the pkg-config result is processed. Pkg-config can in some situations report more than one line of information if more pkg-config meta-data files are available for the same package. This can cause automake to choke as certain variables may be split over more lines. A simple fix was chosen, using 'tail' to ensure only one line is being processed.

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

---
 m4/pkg.m4 | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

m4/pkg.m4 +6 -3

index 9a71878..94e4589 100644
@@ -73,7 +73,7 @@ m4_define([_PKG_CONFIG],
     pkg_cv_[]$1="$$1"
  elif test -n "$PKG_CONFIG"; then
     PKG_CHECK_EXISTS([$3],
-                     [pkg_cv_[]$1=`$PKG_CONFIG --[]$2 "$3" 2>/dev/null`
+                     [pkg_cv_[]$1=`$PKG_CONFIG --[]$2 "$3" | tail -n1 2>/dev/null`
 		      test "x$?" != "x0" && pkg_failed=yes ],
 		     [pkg_failed=yes])
  else

@@ -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

Selva Nair

2016-08-11 06:45 UTC #14 lore raw

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

Hi,

I missed this patch -- must have deleted from the inbox by mistake. So that explains my silly comment about pkg.m4 in a previous mail.

Quoted 5 lines · David Sommerseth

This patch adds extracting the pkg-config --modversion output into a variable configure.ac can consume. The extracted content is put into ${pkgname}_MODVERSION. See config.log for details after ./configure have run.

Quoted 7 lines

The patch also modifies slightly how the pkg-config result is processed. Pkg-config can in some situations report more than one line of information if more pkg-config meta-data files are available for the same package. This can cause automake to choke as certain variables may be split over more lines. A simple fix was chosen, using 'tail' to ensure only one line is being processed.

Keeping aside the drawbacks of pkg-config, this is not optimal. First, pkg.m4 macro works fine without the piping to tail for --cflags and --libs even with multiple modules by design: With those options pkg-config explicitly does duplicate removal and merges the result onto a single line suitable for setting the LIBS and CFLAGS variables. The option --modversion, on the other hand, is not designed for such a use and is documented to print the version for each module on a separate line. Thus this 'tail hack' is necessitated by trying to use --modversion like --cflags.

Secondly, customizing a well known m4 macro (PKG_CHECK_MODULES) without renaming it doesn't look ideal. Thirdly, the version checking needed to find whether --echo is supported can be done without editing the macro at all. Something like the following added after the PKG_CHECK_MODULES will do

PKG_CHECK_EXISTS( [libsystemd > 216],

                     [AC_DEFINE([SYSTEMD_ASKPASS_HAS_ECHO], [1],
                           [systemd-ask-password supports --echo option])]
                    )

Then add --echo only if SYSTEMD_ASKPASS_HAS_ECHO is defined.

Selva

David Sommerseth

2016-08-11 14:20 UTC #15 lore raw

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

Quoted 5 lines · Selva Nair

Hi,
I missed this patch -- must have deleted from the inbox by mistake. So that explains my silly comment about pkg.m4 in a previous mail.

[...snip...]

Quoted 6 lines

PKG_CHECK_EXISTS( [libsystemd > 216],
                     [AC_DEFINE([SYSTEMD_ASKPASS_HAS_ECHO], [1],
                           [systemd-ask-password supports --echo option])]
                    )
Then add --echo only if SYSTEMD_ASKPASS_HAS_ECHO is defined.

I thought I had tested this approach too and that it failed, hence this patch updating pkg.m4 and all these other hacks; I knew it was hacky but it was something which worked across several of my test boxes at that time. But when I test it now, it works very fine on both Fedora 23 and SL7.2 (RHEL7.2 clone). Well, it's about a year since I fought this, and patch 3+4 have mostly been unchanged isince then.

As this approach is far better, I'll retract patch 3 and update patch 4 to use this approach instead. I'll name the macro a bit different though (SYSTEMD_NEWER_THAN_216), as that reflects more what this pkg-config check does.

So we can completely disregard "[PATCH v4 3/4] autotools: Add support for extracting version information from pkg-config modules".

Signature · 4 lines

kind regards,

David Sommerseth
OpenVPN Technologies, Inc

David Sommerseth

author

2016-08-08 19:28 UTC #16 lore raw

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

In reply to David Sommerseth

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.

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

---
 configure.ac                  | 1 +
 src/openvpn/console_systemd.c | 7 +++++++
 2 files changed, 8 insertions(+)

configure.ac +1 -0

index c3088fd..a44e1df 100644
@@ -1023,6 +1023,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 +7 -0

index bf0f99d..62aba44 100644
@@ -64,6 +64,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

Selva Nair

2016-08-10 19:26 UTC #17 lore raw

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

Hi,

Quoted 8 lines · David Sommerseth

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.

Good to see support for --echo.

configure.ac if test "$enable_systemd" = "yes" ; then · line 1029

Quoted 13 lines

configure.ac +1 -0

index c3088fd..a44e1df 100644
@@ -1023,6 +1023,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])

PKG_CHECK_MODULES() only defines prefix_LIBS and prefix_CFLAGS not prefix_MODVERSION. So how does libsystemd_MODVERSION get defined? Is there a new version of pkg.m4 that does this?

With undefined libsystemd_MODVERSION, SYSTEMD_VERSION will get defined with no value causing SYSTEMD_VERSION > 216 check during compile to error.

By the way, echo ${libsystemd_MODVERSION} is very convoluted -- why not write it simply as

AC_DEFINE_UNQUOTED(SYSTEMD_VERSION, [${libsystemd_MODVERSION}], [systemd version installed])

Selva

David Sommerseth

author

2016-08-11 14:33 UTC #18 lore raw

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

In reply to David Sommerseth

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.

[v2 - Avoid pkg.m4 hacks and use pkgconfig/autoconf methods to flag
if systemd is recent enough for --echo support]

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

---
 configure.ac                  | 6 ++++++
 src/openvpn/console_systemd.c | 8 ++++++++
 2 files changed, 14 insertions(+)

configure.ac +6 -0

index c3088fd..b2fa41c 100644
@@ -1012,6 +1012,12 @@ if test "$enable_systemd" = "yes" ; then
                       [],
                       [PKG_CHECK_MODULES([libsystemd], [libsystemd-daemon])]
                       )
+
+    PKG_CHECK_EXISTS( [libsystemd > 216],
+                     [AC_DEFINE([SYSTEMD_NEWER_THAN_216], [1],
+                           [systemd is newer than v216])]
+                    )
+
     AC_CHECK_HEADERS(systemd/sd-daemon.h,
        ,
        [

src/openvpn/console_systemd.c +8 -0

index bf0f99d..f385c0c 100644
@@ -64,6 +64,14 @@ get_console_input_systemd (const char *prompt, const bool echo, char *input, con
 
     argv_init (&argv);
     argv_printf (&argv, SYSTEMD_ASK_PASSWORD_PATH);
+#ifdef SYSTEMD_NEWER_THAN_216
+    /* the --echo support arrived in upstream systemd 217 */
+    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

Selva Nair

2016-08-12 04:08 UTC #19 lore raw

Re: [Openvpn-devel] [PATCH v4.1] systemd: Do not mask usernames when querying for it via systemd-ask-password

Quoted 52 lines · David Sommerseth

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.
[v2 - Avoid pkg.m4 hacks and use pkgconfig/autoconf methods to flag
if systemd is recent enough for --echo support]
Signed-off-by David Sommerseth <[email protected]...>
---
 configure.ac                  | 6 ++++++
 src/openvpn/console_systemd.c | 8 ++++++++
 2 files changed, 14 insertions(+)
configure.ac +0 -0
index c3088fd..b2fa41c 100644
@@ -1012,6 +1012,12 @@ if test "$enable_systemd" = "yes" ; then
                       [],
                       [PKG_CHECK_MODULES([libsystemd],
[libsystemd-daemon])]
                       )
configure.ac +6 -0
+
+    PKG_CHECK_EXISTS( [libsystemd > 216],
+                     [AC_DEFINE([SYSTEMD_NEWER_THAN_216], [1],
+                           [systemd is newer than v216])]
+                    )
+
     AC_CHECK_HEADERS(systemd/sd-daemon.h,
        ,
        [
src/openvpn/console_systemd.c +0 -0
index bf0f99d..f385c0c 100644
@@ -64,6 +64,14 @@ get_console_input_systemd (const char *prompt, const
bool echo, char *input, con
     argv_init (&argv);
     argv_printf (&argv, SYSTEMD_ASK_PASSWORD_PATH);
src/openvpn/console_systemd.c +1 -0
+#ifdef SYSTEMD_NEWER_THAN_216

src/openvpn/console_systemd.c get_console_input_systemd (const char *prompt, const bool echo, char *input, con · line 77

Quoted 10 lines

src/openvpn/console_systemd.c +7 -0

+    /* the --echo support arrived in upstream systemd 217 */
+    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) {

<http://sdm.link/zohodev2dev>
This looks much better now and works fine (tested without applying patches 1/4 and 2/4). ACK from me.

Selva

David Sommerseth

author

2016-10-11 10:28 UTC #20 lore raw

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

In reply to David Sommerseth

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Your patch has been applied to the master branch.

commit 8ba3e25897af5c7bd7b4f706961e9528d6988d83

Author: David Sommerseth
Date:   Thu Aug 11 16:33:55 2016 +0200

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

     Signed-off-by: David Sommerseth <[email protected]...>
     Acked-by: Selva Nair <[email protected]...>
     Message-Id: [email protected]...
     URL: https://www.mail-archive.com/[email protected]/msg12405.html

- --
kind regards,

David Sommerseth

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJX/L7LAAoJEIbPlEyWcf3yGfoP/1Ass3hBCPiKT20Fq9l4L2eG M2Fy+Duczz5MIOfYZ0z8/B/YSFbmNsxlEb0FIPtJI9N3Xz/vY/5vT741taO+cy5/ DXvp0Oa7mia244SPTzw2dRAoB+WuZhgOkl5MvyoSQcAY84tbq6MXC/oMazC720ua dp828YC7q8Mf+8XSgJKge14cfTpW4RqbZ1X4yB3x/vHexG2VpDv5/V6go1sYRABC dsHu62ZbRRO56dpHDxj+WylH1j+NrPwA6gPyKBSAoqewfMy3j6tm7XShinOmUoz/ eO4aFrWVoa3Y5xqYJ6TfBJnK7aQQJKxaA3WEUVMM6hCOGxDhjfvfdLHydfKn6Mwh 9TnL3Td7ntTnkTgVbkTuUFvaqG5I5XPxombKTH6TXdD604rpasnIuSQOnmEPQwYU Aa2niVI6RfT+/1f/95Tr07ggwGyZwojPTxZmIAASZsMbfWrpOiGevDLT3EpYW5NT T7x/niZy9ri5KvURahXfGCxgh/XzYf3O3TXyQ2+a5oZ4t4oj6uNfAZxB+7UX2uz3 U61kbrBplOQZzQJD3ZAsPzl4ufWoxIv6IVFmtrLQGNmuyXZNFsRtnxbB43BXV5gP bDkbyaRTjiSCGqlXIloMOLClwofRzTfaaovabBLtTFGVNFmhxAFURgbGmiV4X6kB BNNUqsUe49JzATIl1cqm
=YPrD
-----END PGP SIGNATURE-----