From: Guy Harris <guy@alum.mit.edu>
Date: Mon, 6 Aug 2018 02:42:34 +0000 (-0700)
Subject: On UN*X, don't tell the client why authentication failed.
X-Git-Tag: libpcap-1.10-bp~425
X-Git-Url: https://git.tcpdump.org/libpcap/commitdiff_plain/bff61ee0b5b6b832faba63ec26a224d2be0a6cf0?ds=sidebyside

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 55e3fa30..76d22f28 100644
--- 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;
 	}
 */