Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle auth_type=password when the stored password is md5 #129

Closed

Conversation

@hashbrowncipher
Copy link

hashbrowncipher commented Apr 18, 2016

When used in the typical way (by querying pg_shadow), auth_query or userlist
generation will return md5 hashed passwords. Under such a situation, we should
still allow clients to authenticate with unhashed passwords. We should also
correctly handle the situation where we have an md5 hashed password, but the
server is expecting a plaintext one. We do this by caching passwords that we
have determined match the expected hash. We can then send those passwords
upstream to a server.

When used in the typical way (by querying pg_shadow), auth_query or userlist
generation will return md5 hashed passwords. Under such a situation, we should
still allow clients to authenticate with unhashed passwords. We should also
correctly handle the situation where we have an md5 hashed password, but the
server is expecting a plaintext one. We do this by caching passwords that we
have determined match the expected hash. We can then send those passwords
upstream to a server.
@PJMODOS

This comment has been minimized.

Copy link
Contributor

PJMODOS commented May 31, 2016

I like the first part of this, but I don't like the caching of plaintext passwords, that's potential security hole.

@hashbrowncipher

This comment has been minimized.

Copy link
Author

hashbrowncipher commented May 31, 2016

I should clarify that the functionality enabled by this change is better than the equivalent functionality with md5 authentication.

With md5 authentication enabled on the server, a prospective client needs only the md5 hashed password in order to authenticate: they do not need the original password. This is a known issue with Postgresql authentication. In effect, with md5 authentication enabled, the user's hashed password is useful equally to their unhashed password. So the md5 authentication handshake provides no value in terms of securing the password on disk, but it does obfuscate the password by re-hashing it with a 4-byte nonce before sending it over the wire. This is vulnerable to an attacker which can build a dictionary of authentication attempts, so it's only marginally valuable as a security measure against a very casual attacker.

For users like myself who force Postgres connections over TLS, the additional obfuscation provides no value at all, and accepting md5'ed passwords provides negative value. I therefore disable md5 authentication entirely in pg_hba.conf, which restores the value of the md5 hashing algorithm [1].

My understanding of pgbouncer's architecture is that it relies upon holding a cached copy of whatever credential is necessary in order to connect to Postgres. Without this PR it caches md5 hashed passwords (which are useful on their own to log in). With this PR it can also cache plaintext passwords. This is a security improvement: previously pgbouncer administrators would have to put useful authentication material (i.e. md5 hashes) into userlist.txt or provide it via auth_query. By disabling Postgres's "pass-the-hash" misfeature, the hashes provided by userlist.txt and auth_query become unuseful: instead a real plaintext password must be supplied by the user in order to log in the first time.

[1] Note that it doesn't provide key stretching, so high security (128 bit or higher) passwords are still important.

@petere petere closed this in 50b89eb Aug 3, 2019
@petere

This comment has been minimized.

Copy link
Contributor

petere commented Aug 3, 2019

The first part of this has been committed, as this is how the PostgreSQL server also works.

The second part (caching of passwords) was disputed. There is probably value in it, but it would need to be done consistently across also password-based authentication methods. Also, it's not strictly needed, since PgBouncer can also use the md5-encrypted password to authenticate to a backend server. With SCRAM support looming, it's not clear how to handle all this, so this would need some deeper thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.