diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d38f5604..51ace031 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,13 +39,21 @@ jobs: strategy: fail-fast: false matrix: - python-version: [2.7, 3.5, 3.6, 3.7, 3.8, 3.9] + python-version: ['3.7', '3.8', '3.9', '3.10'] + os: [ubuntu-latest] experimental: [false] include: - - python-version: 3.10.0-beta.2 - experimental: true + - python-version: '2.7' + os: ubuntu-20.04 + experimental: false + - python-version: '3.5' + os: ubuntu-20.04 + experimental: false + - python-version: '3.6' + os: ubuntu-20.04 + experimental: false - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} name: test-${{ matrix.python-version }} continue-on-error: ${{ matrix.experimental }} steps: diff --git a/CHANGELOG.md b/CHANGELOG.md index 38652577..5e7624eb 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/USER_GUIDE.md b/USER_GUIDE.md index 1b4a7df7..3d3a777f 100644 --- a/USER_GUIDE.md +++ b/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' diff --git a/opensearchpy/_async/client/__init__.pyi b/opensearchpy/_async/client/__init__.pyi index 41ed2ba9..cc423451 100644 --- a/opensearchpy/_async/client/__init__.pyi +++ b/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": ... diff --git a/opensearchpy/_async/http_aiohttp.py b/opensearchpy/_async/http_aiohttp.py index c5353067..20e89ea0 100644 --- a/opensearchpy/_async/http_aiohttp.py +++ b/opensearchpy/_async/http_aiohttp.py @@ -49,15 +49,6 @@ from .compat import get_running_loop 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""" @@ -185,7 +176,7 @@ class AIOHttpConnection(AsyncConnection): ssl_context.check_hostname = False ssl_context.verify_mode = ssl.CERT_NONE - ca_certs = CA_CERTS if ca_certs is None else ca_certs + ca_certs = self.default_ca_certs() if ca_certs is None else ca_certs if verify_certs: if not ca_certs: raise ImproperlyConfigured( @@ -193,6 +184,12 @@ class AIOHttpConnection(AsyncConnection): "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( @@ -200,13 +197,6 @@ class AIOHttpConnection(AsyncConnection): % 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") diff --git a/opensearchpy/connection/base.py b/opensearchpy/connection/base.py index b2f2ef23..435996b9 100644 --- a/opensearchpy/connection/base.py +++ b/opensearchpy/connection/base.py @@ -304,3 +304,22 @@ class Connection(object): def _get_default_user_agent(self): return "opensearch-py/%s (Python %s)" % (__versionstr__, python_version()) + + @staticmethod + def default_ca_certs(): + """ + Get the default CA certificate bundle, preferring those configured in + the standard OpenSSL environment variables before those provided by + certifi (if available) + """ + ca_certs = os.environ.get("SSL_CERT_FILE") or os.environ.get("SSL_CERT_DIR") + + if ca_certs: + return ca_certs + + try: + import certifi + except ImportError: + pass + else: + return certifi.where() diff --git a/opensearchpy/connection/base.pyi b/opensearchpy/connection/base.pyi index 8ade2530..7e51d20c 100644 --- a/opensearchpy/connection/base.pyi +++ b/opensearchpy/connection/base.pyi @@ -114,3 +114,5 @@ class Connection(object): self, status_code: int, raw_data: str, content_type: Optional[str] ) -> NoReturn: ... def _get_default_user_agent(self) -> str: ... + @staticmethod + def default_ca_certs() -> Optional[str]: ... diff --git a/opensearchpy/connection/http_requests.py b/opensearchpy/connection/http_requests.py index e4e5cb38..a918a635 100644 --- a/opensearchpy/connection/http_requests.py +++ b/opensearchpy/connection/http_requests.py @@ -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,10 @@ class RequestsHttpConnection(Connection): "You cannot pass CA certificates when verify SSL is off." ) self.session.verify = ca_certs + elif verify_certs: + ca_certs = self.default_ca_certs() + if ca_certs: + self.session.verify = ca_certs if not ssl_show_warn: requests.packages.urllib3.disable_warnings() diff --git a/opensearchpy/connection/http_urllib3.py b/opensearchpy/connection/http_urllib3.py index c57a6190..51a4f2d9 100644 --- a/opensearchpy/connection/http_urllib3.py +++ b/opensearchpy/connection/http_urllib3.py @@ -24,7 +24,6 @@ # specific language governing permissions and limitations # under the License. - import ssl import time import warnings @@ -49,15 +48,6 @@ from .base import Connection 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): """ @@ -186,7 +176,7 @@ class Urllib3HttpConnection(Connection): if ssl_show_warn is SSL_SHOW_WARN_DEFAULT: ssl_show_warn = True - ca_certs = CA_CERTS if ca_certs is None else ca_certs + ca_certs = self.default_ca_certs() if ca_certs is None else ca_certs if verify_certs: if not ca_certs: raise ImproperlyConfigured( diff --git a/opensearchpy/helpers/test.pyi b/opensearchpy/helpers/test.pyi index 06721f8e..1363f821 100644 --- a/opensearchpy/helpers/test.pyi +++ b/opensearchpy/helpers/test.pyi @@ -30,7 +30,6 @@ from unittest import TestCase from ..client import OpenSearch OPENSEARCH_URL: str -CA_CERTS: str def get_test_client(nowait: bool = ..., **kwargs: Any) -> OpenSearch: ... def _get_version(version_string: str) -> Tuple[int, ...]: ... diff --git a/test_opensearchpy/test_async/test_connection.py b/test_opensearchpy/test_async/test_connection.py index fb8ca660..14f3a3a2 100644 --- a/test_opensearchpy/test_async/test_connection.py +++ b/test_opensearchpy/test_async/test_connection.py @@ -40,6 +40,7 @@ from multidict import CIMultiDict from opensearchpy import AIOHttpConnection, __versionstr__ from opensearchpy.compat import reraise_exceptions +from opensearchpy.connection import Connection from opensearchpy.exceptions import ConnectionError pytestmark = pytest.mark.asyncio @@ -246,6 +247,25 @@ class TestAIOHttpConnection: == 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=Connection.default_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}) diff --git a/test_opensearchpy/test_connection.py b/test_opensearchpy/test_connection.py index 31b8f756..f5523ee2 100644 --- a/test_opensearchpy/test_connection.py +++ b/test_opensearchpy/test_connection.py @@ -62,6 +62,11 @@ from opensearchpy.exceptions import ( from .test_cases import SkipTest, TestCase +try: + from pytest import MonkeyPatch +except ImportError: # Old version of pytest for 2.7 and 3.5 + from _pytest.monkeypatch import MonkeyPatch + def gzip_decompress(data): buf = gzip.GzipFile(fileobj=io.BytesIO(data), mode="rb") @@ -155,6 +160,28 @@ class TestBaseConnection(TestCase): finally: os.environ.pop("ELASTIC_CLIENT_APIVERSIONING") + def test_ca_certs_ssl_cert_file(self): + cert = "/path/to/clientcert.pem" + with MonkeyPatch().context() as monkeypatch: + monkeypatch.setenv("SSL_CERT_FILE", cert) + assert Connection.default_ca_certs() == cert + + def test_ca_certs_ssl_cert_dir(self): + cert = "/path/to/clientcert/dir" + with MonkeyPatch().context() as monkeypatch: + monkeypatch.setenv("SSL_CERT_DIR", cert) + assert Connection.default_ca_certs() == cert + + def test_ca_certs_certifi(self): + import certifi + + assert Connection.default_ca_certs() == certifi.where() + + def test_no_ca_certs(self): + with MonkeyPatch().context() as monkeypatch: + monkeypatch.setitem(sys.modules, "certifi", None) + assert Connection.default_ca_certs() is None + class TestUrllib3Connection(TestCase): def _get_mock_connection(self, connection_params={}, response_body=b"{}"): @@ -394,6 +421,19 @@ class TestUrllib3Connection(TestCase): 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(Connection.default_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 +566,19 @@ class TestRequestsConnection(TestCase): 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(Connection.default_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(