Add healthcheck (format=prometheus) custom auth

Author: yurikiselev

ydb/core/driver_lib/run/run.cpp

@@ -621,11 +621,15 @@ void TKikimrRunner::InitializeMonitoring(const TKikimrRunConfig& runConfig, bool
         monConfig.PrivateKeyFile = appConfig.GetMonitoringConfig().GetMonitoringPrivateKeyFile();
         monConfig.RedirectMainPageTo = appConfig.GetMonitoringConfig().GetRedirectMainPageTo();
         monConfig.RequireCountersAuthentication = appConfig.GetMonitoringConfig().GetRequireCountersAuthentication();
+        monConfig.RequireHealthcheckAuthentication = appConfig.GetMonitoringConfig().GetRequireHealthcheckAuthentication();
 
-        // custom monitoring authentication may be enabled only if enforce_user_token_requirement is enabled
+        // Custom monitoring authentication may be enabled only if user authentication is mandatory
         const auto& securityConfig(runConfig.AppConfig.GetDomainsConfig().GetSecurityConfig());
         const bool enforceUserToken = securityConfig.GetEnforceUserTokenRequirement();
-        Y_ABORT_UNLESS(enforceUserToken || !monConfig.RequireCountersAuthentication, "Setting EnforceUserTokenRequirement is disabled, but RequireCountersAuthentication is enabled");
+        Y_ABORT_UNLESS(enforceUserToken || !monConfig.RequireCountersAuthentication,
+            "Setting EnforceUserTokenRequirement is disabled, but RequireCountersAuthentication is enabled");
+        Y_ABORT_UNLESS(enforceUserToken || !monConfig.RequireHealthcheckAuthentication,
+            "Setting EnforceUserTokenRequirement is disabled, but RequireHealthcheckAuthentication is enabled");
 
         if (includeHostName) {
             if (appConfig.HasNameserviceConfig() && appConfig.GetNameserviceConfig().NodeSize() > 0) {

ydb/core/mon/mon.cpp

@@ -167,8 +167,8 @@ TVector<TString> TMon::GetCountersAllowedSIDs(TActorSystem* actorSystem) {
         return countersAllowedSIDs;
     }
     const auto& securityConfig = appData->DomainsConfig.GetSecurityConfig();
-    auto addSIDs = [&countersAllowedSIDs](const auto& protoAllowedSIDs) {
-        for (const auto& sid : protoAllowedSIDs) {
+    const auto addSIDs = [&countersAllowedSIDs](const auto& allowedSIDs) {
+        for (const auto& sid : allowedSIDs) {
             countersAllowedSIDs.emplace_back(sid);
         }
     };
@@ -1680,12 +1680,11 @@ std::future<void> TMon::Start(TActorSystem* actorSystem) {
     ActorSystem->RegisterLocalService(NodeProxyServiceActorId, nodeProxyActorId);
     IActor* countersMonPageServiceActor;
     if (Config.RequireCountersAuthentication) {
-        const auto countersAllowedSIDs = GetCountersAllowedSIDs(ActorSystem);
         countersMonPageServiceActor = new THttpMonPageService(
             HttpProxyActorId,
             CountersMonPage,
             Config.Authorizer,
-            countersAllowedSIDs);
+            GetCountersAllowedSIDs(ActorSystem));
     } else {
         countersMonPageServiceActor = new THttpMonPageService(
             HttpProxyActorId,

ydb/core/protos/config.proto

@@ -620,6 +620,7 @@ message TMonitoringConfig {
     optional string InactivityTimeout = 18 [default = "2m"];
     optional bool HideHttpEndpoint = 19 [default = false];
     optional bool RequireCountersAuthentication = 21 [default = false];
+    optional bool RequireHealthcheckAuthentication = 22 [default = false];
 }
 
 message TRestartsCountConfig {

ydb/core/viewer/viewer.cpp

@@ -140,7 +140,7 @@ class TViewer : public TActorBootstrapped<TViewer>, public IViewer {
                 .ActorSystem = ctx.ActorSystem(),
                 .ActorId = ctx.SelfID,
                 .AuthMode = enforceUserToken ? TMon::EAuthMode::ExtractOnly : TMon::EAuthMode::Disabled,
-                .AllowedSIDs = enforceUserToken ? databaseAllowedSIDs : TVector<TString>(),
+                .AllowedSIDs = enforceUserToken ? viewerAllowedSIDs : TVector<TString>(),
             });
             mon->RegisterActorPage({
                 .RelPath = "vdisk",

ydb/core/viewer/viewer_healthcheck.h

@@ -89,7 +89,7 @@ class TJsonHealthCheck : public TViewerPipeClient {
         if (Database.empty()) {
             Database = Params.Get("tenant");
         }
-        if (!IsDatabaseRequest() && Format != HealthCheckResponseFormat::PROMETHEUS && !Viewer->CheckAccessMonitoring(GetRequest())) {
+        if (!IsDatabaseRequest() && !CheckAccess()) {
             return TBase::ReplyAndPassAway(GetHTTPFORBIDDEN("text/plain", "Access denied"));
         }
         Cache = FromStringWithDefault<bool>(Params.Get("cache"), Cache);
@@ -107,6 +107,21 @@ class TJsonHealthCheck : public TViewerPipeClient {
         Become(&TThis::StateRequestedInfo, Timeout, new TEvents::TEvWakeup());
     }
 
+    bool CheckAccess() const {
+        if (Format == HealthCheckResponseFormat::PROMETHEUS) {
+            // This format was left without any authentication checks for a long time,
+            // so we check access for it only when it's required with a separate flag.
+            const auto& config = Viewer->GetKikimrRunConfig();
+            const auto requireHealthcheckAuth = config.AppConfig.GetMonitoringConfig().GetRequireHealthcheckAuthentication();
+            // We want metrics collection systems to have minimal permissions, so we check for viewer access
+            return !requireHealthcheckAuth || Viewer->CheckAccessViewer(GetRequest());
+        }
+
+        // But the general healthcheck info should not be accessible for those
+        // who have only viewer grants so we check for the monitoring grants.
+        return Viewer->CheckAccessMonitoring(GetRequest());
+    }
+
     STFUNC(StateRequestedInfo) {
         switch (ev->GetTypeRewrite()) {
             hFunc(NHealthCheck::TEvSelfCheckResult, Handle);

ydb/tests/functional/security/test_mon_endpoints_auth.py

@@ -114,6 +114,7 @@ def create_ydb_configurator(
     certificates,
     enforce_user_token_requirement=True,
     require_counters_authentication=None,
+    require_healthcheck_authentication=None,
     database_allowed_sids=['database@builtin'],
     viewer_allowed_sids=['viewer@builtin'],
     monitoring_allowed_sids=['monitoring@builtin'],
@@ -132,13 +133,17 @@ def create_ydb_configurator(
     config_generator.monitoring_tls_cert_path = certificates['server_cert']
     config_generator.monitoring_tls_key_path = certificates['server_key']
 
-    if require_counters_authentication is not None:
+    if require_counters_authentication is not None or require_healthcheck_authentication is not None:
         if 'monitoring_config' not in config_generator.yaml_config:
             config_generator.yaml_config['monitoring_config'] = {}
         if require_counters_authentication is not None:
             config_generator.yaml_config['monitoring_config'][
                 'require_counters_authentication'
             ] = require_counters_authentication
+        if require_healthcheck_authentication is not None:
+            config_generator.yaml_config['monitoring_config'][
+                'require_healthcheck_authentication'
+            ] = require_healthcheck_authentication
 
     if database_allowed_sids is not None:
         config_generator.yaml_config['domains_config']['security_config'][
@@ -201,6 +206,19 @@ def ydb_cluster_with_require_counters_auth(certificates):
     cluster.stop()
 
 
+@pytest.fixture(scope='module')
+def ydb_cluster_with_require_healthcheck_auth(certificates):
+    configurator = create_ydb_configurator(
+        certificates,
+        enforce_user_token_requirement=True,
+        require_healthcheck_authentication=True,
+    )
+    cluster = KiKiMR(configurator)
+    cluster.start()
+    yield cluster
+    cluster.stop()
+
+
 EXPECTED_RESULTS_WITH_ENFORCE_USER_TOKEN = {
     '/counters': {
         None: 200,
@@ -479,3 +497,33 @@ def test_with_require_counters_authentication(ydb_cluster_with_require_counters_
         },  # checks this just in case
     }
     _test_endpoints(ydb_cluster_with_require_counters_auth, EXPECTED_RESULTS_WITH_REQUIRE_COUNTERS_AUTH)
+
+
+def test_with_require_healthcheck_authentication(ydb_cluster_with_require_healthcheck_auth):
+    EXPECTED_RESULTS_WITH_REQUIRE_HEALTHCHECK_AUTH = {
+        '/healthcheck?format=prometheus': {
+            None: 403,
+            'user@builtin': 403,
+            'database@builtin': 403,
+            'viewer@builtin': 200,
+            'monitoring@builtin': 200,
+            'root@builtin': 200,
+        },
+        '/healthcheck': {
+            None: 403,
+            'user@builtin': 403,
+            'database@builtin': 403,
+            'viewer@builtin': 403,
+            'monitoring@builtin': 200,
+            'root@builtin': 200,
+        },
+        '/ping': {
+            None: 200,
+            'user@builtin': 200,
+            'database@builtin': 200,
+            'viewer@builtin': 200,
+            'monitoring@builtin': 200,
+            'root@builtin': 200,
+        },  # checks this just in case
+    }
+    _test_endpoints(ydb_cluster_with_require_healthcheck_auth, EXPECTED_RESULTS_WITH_REQUIRE_HEALTHCHECK_AUTH)