From: Remi Gacogne Date: Wed, 13 Jan 2021 17:35:02 +0000 (+0100) Subject: dnsdist: Deprecate parameters to webserver(), add 'statsRequireAuthentication' parameter X-Git-Tag: dnsdist-1.6.0-alpha1~25^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fa7e8b5dd7f21d52cd21b5b7cb0e3b9f67558da6;p=thirdparty%2Fpdns.git dnsdist: Deprecate parameters to webserver(), add 'statsRequireAuthentication' parameter This PR deprecates the use of additional parameters with `webserver()`, as the syntax is confusing and could lead to believe that the parameters are per-instance while they actually are global. Also implements an additional 'statsRequireAuthentication' parameter to allow scraping the statistics without any kind of authentication, which is useful to Prometheus setups with dynamic service discovery. --- diff --git a/pdns/dnsdist-console.cc b/pdns/dnsdist-console.cc index c2ac58bed0..29a555dd0b 100644 --- a/pdns/dnsdist-console.cc +++ b/pdns/dnsdist-console.cc @@ -578,7 +578,7 @@ const std::vector g_consoleKeywords{ { "setUDPMultipleMessagesVectorSize", true, "n", "set the size of the vector passed to recvmmsg() to receive UDP messages. Default to 1 which means that the feature is disabled and recvmsg() is used instead" }, { "setUDPTimeout", true, "n", "set the maximum time dnsdist will wait for a response from a backend over UDP, in seconds" }, { "setVerboseHealthChecks", true, "bool", "set whether health check errors will be logged" }, - { "setWebserverConfig", true, "[{password=string, apiKey=string, customHeaders}]", "Updates webserver configuration" }, + { "setWebserverConfig", true, "[{password=string, apiKey=string, customHeaders, statsRequireAuthentication}]", "Updates webserver configuration" }, { "setWeightedBalancingFactor", true, "factor", "Set the balancing factor for bounded-load weighted policies (whashed, wrandom)" }, { "setWHashedPertubation", true, "value", "Set the hash perturbation value to be used in the whashed policy instead of a random one, allowing to have consistent whashed results on different instance" }, { "show", true, "string", "outputs `string`" }, @@ -635,7 +635,7 @@ const std::vector g_consoleKeywords{ { "TrailingDataRule", true, "", "Matches if the query has trailing data" }, { "truncateTC", true, "bool", "if set (defaults to no starting with dnsdist 1.2.0) truncate TC=1 answers so they are actually empty. Fixes an issue for PowerDNS Authoritative Server 2.9.22. Note: turning this on breaks compatibility with RFC 6891." }, { "unregisterDynBPFFilter", true, "DynBPFFilter", "unregister this dynamic BPF filter" }, - { "webserver", true, "address:port, password [, apiKey [, customHeaders ]])", "launch a webserver with stats on that address with that password" }, + { "webserver", true, "address:port", "launch a webserver with stats on that address" }, { "whashed", false, "", "Weighted hashed ('sticky') distribution over available servers, based on the server 'weight' parameter" }, { "chashed", false, "", "Consistent hashed ('sticky') distribution over available servers, also based on the server 'weight' parameter" }, { "wrandom", false, "", "Weighted random over available servers, based on the server 'weight' parameter" }, diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc index e8049beb34..0981a40bf7 100644 --- a/pdns/dnsdist-lua.cc +++ b/pdns/dnsdist-lua.cc @@ -841,7 +841,7 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) g_carbon.setState(ours); }); - luaCtx.writeFunction("webserver", [client,configCheck](const std::string& address, const std::string& password, const boost::optional apiKey, const boost::optional > customHeaders, const boost::optional acl) { + luaCtx.writeFunction("webserver", [client,configCheck](const std::string& address, const boost::optional password, const boost::optional apiKey, const boost::optional > customHeaders, const boost::optional acl) { setLuaSideEffect(); ComboAddress local; try { @@ -855,15 +855,28 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) return; } + if (password || apiKey || customHeaders || acl) { + warnlog("Passing additional parameters to 'webserver()' is deprecated, please use 'setWebserverConfig()' instead."); + } + try { int sock = SSocket(local.sin4.sin_family, SOCK_STREAM, 0); SSetsockopt(sock, SOL_SOCKET, SO_REUSEADDR, 1); SBind(sock, local); SListen(sock, 5); auto launch=[sock, local, password, apiKey, customHeaders, acl]() { - setWebserverPassword(password); - setWebserverAPIKey(apiKey); - setWebserverCustomHeaders(customHeaders); + if (password) { + setWebserverPassword(*password); + } + + if (apiKey) { + setWebserverAPIKey(apiKey); + } + + if (customHeaders) { + setWebserverCustomHeaders(customHeaders); + } + if (acl) { setWebserverACL(*acl); } @@ -877,14 +890,14 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) launch(); } } - catch(std::exception& e) { + catch (const std::exception& e) { g_outputBuffer="Unable to bind to webserver socket on " + local.toStringWithPort() + ": " + e.what(); errlog("Unable to bind to webserver socket on %s: %s", local.toStringWithPort(), e.what()); } }); - typedef std::unordered_map> > webserveropts_t; + typedef std::unordered_map> > webserveropts_t; luaCtx.writeFunction("setWebserverConfig", [](boost::optional vars) { setLuaSideEffect(); @@ -892,26 +905,34 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck) if (!vars) { return ; } - if(vars->count("password")) { + + if (vars->count("password")) { const std::string password = boost::get(vars->at("password")); setWebserverPassword(password); } - if(vars->count("apiKey")) { + + if (vars->count("apiKey")) { const std::string apiKey = boost::get(vars->at("apiKey")); setWebserverAPIKey(apiKey); } + if (vars->count("acl")) { const std::string acl = boost::get(vars->at("acl")); setWebserverACL(acl); } - if(vars->count("customHeaders")) { + + if (vars->count("customHeaders")) { const boost::optional > headers = boost::get >(vars->at("customHeaders")); setWebserverCustomHeaders(headers); } + + if (vars->count("statsRequireAuthentication")) { + setWebserverStatsRequireAuthentication(boost::get(vars->at("statsRequireAuthentication"))); + } }); luaCtx.writeFunction("controlSocket", [client,configCheck](const std::string& str) { diff --git a/pdns/dnsdist-web.cc b/pdns/dnsdist-web.cc index 0dc9a19b22..ebba297f71 100644 --- a/pdns/dnsdist-web.cc +++ b/pdns/dnsdist-web.cc @@ -41,6 +41,21 @@ #include "threadname.hh" #include "sstuff.hh" +struct WebserverConfig +{ + WebserverConfig() + { + acl.toMasks("127.0.0.1, ::1"); + } + + std::mutex lock; + NetmaskGroup acl; + std::string password; + std::string apiKey; + boost::optional > customHeaders; + bool statsRequireAuthentication{true}; +}; + bool g_apiReadWrite{false}; WebserverConfig g_webserverConfig; std::string g_apiConfigDirectory; @@ -190,10 +205,19 @@ static bool isAStatsRequest(const YaHTTP::Request& req) return req.url.path == "/jsonstat" || req.url.path == "/metrics"; } -static bool compareAuthorization(const YaHTTP::Request& req) +static bool handleAuthorization(const YaHTTP::Request& req) { + cerr<<"handling auth for "< lock(g_webserverConfig.lock); + if (isAStatsRequest(req)) { + if (g_webserverConfig.statsRequireAuthentication) { + /* Access to the stats is allowed for both API and Web users */ + return checkAPIKey(req, g_webserverConfig.apiKey) || checkWebPassword(req, g_webserverConfig.password); + } + return true; + } + if (isAnAPIRequest(req)) { /* Access to the API requires a valid API key */ if (checkAPIKey(req, g_webserverConfig.apiKey)) { @@ -203,11 +227,6 @@ static bool compareAuthorization(const YaHTTP::Request& req) return isAnAPIRequestAllowedWithWebAuth(req) && checkWebPassword(req, g_webserverConfig.password); } - if (isAStatsRequest(req)) { - /* Access to the stats is allowed for both API and Web users */ - return checkAPIKey(req, g_webserverConfig.apiKey) || checkWebPassword(req, g_webserverConfig.password); - } - return checkWebPassword(req, g_webserverConfig.password); } @@ -1276,7 +1295,7 @@ static void connectionThread(int sockFD, ComboAddress remote) handleCORS(req, resp); resp.status = 200; } - else if (!compareAuthorization(req)) { + else if (!handleAuthorization(req)) { YaHTTP::strstr_map_t::iterator header = req.headers.find("authorization"); if (header != req.headers.end()) { errlog("HTTP Request \"%s\" from %s: Web Authentication failed", req.url.path, remote.toStringWithPort()); @@ -1350,11 +1369,25 @@ void setWebserverCustomHeaders(const boost::optional lock(g_webserverConfig.lock); + + g_webserverConfig.statsRequireAuthentication = require; +} + void dnsdistWebserverThread(int sock, const ComboAddress& local) { setThreadName("dnsdist/webserv"); warnlog("Webserver launched on %s", local.toStringWithPort()); + { + std::lock_guard lock(g_webserverConfig.lock); + if (g_webserverConfig.password.empty()) { + warnlog("Webserver launched on %s without a password set!", local.toStringWithPort()); + } + } + for(;;) { try { ComboAddress remote(local); diff --git a/pdns/dnsdistdist/dnsdist-web.hh b/pdns/dnsdistdist/dnsdist-web.hh index 889139f6d0..d0d70e6821 100644 --- a/pdns/dnsdistdist/dnsdist-web.hh +++ b/pdns/dnsdistdist/dnsdist-web.hh @@ -1,23 +1,10 @@ #pragma once -struct WebserverConfig -{ - WebserverConfig() - { - acl.toMasks("127.0.0.1, ::1"); - } - - NetmaskGroup acl; - std::string password; - std::string apiKey; - boost::optional > customHeaders; - std::mutex lock; -}; - void setWebserverAPIKey(const boost::optional apiKey); void setWebserverPassword(const std::string& password); void setWebserverACL(const std::string& acl); void setWebserverCustomHeaders(const boost::optional > customHeaders); +void setWebserverStatsRequireAuthentication(bool); void dnsdistWebserverThread(int sock, const ComboAddress& local); diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst index a6f848dd42..de4687d00b 100644 --- a/pdns/dnsdistdist/docs/reference/config.rst +++ b/pdns/dnsdistdist/docs/reference/config.rst @@ -316,12 +316,17 @@ Control Socket, Console and Webserver Webserver configuration ~~~~~~~~~~~~~~~~~~~~~~~ -.. function:: webserver(listen_address, password[, apikey[, custom_headers[, acl]]]) +.. function:: webserver(listen_address [, password[, apikey[, custom_headers[, acl]]]]) .. versionchanged:: 1.5.0 ``acl`` optional parameter added. - Launch the :doc:`../guides/webserver` with statistics and the API. + .. versionchanged:: 1.6.0 + The ``password`` parameter is now optional. + The use of optional parameters is now deprecated. Please use :func:`setWebserverConfig` instead. + + Launch the :doc:`../guides/webserver` with statistics and the API. Note that the parameters are global, so the parameter from the last ``webserver`` will override any existing ones. For this reason + the use of :func:`setWebserverConfig` is advised instead of specifying optional parameters here. :param str listen_address: The IP address and Port to listen on :param str password: The password required to access the webserver @@ -345,6 +350,9 @@ Webserver configuration .. versionchanged:: 1.5.0 ``acl`` optional parameter added. + .. versionchanged:: 1.6.0 + ``statsRequireAuthentication`` optional parameter added. + Setup webserver configuration. See :func:`webserver`. :param table options: A table with key: value pairs with webserver options. @@ -355,6 +363,7 @@ Webserver configuration * ``apiKey=newKey``: string - Changes the API Key (set to an empty string do disable it) * ``custom_headers={[str]=str,...}``: map of string - Allows setting custom headers and removing the defaults. * ``acl=newACL``: string - List of IP addresses, as a string, that are allowed to open a connection to the web server. Defaults to "127.0.0.1, ::1". + * ``statsRequireAuthentication``: bool - Whether access to the statistics (/metrics and /jsonstat endpoints) require a valid password or API key. Defaults to true. .. function:: registerWebHandler(path, handler) diff --git a/regression-tests.dnsdist/test_API.py b/regression-tests.dnsdist/test_API.py index 3419304945..327c781a5b 100644 --- a/regression-tests.dnsdist/test_API.py +++ b/regression-tests.dnsdist/test_API.py @@ -22,7 +22,8 @@ class TestAPIBasics(DNSDistTest): _config_template = """ setACL({"127.0.0.1/32", "::1/128"}) newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ def testBasicAuth(self): @@ -305,7 +306,8 @@ class TestAPIServerDown(DNSDistTest): setACL({"127.0.0.1/32", "::1/128"}) newServer{address="127.0.0.1:%s"} getServer(0):setDown() - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ def testServerDownNoLatencyLocalhost(self): @@ -333,7 +335,8 @@ class TestAPIWritable(DNSDistTest): _config_template = """ setACL({"127.0.0.1/32", "::1/128"}) newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) setAPIWritable(true, "%s") """ @@ -412,7 +415,8 @@ class TestAPICustomHeaders(DNSDistTest): controlSocket("127.0.0.1:%s") setACL({"127.0.0.1/32", "::1/128"}) newServer({address="127.0.0.1:%s"}) - webserver("127.0.0.1:%s", "%s", "%s", {["X-Frame-Options"]="", ["X-Custom"]="custom"}) + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s", customHeaders={["X-Frame-Options"]="", ["X-Custom"]="custom"} }) """ def testBasicHeaders(self): @@ -441,6 +445,63 @@ class TestAPICustomHeaders(DNSDistTest): self.assertEquals(r.headers.get('x-powered-by'), "dnsdist") self.assertTrue("x-frame-options" in r.headers) +class TestStatsWithoutAuthentication(DNSDistTest): + + _webTimeout = 2.0 + _webServerPort = 8083 + _webServerBasicAuthPassword = 'secret' + _webServerAPIKey = 'apisecret' + # paths accessible using the API key only + _apiOnlyPath = '/api/v1/servers/localhost/config' + # paths accessible using basic auth only (list not exhaustive) + _basicOnlyPath = '/' + _noAuthenticationPaths = [ '/metrics', '/jsonstat?command=dynblocklist' ] + _consoleKey = DNSDistTest.generateConsoleKey() + _consoleKeyB64 = base64.b64encode(_consoleKey).decode('ascii') + _config_params = ['_consoleKeyB64', '_consolePort', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] + _config_template = """ + setKey("%s") + controlSocket("127.0.0.1:%s") + setACL({"127.0.0.1/32", "::1/128"}) + newServer({address="127.0.0.1:%s"}) + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s", statsRequireAuthentication=false }) + """ + + def testAuth(self): + """ + API: Stats do not require authentication + """ + + for path in self._noAuthenticationPaths: + url = 'http://127.0.0.1:' + str(self._webServerPort) + path + + r = requests.get(url, timeout=self._webTimeout) + self.assertTrue(r) + self.assertEquals(r.status_code, 200) + + # these should still require basic authentication + for path in [self._basicOnlyPath]: + url = 'http://127.0.0.1:' + str(self._webServerPort) + path + + r = requests.get(url, timeout=self._webTimeout) + self.assertEquals(r.status_code, 401) + + r = requests.get(url, auth=('whatever', self._webServerBasicAuthPassword), timeout=self._webTimeout) + self.assertTrue(r) + self.assertEquals(r.status_code, 200) + + # these should still require API authentication + for path in [self._apiOnlyPath]: + url = 'http://127.0.0.1:' + str(self._webServerPort) + path + + r = requests.get(url, timeout=self._webTimeout) + self.assertEquals(r.status_code, 401) + + headers = {'x-api-key': self._webServerAPIKey} + r = requests.get(url, headers=headers, timeout=self._webTimeout) + self.assertTrue(r) + self.assertEquals(r.status_code, 200) class TestAPIAuth(DNSDistTest): @@ -462,7 +523,8 @@ class TestAPIAuth(DNSDistTest): controlSocket("127.0.0.1:%s") setACL({"127.0.0.1/32", "::1/128"}) newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ def testBasicAuthChange(self): @@ -532,7 +594,8 @@ class TestAPIACL(DNSDistTest): controlSocket("127.0.0.1:%s") setACL({"127.0.0.1/32", "::1/128"}) newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s", {}, "192.0.2.1") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s", acl="192.0.2.1"}) """ def testACLChange(self): diff --git a/regression-tests.dnsdist/test_DynBlocks.py b/regression-tests.dnsdist/test_DynBlocks.py index 53b8935e71..5b174e703c 100644 --- a/regression-tests.dnsdist/test_DynBlocks.py +++ b/regression-tests.dnsdist/test_DynBlocks.py @@ -560,14 +560,15 @@ class TestDynBlockQPS(DynBlocksTest): _dynBlockQPS = 10 _dynBlockPeriod = 2 _dynBlockDuration = 5 - _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] _config_template = """ function maintenance() addDynBlocks(exceedQRate(%d, %d), "Exceeded query rate", %d) end newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ + _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] def testDynBlocksQRate(self): """ @@ -581,7 +582,6 @@ class TestDynBlockGroupQPS(DynBlocksTest): _dynBlockQPS = 10 _dynBlockPeriod = 2 _dynBlockDuration = 5 - _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] _config_template = """ local dbr = dynBlockRulesGroup() dbr:setQueryRate(%d, %d, "Exceeded query rate", %d) @@ -590,8 +590,10 @@ class TestDynBlockGroupQPS(DynBlocksTest): dbr:apply() end newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ + _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] def testDynBlocksQRate(self): """ @@ -1073,7 +1075,6 @@ class TestDynBlockGroupNoOp(DynBlocksTest): _dynBlockQPS = 10 _dynBlockPeriod = 2 _dynBlockDuration = 5 - _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] _config_template = """ local dbr = dynBlockRulesGroup() dbr:setQueryRate(%d, %d, "Exceeded query rate", %d, DNSAction.NoOp) @@ -1083,8 +1084,10 @@ class TestDynBlockGroupNoOp(DynBlocksTest): end newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ + _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] def testNoOp(self): """ @@ -1136,7 +1139,6 @@ class TestDynBlockGroupWarning(DynBlocksTest): _dynBlockQPS = 20 _dynBlockPeriod = 2 _dynBlockDuration = 5 - _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_dynBlockWarningQPS', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] _config_template = """ local dbr = dynBlockRulesGroup() dbr:setQueryRate(%d, %d, "Exceeded query rate", %d, DNSAction.Drop, %d) @@ -1146,8 +1148,10 @@ class TestDynBlockGroupWarning(DynBlocksTest): end newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ + _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_dynBlockWarningQPS', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] def testWarning(self): """ diff --git a/regression-tests.dnsdist/test_Prometheus.py b/regression-tests.dnsdist/test_Prometheus.py index 621c62020a..4b28f6a652 100644 --- a/regression-tests.dnsdist/test_Prometheus.py +++ b/regression-tests.dnsdist/test_Prometheus.py @@ -15,7 +15,8 @@ class TestPrometheus(DNSDistTest): _config_params = ['_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] _config_template = """ newServer{address="127.0.0.1:%s"} - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ def checkPrometheusContentBasic(self, content): diff --git a/regression-tests.dnsdist/test_TCPFastOpen.py b/regression-tests.dnsdist/test_TCPFastOpen.py index 9b7582734e..58d9c96a26 100644 --- a/regression-tests.dnsdist/test_TCPFastOpen.py +++ b/regression-tests.dnsdist/test_TCPFastOpen.py @@ -22,7 +22,8 @@ class TestBrokenTCPFastOpen(DNSDistTest): _config_params = ['_testServerPort', '_testServerRetries', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] _config_template = """ newServer{address="127.0.0.1:%s", useClientSubnet=true, tcpFastOpen=true, retries=%d } - webserver("127.0.0.1:%s", "%s", "%s") + webserver("127.0.0.1:%s") + setWebserverConfig({password="%s", apiKey="%s"}) """ @classmethod