From acbcc3b21c245446897b439637696155068e1d69 Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Mon, 29 Aug 2022 08:29:18 +0000 Subject: Add capability filtering for content layer status pages and metrics This currently only applies to the port exposed for the content node or distributor specific status pages and metrics export, not state V1. --- .../vespa/storage/common/statusmetricconsumer.h | 4 ++ .../frameworkimpl/status/statuswebserver.cpp | 53 ++++++++++++++++------ .../storage/frameworkimpl/status/statuswebserver.h | 10 ++-- .../generic/status/statusreporter.h | 17 ++++++- 4 files changed, 64 insertions(+), 20 deletions(-) diff --git a/storage/src/vespa/storage/common/statusmetricconsumer.h b/storage/src/vespa/storage/common/statusmetricconsumer.h index 3da6bd3151a..337c3ea7ff0 100644 --- a/storage/src/vespa/storage/common/statusmetricconsumer.h +++ b/storage/src/vespa/storage/common/statusmetricconsumer.h @@ -34,6 +34,10 @@ public: const std::string& name = "status"); ~StatusMetricConsumer() override; + // Metric reporting requires the "vespa.content.metrics_api" capability + CapabilitySet required_capabilities() const noexcept override { + return CapabilitySet::of({ Capability::content_metrics_api() }); + } vespalib::string getReportContentType(const framework::HttpUrlPath&) const override; bool reportStatus(std::ostream& out, const framework::HttpUrlPath&) const override; void updateMetrics(const MetricLockGuard & guard) override; diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp index a9b7a727a5b..a053ee1a13e 100644 --- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp +++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -45,15 +46,15 @@ StatusWebServer::~StatusWebServer() void StatusWebServer::configure(std::unique_ptr config) { int newPort = config->httpport; - // If server is already running, ignore config updates that doesn't - // alter port, or suggests random port. + // If server is already running, ignore config updates that doesn't + // alter port, or suggests random port. if (_httpServer) { if (newPort == 0 || newPort == _port) return; } - // Try to create new server before destroying old. + // Try to create new server before destroying old. LOG(info, "Starting status web server on port %u.", newPort); std::unique_ptr server; - // Negative port number means don't run the web server + // Negative port number means don't run the web server if (newPort >= 0) { try { server = std::make_unique(*this, newPort); @@ -145,6 +146,25 @@ StatusWebServer::getListenPort() const return _httpServer ? _httpServer->getListenPort() : -1; } +void +StatusWebServer::invoke_reporter(const framework::StatusReporter& reporter, + const framework::HttpUrlPath& url_path, + vespalib::Portal::GetRequest& request) +{ + try { + std::ostringstream content; + auto content_type = reporter.getReportContentType(url_path); + if (reporter.reportStatus(content, url_path)) { + request.respond_with_content(content_type, content.str()); + } else { + request.respond_with_error(404, "Not Found"); + } + } catch (std::exception &e) { + LOG(warning, "Internal Server Error: %s", e.what()); + request.respond_with_error(500, "Internal Server Error"); + } +} + void StatusWebServer::handlePage(const framework::HttpUrlPath& urlpath, vespalib::Portal::GetRequest request) { @@ -157,22 +177,25 @@ StatusWebServer::handlePage(const framework::HttpUrlPath& urlpath, vespalib::Por if ( ! link.empty()) { const framework::StatusReporter *reporter = _reporterMap.getStatusReporter(link); if (reporter != nullptr) { - try { - std::ostringstream content; - auto content_type = reporter->getReportContentType(urlpath); - if (reporter->reportStatus(content, urlpath)) { - request.respond_with_content(content_type, content.str()); - } else { - request.respond_with_error(404, "Not Found"); - } - } catch (std::exception &e) { - LOG(warning, "Internal Server Error: %s", e.what()); - request.respond_with_error(500, "Internal Server Error"); + const auto& auth_ctx = request.auth_context(); + if (auth_ctx.capabilities().contains_all(reporter->required_capabilities())) { + invoke_reporter(*reporter, urlpath, request); + } else { + // TODO should print peer address as well; not currently exposed + LOG(warning, "Peer with %s denied status page access to '%s' due to insufficient " + "credentials (had %s, needed %s)", + vespalib::net::tls::to_string(auth_ctx.peer_credentials()).c_str(), + link.c_str(), auth_ctx.capabilities().to_string().c_str(), + reporter->required_capabilities().to_string().c_str()); + request.respond_with_error(403, "Forbidden"); } } else { request.respond_with_error(404, "Not Found"); } } else { + // TODO should the index page be capability-restricted? Would be a bit strange if the root + // index '/' page requires status capabilities but '/metrics' does not. + // The index page only leaks the Vespa version and node type (inferred by reporter set). IndexPageReporter indexRep; indexRep << "

Binary version of Vespa: " << vespalib::Vtag::currentVersion.toString() diff --git a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h index 429f5249441..26db7ff5069 100644 --- a/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h +++ b/storage/src/vespa/storage/frameworkimpl/status/statuswebserver.h @@ -24,6 +24,7 @@ namespace config { namespace storage { namespace framework { + struct StatusReporter; struct StatusReporterMap; struct ThreadHandle; struct ComponentRegister; @@ -35,10 +36,10 @@ namespace framework { class StatusWebServer : private config::IFetcherCallback { class WebServer : public vespalib::Portal::GetHandler { - StatusWebServer& _status; - vespalib::Portal::SP _server; + StatusWebServer& _status; + vespalib::Portal::SP _server; vespalib::ThreadStackExecutor _executor; - vespalib::Portal::Token::UP _root; + vespalib::Portal::Token::UP _root; public: WebServer(StatusWebServer&, uint16_t port); @@ -81,6 +82,9 @@ public: int getListenPort() const; void handlePage(const framework::HttpUrlPath&, vespalib::Portal::GetRequest request); private: + void invoke_reporter(const framework::StatusReporter&, + const framework::HttpUrlPath&, + vespalib::Portal::GetRequest&); void configure(std::unique_ptr config) override; }; diff --git a/storage/src/vespa/storageframework/generic/status/statusreporter.h b/storage/src/vespa/storageframework/generic/status/statusreporter.h index cc48bb841fd..3f84d5e8ae4 100644 --- a/storage/src/vespa/storageframework/generic/status/statusreporter.h +++ b/storage/src/vespa/storageframework/generic/status/statusreporter.h @@ -16,12 +16,15 @@ #include #include +#include #include namespace storage::framework { -struct StatusReporter -{ +struct StatusReporter { + using Capability = vespalib::net::tls::Capability; + using CapabilitySet = vespalib::net::tls::CapabilitySet; + StatusReporter(vespalib::stringref id, vespalib::stringref name); virtual ~StatusReporter(); @@ -39,6 +42,16 @@ struct StatusReporter virtual bool isValidStatusRequest() const { return true; } + /** + * By default, a status reporter requires the "vespa.content.status_pages" client capability. + * This can be overridden by subclasses to require reporter-specific capabilities + * (or none at all). If the client does not satisfy the required capabilities, a + * "403 Forbidden" error response will be returned to the client. + */ + virtual CapabilitySet required_capabilities() const noexcept { + return CapabilitySet::of({ Capability::content_status_pages() }); + } + /** * Called to get content type. * An empty string indicates page not found. -- cgit v1.2.3