Skip to content

Commit

Permalink
Properly handle exceptions raised while handling server auth messages (
Browse files Browse the repository at this point in the history
…#862)

When server sends us an authentication request message and we fail to
process it, we must terminate the connection and propagate the exception
immediately.  Currently asyncpg will just timeout waiting for
`ReadyForQuery` from the server, which will never arrive.

Fixes: #861
  • Loading branch information
elprans authored Mar 27, 2022
1 parent fb3b6bf commit bd19262
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 10 deletions.
29 changes: 21 additions & 8 deletions asyncpg/protocol/coreproto.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# the Apache 2.0 License: http://www.apache.org/licenses/LICENSE-2.0


from hashlib import md5 as hashlib_md5 # for MD5 authentication
import hashlib


include "scram.pyx"
Expand Down Expand Up @@ -150,15 +150,28 @@ cdef class CoreProtocol:
cdef _process__auth(self, char mtype):
if mtype == b'R':
# Authentication...
self._parse_msg_authentication()
if self.result_type != RESULT_OK:
try:
self._parse_msg_authentication()
except Exception as ex:
# Exception in authentication parsing code
# is usually either malformed authentication data
# or missing support for cryptographic primitives
# in the hashlib module.
self.result_type = RESULT_FAILED
self.result = apg_exc.InternalClientError(
f"unexpected error while performing authentication: {ex}")
self.result.__cause__ = ex
self.con_status = CONNECTION_BAD
self._push_result()
else:
if self.result_type != RESULT_OK:
self.con_status = CONNECTION_BAD
self._push_result()

elif self.auth_msg is not None:
# Server wants us to send auth data, so do that.
self._write(self.auth_msg)
self.auth_msg = None
elif self.auth_msg is not None:
# Server wants us to send auth data, so do that.
self._write(self.auth_msg)
self.auth_msg = None

elif mtype == b'K':
# BackendKeyData
Expand Down Expand Up @@ -634,7 +647,7 @@ cdef class CoreProtocol:

# 'md5' + md5(md5(password + username) + salt))
userpass = ((self.password or '') + (self.user or '')).encode('ascii')
hash = hashlib_md5(hashlib_md5(userpass).hexdigest().\
hash = hashlib.md5(hashlib.md5(userpass).hexdigest().\
encode('ascii') + salt).hexdigest().encode('ascii')

msg.write_bytestring(b'md5' + hash)
Expand Down
9 changes: 7 additions & 2 deletions tests/test_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,13 @@ async def test_auth_password_scram_sha_256(self):
await self.con.execute(alter_password)
await self.con.execute("SET password_encryption = 'md5';")

async def test_auth_unsupported(self):
pass
@unittest.mock.patch('hashlib.md5', side_effect=ValueError("no md5"))
async def test_auth_md5_unsupported(self, _):
with self.assertRaisesRegex(
exceptions.InternalClientError,
".*no md5.*",
):
await self.connect(user='md5_user', password='correctpassword')


class TestConnectParams(tb.TestCase):
Expand Down

0 comments on commit bd19262

Please sign in to comment.