Use use ssl_context or don't but don't mix (elastic#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.

Author: fxdgear

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()
 

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:

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'),

elasticsearch/connection/http_urllib3.py

@@ -4,6 +4,11 @@
 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 @@ def __init__(self, host='localhost', port=9200, http_auth=None,
         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:

test_elasticsearch/test_connection.py

@@ -10,11 +10,28 @@
 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 @@ def test_http_auth_list(self):
             '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 @@ def test_doesnt_use_https_if_not_specified(self):
         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)