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

Module: check_mk Branch: master Commit: b325aafa11e448547f2b1d5b29068263022b9403 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=b325aafa11e448… Author: Moritz Kiemer <mo(a)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):