From 058d38f0aea00cd9cef7dc7086b6f29992e5a15e Mon Sep 17 00:00:00 2001 From: Nick Lang Date: Sun, 4 Mar 2018 15:09:40 -0800 Subject: [PATCH] Use use ssl_context or don't but don't mix (#714) * Use original SSL process and add SSLContext Not going to deprecate and replace with SSLContext. But instead give option for using SSLContext next to the original way of handling SSL. --- README | 4 +- docs/connection.rst | 17 +++--- docs/index.rst | 4 +- elasticsearch/connection/http_urllib3.py | 76 +++++++++++++----------- test_elasticsearch/test_connection.py | 33 +++++----- 5 files changed, 71 insertions(+), 63 deletions(-) diff --git a/README b/README index bb306bb4..0e18f16c 100644 --- a/README +++ b/README @@ -97,9 +97,9 @@ Elastic Cloud (and SSL) use-case:: Using SSL Context with a self-signed cert use-case:: >>> from elasticsearch import Elasticsearch - >>> from elasticsearch.connection import create_ssl_context + >>> from ssl import create_default_context - >>> context = create_ssl_context(cafile="path/to/cafile.pem") + >>> context = create_default_context(cafile="path/to/cafile.pem") >>> es = Elasticsearch("https://elasticsearch.url:port", ssl_context=context, http_auth=('elastic','yourpassword')) >>> es.info() diff --git a/docs/connection.rst b/docs/connection.rst index a14d19ae..325069bb 100644 --- a/docs/connection.rst +++ b/docs/connection.rst @@ -45,23 +45,20 @@ Connection Selector Urllib3HttpConnection (default connection_class) ------------------------------------------------ -Deprecation Notice: `use_ssl`, `verify_certs`, `ca_certs` and `ssl_version` are being -deprecated in favor of using a `SSLContext` (https://docs.python.org/3/library/ssl.html#ssl.SSLContext) object. - -You can continue to use the deprecated parameters and an `SSLContext` will be created for you. - -If you want to create your own `SSLContext` object you can create one natively using the -python SSL library with the `create_default_context` (https://docs.python.org/3/library/ssl.html#ssl.create_default_context) method -or you can use the wrapper function :function:`~elasticsearch.connection.http_urllib3.create_ssl_context`. +If you have complex SSL logic for connecting to Elasticsearch using an `SSLContext` object +might be more helpful. You can create one natively using the python SSL library with the +`create_default_context` (https://docs.python.org/3/library/ssl.html#ssl.create_default_context) method. To create an `SSLContext` object you only need to use one of cafile, capath or cadata:: - >>> from elasticsearch.connection import create_ssl_context - >>> context = create_ssl_context(cafile=None, capath=None, cadata=None) + >>> from ssl import create_default_context + >>> context = create_default_context(cafile=None, capath=None, cadata=None) * `cafile` is the path to your CA File * `capath` is the directory of a collection of CA's * `cadata` is either an ASCII string of one or more PEM-encoded certificates or a bytes-like object of DER-encoded certificates. +Please note that the use of SSLContext is only available for Urllib3. + .. autoclass:: Urllib3HttpConnection :members: diff --git a/docs/index.rst b/docs/index.rst index e4218a9d..0354a504 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -200,9 +200,9 @@ elasticsearch cluster, including certificate verification and http auth:: # SSL client authentication using client_cert and client_key - from elasticsearch.connection import create_ssl_context + from ssl import create_default_context - context = create_ssl_context(cafile="path/to/cert.pem") + context = create_default_context(cafile="path/to/cert.pem") es = Elasticsearch( ['localhost', 'otherhost'], http_auth=('user', 'secret'), diff --git a/elasticsearch/connection/http_urllib3.py b/elasticsearch/connection/http_urllib3.py index 163ff0cc..36787d5a 100644 --- a/elasticsearch/connection/http_urllib3.py +++ b/elasticsearch/connection/http_urllib3.py @@ -4,6 +4,11 @@ import urllib3 from urllib3.exceptions import ReadTimeoutError, SSLError as UrllibSSLError import warnings +# sentinal value for `verify_certs`. +# This is used to detect if a user is passing in a value for `verify_certs` +# so we can raise a warning if using SSL kwargs AND SSLContext. +VERIFY_CERTS_DEFAULT = None + CA_CERTS = None try: @@ -41,8 +46,8 @@ class Urllib3HttpConnection(Connection): string or a tuple :arg use_ssl: use ssl for the connection if `True` :arg verify_certs: whether to verify SSL certificates - :arg ca_certs: optional path to CA bundle. See - https://urllib3.readthedocs.io/en/latest/security.html#using-certifi-with-urllib3 + :arg ca_certs: optional path to CA bundle. + See https://urllib3.readthedocs.io/en/latest/security.html#using-certifi-with-urllib3 for instructions how to get default set :arg client_cert: path to the file containing the private key and the certificate, or cert only if using client_key @@ -59,7 +64,7 @@ class Urllib3HttpConnection(Connection): :arg headers: any custom http headers to be add to requests """ def __init__(self, host='localhost', port=9200, http_auth=None, - use_ssl=False, verify_certs=True, ca_certs=None, client_cert=None, + use_ssl=False, verify_certs=VERIFY_CERTS_DEFAULT, ca_certs=None, client_cert=None, client_key=None, ssl_version=None, ssl_assert_hostname=None, ssl_assert_fingerprint=None, maxsize=10, headers=None, ssl_context=None, **kwargs): @@ -80,48 +85,51 @@ class Urllib3HttpConnection(Connection): kw = {} # if providing an SSL context, raise error if any other SSL related flag is used - if ssl_context and (ca_certs or ssl_version): - raise ImproperlyConfigured("When using `ssl_context`, `use_ssl`, `verify_certs`, `ca_certs` and `ssl_version` are not permitted") + if ssl_context and ( (verify_certs is not VERIFY_CERTS_DEFAULT) or ca_certs + or client_cert or client_key or ssl_version): + warnings.warn("When using `ssl_context`, all other SSL related kwargs are ignored") # if ssl_context provided use SSL by default - if self.use_ssl or ssl_context: - ca_certs = CA_CERTS if ca_certs is None else ca_certs - - if not ca_certs and not ssl_context and verify_certs: - # If no ca_certs and no sslcontext passed and asking to verify certs - # raise error - raise ImproperlyConfigured("Root certificates are missing for certificate " - "validation. Either pass them in using the ca_certs parameter or " - "install certifi to use it automatically.") - if verify_certs or ca_certs or ssl_version: - warnings.warn('Use of `verify_certs`, `ca_certs`, `ssl_version` have been deprecated in favor of using SSLContext`', DeprecationWarning) + if ssl_context and self.use_ssl: pool_class = urllib3.HTTPSConnectionPool + kw.update({ + 'assert_fingerprint': ssl_assert_fingerprint, + 'ssl_context': ssl_context, + }) + self.pool = pool_class(host, port=port, timeout=self.timeout, maxsize=maxsize, **kw) - if not ssl_context: - # if SSLContext hasn't been passed in, create one. - # need to skip if sslContext isn't avail - try: - ssl_context = create_ssl_context(cafile=ca_certs) - except AttributeError: - ssl_context = None - - if not verify_certs and ssl_context is not None: - ssl_context.check_hostname = False - ssl_context.verify_mode = ssl.CERT_NONE - warnings.warn( - 'Connecting to %s using SSL with verify_certs=False is insecure.' % host) - + elif self.use_ssl: + pool_class = urllib3.HTTPSConnectionPool kw.update({ 'ssl_version': ssl_version, 'assert_hostname': ssl_assert_hostname, 'assert_fingerprint': ssl_assert_fingerprint, - 'ssl_context': ssl_context, - 'cert_file': client_cert, - 'ca_certs': ca_certs, - 'key_file': client_key, }) + + # If `verify_certs` is sentinal value, default `verify_certs` to `True` + if verify_certs is VERIFY_CERTS_DEFAULT: + verify_certs = True + + ca_certs = CA_CERTS if ca_certs is None else ca_certs + if verify_certs: + if not ca_certs: + raise ImproperlyConfigured("Root certificates are missing for certificate " + "validation. Either pass them in using the ca_certs parameter or " + "install certifi to use it automatically.") + + kw.update({ + 'cert_reqs': 'CERT_REQUIRED', + 'ca_certs': ca_certs, + 'cert_file': client_cert, + 'key_file': client_key, + }) + else: + warnings.warn( + 'Connecting to %s using SSL with verify_certs=False is insecure.' % host) + self.pool = pool_class(host, port=port, timeout=self.timeout, maxsize=maxsize, **kw) + def perform_request(self, method, url, params=None, body=None, timeout=None, ignore=(), headers=None): url = self.url_prefix + url if params: diff --git a/test_elasticsearch/test_connection.py b/test_elasticsearch/test_connection.py index 66ac2afa..e03fdf63 100644 --- a/test_elasticsearch/test_connection.py +++ b/test_elasticsearch/test_connection.py @@ -10,11 +10,28 @@ from elasticsearch.exceptions import TransportError, ConflictError, RequestError from elasticsearch.connection import RequestsHttpConnection, \ Urllib3HttpConnection from elasticsearch.exceptions import ImproperlyConfigured -from elasticsearch.connection.http_urllib3 import create_ssl_context from .test_cases import TestCase, SkipTest class TestUrllib3Connection(TestCase): + def test_ssl_context(self): + try: + context = ssl.create_default_context() + except AttributeError: + # if create_default_context raises an AttributeError Exception + # it means SSLContext is not available for that version of python + # and we should skip this test. + raise SkipTest( + "Test test_ssl_context is skipped cause SSLContext is not available for this version of ptyhon") + + con = Urllib3HttpConnection(use_ssl=True, ssl_context=context) + self.assertEqual(len(con.pool.conn_kw.keys()), 1) + self.assertIsInstance( + con.pool.conn_kw['ssl_context'], + ssl.SSLContext + ) + self.assertTrue(con.use_ssl) + def test_timeout_set(self): con = Urllib3HttpConnection(timeout=42) self.assertEquals(42, con.timeout) @@ -45,12 +62,6 @@ class TestUrllib3Connection(TestCase): 'connection': 'keep-alive'}, con.headers) def test_uses_https_if_verify_certs_is_off(self): - if ( - sys.version_info >= (3,0) and sys.version_info <= (3,4) - ) or ( - sys.version_info >= (2,6) and sys.version_info <= (2,7) - ): - raise SkipTest("SSL Context not supported in this version of python") with warnings.catch_warnings(record=True) as w: con = Urllib3HttpConnection(use_ssl=True, verify_certs=False) self.assertEquals(1, len(w)) @@ -62,14 +73,6 @@ class TestUrllib3Connection(TestCase): con = Urllib3HttpConnection() self.assertIsInstance(con.pool, urllib3.HTTPConnectionPool) - def test_ssl_context_and_depreicated_values(self): - try: - ctx = create_ssl_context() - except AttributeError: - raise SkipTest("SSL Context not supported in this version of python") - self.assertRaises(ImproperlyConfigured, Urllib3HttpConnection, ssl_context=ctx, ca_certs="/some/path/to/cert.crt") - self.assertRaises(ImproperlyConfigured, Urllib3HttpConnection, ssl_context=ctx, ssl_version=ssl.PROTOCOL_SSLv23) - class TestRequestsConnection(TestCase): def _get_mock_connection(self, connection_params={}, status_code=200, response_body='{}'): con = RequestsHttpConnection(**connection_params)