Skip to content

Commit 484d60c

Browse files
committed
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.
1 parent 437b273 commit 484d60c

File tree

1 file changed

+44
-7
lines changed

1 file changed

+44
-7
lines changed

Diff for: rpcapd/daemon.c

+44-7
Original file line numberDiff line numberDiff line change
@@ -1176,20 +1176,33 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
11761176
* stop trying to log in with a given user name and move on
11771177
* to another user name.
11781178
*/
1179+
DWORD error;
11791180
HANDLE Token;
1181+
char errmsgbuf[PCAP_ERRBUF_SIZE]; // buffer for errors to log
1182+
11801183
if (LogonUser(username, ".", password, LOGON32_LOGON_NETWORK, LOGON32_PROVIDER_DEFAULT, &Token) == 0)
11811184
{
1182-
pcap_fmt_errmsg_for_win32_err(errbuf, PCAP_ERRBUF_SIZE,
1183-
GetLastError(), "LogonUser() failed");
1185+
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
1186+
error = GetLastError();
1187+
if (error != ERROR_LOGON_FAILURE)
1188+
{
1189+
// Some error other than an authentication error;
1190+
// log it.
1191+
pcap_fmt_errmsg_for_win32_err(errmsgbuf,
1192+
PCAP_ERRBUF_SIZE, error, "LogonUser() failed");
1193+
rpcapd_log(LOGPRIO_ERROR, "%s", errmsgbuf);
1194+
}
11841195
return -1;
11851196
}
11861197

11871198
// This call should change the current thread to the selected user.
11881199
// I didn't test it.
11891200
if (ImpersonateLoggedOnUser(Token) == 0)
11901201
{
1191-
pcap_fmt_errmsg_for_win32_err(errbuf, PCAP_ERRBUF_SIZE,
1202+
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
1203+
pcap_fmt_errmsg_for_win32_err(errmsgbuf, PCAP_ERRBUF_SIZE,
11921204
GetLastError(), "ImpersonateLoggedOnUser() failed");
1205+
rpcapd_log(LOGPRIO_ERROR, "%s", errmsgbuf);
11931206
CloseHandle(Token);
11941207
return -1;
11951208
}
@@ -1217,6 +1230,7 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
12171230
* only password database or some other authentication mechanism,
12181231
* behind its API.
12191232
*/
1233+
int error;
12201234
struct passwd *user;
12211235
char *user_password;
12221236
#ifdef HAVE_GETSPNAM
@@ -1227,15 +1241,15 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
12271241
// This call is needed to get the uid
12281242
if ((user = getpwnam(username)) == NULL)
12291243
{
1230-
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed: user name or password incorrect");
1244+
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
12311245
return -1;
12321246
}
12331247

12341248
#ifdef HAVE_GETSPNAM
12351249
// This call is needed to get the password; otherwise 'x' is returned
12361250
if ((usersp = getspnam(username)) == NULL)
12371251
{
1238-
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed: user name or password incorrect");
1252+
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
12391253
return -1;
12401254
}
12411255
user_password = usersp->sp_pwdp;
@@ -1253,29 +1267,52 @@ daemon_AuthUserPwd(char *username, char *password, char *errbuf)
12531267
user_password = user->pw_passwd;
12541268
#endif
12551269

1270+
//
1271+
// The Single UNIX Specification says that if crypt() fails it
1272+
// sets errno, but some implementatons that haven't been run
1273+
// through the SUS test suite might not do so.
1274+
//
1275+
errno = 0;
12561276
crypt_password = crypt(password, user_password);
12571277
if (crypt_password == NULL)
12581278
{
1279+
error = errno;
12591280
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
1281+
if (error == 0)
1282+
{
1283+
// It didn't set errno.
1284+
rpcapd_log(LOGPRIO_ERROR, "crypt() failed");
1285+
}
1286+
else
1287+
{
1288+
rpcapd_log(LOGPRIO_ERROR, "crypt() failed: %s",
1289+
strerror(error));
1290+
}
12601291
return -1;
12611292
}
12621293
if (strcmp(user_password, crypt_password) != 0)
12631294
{
1264-
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed: user name or password incorrect");
1295+
pcap_snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed");
12651296
return -1;
12661297
}
12671298

12681299
if (setuid(user->pw_uid))
12691300
{
1301+
error = errno;
12701302
pcap_fmt_errmsg_for_errno(errbuf, PCAP_ERRBUF_SIZE,
1271-
errno, "setuid");
1303+
error, "setuid");
1304+
rpcapd_log(LOGPRIO_ERROR, "setuid() failed: %s",
1305+
strerror(error));
12721306
return -1;
12731307
}
12741308

12751309
/* if (setgid(user->pw_gid))
12761310
{
1311+
error = errno;
12771312
pcap_fmt_errmsg_for_errno(errbuf, PCAP_ERRBUF_SIZE,
12781313
errno, "setgid");
1314+
rpcapd_log(LOGPRIO_ERROR, "setgid() failed: %s",
1315+
strerror(error));
12791316
return -1;
12801317
}
12811318
*/

0 commit comments

Comments
 (0)