Fix isatty() check for good.
PATCH 1/1 Fix isatty() check for good.
Commit 079e5b9c13 introduced a check to see if we --daemon'ized before trying to ask for a password (which would then fail with a non-intuitive error), breaking querying systemd under certain conditions.
Move check from get_user_pass_cr() to get_console_input() and make it "full featured" by not only checking isatty() for stdin/stderr but also trying to open /dev/tty in case we still have a controlling tty - which is what getpass() does under the hood, so if either of this works, we're fine.
Trac #618 and #630
--- src/openvpn/console.c | 13 +++++++++++++ src/openvpn/misc.c | 6 ------ 2 files changed, 13 insertions(+), 6 deletions(-)
src/openvpn/console.c +13 -0
@@ -208,6 +208,19 @@ get_console_input (const char *prompt, const bool echo, char *input, const int c#if defined(WIN32 )return get_console_input_win32 (prompt, echo, input, capacity);#elif defined(HAVE_GETPASS ) + +/* did we --daemon'ize before asking for passwords? +* (in which case neither stdin or stderr are connected to a tty and +* /dev/tty can not be open()ed anymore) +*/ +if ( !isatty (0 ) && !isatty (2 ) ) + { +int fd =open ("/dev/tty" ,O_RDWR ); +if ( fd <0 ) + {msg (M_FATAL ,"neither stdin nor stderr are a tty device and you have neither a controlling tty nor systemd - can't ask for '%s'. If you used --daemon, you need to use --askpass to make passphrase-protected keys work, and you can not use --auth-nocache." , prompt ); } +close (fd); + } +if (echo) {FILE *fp;
src/openvpn/misc.c +0 -6
@@ -1137,12 +1137,6 @@ get_user_pass_cr (struct user_pass *up,*/ if (username_from_stdin || password_from_stdin) { -#ifndef WIN32 -/* 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 ); } -#endif -#ifdef ENABLE_CLIENT_CR if (auth_challenge && (flags &GET_USER_PASS_DYNAMIC_CHALLENGE )) {
Signature · 1 line
2.4.9
Hi,
Thanks for this fix. Systemd-ask-password now works!
src/openvpn/console.c get_console_input (const char *prompt, const bool echo, char *input, const int c · line 219
Quoted 5 lines · Gert Doering
src/openvpn/console.c +4
-0+if ( !isatty (0 ) && !isatty (2 ) ) + { +int fd =open ("/dev/tty" ,O_RDWR ); +if ( fd <0 )
This doesn't allow for reading passwords from pipes -- not relevant if getpass doesn't fall back to a non-tty stdin if /dev/tty open fails -- does it? Not saying pipes should be supported --- whatever is consistent with getpass should be fine.
Quoted 6 lines
+ { msg(M_FATAL, "neither stdin nor stderr are a tty device and you
have neither a controlling tty nor systemd - can't ask for '%s'. If you used --daemon, you need to use --askpass to make passphrase-protected keys work, and you can not use --auth-nocache.", prompt ); }
+ close(fd);
The error message is still incomplete/confusing when the triggered by auth-user-pass
without a filename (i.e stdin), and not by a passphrase-protected key. In the former case a reference to the need for management-query or a username-password
file is helpful.
Other than that, the patch does the right thing, and works well. Tested using git- master with and without systemd. ACK from me.
Selva
Hi,
Quoted 1 line · Selva Nair
Thanks for this fix. Systemd-ask-password now works!
Thanks :-)
Quoted 8 lines · Gert Doering
src/openvpn/console.c get_console_input (const char *prompt, const bool echo, char *input, const int c · line 219
Quoted 6 lines
src/openvpn/console.c +4
-0+if ( !isatty (0 ) && !isatty (2 ) ) + { +int fd =open ("/dev/tty" ,O_RDWR ); +if ( fd <0 )
Quoted 4 lines
This doesn't allow for reading passwords from pipes -- not relevant if getpass doesn't fall back to a non-tty stdin if
/dev/ttyopen fails -- does it? Not saying pipes should be supported --- whatever is consistent with getpass should be fine.
I'm not sure this is a use case we actually have - to trigger this, you'd need to run openvpn like this
echo mypass | openvpn 2>/dev/null
(so isatty(2) fails) *and* run it from an RC script or something else that does not have a controlling tty.
Given that we broke this in 2.3.8 already, and the only complaints we heard was "systemd is not working anymore" (apologies!), I think that scenario is somewhat unlikely.
Quoted 14 lines
Quoted 6 lines
+ { msg(M_FATAL, "neither stdin nor stderr are a tty device and you
have neither a controlling tty nor systemd - can't ask for '%s'. If you used --daemon, you need to use --askpass to make passphrase-protected keys work, and you can not use --auth-nocache.", prompt ); }
+ close(fd);The error message is still incomplete/confusing when the triggered by auth-user-pass
without a filename (i.e stdin), and not by a passphrase-protected key. In the former case a reference to the need for management-query or a username-password
file is helpful.
Quoted 2 lines
Other than that, the patch does the right thing, and works well. Tested using git- master with and without systemd. ACK from me.
Given these two comments, shall we merge it "as it is", or do you want to suggest different text? I'm fine with changing the text, but have no good suggestions.
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]...Hi,
As this fixes an important bug, please do merge as is.
I vaguely recall askpass behaves differently on linux than, say, bsd, so even if reading from a pipe (when no controlling tty) is possible, it may not always work. Will test it some other time..
Quoted 6 lines · Gert Doering
Quoted 2 lines
Other than that, the patch does the right thing, and works well. Tested using git- master with and without systemd. ACK from me.
Given these two comments, shall we merge it "as it is", or do you want to suggest different text? I'm fine with changing the text, but have no good suggestions.
Well, any wording is going to be incomplete, perhaps, so leave it as is..
Selva
Quoted 4 lines · Selva Nair
I vaguely recall askpass behaves differently on linux than, say, bsd, so even if reading from a pipe (when no controlling tty) is possible, it may not always work. Will test it some other time..
Meant to say "getpass" not "askpass"
Selva
Patch has been applied to the master and release/2.3 branch.
commit 015fe7177181fb4944ddf33debcfcd20c62ba55a (master) commit db55abd9e28546874edd78fa50df594a78e794b9 (release/2.3)
Author: Gert Doering Date: Wed Dec 9 21:03:55 2015 +0100
Fix isatty() check for good.
Signed-off-by: Gert Doering <[email protected]...>
Acked-by: Selva Nair <[email protected]...>
Message-Id: <[email protected]...>
URL: http://article.gmane.org/gmane.network.openvpn.devel/10709--
kind regards,
Gert Doering