[checkmk-commits] 7222 FIX check_http: port config for certificate check via proxy

Module: check_mk Branch: master Commit: 72180fc378ac1ab430493a452f62382f89b10547 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=72180fc378ac1a… Author: Moritz Kiemer <mo(a)mathias-kettner.de> Date: Fri Mar 22 12:53:22 2019 +0100 7222 FIX check_http: port config for certificate check via proxy If users had the active check "Check HTTP" configured to check a certificate via a proxy, the option "TCP Port" had wrongly been used as the proxys port instead of the certificate servers port. The port defined in the option "TCP Port" is now applied to the certificate server, regardless of whether a proxy is used. If you relied on the wrong behaviour, remove the option "TCP Port" and include the port for the proxy in the proxy settings. NOTE: To make this work, we have to pass proxy address and server name not using the "-I proxy.com" and "-H server.com" option, but as the first and second argument, respectively. This weird hack bypasses a bug in the check_http commandline parsing: When we pass the arguments this way, we can include the *servers* port in the second argument (e.g. "server.com:8443"). When we try the same with "-H server.vom:8443" the port will be stripped, and ignored if the "-p" option is set, and used as the proxys port otherwise. Change-Id: I2f7babed02b652186f996a91e53048a3689c9ebb --- .werks/7222 | 18 ++++++++++ checks/check_http | 68 ++++++++++++++++++++++++------------ tests/unit/checks/test_check_http.py | 35 +++++++++++++++++-- 3 files changed, 97 insertions(+), 24 deletions(-) diff --git a/.werks/7222 b/.werks/7222 new file mode 100644 index 0000000..f55922c --- /dev/null +++ b/.werks/7222 @@ -0,0 +1,18 @@ +Title: check_http: Mixup of ports when configuring certificate check via proxy +Level: 1 +Component: wato +Compatible: incomp +Edition: cre +Version: 1.6.0i1 +Date: 1553255189 +Class: fix + +If users had the active check "Check HTTP" configured to check a certificate +via a proxy, the option "TCP Port" had wrongly been used as the proxys port +instead of the certificate servers port. + +The port defined in the option "TCP Port" is now applied to the certificate +server, regardless of whether a proxy is used. + +If you relied on the wrong behaviour, remove the option "TCP Port" and include +the port for the proxy in the proxy settings. diff --git a/checks/check_http b/checks/check_http index e1c265d..365d927 100644 --- a/checks/check_http +++ b/checks/check_http @@ -43,31 +43,34 @@ def _get_family_and_address(settings): return family, address +def _get_proxy_setting(settings): + """return proxys (address, port, auth) or None""" + proxy = settings.get("proxy") + if not proxy: + return None + + # ':' outside a IPv6 address indicates port + if ':' in proxy.split(']')[-1]: + address, port = proxy.rsplit(':', 1) + else: + address, port = proxy, None + + auth = settings.get("proxy_auth") + if auth: + auth = passwordstore_get_cmdline("%s:%%s" % auth[0], auth[1]) + + proset = collections.namedtuple("ProxySettings", ("address", "port", "auth")) + return proset(address, port, auth) + + def _certificate_args(address_family, address, settings): args = [] server = settings.get('cert_host', address) - proxy = settings.get('proxy') - - args += ['-I', proxy or server] - - if proxy: - args += ['-H', server] - args.append('--ssl') - args += ['-j', 'CONNECT'] - elif settings.get("sni"): - args += ['-H', server] if address_family == 'ipv6': args += ['-6'] - if "port" in settings: - args += ['-p', settings["port"]] - - if "proxy_auth" in settings: - username, password = settings["proxy_auth"] - args += ["-b", passwordstore_get_cmdline("%s:%%s" % username, password)] - if "cert_days" in settings: # legacy behavior if isinstance(settings["cert_days"], int): @@ -79,6 +82,25 @@ def _certificate_args(address_family, address, settings): if "sni" in settings: args += ['--sni'] + proxy = _get_proxy_setting(settings) + server_port = settings.get("port") + + specify_port = proxy.port if proxy else server_port + if specify_port: + args += ['-p', specify_port] + + if proxy: + args += ['--ssl', '-j', 'CONNECT'] + if proxy.auth: + args += ["-b", proxy.auth] + args.append(proxy.address) + if server_port: + server += ':%s' % server_port + + elif settings.get("sni"): + args += ['-H', server] + + args += [server] return args @@ -87,11 +109,12 @@ def _url_args(address_family, address, settings): # get virthost settings: # TODO: when did 'vhost' dissapear from WATO? vhost, omit_ip = settings.get("virthost", (settings.get("vhost"), False)) + proxy = _get_proxy_setting(settings) args = [] - if "proxy" in settings: - args += ["-I", settings["proxy"]] + if proxy: + args += ["-I", proxy.address] elif not omit_ip: args += ["-I", address] @@ -101,6 +124,8 @@ def _url_args(address_family, address, settings): if address_family == 'ipv6': args += ['-6'] + # TODO: I think this should be overridden by the proxy port + # in the same way as in the cert check. (mo) if "port" in settings: args += ['-p', settings["port"]] @@ -136,9 +161,8 @@ def _url_args(address_family, address, settings): username, password = settings["auth"] args += ["-a", passwordstore_get_cmdline("%s:%%s" % username, password)] - if "proxy_auth" in settings: - username, password = settings["proxy_auth"] - args += ["-b", passwordstore_get_cmdline("%s:%%s" % username, password)] + if proxy and proxy.auth: + args += ["-b", proxy.auth] if "onredirect" in settings: args += ['--onredirect=%s' % settings["onredirect"]] diff --git a/tests/unit/checks/test_check_http.py b/tests/unit/checks/test_check_http.py index eae8884..3bcffc7 100644 --- a/tests/unit/checks/test_check_http.py +++ b/tests/unit/checks/test_check_http.py @@ -32,9 +32,40 @@ pytestmark = pytest.mark.checks ( (None, { 'cert_days': (10, 20), - 'cert_host': 'www.test123.com' + 'cert_host': 'www.test123.com', + 'port': '42', }), - ['-I', 'www.test123.com', '-C', '10,20'], + ['-C', '10,20', '-p', '42', 'www.test123.com'], + ), + ( + (None, { + 'cert_days': (10, 20), + 'cert_host': 'www.test123.com', + 'port': '42', + 'proxy': 'p.roxy', + }), + ['-C', '10,20', '--ssl', '-j', 'CONNECT', 'p.roxy', 'www.test123.com:42'], + ), + ( + (None, { + 'cert_days': (10, 20), + 'cert_host': 'www.test123.com', + 'port': '42', + 'proxy': 'p.roxy:23', + }), + ['-C', '10,20', '-p', '23', '--ssl', '-j', 'CONNECT', 'p.roxy', 'www.test123.com:42'], + ), + ( + (None, { + 'cert_days': (10, 20), + 'cert_host': 'www.test123.com', + 'port': '42', + 'proxy': '[dead:beef::face]:23', + }), + [ + '-C', '10,20', '-p', '23', '--ssl', '-j', 'CONNECT', '[dead:beef::face]', + 'www.test123.com:42' + ], ), ( (None, {