From c31582b8574cf3b0a245f5e44fe3d9111c711e9f Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Thu, 22 Aug 2024 11:34:13 -0400 Subject: [PATCH 1/3] HPCC-32465 Add ESP support for trace level - Define an ESP process configuration node that supports specification of global TraceFlags values for each ESP. - Reserve traceLevel to request a specific trace level (0: none, 1: standard, 2: detailed, 3: max). This replaces acting on the last observed occurrence of either traceNone, traceStandard, traceDetailed, and traceMax. - Override default flag settings when not configured and a debug build. Signed-off-by: Tim Klemm --- esp/platform/application_config.cpp | 15 +++++ esp/platform/espp.cpp | 90 +++++++++++++++++++++++++++++ esp/platform/esptrace.h | 73 +++++++++++++++++++++++ helm/examples/esp/README.md | 62 ++++++++++++++++++++ 4 files changed, 240 insertions(+) create mode 100644 esp/platform/esptrace.h create mode 100644 helm/examples/esp/README.md diff --git a/esp/platform/application_config.cpp b/esp/platform/application_config.cpp index fa4228651a8..f8a466e8d1c 100644 --- a/esp/platform/application_config.cpp +++ b/esp/platform/application_config.cpp @@ -27,6 +27,7 @@ #include "espcfg.ipp" #include "esplog.hpp" #include "espcontext.hpp" +#include "esptrace.h" enum class LdapType { LegacyAD, AzureAD }; @@ -436,6 +437,18 @@ void setLDAPSecurityInWSAccess(IPropertyTree *legacyEsp, IPropertyTree *legacyLd } } +// Copy trace flags from appEsp to legacyEsp. The source is expected to be an application's `esp` +// configuration object. The destination is expected to be the `EspProcess` element. +void copyTraceFlags(IPropertyTree *legacyEsp, IPropertyTree *appEsp) +{ + IPropertyTree *traceFlags = appEsp->queryPropTree(propTraceFlags); + if (traceFlags) + { + IPropertyTree *legacyTraceFlags = legacyEsp->addPropTree(propTraceFlags); + copyAttributes(legacyTraceFlags, traceFlags); + } +} + IPropertyTree *buildApplicationLegacyConfig(const char *application, const char* argv[]) { Owned appEspConfig = loadApplicationConfig(application, argv); @@ -468,5 +481,7 @@ IPropertyTree *buildApplicationLegacyConfig(const char *application, const char* IPropertyTree *legacyDirectories = legacy->queryPropTree("Software/Directories"); IPropertyTree *appDirectories = appEspConfig->queryPropTree("directories"); copyDirectories(legacyDirectories, appDirectories); + + copyTraceFlags(legacyEsp, appEspConfig); return legacy.getClear(); } diff --git a/esp/platform/espp.cpp b/esp/platform/espp.cpp index 20d3898d1f3..466062878a7 100644 --- a/esp/platform/espp.cpp +++ b/esp/platform/espp.cpp @@ -49,6 +49,10 @@ #include "jmetrics.hpp" #include "workunit.hpp" +#include "esptrace.h" +#include "tokenserialization.hpp" + +static TokenDeserializer deserializer; using namespace hpccMetrics; @@ -354,6 +358,91 @@ static void usage() IPropertyTree *buildApplicationLegacyConfig(const char *application, const char* argv[]); +// Modified version of jlib's loadTraceFlags. The modification adds special handling for traceLevel. +// +// Using loadTraceFlags, `traceStandard: true`, `traceStandard: false`, and `traceStandard: maybe` +// are non-intuitively equivalent. The property name is the only thing that matters, with the +// value being ignored. +// +// Also using loadTraceFlags, all defined trace levels may be included in one configuration. The +// final, accepted, level is determined by the order of `optNames` list items. An order optimized +// for elevating the level cannot be used in reverse. While a configuration with multiple properties +// should be discouraged, using `helm install --set` to change the level will add a new property +// rather than changing the existing one. +// +// With loadEspTraceFlags, `traceLevel` is the only property that sets the trace level. The value +// is both relevant and intuitive, and can be overridden with `helm install --set`. +static TraceFlags loadEspTraceFlags(const IPropertyTree *ptree, const std::initializer_list &optNames, TraceFlags dft) +{ + for (const TraceOption& option: optNames) + { + VStringBuffer attrName("@%s", option.name); + const char* value = nullptr; + if (!(value = ptree->queryProp(attrName))) + { + attrName.setCharAt(0, '_'); + if (!(value = ptree->queryProp(attrName))) + continue; + } + if (streq(value, "default")) // allow a configuration to explicitly request a default value + continue; + if (streq(option.name, propTraceLevel)) // non-Boolean traceLevel + { + unsigned level = unsigned(dft & TraceFlags::LevelMask); + dft &= ~TraceFlags::LevelMask; + if (streq(value, "traceStandard")) + dft |= traceStandard; + else if (streq(value, "traceDetailed")) + dft |= traceDetailed; + else if (streq(value, "traceMax")) + dft |= traceMax; + else if (deserializer(value, level) == Deserialization_SUCCESS) + dft |= TraceFlags(level) & TraceFlags::LevelMask; + else + dft |= TraceFlags(level); + } + else if (option.value <= TraceFlags::LevelMask) // block individual trace level names + continue; + else // Boolean trace options + { + bool flag = false; + deserializer(value, flag); + if (flag) + dft |= option.value; + else + dft &= ~option.value; + } + } + return dft; +} + +// +// Initialize trace settings +void initializeTraceFlags(CEspConfig* config) +{ + IPropertyTree* pEspTree = config->queryConfigPTree(); + Owned pTraceTree = pEspTree->getPropTree(propTraceFlags); + if (!pTraceTree) + { + pTraceTree.setown(getComponentConfigSP()->getPropTree(propTraceFlags)); + } +#ifdef _DEBUG + if (!pTraceTree) + { + static const char * defaultTraceYaml = R"!!( +traceLevel: traceMax +)!!"; + pTraceTree.setown(createPTreeFromYAMLString(defaultTraceYaml)); + } +#endif + + if (pTraceTree) + { + TraceFlags defaults = loadEspTraceFlags(pTraceTree, mapTraceOptions(pEspTree), queryDefaultTraceFlags()); + updateTraceFlags(defaults, true); + } +} + // // Initialize metrics void initializeMetrics(CEspConfig* config) @@ -577,6 +666,7 @@ int init_main(int argc, const char* argv[]) config->bindServer(*server.get(), *server.get()); config->checkESPCache(*server.get()); + initializeTraceFlags(config); initializeMetrics(config); initializeStorageGroups(daliClientActive()); } diff --git a/esp/platform/esptrace.h b/esp/platform/esptrace.h new file mode 100644 index 00000000000..0b2780ce8b0 --- /dev/null +++ b/esp/platform/esptrace.h @@ -0,0 +1,73 @@ +/*############################################################################## + + HPCC SYSTEMS software Copyright (C) 2024 HPCC Systems®. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +############################################################################## */ + +#pragma once + +#include "jtrace.hpp" +#include + +// Refer to helm/examples/esp/README.md for more information on the trace options. + +constexpr const char* propTraceFlags = "traceFlags"; +constexpr const char* propTraceLevel = "traceLevel"; + +// ESP process names for use with options mapping +constexpr const char* eclwatchApplication = "eclwatch"; +constexpr const char* eclservicesApplication = "eclservices"; +constexpr const char* eclqueriesApplication = "eclqueires"; +constexpr const char* esdlsandboxApplication = "esdl-sandbox"; +constexpr const char* esdlApplication = "esdl"; +constexpr const char* sql2eclApplication = "sql2ecl"; +constexpr const char* dfsApplication = "dfs"; +constexpr const char* ldapenvironmentApplication = "ldapenvironment"; +constexpr const char* loggingserviceApplication = "loggingservice"; + +// Trace options for ESP +constexpr TraceFlags traceLevel = TraceFlags::LevelMask; + +// Trace option list fragment for jtrace-defined options used by ESPs +#define PLATFORM_OPTIONS_FRAGMENT + +// Trace option list fragment for options used by most ESPs +#define ESP_OPTIONS_FRAGMENT \ + PLATFORM_OPTIONS_FRAGMENT \ + TRACEOPT(traceLevel), + +// Trace option initializer list for ESPs that do not define their own options. +constexpr std::initializer_list espTraceOptions +{ + ESP_OPTIONS_FRAGMENT +}; + +/** + * @brief Returns the trace options appropriate for the ESP process being initialized. + * + * Most ESPs will simply return espTraceOptions. Any ESP that defines options not applicable to + * other ESPs would return a different list. The determination of which list to return is + * expected to be based on the configuration's `application` property. + * + * If options for all ESPs are defined with no value collisions, there may be no need to define + * separate option lists for individual ESPs. However, if value collisions cannot be avoided, + * separate lists will be needed. + * + * Consider ESDL Script, and the applications that use it. The potential for a significant number + * of options is high, increasing the likelihood of collisions with applications that don't use it. + */ +inline const std::initializer_list& mapTraceOptions(const IPTree* config) +{ + return espTraceOptions; +} diff --git a/helm/examples/esp/README.md b/helm/examples/esp/README.md new file mode 100644 index 00000000000..05bb4bc5994 --- /dev/null +++ b/helm/examples/esp/README.md @@ -0,0 +1,62 @@ +# ESP Trace Flags + +Each ESP process may specify its own set of flags for controlling trace behavior. The specific flags may be shared with other platform components, shared amongst ESPs, or unique to an ESP. + +## Accepted Flags + +Detailed description of flags used by any ESP. + +### Shared Platform Flags + +Flags defined in `system/jlib/jtrace.hpp` and used by multiple platform processes. + +Flags will be added to this list as tracing logic is updated in ESP code. For example, the shared platform flag `traceHttp` is expected to be used, as are a number of ESP-specific options. + +### Shared ESP Flags + +Flags defined in `esp/platform/esptrace.h` and applicable to most, if not all, ESP configurations. + +#### traceLevel + +Set the defailt trace level in the process. Accepted values are: +- 1, traceStandard: most important output +- 2, traceDetailed: average verbosity output +- 3, traceMax: highest verbosity output +- default: use existing level + - Release builds default to traceStandard + - Debug builds default to traceMax +- 0, traceNone, *all other values*: no trace output + +## Process Configuration + +### Containerized + +#### esp.yaml + +Each ESP application's configuration object may embed one instance of a `traceFlags` object. Within this object, at most one property per [available flag]]#availableflags] is expected. Properties not described here are ignored. + +For example, the `eclwatch` process might be configured to use detailed reporting like this: + +```yml +esp: +- name: eclwatch + traceFlags: + traceLevel: 2 +``` + +## Cluster Overrides + +A values file may be used with the `helm install` command to override the settings of all ESPs. The `--set` option may be used to target the settings of a specific ESP in the configured array. + +### Bare-Metal + +No support for defining trace flags is provided by the `configmgr` application. Within a stand-alone `esp.xml` file, however, a `traceFlags` child of the `EspProcess` element may be defined. + +The previous YAML example may be reproduced in XML with the following: + +```xml + + + ... + +``` From 3a034a6bccf9e5773271bee2aeafb8fb7853eb48 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Thu, 22 Aug 2024 11:34:13 -0400 Subject: [PATCH 2/3] HPCC-32452 Add conditional tracing for ESP security manager authorization calls. - Define a new, common, trace option for security manager tracing. Both ESP and Roxie use security managers, and the flag can be shared by both. - Refactor CEspHttpServer to create server spans before initial authorization requests. - Create a security manager decorator encapsulating the logic of what to trace and when. - Replace security manager authorization calls with decorated calls. This change excludes calls specific to the LDAP security manager. Signed-off-by: Tim Klemm --- common/workunit/workunit.cpp | 9 +- esp/bindings/http/platform/httpbinding.cpp | 6 +- esp/bindings/http/platform/httpservice.cpp | 35 +- esp/platform/esptrace.h | 3 +- esp/platform/sechandler.cpp | 5 +- esp/services/ws_dfu/ws_dfuService.cpp | 3 +- helm/examples/esp/README.md | 7 +- system/jlib/jtrace.hpp | 1 + .../shared/secmanagertracedecorator.hpp | 696 ++++++++++++++++++ 9 files changed, 748 insertions(+), 17 deletions(-) create mode 100644 system/security/shared/secmanagertracedecorator.hpp diff --git a/common/workunit/workunit.cpp b/common/workunit/workunit.cpp index 78b2a1c864b..b02ecde06c1 100644 --- a/common/workunit/workunit.cpp +++ b/common/workunit/workunit.cpp @@ -59,6 +59,7 @@ #include "daqueue.hpp" #include "workunit.ipp" #include "digisign.hpp" +#include "secmanagertracedecorator.hpp" #include #include @@ -113,7 +114,7 @@ static bool checkWuScopeSecAccess(const char *wuscope, ISecManager *secmgr, ISec { if (!secmgr || !secuser) return true; - bool ret = secmgr->authorizeEx(RT_WORKUNIT_SCOPE, *secuser, wuscope)>=required; + bool ret = CSecManagerTraceDecorator(*secmgr).authorizeEx(RT_WORKUNIT_SCOPE, *secuser, wuscope)>=required; if (!ret && (log || excpt)) wuAccessError(secuser->getName(), action, wuscope, NULL, excpt, log); return ret; @@ -146,7 +147,7 @@ static bool checkWuSecAccess(IConstWorkUnit &cw, ISecManager *secmgr, ISecUser * { if (!secmgr || !secuser) return true; - bool ret=secmgr->authorizeEx(RT_WORKUNIT_SCOPE, *secuser, cw.queryWuScope())>=required; + bool ret=CSecManagerTraceDecorator(*secmgr).authorizeEx(RT_WORKUNIT_SCOPE, *secuser, cw.queryWuScope())>=required; if (!ret && (log || excpt)) { wuAccessError(secuser->getName(), action, cw.queryWuScope(), cw.queryWuid(), excpt, log); @@ -159,7 +160,7 @@ static bool checkWuSecAccess(const char *wuid, ISecManager *secmgr, ISecUser *se return true; Owned factory = getWorkUnitFactory(); Owned cw = factory->openWorkUnit(wuid); - bool ret=secmgr->authorizeEx(RT_WORKUNIT_SCOPE, *secuser, cw->queryWuScope())>=required; + bool ret=CSecManagerTraceDecorator(*secmgr).authorizeEx(RT_WORKUNIT_SCOPE, *secuser, cw->queryWuScope())>=required; if (!ret && (log || excpt)) { wuAccessError(secuser->getName(), action, cw->queryWuScope(), cw->queryWuid(), excpt, log); @@ -5219,7 +5220,7 @@ class CSecureConstWUIterator : public CInterfaceOf SecAccessFlags perm; if (!perms) { - perm = secuser.get() ? secmgr->authorizeWorkunitScope(*secuser, scopeName) : SecAccess_Unavailable; + perm = secuser.get() ? CSecManagerTraceDecorator(*secmgr).authorizeWorkunitScope(*secuser, scopeName) : SecAccess_Unavailable; scopePermissions.setValue(scopeName, perm); } else diff --git a/esp/bindings/http/platform/httpbinding.cpp b/esp/bindings/http/platform/httpbinding.cpp index c5a9e831844..a32b7d8fc0d 100644 --- a/esp/bindings/http/platform/httpbinding.cpp +++ b/esp/bindings/http/platform/httpbinding.cpp @@ -49,7 +49,7 @@ #include #include "esdl_def_helper.hpp" - +#include "secmanagertracedecorator.hpp" #define FILE_UPLOAD "FileUploadAccess" #define DEFAULT_HTTP_PORT 80 @@ -972,7 +972,7 @@ bool EspHttpBinding::basicAuth(IEspContext* ctx) return false; } - bool authenticated = m_secmgr->authorize(*user, rlist, ctx->querySecureContext()); + bool authenticated = CSecManagerTraceDecorator(*m_secmgr).authorize(*user, rlist, ctx->querySecureContext()); if(!authenticated) { VStringBuffer err("User %s : ", user->getName()); @@ -1029,7 +1029,7 @@ bool EspHttpBinding::basicAuth(IEspContext* ctx) if(securitySettings == NULL) return authorized; - m_secmgr->updateSettings(*user,securitySettings, ctx->querySecureContext()); + CSecManagerTraceDecorator(*m_secmgr).updateSettings(*user,securitySettings, ctx->querySecureContext()); ctx->addTraceSummaryTimeStamp(LogMin, "basicAuth", TXSUMMARY_GRP_CORE); return authorized; diff --git a/esp/bindings/http/platform/httpservice.cpp b/esp/bindings/http/platform/httpservice.cpp index 4f3715dc2b9..1a59ea02edc 100644 --- a/esp/bindings/http/platform/httpservice.cpp +++ b/esp/bindings/http/platform/httpservice.cpp @@ -384,6 +384,36 @@ int CEspHttpServer::processRequest() else //user ID is in HTTP header ESPLOG(LogMin, "%s %s, from %s@%s", method.str(), m_request->getPath(pathStr).str(), userid, m_request->getPeer(peerStr).str()); + // The user auth check may create trace output, in which case case the root server span + // must be created first. Since the span was intentionally created after dispatching a + // variety of requests, span creation is conditional. + bool wantTracing = true; + if (!queryTraceManager().isTracingEnabled()) + wantTracing = false; + else if (streq(method, GET_METHOD)) + { + if (sub_serv_root == stype) + wantTracing = false; + else if (strieq(serviceName, "esp")) + { + wantTracing = !(methodName.isEmpty() || + strieq(methodName, "files") || + strieq(methodName, "xslt") || + strieq(methodName, "frame") || + strieq(methodName, "titlebar") || + strieq(methodName, "nav") || + strieq(methodName, "navdata") || + strieq(methodName, "navmenuevent") || + strieq(methodName, "soapreq")); + } + } + else if (!m_apport) + wantTracing = false; + //The context will be destroyed when this request is destroyed. So initialise a SpanScope in the context to + //ensure the span is also terminated at the same time. + Owned serverSpan = (wantTracing ? m_request->createServerSpan(serviceName, methodName) : getNullSpan()); + ctx->setRequestSpan(serverSpan); + authState = checkUserAuth(); if ((authState == authTaskDone) || (authState == authFailed)) return 0; @@ -500,11 +530,6 @@ int CEspHttpServer::processRequest() return 0; } - //The context will be destroyed when this request is destroyed. So initialise a SpanScope in the context to - //ensure the span is also terminated at the same time. - Owned serverSpan = m_request->createServerSpan(serviceName, methodName); - ctx->setRequestSpan(serverSpan); - if (thebinding!=NULL) { if(stricmp(method.str(), POST_METHOD)==0) diff --git a/esp/platform/esptrace.h b/esp/platform/esptrace.h index 0b2780ce8b0..f90b1449b9f 100644 --- a/esp/platform/esptrace.h +++ b/esp/platform/esptrace.h @@ -40,7 +40,8 @@ constexpr const char* loggingserviceApplication = "loggingservice"; constexpr TraceFlags traceLevel = TraceFlags::LevelMask; // Trace option list fragment for jtrace-defined options used by ESPs -#define PLATFORM_OPTIONS_FRAGMENT +#define PLATFORM_OPTIONS_FRAGMENT \ + TRACEOPT(traceSecMgr), // Trace option list fragment for options used by most ESPs #define ESP_OPTIONS_FRAGMENT \ diff --git a/esp/platform/sechandler.cpp b/esp/platform/sechandler.cpp index 9dcf48ec859..346adfeddeb 100644 --- a/esp/platform/sechandler.cpp +++ b/esp/platform/sechandler.cpp @@ -24,6 +24,7 @@ #include #include #include "espcontext.hpp" +#include "secmanagertracedecorator.hpp" ////////////////////////////////////////////////////////////////////// // Construction/Destruction ////////////////////////////////////////////////////////////////////// @@ -135,7 +136,7 @@ bool SecHandler::authorizeSecFeature(const char * pszFeatureUrl, const char* Use bool SecHandler::authorizeTrial(ISecUser& user,const char* pszFeatureUrl, SecAccessFlags & required_access) { - int trial_access = m_secmgr->authorizeEx(RT_TRIAL,user,pszFeatureUrl); + int trial_access = CSecManagerTraceDecorator(*m_secmgr).authorizeEx(RT_TRIAL,user,pszFeatureUrl); if(trial_access < required_access) throw MakeStringException(201,"Your company has used up all of their free transaction credits"); return true; @@ -266,7 +267,7 @@ bool SecHandler::authorizeSecReqFeatures(StringArray & features, IEspStringIntMa bool auth_ok = false; try { - auth_ok = m_secmgr->authorize(*m_user.get(), plist, m_secureContext.get()); + auth_ok = CSecManagerTraceDecorator(*m_secmgr).authorize(*m_user.get(), plist, m_secureContext.get()); } catch(IException* e) { diff --git a/esp/services/ws_dfu/ws_dfuService.cpp b/esp/services/ws_dfu/ws_dfuService.cpp index c4b3d83b220..edc86ab58df 100644 --- a/esp/services/ws_dfu/ws_dfuService.cpp +++ b/esp/services/ws_dfu/ws_dfuService.cpp @@ -65,6 +65,7 @@ #include "wsdfuaccess/wsdfuaccess.hpp" #include "ws_dfuService.hpp" +#include "secmanagertracedecorator.hpp" using namespace wsdfuaccess; using namespace cryptohelper; @@ -2020,7 +2021,7 @@ static void getFilePermission(CDfsLogicalFileName &dlfn, ISecUser & user, IUserD StringBuffer scopes; dlfn.getScopes(scopes); - permissionTemp = secmgr->authorizeFileScope(user, scopes.str()); + permissionTemp = CSecManagerTraceDecorator(*secmgr).authorizeFileScope(user, scopes.str()); } //Descrease the permission whenever a component has a lower permission. diff --git a/helm/examples/esp/README.md b/helm/examples/esp/README.md index 05bb4bc5994..fd4ca0ce1f9 100644 --- a/helm/examples/esp/README.md +++ b/helm/examples/esp/README.md @@ -10,7 +10,12 @@ Detailed description of flags used by any ESP. Flags defined in `system/jlib/jtrace.hpp` and used by multiple platform processes. -Flags will be added to this list as tracing logic is updated in ESP code. For example, the shared platform flag `traceHttp` is expected to be used, as are a number of ESP-specific options. +#### traceSecMgr + +Control security manager trace output. The ESP enables or suppresses timing span creation based on this flag. Security managers which generate significant amounts of trace output are encouraged to observe this setting. + +- true: record security manager-related trace output +- false, *omitted*: suppress security manager-related trace output ### Shared ESP Flags diff --git a/system/jlib/jtrace.hpp b/system/jlib/jtrace.hpp index 7f1e7d9431b..62bf95bdb2f 100644 --- a/system/jlib/jtrace.hpp +++ b/system/jlib/jtrace.hpp @@ -337,6 +337,7 @@ constexpr TraceFlags traceFilters = TraceFlags::flag6; constexpr TraceFlags traceKafka = TraceFlags::flag7; constexpr TraceFlags traceJava = TraceFlags::flag8; constexpr TraceFlags traceOptimizations = TraceFlags::flag9; // code generator, but IHqlExpressions also used by esp/engines +constexpr TraceFlags traceSecMgr = TraceFlags::flag10; // esp and dali // Specific to Roxie constexpr TraceFlags traceRoxieLock = TraceFlags::flag16; diff --git a/system/security/shared/secmanagertracedecorator.hpp b/system/security/shared/secmanagertracedecorator.hpp new file mode 100644 index 00000000000..4699f66a0f5 --- /dev/null +++ b/system/security/shared/secmanagertracedecorator.hpp @@ -0,0 +1,696 @@ +/*############################################################################## + + HPCC SYSTEMS software Copyright (C) 2024 HPCC Systems®. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +############################################################################## */ + +#pragma once + +#include "seclib.hpp" +#include "jscm.hpp" +#include "jthread.hpp" +#include "jtrace.hpp" + +/** + * @brief Templated utility class to decorate an object with additional functionality. + * + * A decorator implements the same named interface as the objects it decorates. This class extends + * the interface, leaving implementation to subclasses. This class requires the decorated object to + * implement a named interface, even if only IInterface. + * + * In the ideal situation, the decorated object's interface is described completely by a named + * interface. By implementing the same interface, a decorator is interchangeable with the object + * it decoratos. + * + * In less than ideal situations, the decorated object's interface is an extension of a named + * interface. The decorator extends the extends the named interface, with subclasses required to + * implement both the named interface and all extensions. The decorator should then be + * interchangeable with its decorated object as a templated argument, but not be cast to the + * decorated type. + * + * The less ideal scenario is suported by two template parameters. The ideal situation requires + * only the first. + * - decorated_t is the type of the object to be decorated. If the the decorated object conforms + * to an interface, use of the interface is preferred. + * - secorated_interface_t is the interface implemented by the decorated object. If not the same + * as decorated_t, it is assumed to be a base of that type. + * + * Consider the example of ISecManager, generally, and the special case of CLdapSecManager. For + * most security managers, both template parameters may be ISecManager. CLdapSecManager is an + * exception because it exposes additional interfaces not included in ISecManager or any other + * interface. In this case, decorated_t should be CLdapSecManager and decorated_interface_t should + * be ISecManager. + */ +template +class TDecorator : public CInterfaceOf +{ +protected: + Linked decorated; + +public: + TDecorator(decorated_t &_decorated) : decorated(&_decorated) {} + virtual ~TDecorator() {} + decorated_t *queryDecorated() { return decorated.get(); } + decorated_t *getDecorated() { return decorated.getLink(); } +}; + +/** + * @brief Macro used start tracing a block of code in the security manager decorator. + * + * Create a new named internal span and enter a try block. Used with END_SEC_MANAGER_TRACE_BLOCK, + * provides consistent timing and exception handling for the inned code block. + * + * Some security manager requests include outgoing remote calls, but the ESP does not assume + * which requests are remote. Managers making remote calls may considier creating client spans + * as needed. + */ +#define START_SEC_MANAGER_TRACE_BLOCK(name) \ + OwnedSpanScope spanScope(queryThreadedActiveSpan()->createInternalSpan(name)); \ + try \ + { + +/** + * @brief Macro used to end tracing a block of code in the security manager decorator. + * + * Ends a try block and defines standard exception handling. Use with START_SEC_MANAGER_TRACE_BLOCK, + * provides consistent timing and exception handling for the timed code block. + */ +#define END_SEC_MANAGER_TRACE_BLOCK \ + } \ + catch (IException * e) \ + { \ + spanScope->recordException(e); \ + throw; \ + } \ + catch (...) \ + { \ + Owned e(makeStringException(-1, "unknown exception")); \ + spanScope->recordException(e); \ + throw; \ + } + +/** + * @brief Decorator for ISecManager that adds tracing to the interface. + * + * Tracing is added to selected methods of ISecManager by calling the decorated manager's method + * within the scope of a new internal span. Tracing is conditional to reduce overhead: + * - The `traceSecMgr` trace option must be enabled. Processes using security manager's that don't + * warrant tracing should disable the option. + * - The decorated manager must claim method implementation through feature flags. There is no + * mechanism by which the decorator can determine whether a decorated manager's implemented + * method warrants tracing. + * + * All methods not traced are passed through to the decorated manager instance. + * + * The default decorated type is sufficient for most security managers. CLdapSecManager is an + * exception because platform processes depend on interfaces not included in the default. + * - If ISecManager and CLdapSecManager interfaces are standardized, this point becomes moot. + * - If ISecManager is extended to declare LDAP-specific interfaces, and CLdapSecManager implements + * this interface, create a subclass of TSecManagerTraceDecorator to decorate the new interface + * and replace ISecManager with the new interface name as template parameters. + * - If nothing else changes, create a subclasses of TSecManagerTraceDecorator, with template + * parameters CLdapSecManager and ISecManager, to decorate the LDAP-specific interfaces. + */ +template +class TSecManagerTraceDecorator : public TDecorator +{ + using TDecorator::decorated; + +public: + virtual SecFeatureSet queryFeatures(SecFeatureSupportLevel level) const override + { + return decorated->queryFeatures(level); + } + virtual ISecUser *createUser(const char *user_name, IEspSecureContext *secureContext = nullptr) override + { + return decorated->createUser(user_name, secureContext); + } + virtual ISecResourceList *createResourceList(const char *rlname, IEspSecureContext *secureContext = nullptr) override + { + return decorated->createResourceList(rlname, secureContext); + } + virtual bool subscribe(ISecAuthenticEvents &events, IEspSecureContext *secureContext = nullptr) override + { + return decorated->subscribe(events, secureContext); + } + virtual bool unsubscribe(ISecAuthenticEvents &events, IEspSecureContext *secureContext = nullptr) override + { + return decorated->unsubscribe(events, secureContext); + } + virtual bool authorize(ISecUser &user, ISecResourceList *resources, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_Authorize)) + return decorated->authorize(user, resources, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.authorize"); + ResourceListTracer tracer(user, RT_DEFAULT, resources); + return tracer.authenticated = decorated->authorize(user, resources, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual bool authorizeEx(SecResourceType rtype, ISecUser &user, ISecResourceList *resources, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_AuthorizeEx_List)) + return decorated->authorizeEx(rtype, user, resources, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.authorize_ex.list"); + ResourceListTracer tracer(user, rtype, resources); + return tracer.authenticated = decorated->authorizeEx(rtype, user, resources, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual SecAccessFlags authorizeEx(SecResourceType rtype, ISecUser &user, const char *resourcename, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_AuthorizeEx_Named)) + return decorated->authorizeEx(rtype, user, resourcename, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.authorize_ex.named"); + ResourceTracer tracer(user, rtype, resourcename); + return tracer.accessFlags = decorated->authorizeEx(rtype, user, resourcename, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual SecAccessFlags getAccessFlagsEx(SecResourceType rtype, ISecUser &user, const char *resourcename, IEspSecureContext *secureContext = nullptr) override + { + return decorated->getAccessFlagsEx(rtype, user, resourcename, secureContext); + } + virtual SecAccessFlags authorizeFileScope(ISecUser &user, const char *filescope, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_AuthorizeFileScope_Named)) + return decorated->authorizeFileScope(user, filescope, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.authorize_file_scope.named"); + ResourceTracer tracer(user, RT_FILE_SCOPE, filescope); + return tracer.accessFlags = decorated->authorizeFileScope(user, filescope, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual bool authorizeFileScope(ISecUser &user, ISecResourceList *resources, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_AuthorizeFileScope_List)) + return decorated->authorizeFileScope(user, resources, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.authorize_file_scope.list"); + ResourceListTracer tracer(user, RT_FILE_SCOPE, resources); + return tracer.authenticated = decorated->authorizeFileScope(user, resources, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual bool addResources(ISecUser &user, ISecResourceList *resources, IEspSecureContext *secureContext = nullptr) override + { + return decorated->addResources(user, resources, secureContext); + } + virtual bool addResourcesEx(SecResourceType rtype, ISecUser &user, ISecResourceList *resources, SecPermissionType ptype, const char *basedn, IEspSecureContext *secureContext = nullptr) override + { + return decorated->addResourcesEx(rtype, user, resources, ptype, basedn, secureContext); + } + virtual bool addResourceEx(SecResourceType rtype, ISecUser &user, const char *resourcename, SecPermissionType ptype, const char *basedn, IEspSecureContext *secureContext = nullptr) override + { + return decorated->addResourceEx(rtype, user, resourcename, ptype, basedn, secureContext); + } + virtual bool getResources(SecResourceType rtype, const char *basedn, IResourceArray &resources, IEspSecureContext *secureContext = nullptr) override + { + return decorated->getResources(rtype, basedn, resources, secureContext); + } + virtual bool updateResources(ISecUser &user, ISecResourceList *resources, IEspSecureContext *secureContext = nullptr) override + { + return decorated->updateResources(user, resources, secureContext); + } + virtual bool updateSettings(ISecUser &user, ISecPropertyList *resources, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_UpdateSettings)) + return decorated->updateSettings(user, resources, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.update_settings"); + SettingListTracer tracer(user, resources); + return tracer.authenticated = decorated->updateSettings(user, resources, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual bool addUser(ISecUser &user, IEspSecureContext *secureContext = nullptr) override + { + return decorated->addUser(user, secureContext); + } + virtual ISecUser *findUser(const char *username, IEspSecureContext *secureContext = nullptr) override + { + return decorated->findUser(username, secureContext); + } + virtual ISecUser *lookupUser(unsigned uid, IEspSecureContext *secureContext = nullptr) override + { + return decorated->lookupUser(uid, secureContext); + } + virtual ISecUserIterator *getAllUsers(IEspSecureContext *secureContext = nullptr) override + { + return decorated->getAllUsers(secureContext); + } + virtual void getAllGroups(StringArray &groups, StringArray &managedBy, StringArray &descriptions, IEspSecureContext *secureContext = nullptr) override + { + decorated->getAllGroups(groups, managedBy, descriptions, secureContext); + } + virtual bool updateUserPassword(ISecUser &user, const char *newPassword, const char *currPassword = nullptr, IEspSecureContext *secureContext = nullptr) override + { + return decorated->updateUserPassword(user, newPassword, currPassword, secureContext); + } + virtual bool initUser(ISecUser &user, IEspSecureContext *secureContext = nullptr) override + { + return decorated->initUser(user, secureContext); + } + virtual void setExtraParam(const char *name, const char *value, IEspSecureContext *secureContext = nullptr) override + { + decorated->setExtraParam(name, value, secureContext); + } + virtual IAuthMap *createAuthMap(IPropertyTree *authconfig, IEspSecureContext *secureContext = nullptr) override + { + return decorated->createAuthMap(authconfig, secureContext); + } + virtual IAuthMap *createFeatureMap(IPropertyTree *authconfig, IEspSecureContext *secureContext = nullptr) override + { + return decorated->createFeatureMap(authconfig, secureContext); + } + virtual IAuthMap *createSettingMap(IPropertyTree *authconfig, IEspSecureContext *secureContext = nullptr) override + { + return decorated->createSettingMap(authconfig, secureContext); + } + virtual void deleteResource(SecResourceType rtype, const char *name, const char *basedn, IEspSecureContext *secureContext = nullptr) override + { + decorated->deleteResource(rtype, name, basedn, secureContext); + } + virtual void renameResource(SecResourceType rtype, const char *oldname, const char *newname, const char *basedn, IEspSecureContext *secureContext = nullptr) override + { + decorated->renameResource(rtype, oldname, newname, basedn, secureContext); + } + virtual void copyResource(SecResourceType rtype, const char *oldname, const char *newname, const char *basedn, IEspSecureContext *secureContext = nullptr) override + { + decorated->copyResource(rtype, oldname, newname, basedn, secureContext); + } + virtual void cacheSwitch(SecResourceType rtype, bool on, IEspSecureContext *secureContext = nullptr) override + { + decorated->cacheSwitch(rtype, on, secureContext); + } + virtual bool authTypeRequired(SecResourceType rtype, IEspSecureContext *secureContext = nullptr) override + { + return decorated->authTypeRequired(rtype, secureContext); + } + virtual SecAccessFlags authorizeWorkunitScope(ISecUser &user, const char *wuscope, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_AuthorizeWorkUnitScope_Named)) + return decorated->authorizeWorkunitScope(user, wuscope, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.authorize_work_unit_scope.named"); + ResourceTracer tracer(user, RT_WORKUNIT_SCOPE, wuscope); + return tracer.accessFlags = decorated->authorizeWorkunitScope(user, wuscope, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual bool authorizeWorkunitScope(ISecUser &user, ISecResourceList *resources, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_AuthorizeWorkUnitScope_List)) + return decorated->authorizeWorkunitScope(user, resources, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.authorize_work_unit_scope.list"); + ResourceListTracer tracer(user, RT_WORKUNIT_SCOPE, resources); + return tracer.authenticated = decorated->authorizeWorkunitScope(user, resources, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual const char *getDescription() override + { + return decorated->getDescription(); + } + virtual unsigned getPasswordExpirationWarningDays(IEspSecureContext *secureContext = nullptr) override + { + return decorated->getPasswordExpirationWarningDays(secureContext); + } + virtual aindex_t getManagedScopeTree(SecResourceType rtype, const char *basedn, IArrayOf &scopes, IEspSecureContext *secureContext = nullptr) override + { + return decorated->getManagedScopeTree(rtype, basedn, scopes, secureContext); + } + virtual SecAccessFlags queryDefaultPermission(ISecUser &user, IEspSecureContext *secureContext = nullptr) override + { + return decorated->queryDefaultPermission(user, secureContext); + } + virtual bool clearPermissionsCache(ISecUser &user, IEspSecureContext *secureContext = nullptr) override + { + return decorated->clearPermissionsCache(user, secureContext); + } + virtual bool authenticateUser(ISecUser &user, bool *superUser, IEspSecureContext *secureContext = nullptr) override + { + if (!doTraceFeature(SMF_AuthenticateUser)) + return decorated->authenticateUser(user, superUser, secureContext); + START_SEC_MANAGER_TRACE_BLOCK("security.authenticate_user"); + SuperUserTracer tracer(user, superUser); + return tracer.authenticated = decorated->authenticateUser(user, superUser, secureContext); + END_SEC_MANAGER_TRACE_BLOCK + } + virtual secManagerType querySecMgrType() override + { + return decorated->querySecMgrType(); + } + virtual const char *querySecMgrTypeName() override + { + return decorated->querySecMgrTypeName(); + } + virtual bool logoutUser(ISecUser &user, IEspSecureContext *secureContext = nullptr) override + { + return decorated->logoutUser(user, secureContext); + } + virtual bool retrieveUserData(ISecUser &requestedUser, ISecUser *requestingUser = nullptr, IEspSecureContext *secureContext = nullptr) override + { + return decorated->retrieveUserData(requestedUser, requestingUser, secureContext); + } + virtual bool removeResources(ISecUser &user, ISecResourceList *resources, IEspSecureContext *secureContext = nullptr) override + { + return decorated->removeResources(user, resources, secureContext); + } + +protected: + SecFeatureSet implemented = 0; /// The decorated manager instance's implemented feature set. + bool tracing = true; /// Flag indicating if tracing is enabled. + +public: + TSecManagerTraceDecorator(secmgr_t &_decorated) + : TDecorator(_decorated) + , implemented(_decorated.queryFeatures(SecFeatureSupportLevel::SFSL_Implemented)) + , tracing(queryTraceManager().isTracingEnabled() && doTrace(traceSecMgr)) + { + } + virtual ~TSecManagerTraceDecorator() + { + } + +protected: + /** + * @brief Produce trace output describing a secure user. + * + * - Name and authenicated status are recorded unconditionally. + * - User status is recorded when known. + * - User properties are record when the trace level is at least traceDetailed. + */ + struct UserTracer + { + ISecUser& user; + bool authenticated = false; + + UserTracer(ISecUser& _user) + : user(_user) + { + queryThreadedActiveSpan()->setSpanAttribute("user.name", user.getName()); + } + + ~UserTracer() + { + // Authorization and authentication requests with Boolean return values use the result + // to indicate successful user authentication. Managers are not required to set the user + // status to reflect this result. For brevity and clarity, express an affirmative + // Boolean outcome as an authenticated status regardless of the user's actual status. + authStatus as = user.getAuthenticateStatus(); + if (authenticated) + { + if (as != AS_AUTHENTICATED) + as = AS_AUTHENTICATED; + else + authenticated = false; + } + + ISpan* span = queryThreadedActiveSpan(); + span->setSpanAttribute("user.auth_status", map(as)); + if (user.getStatus() != SecUserStatus_Unknown) + span->setSpanAttribute("user.status", map(user.getStatus())); + if (doTrace(TraceFlags::Always, traceDetailed)) + traceProperties(span); + } + + inline const char* map(authStatus updated) const + { + switch (updated) + { + case AS_AUTHENTICATED: return "authenticated"; + case AS_UNKNOWN: return "unknown"; + case AS_UNEXPECTED_ERROR: return "unexpected_error"; + case AS_INVALID_CREDENTIALS: return "invalid_credentials"; + case AS_PASSWORD_EXPIRED: return "password_expired"; + case AS_PASSWORD_VALID_BUT_EXPIRED: return "password_valid_but_expired"; + case AS_ACCOUNT_DISABLED: return "account_disabled"; + case AS_ACCOUNT_EXPIRED: return "account_expired"; + case AS_ACCOUNT_LOCKED: return "account_locked"; + case AS_ACCOUNT_ROOT_ACCESS_DENIED: return "account_root_access_denied"; + default: return "unexpected_status"; + } + } + + inline const char* map(SecUserStatus updated) const + { + switch (updated) + { + case SecUserStatus_Inhouse: return "inhouse"; + case SecUserStatus_Active: return "active"; + case SecUserStatus_Exempt: return "exempt"; + case SecUserStatus_FreeTrial: return "free_trial"; + case SecUserStatus_csdemo: return "csdemo"; + case SecUserStatus_Rollover: return "rollover"; + case SecUserStatus_Suspended: return "suspended"; + case SecUserStatus_Terminated: return "terminated"; + case SecUserStatus_TrialExpired: return "trial_expired"; + case SecUserStatus_Status_Hold: return "status_hold"; + case SecUserStatus_Unknown: return "unknown"; + default: return "unexpected_status"; + } + } + + void traceProperties(ISpan* span) + { + StringBuffer key("user.property."); + size_t baseKeyLength = key.length(); + Owned props(user.getPropertyIterator()); + ForEach(*props) + { + const char* name = props->getPropKey(); + if (isEmptyString(name)) + continue; + const char* value = props->queryPropValue(); + if (isEmptyString(value)) + value = ""; + key.setLength(baseKeyLength); + getSnakeCase(key, name); + span->setSpanAttribute(key, value); + } + } + }; + + /** + * @brief Extends secure user tracing with super user status. + * + * `authenticateUser` introduces the concept of a *super user*. Super user status will be + * recorded when requested by the caller. + */ + struct SuperUserTracer : public UserTracer + { + bool *superUser; + + SuperUserTracer(ISecUser& _user, bool* _superUser) + : UserTracer(_user) + , superUser(_superUser) + { + } + + ~SuperUserTracer() + { + if (superUser) + queryThreadedActiveSpan()->setSpanAttribute("user.super_user", (*superUser ? "true" : "false")); + } + }; + + /** + * @brief Extends secure user tracing with utility methods for security resource tracing. + * + * Resources authorized by name are handled differently from resources from a list, yet share + * tracing requirements. This is a base class to provide shared functionality. + */ + struct BaseResourceTracer : public UserTracer + { + using UserTracer::UserTracer; + + StringBuffer& getKey(StringBuffer& key, SecResourceType type, const char* name) + { + key.set(map(type)).append('.'); + getSnakeCase(key, name); + return key; + } + + inline const char* map(SecResourceType type) const + { + switch(type) + { + case RT_DEFAULT: return "permission"; + case RT_MODULE: return "module"; + case RT_SERVICE: return "service"; + case RT_FILE_SCOPE: return "file_scope"; + case RT_WORKUNIT_SCOPE: return "Workunit_scope"; + case RT_TRIAL: return "trial"; + case RT_VIEW_SCOPE: return "view"; + default: return "unknown"; + } + } + + inline const char* map(SecAccessFlags updated) const + { + switch (updated) + { + case SecAccess_Full: return "full"; + case SecAccess_Read: return "read"; + case SecAccess_Write: return "write"; + case SecAccess_Access: return "access"; + case SecAccess_None: return "none"; + case SecAccess_Unavailable: return "unavailable"; + case SecAccess_Unknown: return "unknown"; + default: return "bad_access"; + } + } + }; + + /** + * @brief Extends secure user tracing with the access rights for a single named resource. + * + * User authentication status is implied by the access flags returned for the named resource. + * A value of `SecAccess_Unavailable` is used to indicated to failed authentication. All + * other values imply successful authentication. + */ + struct ResourceTracer : public BaseResourceTracer + { + StringBuffer key; + SecAccessFlags accessFlags = SecAccess_Unknown; + + ResourceTracer(ISecUser& _user, SecResourceType type, const char* name) + : BaseResourceTracer(_user) + { + BaseResourceTracer::getKey(key, type, name); + } + + ~ResourceTracer() + { + if (accessFlags != SecAccess_Unavailable) + UserTracer::authenticated = true; + queryThreadedActiveSpan()->setSpanAttribute(key, BaseResourceTracer::map(accessFlags)); + } + }; + + /** + * @brief Extends secure user tracing with security resource access rights for any number of + * resources. + * + * All resources with access flags updated by the decorated manager will be recorded if tracing + * is enabled. Unchanged resources will recorded when the trace level is elevated to (at least) + * traceDetailed. + */ + struct ResourceListTracer : public BaseResourceTracer + { + /** + * @brief Short-term cache of a resource and associated state information used to control + * the output of the tracer that created it. + */ + struct Resource + { + Linked resource; + SecResourceType type; + SecAccessFlags originalAccess; + + Resource(ISecResource& _resource, SecResourceType _type) + : resource(&_resource) + , type(RT_DEFAULT == _type ? _resource.getResourceType() : _type) + , originalAccess(_resource.getAccessFlags()) + { + } + }; + std::vector resources; + + ResourceListTracer(ISecUser& user, SecResourceType type, ISecResourceList* _resources) + : BaseResourceTracer(user) + { + if (_resources) + { + for (unsigned idx = 0, lmt = _resources->count(); idx < lmt; idx++) + { + ISecResource* resource = _resources->queryResource(idx); + if (!resource) + continue; + resources.emplace_back(*resource, type); + } + } + } + + ~ResourceListTracer() + { + ISpan* span = queryThreadedActiveSpan(); + StringBuffer key; + bool recordUnchanged = doTrace(TraceFlags::Always, traceDetailed); + for (const Resource& r : resources) + { + SecAccessFlags currentAccess = r.resource->getAccessFlags(); + if (r.originalAccess != currentAccess || currentAccess < SecAccess_None || recordUnchanged) + { + BaseResourceTracer::getKey(key, r.type, r.resource->getName()); + span->setSpanAttribute(key, BaseResourceTracer::map(currentAccess)); + } + } + } + }; + + /** + * @brief Extends secure user tracing with secure user settings. + * + * User setting names and values are recorded when the trace level is at least detailed. + */ + struct SettingListTracer : public UserTracer + { + ISecPropertyList* settings; + + SettingListTracer(ISecUser& user, ISecPropertyList* _settings) + : UserTracer(user) + , settings(_settings) + { + } + + ~SettingListTracer() + { + if (settings && doTrace(TraceFlags::Always, traceDetailed)) + { + ISpan* span = queryThreadedActiveSpan(); + StringBuffer key("setting."); + size_t baseKeyLen = key.length(); + Owned iter = settings->getPropertyItr(); + ForEach(*iter) + { + const char* name = iter->query().getName(); + if (isEmptyString(name)) + continue; + const char* value = iter->query().getValue(); + if (isEmptyString(value)) + value = ""; + key.setLength(baseKeyLen); + getSnakeCase(key, name); + span->setSpanAttribute(key, value); + } + } + } + }; + + /** + * @brief Determine if a method should be called directly or decorated with tracing. + * + * The determination is based on three factors, the trace manager state, the `traceSecMgr` + * trace option, and the method's implented feature state. This method should not be invoked + * for methods never presumed to warrant tracing as it makes no assumptions about the relevance + * of tracing an individual method. Each overridden method should make this determination for + * itself. + * + * @param feature The method feature flag to be tested. + * @return true if security manager tracing is enabled and the feature is implemented + * @return false if security manager tracing is disabled or the feature is not implemented + */ + inline bool doTraceFeature(SecFeatureBit feature) const + { + return (tracing && (implemented & feature) != 0); + } +}; + +/** + * @brief Decorate for all ISecManager interfaces. + * + * Decoration of methods implemented by extensions to ISecManager requires a different class. + */ +using CSecManagerTraceDecorator = TSecManagerTraceDecorator<>; From f23b4e475fc14da660309500aba64e6ae72816c7 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Thu, 17 Oct 2024 13:00:39 -0400 Subject: [PATCH 3/3] Review comments Introduce a persistent decorator instance to the HTTP binding, ESP context, and security handler to avoid repeated construction and destruction. Improve the binding's handling of security manager creation failures to avoid null dereferences when creating the decorator, or resource auth maps. --- esp/bindings/bind_ng.hpp | 17 ++++- esp/bindings/http/platform/httpbinding.cpp | 13 +++- esp/bindings/http/platform/httpbinding.hpp | 2 + esp/platform/esp.hpp | 2 + esp/platform/espcontext.cpp | 20 +++++- esp/platform/sechandler.cpp | 15 ++++- esp/platform/sechandler.hpp | 2 + esp/services/ws_dfu/ws_dfuService.cpp | 4 +- helm/examples/esp/README.md | 3 +- .../shared/secmanagertracedecorator.hpp | 64 ++----------------- 10 files changed, 75 insertions(+), 67 deletions(-) diff --git a/esp/bindings/bind_ng.hpp b/esp/bindings/bind_ng.hpp index 3ff887f6c59..addbed50680 100644 --- a/esp/bindings/bind_ng.hpp +++ b/esp/bindings/bind_ng.hpp @@ -22,6 +22,7 @@ #include "jliball.hpp" #include "esp.hpp" #include "soapesp.hpp" +#include "secmanagertracedecorator.hpp" class CEspNgContext : implements IEspContext, public CInterface { @@ -31,6 +32,7 @@ class CEspNgContext : implements IEspContext, public CInterface Owned secuser; Owned secmgr; + Owned tracingSecMgrDecorator; Owned reslist; Owned authMap; Owned secprops; @@ -78,7 +80,20 @@ class CEspNgContext : implements IEspContext, public CInterface } - void setSecManger(ISecManager * mgr){secmgr.setown(mgr);} + void setSecManger(ISecManager * mgr) + { + setSecManager(mgr, nullptr); + } + virtual void setSecManager(ISecManager* mgr, ISecManager * _tracingSecMgrDecorator) + { + secmgr.setown(mgr); + if (!mgr) + tracingSecMgrDecorator.clear(); + else if (!_tracingSecMgrDecorator) + tracingSecMgrDecorator.setown(new CSecManagerTraceDecorator(*mgr)); + else + tracingSecMgrDecorator.set(_tracingSecMgrDecorator); + } ISecManager * querySecManager(){return secmgr.get();} void setContextPath(const char * path){contextPath.clear().append(path);} diff --git a/esp/bindings/http/platform/httpbinding.cpp b/esp/bindings/http/platform/httpbinding.cpp index a32b7d8fc0d..fbc242b636c 100644 --- a/esp/bindings/http/platform/httpbinding.cpp +++ b/esp/bindings/http/platform/httpbinding.cpp @@ -406,6 +406,9 @@ EspHttpBinding::EspHttpBinding(IPropertyTree* tree, const char *bindname, const //This is a Pluggable Security Manager setBndCfgServiceType(tree, procname, bnd_cfg); m_secmgr.setown(SecLoader::loadPluggableSecManager(bindname, bnd_cfg, secMgrCfg)); + if (!m_secmgr) + throw makeStringException(-1, "error loading pluggable security manager"); + m_tracingSecMgrDecorator.setown(new CSecManagerTraceDecorator(*m_secmgr)); m_authmap.setown(m_secmgr->createAuthMap(authcfg)); m_feature_authmap.setown(m_secmgr->createFeatureMap(authcfg)); m_setting_authmap.setown(m_secmgr->createSettingMap(authcfg)); @@ -434,6 +437,7 @@ EspHttpBinding::EspHttpBinding(IPropertyTree* tree, const char *bindname, const { throw MakeStringException(-1, "error generating SecManager"); } + m_tracingSecMgrDecorator.setown(new CSecManagerTraceDecorator(*m_secmgr));; StringBuffer basednbuf; authcfg->getProp("@resourcesBasedn", basednbuf); @@ -449,6 +453,9 @@ EspHttpBinding::EspHttpBinding(IPropertyTree* tree, const char *bindname, const else if(stricmp(m_authmethod.str(), "Local") == 0) { m_secmgr.setown(SecLoader::loadSecManager("Local", "EspHttpBinding", NULL)); + if (m_secmgr.get() == NULL) + throw MakeStringException(-1, "error loading local security manager"); + m_tracingSecMgrDecorator.setown(new CSecManagerTraceDecorator(*m_secmgr)); m_authmap.setown(m_secmgr->createAuthMap(authcfg)); } IRestartManager* restartManager = dynamic_cast(m_secmgr.get()); @@ -847,7 +854,7 @@ void EspHttpBinding::populateRequest(CHttpRequest *request) { IEspContext* ctx = request->queryContext(); - ctx->setSecManger(m_secmgr.getLink()); + ctx->setSecManager(m_secmgr.getLink(), m_tracingSecMgrDecorator.get()); ctx->setFeatureAuthMap(m_feature_authmap.getLink()); StringBuffer userid, password,realm,peer; @@ -972,7 +979,7 @@ bool EspHttpBinding::basicAuth(IEspContext* ctx) return false; } - bool authenticated = CSecManagerTraceDecorator(*m_secmgr).authorize(*user, rlist, ctx->querySecureContext()); + bool authenticated = m_tracingSecMgrDecorator->authorize(*user, rlist, ctx->querySecureContext()); if(!authenticated) { VStringBuffer err("User %s : ", user->getName()); @@ -1029,7 +1036,7 @@ bool EspHttpBinding::basicAuth(IEspContext* ctx) if(securitySettings == NULL) return authorized; - CSecManagerTraceDecorator(*m_secmgr).updateSettings(*user,securitySettings, ctx->querySecureContext()); + m_tracingSecMgrDecorator->updateSettings(*user,securitySettings, ctx->querySecureContext()); ctx->addTraceSummaryTimeStamp(LogMin, "basicAuth", TXSUMMARY_GRP_CORE); return authorized; diff --git a/esp/bindings/http/platform/httpbinding.hpp b/esp/bindings/http/platform/httpbinding.hpp index fa94e6146df..65f3137d0f4 100644 --- a/esp/bindings/http/platform/httpbinding.hpp +++ b/esp/bindings/http/platform/httpbinding.hpp @@ -153,6 +153,7 @@ class esp_http_decl EspHttpBinding : StringBuffer m_filespath; StringBuffer m_wsdlAddress; Owned m_secmgr; + Owned m_tracingSecMgrDecorator; Owned m_authmap; Owned m_feature_authmap; Owned m_setting_authmap; @@ -372,6 +373,7 @@ class esp_http_decl EspHttpBinding : return false; } ISecManager* querySecManager() const { return m_secmgr.get(); } + ISecManager* queryTracingSecManager() const { return m_tracingSecMgrDecorator.get(); } IAuthMap* queryAuthMAP() const { return m_authmap.get();} const char* queryAuthMethod() const { return m_authmethod.str(); } void setProcessName(const char* name) { processName.set(name); } diff --git a/esp/platform/esp.hpp b/esp/platform/esp.hpp index 2dd9f550fba..8bad8998c92 100644 --- a/esp/platform/esp.hpp +++ b/esp/platform/esp.hpp @@ -125,7 +125,9 @@ interface IEspContext : extends IInterface virtual void setResources(ISecResourceList * rlist) = 0; virtual ISecResourceList * queryResources() = 0; virtual void setSecManger(ISecManager * mgr) = 0; + virtual void setSecManager(ISecManager * mgr, ISecManager * tracingSecMgrDecoratorDecorator) = 0; virtual ISecManager * querySecManager() = 0; + virtual ISecManager * queryTracingSecManager() = 0; virtual void setContextPath(const char * path) = 0; virtual const char * getContextPath() = 0; virtual void setBindingValue(void * value) = 0; diff --git a/esp/platform/espcontext.cpp b/esp/platform/espcontext.cpp index e5016f6506e..385c554149c 100755 --- a/esp/platform/espcontext.cpp +++ b/esp/platform/espcontext.cpp @@ -28,6 +28,7 @@ #include "espsecurecontext.hpp" #include "ldapsecurity.ipp" #include "dasds.hpp" +#include "secmanagertracedecorator.hpp" class CEspContext : public CInterface, implements IEspContext { @@ -50,6 +51,7 @@ class CEspContext : public CInterface, implements IEspContext Owned m_user; Owned m_resources; Owned m_secmgr; + Owned m_tracingSecMgrDecorator; Owned m_feature_authmap; Owned m_sec_settings; @@ -289,9 +291,20 @@ class CEspContext : public CInterface, implements IEspContext } virtual void setSecManger(ISecManager* mgr) + { + setSecManager(mgr, nullptr); + } + + virtual void setSecManager(ISecManager* mgr, ISecManager* tracingSecMgrDecorator) override { m_secmgr.setown(mgr); - m_SecurityHandler.setSecManger(mgr); + if (!mgr) + m_tracingSecMgrDecorator.clear(); + else if (!tracingSecMgrDecorator) + m_tracingSecMgrDecorator.setown(new CSecManagerTraceDecorator(*mgr)); + else + m_tracingSecMgrDecorator.set(tracingSecMgrDecorator); + m_SecurityHandler.setSecManager(mgr, tracingSecMgrDecorator); } virtual ISecManager* querySecManager() @@ -299,6 +312,11 @@ class CEspContext : public CInterface, implements IEspContext return m_secmgr.get(); } + virtual ISecManager* queryTracingSecManager() override + { + return m_tracingSecMgrDecorator.get(); + } + virtual void setBindingValue(void * value) { m_bindingValue=value; diff --git a/esp/platform/sechandler.cpp b/esp/platform/sechandler.cpp index 346adfeddeb..df9a30e9c12 100644 --- a/esp/platform/sechandler.cpp +++ b/esp/platform/sechandler.cpp @@ -41,8 +41,19 @@ SecHandler::~SecHandler() } void SecHandler::setSecManger(ISecManager* mgr) +{ + setSecManager(mgr, nullptr); +} + +void SecHandler::setSecManager(ISecManager* mgr, ISecManager* tracingSecMgrDecoratorDecorator) { m_secmgr.set(mgr); + if (!mgr) + m_tracingSecMgrDecorator.clear(); + else if (!tracingSecMgrDecoratorDecorator) + m_tracingSecMgrDecorator.setown(new CSecManagerTraceDecorator(*mgr)); + else + m_tracingSecMgrDecorator.set(tracingSecMgrDecoratorDecorator); } @@ -136,7 +147,7 @@ bool SecHandler::authorizeSecFeature(const char * pszFeatureUrl, const char* Use bool SecHandler::authorizeTrial(ISecUser& user,const char* pszFeatureUrl, SecAccessFlags & required_access) { - int trial_access = CSecManagerTraceDecorator(*m_secmgr).authorizeEx(RT_TRIAL,user,pszFeatureUrl); + int trial_access = m_tracingSecMgrDecorator->authorizeEx(RT_TRIAL,user,pszFeatureUrl); if(trial_access < required_access) throw MakeStringException(201,"Your company has used up all of their free transaction credits"); return true; @@ -267,7 +278,7 @@ bool SecHandler::authorizeSecReqFeatures(StringArray & features, IEspStringIntMa bool auth_ok = false; try { - auth_ok = CSecManagerTraceDecorator(*m_secmgr).authorize(*m_user.get(), plist, m_secureContext.get()); + auth_ok = m_tracingSecMgrDecorator->authorize(*m_user.get(), plist, m_secureContext.get()); } catch(IException* e) { diff --git a/esp/platform/sechandler.hpp b/esp/platform/sechandler.hpp index 0d30b77dded..cb29845c805 100644 --- a/esp/platform/sechandler.hpp +++ b/esp/platform/sechandler.hpp @@ -28,6 +28,7 @@ class SecHandler : public CInterface { Owned m_secmgr; + Owned m_tracingSecMgrDecorator; Owned m_resources; Owned m_user; Owned m_feature_authmap; @@ -49,6 +50,7 @@ class SecHandler : public CInterface bool authorizeSecFeature(const char * pszFeatureUrl, const char* UserID, const char* CompanyID, SecAccessFlags & access,bool bCheckTrial,int DebitUnits, SecUserStatus & user_status); void setSecManger(ISecManager* mgr); + void setSecManager(ISecManager* mgr, ISecManager* tracingSecMgrDecoratorDecorator); void setResources(ISecResourceList* rlist); void setUser(ISecUser* user); void setFeatureAuthMap(IAuthMap * map); diff --git a/esp/services/ws_dfu/ws_dfuService.cpp b/esp/services/ws_dfu/ws_dfuService.cpp index edc86ab58df..1b012525fe8 100644 --- a/esp/services/ws_dfu/ws_dfuService.cpp +++ b/esp/services/ws_dfu/ws_dfuService.cpp @@ -2021,7 +2021,7 @@ static void getFilePermission(CDfsLogicalFileName &dlfn, ISecUser & user, IUserD StringBuffer scopes; dlfn.getScopes(scopes); - permissionTemp = CSecManagerTraceDecorator(*secmgr).authorizeFileScope(user, scopes.str()); + permissionTemp = secmgr->authorizeFileScope(user, scopes.str()); } //Descrease the permission whenever a component has a lower permission. @@ -2034,7 +2034,7 @@ static void getFilePermission(CDfsLogicalFileName &dlfn, ISecUser & user, IUserD bool CWsDfuEx::getUserFilePermission(IEspContext &context, IUserDescriptor* udesc, const char* logicalName, SecAccessFlags& permission) { - ISecManager* secmgr = context.querySecManager(); + ISecManager* secmgr = context.queryTracingSecManager(); if (!secmgr) { return false; diff --git a/helm/examples/esp/README.md b/helm/examples/esp/README.md index fd4ca0ce1f9..28f5cbdb30d 100644 --- a/helm/examples/esp/README.md +++ b/helm/examples/esp/README.md @@ -47,6 +47,7 @@ esp: - name: eclwatch traceFlags: traceLevel: 2 + traceSecMgr: true ``` ## Cluster Overrides @@ -61,7 +62,7 @@ The previous YAML example may be reproduced in XML with the following: ```xml - + ... ``` diff --git a/system/security/shared/secmanagertracedecorator.hpp b/system/security/shared/secmanagertracedecorator.hpp index 4699f66a0f5..949fad21739 100644 --- a/system/security/shared/secmanagertracedecorator.hpp +++ b/system/security/shared/secmanagertracedecorator.hpp @@ -22,54 +22,11 @@ #include "jthread.hpp" #include "jtrace.hpp" -/** - * @brief Templated utility class to decorate an object with additional functionality. - * - * A decorator implements the same named interface as the objects it decorates. This class extends - * the interface, leaving implementation to subclasses. This class requires the decorated object to - * implement a named interface, even if only IInterface. - * - * In the ideal situation, the decorated object's interface is described completely by a named - * interface. By implementing the same interface, a decorator is interchangeable with the object - * it decoratos. - * - * In less than ideal situations, the decorated object's interface is an extension of a named - * interface. The decorator extends the extends the named interface, with subclasses required to - * implement both the named interface and all extensions. The decorator should then be - * interchangeable with its decorated object as a templated argument, but not be cast to the - * decorated type. - * - * The less ideal scenario is suported by two template parameters. The ideal situation requires - * only the first. - * - decorated_t is the type of the object to be decorated. If the the decorated object conforms - * to an interface, use of the interface is preferred. - * - secorated_interface_t is the interface implemented by the decorated object. If not the same - * as decorated_t, it is assumed to be a base of that type. - * - * Consider the example of ISecManager, generally, and the special case of CLdapSecManager. For - * most security managers, both template parameters may be ISecManager. CLdapSecManager is an - * exception because it exposes additional interfaces not included in ISecManager or any other - * interface. In this case, decorated_t should be CLdapSecManager and decorated_interface_t should - * be ISecManager. - */ -template -class TDecorator : public CInterfaceOf -{ -protected: - Linked decorated; - -public: - TDecorator(decorated_t &_decorated) : decorated(&_decorated) {} - virtual ~TDecorator() {} - decorated_t *queryDecorated() { return decorated.get(); } - decorated_t *getDecorated() { return decorated.getLink(); } -}; - /** * @brief Macro used start tracing a block of code in the security manager decorator. * * Create a new named internal span and enter a try block. Used with END_SEC_MANAGER_TRACE_BLOCK, - * provides consistent timing and exception handling for the inned code block. + * provides consistent timing and exception handling for the inner code block. * * Some security manager requests include outgoing remote calls, but the ESP does not assume * which requests are remote. Managers making remote calls may considier creating client spans @@ -122,11 +79,9 @@ class TDecorator : public CInterfaceOf * - If nothing else changes, create a subclasses of TSecManagerTraceDecorator, with template * parameters CLdapSecManager and ISecManager, to decorate the LDAP-specific interfaces. */ -template -class TSecManagerTraceDecorator : public TDecorator +template +class TSecManagerTraceDecorator : public CInterfaceOf { - using TDecorator::decorated; - public: virtual SecFeatureSet queryFeatures(SecFeatureSupportLevel level) const override { @@ -359,17 +314,12 @@ class TSecManagerTraceDecorator : public TDecorator decorated; + bool tracing = queryTraceManager().isTracingEnabled(); public: TSecManagerTraceDecorator(secmgr_t &_decorated) - : TDecorator(_decorated) - , implemented(_decorated.queryFeatures(SecFeatureSupportLevel::SFSL_Implemented)) - , tracing(queryTraceManager().isTracingEnabled() && doTrace(traceSecMgr)) - { - } - virtual ~TSecManagerTraceDecorator() + : decorated(&_decorated) { } @@ -684,7 +634,7 @@ class TSecManagerTraceDecorator : public TDecoratorcheckImplementedFeatures(feature)); } };