Prefer SSL connections by default (MagicStack#660)

Switch the default SSL mode from 'disabled' to 'prefer'. This matches libpq's behavior and is a sensible thing to do. Fixes: MagicStack#654

Author: elprans

asyncpg/connect_utils.py

@@ -380,6 +380,7 @@ def _parse_connect_dsn_and_args(*, dsn, host, port, user,
 passfile=passfile)
 
 addrs= []
+have_tcp_addrs=False
 forh, pinzip(host, port):
 ifh.startswith('/'):
 # UNIX socket name
@@ -389,6 +390,7 @@ def _parse_connect_dsn_and_args(*, dsn, host, port, user,
 else:
 # TCP host/port
 addrs.append((h, p))
+have_tcp_addrs=True
 
 ifnotaddrs:
 raiseValueError(
@@ -397,6 +399,9 @@ def _parse_connect_dsn_and_args(*, dsn, host, port, user,
 ifsslisNone:
 ssl=os.getenv('PGSSLMODE')
 
+ifsslisNoneandhave_tcp_addrs:
+ssl='prefer'
+
 # ssl_is_advisory is only allowed to come from the sslmode parameter.
 ssl_is_advisory=None
 ifisinstance(ssl, str):
@@ -435,14 +440,8 @@ def _parse_connect_dsn_and_args(*, dsn, host, port, user,
 ifsslmode<=SSLMODES['require']:
 ssl.verify_mode=ssl_module.CERT_NONE
 ssl_is_advisory=sslmode<=SSLMODES['prefer']
-
-ifssl:
-foraddrinaddrs:
-ifisinstance(addr, str):
-# UNIX socket
-raiseexceptions.InterfaceError(
-'`ssl` parameter can only be enabled for TCP addresses, '
-'got a UNIX socket path:{!r}'.format(addr))
+elifsslisTrue:
+ssl=ssl_module.create_default_context()
 
 ifserver_settingsisnotNoneand (
 notisinstance(server_settings, dict) or
@@ -542,9 +541,6 @@ def connection_lost(self, exc):
 asyncdef_create_ssl_connection(protocol_factory, host, port, *,
 loop, ssl_context, ssl_is_advisory=False):
 
-ifssl_contextisTrue:
-ssl_context=ssl_module.create_default_context()
-
 tr, pr=awaitloop.create_connection(
 lambda: TLSUpgradeProto(loop, host, port,
 ssl_context, ssl_is_advisory),
@@ -625,7 +621,6 @@ async def _connect_addr(
 
 ifisinstance(addr, str):
 # UNIX socket
-assertnotparams.ssl
 connector=loop.create_unix_connection(proto_factory, addr)
 elifparams.ssl:
 connector=_create_ssl_connection(

asyncpg/connection.py

@@ -1869,7 +1869,28 @@ async def connect(dsn=None, *,
  Pass ``True`` or an `ssl.SSLContext <SSLContext_>`_ instance to
  require an SSL connection. If ``True``, a default SSL context
  returned by `ssl.create_default_context() <create_default_context_>`_
- will be used.
+ will be used. The value can also be one of the following strings:
+
+ - ``'disable'`` - SSL is disabled (equivalent to ``False``)
+ - ``'prefer'`` - try SSL first, fallback to non-SSL connection
+ if SSL connection fails
+ - ``'allow'`` - currently equivalent to ``'prefer'``
+ - ``'require'`` - only try an SSL connection. Certificate
+ verifiction errors are ignored
+ - ``'verify-ca'`` - only try an SSL connection, and verify
+ that the server certificate is issued by a trusted certificate
+ authority (CA)
+ - ``'verify-full'`` - only try an SSL connection, verify
+ that the server certificate is issued by a trusted CA and
+ that the requested server host name matches that in the
+ certificate.
+
+ The default is ``'prefer'``: try an SSL connection and fallback to
+ non-SSL connection if that fails.
+
+ .. note::
+
+ *ssl* is ignored for Unix domain socket communication.
 
  :param dict server_settings:
  An optional dict of server runtime parameters. Refer to
@@ -1926,6 +1947,9 @@ async def connect(dsn=None, *,
  .. versionchanged:: 0.22.0
  Added the *record_class* parameter.
 
+ .. versionchanged:: 0.22.0
+ The *ssl* argument now defaults to ``'prefer'``.
+
  .. _SSLContext: https://docs.python.org/3/library/ssl.html#ssl.SSLContext
  .. _create_default_context:
  https://docs.python.org/3/library/ssl.html#ssl.create_default_context

tests/test_connect.py

@@ -318,7 +318,9 @@ class TestConnectParams(tb.TestCase):
 'result': ([('host', 123)],{
 'user': 'user',
 'password': 'passw',
-'database': 'testdb'})
+'database': 'testdb',
+'ssl': True,
+'ssl_is_advisory': True})
  },
 
 {
@@ -384,7 +386,7 @@ class TestConnectParams(tb.TestCase):
 'user': 'user3',
 'password': '123123',
 'database': 'abcdef',
-'ssl': ssl.SSLContext,
+'ssl': True,
 'ssl_is_advisory': True})
  },
 
@@ -461,7 +463,7 @@ class TestConnectParams(tb.TestCase):
 'user': 'me',
 'password': 'ask',
 'database': 'db',
-'ssl': ssl.SSLContext,
+'ssl': True,
 'ssl_is_advisory': False})
  },
 
@@ -545,6 +547,7 @@ class TestConnectParams(tb.TestCase):
 {
 'user': 'user',
 'database': 'user',
+'ssl': None
  }
  )
  },
@@ -574,7 +577,9 @@ class TestConnectParams(tb.TestCase):
  ('localhost', 5433)
  ],{
 'user': 'spam',
-'database': 'db'
+'database': 'db',
+'ssl': True,
+'ssl_is_advisory': True
  }
  )
  },
@@ -617,7 +622,7 @@ def run_testcase(self, testcase):
 password=testcase.get('password')
 passfile=testcase.get('passfile')
 database=testcase.get('database')
-ssl=testcase.get('ssl')
+sslmode=testcase.get('ssl')
 server_settings=testcase.get('server_settings')
 
 expected=testcase.get('result')
@@ -640,21 +645,26 @@ def run_testcase(self, testcase):
 
 addrs, params=connect_utils._parse_connect_dsn_and_args(
 dsn=dsn, host=host, port=port, user=user, password=password,
-passfile=passfile, database=database, ssl=ssl,
+passfile=passfile, database=database, ssl=sslmode,
 connect_timeout=None, server_settings=server_settings)
 
-params={k: vfork, vinparams._asdict().items()
-ifvisnotNone}
+params={
+k: vfork, vinparams._asdict().items()
+ifvisnotNoneor (expectedisnotNoneandkinexpected[1])
+ }
+
+ifisinstance(params.get('ssl'), ssl.SSLContext):
+params['ssl'] =True
 
 result= (addrs, params)
 
 ifexpectedisnotNone:
-fork, vinexpected[1].items():
-# If `expected` contains a type, allow that to "match" any
-# instance of that type tyat `result` may contain. We need
-# this because different SSLContexts don't compare equal.
-ifisinstance(v, type) andisinstance(result[1].get(k), v):
-result[1][k] =v
+if'ssl'notinexpected[1]:
+# Avoid the hassle of specifying the default SSL mode
+# unless explicitly tested for.
+params.pop('ssl', None)
+params.pop('ssl_is_advisory', None)
+
 self.assertEqual(expected, result, 'Testcase:{}'.format(testcase))
 
 deftest_test_connect_params_environ(self):
@@ -1063,16 +1073,6 @@ async def verify_fails(sslmode):
 awaitverify_fails('verify-ca')
 awaitverify_fails('verify-full')
 
-asyncdeftest_connection_ssl_unix(self):
-ssl_context=ssl.SSLContext(ssl.PROTOCOL_SSLv23)
-ssl_context.load_verify_locations(SSL_CA_CERT_FILE)
-
-withself.assertRaisesRegex(asyncpg.InterfaceError,
-'can only be enabled for TCP addresses'):
-awaitself.connect(
-host='/tmp',
-ssl=ssl_context)
-
 asyncdeftest_connection_implicit_host(self):
 conn_spec=self.get_connection_spec()
 con=awaitasyncpg.connect(