fix: use the correct SSL config if cert verification is disabled (#187)

It turned out, disabling the SSL certificate verification didn't work as expected.
We passed the right arguments to the `requests` package, but since we use
a custom SSL context, it couldn't turn off the verification entirely, because the
default SSL context has the `check_hostname` set to `True` and the documentation
says the following: "`verify_mode` is now automatically changed to `CERT_REQUIRED`
when hostname checking is enabled and `verify_mode` is `CERT_NONE`."
This commit adds an addition step which disables the hostname checking in our
custom SSL context.

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

Author: pyrooka

.secrets.baseline

@@ -3,7 +3,7 @@
     "files": "package-lock.json|^.secrets.baseline$",
     "lines": null
   },
-  "generated_at": "2023-12-11T17:30:56Z",
+  "generated_at": "2024-01-24T12:09:17Z",
   "plugins_used": [
     {
       "name": "AWSKeyDetector"
@@ -207,6 +207,16 @@
         "verified_result": null
       }
     ],
+    "resources/test_ssl.key": [
+      {
+        "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9",
+        "is_secret": false,
+        "is_verified": false,
+        "line_number": 1,
+        "type": "Private Key",
+        "verified_result": null
+      }
+    ],
     "test/test_base_service.py": [
       {
         "hashed_secret": "da2f27d2c57a0e1ed2dc3a34b4ef02faf2f7a4c2",

ibm_cloud_sdk_core/base_service.py

@@ -108,7 +108,7 @@ def __init__(
         self.enable_gzip_compression = enable_gzip_compression
         self._set_user_agent_header(self._build_user_agent())
         self.retry_config = None
-        self.http_adapter = SSLHTTPAdapter()
+        self.http_adapter = SSLHTTPAdapter(_disable_ssl_verification=self.disable_ssl_verification)
         if not self.authenticator:
             raise ValueError('authenticator must be provided')
         if not isinstance(self.authenticator, Authenticator):
@@ -138,14 +138,16 @@ def enable_retries(self, max_retries: int = 4, retry_interval: float = 30.0) ->
             # Omitting this will default to all methods except POST
             allowed_methods=['HEAD', 'GET', 'PUT', 'DELETE', 'OPTIONS', 'TRACE', 'POST'],
         )
-        self.http_adapter = SSLHTTPAdapter(max_retries=self.retry_config)
+        self.http_adapter = SSLHTTPAdapter(
+            max_retries=self.retry_config, _disable_ssl_verification=self.disable_ssl_verification
+        )
         self.http_client.mount('http://', self.http_adapter)
         self.http_client.mount('https://', self.http_adapter)
 
     def disable_retries(self):
         """Remove retry config from http_adapter"""
         self.retry_config = None
-        self.http_adapter = SSLHTTPAdapter()
+        self.http_adapter = SSLHTTPAdapter(_disable_ssl_verification=self.disable_ssl_verification)
         self.http_client.mount('http://', self.http_adapter)
         self.http_client.mount('https://', self.http_adapter)
 
@@ -223,8 +225,18 @@ def set_disable_ssl_verification(self, status: bool = False) -> None:
         Keyword Arguments:
             status: set to true to disable ssl verification (default: {False})
         """
+        if self.disable_ssl_verification == status:
+            # Do nothing if the state doesn't change.
+            return
+
         self.disable_ssl_verification = status
 
+        self.http_adapter = SSLHTTPAdapter(
+            max_retries=self.retry_config, _disable_ssl_verification=self.disable_ssl_verification
+        )
+        self.http_client.mount('http://', self.http_adapter)
+        self.http_client.mount('https://', self.http_adapter)
+
     def set_service_url(self, service_url: str) -> None:
         """Set the url the service will make HTTP requests too.
 

ibm_cloud_sdk_core/utils.py

@@ -25,7 +25,7 @@
 from typing import List, Union
 from urllib.parse import urlparse, parse_qs
 
-from requests.adapters import HTTPAdapter
+from requests.adapters import HTTPAdapter, DEFAULT_POOLBLOCK
 from urllib3.util.ssl_ import create_urllib3_context
 
 import dateutil.parser as date_parser
@@ -35,14 +35,21 @@ 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)
 
-    # pylint: disable=arguments-differ
-    def init_poolmanager(self, connections, maxsize, block):
-        """Extends the parent's method by adding minimum SSL version to the args."""
+    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
-        super().init_poolmanager(connections, maxsize, block, ssl_context=ssl_context)
+
+        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):
@@ -60,7 +67,7 @@ class GzipStream(io.RawIOBase):
                It can be a file-like object, bytes or string.
     """
 
-    def __init__(self, source: Union[io.IOBase, bytes, str]) -> 'GzipStream':
+    def __init__(self, source: Union[io.IOBase, bytes, str]):
         self.buffer = b''
 
         if isinstance(source, io.IOBase):

resources/test_ssl.cert

@@ -0,0 +1,30 @@
+-----BEGIN CERTIFICATE-----
+MIIFJTCCAw2gAwIBAgIUfElLZwAd6ZSPKCWsVzp5vjUmytQwDQYJKoZIhvcNAQEL
+BQAwITELMAkGA1UEBhMCVVMxEjAQBgNVBAMMCWxvY2FsaG9zdDAgFw0yNDAxMjQx
+MTM5MzRaGA8yMTIzMTIzMTExMzkzNFowITELMAkGA1UEBhMCVVMxEjAQBgNVBAMM
+CWxvY2FsaG9zdDCCAiIwDQYJKoZIhvcNAQEBBQADggIPADCCAgoCggIBAMT6hPMi
+5KO3Jeky+lhsguEqngxKsfSBlicjRGHrYX62ouGafcK4KZJkgwoC7XECi1O//KAJ
+EPoOxyesxZKpxa3X+AetI7zUecyfA0J0qQbgtYns4zHOwfpPG4wyprzcEQ9+xlMp
+iMJYpY1e0YzqHKaiSlHv6KJBWuNhT/LS7b5P98JJzJtcUQn8fDZ5D7WM+nsHMFn8
+Rt9C715GlCyGBS3qe2Eme23h5XvUM/Tqp9f/FmZ0t9GrEovhDScGvtxO3dQ5hLHV
+ifhEKyIGhcKoG5+53iYuWaRDhgnLtKZjNg/cAEkmpU0NgORP3tbNKD4wtGJMXQA8
+uAc0dXsWfFYqMR9i+rM3F3bqV7VSOaw0rfZSztlQhfgm4XuZTouMF9jHE6xFvLum
+Bjvzq2YtR/Xdv1lygRzXA6uMO2ppnAw9ofVn2cBirjdi6qzAzf+X+yiORP2QXcG7
+BoGYU3JDLMZVMpQ81/Jm8HuJfX2yBXcKtinXjWCa4up9dI4+RVQg0wyIKtTLpxBh
+Gw/iRzyzAto7cjD9NOr7MsqqmhpCwowHeitqCCPJfTmcFxaiKA6SzyUS4+TRJUnf
+2DWGjDyb8kfg2NkFVeUkn6WXx8VsbvMYg2VCdpIO0XK40ofBBuVaOrPrQpm6fyFf
+6SFusvTOuXJpnI4CeyefO2Nd+2xjN9balXFtAgMBAAGjUzBRMB0GA1UdDgQWBBSx
+Lwf6XjMsF54JX6SBpYhwbw8SezAfBgNVHSMEGDAWgBSxLwf6XjMsF54JX6SBpYhw
+bw8SezAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4ICAQCEgNyk5Bon
+F9hDHb6jiR0p0RkNiPZeOkFFHyonmG7WTkQMJElaFJJQN2wuPeok8ky0B65S4V8e
+uGRQSvZ9rt/h5/K09G9h/haN/UTWXj1AUWcblC8hw8ZnOSg2tLkOLwqQsb+Q4pUM
+eS+Gr10cTENnW0hwG5husiiNj58Mr1+CL+LqfmHPK85vPco2Q0uQ4MEpmtwM5R2g
+t4d7hI81vXKeTBTeB6sEI9EkHPTyy1oTk7uEtBK+d62ZJPQCsco1CExNRIkYTUgh
+LIIyDfXxO8Vo2fhbLM95rjlDdoM87Db79EQd6REdwLh1lRByD3IvkXouZnbVfkaw
+BeCOGtDjcTXcNHTVdjmNZ723sU4OSAqHDyVazeIpKZbOKIRlHgYqylYcxT/MOL0l
+Mtzw2o83xoSYrMQ8rcYO5RX1EIPwUijo3k3T8OMPZbYP724hu3YxbEF+kOgCty4A
+9UTDIUqlr6Y25C/vf3L792nrhduqrWALXumbENVelqow8u2HBaMj1k+LjVSNMIa+
+HFP8ex7bYdNjjikTHDM/a4Rs+yBg2WWkfvcYRdkn/IkjAPt4vjdiuPD5pDmjkIBt
+ZFVUM9U6V+DAlPULZXJOb348K0KcnQdbsn9ti+LFWsfLJSaUbAirt4yQ/H6FW35u
+iyFlAJPkdeGhoBhtbf5f2IVHqcvrOO34Dw==
+-----END CERTIFICATE-----

resources/test_ssl.key

@@ -0,0 +1,52 @@
+-----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
+-----END PRIVATE KEY-----

test_integration/test_ssl_verification.py

@@ -0,0 +1,62 @@
+# pylint: disable=missing-docstring
+import os
+import threading
+from http.server import HTTPServer, SimpleHTTPRequestHandler
+from ssl import PROTOCOL_TLS_SERVER, SSLContext
+
+import pytest
+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:
+# openssl req \
+#    -new \
+#    -newkey rsa:4096 \
+#    -days 36500 \
+#    -nodes \
+#    -x509 \
+#    -subj "/C=US/CN=localhost" \
+#    -keyout test_ssl.key \
+#    -out test_ssl.cert
+
+
+def test_ssl_verification():
+    # Load the certificate and the key files.
+    cert = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.cert')
+    key = os.path.join(os.path.dirname(__file__), '../resources/test_ssl.key')
+
+    # Build the SSL context for the server.
+    ssl_context = SSLContext(PROTOCOL_TLS_SERVER)
+    ssl_context.load_cert_chain(certfile=cert, keyfile=key)
+
+    # Create and start the server on a separate thread.
+    server = HTTPServer(('127.0.0.1', 3333), 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:
+        service = BaseService(service_url='https://127.0.0.1:3333', authenticator=NoAuthAuthenticator())
+        #
+        # First call the server with the default configuration.
+        # It should fail due to the invalid SSL cert.
+        assert service.disable_ssl_verification is False
+        prepped = service.prepare_request('GET', url='/')
+        with pytest.raises(SSLError):
+            res = service.send(prepped)
+
+        # 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
+    except Exception:  # pylint: disable=try-except-raise
+        raise
+    finally:
+        server.shutdown()