Allow configuring TLS min/max version for REST notifier

Add rest_notifier_tls_min_version and rest_notifier_tls_max_version
options so HTTPS alarm actions can pin TLS protocol versions (1.2/1.3)
via a custom requests adapter. Defaults remain unset to preserve prior
behavior.

Change-Id: Ia81cfb20d1799182ae4e50e11c19ff0b10039f3e
Signed-off-by: Daniel Pawlik <dpawlik@redhat.com>

aodh/notifier/rest.py

@@ -14,17 +14,27 @@
 # under the License.
 """Rest alarm notifier."""
 import json
+import ssl
 
 from oslo_config import cfg
 from oslo_log import log
 from oslo_utils import uuidutils
 import requests
+from requests.adapters import HTTPAdapter
 import urllib.parse as urlparse
+from urllib3.util.ssl_ import create_urllib3_context
 
 from aodh import notifier
 
 LOG = log.getLogger(__name__)
 
+TLS_VERSION_CHOICES = ['1.2', '1.3']
+
+_TLS_VERSIONS = {
+    '1.2': ssl.TLSVersion.TLSv1_2,
+    '1.3': ssl.TLSVersion.TLSv1_3,
+}
+
 OPTS = [
     cfg.StrOpt('rest_notifier_certificate_file',
                default='',
@@ -46,10 +56,55 @@ OPTS = [
                default=0,
                help='Number of retries for REST notifier',
                ),
-
+    cfg.StrOpt('rest_notifier_tls_min_version',
+               default=None,
+               choices=TLS_VERSION_CHOICES,
+               help='Minimum TLS protocol version for HTTPS REST notifier '
+                    'connections. If unset, the system default applies. '
+                    'Accepted values are 1.2 and 1.3, matching '
+                    'keystoneauth1 tls_min_version spelling.'),
+    cfg.StrOpt('rest_notifier_tls_max_version',
+               default=None,
+               choices=TLS_VERSION_CHOICES,
+               help='Maximum TLS protocol version for HTTPS REST notifier '
+                    'connections. If unset, the system default applies. '
+                    'Accepted values are 1.2 and 1.3, matching '
+                    'keystoneauth1 tls_max_version spelling.'),
 ]
 
 
+class _TLSAdapter(HTTPAdapter):
+    """HTTP adapter pinning TLS protocol versions on new connections."""
+
+    def __init__(self, min_version, max_version, verify, **kwargs):
+        self._min_version = min_version
+        self._max_version = max_version
+        # NOTE(dpawlik): requests/urllib3 set verify_mode and load the CA
+        # bundle per request, but CERT_NONE on a context with check_hostname
+        # enabled raises, so the context has to be built knowing whether
+        # we verify.
+        self._cert_reqs = None if verify else ssl.CERT_NONE
+        super().__init__(**kwargs)
+
+    def _create_ssl_context(self):
+        ctx_kwargs = {}
+        if self._min_version is not None:
+            ctx_kwargs['ssl_minimum_version'] = self._min_version
+        if self._max_version is not None:
+            ctx_kwargs['ssl_maximum_version'] = self._max_version
+        if self._cert_reqs is not None:
+            ctx_kwargs['cert_reqs'] = self._cert_reqs
+        return create_urllib3_context(**ctx_kwargs)
+
+    def init_poolmanager(self, *args, **kwargs):
+        kwargs['ssl_context'] = self._create_ssl_context()
+        return super().init_poolmanager(*args, **kwargs)
+
+    def proxy_manager_for(self, proxy, **proxy_kwargs):
+        proxy_kwargs['ssl_context'] = self._create_ssl_context()
+        return super().proxy_manager_for(proxy, **proxy_kwargs)
+
+
 class RestAlarmNotifier(notifier.AlarmNotifier):
     """Rest alarm notifier."""
 
@@ -96,10 +151,27 @@ class RestAlarmNotifier(notifier.AlarmNotifier):
         # retries (future work).
         max_retries = self.conf.rest_notifier_max_retries
         session = requests.Session()
-        session.mount(action.geturl(),
-                      requests.adapters.HTTPAdapter(max_retries=max_retries))
+        adapter = self._build_adapter(
+            action, kwargs.get('verify', True), max_retries)
+        session.mount(action.geturl(), adapter)
         resp = session.post(action.geturl(), **kwargs)
         LOG.info('Notifying alarm <%(id)s> gets response: %(status_code)s '
                  '%(reason)s.', {'id': alarm_id,
                                  'status_code': resp.status_code,
                                  'reason': resp.reason})
+
+    def _build_adapter(self, action, verify, max_retries):
+        min_opt = self.conf.rest_notifier_tls_min_version
+        max_opt = self.conf.rest_notifier_tls_max_version
+        if action.scheme == 'https' and (min_opt or max_opt):
+            min_version = _TLS_VERSIONS.get(min_opt)
+            max_version = _TLS_VERSIONS.get(max_opt)
+            if (min_version is not None and max_version is not None and
+                    min_version > max_version):
+                raise ValueError(
+                    'rest_notifier_tls_min_version (%s) cannot be greater '
+                    'than rest_notifier_tls_max_version (%s)' %
+                    (min_opt, max_opt))
+            return _TLSAdapter(min_version, max_version, verify,
+                               max_retries=max_retries)
+        return HTTPAdapter(max_retries=max_retries)

aodh/tests/unit/test_notifier.py

@@ -14,6 +14,7 @@
 # under the License.
 import fixtures
 import json
+import ssl
 import time
 from unittest import mock
 
@@ -26,6 +27,7 @@ from urllib import parse as urlparse
 
 from aodh import keystone_client
 from aodh import notifier
+from aodh.notifier import rest
 from aodh import service
 
 from aodh.tests import base as tests_base
@@ -366,6 +368,68 @@ class TestAlarmNotifier(tests_base.BaseTestCase):
                 kwargs['headers'])
             self.assertEqual(DATA_JSON, json.loads(kwargs['data']))
 
+    def _notify_https_and_get_adapter(self, action):
+        real_session = requests.Session()
+        with mock.patch('requests.Session', return_value=real_session):
+            with mock.patch.object(real_session, 'post') as poster:
+                self._msg_notifier.sample(
+                    {}, 'alarm.update', self._notification(action))
+                time.sleep(1)
+                poster.assert_called()
+                return real_session.get_adapter(action)
+
+    def test_notify_alarm_rest_action_with_tls_min_version(self):
+        action = 'https://host/action'
+        self.CONF.set_override('rest_notifier_tls_min_version', '1.2')
+
+        adapter = self._notify_https_and_get_adapter(action)
+        ctx = adapter.poolmanager.connection_pool_kw['ssl_context']
+        self.assertEqual(ssl.TLSVersion.TLSv1_2, ctx.minimum_version)
+        self.assertEqual(ssl.CERT_REQUIRED, ctx.verify_mode)
+
+    def test_notify_alarm_rest_action_with_tls_max_version(self):
+        action = 'https://host/action'
+        self.CONF.set_override('rest_notifier_tls_max_version', '1.3')
+
+        adapter = self._notify_https_and_get_adapter(action)
+        ctx = adapter.poolmanager.connection_pool_kw['ssl_context']
+        self.assertEqual(ssl.TLSVersion.TLSv1_3, ctx.maximum_version)
+
+    def test_notify_alarm_rest_action_with_tls_min_and_max_version(self):
+        action = 'https://host/action'
+        self.CONF.set_override('rest_notifier_tls_min_version', '1.2')
+        self.CONF.set_override('rest_notifier_tls_max_version', '1.3')
+
+        adapter = self._notify_https_and_get_adapter(action)
+        ctx = adapter.poolmanager.connection_pool_kw['ssl_context']
+        self.assertEqual(ssl.TLSVersion.TLSv1_2, ctx.minimum_version)
+        self.assertEqual(ssl.TLSVersion.TLSv1_3, ctx.maximum_version)
+
+    def test_notify_alarm_rest_action_tls_with_verify_disabled(self):
+        action = 'https://host/action'
+        self.CONF.set_override('rest_notifier_tls_min_version', '1.3')
+        self.CONF.set_override('rest_notifier_ssl_verify', False)
+
+        adapter = self._notify_https_and_get_adapter(action)
+        ctx = adapter.poolmanager.connection_pool_kw['ssl_context']
+        self.assertEqual(ssl.TLSVersion.TLSv1_3, ctx.minimum_version)
+        self.assertEqual(ssl.CERT_NONE, ctx.verify_mode)
+
+    def test_notify_alarm_rest_action_without_tls_version_uses_http_adapter(
+            self):
+        action = 'https://host/action'
+        adapter = self._notify_https_and_get_adapter(action)
+        self.assertIsInstance(adapter, requests.adapters.HTTPAdapter)
+        self.assertNotIsInstance(adapter, rest._TLSAdapter)
+
+    def test_build_adapter_rejects_inverted_tls_versions(self):
+        self.CONF.set_override('rest_notifier_tls_min_version', '1.3')
+        self.CONF.set_override('rest_notifier_tls_max_version', '1.2')
+        notifier_obj = rest.RestAlarmNotifier(self.CONF)
+        action = urlparse.urlsplit('https://host/action')
+        self.assertRaises(ValueError, notifier_obj._build_adapter,
+                          action, True, 0)
+
     @staticmethod
     def _fake_urlsplit(*args, **kwargs):
         raise Exception("Evil urlsplit!")

releasenotes/notes/rest-notifier-tls-versions-2e03f17d100fc31a.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    Added ``rest_notifier_tls_min_version`` and ``rest_notifier_tls_max_version``
+    options to control the TLS protocol versions used for HTTPS alarm action
+    notifications. Accepted values are ``1.2`` and ``1.3``. When unset, the
+    system default applies.