diff options
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 <vespa/vespalib/util/exceptions.h> #include <vespa/vespalib/util/size_literals.h> #include <vespa/vespalib/component/vtag.h> +#include <vespa/vespalib/net/connection_auth_context.h> #include <vespa/vespalib/net/crypto_engine.h> #include <vespa/config/subscription/configuri.h> #include <vespa/config/helper/configfetcher.hpp> @@ -45,15 +46,15 @@ StatusWebServer::~StatusWebServer() void StatusWebServer::configure(std::unique_ptr<vespa::config::content::core::StorStatusConfig> 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<WebServer> 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<WebServer>(*this, newPort); @@ -146,6 +147,25 @@ StatusWebServer::getListenPort() const } 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) { vespalib::string link(urlpath.getPath()); @@ -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 << "<p><b>Binary version of Vespa:</b> " << 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<vespa::config::content::core::StorStatusConfig> { 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<vespa::config::content::core::StorStatusConfig> 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 <ostream> #include <vespa/storageframework/generic/status/httpurlpath.h> +#include <vespa/vespalib/net/tls/capability_set.h> #include <vespa/vespalib/stllike/string.h> 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(); @@ -40,6 +43,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. */ |