Stop using deprecated getpass()

The getpass() function is present in SUSv2, but marked LEGACY. It is
removed in POSIX.1-2001. Additionally, on Solaris getpass() returns
maximum 9 bytes. This will make longer passwords fail with no
possibility for user to know what is happening.

This patch removes usage of getpass() completely and replaces it with
direct implementation of what getpass() does: opens tty (existing code),
outputs the prompt (existing code), turns off echoing (new code), reads
one line (existing code shared with echoed mode), restores tty state
(new code) and closes tty (existing code).

Patch v2: incorporate review feedback, incl. style fixes, merge
          termios.h check in configure.ac with an existing
          AC_CHECK_HEADERS, add error check and logging after
          tcsettattr() when restoring tty settings

Signed-off-by: Tõivo Leedjärv <toivol@gmail.com>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210328171151.12056-1-toivol@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21889.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/configure.ac b/configure.ac
index 8e31dcb29c4c44115cdcb76ba90899665dad572c..3cb9fc2fc2dd18e168e08a264cfb1e99acf5907e 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -435,7 +435,7 @@ AC_CHECK_HEADERS([ \
        sys/time.h sys/ioctl.h sys/stat.h \
        sys/mman.h sys/file.h sys/wait.h \
        unistd.h signal.h libgen.h stropts.h \
-       syslog.h pwd.h grp.h \
+       syslog.h pwd.h grp.h termios.h \
        sys/sockio.h sys/uio.h linux/sockios.h \
        linux/types.h poll.h sys/epoll.h err.h \
 ])
@@ -649,7 +649,7 @@ AC_FUNC_FORK
 
 AC_CHECK_FUNCS([ \
        daemon chroot getpwnam setuid nice system getpid dup dup2 \
-       getpass syslog openlog mlockall getrlimit getgrnam setgid \
+       syslog openlog mlockall getrlimit getgrnam setgid \
        setgroups stat flock readv writev time gettimeofday \
        ctime memset vsnprintf strdup \
        setsid chdir putenv getpeername unlink \

diff --git a/src/openvpn/console_builtin.c b/src/openvpn/console_builtin.c
index 445928bff4c25c51e211060349e4d76649ddea20..22ac9f7973c26a628e1d4bafee1623eb5e61de0d 100644 (file)
--- a/src/openvpn/console_builtin.c
+++ b/src/openvpn/console_builtin.c
@@ -40,6 +40,10 @@
 #include "buffer.h"
 #include "misc.h"
 
+#ifdef HAVE_TERMIOS_H
+#include <termios.h>
+#endif
+
 #ifdef _WIN32
 
 #include "win32.h"
@@ -138,7 +142,7 @@ get_console_input_win32(const char *prompt, const bool echo, char *input, const
 #endif   /* _WIN32 */
 
 
-#ifdef HAVE_GETPASS
+#ifdef HAVE_TERMIOS_H
 
 /**
  * Open the current console TTY for read/write operations
@@ -177,7 +181,7 @@ close_tty(FILE *fp)
     }
 }
 
-#endif   /* HAVE_GETPASS */
+#endif   /* HAVE_TERMIOS_H */
 
 
 /**
@@ -201,7 +205,9 @@ get_console_input(const char *prompt, const bool echo, char *input, const int ca
 
 #if defined(_WIN32)
     return get_console_input_win32(prompt, echo, input, capacity);
-#elif defined(HAVE_GETPASS)
+#elif defined(HAVE_TERMIOS_H)
+    bool restore_tty = false;
+    struct termios tty_tmp, tty_save;
 
     /* did we --daemon'ize before asking for passwords?
      * (in which case neither stdin or stderr are connected to a tty and
@@ -220,33 +226,41 @@ get_console_input(const char *prompt, const bool echo, char *input, const int ca
         close(fd);
     }
 
-    if (echo)
-    {
-        FILE *fp;
+    FILE *fp = open_tty(true);
+    fprintf(fp, "%s", prompt);
+    fflush(fp);
+    close_tty(fp);
 
-        fp = open_tty(true);
-        fprintf(fp, "%s", prompt);
-        fflush(fp);
-        close_tty(fp);
+    fp = open_tty(false);
 
-        fp = open_tty(false);
-        if (fgets(input, capacity, fp) != NULL)
-        {
-            chomp(input);
-            ret = true;
-        }
-        close_tty(fp);
+    if (!echo && (tcgetattr(fileno(fp), &tty_tmp) == 0))
+    {
+        tty_save = tty_tmp;
+        tty_tmp.c_lflag &= ~(ECHO | ECHOE | ECHOK | ECHONL | ISIG);
+        restore_tty = (tcsetattr(fileno(fp), TCSAFLUSH, &tty_tmp) == 0);
     }
-    else
+
+    if (fgets(input, capacity, fp) != NULL)
+    {
+        chomp(input);
+        ret = true;
+    }
+
+    if (restore_tty)
     {
-        char *gp = getpass(prompt);
-        if (gp)
+        if (tcsetattr(fileno(fp), TCSAFLUSH, &tty_save) == -1)
         {
-            strncpynt(input, gp, capacity);
-            secure_memzero(gp, strlen(gp));
-            ret = true;
+            msg(M_WARN | M_ERRNO, "tcsetattr() failed to restore tty settings");
         }
+
+        /* Echo the non-echoed newline */
+        close_tty(fp);
+        fp = open_tty(true);
+        fprintf(fp, "\n");
+        fflush(fp);
     }
+
+    close_tty(fp);
 #else  /* if defined(_WIN32) */
     msg(M_FATAL, "Sorry, but I can't get console input on this OS (%s)", prompt);
 #endif /* if defined(_WIN32) */