Check OpenSSL environment variables before defaulting to certifi

CHANGELOG.md

@@ -7,6 +7,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Github workflow for changelog verification ([#218](https://github.com/opensearch-project/opensearch-py/pull/218))
 ### Changed
 - Updated getting started to user guide ([#233](https://github.com/opensearch-project/opensearch-py/pull/233))
+- Updated CA certificate handling to check OpenSSL environment variables before defaulting to certifi ([#196](https://github.com/opensearch-project/opensearch-py/pull/196))
 ### Deprecated
 
 ### Removed

USER_GUIDE.md

@@ -53,7 +53,14 @@ from opensearchpy import OpenSearch
 host = 'localhost'
 port = 9200
 auth = ('admin', 'admin') # For testing only. Don't store credentials in code.
-ca_certs_path = '/full/path/to/root-ca.pem' # Provide a CA bundle if you use intermediate CAs with your root CA.
+
+# Provide a CA bundle if you use intermediate CAs with your root CA.
+# If this is not given, the CA bundle is is discovered from the first available
+# following options:
+# - OpenSSL environment variables SSL_CERT_FILE and SSL_CERT_DIR
+# - certifi bundle (https://pypi.org/project/certifi/)
+# - default behavior of the connection backend (most likely system certs)
+ca_certs_path = '/full/path/to/root-ca.pem'
 
 # Optional client certificates if you don't want to use HTTP basic authentication.
 # client_cert_path = '/full/path/to/client.pem'

opensearchpy/_async/client/__init__.pyi

@@ -60,7 +60,7 @@ class AsyncOpenSearch(object):
         self,
         hosts: Any = ...,
         transport_class: Type[AsyncTransport] = ...,
-        **kwargs: Any
+        **kwargs: Any,
     ) -> None: ...
     def __repr__(self) -> str: ...
     async def __aenter__(self) -> "AsyncOpenSearch": ...

opensearchpy/_async/http_aiohttp.py

@@ -33,7 +33,7 @@
 import urllib3  # type: ignore
 
 from ..compat import reraise_exceptions, urlencode
-from ..connection.base import Connection
+from ..connection.base import CA_CERTS, Connection
 from ..exceptions import (
     ConnectionError,
     ConnectionTimeout,
@@ -49,15 +49,6 @@
 VERIFY_CERTS_DEFAULT = object()
 SSL_SHOW_WARN_DEFAULT = object()
 
-CA_CERTS = None
-
-try:
-    import certifi
-
-    CA_CERTS = certifi.where()
-except ImportError:
-    pass
-
 
 class AsyncConnection(Connection):
     """Base class for Async HTTP connection implementations"""
@@ -193,20 +184,19 @@ def __init__(
                         "validation. Either pass them in using the ca_certs parameter or "
                         "install certifi to use it automatically."
                     )
+                if os.path.isfile(ca_certs):
+                    ssl_context.load_verify_locations(cafile=ca_certs)
+                elif os.path.isdir(ca_certs):
+                    ssl_context.load_verify_locations(capath=ca_certs)
+                else:
+                    raise ImproperlyConfigured("ca_certs parameter is not a path")
             else:
                 if ssl_show_warn:
                     warnings.warn(
                         "Connecting to %s using SSL with verify_certs=False is insecure."
                         % self.host
                     )
 
-            if os.path.isfile(ca_certs):
-                ssl_context.load_verify_locations(cafile=ca_certs)
-            elif os.path.isdir(ca_certs):
-                ssl_context.load_verify_locations(capath=ca_certs)
-            else:
-                raise ImproperlyConfigured("ca_certs parameter is not a path")
-
             # Use client_cert and client_key variables for SSL certificate configuration.
             if client_cert and not os.path.isfile(client_cert):
                 raise ImproperlyConfigured("client_cert is not a path to a file")

opensearchpy/connection/base.py

@@ -52,6 +52,18 @@
 
 _WARNING_RE = re.compile(r"\"([^\"]*)\"")
 
+# Default CA certificate bundle, preferring those configured in the standard
+# OpenSSL environment variables before provided by certifi (if available)
+CA_CERTS = os.environ.get("SSL_CERT_FILE") or os.environ.get("SSL_CERT_DIR")
+
+if CA_CERTS is None:
+    try:
+        import certifi
+
+        CA_CERTS = certifi.where()
+    except ImportError:
+        pass
+
 
 class Connection(object):
     """

opensearchpy/connection/base.pyi

@@ -42,6 +42,8 @@ from typing import (
 logger: logging.Logger
 tracer: logging.Logger
 
+CA_CERTS: Optional[str]
+
 class Connection(object):
     headers: Dict[str, str]
     use_ssl: bool

opensearchpy/connection/http_requests.py

@@ -42,7 +42,7 @@
     ImproperlyConfigured,
     SSLError,
 )
-from .base import Connection
+from .base import CA_CERTS, Connection
 
 
 class RequestsHttpConnection(Connection):
@@ -54,8 +54,10 @@ class RequestsHttpConnection(Connection):
     :arg use_ssl: use ssl for the connection if `True`
     :arg verify_certs: whether to verify SSL certificates
     :arg ssl_show_warn: show warning when verify certs is disabled
-    :arg ca_certs: optional path to CA bundle. By default standard requests'
-        bundle will be used.
+    :arg ca_certs: optional path to CA bundle. Defaults to configured OpenSSL
+        bundles from environment variables and then certifi before falling
+        back to the standard requests bundle to improve consistency with
+        other Connection implementations
     :arg client_cert: path to the file containing the private key and the
         certificate, or cert only if using client_key
     :arg client_key: path to the file containing the private key if using
@@ -129,6 +131,8 @@ def __init__(
                     "You cannot pass CA certificates when verify SSL is off."
                 )
             self.session.verify = ca_certs
+        elif verify_certs and CA_CERTS:
+            self.session.verify = CA_CERTS
 
         if not ssl_show_warn:
             requests.packages.urllib3.disable_warnings()

opensearchpy/connection/http_urllib3.py

@@ -24,7 +24,6 @@
 #  specific language governing permissions and limitations
 #  under the License.
 
-
 import ssl
 import time
 import warnings
@@ -41,23 +40,14 @@
     ImproperlyConfigured,
     SSLError,
 )
-from .base import Connection
+from .base import CA_CERTS, Connection
 
 # sentinel value for `verify_certs` and `ssl_show_warn`.
 # This is used to detect if a user is passing in a value
 # for SSL kwargs if also using an SSLContext.
 VERIFY_CERTS_DEFAULT = object()
 SSL_SHOW_WARN_DEFAULT = object()
 
-CA_CERTS = None
-
-try:
-    import certifi
-
-    CA_CERTS = certifi.where()
-except ImportError:
-    pass
-
 
 def create_ssl_context(**kwargs):
     """

test_opensearchpy/test_async/test_connection.py

@@ -40,6 +40,7 @@
 
 from opensearchpy import AIOHttpConnection, __versionstr__
 from opensearchpy.compat import reraise_exceptions
+from opensearchpy.connection.base import CA_CERTS
 from opensearchpy.exceptions import ConnectionError
 
 pytestmark = pytest.mark.asyncio
@@ -246,6 +247,23 @@ def test_warns_if_using_non_default_ssl_kwargs_with_ssl_context(self):
                     == str(w[0].message)
                 )
 
+    @patch("ssl.SSLContext.load_verify_locations")
+    def test_uses_given_ca_certs(self, load_verify_locations, tmp_path):
+        path = tmp_path / "ca_certs.pem"
+        path.touch()
+        AIOHttpConnection(use_ssl=True, ca_certs=str(path))
+        load_verify_locations.assert_called_once_with(cafile=str(path))
+
+    @patch("ssl.SSLContext.load_verify_locations")
+    def test_uses_default_ca_certs(self, load_verify_locations):
+        AIOHttpConnection(use_ssl=True)
+        load_verify_locations.assert_called_once_with(cafile=CA_CERTS)
+
+    @patch("ssl.SSLContext.load_verify_locations")
+    def test_uses_no_ca_certs(self, load_verify_locations):
+        AIOHttpConnection(use_ssl=True, verify_certs=False)
+        load_verify_locations.assert_not_called()
+
     @patch("opensearchpy.connection.base.logger")
     async def test_uncompressed_body_logged(self, logger):
         con = await self._get_mock_connection(connection_params={"http_compress": True})

test_opensearchpy/test_connection.py

@@ -52,6 +52,7 @@
     RequestsHttpConnection,
     Urllib3HttpConnection,
 )
+from opensearchpy.connection.base import CA_CERTS
 from opensearchpy.exceptions import (
     ConflictError,
     ConnectionError,
@@ -394,6 +395,19 @@ def test_warns_if_using_non_default_ssl_kwargs_with_ssl_context(self):
                     str(w[0].message),
                 )
 
+    def test_uses_given_ca_certs(self):
+        path = "/path/to/my/ca_certs.pem"
+        c = Urllib3HttpConnection(use_ssl=True, ca_certs=path)
+        self.assertEqual(path, c.pool.ca_certs)
+
+    def test_uses_default_ca_certs(self):
+        c = Urllib3HttpConnection(use_ssl=True)
+        self.assertEqual(CA_CERTS, c.pool.ca_certs)
+
+    def test_uses_no_ca_certs(self):
+        c = Urllib3HttpConnection(use_ssl=True, verify_certs=False)
+        self.assertIsNone(c.pool.ca_certs)
+
     @patch("opensearchpy.connection.base.logger")
     def test_uncompressed_body_logged(self, logger):
         con = self._get_mock_connection(connection_params={"http_compress": True})
@@ -526,6 +540,19 @@ def test_uses_https_if_verify_certs_is_off(self):
         self.assertEqual("GET", request.method)
         self.assertEqual(None, request.body)
 
+    def test_uses_given_ca_certs(self):
+        path = "/path/to/my/ca_certs.pem"
+        c = RequestsHttpConnection(ca_certs=path)
+        self.assertEqual(path, c.session.verify)
+
+    def test_uses_default_ca_certs(self):
+        c = RequestsHttpConnection()
+        self.assertEqual(CA_CERTS, c.session.verify)
+
+    def test_uses_no_ca_certs(self):
+        c = RequestsHttpConnection(verify_certs=False)
+        self.assertFalse(c.session.verify)
+
     def test_nowarn_when_uses_https_if_verify_certs_is_off(self):
         with warnings.catch_warnings(record=True) as w:
             con = self._get_mock_connection(