Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f6fc72c

Browse files
committed
Don't allow logging in with empty password.
Some authentication methods allowed it, others did not. In the client-side, libpq does not even try to authenticate with an empty password, which makes using empty passwords hazardous: an administrator might think that an account with an empty password cannot be used to log in, because psql doesn't allow it, and not realize that a different client would in fact allow it. To clear that confusion and to be be consistent, disallow empty passwords in all authentication methods. All the authentication methods that used plaintext authentication over the wire, except for BSD authentication, already checked that the password received from the user was not empty. To avoid forgetting it in the future again, move the check to the recv_password_packet function. That only forbids using an empty password with plaintext authentication, however. MD5 and SCRAM need a different fix: * In stable branches, check that the MD5 hash stored for the user does not not correspond to an empty string. This adds some overhead to MD5 authentication, because the server needs to compute an extra MD5 hash, but it is not noticeable in practice. * In HEAD, modify CREATE and ALTER ROLE to clear the password if an empty string, or a password hash that corresponds to an empty string, is specified. The user-visible behavior is the same as in the stable branches, the user cannot log in, but it seems better to stop the empty password from entering the system in the first place. Secondly, it is fairly expensive to check that a SCRAM hash doesn't correspond to an empty string, because computing a SCRAM hash is much more expensive than an MD5 hash by design, so better avoid doing that on every authentication. We could clear the password on CREATE/ALTER ROLE also in stable branches, but we would still need to check at authentication time, because even if we prevent empty passwords from being stored in pg_authid, there might be existing ones there already. Reported by Jeroen van der Ham, Ben de Graaff and Jelte Fennema. Security: CVE-2017-7546
1 parent 49c1935 commit f6fc72c

File tree

2 files changed

+41
-20
lines changed

2 files changed

+41
-20
lines changed

src/backend/libpq/auth.c

+16-20
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,20 @@ recv_password_packet(Port *port)
697697
(errcode(ERRCODE_PROTOCOL_VIOLATION),
698698
errmsg("invalid password packet size")));
699699

700+
/*
701+
* Don't allow an empty password. Libpq treats an empty password the same
702+
* as no password at all, and won't even try to authenticate. But other
703+
* clients might, so allowing it would be confusing.
704+
*
705+
* Note that this only catches an empty password sent by the client in
706+
* plaintext. There's another check in md5_crypt_verify to prevent an
707+
* empty password from being used with MD5 authentication.
708+
*/
709+
if (buf.data[0] == '\0')
710+
ereport(ERROR,
711+
(errcode(ERRCODE_INVALID_PASSWORD),
712+
errmsg("empty password returned by client")));
713+
700714
/* Do not echo password to logs, for security. */
701715
elog(DEBUG5, "received password packet");
702716

@@ -1820,12 +1834,6 @@ pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg,
18201834
*/
18211835
goto fail;
18221836
}
1823-
if (strlen(passwd) == 0)
1824-
{
1825-
ereport(LOG,
1826-
(errmsg("empty password returned by client")));
1827-
goto fail;
1828-
}
18291837
}
18301838
if ((reply[i].resp = strdup(passwd)) == NULL)
18311839
goto fail;
@@ -2146,16 +2154,11 @@ CheckLDAPAuth(Port *port)
21462154
if (passwd == NULL)
21472155
return STATUS_EOF; /* client wouldn't send password */
21482156

2149-
if (strlen(passwd) == 0)
2150-
{
2151-
ereport(LOG,
2152-
(errmsg("empty password returned by client")));
2153-
return STATUS_ERROR;
2154-
}
2155-
21562157
if (InitializeLDAPConnection(port, &ldap) == STATUS_ERROR)
2158+
{
21572159
/* Error message already sent */
21582160
return STATUS_ERROR;
2161+
}
21592162

21602163
if (port->hba->ldapbasedn)
21612164
{
@@ -2509,13 +2512,6 @@ CheckRADIUSAuth(Port *port)
25092512
if (passwd == NULL)
25102513
return STATUS_EOF; /* client wouldn't send password */
25112514

2512-
if (strlen(passwd) == 0)
2513-
{
2514-
ereport(LOG,
2515-
(errmsg("empty password returned by client")));
2516-
return STATUS_ERROR;
2517-
}
2518-
25192515
if (strlen(passwd) > RADIUS_MAX_PASSWORD_LENGTH)
25202516
{
25212517
ereport(LOG,

src/backend/libpq/crypt.c

+25
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,37 @@ md5_crypt_verify(const Port *port, const char *role, char *client_pass,
7474

7575
ReleaseSysCache(roleTup);
7676

77+
/*
78+
* Don't allow an empty password. Libpq treats an empty password the same
79+
* as no password at all, and won't even try to authenticate. But other
80+
* clients might, so allowing it would be confusing.
81+
*
82+
* For a plaintext password, we can simply check that it's not an empty
83+
* string. For an encrypted password, check that it does not match the MD5
84+
* hash of an empty string.
85+
*/
7786
if (*shadow_pass == '\0')
7887
{
7988
*logdetail = psprintf(_("User \"%s\" has an empty password."),
8089
role);
8190
return STATUS_ERROR; /* empty password */
8291
}
92+
if (isMD5(shadow_pass))
93+
{
94+
char crypt_empty[MD5_PASSWD_LEN + 1];
95+
96+
if (!pg_md5_encrypt("",
97+
port->user_name,
98+
strlen(port->user_name),
99+
crypt_empty))
100+
return STATUS_ERROR;
101+
if (strcmp(shadow_pass, crypt_empty) == 0)
102+
{
103+
*logdetail = psprintf(_("User \"%s\" has an empty password."),
104+
role);
105+
return STATUS_ERROR; /* empty password */
106+
}
107+
}
83108

84109
/*
85110
* Compare with the encrypted or plain password depending on the

0 commit comments

Comments
 (0)