On UN*X, don't tell the client why authentication failed.

"no such user" tells the client that the user ID isn't valid and,
therefore, that it needn't bother trying to do password cracking for
that user ID; just saying that the authentication failed dosn't give
them that hint.

This resolves the third problem in Include Security issue F11: [libpcap]
Remote Packet Capture Daemon Multiple Authentication Improvements.

The Windows LogonUser() API returns ERROR_LOGON_FAILURE for both cases,
so the Windows code doesn't have this issue.  Just return the same
"Authentication failed" message on Windows to the user.

For various authentication failures *other* than "no such user" and
"password not valid", log a message, as there's a problem that may need
debugging.  We don't need to tell the end user what the problem is, as
they may not bother reporting it and, even if they do, they may not give
the full error message.

diff --git a/rpcapd/daemon.c b/rpcapd/daemon.c
index 55e3fa309242469440b5cb2e04c323fad9f731d7..76d22f2808626b2977e7a58bb10c769d71f51fb4 100644 (file)
--- a/rpcapd/daemon.c
+++ b/rpcapd/daemon.c
@@ -1412,11 +1412,22 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
         * stop trying to log in with a given user name and move on
         * to another user name.
         */
+       DWORD error;
        HANDLE Token;
+       char errmsgbuf[PCAP_ERRBUF_SIZE];       // buffer for errors to log
+
        if (LogonUser(username, ".", password, LOGON32_LOGON_NETWORK, LOGON32_PROVIDER_DEFAULT, &Token) == 0)
        {
-               pcap_fmt_errmsg_for_win32_err(errbuf, PCAP_ERRBUF_SIZE,
-                   GetLastError(), "LogonUser() failed");
+               snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
+               error = GetLastError();
+               if (error != ERROR_LOGON_FAILURE)
+               {
+                       // Some error other than an authentication error;
+                       // log it.
+                       pcap_fmt_errmsg_for_win32_err(errmsgbuf,
+                           PCAP_ERRBUF_SIZE, error, "LogonUser() failed");
+                       rpcapd_log(LOGPRIO_ERROR, "%s", errmsgbuf);
+               }                           
                return -1;
        }
 
@@ -1424,8 +1435,10 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
        // I didn't test it.
        if (ImpersonateLoggedOnUser(Token) == 0)
        {
-               pcap_fmt_errmsg_for_win32_err(errbuf, PCAP_ERRBUF_SIZE,
+               snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
+               pcap_fmt_errmsg_for_win32_err(errmsgbuf, PCAP_ERRBUF_SIZE,
                    GetLastError(), "ImpersonateLoggedOnUser() failed");
+               rpcapd_log(LOGPRIO_ERROR, "%s", errmsgbuf);
                CloseHandle(Token);
                return -1;
        }
@@ -1453,6 +1466,7 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
         * only password database or some other authentication mechanism,
         * behind its API.
         */
+       int error;
        struct passwd *user;
        char *user_password;
 #ifdef HAVE_GETSPNAM
@@ -1463,7 +1477,7 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
        // This call is needed to get the uid
        if ((user = getpwnam(username)) == NULL)
        {
-               snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed: user name or password incorrect");
+               snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
                return -1;
        }
 
@@ -1471,7 +1485,7 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
        // This call is needed to get the password; otherwise 'x' is returned
        if ((usersp = getspnam(username)) == NULL)
        {
-               snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed: user name or password incorrect");
+               snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
                return -1;
        }
        user_password = usersp->sp_pwdp;
@@ -1489,29 +1503,52 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
        user_password = user->pw_passwd;
 #endif
 
+       //
+       // The Single UNIX Specification says that if crypt() fails it
+       // sets errno, but some implementatons that haven't been run
+       // through the SUS test suite might not do so.
+       //
+       errno = 0;
        crypt_password = crypt(password, user_password);
        if (crypt_password == NULL)
        {
+               error = errno;
                snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
+               if (error == 0)
+               {
+                       // It didn't set errno.
+                       rpcapd_log(LOGPRIO_ERROR, "crypt() failed");
+               }
+               else
+               {
+                       rpcapd_log(LOGPRIO_ERROR, "crypt() failed: %s",
+                           strerror(error));
+               }
                return -1;
        }
        if (strcmp(user_password, crypt_password) != 0)
        {
-               snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed: user name or password incorrect");
+               snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
                return -1;
        }
 
        if (setuid(user->pw_uid))
        {
+               error = errno;
                pcap_fmt_errmsg_for_errno(errbuf, PCAP_ERRBUF_SIZE,
-                   errno, "setuid");
+                   error, "setuid");
+               rpcapd_log(LOGPRIO_ERROR, "setuid() failed: %s",
+                   strerror(error));
                return -1;
        }
 
 /*     if (setgid(user->pw_gid))
        {
+               error = errno;
                pcap_fmt_errmsg_for_errno(errbuf, PCAP_ERRBUF_SIZE,
                    errno, "setgid");
+               rpcapd_log(LOGPRIO_ERROR, "setgid() failed: %s",
+                   strerror(error));
                return -1;
        }
 */