Produce a meaningful error message if --daemon gets in the way of asking for passwords.

PATCH In review Gert Doering 2 participants 3 messages Jul 13, 2015

Share & tools

Gert Doering

author

2015-07-13 19:10 UTC #0 lore raw

PATCH 1/1 Produce a meaningful error message if --daemon gets in the way of asking for passwords.

With the --daemon / SSL init reordering in da9b292733, we fail if we daemonize first and then try to ask for a private key passphrase (or, for that matter, username+password if --auth-nocache is set) - but no meaningful error message was printed, instead depending on operating system and library versions, either we looped around "ssl init failed" or died with an unspecified "fatal error".

So: check if get_user_pass_cr() is called in a context that needs "from_stdin", but both stdin and stderr are not connected to a tty device (which getpass() needs). In that case, print a meaningful error message pointing to --askpass, and die.

Trac #574 and #576

Signed-off-by Gert Doering <[email protected]...>

---
 src/openvpn/misc.c | 4 ++++
 1 file changed, 4 insertions(+)

src/openvpn/misc.c +4 -0

index 4fdbf17..c4438b6 100644
@@ -1056,6 +1056,10 @@ get_user_pass_cr (struct user_pass *up,
        */
       else if (from_stdin)
 	{
+	  /* did we --daemon'ize before asking for passwords? */
+	  if ( !isatty(0) && !isatty(2) )
+	    { msg(M_FATAL, "neither stdin nor stderr are a tty device, can't ask for %s password.  If you used --daemon, you need to use --askpass to make passphrase-protected keys work, and you can not use --auth-nocache.", prefix ); }
+
 #ifdef ENABLE_CLIENT_CR
 	  if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
 	    {

Signature · 1 line

2.3.2

Steffan Karger

2015-07-13 19:12 UTC #1 lore raw

Quoted 25 lines · Gert Doering

With the --daemon / SSL init reordering in da9b292733, we fail if we daemonize first and then try to ask for a private key passphrase (or, for that matter, username+password if --auth-nocache is set) - but no meaningful error message was printed, instead depending on operating system and library versions, either we looped around "ssl init failed" or died with an unspecified "fatal error".
So: check if get_user_pass_cr() is called in a context that needs "from_stdin", but both stdin and stderr are not connected to a tty device (which getpass() needs). In that case, print a meaningful error message pointing to --askpass, and die.
Trac #574 and #576
Signed-off-by Gert Doering <[email protected]...>
---
  src/openvpn/misc.c | 4 ++++
  1 file changed, 4 insertions(+)
src/openvpn/misc.c +0 -0
index 4fdbf17..c4438b6 100644

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

Quoted 10 lines

src/openvpn/misc.c +4 -0

@@ -1056,6 +1056,10 @@ get_user_pass_cr (struct user_pass *up,
         */
        else if (from_stdin)
  	{
+	  /* did we --daemon'ize before asking for passwords? */
+	  if ( !isatty(0) && !isatty(2) )
+	    { msg(M_FATAL, "neither stdin nor stderr are a tty device, can't ask for %s password.  If you used --daemon, you need to use --askpass to make passphrase-protected keys work, and you can not use --auth-nocache.", prefix ); }
+
  #ifdef ENABLE_CLIENT_CR
  	  if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
  	    {

ACK

-Steffan

Gert Doering

author

2015-07-13 19:35 UTC #2 lore raw

In reply to Gert Doering

Patch has been applied to the master and release/2.3 branch.

commit 079e5b9c13bf81d7afc6f932b5417d2f08f8e64b (master) commit b131c7b974d9d4d3f0a6ab3a81719af6f7ab2ad6 (release/2.3)

Author: Gert Doering
Date:   Mon Jul 13 21:10:07 2015 +0200

     Produce a meaningful error message if --daemon gets in the way of asking for passwords.

     Signed-off-by: Gert Doering <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/9916

--
kind regards,

Gert Doering