[checkmk-commits] 7389 FIX check_http: Enable SSL/TLS hostname extension support by default

Moritz Kiemer mo at mathias-kettner.de
Thu Apr 4 07:30:29 CEST 2019


Module: check_mk
Branch: master
Commit: b325aafa11e448547f2b1d5b29068263022b9403
URL:    http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=b325aafa11e448547f2b1d5b29068263022b9403

Author: Moritz Kiemer <mo at mathias-kettner.de>
Date:   Wed Apr  3 11:08:24 2019 +0200

7389 FIX check_http: Enable SSL/TLS hostname extension support by default

Previously users had to {enable SSL/TLS hostname extension support} (SNI)
explicitly. Omiting the option could result in unexpectedly checking
different content/certificates than the {host address} option would suggest.
We now assume users want SNI support. Users that relied on the previous
behaviour of disabled SNI can disable SNI explicitly useing the option
{Advanced: Disable SSL/TLS hostname extension support (SNI)}.

Change-Id: Icbc6fd063c93e510b91b119c89894acc3ebbd069

---

 .werks/7389                           | 15 +++++++++++
 checks/check_http                     | 25 +++++++------------
 cmk/gui/plugins/wato/active_checks.py | 17 +++++++------
 tests/unit/checks/test_check_http.py  | 47 ++++++++++++++++++++++++++++-------
 4 files changed, 72 insertions(+), 32 deletions(-)

diff --git a/.werks/7389 b/.werks/7389
new file mode 100644
index 0000000..066ae8f
--- /dev/null
+++ b/.werks/7389
@@ -0,0 +1,15 @@
+Title: check_http: Enable SSL/TLS hostname extension support by default
+Level: 1
+Component: checks
+Compatible: incomp
+Edition: cre
+Version: 1.6.0i1
+Date: 1554282151
+Class: fix
+
+Previously users had to {enable SSL/TLS hostname extension support} (SNI)
+explicitly. Omiting the option could result in unexpectedly checking
+different content/certificates than the {host address} option would suggest.
+We now assume users want SNI support. Users that relied on the previous
+behaviour of disabled SNI can disable SNI explicitly useing the option
+{Advanced: Disable SSL/TLS hostname extension support (SNI)}.
diff --git a/checks/check_http b/checks/check_http
index 7f9b75c..3d86c19 100644
--- a/checks/check_http
+++ b/checks/check_http
@@ -58,8 +58,7 @@ def _transform_check_http(params):
         if key in mode:
             host_settings[key] = mode.pop(key)
 
-    if "sni" in mode:
-        transformed["sni"] = mode.pop("sni")
+    mode.pop("sni", None)
 
     return transformed
 
@@ -94,8 +93,8 @@ def _get_proxy(params):
     return ProxySettings(proxy.get("address"), proxy.get("port"), auth)
 
 
-def _certificate_args(host, proxy, settings, sni_flag):
-    args = []
+def _certificate_args(host, proxy, settings):
+    args = ['-H', host.address]
 
     if "cert_days" in settings:
         # legacy behavior
@@ -107,17 +106,12 @@ def _certificate_args(host, proxy, settings, sni_flag):
 
     if proxy:
         args += ['--ssl', '-j', 'CONNECT']
-    elif sni_flag:
-        args += ['-H', host.address]
 
     return args
 
 
-def _url_args(host, proxy, settings):
-    args = []
-
-    if "virthost" in settings:
-        args += ["-H", settings["virthost"]]
+def _url_args(host, _proxy, settings):
+    args = ["-H", settings.get("virthost", host.address)]
 
     if "uri" in settings:
         args += ['-u', settings["uri"]]
@@ -198,12 +192,12 @@ def _url_args(host, proxy, settings):
     return args
 
 
-def _common_args(host, proxy, sni_flag):
+def _common_args(host, proxy, params):
     args = []
 
     if host.family == 'ipv6':
         args += ['-6']
-    if sni_flag:
+    if not params.get("disable_sni"):
         args += ['--sni']
     if proxy and proxy.auth:
         args += ["-b", proxy.auth]
@@ -231,13 +225,12 @@ def check_http_arguments(params):
     host = _get_host(params)
     proxy = _get_proxy(params)
 
-    sni_flag = bool(params.get("sni"))
     if mode_name == 'cert':
-        args = _certificate_args(host, proxy, settings, sni_flag)
+        args = _certificate_args(host, proxy, settings)
     else:
         args = _url_args(host, proxy, settings)
 
-    return args + _common_args(host, proxy, sni_flag)
+    return args + _common_args(host, proxy, params)
 
 
 def check_http_description(params):
diff --git a/cmk/gui/plugins/wato/active_checks.py b/cmk/gui/plugins/wato/active_checks.py
index 6ac5d8f..db89d54 100644
--- a/cmk/gui/plugins/wato/active_checks.py
+++ b/cmk/gui/plugins/wato/active_checks.py
@@ -1021,11 +1021,6 @@ class RulespecActiveChecksHttp(HostRulespec):
                          allow_empty=False)),
                     ("host", self._hostspec()),
                     ("proxy", self._proxyspec()),
-                    ("sni",
-                     FixedValue(
-                         value=True,
-                         totext="",
-                         title=_("Enable SSL/TLS hostname extension support (SNI)"))),
                     ("mode",
                      CascadingDropdown(
                          title=_("Mode of the Check"),
@@ -1274,6 +1269,15 @@ class RulespecActiveChecksHttp(HostRulespec):
                               )),
                          ],
                      )),
+                    ("disable_sni",
+                     FixedValue(
+                         value=True,
+                         totext="",
+                         title=_("Advanced: Disable SSL/TLS hostname extension support (SNI)"),
+                         help=_(
+                             "In earlier versions of Check_MK users had to enable SNI explicitly."
+                             " We now assume users allways want SNI support. If you don't, you"
+                             " can disable it with this option."))),
                 ],
                 required_keys=["name", "host", "mode"],
                 validate=self._validate_all,
@@ -1332,8 +1336,7 @@ class RulespecActiveChecksHttp(HostRulespec):
             if key in mode:
                 host_settings[key] = mode.pop(key)
 
-        if "sni" in mode:
-            transformed["sni"] = mode.pop("sni")
+        mode.pop("sni", None)
 
         return transformed
 
diff --git a/tests/unit/checks/test_check_http.py b/tests/unit/checks/test_check_http.py
index 5426dc3..f6e316b 100644
--- a/tests/unit/checks/test_check_http.py
+++ b/tests/unit/checks/test_check_http.py
@@ -19,6 +19,7 @@ pytestmark = pytest.mark.checks
             '/images',
             '--onredirect=follow',
             '-L',
+            '--sni',
             '-p',
             80,
             'www.test123.de',
@@ -43,6 +44,7 @@ pytestmark = pytest.mark.checks
             '--extended-perfdata',
             '-j',
             'CONNECT',
+            '--sni',
             '163.172.86.64',
             'www.test123.de:3128',
         ],
@@ -53,7 +55,7 @@ pytestmark = pytest.mark.checks
             'cert_host': 'www.test123.com',
             'port': '42',
         }),
-        ['-C', '10,20', '-p', '42', 'www.test123.com'],
+        ['-H', 'www.test123.com', '-C', '10,20', '--sni', '-p', '42', 'www.test123.com'],
     ),
     (
         (None, {
@@ -62,7 +64,10 @@ pytestmark = pytest.mark.checks
             'port': '42',
             'proxy': 'p.roxy',
         }),
-        ['-C', '10,20', '--ssl', '-j', 'CONNECT', 'p.roxy', 'www.test123.com:42'],
+        [
+            '-H', 'www.test123.com', '-C', '10,20', '--ssl', '-j', 'CONNECT', '--sni', 'p.roxy',
+            'www.test123.com:42'
+        ],
     ),
     (
         (None, {
@@ -71,7 +76,10 @@ pytestmark = pytest.mark.checks
             'port': '42',
             'proxy': 'p.roxy:23',
         }),
-        ['-C', '10,20', '--ssl', '-j', 'CONNECT', '-p', '23', 'p.roxy', 'www.test123.com:42'],
+        [
+            '-H', 'www.test123.com', '-C', '10,20', '--ssl', '-j', 'CONNECT', '--sni', '-p', '23',
+            'p.roxy', 'www.test123.com:42'
+        ],
     ),
     (
         (None, {
@@ -81,8 +89,29 @@ pytestmark = pytest.mark.checks
             'proxy': '[dead:beef::face]:23',
         }),
         [
-            '-C', '10,20', '--ssl', '-j', 'CONNECT', '-p', '23', '[dead:beef::face]',
-            'www.test123.com:42'
+            '-H', 'www.test123.com', '-C', '10,20', '--ssl', '-j', 'CONNECT', '--sni', '-p', '23',
+            '[dead:beef::face]', 'www.test123.com:42'
+        ],
+    ),
+    (
+        {
+            'host': {
+                "address": 'www.test123.com',
+                "port": 42,
+                "address_family": 'ipv6'
+            },
+            'proxy': {
+                "address": '[dead:beef::face]',
+                "port": 23
+            },
+            'mode': ('cert', {
+                'cert_days': (10, 20)
+            }),
+            'disable_sni': True
+        },
+        [
+            '-H', 'www.test123.com', '-C', '10,20', '--ssl', '-j', 'CONNECT', '-6', '-p', 23,
+            '[dead:beef::face]', 'www.test123.com:42'
         ],
     ),
     (
@@ -90,26 +119,26 @@ pytestmark = pytest.mark.checks
             'virthost': ("virtual.host", True),
             'proxy': "foo.bar",
         }),
-        ['-H', 'virtual.host', 'foo.bar', 'virtual.host'],
+        ['-H', 'virtual.host', '--sni', 'foo.bar', 'virtual.host'],
     ),
     (
         (None, {
             'virthost': ("virtual.host", False),
             'proxy': "foo.bar",
         }),
-        ['-H', 'virtual.host', 'foo.bar', 'virtual.host'],
+        ['-H', 'virtual.host', '--sni', 'foo.bar', 'virtual.host'],
     ),
     (
         (None, {
             'virthost': ("virtual.host", True),
         }),
-        ['-H', 'virtual.host', 'virtual.host'],
+        ['-H', 'virtual.host', '--sni', 'virtual.host'],
     ),
     (
         (None, {
             'virthost': ("virtual.host", False),
         }),
-        ['-H', 'virtual.host', '$_HOSTADDRESS_4$'],
+        ['-H', 'virtual.host', '--sni', '$_HOSTADDRESS_4$'],
     ),
 ])
 def test_check_http_argument_parsing(check_manager, params, expected_args):



More information about the checkmk-commits mailing list