fix: always load the default certs on our custom SSL context (#196)

This is a follow-up PR of a previous one where we started restricting the
maximum version of the `request` package to avoid a recently introduced
regression.
This PR contains the fix for that, so we can continue using always the
latest version. Furthermore, the HTTPS related tests have been improved
and moved to the directory of the unit tests, to always include them in our builds.

Signed-off-by: Norbert Biczo <[email protected]>

Author: pyrooka

ibm_cloud_sdk_core/base_service.py

@@ -30,6 +30,7 @@
 from ibm_cloud_sdk_core.authenticators import Authenticator
 from .api_exception import ApiException
 from .detailed_response import DetailedResponse
+from .http_adapter import SSLHTTPAdapter
 from .token_managers.token_manager import TokenManager
 from .utils import (
     has_bad_first_or_last_char,
@@ -38,7 +39,6 @@
     cleanup_values,
     read_external_sources,
     strip_extra_slashes,
-    SSLHTTPAdapter,
     GzipStream,
 )
 from .private_helpers import _build_user_agent

ibm_cloud_sdk_core/http_adapter.py

@@ -0,0 +1,27 @@
+import ssl
+
+from requests.adapters import HTTPAdapter, DEFAULT_POOLBLOCK
+from urllib3.util.ssl_ import create_urllib3_context
+
+
+# pylint: disable=fixme
+class SSLHTTPAdapter(HTTPAdapter):
+    """Wraps the original HTTP adapter and adds additional SSL context."""
+
+    def __init__(self, *args, **kwargs):
+        self._disable_ssl_verification = kwargs.pop('_disable_ssl_verification', None)
+
+        super().__init__(*args, **kwargs)
+
+    def init_poolmanager(self, connections, maxsize, block=DEFAULT_POOLBLOCK, **pool_kwargs):
+        """Create and use custom SSL configuration."""
+
+        ssl_context = create_urllib3_context()
+        ssl_context.load_default_certs()
+        ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2
+
+        if self._disable_ssl_verification:
+            ssl_context.check_hostname = False
+            ssl_context.verify_mode = ssl.CERT_NONE
+
+        super().init_poolmanager(connections, maxsize, block, ssl_context=ssl_context, **pool_kwargs)

ibm_cloud_sdk_core/utils.py

@@ -19,41 +19,14 @@
 import io
 import json as json_import
 import re
-import ssl
 from os import getenv, environ, getcwd
 from os.path import isfile, join, expanduser
 from typing import List, Union
 from urllib.parse import urlparse, parse_qs
 
-from requests.adapters import HTTPAdapter, DEFAULT_POOLBLOCK
-from urllib3.util.ssl_ import create_urllib3_context
-
 import dateutil.parser as date_parser
 
 
-# pylint: disable=fixme
-# TODO: revert the change in the `requirement.txt` once this class become deprecated!
-class SSLHTTPAdapter(HTTPAdapter):
-    """Wraps the original HTTP adapter and adds additional SSL context."""
-
-    def __init__(self, *args, **kwargs):
-        self._disable_ssl_verification = kwargs.pop('_disable_ssl_verification', None)
-
-        super().__init__(*args, **kwargs)
-
-    def init_poolmanager(self, connections, maxsize, block=DEFAULT_POOLBLOCK, **pool_kwargs):
-        """Create and use custom SSL configuration."""
-
-        ssl_context = create_urllib3_context()
-        ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2
-
-        if self._disable_ssl_verification:
-            ssl_context.check_hostname = False
-            ssl_context.verify_mode = ssl.CERT_NONE
-
-        super().init_poolmanager(connections, maxsize, block, ssl_context=ssl_context, **pool_kwargs)
-
-
 class GzipStream(io.RawIOBase):
     """Compress files on the fly.
 

requirements.txt

@@ -1,4 +1,4 @@
-requests>=2.31.0,<2.32.3
+requests>=2.31.0,<3.0.0
 urllib3>=2.1.0,<3.0.0
 python_dateutil>=2.8.2,<3.0.0
 PyJWT>=2.8.0,<3.0.0

resources/test_ssl.cert

@@ -0,0 +1,19 @@
+-----BEGIN CERTIFICATE-----
+MIIDETCCAfmgAwIBAgIUVQfATsBxBkHqAgmrv9Eb/KQhA2IwDQYJKoZIhvcNAQEL
+BQAwFDESMBAGA1UEAwwJbG9jYWxob3N0MCAXDTI0MDcwMjA5MTkyN1oYDzIxMjQw
+NjA4MDkxOTI3WjAUMRIwEAYDVQQDDAlsb2NhbGhvc3QwggEiMA0GCSqGSIb3DQEB
+AQUAA4IBDwAwggEKAoIBAQCx7eyXu0A1IH18U0fmVbHlC0OpzpGfGOQayrlrxyxN
+kNc8T2ehnt7W33FHI2tjcmO11wgsN+U2+uB2aq0q1OYaqo3OlZtTF4A91CgAdbGd
+Ix5aEEzjogiQIBvrBQhaU6uFTzUBQ5tWs+pLcorVrp8G/ONN/1e4Z3NCg036ibSs
+Vkfdw1zX6vTR674uTq8aIG7sH3DCF1Q+CzvxhQrhjkZOha+u0H+OhZ9yd30hU/xy
+AKZsoGHNY65bVSYAPxP6XLw5inF534TLriggFDonEk16eHjAi7SxcjdKcyxhfX7u
+DefD/s9cKUY4Tf1JeAx2F1Y++ffqWlQSde5DKkfC6xU5AgMBAAGjWTBXMBQGA1Ud
+EQQNMAuCCWxvY2FsaG9zdDALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH
+AwEwHQYDVR0OBBYEFIHsshXxnhcdXRSPxHvTvfMWhfTgMA0GCSqGSIb3DQEBCwUA
+A4IBAQCnEU3NtmskVv2/3U0DQM8FM8jKc/V7WCfK/PWPFUZgCkj8pUg4yYxjOZ4K
+pFV7cPJjtArmjyU9lB0g2wQvlpXEYbDJ5K0LK9GsdhowZQatZaTB0nVZeG87mlV7
+kxrQTMsMTYf5I6S3SW63SorlJQiuaQjOKwvommCS+6Q5goEOodZpGr+5sQSuRLlw
+XMKzcU7ZDfe1jidjjcWSyf6UMKB/mhMQVMTTDURt/jS7koA5lXiU+m0XCSi28wTr
+lV9ZRzZiE4mRDANlEkoqUCYPG/PDF0KOgDROiavlrBcBycsqBD5iRdGYGaYwUmSo
+UpaVGGLYBguqwEeQ2ixC2rTGkyg7
+-----END CERTIFICATE-----

resources/test_ssl.crt

@@ -1,52 +1,28 @@
 -----BEGIN PRIVATE KEY-----
-MIIJRAIBADANBgkqhkiG9w0BAQEFAASCCS4wggkqAgEAAoICAQDE+oTzIuSjtyXp
-MvpYbILhKp4MSrH0gZYnI0Rh62F+tqLhmn3CuCmSZIMKAu1xAotTv/ygCRD6Dscn
-rMWSqcWt1/gHrSO81HnMnwNCdKkG4LWJ7OMxzsH6TxuMMqa83BEPfsZTKYjCWKWN
-XtGM6hymokpR7+iiQVrjYU/y0u2+T/fCScybXFEJ/Hw2eQ+1jPp7BzBZ/EbfQu9e
-RpQshgUt6nthJntt4eV71DP06qfX/xZmdLfRqxKL4Q0nBr7cTt3UOYSx1Yn4RCsi
-BoXCqBufud4mLlmkQ4YJy7SmYzYP3ABJJqVNDYDkT97WzSg+MLRiTF0APLgHNHV7
-FnxWKjEfYvqzNxd26le1UjmsNK32Us7ZUIX4JuF7mU6LjBfYxxOsRby7pgY786tm
-LUf13b9ZcoEc1wOrjDtqaZwMPaH1Z9nAYq43YuqswM3/l/sojkT9kF3BuwaBmFNy
-QyzGVTKUPNfyZvB7iX19sgV3CrYp141gmuLqfXSOPkVUINMMiCrUy6cQYRsP4kc8
-swLaO3Iw/TTq+zLKqpoaQsKMB3oraggjyX05nBcWoigOks8lEuPk0SVJ39g1how8
-m/JH4NjZBVXlJJ+ll8fFbG7zGINlQnaSDtFyuNKHwQblWjqz60KZun8hX+khbrL0
-zrlyaZyOAnsnnztjXftsYzfW2pVxbQIDAQABAoICABLp3h2tbkhFA/QgE7ctViTS
-L4JNIsiwL6963KxNSlt9JGcmqygpAD7g/U8XCF8HSEMGpnZkYHeuNxO5bHAgco12
-dQehqZKOUVKjOxAkvP0e0veXIhqMeIY1FddQnr94HwA+o0LldE767YyFO/g8m3sp
-jprO/yajQVufYqqVc8QIEClc5jNNui9MCc4+MhKz4nIxNsSRK2nxFqRWARDEXpdx
-0h56MDRVEjChZ8q+xFaCVQ+J6gONGlcJiTaD2Iw1W2nvCu17bEfFHeIiv7G47Awa
-b/j5Dtztqd9jaqlmUdDUhkd/2TPslcF2ZNZ5tQFBsnRU0kI9UktIz3X96vroCrbG
-aRryT6sJUHdvdc+d6l53sJLgpviQLHlDiovMSOWeNw+YdUzc55kiwubO4S0AuNho
-M93LUuAjqoSpbBuwU9vZcLHKlhzsp290KfSbzEhIPsgpHotw9B1sjrM7ykiT9ggc
-w0aaOL5bOc8RHikTARzA9dyR0nJLE22ZTgbtgUoDgku8g5z0QLpGxOkV14qcSthx
-TS5qRoRm7gvJo6d474cW1CHjooxdkCqpQYjZ0V4HzqemFjd8M0FGGbsuA/okWzT9
-YKtF5VyBhvgkb0uLAkPb20eauLHZVS7Nm6mU6yf3nD9rYZo4W5bMXZuWt43AXxY7
-B0PnWJPH+VIfIl0ptuDZAoIBAQDnJYuETD1gQSsotZekUZtqyj3kz1NPA2o2tggn
-hNgLNKHBbjH0+H0/kj2X1vTuC1LCaISRggEXkqa6O5m7N33idNR8YVL3lBltI1gW
-56rIYUggjMyWHUQXF33ALvP2RuLKLHToms/p6tx73/TCLCa4RAB5HOGMVyGwuRoW
-73xrdZNRQE0S+fNw/b13GWtMOv8NBHu48BAr1GJ9g/sfXLz/bKvl9A+BleKO2s+q
-R9d4jzmz5rnR6S/QJA8Q3u0/belZI71mUyL/9MDAFD4C9v1+DYHdS7LW/Is1i8hH
-iGOL683+Ouq6xIgABwyoBwdGqtnnVMa5/OVUCm8EcyUuvoPJAoIBAQDaKHoiGCuY
-sVLT6/VABmtw6Q17r+w0CmSo+QRR9u5sKx+mdANU86qSXyDXwNiqY0HoioD5Ng9y
-SHpMMaDWoAmbcSMNeHjdwuLgPGKzeZ6aH53OAaazHzrafruG8nTiZkt0/kh0bLvY
-rubck5DHmdZJuSCEwGfuzBgZLCrsjwkjawg57+maMmEabyTJy18YfTBI3s7c2AXz
-qcqJnhUMKLL7LmGtBlox6m2C0AXjgXDjHO0R4RqJ1yndN5UUEzxZigsuIxjM41JB
-7QHJ7B1uPhFf5omZDjQRbqY8pB+KiQbRtP5Bwz+LvBy0NelMIOa116Hv3V4igFxv
-AtCKbju2HCqFAoIBAQDY9+AfHiVajbGCc/pUrpmRQyeX+Jh9iXoQwwuidMsKsavI
-UrSn+vwuSQpx1b9xFsXnYI5Xu01lIC5Kj5l9J9iNUhcGbaCgbq7zSALu9STVFKPM
-kf2URwJcHpvWYvxzRxSoq9RNZswVCXVO/ejUvvbVbld3WAnLXxprtURtFP2YLPRM
-h2wRjPfbLwLCoeSa2KICSRwNe6HiUmjk4pc9WCK8K/irUE2h2NyiNXhKoUb7jo2e
-dcwk4psT6FUQBAF00aoBF1A4lX87/TVU12tiAw/tW6Zz4BOOQ940M/KaWsb+Vyi0
-I/+jssjqJbPWoUpOJh+GSoiDmoR1P5n39lGHsCMpAoIBAQCX6lnqRhSN1uWLx6NX
-+2B0FwYZnI8KSjaAaC+2+BJdZsY6fk0XqjqchPv04ki+ljH+QfzADgJBnfD0ABc1
-fepSwT0ck0jvfFfKuKIuwsFMKDoWi5XO5C9ymY/y0AHO6lcfWDeSQ2mn4VvIPEY0
-iI7tdaoMZ4O4iY06ckRNyOkfLdhjqApvIyf1ZXIjx6goAH1QMT+yEAhM/m6Y2Gll
-ty2ztj+0YlkKq2mpDz0aiTfYH3uC2NNHK3runlcEzMRYwcU5Up1hh+bvG6EEQJTa
-AQTOWFZ3K6ncfcXrMor4SKVkAPqRRuqIXu1KHMSiC8M827TbuLZlpic38qjPzSVt
-kj2VAoIBAQDMiurZJIVLZ+aX/TJ7DoKCkhD/hL2delu0prLBQrd05/0axEXspM03
-kIvO+vQ5CzQ9JssA4A3vYni68yrDLUtucxwaEADzA6vLy6re7Y+ApLFHv5fgQYX9
-EK830RUuVgI9XQc7O0ziAb1CVGg/XaKzuFfubbJIMHPEGVsGll18L1sJCfcgCQpa
-pbSyRFUROdR6kDQUsMv3uq1LFLeVL5hYyS4k/LRvlg/3+Zk0XIKOaj5nfk1ax2bF
-uViwPLq4l+CAtHNcASv30+P2ejmZDqOt2ctiFhjtZZPg8+LC7gzkJh+e/2FTkBk1
-l7zlgithSVBlZvD3zalH8RxDX+9NhnHw
+MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCx7eyXu0A1IH18
+U0fmVbHlC0OpzpGfGOQayrlrxyxNkNc8T2ehnt7W33FHI2tjcmO11wgsN+U2+uB2
+aq0q1OYaqo3OlZtTF4A91CgAdbGdIx5aEEzjogiQIBvrBQhaU6uFTzUBQ5tWs+pL
+corVrp8G/ONN/1e4Z3NCg036ibSsVkfdw1zX6vTR674uTq8aIG7sH3DCF1Q+Czvx
+hQrhjkZOha+u0H+OhZ9yd30hU/xyAKZsoGHNY65bVSYAPxP6XLw5inF534TLrigg
+FDonEk16eHjAi7SxcjdKcyxhfX7uDefD/s9cKUY4Tf1JeAx2F1Y++ffqWlQSde5D
+KkfC6xU5AgMBAAECggEABbqBSYlP0eYP5DbSM8pChftM3GS4L4UfovUv7xZkiMLH
+CzwLPBrfVc+v1/h99p+yMiKQMsxB5vlAzM82cBCWr/kZw7LxY0V4bYUtHIathz+g
+NIodz55h5DIEdBafZDkZZptcO4QvtiTowDEZ4zNSD2mI7/PuoRNDlLqhghV46at3
+nfJmOgbWxzciOhpYUXtXl1+n1ywkXIbL58sfp5HgkuM9dL2BTZQdUwbS58TgyYla
+DBYZQfiZ+MgrCqJKCYEy+lycVHz9LaSCVXgEYmstfcLqdC7X3/nQuUn587rV5FDA
+75bDZ7r2jvrQT7/MEGm3b2fgk0+0gMew52wrDsncuQKBgQDygvVlm6BU2cIm8qvR
+l1LqU06jbEVW2fzrXi73RVgCR+ZQoy+hB8LviqNO0ayAOWIH/6iMGU09bK72DlJ0
+K4P58X6iJaop1ntdYa2DApPzrz3/4weMl4ZVjz5tPBUxhkfJCMfZF2q+CoHOsdlQ
+rTXNFQgfq4XNGpj3YX0L94pkgwKBgQC702bRFk7lHoMyn8hs8MQqumSsefpTsc6v
+3fLl6DgwhU9/FAEymIt+wEIchoU0GkVfZ16OmO1whZAgj3zsBr/zG7C7Din9s7hB
+KV57o7KIRP3IUo+N6qQ2xNPG8lrCd7Xz+430RHT5BRudqNXWsZziFOrPXZz7Yh8/
+TMy4ZwzKkwKBgQCOTToh/Uf/gifjItKfkeQdi/TBAG9Pn2pB0mpMvmv+KqKC/r6c
+Bynj1b4uKerG8uULPIFydAZW3MdtqsnHUSGIMKTWELPhCPIqwX5HOeQHQfVniZiM
+bv1shzlib7cf8GN/G5/pS0xfZ1r0JngWVw0S4hx6OPOyfsDzqEjwFLkocQKBgBOQ
+2xYO19sgSZR9dph6oES/M/uPnVcYn6pMWaA/h5LuYDChudo2b9mdV4W3MasSzYU5
+tGzwW1OsZi4uJFpF/brqeIeT2yX1kc0f7Rq+G7v8S9+RUij7d23JJTKFTpUReV/Y
+JZp7gx/pu026J8R8rhYTDb7aRp8dQpoKewz+lyOHAoGAUGOfsLybzE2w/gL1gZ8W
+L6sdkQ7zq/IF04cqM5jw1g82f9CGdZlWrT6wv5D++O9zpZSmuwJ2p/os5Tsygmb4
+M0IjG23Mw5IWLCC6n2riYwpQ8sjuL3SOhqgt4k5mnu4H0RpVLz69JPBJkaGuE1Ld
+k4zeVfI1+X2clcHVfDS6xhc=
 -----END PRIVATE KEY-----

resources/test_ssl.key

@@ -0,0 +1,123 @@
+# pylint: disable=missing-docstring
+import os
+import threading
+import warnings
+from http.server import HTTPServer, SimpleHTTPRequestHandler
+from ssl import get_default_verify_paths, SSLContext, PROTOCOL_TLSv1_1, PROTOCOL_TLSv1_2
+from typing import Callable
+
+import pytest
+import urllib3
+from requests.exceptions import SSLError
+
+from ibm_cloud_sdk_core.base_service import BaseService
+from ibm_cloud_sdk_core.authenticators import NoAuthAuthenticator
+
+
+# The certificate files that are used in this tests are generated by this command:
+# pylint: disable=line-too-long,pointless-string-statement
+"""
+openssl req -x509 -out test_ssl.crt -keyout test_ssl.key \
+  -newkey rsa:2048 -nodes -sha256 -days 36500 \
+  -subj '/CN=localhost' -extensions EXT -config <( \
+   printf "[dn]\nCN=localhost\n[req]\ndistinguished_name = dn\n[EXT]\nsubjectAltName=DNS:localhost\nkeyUsage=digitalSignature\nextendedKeyUsage=serverAuth")
+"""
+
+
+# Load the certificate and the key files.
+cert = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.crt')
+key = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.key')
+
+
+def _local_server(tls_version: int, port: int) -> Callable:
+    def decorator(test_function: Callable) -> Callable:
+        def inner():
+            # Disable warnings caused by the self-signed certificate.
+            urllib3.disable_warnings()
+
+            # Build the SSL context for the server.
+            ssl_context = SSLContext(tls_version)
+            ssl_context.load_cert_chain(certfile=cert, keyfile=key)
+
+            # Create and start the server on a separate thread.
+            server = HTTPServer(('localhost', port), SimpleHTTPRequestHandler)
+            server.socket = ssl_context.wrap_socket(server.socket, server_side=True)
+            t = threading.Thread(target=server.serve_forever)
+            t.start()
+
+            # We run everything in a big try-except-finally block to make sure we always
+            # shutdown the HTTP server gracefully.
+            try:
+                test_function()
+            except Exception:  # pylint: disable=try-except-raise
+                raise
+            finally:
+                server.shutdown()
+                t.join()
+                # Re-enable warnings.
+                warnings.resetwarnings()
+
+        return inner
+
+    return decorator
+
+
+@_local_server(PROTOCOL_TLSv1_1, 3333)
+def test_tls_v1_1():
+    service = BaseService(service_url='https://localhost:3333', authenticator=NoAuthAuthenticator())
+    prepped = service.prepare_request('GET', url='/')
+    # The following request should fail, because the server will try
+    # to use TLS v1.1 but that's not allowed in our client.
+    with pytest.raises(Exception) as exception:
+        service.send(prepped, verify=cert)
+    # Errors can be differ based on the Python version.
+    assert exception.type is SSLError or exception.type is ConnectionError
+
+
+@_local_server(PROTOCOL_TLSv1_2, 3334)
+def test_tls_v1_2():
+    service = BaseService(service_url='https://localhost:3334', authenticator=NoAuthAuthenticator())
+
+    # First call the server with the default configuration.
+    # It should fail due to the self-signed SSL cert.
+    assert service.disable_ssl_verification is False
+    prepped = service.prepare_request('GET', url='/')
+    with pytest.raises(SSLError, match='certificate verify failed: self-signed certificate'):
+        res = service.send(prepped)
+
+    # Next configure it to validate by using our local certificate. Should raise no exception.
+    res = service.send(prepped, verify=cert)
+    assert res is not None
+
+    # Now disable the SSL verification. The request shouldn't raise any issue.
+    service.set_disable_ssl_verification(True)
+    assert service.disable_ssl_verification is True
+    prepped = service.prepare_request('GET', url='/')
+    res = service.send(prepped)
+    assert res is not None
+
+    # Lastly, try with an external URL.
+    # This test case is mainly here to reproduce the regression
+    # in the `requests` package that was introduced in `2.32.3`.
+    # More details on the issue can be found here: https://github.com/psf/requests/issues/6730
+    service = BaseService(service_url='https://cloud.ibm.com', authenticator=NoAuthAuthenticator())
+    assert service.disable_ssl_verification is False
+
+    ssl_context = service.http_adapter.poolmanager.connection_pool_kw.get("ssl_context")
+    assert ssl_context is not None
+    # In some cases (especially in Ubuntu containers that we use for testing on Travis)
+    # the default CA certificates are stored in a different place, so let's try to
+    # load those before making the final decision for this test case.
+    if len(ssl_context.get_ca_certs()) == 0:
+        try:
+            default_ca_path = get_default_verify_paths().capath
+            ssl_context.load_verify_locations(os.path.join(default_ca_path, 'ca-certificates.crt'))
+        except:
+            # Errors are ignored, let's jump straight to the assertion.
+            pass
+
+    assert len(ssl_context.get_ca_certs()) > 0
+
+    prepped = service.prepare_request('GET', url='/status')
+    res = service.send(prepped)
+    assert res is not None