From 912fdb9700c5578658b099fa077565ff941f030d Mon Sep 17 00:00:00 2001 From: Mattia Codato Date: Fri, 30 Jul 2021 10:17:43 +0200 Subject: [PATCH 1/2] Fix update execution message discarded refs Icinga#8616 --- lib/icinga/apiactions.cpp | 10 ++++ lib/icinga/clusterevents.cpp | 88 ++++++++++++++++++++++++++++++++---- 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/lib/icinga/apiactions.cpp b/lib/icinga/apiactions.cpp index 0cb8c98f946..2d35c466770 100644 --- a/lib/icinga/apiactions.cpp +++ b/lib/icinga/apiactions.cpp @@ -681,6 +681,15 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons if (!endpointPtr) return ApiActions::CreateResult(404, "Can't find a valid endpoint for '" + resolved_endpoint + "'."); + /* Check if the endpoint zone can access the checkable */ + Zone::Ptr endpointZone = endpointPtr->GetZone(); + if (!endpointZone->CanAccessObject(checkable)) { + return ApiActions::CreateResult( + 409, + "Zone '" + endpointZone->GetName() + "' cannot access checkable '" + checkable->GetName() + "'." + ); + } + /* Get command */ String command; @@ -806,6 +815,7 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons Dictionary::Ptr pending_execution = new Dictionary(); pending_execution->Set("pending", true); pending_execution->Set("deadline", deadline); + pending_execution->Set("endpoint", resolved_endpoint); Dictionary::Ptr executions = checkable->GetExecutions(); if (!executions) diff --git a/lib/icinga/clusterevents.cpp b/lib/icinga/clusterevents.cpp index 0bb4a06a09a..d1e58c55af0 100644 --- a/lib/icinga/clusterevents.cpp +++ b/lib/icinga/clusterevents.cpp @@ -761,9 +761,18 @@ Value ClusterEvents::ExecuteCommandAPIHandler(const MessageOrigin::Ptr& origin, } } + String executionUuid = params->Get("source"); + if (params->Contains("endpoint")) { Endpoint::Ptr execEndpoint = Endpoint::GetByName(params->Get("endpoint")); + if (!execEndpoint) { + Log(LogWarning, "ClusterEvents") + << "Discarding 'execute command' message " << executionUuid + << ": Endpoint " << params->Get("endpoint") << " does not exist"; + return Empty; + } + if (execEndpoint != Endpoint::GetLocalEndpoint()) { Zone::Ptr endpointZone = execEndpoint->GetZone(); Zone::Ptr localZone = Zone::GetLocalZone(); @@ -782,7 +791,7 @@ Value ClusterEvents::ExecuteCommandAPIHandler(const MessageOrigin::Ptr& origin, if (!(childEndpoint->GetCapabilities() & (uint_fast64_t)ApiCapabilities::ExecuteArbitraryCommand)) { double now = Utility::GetTime(); Dictionary::Ptr executedParams = new Dictionary(); - executedParams->Set("execution", params->Get("source")); + executedParams->Set("execution", executionUuid); executedParams->Set("host", params->Get("host")); if (params->Contains("service")) @@ -803,6 +812,58 @@ Value ClusterEvents::ExecuteCommandAPIHandler(const MessageOrigin::Ptr& origin, return Empty; } } + + Checkable::Ptr checkable; + Host::Ptr host = Host::GetByName(params->Get("host")); + if (!host) { + Log(LogWarning, "ClusterEvents") + << "Discarding 'execute command' message " << executionUuid + << ": host " << params->Get("host") << " does not exist"; + return Empty; + } + + if (params->Contains("service")) + checkable = host->GetServiceByShortName(params->Get("service")); + else + checkable = host; + + if (!checkable) { + String checkableName = host->GetName(); + if (params->Contains("service")) + checkableName += "!" + params->Get("service"); + + Log(LogWarning, "ClusterEvents") + << "Discarding 'execute command' message " << executionUuid + << ": " << checkableName << " does not exist"; + return Empty; + } + + /* Check if the child zone can access the checkable, and if it's the same endpoint zone */ + if (!zone->CanAccessObject(checkable) && zone != endpointZone) { + double now = Utility::GetTime(); + Dictionary::Ptr executedParams = new Dictionary(); + executedParams->Set("execution", executionUuid); + executedParams->Set("host", params->Get("host")); + + if (params->Contains("service")) + executedParams->Set("service", params->Get("service")); + + executedParams->Set("exit", 126); + executedParams->Set( + "output", + "Zone '" + zone->GetName() + "' cannot access to checkable '" + checkable->GetName() + "'." + ); + executedParams->Set("start", now); + executedParams->Set("end", now); + + Dictionary::Ptr executedMessage = new Dictionary(); + executedMessage->Set("jsonrpc", "2.0"); + executedMessage->Set("method", "event::ExecutedCommand"); + executedMessage->Set("params", executedParams); + + listener->RelayMessage(nullptr, nullptr, executedMessage, true); + return Empty; + } } } @@ -1179,13 +1240,6 @@ Value ClusterEvents::ExecutedCommandAPIHandler(const MessageOrigin::Ptr& origin, ObjectLock oLock (checkable); - if (origin->FromZone && !origin->FromZone->CanAccessObject(checkable)) { - Log(LogNotice, "ClusterEvents") - << "Discarding 'update executions API handler' message for checkable '" << checkable->GetName() - << "' from '" << origin->FromClient->GetIdentity() << "': Unauthorized access."; - return Empty; - } - if (!params->Contains("execution")) { Log(LogNotice, "ClusterEvents") << "Discarding 'update executions API handler' message for checkable '" << checkable->GetName() @@ -1213,6 +1267,22 @@ Value ClusterEvents::ExecutedCommandAPIHandler(const MessageOrigin::Ptr& origin, return Empty; } + Endpoint::Ptr command_endpoint = Endpoint::GetByName(execution->Get("endpoint")); + if (!command_endpoint) { + Log(LogNotice, "ClusterEvents") + << "Discarding 'update executions API handler' message from '" << origin->FromClient->GetIdentity() + << "': Command endpoint does not exists."; + + return Empty; + } + + if (origin->FromZone && !origin->FromZone->CanAccessObject(command_endpoint->GetZone())) { + Log(LogNotice, "ClusterEvents") + << "Discarding 'update executions API handler' message for checkable '" << checkable->GetName() + << "' from '" << origin->FromClient->GetIdentity() << "': Unauthorized access."; + return Empty; + } + if (params->Contains("exit")) execution->Set("exit", params->Get("exit")); @@ -1303,7 +1373,7 @@ Value ClusterEvents::UpdateExecutionsAPIHandler(const MessageOrigin::Ptr& origin updateMessage->Set("method", "event::UpdateExecutions"); updateMessage->Set("params", params); - listener->RelayMessage(origin, Zone::GetLocalZone(), updateMessage, true); + listener->RelayMessage(origin, checkable, updateMessage, true); return Empty; } From c5c17928a6e11c7996822dfd794ed994a8227ce7 Mon Sep 17 00:00:00 2001 From: Mattia Codato Date: Wed, 12 Apr 2023 17:30:04 +0200 Subject: [PATCH 2/2] Allow to exec command on endpoint where the checkable is not present but checkable has command_endpoint specified --- lib/icinga/apiactions.cpp | 10 ++++++++-- lib/icinga/clusterevents.cpp | 12 ++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/icinga/apiactions.cpp b/lib/icinga/apiactions.cpp index 2d35c466770..4a2759bb2a5 100644 --- a/lib/icinga/apiactions.cpp +++ b/lib/icinga/apiactions.cpp @@ -681,9 +681,15 @@ Dictionary::Ptr ApiActions::ExecuteCommand(const ConfigObject::Ptr& object, cons if (!endpointPtr) return ApiActions::CreateResult(404, "Can't find a valid endpoint for '" + resolved_endpoint + "'."); - /* Check if the endpoint zone can access the checkable */ + /* Return an error when + * the endpoint is different from the command endpoint of the checkable + * and the endpoint zone can't access the checkable. + * The endpoints are checked to allow for the case where command_endpoint is specified in the checkable + * but checkable is not actually present in the agent. + */ Zone::Ptr endpointZone = endpointPtr->GetZone(); - if (!endpointZone->CanAccessObject(checkable)) { + Endpoint::Ptr commandEndpoint = checkable->GetCommandEndpoint(); + if (endpointPtr != commandEndpoint && !endpointZone->CanAccessObject(checkable)) { return ApiActions::CreateResult( 409, "Zone '" + endpointZone->GetName() + "' cannot access checkable '" + checkable->GetName() + "'." diff --git a/lib/icinga/clusterevents.cpp b/lib/icinga/clusterevents.cpp index d1e58c55af0..b76ed492305 100644 --- a/lib/icinga/clusterevents.cpp +++ b/lib/icinga/clusterevents.cpp @@ -817,7 +817,7 @@ Value ClusterEvents::ExecuteCommandAPIHandler(const MessageOrigin::Ptr& origin, Host::Ptr host = Host::GetByName(params->Get("host")); if (!host) { Log(LogWarning, "ClusterEvents") - << "Discarding 'execute command' message " << executionUuid + << "Discarding 'execute command' message " << executionUuid << ": host " << params->Get("host") << " does not exist"; return Empty; } @@ -833,12 +833,16 @@ Value ClusterEvents::ExecuteCommandAPIHandler(const MessageOrigin::Ptr& origin, checkableName += "!" + params->Get("service"); Log(LogWarning, "ClusterEvents") - << "Discarding 'execute command' message " << executionUuid + << "Discarding 'execute command' message " << executionUuid << ": " << checkableName << " does not exist"; return Empty; } - /* Check if the child zone can access the checkable, and if it's the same endpoint zone */ + /* Return an error when the endpointZone is different than the child zone and + * the child zone can't access the checkable. + * The zones are checked to allow for the case where command_endpoint is specified in the checkable + * but checkable is not actually present in the agent. + */ if (!zone->CanAccessObject(checkable) && zone != endpointZone) { double now = Utility::GetTime(); Dictionary::Ptr executedParams = new Dictionary(); @@ -1276,7 +1280,7 @@ Value ClusterEvents::ExecutedCommandAPIHandler(const MessageOrigin::Ptr& origin, return Empty; } - if (origin->FromZone && !origin->FromZone->CanAccessObject(command_endpoint->GetZone())) { + if (origin->FromZone && !command_endpoint->GetZone()->IsChildOf(origin->FromZone)) { Log(LogNotice, "ClusterEvents") << "Discarding 'update executions API handler' message for checkable '" << checkable->GetName() << "' from '" << origin->FromClient->GetIdentity() << "': Unauthorized access.";