diff mbox series

[Openvpn-devel,1/1] console_systemd: remove the timeout when using `systemd-ask-password`

Message ID 20241231204629.1210040-2-ben.boeckel@kitware.com
State Accepted
Headers
Series Remove system password timeout |

Commit Message

Ben Boeckel Dec. 31, 2024, 8:46 p.m. UTC 
  Without this, the password request will expire after 90 seconds leaving
no way to provide the password without OpenVPN asking for it again.
Given that interactive use will wait for input without a timeout, it
makes sense to have non-interactive usage also wait until the user is
ready instead of forcing users to race against the timeout.
---
 src/openvpn/console_systemd.c | 1 +
 1 file changed, 1 insertion(+)

Comments

David Sommerseth Jan. 8, 2025, 2:59 p.m. UTC | #1 
On 31/12/2024 21:46, Ben Boeckel via Openvpn-devel wrote:
> Without this, the password request will expire after 90 seconds leaving
> no way to provide the password without OpenVPN asking for it again.
> Given that interactive use will wait for input without a timeout, it
> makes sense to have non-interactive usage also wait until the user is
> ready instead of forcing users to race against the timeout.
> ---
>   src/openvpn/console_systemd.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c
> index cc91cd10..b208a614 100644
> --- a/src/openvpn/console_systemd.c
> +++ b/src/openvpn/console_systemd.c
> @@ -71,6 +71,7 @@ get_console_input_systemd(const char *prompt, const bool echo, char *input, cons
>       }
>   #endif
>       argv_printf_cat(&argv, "--icon network-vpn");
> +    argv_printf_cat(&argv, "--timeout=0");
>       argv_printf_cat(&argv, "%s", prompt);
>   
>       if ((std_out = openvpn_popen(&argv, NULL)) < 0)

Thanks a lot!  Since I wrote this integration years ago ... I'd like to 
chime in here.

Generally, change looks conceptually good and I agree to the reasoning 
for this change.  What I'd like to ensure is that we're not hitting some 
systemd version regression situations.

The oldest Linux distributions OpenVPN 2.x care about today are, to my 
knowledge, RHEL/Alma Linux/Rocky Linux 8 on the RPM side.  On the .deb 
side of things, I believe Debian 12 is the oldest supported stable release.

 From what I see, RHEL-8 ships with systemd v239, which has this 
argument.  I don't have a Debian 12 (or 11, if OpenVPN 2.x is still 
supported there) handy now to check.

Given that the oldest Linux distro with systemd which supports the
--timeout argument in systemd-ask-password, this can get my ACK.
Ben Boeckel Jan. 8, 2025, 3:17 p.m. UTC | #2 
On Wed, Jan 08, 2025 at 15:59:42 +0100, David Sommerseth wrote:
> Given that the oldest Linux distro with systemd which supports the
> --timeout argument in systemd-ask-password, this can get my ACK.

Thanks. I thought to look at when it was introduced. It has been there
(manpage documentation was added in commit
f3bc7fdc7bf47193a9f8618a7d22a6ceec2df6f7) since 2011, released with
systemd v25. I think we can assume anything using a modern openvpn is
also using something newer than this as well.

--Ben
David Sommerseth Jan. 13, 2025, 11:44 a.m. UTC | #3 
On 31/12/2024 21:46, Ben Boeckel via Openvpn-devel wrote:
> Without this, the password request will expire after 90 seconds leaving
> no way to provide the password without OpenVPN asking for it again.
> Given that interactive use will wait for input without a timeout, it
> makes sense to have non-interactive usage also wait until the user is
> ready instead of forcing users to race against the timeout.
> ---
>   src/openvpn/console_systemd.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c
> index cc91cd10..b208a614 100644
> --- a/src/openvpn/console_systemd.c
> +++ b/src/openvpn/console_systemd.c
> @@ -71,6 +71,7 @@ get_console_input_systemd(const char *prompt, const bool echo, char *input, cons
>       }
>   #endif
>       argv_printf_cat(&argv, "--icon network-vpn");
> +    argv_printf_cat(&argv, "--timeout=0");
>       argv_printf_cat(&argv, "%s", prompt);
>   
>       if ((std_out = openvpn_popen(&argv, NULL)) < 0)

Given the confirmation by Ben in this reply [1], I give this my ACK. 
I've also double checked the git commit log in systemd to verify his 
finding.

Acked-By: David Sommerseth <davids@openvpn.net>


[1] 
<https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30362.html>
Frank Lichtenheld Jan. 14, 2025, 1:47 p.m. UTC | #4 
On Mon, Jan 13, 2025 at 12:44:48PM +0100, David Sommerseth via Openvpn-devel wrote:
> Given the confirmation by Ben in this reply [1], I give this my ACK. I've
> also double checked the git commit log in systemd to verify his finding.
> 
> Acked-By: David Sommerseth <davids@openvpn.net>
> 
> 
> [1] <https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30362.html>

I created https://gerrit.openvpn.net/c/openvpn/+/866 in Gerrit from this
patch which contains the following clean-ups to the commit:
 - Fix "Author:" line to not contain "Via OpenVPN-Devel"
 - Add Signed-off-by line to commit message

Doing this as part of an experiment to add mail-submitted patches
to Gerrit.

Regards,
Gert Doering Jan. 14, 2025, 5:04 p.m. UTC | #5 
Hi,

On Tue, Jan 14, 2025 at 02:47:58PM +0100, Frank Lichtenheld wrote:
> Doing this as part of an experiment to add mail-submitted patches
> to Gerrit.

I *was* wondering since we had an ACK on the list already :-)

gert
Gert Doering Jan. 14, 2025, 5:17 p.m. UTC | #6 
I have taken this from the mailing list, as it has a thread hanging off
it with an ACK in it - which is easier than making the gerrit submission
a mail again, so it has a message-id to link to.

Not much to test here - it's a trivial addition of an argv argument, and
if David says "this is good" then it is :-)  (gerrit build tests also
agree that it does not break anything).

Your patch has been applied to the master and release/2.6 branch
(it could be argued whether it's a bugfix or a feature).

commit 8084990ccecbf803498419e553bc1a6f073f8175 (master)
commit f67bd153a18192e0a8ed9b57738d8589f26369e3 (release/2.6)
Author: Ben Boeckel
Date:   Tue Dec 31 21:46:29 2024 +0100

     console_systemd: remove the timeout when using 'systemd-ask-password'

     Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
     Message-Id: <20241231204629.1210040-2-ben.boeckel@kitware.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30336.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Ben Boeckel Jan. 14, 2025, 5:58 p.m. UTC | #7 
On Tue, Jan 14, 2025 at 18:17:30 +0100, Gert Doering wrote:
> I have taken this from the mailing list, as it has a thread hanging off
> it with an ACK in it - which is easier than making the gerrit submission
> a mail again, so it has a message-id to link to.

Thanks!

> Not much to test here - it's a trivial addition of an argv argument, and
> if David says "this is good" then it is :-)  (gerrit build tests also
> agree that it does not break anything).
> 
> Your patch has been applied to the master and release/2.6 branch
> (it could be argued whether it's a bugfix or a feature).

Looking forward to it :) .

--Ben
diff mbox series

Patch

diff --git a/src/openvpn/console_systemd.c b/src/openvpn/console_systemd.c
index cc91cd10..b208a614 100644
--- a/src/openvpn/console_systemd.c
+++ b/src/openvpn/console_systemd.c
@@ -71,6 +71,7 @@  get_console_input_systemd(const char *prompt, const bool echo, char *input, cons
     }
 #endif
     argv_printf_cat(&argv, "--icon network-vpn");
+    argv_printf_cat(&argv, "--timeout=0");
     argv_printf_cat(&argv, "%s", prompt);
 
     if ((std_out = openvpn_popen(&argv, NULL)) < 0)