From e73cd16ca3a9c658da6a752e04525853a1ca32ac Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Fri, 10 Nov 2023 09:52:15 -0500 Subject: [PATCH 1/9] HPCC-26029 Improve ESDL script secret support - Add a transactional secret cache to the script context. - Add secret support to http-post-xml. - Update secret support in mysql. Signed-off-by: Tim Klemm --- esp/esdllib/README.md | 16 ++- esp/esdlscriptlib/esdl_script.cpp | 212 +++++++++++++++++++++++++----- esp/esdlscriptlib/esdl_script.hpp | 102 ++++++++++++++ 3 files changed, 293 insertions(+), 37 deletions(-) diff --git a/esp/esdllib/README.md b/esp/esdllib/README.md index 7219c2a93ca..0c46b84a723 100644 --- a/esp/esdllib/README.md +++ b/esp/esdllib/README.md @@ -1555,8 +1555,11 @@ Iterates a possibly empty set of input nodes. Child operations are processed onc optional="Boolean value" trace="String value" url="String value" + secret="String value" section="XPath node expression" - name="String value"> + name="String value" + vault="String value" + version="String value> @@ -1570,13 +1573,16 @@ Create then send an HTTP post message with XML content. Content type of the outg | - | - | - | | @optional | 0..1 | Boolean flag indicating whether script syntax errors are fatal (*false*) or merely generate warnings (*true*). Defaults to *false*. | | @trace | 0..1 | Label used in trace log output messages. If omitted or empty, the element name is used. | -| @url | 1..1 | Endpoint to send the HTTP post message. | +| @url | 0..1 | Endpoint to send the HTTP post message. Ignored when `secret` is given; required otherwise. | +| @secret | 0..1 | Name of an "esp" category http-connect secret from which the message endpoint and authorization values are to be obtained. A named secret always takes precedence of `@url`. A named secret will be used to auto-fill an *Authorization* `http-header` for supported authorization methods, including:

- Basic: `username` and `password` secret properties required | | @section | 0..1 | Path to the section of script context where output is placed. If omitted defaults to `temporaries`.| | @name | 1..1 | Name of the node inside `@section` where the output is placed. If it does not exist it is created. | +| @vault | 0..1 | String identifier of the repository containing `secret` referenced http-connect secret. Ignored when `secret` is unused; optional otherwise. | +| @version | 0..1 | String representation of a secret version. Ignored when `secret` is unused; optional otherwise. | | http-header | 0..n | An HTTP Header name and value to include with the POST. Use one element for each header | | http-header/@name | 0..1 | String value giving the name of the HTTP header.| | http-header/@xpath-name | 0..1 | XPath expression evaluated as a string giving the name of the HTTP header. | -| http-header/@value | 1..1 | The value of the HTTP header. | +| http-header/@value | 1..1 | The value of the HTTP header. When used with an http-connect secret, an *Authorization* header's value must be the authorization method, e.g., *Basic*., or empty to default to *Basic*. | | content | 1..1 | Contains child script operations that construct the XML payload of the HTTP Post. | Note: @@ -1751,7 +1757,8 @@ See also [ensure-target](#ensure-target) and [target](#target). server="String value" trace="String value" user="String value" - vault="String value"> + vault="String value" + version="String value"> ... @@ -1773,6 +1780,7 @@ See also [ensure-target](#ensure-target) and [target](#target). | @trace | 0..1 | Label used in trace log output messages. If omitted or empty, the element name is used. | | @user | 0..1 | String of the user name to login to the database server. Required if not provided in a _vault_ or _secret_. See the following [Credentials](#credentials) section for further explanation. | | @vault | 0..1 | String of the vault ID to retrieve database login info from. The category name used is *esp*. For security, preferred usage is to include user, password, server and database together. See the following [Credentials](#credentials) section for further explanation. | +| @version | 0..1 | String representation of a secret version. Ignored when `secret` is unused; optional otherwise. | | bind | 0..n | Node to bind a value to a sql parameter. | | bind/@name | 1..1 | String of the name of the sql parameter. Use only one instance per `bind` node. | | bind/@value | 1..1 | XPath string expression of the value of the parameter. Use only one instance per `bind` node. | diff --git a/esp/esdlscriptlib/esdl_script.cpp b/esp/esdlscriptlib/esdl_script.cpp index 4c4c24b7f5c..a252eda6a4e 100644 --- a/esp/esdlscriptlib/esdl_script.cpp +++ b/esp/esdlscriptlib/esdl_script.cpp @@ -176,6 +176,11 @@ class CEsdlScriptContext : public CInterfaceOf return (!traceOptionsScopes.empty() && traceOptionsScopes.back().second); } + virtual IPTree* getSecret(const char* category, const SecretId& id) override + { + return secrets.getSecret(category, id); + } + protected: virtual void pushMaskerScope() override { @@ -225,6 +230,7 @@ class CEsdlScriptContext : public CInterfaceOf using TraceOptionsScopeStack = std::list; MaskerScopeStack maskerScopes; TraceOptionsScopeStack traceOptionsScopes; + TransactionSecrets secrets; public: CEsdlScriptContext(IEspContext* _espCtx, IEsdlFunctionRegister* _functionRegister, IDataMaskingEngine* _engine) : functionRegister(_functionRegister) @@ -766,8 +772,7 @@ class CEsdlTransformOperationMySqlCall : public CEsdlTransformOperationBase protected: StringAttr m_name; - Owned m_vaultName; - Owned m_secretName; + EsdlScriptSecretSpec m_secretSpec; Owned m_section; Owned m_resultsetTag; Owned m_server; @@ -785,7 +790,9 @@ class CEsdlTransformOperationMySqlCall : public CEsdlTransformOperationBase IArrayOf m_parameters; public: - CEsdlTransformOperationMySqlCall(IXmlPullParser &xpp, StartTag &stag, const StringBuffer &prefix) : CEsdlTransformOperationBase(xpp, stag, prefix) + CEsdlTransformOperationMySqlCall(IXmlPullParser &xpp, StartTag &stag, const StringBuffer &prefix) + : CEsdlTransformOperationBase(xpp, stag, prefix) + , m_secretSpec(stag) { ensureMysqlEmbed(); @@ -797,8 +804,6 @@ class CEsdlTransformOperationMySqlCall : public CEsdlTransformOperationBase //without select, it executes once in the current context m_select.setown(compileOptionalXpath(stag.getValue("select"))); - m_vaultName.setown(compileOptionalXpath(stag.getValue("vault"))); - m_secretName.setown(compileOptionalXpath(stag.getValue("secret"))); m_section.setown(compileOptionalXpath(stag.getValue("section"))); m_resultsetTag.setown(compileOptionalXpath(stag.getValue("resultset-tag"))); @@ -917,19 +922,15 @@ class CEsdlTransformOperationMySqlCall : public CEsdlTransformOperationBase // the most secure option in my opinion is to at least have the server, name, and password all in the secret // with the server included the credentials can't be hijacked and sent somewhere else for capture. // - if (!m_secretName) + if (!m_secretSpec.name) return nullptr; - StringBuffer name; - sourceContext->evaluateAsString(m_secretName, name); - if (name.isEmpty()) + SecretId id(m_secretSpec, sourceContext); + if (id.name.isEmpty()) { - missingMySqlOptionError(name, true); + missingMySqlOptionError("secret name", true); return nullptr; } - StringBuffer vault; - if (m_vaultName) - sourceContext->evaluateAsString(m_vaultName, vault); - return getSecret("esp", name, vault); + return getSecret("esp", id.name, id.vault); } void appendOption(StringBuffer &options, const char *name, const char *value, bool required) { @@ -1091,13 +1092,16 @@ class CEsdlTransformOperationMySqlCall : public CEsdlTransformOperationBase class CEsdlTransformOperationHttpHeader : public CEsdlTransformOperationWithoutChildren, implements IEsdlTransformOperationHttpHeader { protected: + EsdlScriptSecretSpec m_secretSpec; XPathLiteralUnion m_name; Owned m_value; public: IMPLEMENT_IINTERFACE_USING(CEsdlTransformOperationWithoutChildren) - CEsdlTransformOperationHttpHeader(IXmlPullParser &xpp, StartTag &stag, const StringBuffer &prefix) : CEsdlTransformOperationWithoutChildren(xpp, stag, prefix) + CEsdlTransformOperationHttpHeader(IXmlPullParser &xpp, StartTag &stag, const StringBuffer &prefix, EsdlScriptSecretSpec& secretSpec) + : CEsdlTransformOperationWithoutChildren(xpp, stag, prefix) + , m_secretSpec(secretSpec) { m_name.setRequired(stag, "name", *this); if (m_name.isEmpty()) @@ -1119,14 +1123,56 @@ class CEsdlTransformOperationHttpHeader : public CEsdlTransformOperationWithoutC bool processHeader(IEsdlScriptContext * scriptContext, IXpathContext * targetContext, IXpathContext * sourceContext, IProperties *headers) override { - CXpathContextLocation location(targetContext); - targetContext->addElementToLocation("header"); - StringBuffer name; + StringBuffer name, value; m_name.get(name, *sourceContext); - - StringBuffer value; if (m_value) sourceContext->evaluateAsString(m_value, value); + if (m_secretSpec.name && streq(name, "Authorization")) + generateAuthorizationValue(scriptContext, sourceContext, value); + return processHeaderValue(scriptContext, targetContext, sourceContext, headers, name, value); + } + + virtual void toDBGLog () override + { + #if defined(_DEBUG) + DBGLOG ("> %s (%s, value(%s)) >>>>>>>>>>", m_tagname.str(), m_name.configValue(), m_value ? m_value->getXpath() : ""); + #endif + } + +protected: + void generateAuthorizationValue(IEsdlScriptContext * scriptContext, IXpathContext * sourceContext, StringBuffer& method) + { + HttpConnectSecretId id(m_secretSpec, sourceContext); + if (id.name.isEmpty()) + recordException(ESDL_SCRIPT_InvalidOperationAttr, "empty or missing http-connect secret name"); + Owned secret(scriptContext->getSecret("esp", id)); + if (!secret) + recordException(ESDL_SCRIPT_MissingOperationAttr, "missing http-connect secret"); + + if (method.isEmpty() || streq(method, "Basic")) + generateBasicAuthValue(method, *secret); + else + recordException(ESDL_SCRIPT_InvalidOperationAttr, VStringBuffer("unrecognized authorization method '%s'", method.str())); + } + + void generateBasicAuthValue(StringBuffer& method, const IPTree& secret) + { + StringBuffer username, password; + if (!getSecretKeyValue(username, &secret, "username") || !getSecretKeyValue(password, &secret, "password")) + recordException(ESDL_SCRIPT_MissingOperationAttr, "empty or missing http-connect basic authorization credentials"); + VStringBuffer credentials("%s:%s", username.str(), password.str()); + StringBuffer encoded; + JBASE64_Encode(credentials, credentials.length(), encoded, false); + if (method.isEmpty()) + method.append("Basic"); + method.append(' ').append(encoded); + } + + bool processHeaderValue(IEsdlScriptContext * scriptContext, IXpathContext * targetContext, IXpathContext * sourceContext, IProperties *headers, const StringBuffer& name, const StringBuffer& value) + { + CXpathContextLocation location(targetContext); + targetContext->addElementToLocation("header"); + if (name.length() && value.length()) { if (headers) @@ -1136,13 +1182,6 @@ class CEsdlTransformOperationHttpHeader : public CEsdlTransformOperationWithoutC } return false; } - - virtual void toDBGLog () override - { - #if defined(_DEBUG) - DBGLOG ("> %s (%s, value(%s)) >>>>>>>>>>", m_tagname.str(), m_name.configValue(), m_value ? m_value->getXpath() : ""); - #endif - } }; class OperationStateHttpPostXml : public CInterfaceOf //plain CInterface doesn't actually give us our opaque IInterface pointer @@ -1158,6 +1197,7 @@ class OperationStateHttpPostXml : public CInterfaceOf //plain CInter class CEsdlTransformOperationHttpPostXml : public CEsdlTransformOperationBase { protected: + EsdlScriptSecretSpec m_secretSpec; StringAttr m_name; StringAttr m_section; Owned m_url; @@ -1166,7 +1206,9 @@ class CEsdlTransformOperationHttpPostXml : public CEsdlTransformOperationBase Owned m_testDelay; public: - CEsdlTransformOperationHttpPostXml(IXmlPullParser &xpp, StartTag &stag, const StringBuffer &prefix, IEsdlFunctionRegister *functionRegister) : CEsdlTransformOperationBase(xpp, stag, prefix) + CEsdlTransformOperationHttpPostXml(IXmlPullParser &xpp, StartTag &stag, const StringBuffer &prefix, IEsdlFunctionRegister *functionRegister) + : CEsdlTransformOperationBase(xpp, stag, prefix) + , m_secretSpec(stag) { m_name.set(stag.getValue("name")); if (m_traceName.isEmpty()) @@ -1176,10 +1218,13 @@ class CEsdlTransformOperationHttpPostXml : public CEsdlTransformOperationBase m_section.set("temporaries"); if (m_name.isEmpty()) recordError(ESDL_SCRIPT_MissingOperationAttr, "without name"); - const char *url = stag.getValue("url"); - if (isEmptyString(url)) - recordError(ESDL_SCRIPT_MissingOperationAttr, "without url"); - m_url.setown(compileXpath(url)); + if (!m_secretSpec.name) + { + const char* url = stag.getValue("url"); + if (isEmptyString(url)) + recordError(ESDL_SCRIPT_MissingOperationAttr, "without url"); + m_url.setown(compileXpath(url)); + } const char *msTestDelayStr = stag.getValue("test-delay"); if (!isEmptyString(msTestDelayStr)) m_testDelay.setown(compileXpath(msTestDelayStr)); @@ -1195,7 +1240,7 @@ class CEsdlTransformOperationHttpPostXml : public CEsdlTransformOperationBase if (isEmptyString(op)) recordError(ESDL_SCRIPT_Error, "unknown error"); if (streq(op, "http-header")) - m_headers.append(*new CEsdlTransformOperationHttpHeader(xpp, stag, prefix)); + m_headers.append(*new CEsdlTransformOperationHttpHeader(xpp, stag, prefix, m_secretSpec)); else if (streq(op, "content")) m_content.setown(new CEsdlTransformOperationHttpContentXml(xpp, stag, prefix, functionRegister)); else @@ -1240,6 +1285,17 @@ class CEsdlTransformOperationHttpPostXml : public CEsdlTransformOperationBase targetContext->ensureLocation(xpath, true); if (m_url) sourceContext->evaluateAsString(m_url, preparedState->url); + else if (m_secretSpec.name) + { + HttpConnectSecretId id(m_secretSpec, sourceContext); + if (id.name.isEmpty()) + recordException(ESDL_SCRIPT_MissingOperationAttr, "missing http-connect secret identification"); + Owned secret(scriptContext->getSecret("esp", id)); + if (!secret) + recordException(ESDL_SCRIPT_MissingOperationAttr, "missing http-connect secret"); + if (!getSecretKeyValue(preparedState->url, secret, "url")) + recordException(ESDL_SCRIPT_MissingOperationAttr, "missing http-connect secret url"); + } //don't complain if test-delay is used but test mode is off. Any script can be instrumented for testing but won't run those features outside of testing if (scriptContext->getTestMode() && m_testDelay) @@ -3909,6 +3965,96 @@ class CEsdlTransformMethodMap : public CInterfaceOf } }; +EsdlScriptSecretSpec::EsdlScriptSecretSpec(StartTag& stag) +{ + name.setown(compileOptionalXpath(stag.getValue("secret"))); + vault.setown(compileOptionalXpath(stag.getValue("vault"))); + version.setown(compileOptionalXpath(stag.getValue("version"))); +} + +SecretId::SecretId(EsdlScriptSecretSpec& spec, IXpathContext* context) +{ + if (spec.name) + { + context->evaluateAsString(spec.name, name); + if (!name.isEmpty()) + { + if (spec.vault) + context->evaluateAsString(spec.vault, vault); + if (spec.version) + context->evaluateAsString(spec.version, version); + } + } +} + +SecretId::SecretId(const char* identifier) +{ + if (!isEmptyString(identifier)) + { + StringArray tokens; + tokens.appendList(identifier, ":", true); + switch (tokens.ordinality()) + { + case 3: + version.append(tokens.item(2)); + // fall through + case 2: + vault.append(tokens.item(1)); + // fall through + case 1: + name.append(tokens.item(0)); + break; + default: + break; + } + } +} + +IPTree* TransactionSecrets::getSecret(const char* category, const SecretId& id) +{ + Owned secret(lookup(category, id)); + if (!secret) + { + secret.setown(::getSecret(category, id.name, id.vault, id.version)); + if (secret) + store(*secret, category, id); + } + return secret; +} + +IPTree* TransactionSecrets::lookup(const char* category, const SecretId& id) const +{ + Cache::const_iterator it = cache.find(makeKey(category, id)); + if (it != cache.end()) + return it->second.getLink(); + return nullptr; +} + +void TransactionSecrets::store(IPTree& secret, const char* category, const SecretId& id) +{ + cache[makeKey(category, id)].set(&secret); +} + +HttpConnectSecretId::HttpConnectSecretId(EsdlScriptSecretSpec& spec, IXpathContext* context) + : SecretId(spec, context) +{ + normalize(); +} + +HttpConnectSecretId::HttpConnectSecretId(const char* identifier) + : SecretId(identifier) +{ + normalize(); +} + +void HttpConnectSecretId::normalize() +{ + static const char* prefix = "http-connect-"; + static size_t prefixLength = strlen(prefix); + if (!name.isEmpty() && strncmp(name, prefix, prefixLength) != 0) + name.insert(0, prefix); +} + esdlscript_decl IEsdlScriptContext* createEsdlScriptContext(IEspContext* espCtx, IEsdlFunctionRegister* functionRegister, IDataMaskingEngine* engine) { return new CEsdlScriptContext(espCtx, functionRegister, engine); diff --git a/esp/esdlscriptlib/esdl_script.hpp b/esp/esdlscriptlib/esdl_script.hpp index 9a935a9db1a..56288158450 100644 --- a/esp/esdlscriptlib/esdl_script.hpp +++ b/esp/esdlscriptlib/esdl_script.hpp @@ -52,6 +52,101 @@ interface IEsdlFunctionRegister; +namespace xpp +{ + class StartTag; +} + +/** + * @brief Encapsulation of scripted secret identification tokens. + * + * A value for `name` is required to identify a secret. The distinction of whether a secret is + * required or accepted beyond the scope of this structure. + * + * Vault and version strings may be specified in an operation's attribute list. These values are + * ignored in the absence of `name`. + */ +struct EsdlScriptSecretSpec +{ + Owned name; + Owned vault; + Owned version; + + EsdlScriptSecretSpec(xpp::StartTag& stag); +}; + +/** + * @brief Encapsulation of a secret identification. + * + * Extracts secret identification labels in multiple formats: + * + * - The compiled XPath expressions from an `EsdlScriptSecretSpec` are evaluated as strings. + * - An identifier of the form `name [ ":" vault [ ":" version ] ]` is split into parts. + */ +struct SecretId +{ + StringBuffer name; + StringBuffer vault; + StringBuffer version; + + SecretId(EsdlScriptSecretSpec& spec, IXpathContext* context); + SecretId(const char* identifier); +}; + +/** + * @brief Cache of secrets used during a transaction. + * + * Secrets expire. On expiration, they are reloaded. When reloaded, their contents can differ + * from prior values. During a single transaction, it is preferred that two or more references + * to the same secret should always yield the same values. + * + * This addresses the edge case of a secret expiring mid-transaction. Given a secret's category + * and identity, a previously loaded secret will be returned before looking up new data. The + * life expectancy of data in this cache is intented to be for a single transaction. + * + * It is generally incorrect to treat secrets as unchanging data for extended periods of time. + * Ensure instances of this class are retained any longer than necessary. + */ +class TransactionSecrets +{ +private: + /** + * @brief The cache key type is a comination of secret category, name, vault ID, and version. + */ + using Key = std::tuple; + using Cache = std::map>; + Cache cache; +public: + IPTree* getSecret(const char* category, const SecretId& id); +protected: + inline Key makeKey(const char* category, const SecretId& id) const + { + return std::make_tuple( + (category ? category : ""), + id.name.str(), + id.vault.str(), + id.version.str() + ); + } + IPTree* lookup(const char* category, const SecretId& id) const; + void store(IPTree& secret, const char* category, const SecretId& id); +}; + +/** + * @brief `SecretId` extension enforcing `http-connect` secret naming conventions. + * + * If a stored secret used to establish HTTP connections is required to have a name prefix of + * `http-connect-`, this ensures that a configured reference to `foo` becomes `http-connect-foo` + * while a configured reference to `http-connect-foo` remains unchanged. + */ +struct HttpConnectSecretId : public SecretId +{ + HttpConnectSecretId(EsdlScriptSecretSpec& spec, IXpathContext* context); + HttpConnectSecretId(const char* identifier); +private: + void normalize(); +}; + /** * @brief Script-specific context associated with a sectional document model. * @@ -124,6 +219,13 @@ interface IEsdlScriptContext : extends ISectionalXmlDocModel */ virtual bool isTraceLocked() const = 0; + /** + * @brief Get a secret based on a category and a secret identifier. + * + * @return a secret property tree. + */ + virtual IPTree* getSecret(const char* category, const SecretId& id) = 0; + protected: /** * @brief Helper class encapsulating the use of `pushMaskerScope` and `popMaskerScope`. From 603c5378771fe7020234802e27e3f36777c0f642 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Thu, 16 Nov 2023 07:27:27 -0500 Subject: [PATCH 2/9] Updates from review and discussion comments --- esp/esdllib/README.md | 8 +-- esp/esdlscriptlib/esdl_script.cpp | 85 ++++++++++++++++++++----------- esp/esdlscriptlib/esdl_script.hpp | 14 +++-- 3 files changed, 67 insertions(+), 40 deletions(-) diff --git a/esp/esdllib/README.md b/esp/esdllib/README.md index 0c46b84a723..989cdce959a 100644 --- a/esp/esdllib/README.md +++ b/esp/esdllib/README.md @@ -1559,7 +1559,6 @@ Iterates a possibly empty set of input nodes. Child operations are processed onc section="XPath node expression" name="String value" vault="String value" - version="String value> @@ -1577,8 +1576,7 @@ Create then send an HTTP post message with XML content. Content type of the outg | @secret | 0..1 | Name of an "esp" category http-connect secret from which the message endpoint and authorization values are to be obtained. A named secret always takes precedence of `@url`. A named secret will be used to auto-fill an *Authorization* `http-header` for supported authorization methods, including:

- Basic: `username` and `password` secret properties required | | @section | 0..1 | Path to the section of script context where output is placed. If omitted defaults to `temporaries`.| | @name | 1..1 | Name of the node inside `@section` where the output is placed. If it does not exist it is created. | -| @vault | 0..1 | String identifier of the repository containing `secret` referenced http-connect secret. Ignored when `secret` is unused; optional otherwise. | -| @version | 0..1 | String representation of a secret version. Ignored when `secret` is unused; optional otherwise. | +| @vault | 0..1 | String identifier of the repository containing `secret` referenced http-connect secret. Ignored when `secret` is unused; optional otherwise.

This value is deprecated. The preferred way to specify a vault is to include it in `@secret` as `vault::secret`. | | http-header | 0..n | An HTTP Header name and value to include with the POST. Use one element for each header | | http-header/@name | 0..1 | String value giving the name of the HTTP header.| | http-header/@xpath-name | 0..1 | XPath expression evaluated as a string giving the name of the HTTP header. | @@ -1758,7 +1756,6 @@ See also [ensure-target](#ensure-target) and [target](#target). trace="String value" user="String value" vault="String value" - version="String value"> ... @@ -1779,8 +1776,7 @@ See also [ensure-target](#ensure-target) and [target](#target). | @server | 1..1 | String value of the database server in the format *ip:port* or *hostname:port*. Required if not provided in a _vault_ or _secret_. See the following [Credentials](#credentials) section for further explanation.| | @trace | 0..1 | Label used in trace log output messages. If omitted or empty, the element name is used. | | @user | 0..1 | String of the user name to login to the database server. Required if not provided in a _vault_ or _secret_. See the following [Credentials](#credentials) section for further explanation. | -| @vault | 0..1 | String of the vault ID to retrieve database login info from. The category name used is *esp*. For security, preferred usage is to include user, password, server and database together. See the following [Credentials](#credentials) section for further explanation. | -| @version | 0..1 | String representation of a secret version. Ignored when `secret` is unused; optional otherwise. | +| @vault | 0..1 | String of the vault ID to retrieve database login info from. The category name used is *esp*. For security, preferred usage is to include user, password, server and database together. See the following [Credentials](#credentials) section for further explanation.

This value is deprecated. The preferred way to specify a vault is to include it in `@secret` as `vault::secret`. | | bind | 0..n | Node to bind a value to a sql parameter. | | bind/@name | 1..1 | String of the name of the sql parameter. Use only one instance per `bind` node. | | bind/@value | 1..1 | XPath string expression of the value of the parameter. Use only one instance per `bind` node. | diff --git a/esp/esdlscriptlib/esdl_script.cpp b/esp/esdlscriptlib/esdl_script.cpp index a252eda6a4e..ec35910ad6f 100644 --- a/esp/esdlscriptlib/esdl_script.cpp +++ b/esp/esdlscriptlib/esdl_script.cpp @@ -1127,9 +1127,13 @@ class CEsdlTransformOperationHttpHeader : public CEsdlTransformOperationWithoutC m_name.get(name, *sourceContext); if (m_value) sourceContext->evaluateAsString(m_value, value); - if (m_secretSpec.name && streq(name, "Authorization")) + bool maskInContext = false; + if (m_secretSpec.name && strieq(name, "Authorization")) + { generateAuthorizationValue(scriptContext, sourceContext, value); - return processHeaderValue(scriptContext, targetContext, sourceContext, headers, name, value); + maskInContext = true; + } + return processHeaderValue(scriptContext, targetContext, sourceContext, headers, name, value, maskInContext); } virtual void toDBGLog () override @@ -1149,7 +1153,7 @@ class CEsdlTransformOperationHttpHeader : public CEsdlTransformOperationWithoutC if (!secret) recordException(ESDL_SCRIPT_MissingOperationAttr, "missing http-connect secret"); - if (method.isEmpty() || streq(method, "Basic")) + if (method.trim().isEmpty() || strieq(method, "Basic")) generateBasicAuthValue(method, *secret); else recordException(ESDL_SCRIPT_InvalidOperationAttr, VStringBuffer("unrecognized authorization method '%s'", method.str())); @@ -1160,15 +1164,15 @@ class CEsdlTransformOperationHttpHeader : public CEsdlTransformOperationWithoutC StringBuffer username, password; if (!getSecretKeyValue(username, &secret, "username") || !getSecretKeyValue(password, &secret, "password")) recordException(ESDL_SCRIPT_MissingOperationAttr, "empty or missing http-connect basic authorization credentials"); + if (strchr(username, ':')) + recordException(ESDL_SCRIPT_InvalidOperationAttr, "invslid http-connect basic authorization username"); VStringBuffer credentials("%s:%s", username.str(), password.str()); StringBuffer encoded; JBASE64_Encode(credentials, credentials.length(), encoded, false); - if (method.isEmpty()) - method.append("Basic"); - method.append(' ').append(encoded); + method.set("Basic ").append(encoded); } - bool processHeaderValue(IEsdlScriptContext * scriptContext, IXpathContext * targetContext, IXpathContext * sourceContext, IProperties *headers, const StringBuffer& name, const StringBuffer& value) + bool processHeaderValue(IEsdlScriptContext * scriptContext, IXpathContext * targetContext, IXpathContext * sourceContext, IProperties *headers, const StringBuffer& name, const StringBuffer& value, bool maskInContext) { CXpathContextLocation location(targetContext); targetContext->addElementToLocation("header"); @@ -1178,7 +1182,14 @@ class CEsdlTransformOperationHttpHeader : public CEsdlTransformOperationWithoutC if (headers) headers->setProp(name, value); targetContext->ensureSetValue("@name", name, true); - targetContext->ensureSetValue("@value", value, true); + if (maskInContext) + { + StringBuffer masked; + masked.appendN(value.length(), '*'); + targetContext->ensureSetValue("@value", masked, true); + } + else + targetContext->ensureSetValue("@value", value, true); } return false; } @@ -3968,10 +3979,11 @@ class CEsdlTransformMethodMap : public CInterfaceOf EsdlScriptSecretSpec::EsdlScriptSecretSpec(StartTag& stag) { name.setown(compileOptionalXpath(stag.getValue("secret"))); - vault.setown(compileOptionalXpath(stag.getValue("vault"))); - version.setown(compileOptionalXpath(stag.getValue("version"))); + deprecatedVault.setown(compileOptionalXpath(stag.getValue("vault"))); } +static const char* secretTokenDelimiter = ":::"; + SecretId::SecretId(EsdlScriptSecretSpec& spec, IXpathContext* context) { if (spec.name) @@ -3979,10 +3991,14 @@ SecretId::SecretId(EsdlScriptSecretSpec& spec, IXpathContext* context) context->evaluateAsString(spec.name, name); if (!name.isEmpty()) { - if (spec.vault) - context->evaluateAsString(spec.vault, vault); - if (spec.version) - context->evaluateAsString(spec.version, version); + parse(name); + if (spec.deprecatedVault) + { + StringBuffer tmp; + context->evaluateAsString(spec.deprecatedVault, tmp); + if (!tmp.isEmpty() && !vault.isEmpty() && !streq(tmp, vault)) + UERRLOG("deprecated vault ID mismatch ('%s' versus '%s')", tmp.str(), vault.str()); + } } } } @@ -3990,23 +4006,30 @@ SecretId::SecretId(EsdlScriptSecretSpec& spec, IXpathContext* context) SecretId::SecretId(const char* identifier) { if (!isEmptyString(identifier)) - { - StringArray tokens; - tokens.appendList(identifier, ":", true); - switch (tokens.ordinality()) - { - case 3: - version.append(tokens.item(2)); - // fall through - case 2: - vault.append(tokens.item(1)); - // fall through - case 1: - name.append(tokens.item(0)); - break; - default: - break; - } + parse(identifier); +} + +void SecretId::parse(const char* identifier) +{ + StringArray tokens; + tokens.appendList(name, secretTokenDelimiter, true); + switch (tokens.ordinality()) + { + case 1: // name + name.append(tokens.item(0)); + break; + case 2: // vault :: name + vault.append(tokens.item(0)); + name.append(tokens.item(1)); + break; + case 3: // vault :: name :: version + vault.append(tokens.item(0)); + name.append(tokens.item(1)); + version.append(tokens.item(2)); + break; + default: // uh-oh + UERRLOG("invalid secret identifier '%s'", identifier); + break; } } diff --git a/esp/esdlscriptlib/esdl_script.hpp b/esp/esdlscriptlib/esdl_script.hpp index 56288158450..d6208f20b1b 100644 --- a/esp/esdlscriptlib/esdl_script.hpp +++ b/esp/esdlscriptlib/esdl_script.hpp @@ -69,8 +69,7 @@ namespace xpp struct EsdlScriptSecretSpec { Owned name; - Owned vault; - Owned version; + Owned deprecatedVault; EsdlScriptSecretSpec(xpp::StartTag& stag); }; @@ -81,7 +80,14 @@ struct EsdlScriptSecretSpec * Extracts secret identification labels in multiple formats: * * - The compiled XPath expressions from an `EsdlScriptSecretSpec` are evaluated as strings. - * - An identifier of the form `name [ ":" vault [ ":" version ] ]` is split into parts. + * - An evaluated spec.name must take one of these forms: + * - name + * - vault "::" name + * - vault "::" name "::" version + * - An evaluated spec.deprecatedVault must either: + * - be empty + * - match vault when extracted from name + * - be a well formed name when vault not extracted from name */ struct SecretId { @@ -91,6 +97,8 @@ struct SecretId SecretId(EsdlScriptSecretSpec& spec, IXpathContext* context); SecretId(const char* identifier); +private: + void parse(const char* identifier); }; /** From 26668051d031582c3bae06041fb5809677b9dc92 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Wed, 2 Aug 2023 14:29:56 -0400 Subject: [PATCH 3/9] HPCC-29981 Improve gateway support in ESDL script - Add support for gateway URLs that are references to secrets that can be resolved by the query target. - Add support for gateway URLs that are references to secrets that can be resolved by the ESP. Only url, username, and password secret properties are used to assemble a replacement URL for the gateway. Each secret must explicitly enable compatibility with this use case. - Add support for explicit pass-through values not interpreted by the ESP. - Ensure inline URLs can be updated with separately configured username and password values. These updates must be enabled by defining either Gateways/@resolveInlineURLs as true or, for backward compatibility, Gateways/@legacyTransformTarget as a non-empty value. Signed-off-by: Tim Klemm --- esp/services/esdl_svc_engine/esdl_binding.cpp | 271 +++++++++++------- esp/services/esdl_svc_engine/esdl_binding.hpp | 100 ++++++- 2 files changed, 263 insertions(+), 108 deletions(-) diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp index 5c63e363737..d9cfff65fb6 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.cpp +++ b/esp/services/esdl_svc_engine/esdl_binding.cpp @@ -35,6 +35,7 @@ #include "thorxmlwrite.hpp" //JSON WRITER #include "workunit.hpp" #include "wuwebview.hpp" +#include "jsecrets.hpp" #include "jsmartsock.ipp" #include "esdl_monitor.hpp" #include "EsdlAccessMapGenerator.hpp" @@ -734,6 +735,34 @@ void EsdlServiceImpl::configureTargets(IPropertyTree *cfg, const char *service) DBGLOG("Purely scripted service method %s", method); else configureUrlMethod(method, methodCfg); + + // Relocate legacy gateway inline URL resolution from prepareFinalRequest. To deprecate + // that hard-coded logic, scripts need access to resolved URL values in the script + // context's 'target' section. Resolve once per method here, instead of per request in + // adjustTargetConfig. + + IPTree* gateways = methodCfg.queryBranch("Gateways"); + if (!gateways) + continue; + bool resolveInline = (gateways->getPropBool("resolveInlineURLs") || !isEmptyString(gateways->queryProp("@legacyTransformBranch"))); + Owned gwIt(gateways->getElements("Gateway")); + ForEach(*gwIt) + { + IPTree& gw = gwIt->query(); + const char* url = gw.queryProp("@url"); + if (isEmptyString(url)) + continue; + if (strncmp(url, gwTargetSecretPrefix, gwTargetSecretPrefixLength) == 0) + continue; // no change required + if (strncmp(url, gwLocalSecretPrefix, gwLocalSecretPrefixLength) == 0) + continue; // change deferred until use + if (strncmp(url, gwPassThroughPrefix, gwPassThroughPrefixLength) == 0) + gw.setProp("@url", url + gwPassThroughPrefixLength); + else if (!resolveInline) + continue; // no change required + resolveGatewayInlineURL(gw); + } + DBGLOG("Method %s configured", method); } m_transforms->bindFunctionCalls(); @@ -1113,6 +1142,8 @@ void EsdlServiceImpl::handleServiceRequest(IEspContext &context, } else { + adjustTargetConfig(tgtcfg); + //Future: support transforms for all transaction types by moving scripts and script processing up scriptContext.setown(checkCreateEsdlServiceScriptContext(context, srvdef, mthdef, tgtcfg, req)); @@ -1320,6 +1351,138 @@ void EsdlServiceImpl::getSoapError( StringBuffer& out, out.clear().append(finger-start,start); } +void EsdlServiceImpl::adjustTargetConfig(Owned& tgtcfg) const +{ + static const VStringBuffer gwLocalSecretXPath("Gateways/Gateway[@url='%s*']", gwLocalSecretPrefix); + + if (!tgtcfg) + return; + + if (tgtcfg->hasProp(gwLocalSecretXPath)) + { + tgtcfg.setown(createPTreeFromIPT(tgtcfg)); + const char* qt = tgtcfg->queryProp("@querytype"); + bool published = (qt && (strieq(qt, "roxie") || strieq(qt, "wsecl"))); + const char* permission = (published ? "allowPublishedGatewayUsage" : "allowUnpublishedGatewayUsage"); + Owned gwsToResolve(tgtcfg->getElements(gwLocalSecretXPath)); + ForEach(*gwsToResolve) + resolveGatewayLocalSecret(gwsToResolve->query(), permission); + } +} + +void EsdlServiceImpl::resolveGatewayLocalSecret(IPTree& gateway, const char* permission) const +{ + const char* name = gateway.queryProp("@name"); + StringBuffer url(gateway.queryProp("@url")); + if (url.isEmpty()) + throw makeStringExceptionV(-1, "gateway %s: missing url property", name); + if (strncmp(url, gwLocalSecretPrefix, gwLocalSecretPrefixLength)) + throw makeStringExceptionV(-1, "gateway %s: expected '%s...'; got '%s'", name, gwLocalSecretPrefix, url.str()); + const char* identification = url.str() + gwLocalSecretPrefixLength; + StringArray tokens; + tokens.appendList(identification, ":", true); + Owned secret; + switch (tokens.ordinality()) + { + case 1: + secret.setown(getSecret("esp", tokens.item(0))); + break; + case 2: + secret.setown(getVaultSecret("esp", tokens.item(0), tokens.item(1))); + break; + default: + throw makeStringExceptionV(-1, "gateway %s: '%s' is not of the form \"[ vault-id ':' ] secret-name\"", name, identification); + } + if (!secret) + throw makeStringExceptionV(-1, "gateway %s: '%s' does not identify an 'esp' category secret", name, identification); + if (!isEmptyString(permission) && !secret->getPropBool(permission)) + throw makeStringExceptionV(-1, "gateway %s: '%s' does not grant '%s' permission", name, identification, permission); + if (!secret->getProp("url", url.clear()) || url.isEmpty()) + throw makeStringExceptionV(-1, "gateway %s: '%s' missing required 'url' property; credential-only secrets not supported", name, identification); + const char* username = secret->queryProp("username"); + if (isEmptyString(username) && !secret->getPropBool("insecure")) + throw makeStringExceptionV(-1, "gateway %s: '%s' missing expected 'username' property; set 'insecure` property to 'true' if credentials are not required", name, identification); + const char* password = secret->queryProp("password"); + if (isEmptyString(username) && password) + throw makeStringExceptionV(-1, "gateaay %s: '%s' invalid use of password without username", name, identification); + adjustURL(url, username, password); + gateway.setProp("@url", url); +} + +void EsdlServiceImpl::resolveGatewayInlineURL(IPTree& gateway) const +{ + const char* name = gateway.queryProp("@name"); + StringBuffer url(gateway.queryProp("@url")); + if (url.isEmpty()) + throw makeStringExceptionV(-1, "gateway %s: missing required '@url' property", name); + const char* username = gateway.queryProp("@username"); + const char* password = gateway.queryProp("@password"); + if (isEmptyString(username) && password) + throw makeStringExceptionV(-1, "gateway %s: invalid credentials - password without username", name); + StringBuffer decryptedPassword; + if (!isEmptyString(password)) + { + decrypt(decryptedPassword, password); + password = decryptedPassword; + } + adjustURL(url, username, password); + gateway.setProp("@url", url); +} + +void EsdlServiceImpl::adjustURL(StringBuffer& url, const char* username, const char* password) const +{ + StringBuffer scheme, usernameSink, passwordSink, host, port, path; + Utils::SplitURL(url, scheme, usernameSink, passwordSink, host, port, path); + + if (scheme.length() > 0 && host.length() > 0) + { + StringBuffer userinfo; + if (!isEmptyString(username)) + { + encodeUrlUseridPassword(userinfo, username); + if (password && *password) // TODO: remove '&& *password' to distinguish empty from omitted passwords + { + userinfo.append(':'); + encodeUrlUseridPassword(userinfo, password); + } + } + + url.clear(); + url.append(scheme); + url.append("://"); + if (userinfo.length()>0 ) + url.appendf("%s@", userinfo.str()); + url.append(host); + if (port.length() > 0) + url.appendf(":%s", port.str()); + if (path.length() > 0) + url.append(path); + } +} + +void EsdlServiceImpl::transformGatewaysConfig( IPropertyTree* srvcfg, IPropertyTree* forRoxie, const char* altElementName ) const +{ + // Do we need to handle 'local FQDN'? It doesn't appear to be in the + // Gateway element. Not sure where it's set but the RemoteNSClient + // references it in RemoteNSFactory.cpp translateGateway() and modifies + // resulting gateway URL if it's set. + if( srvcfg && forRoxie) + { + const char* treeName = (!isEmptyString(altElementName) ? altElementName : "Gateway"); + Owned cfgIter = srvcfg->getElements("Gateways/Gateway"); + ForEach(*cfgIter) + { + IPropertyTree& cfgGateway = cfgIter->query(); + StringBuffer url(cfgGateway.queryProp("@url")); + StringBuffer service(cfgGateway.queryProp("@name")); + Owned gw = createPTree(treeName, false); + gw->addProp("ServiceName", service.toLowerCase().str()); + gw->addProp("URL", url.str()); + forRoxie->addPropTree(treeName, gw.getLink()); + } + } +} + void EsdlServiceImpl::handleFinalRequest(IEspContext &context, IEsdlScriptContext *scriptContext, Owned &tgtcfg, @@ -1693,7 +1856,7 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, if (rowName.isEmpty()) rowName.append("row"); - EsdlBindingImpl::transformGatewaysConfig(tgtcfg, gws, rowName); + transformGatewaysConfig(tgtcfg, gws, rowName); xpath.replaceString("{$query}", tgtQueryName); xpath.replaceString("{$method}", mthName); xpath.replaceString("{$service}", srvdef.queryName()); @@ -3845,6 +4008,7 @@ int EsdlBindingImpl::onGetRoxieBuilder(CHttpRequest* request, CHttpResponse* res StringBuffer reqcontent; getRequestContent(*context, reqcontent, request, srvname, mthname, ns, ROXIEREQ_FLAGS); + m_pESDLService->adjustTargetConfig(tgtcfg); tgtctx.setown( m_pESDLService->createTargetContext(*context, tgtcfg, *defsrv, *defmth, req_pt)); Owned scriptContext = m_pESDLService->checkCreateEsdlServiceScriptContext(*context, *defsrv, *defmth, tgtcfg.get(), req_pt); @@ -4023,111 +4187,6 @@ int EsdlBindingImpl::onGetSampleXml(bool isRequest, IEspContext &ctx, CHttpReque return 0; } -void EsdlBindingImpl::transformGatewaysConfig( IPropertyTree* srvcfg, IPropertyTree* forRoxie, const char* altElementName ) -{ - // Do we need to handle 'local FQDN'? It doesn't appear to be in the - // Gateway element. Not sure where it's set but the RemoteNSClient - // references it in RemoteNSFactory.cpp translateGateway() and modifies - // resulting gateway URL if it's set. - if( srvcfg && forRoxie) - { - Owned cfgIter = srvcfg->getElements("Gateways/Gateway"); - cfgIter->first(); - if( cfgIter->isValid() ) - { - const char* treeName = (!isEmptyString(altElementName) ? altElementName : "Gateway"); - - while( cfgIter->isValid() ) - { - IPropertyTree& cfgGateway = cfgIter->query(); - - StringBuffer url, service; - if( makeURL( url, cfgGateway ) ) - { - cfgGateway.getProp("@name", service); - service.toLowerCase(); - - Owned gw = createPTree(treeName, false); - gw->addProp("ServiceName", service.str()); - gw->addProp("URL", url.str()); - - forRoxie->addPropTree(treeName, gw.getLink()); - } - else - { - DBGLOG( "transformGatewaysConfig: Gateways/Gateway url in config for service='%s'", service.str() ); - } - - cfgIter->next(); - } - } - } -} - -bool EsdlBindingImpl::makeURL( StringBuffer& url, IPropertyTree& cfg ) -{ - bool result = false; - - // decrypt password - // encode password - // construct gateway URL including password - - StringBuffer decryptPass, password; - StringBuffer user; - const char* pw = NULL; - const char* usr = NULL; - - pw = cfg.queryProp("@password"); - if( pw ) - { - decrypt( decryptPass, pw ); - encodeUrlUseridPassword( password, decryptPass.str() ); - } - - usr = cfg.queryProp("@username"); - if( usr ) - { - encodeUrlUseridPassword( user, usr ); - } - - bool roxieClient = cfg.getPropBool("@roxieClient", true); - const char* cfgURL = cfg.queryProp("@url"); - - if( cfgURL && *cfgURL ) - { - StringBuffer protocol, name, pw, host, port, path; - Utils::SplitURL( cfgURL, protocol, name, pw, host, port, path ); - - if( protocol.length()>0 && host.length()>0 ) - { - StringBuffer roxieURL; - url.append( protocol ); - url.append( "://" ); - - if( roxieClient && user.length()>0 && password.length()>0 ) - { - url.appendf("%s:%s@", user.str(), password.str()); - } - - url.append(host); - - if( port.length()>0 ) - { - url.appendf(":%s", port.str()); - } - - if( path.length()>0 ) - { - url.append(path); - } - - result = true; - } - } - - return result; -} - bool EsdlBindingImpl::usesESDLDefinition(const char * name, int version) { if (!name || !*name) diff --git a/esp/services/esdl_svc_engine/esdl_binding.hpp b/esp/services/esdl_svc_engine/esdl_binding.hpp index 51f855f1fce..c89e50a5e62 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.hpp +++ b/esp/services/esdl_svc_engine/esdl_binding.hpp @@ -210,11 +210,109 @@ class EsdlServiceImpl : public CInterface, implements IEspService void getSoapBody(StringBuffer& out,StringBuffer& soapresp); void getSoapError(StringBuffer& out,StringBuffer& soapresp,const char *,const char *); + /** + * @brief Adjust the contents of the target configuration on a per-transaction basis. + * + * If changes to the shared method configuration are required on a per-transaction basis, + * replace the shared configuration with a copy that contains the updates. This must be + * invoked before `createTargetContext` or `checkCreateEsdlServceScriptContext`, both of which + * use the target configuration. + * + * Gateway elements may require updates on a per-transaction basis. Specifically, the use + * of local secret references must be updated with each transaction because the secret + * values may have changed since a previous use. + * + * @param tgtcfg the shared configuration on input; either the unchanged shared configuration + * or modified copy on output + */ + void adjustTargetConfig(Owned& tgtcfg) const; + virtual bool unsubscribeServiceFromDali() override {return true;} virtual bool subscribeServiceToDali() override {return false;} virtual bool attachServiceToDali() override {return false;} virtual bool detachServiceFromDali() override {return false;} +protected: + static constexpr const char* gwTargetSecretPrefix = "secret:"; + static constexpr const size_t gwTargetSecretPrefixLength = strlen(gwTargetSecretPrefix); + static constexpr const char* gwLocalSecretPrefix = "local-secret:"; + static constexpr const size_t gwLocalSecretPrefixLength = strlen(gwLocalSecretPrefix); + static constexpr const char* gwPassThroughPrefix = "pass-through:"; + static constexpr const size_t gwPassThroughPrefixLength = strlen(gwPassThroughPrefix); + + /** + * @brief Possibly construct a new gateway URL value by resolving a given connection secret + * identifier. + * + * The gateway element is expected to contain a non-empty value for property `@url`. This value + * must begin with gwLocalSecretPrefix and be followed by a secret identification in the form + * of `[ vault-id ":" ] secret-name`. This is assumed to identify connection secret known only + * to the ESP, exceptions will be thrown on failure: + * + * - `gateway` must contain property `@url`; and + * - property '@url` must begin with gwLocalSecretPrefix; and + * - property `@url` must include a secret name, and may include a vault identifier; and + * - the optional vault identifier and required secret name combination must identify a secret + * in the `esp` category; and + * - the secret must grant the requested `permission`, when given, by defining a Boolean + * property with the permission name set to true; and + * - the secret must define a non-empty `url` property; and + * - the secret must define either a non-empty `username` property or a Boolean property + * `insecure` set to to true; and + * - the secret may define a `password` property only if a non-empty `username` property is + * also defined. + * + * Use of local secrets in gateway configurations is preferred to the use of inline URLs, but + * is not considered a best practice. Although it does remove user credentials from the + * configuration file, it increases exposure of the secret data and precludes the use of + * secret-defined tokens and certificates for connection security. + * + * @param gateway property tree that must contain at least a `@url` property + * @param permission name of local secret property controlling use of the secret in a gateway + */ + void resolveGatewayLocalSecret(IPTree& gateway, const char* permission) const; + + /** + * @brief Possibly construct a new gateway URL value by removing embedded user credentials and + * inserting separately configured values. + * + * The gateway element must contain an `@url` property, and may contain `@username` and + * `@password` properties. A password must not be specified in the absence of a username. + * + * Use of inline URLs is strongly discouraged. Secure connections are not possible without + * including user credentials in the configuration. Support is provided for backward + * compatibility purposes only. + * + * Support for a `@roxieClient` gateway property is obsolete. When used with gateway + * configurations, its effect would be to either require both username and password or prevent + * the use of inline credentials. DESDL configurations should define credentials when needed, + * and omit them when not. + * + * @param gateway property tree that should contain at least a `@url` property + */ + void resolveGatewayInlineURL(IPTree &gateway) const; + + /** + * @brief Helper function to assemble a URL with optional inline user credentials. + * + * @param url a base URL on input; a possibly modified URL on output + * @param username optional user identifier + * @param password optional user password + */ + void adjustURL(StringBuffer& url, const char* username, const char* password) const; + + /** + * @brief Implementation of legacy gateway transformation invoked only during preparation of + * published requests. + * + * This can be deprecated once scripts can access resolved URL values. + * + * @param srvcfg the target configuration containing all gateways + * @param forRoxie the transformed gateway structure + * @param altElementName configurable element name used in the transformed gateway structure + */ + void transformGatewaysConfig( IPropertyTree* srvcfg, IPropertyTree* forRoxie, const char* altElementName = nullptr ) const; + private: bool initMaskingEngineDirectory(const char* dir); template @@ -327,8 +425,6 @@ class EsdlBindingImpl : public CHttpSoapBinding int onGetSampleXml(bool isRequest, IEspContext &ctx, CHttpRequest* request, CHttpResponse* response, const char *serv, const char *method); static void splitURLList(const char* urlList, StringBuffer& protocol,StringBuffer& UserName,StringBuffer& Password, StringBuffer& ipportlistbody, StringBuffer& path, StringBuffer& options); - static void transformGatewaysConfig( IPropertyTree* srvcfg, IPropertyTree* forRoxie, const char* altElementName = nullptr ); - static bool makeURL( StringBuffer& url, IPropertyTree& cfg ); bool usesESDLDefinition(const char * name, int version); bool usesESDLDefinition(const char * id); From 44a6f7c644088361c9627fb2440804d188c8fdb3 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Fri, 8 Sep 2023 07:15:46 -0400 Subject: [PATCH 4/9] Review feedback --- esp/services/esdl_svc_engine/esdl_binding.cpp | 67 ++++++++++--------- esp/services/esdl_svc_engine/esdl_binding.hpp | 6 +- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp index d9cfff65fb6..34e6443b3dc 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.cpp +++ b/esp/services/esdl_svc_engine/esdl_binding.cpp @@ -744,23 +744,20 @@ void EsdlServiceImpl::configureTargets(IPropertyTree *cfg, const char *service) IPTree* gateways = methodCfg.queryBranch("Gateways"); if (!gateways) continue; - bool resolveInline = (gateways->getPropBool("resolveInlineURLs") || !isEmptyString(gateways->queryProp("@legacyTransformBranch"))); - Owned gwIt(gateways->getElements("Gateway")); - ForEach(*gwIt) + bool resolveInline = (gateways->getPropBool("@resolveInlineURLs") || !isEmptyString(gateways->queryProp("@legacyTransformBranch"))); + if (resolveInline) { - IPTree& gw = gwIt->query(); - const char* url = gw.queryProp("@url"); - if (isEmptyString(url)) - continue; - if (strncmp(url, gwTargetSecretPrefix, gwTargetSecretPrefixLength) == 0) - continue; // no change required - if (strncmp(url, gwLocalSecretPrefix, gwLocalSecretPrefixLength) == 0) - continue; // change deferred until use - if (strncmp(url, gwPassThroughPrefix, gwPassThroughPrefixLength) == 0) - gw.setProp("@url", url + gwPassThroughPrefixLength); - else if (!resolveInline) - continue; // no change required - resolveGatewayInlineURL(gw); + Owned gwIt(gateways->getElements("Gateway")); + ForEach(*gwIt) + { + IPTree& gw = gwIt->query(); + const char* url = gw.queryProp("@url"); + if (isEmptyString(url)) + continue; + StringBuffer prefix(4, url); + if (strieq(prefix, "http")) + resolveGatewayInlineURL(gw); + } } DBGLOG("Method %s configured", method); @@ -1363,10 +1360,10 @@ void EsdlServiceImpl::adjustTargetConfig(Owned& tgtcfg) const tgtcfg.setown(createPTreeFromIPT(tgtcfg)); const char* qt = tgtcfg->queryProp("@querytype"); bool published = (qt && (strieq(qt, "roxie") || strieq(qt, "wsecl"))); - const char* permission = (published ? "allowPublishedGatewayUsage" : "allowUnpublishedGatewayUsage"); + const char* requiredUsage = (published ? "allowPublishedGatewayUsage" : "allowUnpublishedGatewayUsage"); Owned gwsToResolve(tgtcfg->getElements(gwLocalSecretXPath)); ForEach(*gwsToResolve) - resolveGatewayLocalSecret(gwsToResolve->query(), permission); + resolveGatewayLocalSecret(gwsToResolve->query(), requiredUsage); } } @@ -1385,27 +1382,33 @@ void EsdlServiceImpl::resolveGatewayLocalSecret(IPTree& gateway, const char* per switch (tokens.ordinality()) { case 1: - secret.setown(getSecret("esp", tokens.item(0))); + if (strncmp(tokens.item(0), "http-connect-", 13) != 0) + secret.setown(getSecret("esp", VStringBuffer("http-connect-%s", tokens.item(0)))); + else + secret.setown(getSecret("esp", tokens.item(0))); break; case 2: - secret.setown(getVaultSecret("esp", tokens.item(0), tokens.item(1))); + if (strncmp(tokens.item(1), "http-connect-", 13) != 0) + secret.setown(getVaultSecret("esp", tokens.item(0), VStringBuffer("http-connect-%s", tokens.item(1)))); + else + secret.setown(getVaultSecret("esp", tokens.item(0), tokens.item(1))); break; default: throw makeStringExceptionV(-1, "gateway %s: '%s' is not of the form \"[ vault-id ':' ] secret-name\"", name, identification); } if (!secret) - throw makeStringExceptionV(-1, "gateway %s: '%s' does not identify an 'esp' category secret", name, identification); + throw makeStringExceptionV(-1, "gateway %s: 'esp' category secret '%s' not found", name, identification); if (!isEmptyString(permission) && !secret->getPropBool(permission)) throw makeStringExceptionV(-1, "gateway %s: '%s' does not grant '%s' permission", name, identification, permission); if (!secret->getProp("url", url.clear()) || url.isEmpty()) throw makeStringExceptionV(-1, "gateway %s: '%s' missing required 'url' property; credential-only secrets not supported", name, identification); const char* username = secret->queryProp("username"); - if (isEmptyString(username) && !secret->getPropBool("insecure")) - throw makeStringExceptionV(-1, "gateway %s: '%s' missing expected 'username' property; set 'insecure` property to 'true' if credentials are not required", name, identification); + if (isEmptyString(username) && !secret->getPropBool("omitCredentials")) + throw makeStringExceptionV(-1, "gateway %s: '%s' missing expected 'username' property; set 'omitCredentials` property to 'true' if credentials are not required", name, identification); const char* password = secret->queryProp("password"); if (isEmptyString(username) && password) throw makeStringExceptionV(-1, "gateaay %s: '%s' invalid use of password without username", name, identification); - adjustURL(url, username, password); + updateURLCredentials(url, username, password); gateway.setProp("@url", url); } @@ -1425,16 +1428,16 @@ void EsdlServiceImpl::resolveGatewayInlineURL(IPTree& gateway) const decrypt(decryptedPassword, password); password = decryptedPassword; } - adjustURL(url, username, password); + updateURLCredentials(url, username, password); gateway.setProp("@url", url); } -void EsdlServiceImpl::adjustURL(StringBuffer& url, const char* username, const char* password) const +void EsdlServiceImpl::updateURLCredentials(StringBuffer& url, const char* username, const char* password) const { StringBuffer scheme, usernameSink, passwordSink, host, port, path; Utils::SplitURL(url, scheme, usernameSink, passwordSink, host, port, path); - if (scheme.length() > 0 && host.length() > 0) + if (scheme.length() && host.length()) { StringBuffer userinfo; if (!isEmptyString(username)) @@ -1450,12 +1453,12 @@ void EsdlServiceImpl::adjustURL(StringBuffer& url, const char* username, const c url.clear(); url.append(scheme); url.append("://"); - if (userinfo.length()>0 ) - url.appendf("%s@", userinfo.str()); + if (userinfo.length()) + url.append(userinfo).append('@'); url.append(host); - if (port.length() > 0) - url.appendf(":%s", port.str()); - if (path.length() > 0) + if (port.length()) + url.append(':').append(port); + if (path.length()) url.append(path); } } diff --git a/esp/services/esdl_svc_engine/esdl_binding.hpp b/esp/services/esdl_svc_engine/esdl_binding.hpp index c89e50a5e62..4091fe5f8bd 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.hpp +++ b/esp/services/esdl_svc_engine/esdl_binding.hpp @@ -233,12 +233,8 @@ class EsdlServiceImpl : public CInterface, implements IEspService virtual bool detachServiceFromDali() override {return false;} protected: - static constexpr const char* gwTargetSecretPrefix = "secret:"; - static constexpr const size_t gwTargetSecretPrefixLength = strlen(gwTargetSecretPrefix); static constexpr const char* gwLocalSecretPrefix = "local-secret:"; static constexpr const size_t gwLocalSecretPrefixLength = strlen(gwLocalSecretPrefix); - static constexpr const char* gwPassThroughPrefix = "pass-through:"; - static constexpr const size_t gwPassThroughPrefixLength = strlen(gwPassThroughPrefix); /** * @brief Possibly construct a new gateway URL value by resolving a given connection secret @@ -299,7 +295,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService * @param username optional user identifier * @param password optional user password */ - void adjustURL(StringBuffer& url, const char* username, const char* password) const; + void updateURLCredentials(StringBuffer& url, const char* username, const char* password) const; /** * @brief Implementation of legacy gateway transformation invoked only during preparation of From bb7f7094a3eb83d91dc1ee44304ea3bde63b7fca Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Tue, 19 Sep 2023 15:24:35 -0400 Subject: [PATCH 5/9] Refactor - Anticipate alternate update requirements, such as for database access. - Prevent two versions of one secret from being used in one transaction. --- esp/services/esdl_svc_engine/esdl_binding.cpp | 437 ++++++++++++------ esp/services/esdl_svc_engine/esdl_binding.hpp | 357 +++++++++++--- 2 files changed, 588 insertions(+), 206 deletions(-) diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp index 34e6443b3dc..e807d4350c9 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.cpp +++ b/esp/services/esdl_svc_engine/esdl_binding.cpp @@ -108,6 +108,8 @@ struct ReporterHelper }; static ReporterHelper g_reporterHelper; +static bool hasHttpPrefix(const char *addr); + /* * trim xpath at first instance of element */ @@ -674,6 +676,7 @@ void EsdlServiceImpl::configureTargets(IPropertyTree *cfg, const char *service) m_methodAccessMaps.kill(); m_returnSchemaLocationOnOK = definition_cfg->getPropBool("@" ANNOTATION_RETURNSCHEMALOCATION_ONOK); + m_methodGatewaysCache.clear(); Owned iter = m_pServiceMethodTargets->getElements("Target"); ForEach(*iter) @@ -736,28 +739,56 @@ void EsdlServiceImpl::configureTargets(IPropertyTree *cfg, const char *service) else configureUrlMethod(method, methodCfg); - // Relocate legacy gateway inline URL resolution from prepareFinalRequest. To deprecate - // that hard-coded logic, scripts need access to resolved URL values in the script - // context's 'target' section. Resolve once per method here, instead of per request in - // adjustTargetConfig. + // Analyze defined gateways to identify use of local secrets and/or inline URLs. Cache + // relevant data for reuse on a per-transaction basis. IPTree* gateways = methodCfg.queryBranch("Gateways"); if (!gateways) continue; - bool resolveInline = (gateways->getPropBool("@resolveInlineURLs") || !isEmptyString(gateways->queryProp("@legacyTransformBranch"))); - if (resolveInline) + GatewaysCacheEntry& gce = m_methodGatewaysCache[method]; + std::set uniqueNames; + bool updateInlines = gateways->getPropBool("@updateInlineUrls"); + bool doLegacyTransform = !isEmptyString(gateways->queryProp("@legacyTransformTarget")); + Secrets secrets; + Owned gwIt(gateways->getElements("Gateway")); + ForEach(*gwIt) { - Owned gwIt(gateways->getElements("Gateway")); - ForEach(*gwIt) + IPTree& gw = gwIt->query(); + StringBuffer name(gw.queryProp("@name")); + if (name.trim().isEmpty()) + { + IWARNLOG("Ignoring method '%s' gateway missing or empty name", method); + continue; + } + std::string key(name.str()); + if (uniqueNames.count(key)) + throw makeStringExceptionV(-1, "method '%s' duplicates gateway name '%s'", method, name.str()); + uniqueNames.insert(key); + const char* url = gw.queryProp("@url"); + if (isEmptyString(url)) { - IPTree& gw = gwIt->query(); - const char* url = gw.queryProp("@url"); - if (isEmptyString(url)) - continue; - StringBuffer prefix(4, url); - if (strieq(prefix, "http")) - resolveGatewayInlineURL(gw); + DBGLOG("Ignoring method '%s' gateway '%s' missing url", method, name.str()); + continue; } + Owned handler; + if (strncmp(url, gwLocalSecretPrefix, gwLocalSecretPrefixLength) == 0) + { + handler.setown(createLocalSecretGateway(gw, name, url)); + GatewayUpdaters updaters; + Owned updater(handler->getUpdater(updaters, secrets)); + updater.clear(); + gce.targetContext[key].set(handler.get()); + } + else if (hasHttpPrefix(url)) + { + handler.setown(createInlineGateway(gw, name, url)); + if (updateInlines) + gce.targetContext[key].set(handler.get()); + } + else + continue; + if (doLegacyTransform) + gce.legacyTransform[key].set(handler.get()); } DBGLOG("Method %s configured", method); @@ -1139,8 +1170,6 @@ void EsdlServiceImpl::handleServiceRequest(IEspContext &context, } else { - adjustTargetConfig(tgtcfg); - //Future: support transforms for all transaction types by moving scripts and script processing up scriptContext.setown(checkCreateEsdlServiceScriptContext(context, srvdef, mthdef, tgtcfg, req)); @@ -1348,134 +1377,46 @@ void EsdlServiceImpl::getSoapError( StringBuffer& out, out.clear().append(finger-start,start); } -void EsdlServiceImpl::adjustTargetConfig(Owned& tgtcfg) const -{ - static const VStringBuffer gwLocalSecretXPath("Gateways/Gateway[@url='%s*']", gwLocalSecretPrefix); - - if (!tgtcfg) - return; - - if (tgtcfg->hasProp(gwLocalSecretXPath)) - { - tgtcfg.setown(createPTreeFromIPT(tgtcfg)); - const char* qt = tgtcfg->queryProp("@querytype"); - bool published = (qt && (strieq(qt, "roxie") || strieq(qt, "wsecl"))); - const char* requiredUsage = (published ? "allowPublishedGatewayUsage" : "allowUnpublishedGatewayUsage"); - Owned gwsToResolve(tgtcfg->getElements(gwLocalSecretXPath)); - ForEach(*gwsToResolve) - resolveGatewayLocalSecret(gwsToResolve->query(), requiredUsage); - } -} - -void EsdlServiceImpl::resolveGatewayLocalSecret(IPTree& gateway, const char* permission) const +EsdlServiceImpl::IUpdatableGateway* EsdlServiceImpl::createLocalSecretGateway(const IPTree& gw, const char* gwName, const char* gwUrl) const { - const char* name = gateway.queryProp("@name"); - StringBuffer url(gateway.queryProp("@url")); - if (url.isEmpty()) - throw makeStringExceptionV(-1, "gateway %s: missing url property", name); - if (strncmp(url, gwLocalSecretPrefix, gwLocalSecretPrefixLength)) - throw makeStringExceptionV(-1, "gateway %s: expected '%s...'; got '%s'", name, gwLocalSecretPrefix, url.str()); - const char* identification = url.str() + gwLocalSecretPrefixLength; - StringArray tokens; - tokens.appendList(identification, ":", true); - Owned secret; - switch (tokens.ordinality()) - { - case 1: - if (strncmp(tokens.item(0), "http-connect-", 13) != 0) - secret.setown(getSecret("esp", VStringBuffer("http-connect-%s", tokens.item(0)))); - else - secret.setown(getSecret("esp", tokens.item(0))); - break; - case 2: - if (strncmp(tokens.item(1), "http-connect-", 13) != 0) - secret.setown(getVaultSecret("esp", tokens.item(0), VStringBuffer("http-connect-%s", tokens.item(1)))); - else - secret.setown(getVaultSecret("esp", tokens.item(0), tokens.item(1))); - break; - default: - throw makeStringExceptionV(-1, "gateway %s: '%s' is not of the form \"[ vault-id ':' ] secret-name\"", name, identification); - } - if (!secret) - throw makeStringExceptionV(-1, "gateway %s: 'esp' category secret '%s' not found", name, identification); - if (!isEmptyString(permission) && !secret->getPropBool(permission)) - throw makeStringExceptionV(-1, "gateway %s: '%s' does not grant '%s' permission", name, identification, permission); - if (!secret->getProp("url", url.clear()) || url.isEmpty()) - throw makeStringExceptionV(-1, "gateway %s: '%s' missing required 'url' property; credential-only secrets not supported", name, identification); - const char* username = secret->queryProp("username"); - if (isEmptyString(username) && !secret->getPropBool("omitCredentials")) - throw makeStringExceptionV(-1, "gateway %s: '%s' missing expected 'username' property; set 'omitCredentials` property to 'true' if credentials are not required", name, identification); - const char* password = secret->queryProp("password"); - if (isEmptyString(username) && password) - throw makeStringExceptionV(-1, "gateaay %s: '%s' invalid use of password without username", name, identification); - updateURLCredentials(url, username, password); - gateway.setProp("@url", url); + const char* secretUsage = gw.queryProp("@secretUsage"); + if (isEmptyString(secretUsage) || strieq(secretUsage, "http")) + return new CHttpConnectGateway(gw, gwName, gwUrl); + // TODO: add support for additional updaters as needed + else + throw makeStringExceptionV(-1, "gateway %s: unexpected secretUsage '%s'", gw.queryProp("@name"), secretUsage); } -void EsdlServiceImpl::resolveGatewayInlineURL(IPTree& gateway) const +EsdlServiceImpl::IUpdatableGateway* EsdlServiceImpl::createInlineGateway(const IPTree& gw, const char* gwName, const char* gwUrl) const { - const char* name = gateway.queryProp("@name"); - StringBuffer url(gateway.queryProp("@url")); - if (url.isEmpty()) - throw makeStringExceptionV(-1, "gateway %s: missing required '@url' property", name); - const char* username = gateway.queryProp("@username"); - const char* password = gateway.queryProp("@password"); - if (isEmptyString(username) && password) - throw makeStringExceptionV(-1, "gateway %s: invalid credentials - password without username", name); - StringBuffer decryptedPassword; - if (!isEmptyString(password)) - { - decrypt(decryptedPassword, password); - password = decryptedPassword; - } - updateURLCredentials(url, username, password); - gateway.setProp("@url", url); + // TODO: add support for addiitional URL options if needed + return new CLegacyUrlGateway(gw, gwName, gwUrl); } -void EsdlServiceImpl::updateURLCredentials(StringBuffer& url, const char* username, const char* password) const +EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::getGatewayUpdater(IPTree& gw, const UpdatableGateways& updatables, GatewayUpdaters& updaters, Secrets& secrets) const { - StringBuffer scheme, usernameSink, passwordSink, host, port, path; - Utils::SplitURL(url, scheme, usernameSink, passwordSink, host, port, path); - - if (scheme.length() && host.length()) - { - StringBuffer userinfo; - if (!isEmptyString(username)) - { - encodeUrlUseridPassword(userinfo, username); - if (password && *password) // TODO: remove '&& *password' to distinguish empty from omitted passwords - { - userinfo.append(':'); - encodeUrlUseridPassword(userinfo, password); - } - } - - url.clear(); - url.append(scheme); - url.append("://"); - if (userinfo.length()) - url.append(userinfo).append('@'); - url.append(host); - if (port.length()) - url.append(':').append(port); - if (path.length()) - url.append(path); - } + const char* name = gw.queryProp("@name"); + if (isEmptyString(name)) + return nullptr; + std::string key(name); + UpdatableGateways::const_iterator it = updatables.find(key); + if (updatables.end() == it) + return nullptr; + return it->second->getUpdater(updaters, secrets); } -void EsdlServiceImpl::transformGatewaysConfig( IPropertyTree* srvcfg, IPropertyTree* forRoxie, const char* altElementName ) const +void EsdlServiceImpl::transformGatewaysConfig( IPTreeIterator* inputs, IPropertyTree* forRoxie, const char* altElementName ) const { // Do we need to handle 'local FQDN'? It doesn't appear to be in the // Gateway element. Not sure where it's set but the RemoteNSClient // references it in RemoteNSFactory.cpp translateGateway() and modifies // resulting gateway URL if it's set. - if( srvcfg && forRoxie) + if (forRoxie) { const char* treeName = (!isEmptyString(altElementName) ? altElementName : "Gateway"); - Owned cfgIter = srvcfg->getElements("Gateways/Gateway"); - ForEach(*cfgIter) + ForEach(*inputs) { - IPropertyTree& cfgGateway = cfgIter->query(); + IPropertyTree& cfgGateway = inputs->query(); StringBuffer url(cfgGateway.queryProp("@url")); StringBuffer service(cfgGateway.queryProp("@name")); Owned gw = createPTree(treeName, false); @@ -1837,29 +1778,70 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, reqProcessed.append("<").append(tgtQueryName).append(">"); reqProcessed.appendf("<_TransactionId>%s", context.queryTransactionID()); + GatewaysCache::const_iterator mit = m_methodGatewaysCache.find(mthName); + GatewayUpdaters updaters; + Secrets secrets; if (tgtctx) + { + // The target context is based on a copy of the target configuration. One copy per + // transaction. It will have already been copied into the script context, so local + // secret and inline URL resolutions can be made in the given property tree. + if (mit != m_methodGatewaysCache.end() && !mit->second.targetContext.empty()) + { + IPTree* ctxGateways = tgtctx->queryBranch("Gateways"); + if (ctxGateways) + { + Owned it(ctxGateways->getElements("Gateway")); + ForEach(*it) + { + IPTree& gw = it->query(); + Owned updater(getGatewayUpdater(gw, mit->second.targetContext, updaters, secrets)); + if (updater) + updater->updateGateway(gw, "allowPublishedGatewayUsage"); + } + } + } toXML(tgtctx.get(), reqProcessed); + } reqProcessed.append(reqcontent.str()); reqProcessed.append(""); // Transform gateways - auto cfgGateways = tgtcfg->queryBranch("Gateways"); + IPTree* cfgGateways = tgtcfg->queryBranch("Gateways"); if (cfgGateways) { - auto baseXpath = cfgGateways->queryProp("@legacyTransformTarget"); - if (!isEmptyString(baseXpath)) + StringBuffer xpath(cfgGateways->queryProp("@legacyTransformTarget")); + if (!xpath.isEmpty()) { + Owned it; + if (mit != m_methodGatewaysCache.end() && !mit->second.legacyTransform.empty()) + { + // The target configuration is shared by all transactions. Local secret and + // inline URL resolutions must be made to a copy of the gateways, to avoid + // contaminating values used in subsequent transactions. + Owned copy(createPTreeFromIPT(cfgGateways)); + it.setown(copy->getElements("Gateway")); + ForEach(*it) + { + IPTree& gw = it->query(); + Owned updater(getGatewayUpdater(gw, mit->second.legacyTransform, updaters, secrets)); + if (updater) + updater->updateGateway(gw, "allowPublishedGatewayUsage"); + } + } + else + it.setown(cfgGateways->getElements("Gateway")); + Owned gws = createPTree("gateways", 0); // Temporarily add the closing tag so we have valid // XML to transform the gateways Owned soapTree = createPTreeFromXMLString(reqProcessed.append(""), ipt_ordered); - StringBuffer xpath(baseXpath); StringBuffer rowName(cfgGateways->queryProp("@legacyRowName")); if (rowName.isEmpty()) rowName.append("row"); - transformGatewaysConfig(tgtcfg, gws, rowName); + transformGatewaysConfig(it, gws, rowName); xpath.replaceString("{$query}", tgtQueryName); xpath.replaceString("{$method}", mthName); xpath.replaceString("{$service}", srvdef.queryName()); @@ -1929,6 +1911,194 @@ EsdlServiceImpl::~EsdlServiceImpl() } } +IPTree* EsdlServiceImpl::Secrets::getVaultSecret(const char* category, const char* vaultId, const char* name) +{ + Owned secret(lookup(category, vaultId, name)); + if (!secret) + { + secret.setown(::getVaultSecret(category, vaultId, name)); + if (secret) + store(*secret, category, vaultId, name); + } + return secret.getClear(); +} + +IPTree* EsdlServiceImpl::Secrets::getSecret(const char* category, const char* name) +{ + Owned secret(lookup(category, "", name)); + if (!secret) + { + secret.setown(::getVaultSecret(category, "", name)); + if (secret) + store(*secret, category, "", name); + } + return secret; +} + +IPTree* EsdlServiceImpl::Secrets::lookup(const char* category, const char* vaultId, const char* name) const +{ + Key key = std::make_tuple(category ? category : "", vaultId ? vaultId : "", name ? name : ""); + Cache::const_iterator it = cache.find(key); + if (it != cache.end()) + return it->second.getLink(); + return nullptr; +} + +void EsdlServiceImpl::Secrets::store(IPTree& secret, const char* category, const char* vaultId, const char* name) +{ + Key key = std::make_tuple(category ? category : "", vaultId ? vaultId : "", name ? name : ""); + cache[key].set(&secret); +} + +EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, Secrets& secrets) const +{ + GatewayUpdaters::iterator it = updaters.find(updatersKey); + if (it != updaters.end()) + return it->second.getLink(); + Owned spawned(getUpdater(secrets)); + updaters[updatersKey].setown(spawned.getLink()); + return spawned.getClear(); +} + +EsdlServiceImpl::CUpdatableGateway::CUpdatableGateway(const char* gwName) +{ + updatersKey.assign(gwName); +} + +bool EsdlServiceImpl::CUpdatableGateway::updateURLCredentials(StringBuffer& url, const char* username, const char* password) const +{ + StringBuffer original(url), scheme, usernameSink, passwordSink, host, port, path; + Utils::SplitURL(original, scheme, usernameSink, passwordSink, host, port, path); + + if (scheme.length() && host.length()) + { + StringBuffer userinfo; + if (!isEmptyString(username)) + { + encodeUrlUseridPassword(userinfo, username); + if (password && *password) // TODO: remove '&& *password' to distinguish empty from omitted passwords + { + userinfo.append(':'); + encodeUrlUseridPassword(userinfo, password); + } + } + + url.clear(); + url.append(scheme); + url.append("://"); + if (userinfo.length()) + url.append(userinfo).append('@'); + url.append(host); + if (port.length()) + url.append(':').append(port); + if (path.length()) + url.append(path); + return !streq(original, url); + } + return false; +} + +EsdlServiceImpl::CLegacyUrlGateway* EsdlServiceImpl::CLegacyUrlGateway::getUpdater(Secrets&) const +{ + return LINK(const_cast(this)); +} + +void EsdlServiceImpl::CLegacyUrlGateway::updateGateway(IPTree& gw, const char*) +{ + gw.setProp("@url", url); +} + +EsdlServiceImpl::CLegacyUrlGateway::CLegacyUrlGateway(const IPTree& gw, const char* gwName, const char* gwUrl) + : CUpdatableGateway(gwName) + , url(gwUrl) +{ + const char* username = gw.queryProp("@username"); + const char* password = gw.queryProp("@password"); + if (isEmptyString(username) && password) + throw makeStringExceptionV(-1, "gateway %s: invalid credentials - password without username", gwName); + StringBuffer decrypted; + if (!isEmptyString(password)) + { + decrypt(decrypted, password); + password = decrypted; + } + updateURLCredentials(url, username, password); +} + +void EsdlServiceImpl::CLocalSecretGateway::CUpdater::updateGateway(IPTree& gw, const char* requiredUsage) +{ + if (!secret) + throw makeStringExceptionV(-1, "gateway %s: 'esp' category secret '%s' not found", "?", entry->secretId.str()); + entry->doUpdate(gw, *secret, requiredUsage); +} + +EsdlServiceImpl::CLocalSecretGateway::CUpdater::CUpdater(const CLocalSecretGateway& _entry, Secrets& secrets) +{ + entry.set(&_entry); + if (entry->vaultId.isEmpty()) + secret.setown(secrets.getSecret("esp", entry->secretName)); + else + secret.setown(secrets.getVaultSecret("esp", entry->vaultId.str(), entry->secretName)); +} + +EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CLocalSecretGateway::getUpdater(Secrets& secrets) const +{ + return new CUpdater(*this, secrets); +} + +EsdlServiceImpl::CLocalSecretGateway::CLocalSecretGateway(const IPTree& gw, const char* gwName, const char* gwUrl, const char* classPrefix) + : CUpdatableGateway(gwName) +{ + secretId.append(gwUrl + gwLocalSecretPrefixLength); + StringArray tokens; + tokens.appendList(secretId, ":", true); + const char* vaultToken = nullptr; + const char* secretToken = nullptr; + switch (tokens.ordinality()) + { + case 1: // secret name without vault name + secretToken = tokens.item(0); + break; + case 2: // secret name after vault name + vaultToken = tokens.item(0); + secretToken = tokens.item(1); + break; + default: + throw makeStringExceptionV(-1, "gateway %s: '%s' is not of the form \"[ vault-id ':' ] secret-name\"", gwName, secretId.str()); + } + + if (!isEmptyString(vaultToken)) + vaultId.set(vaultToken); + secretName.append(classPrefix).append(secretToken); +} + +void EsdlServiceImpl::CLocalSecretGateway::doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const +{ + if (!isEmptyString(requiredUsage) && !secret.getPropBool(requiredUsage)) + throw makeStringExceptionV(-1, "gateway %s: '%s' does not allow '%s' usage", updatersKey.c_str(), secretId.str(), requiredUsage); +} + +void EsdlServiceImpl::CHttpConnectGateway::doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const +{ + CLocalSecretGateway::doUpdate(gw, secret, requiredUsage); + StringBuffer url; + if (!secret.getProp("url", url.clear()) || url.isEmpty()) + throw makeStringExceptionV(-1, "gateway %s: '%s' missing required 'url' property; credential-only secrets not supported", gw.queryProp("@name"), secretId.str()); + const char* username = secret.queryProp("username"); + if (isEmptyString(username) && !secret.getPropBool("omitCredentials")) + throw makeStringExceptionV(-1, "gateway %s: '%s' missing expected 'username' property; set 'omitCredentials` property to 'true' if credentials are not required", gw.queryProp("@name"), secretId.str()); + const char* password = secret.queryProp("password"); + if (isEmptyString(username) && password) + throw makeStringExceptionV(-1, "gateway %s: '%s' invalid use of password without username", gw.queryProp("@name"), secretId.str()); + updateURLCredentials(url, username, password); + gw.setProp("@url", url); +} + +EsdlServiceImpl::CHttpConnectGateway::CHttpConnectGateway(const IPTree& gw, const char* gwName, const char* gwUrl) + : CLocalSecretGateway(gw, gwName, gwUrl, "http-connect-") +{ +} + EsdlBindingImpl::EsdlBindingImpl() { m_pESDLService = nullptr; @@ -4011,7 +4181,6 @@ int EsdlBindingImpl::onGetRoxieBuilder(CHttpRequest* request, CHttpResponse* res StringBuffer reqcontent; getRequestContent(*context, reqcontent, request, srvname, mthname, ns, ROXIEREQ_FLAGS); - m_pESDLService->adjustTargetConfig(tgtcfg); tgtctx.setown( m_pESDLService->createTargetContext(*context, tgtcfg, *defsrv, *defmth, req_pt)); Owned scriptContext = m_pESDLService->checkCreateEsdlServiceScriptContext(*context, *defsrv, *defmth, tgtcfg.get(), req_pt); diff --git a/esp/services/esdl_svc_engine/esdl_binding.hpp b/esp/services/esdl_svc_engine/esdl_binding.hpp index 4091fe5f8bd..384f74f7711 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.hpp +++ b/esp/services/esdl_svc_engine/esdl_binding.hpp @@ -72,6 +72,205 @@ typedef int (*cpp_service_method_t)(const char* CtxXML, const char* ReqXML, Stri class EsdlServiceImpl : public CInterface, implements IEspService { +private: + /** + * @brief Cache of secrets accessed as part of transaction processing. + * + * In the context of a single transaction, multiple requests for the same secret should always + * return the same secret data for each request. Cached data does not expire and there is no + * option to reload data. + * + * TODO: Refactor to support additional use cases + * - Provide ESDL script operation support so mysql (now) and http-post-xml (eventually) + * can benefit secret reuse. + * - Encapsulate non-jsecrets access to support non-standard secret sources, unless + * jsecrets is changed to provide this encapsulation. + */ + class Secrets + { + private: + using Key = std::tuple; + using Cache = std::map>; + Cache cache; + public: + IPTree* getVaultSecret(const char* category, const char* vaultId, const char* name); + IPTree* getSecret(const char* category, const char* name); + protected: + IPTree* lookup(const char* category, const char* vaultId, const char* name) const; + void store(IPTree& secret, const char* category, const char* vaultId, const char* name); + }; + +private: + /** + * @brief Abstraction of a per-transaction gateway updater. + * + * During service load, the method configurations are parsed for Gateways/Gateway elements. + * Matching elements containing both `@name` and `@url` properties may be updated when + * preparing backend service requests. Updates may inject user credentials into a configured + * URL or assemble a new URL from a secret. + * + * Each instance contains the information and logic to update a single Gateway property tree + * as needed. Only one instance per gateay should be created per transaction. No instance + * should be created for gateways that do not require updates. + */ + interface IGatewayUpdater : public IInterface + { + virtual void updateGateway(IPTree& gw, const char* requiredUsage) = 0; + }; + using GatewayUpdaters = std::map>; + + /** + * @brief Abstraction of a per-service-method gateway update handler. + * + * During service load, the method configurations are parsed for Gateways/Gateway elements. + * Matching elements containing both `@name` and `@url` properties may be updated when + * preparing backend service requests. Updates may inject user credentials into a configured + * URL or assemble a new URL from a secret. + * + * Implementations are expected to retain all gateway configuration values at load time to + * enable an `IGatewayUpdater` instance to update a copy of the gateway element during + * backend request preparation. + * + * - If the `Gateway` element is a self-contained gateway specification, the retained data + * may be the updated values. Inline URLs fit this description. + * - If the `Gateway` element refers to external data that could change between service load + * and a transaction, the retained data must be sufficient to retrieve current values on + * demand. Local secrets fit this description. + * + * Implementations may assume that each gateway configuration will include both `@name` and + * `@url` properties. No other assumptions are valid. + */ + interface IUpdatableGateway : public IInterface + { + virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, Secrets& secrets) const = 0; + }; + using UpdatableGateways = std::map>; + + /** + * @brief Abstract extension of `IUpdatableGateway` with some standardized behaviors. + * + * - Subclasses supply the updater map key used to locate an existing updater or store a new + * updater. It is assumed the key given is the value of `Gateway/@name`. This value must not + * be null. + * - An existing updater in a given map of updaters will always be reused before creating a + * new updater. + * - Insertion (or replacement) of user credentials in a URL string is available to any + * subclass that needs it.. + * + * All extensions must implement `getUpdater(Secrets&)`. In some cases this may entail creation + * of a new updater instance. In other cases a single instance may be reused. + */ + class CUpdatableGateway : public CInterfaceOf + { + public: + virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, Secrets& secrets) const override; + protected: + std::string updatersKey; + protected: + CUpdatableGateway(const char* gwName); + virtual IGatewayUpdater* getUpdater(Secrets& secrets) const = 0; + bool updateURLCredentials(StringBuffer& url, const char* username, const char* password) const; + }; + + /** + * @brief Concrete extension of both `CUpdatableGateway` and `IGatewayUpdater` for handling + * legecy URL gateways + * + * - User credentials embedded in `Gateway/@url` are dropped. + * - `Gateway/@username` may specify a user name to embed in an updated URL. + * - If `Gateway/@username` is given, `Gateway/@password` may specify an exncrypted password + * value to be decrypted and embedded in an updated URL. + * - It is an error to specify `Gateway/@password` without `Gateway/@username`. + */ + class CLegacyUrlGateway : public CUpdatableGateway, public IGatewayUpdater + { + protected: // CUpdatableGateway + virtual CLegacyUrlGateway* getUpdater(Secrets&) const override; + public: // IGatewayUpdater + virtual void updateGateway(IPTree& gw, const char*) override; + protected: + StringBuffer url; + public: + IMPLEMENT_IINTERFACE_USING(CUpdatableGateway); + CLegacyUrlGateway(const IPTree& gw, const char* gwName, const char* gwUrl); + }; + + /** + * @brief Concrete extension of `CUpdatableGateway` for handling local secrets that is always + * extended with secret usage-specific logic. + * + * - Ensures `Gateway/@url` matches the pattern `"local-secret:" [ vault-id ":" ] secret-name".` + * - Defines a standard updater separating dynamically changing secrets from the configuration. + * - Enforces a requirement for a local secret to explicitly allow its data to be shared with + * a backend roxie service. + */ + class CLocalSecretGateway : public CUpdatableGateway + { + protected: + class CUpdater : public CInterfaceOf + { + friend class CLocalSecretGateway; + public: // IGatewayUpdater + virtual void updateGateway(IPTree& gw, const char* requiredUsage) override; + protected: + Linked entry; + Owned secret; + public: + CUpdater(const CLocalSecretGateway& _entry, Secrets& secrets); + }; + protected: // CUpdatableGateway + virtual IGatewayUpdater* getUpdater(Secrets& secrets) const override; + protected: + StringBuffer secretId; + StringAttr vaultId; + StringBuffer secretName; + public: + CLocalSecretGateway(const IPTree& gw, const char* gwName, const char* gwUrl, const char* classPrefix); + protected: + virtual void doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const; + }; + + /** + * @brief Concete extension of `CLocalSecretGateway` to construct a new `Gateway/@url` value + * from secret-defined values. + * + * - A secret name must begin with "http-connect-". Configurations should omit this prefix, + * but its presence is acceptable. + * - A secret must satisfy the requirements defined in `CLocalSecretGateway`. + * - A secret must define a non-empty `url` property. + * - A secret must define either a non-empty `username` property or set the `omitCredentials` + * property to true. + * - A secret may define a `password` property if `username` is also defined. + * - A secret must not define a `password` property if `username` is not defined. + */ + class CHttpConnectGateway : public CLocalSecretGateway + { + protected: // CLocalSecretGateway + void doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const override; + public: + CHttpConnectGateway(const IPTree& gw, const char* gwName, const char* gwUrl); + }; + + /** + * @brief Storage of updateable gateway handlers for a single method. + * + * Gateway updates may occur in either or both of two locations within a backend + * service request. The storage layout must enable support for both locations. + * + * 1. The optional inclusion of a methods binding definition that does not exclude defined + * gateways, also known as the target context. Target context inclusion must update all + * gateways referring to local secrets. Target context inclusion must not, for backward + * compatibility, update any gateways referring to inline URLs. + * 2. The optional request for "legacy gateway transformation". Legacy transformations must + * update all gateways referring to local secrets and inline URLs. + */ + struct GatewaysCacheEntry + { + UpdatableGateways targetContext; + UpdatableGateways legacyTransform; + }; + using GatewaysCache = std::map; + private: inline Owned& loggingManager() { return m_oDynamicLoggingManager ? m_oDynamicLoggingManager : m_oStaticLoggingManager; } inline Owned& maskingEngine() { return m_oDynamicMaskingEngine ? m_oDynamicMaskingEngine : m_oStaticMaskingEngine; } @@ -101,6 +300,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService Owned javaPluginDll; #endif Owned javaplugin; + GatewaysCache m_methodGatewaysCache; public: StringBuffer m_espServiceType; @@ -201,6 +401,59 @@ class EsdlServiceImpl : public CInterface, implements IEspService virtual void esdl_log(IEspContext &context, IEsdlDefService &srvdef, IEsdlDefMethod &mthdef, IPropertyTree *tgtcfg, IPropertyTree *tgtctx, IPropertyTree *req_pt, const char *xmlresp, const char *logdata, unsigned int timetaken){} virtual void processHeaders(IEspContext &context, IEsdlDefService &srvdef, IEsdlDefMethod &mthdef, const char *ns, StringBuffer &req, StringBuffer &headers){}; virtual void processRequest(IEspContext &context, IEsdlDefService &srvdef, IEsdlDefMethod &mthdef, const char *ns, StringBuffer &req) {}; + + /** + * @brief Assemble a SOAP request to be sent to a backend service. + * + * Assembly includes construction of an initial SOAP request and processing backend request + * ESDL script entry point transforms, if any exist. + * + * Initial request construction of all backend requests wraps the given request content in a + * SOAP envelope and SOAP body. For published requests, where `isroxie` is true, special + * handling for gateways is provided: + * + * - For `Gateways/Gateway` elements found in `tgtctx`: + * - Elements with local secret references are updated to replace `@url` with secret- + * defined data. + * - Elements with inline URL data are updated to insert user credentials into the URL, but + * only if `Gateways/@updateInlineUrls` is true. + * - For `Gateways/Gateway1 elements found in `tgtctx` when `Gateways/@legacyTransformTarget` + * is not empty: + * - Elements with local secret references are updated to replace `@url` with secret- + * defined data. + * - Elements with inline URL data are updated to insert user credentials into the URL. + * - `` becomes + * ``, with `Gateways/@legacyRowName` + * allowing an alternate name for element `row`. + * - The new `gateways` element is placed in the request at the location referred to by + * `Gateways/@legacyTransformTarget`. This XPath supports multiple variable substitutions + * such as: + * - {$query} becomes the value of `Method/@queryname` + * - {$method} becomes the ESDL definition's method name + * - {$service} becomes the ESDL definition's service name + * - {$request} becomes the ESDL definition's method request type + * + * Support for inline URLs in the target configuration, and context which is a subset of the + * configuration, is provided for backward compatibility. Use is strongly discouraged. + * + * Support for local secrets in the target configuration is offered for use cases where the + * ESP cannot identify a connection secret known to the target service. It is an improvement + * on inline URLs, but still involves risk by disclosing secret data. + * + * It is a best practice for all Gateay URLs to be defined in terms of secrets known to the + * target service, eliminating the need for any additional data disclosures in the request. + * + * @param context + * @param scriptContext + * @param tgtcfg + * @param tgtctx + * @param srvdef + * @param mthdef + * @param isroxie + * @param ns + * @param reqcontent + * @param reqProcessed + */ void prepareFinalRequest(IEspContext &context, IEsdlScriptContext *scriptContext, Owned &tgtcfg, Owned &tgtctx, IEsdlDefService &srvdef, IEsdlDefMethod &mthdef, bool isroxie, const char* ns, StringBuffer &reqcontent, StringBuffer &reqProcessed); virtual void createServersList(IEspContext &context, IEsdlDefService &srvdef, IEsdlDefMethod &mthdef, StringBuffer &servers) {}; virtual bool handleResultLogging(IEspContext &espcontext, IEsdlScriptContext *scriptContext, IEsdlDefService &srvdef, IEsdlDefMethod &mthdef, IPropertyTree * reqcontext, IPropertyTree * request, const char * rawreq, const char * rawresp, const char * finalresp, const char * logdata); @@ -210,23 +463,6 @@ class EsdlServiceImpl : public CInterface, implements IEspService void getSoapBody(StringBuffer& out,StringBuffer& soapresp); void getSoapError(StringBuffer& out,StringBuffer& soapresp,const char *,const char *); - /** - * @brief Adjust the contents of the target configuration on a per-transaction basis. - * - * If changes to the shared method configuration are required on a per-transaction basis, - * replace the shared configuration with a copy that contains the updates. This must be - * invoked before `createTargetContext` or `checkCreateEsdlServceScriptContext`, both of which - * use the target configuration. - * - * Gateway elements may require updates on a per-transaction basis. Specifically, the use - * of local secret references must be updated with each transaction because the secret - * values may have changed since a previous use. - * - * @param tgtcfg the shared configuration on input; either the unchanged shared configuration - * or modified copy on output - */ - void adjustTargetConfig(Owned& tgtcfg) const; - virtual bool unsubscribeServiceFromDali() override {return true;} virtual bool subscribeServiceToDali() override {return false;} virtual bool attachServiceToDali() override {return false;} @@ -237,77 +473,54 @@ class EsdlServiceImpl : public CInterface, implements IEspService static constexpr const size_t gwLocalSecretPrefixLength = strlen(gwLocalSecretPrefix); /** - * @brief Possibly construct a new gateway URL value by resolving a given connection secret - * identifier. - * - * The gateway element is expected to contain a non-empty value for property `@url`. This value - * must begin with gwLocalSecretPrefix and be followed by a secret identification in the form - * of `[ vault-id ":" ] secret-name`. This is assumed to identify connection secret known only - * to the ESP, exceptions will be thrown on failure: - * - * - `gateway` must contain property `@url`; and - * - property '@url` must begin with gwLocalSecretPrefix; and - * - property `@url` must include a secret name, and may include a vault identifier; and - * - the optional vault identifier and required secret name combination must identify a secret - * in the `esp` category; and - * - the secret must grant the requested `permission`, when given, by defining a Boolean - * property with the permission name set to true; and - * - the secret must define a non-empty `url` property; and - * - the secret must define either a non-empty `username` property or a Boolean property - * `insecure` set to to true; and - * - the secret may define a `password` property only if a non-empty `username` property is - * also defined. - * - * Use of local secrets in gateway configurations is preferred to the use of inline URLs, but - * is not considered a best practice. Although it does remove user credentials from the - * configuration file, it increases exposure of the secret data and precludes the use of - * secret-defined tokens and certificates for connection security. - * - * @param gateway property tree that must contain at least a `@url` property - * @param permission name of local secret property controlling use of the secret in a gateway + * @brief Factory method to create an updatable gateway handler for a gateway referencing a + * local secret. + * + * @param gw + * @param gwName + * @param gwUrl + * @return IUpdatableGateway* */ - void resolveGatewayLocalSecret(IPTree& gateway, const char* permission) const; + IUpdatableGateway* createLocalSecretGateway(const IPTree& gw, const char* gwName, const char* gwUrl) const; /** - * @brief Possibly construct a new gateway URL value by removing embedded user credentials and - * inserting separately configured values. - * - * The gateway element must contain an `@url` property, and may contain `@username` and - * `@password` properties. A password must not be specified in the absence of a username. - * - * Use of inline URLs is strongly discouraged. Secure connections are not possible without - * including user credentials in the configuration. Support is provided for backward - * compatibility purposes only. - * - * Support for a `@roxieClient` gateway property is obsolete. When used with gateway - * configurations, its effect would be to either require both username and password or prevent - * the use of inline credentials. DESDL configurations should define credentials when needed, - * and omit them when not. - * - * @param gateway property tree that should contain at least a `@url` property + * @brief Factory method to create an updatable gateway handler for a gateway defined inline. + * + * @param gw + * @param gwName + * @param gwUrl + * @return IUpdatableGateway* */ - void resolveGatewayInlineURL(IPTree &gateway) const; + IUpdatableGateway* createInlineGateway(const IPTree& gw, const char* gwName, const char* gwUrl) const; /** - * @brief Helper function to assemble a URL with optional inline user credentials. + * @brief Obtain an updater instance for use with a specific gateway. + * + * - Null is returned if the gateway does not contain `@name` or `updatables` does not include + * this key. + * - A new link to an existing updater (in `updaters`) is returned if the updatable handler + * (from `updatables`) can find a match. Matching logic is an imlementation detail of the + * updatable handler. + * - A new updater is created, cached in `updaters`, and returned the first time the gateway + * is evaluated. * - * @param url a base URL on input; a possibly modified URL on output - * @param username optional user identifier - * @param password optional user password + * @param gw + * @param updatables + * @param updaters + * @param secrets + * @return IGatewayUpdater* */ - void updateURLCredentials(StringBuffer& url, const char* username, const char* password) const; + IGatewayUpdater* getGatewayUpdater(IPTree& gw, const UpdatableGateways& updatables, GatewayUpdaters& updaters, Secrets& secrets) const; /** * @brief Implementation of legacy gateway transformation invoked only during preparation of * published requests. * - * This can be deprecated once scripts can access resolved URL values. - * - * @param srvcfg the target configuration containing all gateways + * @param inputs iterator of all gateways * @param forRoxie the transformed gateway structure * @param altElementName configurable element name used in the transformed gateway structure */ - void transformGatewaysConfig( IPropertyTree* srvcfg, IPropertyTree* forRoxie, const char* altElementName = nullptr ) const; + void transformGatewaysConfig( IPTreeIterator* inputs, IPropertyTree* forRoxie, const char* altElementName = nullptr ) const; private: bool initMaskingEngineDirectory(const char* dir); From 3797238aa8aca7bbb98002f091e0b42d3181b700 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Wed, 20 Sep 2023 09:45:10 -0400 Subject: [PATCH 6/9] additional cleanup --- esp/services/esdl_svc_engine/esdl_binding.cpp | 76 +++++++++---------- esp/services/esdl_svc_engine/esdl_binding.hpp | 21 ++--- 2 files changed, 41 insertions(+), 56 deletions(-) diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp index e807d4350c9..1043d648804 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.cpp +++ b/esp/services/esdl_svc_engine/esdl_binding.cpp @@ -1393,16 +1393,21 @@ EsdlServiceImpl::IUpdatableGateway* EsdlServiceImpl::createInlineGateway(const I return new CLegacyUrlGateway(gw, gwName, gwUrl); } -EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::getGatewayUpdater(IPTree& gw, const UpdatableGateways& updatables, GatewayUpdaters& updaters, Secrets& secrets) const +void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, Secrets& secrets) const { - const char* name = gw.queryProp("@name"); - if (isEmptyString(name)) - return nullptr; - std::string key(name); - UpdatableGateways::const_iterator it = updatables.find(key); - if (updatables.end() == it) - return nullptr; - return it->second->getUpdater(updaters, secrets); + ForEach(gwIt) + { + IPTree& gw = gwIt.query(); + const char* name = gw.queryProp("@name"); + if (isEmptyString(name)) + continue; + UpdatableGateways::const_iterator ugIt = updatables.find(name); + if (updatables.end() == ugIt) + continue; + Owned updater(ugIt->second->getUpdater(updaters, secrets)); + if (updater) + updater->updateGateway(gw, "allowPublishedGatewayUsage"); + } } void EsdlServiceImpl::transformGatewaysConfig( IPTreeIterator* inputs, IPropertyTree* forRoxie, const char* altElementName ) const @@ -1778,27 +1783,22 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, reqProcessed.append("<").append(tgtQueryName).append(">"); reqProcessed.appendf("<_TransactionId>%s", context.queryTransactionID()); - GatewaysCache::const_iterator mit = m_methodGatewaysCache.find(mthName); + GatewaysCache::const_iterator mgcIt = m_methodGatewaysCache.find(mthName); + Owned gwIt; GatewayUpdaters updaters; Secrets secrets; if (tgtctx) { - // The target context is based on a copy of the target configuration. One copy per - // transaction. It will have already been copied into the script context, so local - // secret and inline URL resolutions can be made in the given property tree. - if (mit != m_methodGatewaysCache.end() && !mit->second.targetContext.empty()) + if (mgcIt != m_methodGatewaysCache.end() && !mgcIt->second.targetContext.empty()) { + // The target context is based on a copy of the target configuration. One copy + // per transaction. It will have already been copied into the script context, + // so gateway updates can be made in the given property tree. IPTree* ctxGateways = tgtctx->queryBranch("Gateways"); if (ctxGateways) { - Owned it(ctxGateways->getElements("Gateway")); - ForEach(*it) - { - IPTree& gw = it->query(); - Owned updater(getGatewayUpdater(gw, mit->second.targetContext, updaters, secrets)); - if (updater) - updater->updateGateway(gw, "allowPublishedGatewayUsage"); - } + gwIt.setown(ctxGateways->getElements("Gateway")); + applyGatewayUpdates(*gwIt, mgcIt->second.targetContext, updaters, secrets); } } toXML(tgtctx.get(), reqProcessed); @@ -1814,34 +1814,28 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, StringBuffer xpath(cfgGateways->queryProp("@legacyTransformTarget")); if (!xpath.isEmpty()) { - Owned it; - if (mit != m_methodGatewaysCache.end() && !mit->second.legacyTransform.empty()) + if (mgcIt != m_methodGatewaysCache.end() && !mgcIt->second.legacyTransform.empty()) { - // The target configuration is shared by all transactions. Local secret and - // inline URL resolutions must be made to a copy of the gateways, to avoid - // contaminating values used in subsequent transactions. + // The target configuration is shared by all transactions.gateway updates + // must be made to a copy of the gateways, to avoid contaminating values + // used in subsequent transactions. Owned copy(createPTreeFromIPT(cfgGateways)); - it.setown(copy->getElements("Gateway")); - ForEach(*it) - { - IPTree& gw = it->query(); - Owned updater(getGatewayUpdater(gw, mit->second.legacyTransform, updaters, secrets)); - if (updater) - updater->updateGateway(gw, "allowPublishedGatewayUsage"); - } + gwIt.setown(copy->getElements("Gateway")); + applyGatewayUpdates(*gwIt, mgcIt->second.legacyTransform, updaters, secrets); } else - it.setown(cfgGateways->getElements("Gateway")); - + gwIt.setown(cfgGateways->getElements("Gateway")); + Owned gws = createPTree("gateways", 0); + StringBuffer rowName(cfgGateways->queryProp("@legacyRowName")); + if (rowName.isEmpty()) + rowName.append("row"); + transformGatewaysConfig(gwIt, gws, rowName); + // Temporarily add the closing tag so we have valid // XML to transform the gateways Owned soapTree = createPTreeFromXMLString(reqProcessed.append(""), ipt_ordered); - StringBuffer rowName(cfgGateways->queryProp("@legacyRowName")); - if (rowName.isEmpty()) - rowName.append("row"); - transformGatewaysConfig(it, gws, rowName); xpath.replaceString("{$query}", tgtQueryName); xpath.replaceString("{$method}", mthName); xpath.replaceString("{$service}", srvdef.queryName()); diff --git a/esp/services/esdl_svc_engine/esdl_binding.hpp b/esp/services/esdl_svc_engine/esdl_binding.hpp index 384f74f7711..1c0149d676c 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.hpp +++ b/esp/services/esdl_svc_engine/esdl_binding.hpp @@ -494,23 +494,14 @@ class EsdlServiceImpl : public CInterface, implements IEspService IUpdatableGateway* createInlineGateway(const IPTree& gw, const char* gwName, const char* gwUrl) const; /** - * @brief Obtain an updater instance for use with a specific gateway. - * - * - Null is returned if the gateway does not contain `@name` or `updatables` does not include - * this key. - * - A new link to an existing updater (in `updaters`) is returned if the updatable handler - * (from `updatables`) can find a match. Matching logic is an imlementation detail of the - * updatable handler. - * - A new updater is created, cached in `updaters`, and returned the first time the gateway - * is evaluated. + * @brief Update all iterated nodes, as needed. * - * @param gw - * @param updatables - * @param updaters - * @param secrets - * @return IGatewayUpdater* + * @param gwIt property tree nodes, presumed to be `Gateway` nodes, for possible updates + * @param updatables set of update handlers to be applied to nodes + * @param updaters cache of updaters used in the current transaction + * @param secrets cache of secrets used in the current transaction */ - IGatewayUpdater* getGatewayUpdater(IPTree& gw, const UpdatableGateways& updatables, GatewayUpdaters& updaters, Secrets& secrets) const; + void applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, Secrets& secrets) const; /** * @brief Implementation of legacy gateway transformation invoked only during preparation of From 8378d3c76c9c4da7f8088b0e80f6cd9ce2a7c7a0 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Wed, 20 Sep 2023 14:41:46 -0400 Subject: [PATCH 7/9] more cleanup --- esp/services/esdl_svc_engine/esdl_binding.cpp | 39 ++++++------ esp/services/esdl_svc_engine/esdl_binding.hpp | 60 ++++++++----------- 2 files changed, 45 insertions(+), 54 deletions(-) diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp index 1043d648804..21b82a8bde7 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.cpp +++ b/esp/services/esdl_svc_engine/esdl_binding.cpp @@ -747,9 +747,9 @@ void EsdlServiceImpl::configureTargets(IPropertyTree *cfg, const char *service) continue; GatewaysCacheEntry& gce = m_methodGatewaysCache[method]; std::set uniqueNames; - bool updateInlines = gateways->getPropBool("@updateInlineUrls"); + bool updateInlines = gateways->getPropBool("@updateInline"); bool doLegacyTransform = !isEmptyString(gateways->queryProp("@legacyTransformTarget")); - Secrets secrets; + TransactionSecrets secrets; Owned gwIt(gateways->getElements("Gateway")); ForEach(*gwIt) { @@ -774,10 +774,11 @@ void EsdlServiceImpl::configureTargets(IPropertyTree *cfg, const char *service) if (strncmp(url, gwLocalSecretPrefix, gwLocalSecretPrefixLength) == 0) { handler.setown(createLocalSecretGateway(gw, name, url)); + gce.targetContext[key].set(handler.get()); + // sanity check that a secret is identified; the updater is not to be retained GatewayUpdaters updaters; Owned updater(handler->getUpdater(updaters, secrets)); updater.clear(); - gce.targetContext[key].set(handler.get()); } else if (hasHttpPrefix(url)) { @@ -1393,7 +1394,7 @@ EsdlServiceImpl::IUpdatableGateway* EsdlServiceImpl::createInlineGateway(const I return new CLegacyUrlGateway(gw, gwName, gwUrl); } -void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, Secrets& secrets) const +void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, TransactionSecrets& secrets) const { ForEach(gwIt) { @@ -1786,7 +1787,7 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, GatewaysCache::const_iterator mgcIt = m_methodGatewaysCache.find(mthName); Owned gwIt; GatewayUpdaters updaters; - Secrets secrets; + TransactionSecrets secrets; if (tgtctx) { if (mgcIt != m_methodGatewaysCache.end() && !mgcIt->second.targetContext.empty()) @@ -1835,7 +1836,6 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, // Temporarily add the closing tag so we have valid // XML to transform the gateways Owned soapTree = createPTreeFromXMLString(reqProcessed.append(""), ipt_ordered); - xpath.replaceString("{$query}", tgtQueryName); xpath.replaceString("{$method}", mthName); xpath.replaceString("{$service}", srvdef.queryName()); @@ -1905,7 +1905,7 @@ EsdlServiceImpl::~EsdlServiceImpl() } } -IPTree* EsdlServiceImpl::Secrets::getVaultSecret(const char* category, const char* vaultId, const char* name) +IPTree* EsdlServiceImpl::TransactionSecrets::getVaultSecret(const char* category, const char* vaultId, const char* name) { Owned secret(lookup(category, vaultId, name)); if (!secret) @@ -1917,7 +1917,7 @@ IPTree* EsdlServiceImpl::Secrets::getVaultSecret(const char* category, const cha return secret.getClear(); } -IPTree* EsdlServiceImpl::Secrets::getSecret(const char* category, const char* name) +IPTree* EsdlServiceImpl::TransactionSecrets::getSecret(const char* category, const char* name) { Owned secret(lookup(category, "", name)); if (!secret) @@ -1929,7 +1929,7 @@ IPTree* EsdlServiceImpl::Secrets::getSecret(const char* category, const char* na return secret; } -IPTree* EsdlServiceImpl::Secrets::lookup(const char* category, const char* vaultId, const char* name) const +IPTree* EsdlServiceImpl::TransactionSecrets::lookup(const char* category, const char* vaultId, const char* name) const { Key key = std::make_tuple(category ? category : "", vaultId ? vaultId : "", name ? name : ""); Cache::const_iterator it = cache.find(key); @@ -1938,13 +1938,13 @@ IPTree* EsdlServiceImpl::Secrets::lookup(const char* category, const char* vault return nullptr; } -void EsdlServiceImpl::Secrets::store(IPTree& secret, const char* category, const char* vaultId, const char* name) +void EsdlServiceImpl::TransactionSecrets::store(IPTree& secret, const char* category, const char* vaultId, const char* name) { Key key = std::make_tuple(category ? category : "", vaultId ? vaultId : "", name ? name : ""); cache[key].set(&secret); } -EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, Secrets& secrets) const +EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const { GatewayUpdaters::iterator it = updaters.find(updatersKey); if (it != updaters.end()) @@ -1992,7 +1992,7 @@ bool EsdlServiceImpl::CUpdatableGateway::updateURLCredentials(StringBuffer& url, return false; } -EsdlServiceImpl::CLegacyUrlGateway* EsdlServiceImpl::CLegacyUrlGateway::getUpdater(Secrets&) const +EsdlServiceImpl::CLegacyUrlGateway* EsdlServiceImpl::CLegacyUrlGateway::getUpdater(TransactionSecrets&) const { return LINK(const_cast(this)); } @@ -2026,7 +2026,7 @@ void EsdlServiceImpl::CLocalSecretGateway::CUpdater::updateGateway(IPTree& gw, c entry->doUpdate(gw, *secret, requiredUsage); } -EsdlServiceImpl::CLocalSecretGateway::CUpdater::CUpdater(const CLocalSecretGateway& _entry, Secrets& secrets) +EsdlServiceImpl::CLocalSecretGateway::CUpdater::CUpdater(const CLocalSecretGateway& _entry, TransactionSecrets& secrets) { entry.set(&_entry); if (entry->vaultId.isEmpty()) @@ -2035,7 +2035,7 @@ EsdlServiceImpl::CLocalSecretGateway::CUpdater::CUpdater(const CLocalSecretGatew secret.setown(secrets.getVaultSecret("esp", entry->vaultId.str(), entry->secretName)); } -EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CLocalSecretGateway::getUpdater(Secrets& secrets) const +EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CLocalSecretGateway::getUpdater(TransactionSecrets& secrets) const { return new CUpdater(*this, secrets); } @@ -2069,7 +2069,7 @@ EsdlServiceImpl::CLocalSecretGateway::CLocalSecretGateway(const IPTree& gw, cons void EsdlServiceImpl::CLocalSecretGateway::doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const { if (!isEmptyString(requiredUsage) && !secret.getPropBool(requiredUsage)) - throw makeStringExceptionV(-1, "gateway %s: '%s' does not allow '%s' usage", updatersKey.c_str(), secretId.str(), requiredUsage); + throw makeStringExceptionV(-1, "gateway %s: secret '%s' does not allow '%s' usage", updatersKey.c_str(), secretId.str(), requiredUsage); } void EsdlServiceImpl::CHttpConnectGateway::doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const @@ -2077,10 +2077,13 @@ void EsdlServiceImpl::CHttpConnectGateway::doUpdate(IPTree& gw, const IPTree& se CLocalSecretGateway::doUpdate(gw, secret, requiredUsage); StringBuffer url; if (!secret.getProp("url", url.clear()) || url.isEmpty()) - throw makeStringExceptionV(-1, "gateway %s: '%s' missing required 'url' property; credential-only secrets not supported", gw.queryProp("@name"), secretId.str()); + throw makeStringExceptionV(-1, "gateway %s: secret '%s' missing required 'url' property; credential-only secrets not supported", gw.queryProp("@name"), secretId.str()); const char* username = secret.queryProp("username"); - if (isEmptyString(username) && !secret.getPropBool("omitCredentials")) - throw makeStringExceptionV(-1, "gateway %s: '%s' missing expected 'username' property; set 'omitCredentials` property to 'true' if credentials are not required", gw.queryProp("@name"), secretId.str()); + bool omitCredentials = secret.getPropBool("omitCredentials"); + if (isEmptyString(username) && !omitCredentials) + throw makeStringExceptionV(-1, "gateway %s: secret '%s' missing expected 'username' property; set 'omitCredentials` property to 'true' if credentials are not required", gw.queryProp("@name"), secretId.str()); + if (!isEmptyString(username) && omitCredentials) + throw makeStringExceptionV(-1, "gateway %s: secret '%s' contains 'username' and sets 'omitCredentials'", gw.queryProp("@name"), secretId.str()); const char* password = secret.queryProp("password"); if (isEmptyString(username) && password) throw makeStringExceptionV(-1, "gateway %s: '%s' invalid use of password without username", gw.queryProp("@name"), secretId.str()); diff --git a/esp/services/esdl_svc_engine/esdl_binding.hpp b/esp/services/esdl_svc_engine/esdl_binding.hpp index 1c0149d676c..68fdda9ab40 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.hpp +++ b/esp/services/esdl_svc_engine/esdl_binding.hpp @@ -86,7 +86,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService * - Encapsulate non-jsecrets access to support non-standard secret sources, unless * jsecrets is changed to provide this encapsulation. */ - class Secrets + class TransactionSecrets { private: using Key = std::tuple; @@ -142,7 +142,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService */ interface IUpdatableGateway : public IInterface { - virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, Secrets& secrets) const = 0; + virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const = 0; }; using UpdatableGateways = std::map>; @@ -157,18 +157,18 @@ class EsdlServiceImpl : public CInterface, implements IEspService * - Insertion (or replacement) of user credentials in a URL string is available to any * subclass that needs it.. * - * All extensions must implement `getUpdater(Secrets&)`. In some cases this may entail creation + * All extensions must implement `getUpdater(TransactionSecrets&)`. In some cases this may entail creation * of a new updater instance. In other cases a single instance may be reused. */ class CUpdatableGateway : public CInterfaceOf { public: - virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, Secrets& secrets) const override; + virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const override; protected: std::string updatersKey; protected: CUpdatableGateway(const char* gwName); - virtual IGatewayUpdater* getUpdater(Secrets& secrets) const = 0; + virtual IGatewayUpdater* getUpdater(TransactionSecrets& secrets) const = 0; bool updateURLCredentials(StringBuffer& url, const char* username, const char* password) const; }; @@ -181,11 +181,14 @@ class EsdlServiceImpl : public CInterface, implements IEspService * - If `Gateway/@username` is given, `Gateway/@password` may specify an exncrypted password * value to be decrypted and embedded in an updated URL. * - It is an error to specify `Gateway/@password` without `Gateway/@username`. + * + * Support is provided for backward compatibility. Use of these secrets is strongly discouraged + * and should be avoided whenever possible. */ class CLegacyUrlGateway : public CUpdatableGateway, public IGatewayUpdater { protected: // CUpdatableGateway - virtual CLegacyUrlGateway* getUpdater(Secrets&) const override; + virtual CLegacyUrlGateway* getUpdater(TransactionSecrets&) const override; public: // IGatewayUpdater virtual void updateGateway(IPTree& gw, const char*) override; protected: @@ -203,6 +206,9 @@ class EsdlServiceImpl : public CInterface, implements IEspService * - Defines a standard updater separating dynamically changing secrets from the configuration. * - Enforces a requirement for a local secret to explicitly allow its data to be shared with * a backend roxie service. + * + * Support is provided as an alternative to inline definitions when secret names cannot be + * passed to the target roxie for resolution. Use is discouraged unless it is unavoidable. */ class CLocalSecretGateway : public CUpdatableGateway { @@ -216,15 +222,15 @@ class EsdlServiceImpl : public CInterface, implements IEspService Linked entry; Owned secret; public: - CUpdater(const CLocalSecretGateway& _entry, Secrets& secrets); + CUpdater(const CLocalSecretGateway& _entry, TransactionSecrets& secrets); }; protected: // CUpdatableGateway - virtual IGatewayUpdater* getUpdater(Secrets& secrets) const override; + virtual IGatewayUpdater* getUpdater(TransactionSecrets& secrets) const override; protected: StringBuffer secretId; StringAttr vaultId; StringBuffer secretName; - public: + protected: CLocalSecretGateway(const IPTree& gw, const char* gwName, const char* gwUrl, const char* classPrefix); protected: virtual void doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const; @@ -241,6 +247,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService * - A secret must define either a non-empty `username` property or set the `omitCredentials` * property to true. * - A secret may define a `password` property if `username` is also defined. + * - A secret must not define `username` and set `omitCredentials` to true. * - A secret must not define a `password` property if `username` is not defined. */ class CHttpConnectGateway : public CLocalSecretGateway @@ -257,12 +264,13 @@ class EsdlServiceImpl : public CInterface, implements IEspService * Gateway updates may occur in either or both of two locations within a backend * service request. The storage layout must enable support for both locations. * - * 1. The optional inclusion of a methods binding definition that does not exclude defined + * 1. The optional inclusion of a method's binding definition that does not exclude defined * gateways, also known as the target context. Target context inclusion must update all * gateways referring to local secrets. Target context inclusion must not, for backward - * compatibility, update any gateways referring to inline URLs. + * compatibility, update any inline gateways unless `Gateways/@updateInline` is true. * 2. The optional request for "legacy gateway transformation". Legacy transformations must - * update all gateways referring to local secrets and inline URLs. + * update all local secret and inline gateways when `Gateways/@legacyTransformTarget` is + * not empty. */ struct GatewaysCacheEntry { @@ -409,29 +417,9 @@ class EsdlServiceImpl : public CInterface, implements IEspService * ESDL script entry point transforms, if any exist. * * Initial request construction of all backend requests wraps the given request content in a - * SOAP envelope and SOAP body. For published requests, where `isroxie` is true, special - * handling for gateways is provided: - * - * - For `Gateways/Gateway` elements found in `tgtctx`: - * - Elements with local secret references are updated to replace `@url` with secret- - * defined data. - * - Elements with inline URL data are updated to insert user credentials into the URL, but - * only if `Gateways/@updateInlineUrls` is true. - * - For `Gateways/Gateway1 elements found in `tgtctx` when `Gateways/@legacyTransformTarget` - * is not empty: - * - Elements with local secret references are updated to replace `@url` with secret- - * defined data. - * - Elements with inline URL data are updated to insert user credentials into the URL. - * - `` becomes - * ``, with `Gateways/@legacyRowName` - * allowing an alternate name for element `row`. - * - The new `gateways` element is placed in the request at the location referred to by - * `Gateways/@legacyTransformTarget`. This XPath supports multiple variable substitutions - * such as: - * - {$query} becomes the value of `Method/@queryname` - * - {$method} becomes the ESDL definition's method name - * - {$service} becomes the ESDL definition's service name - * - {$request} becomes the ESDL definition's method request type + * SOAP envelope and SOAP body. For published requests, where `isroxie` is true, updatable + * gateways are updated prior to inclusion with the `tgtctx` configuration and as part of + * legacy gateway transformations. * * Support for inline URLs in the target configuration, and context which is a subset of the * configuration, is provided for backward compatibility. Use is strongly discouraged. @@ -501,7 +489,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService * @param updaters cache of updaters used in the current transaction * @param secrets cache of secrets used in the current transaction */ - void applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, Secrets& secrets) const; + void applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, TransactionSecrets& secrets) const; /** * @brief Implementation of legacy gateway transformation invoked only during preparation of From 8a23f370a845a47db71430c440df1eaece5dc14c Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Thu, 16 Nov 2023 13:10:14 -0500 Subject: [PATCH 8/9] Retrofit to use IEsdlScriptContext secrets --- esp/services/esdl_svc_engine/esdl_binding.cpp | 128 ++++-------------- esp/services/esdl_svc_engine/esdl_binding.hpp | 105 +++++++------- 2 files changed, 79 insertions(+), 154 deletions(-) diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp index 21b82a8bde7..e31d00007e2 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.cpp +++ b/esp/services/esdl_svc_engine/esdl_binding.cpp @@ -1394,7 +1394,7 @@ EsdlServiceImpl::IUpdatableGateway* EsdlServiceImpl::createInlineGateway(const I return new CLegacyUrlGateway(gw, gwName, gwUrl); } -void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, TransactionSecrets& secrets) const +void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const { ForEach(gwIt) { @@ -1405,9 +1405,9 @@ void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableG UpdatableGateways::const_iterator ugIt = updatables.find(name); if (updatables.end() == ugIt) continue; - Owned updater(ugIt->second->getUpdater(updaters, secrets)); + Owned updater(ugIt->second->getUpdater(updaters, scriptContext)); if (updater) - updater->updateGateway(gw, "allowPublishedGatewayUsage"); + updater->updateGateway(gw); } } @@ -1799,7 +1799,7 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, if (ctxGateways) { gwIt.setown(ctxGateways->getElements("Gateway")); - applyGatewayUpdates(*gwIt, mgcIt->second.targetContext, updaters, secrets); + applyGatewayUpdates(*gwIt, mgcIt->second.targetContext, updaters, scriptContext); } } toXML(tgtctx.get(), reqProcessed); @@ -1822,7 +1822,7 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, // used in subsequent transactions. Owned copy(createPTreeFromIPT(cfgGateways)); gwIt.setown(copy->getElements("Gateway")); - applyGatewayUpdates(*gwIt, mgcIt->second.legacyTransform, updaters, secrets); + applyGatewayUpdates(*gwIt, mgcIt->second.legacyTransform, updaters, scriptContext); } else gwIt.setown(cfgGateways->getElements("Gateway")); @@ -1905,51 +1905,22 @@ EsdlServiceImpl::~EsdlServiceImpl() } } -IPTree* EsdlServiceImpl::TransactionSecrets::getVaultSecret(const char* category, const char* vaultId, const char* name) -{ - Owned secret(lookup(category, vaultId, name)); - if (!secret) - { - secret.setown(::getVaultSecret(category, vaultId, name)); - if (secret) - store(*secret, category, vaultId, name); - } - return secret.getClear(); -} - -IPTree* EsdlServiceImpl::TransactionSecrets::getSecret(const char* category, const char* name) -{ - Owned secret(lookup(category, "", name)); - if (!secret) - { - secret.setown(::getVaultSecret(category, "", name)); - if (secret) - store(*secret, category, "", name); - } - return secret; -} - -IPTree* EsdlServiceImpl::TransactionSecrets::lookup(const char* category, const char* vaultId, const char* name) const +EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const { - Key key = std::make_tuple(category ? category : "", vaultId ? vaultId : "", name ? name : ""); - Cache::const_iterator it = cache.find(key); - if (it != cache.end()) + GatewayUpdaters::iterator it = updaters.find(updatersKey); + if (it != updaters.end()) return it->second.getLink(); - return nullptr; -} - -void EsdlServiceImpl::TransactionSecrets::store(IPTree& secret, const char* category, const char* vaultId, const char* name) -{ - Key key = std::make_tuple(category ? category : "", vaultId ? vaultId : "", name ? name : ""); - cache[key].set(&secret); + Owned spawned(getUpdater(secrets)); + updaters[updatersKey].setown(spawned.getLink()); + return spawned.getClear(); } -EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const +EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const { GatewayUpdaters::iterator it = updaters.find(updatersKey); if (it != updaters.end()) return it->second.getLink(); - Owned spawned(getUpdater(secrets)); + Owned spawned(getUpdater(scriptContext)); updaters[updatersKey].setown(spawned.getLink()); return spawned.getClear(); } @@ -1997,7 +1968,12 @@ EsdlServiceImpl::CLegacyUrlGateway* EsdlServiceImpl::CLegacyUrlGateway::getUpdat return LINK(const_cast(this)); } -void EsdlServiceImpl::CLegacyUrlGateway::updateGateway(IPTree& gw, const char*) +EsdlServiceImpl::CLegacyUrlGateway* EsdlServiceImpl::CLegacyUrlGateway::getUpdater(IEsdlScriptContext*) const +{ + return LINK(const_cast(this)); +} + +void EsdlServiceImpl::CLegacyUrlGateway::updateGateway(IPTree& gw) { gw.setProp("@url", url); } @@ -2019,80 +1995,26 @@ EsdlServiceImpl::CLegacyUrlGateway::CLegacyUrlGateway(const IPTree& gw, const ch updateURLCredentials(url, username, password); } -void EsdlServiceImpl::CLocalSecretGateway::CUpdater::updateGateway(IPTree& gw, const char* requiredUsage) -{ - if (!secret) - throw makeStringExceptionV(-1, "gateway %s: 'esp' category secret '%s' not found", "?", entry->secretId.str()); - entry->doUpdate(gw, *secret, requiredUsage); -} - -EsdlServiceImpl::CLocalSecretGateway::CUpdater::CUpdater(const CLocalSecretGateway& _entry, TransactionSecrets& secrets) -{ - entry.set(&_entry); - if (entry->vaultId.isEmpty()) - secret.setown(secrets.getSecret("esp", entry->secretName)); - else - secret.setown(secrets.getVaultSecret("esp", entry->vaultId.str(), entry->secretName)); -} - -EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CLocalSecretGateway::getUpdater(TransactionSecrets& secrets) const -{ - return new CUpdater(*this, secrets); -} - -EsdlServiceImpl::CLocalSecretGateway::CLocalSecretGateway(const IPTree& gw, const char* gwName, const char* gwUrl, const char* classPrefix) - : CUpdatableGateway(gwName) -{ - secretId.append(gwUrl + gwLocalSecretPrefixLength); - StringArray tokens; - tokens.appendList(secretId, ":", true); - const char* vaultToken = nullptr; - const char* secretToken = nullptr; - switch (tokens.ordinality()) - { - case 1: // secret name without vault name - secretToken = tokens.item(0); - break; - case 2: // secret name after vault name - vaultToken = tokens.item(0); - secretToken = tokens.item(1); - break; - default: - throw makeStringExceptionV(-1, "gateway %s: '%s' is not of the form \"[ vault-id ':' ] secret-name\"", gwName, secretId.str()); - } - - if (!isEmptyString(vaultToken)) - vaultId.set(vaultToken); - secretName.append(classPrefix).append(secretToken); -} - -void EsdlServiceImpl::CLocalSecretGateway::doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const -{ - if (!isEmptyString(requiredUsage) && !secret.getPropBool(requiredUsage)) - throw makeStringExceptionV(-1, "gateway %s: secret '%s' does not allow '%s' usage", updatersKey.c_str(), secretId.str(), requiredUsage); -} - -void EsdlServiceImpl::CHttpConnectGateway::doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const +void EsdlServiceImpl::CHttpConnectGateway::doUpdate(IPTree& gw, const IPTree& secret) const { - CLocalSecretGateway::doUpdate(gw, secret, requiredUsage); StringBuffer url; if (!secret.getProp("url", url.clear()) || url.isEmpty()) - throw makeStringExceptionV(-1, "gateway %s: secret '%s' missing required 'url' property; credential-only secrets not supported", gw.queryProp("@name"), secretId.str()); + throw makeStringExceptionV(-1, "gateway %s: secret '%s' missing required 'url' property; credential-only secrets not supported", gw.queryProp("@name"), identifier.str()); const char* username = secret.queryProp("username"); bool omitCredentials = secret.getPropBool("omitCredentials"); if (isEmptyString(username) && !omitCredentials) - throw makeStringExceptionV(-1, "gateway %s: secret '%s' missing expected 'username' property; set 'omitCredentials` property to 'true' if credentials are not required", gw.queryProp("@name"), secretId.str()); + throw makeStringExceptionV(-1, "gateway %s: secret '%s' missing expected 'username' property; set 'omitCredentials` property to 'true' if credentials are not required", gw.queryProp("@name"), identifier.str()); if (!isEmptyString(username) && omitCredentials) - throw makeStringExceptionV(-1, "gateway %s: secret '%s' contains 'username' and sets 'omitCredentials'", gw.queryProp("@name"), secretId.str()); + throw makeStringExceptionV(-1, "gateway %s: secret '%s' contains 'username' and sets 'omitCredentials'", gw.queryProp("@name"), identifier.str()); const char* password = secret.queryProp("password"); if (isEmptyString(username) && password) - throw makeStringExceptionV(-1, "gateway %s: '%s' invalid use of password without username", gw.queryProp("@name"), secretId.str()); + throw makeStringExceptionV(-1, "gateway %s: '%s' invalid use of password without username", gw.queryProp("@name"), identifier.str()); updateURLCredentials(url, username, password); gw.setProp("@url", url); } EsdlServiceImpl::CHttpConnectGateway::CHttpConnectGateway(const IPTree& gw, const char* gwName, const char* gwUrl) - : CLocalSecretGateway(gw, gwName, gwUrl, "http-connect-") + : TLocalSecretGateway(gw, gwName, gwUrl) { } diff --git a/esp/services/esdl_svc_engine/esdl_binding.hpp b/esp/services/esdl_svc_engine/esdl_binding.hpp index 68fdda9ab40..5b7ac65d52e 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.hpp +++ b/esp/services/esdl_svc_engine/esdl_binding.hpp @@ -72,34 +72,6 @@ typedef int (*cpp_service_method_t)(const char* CtxXML, const char* ReqXML, Stri class EsdlServiceImpl : public CInterface, implements IEspService { -private: - /** - * @brief Cache of secrets accessed as part of transaction processing. - * - * In the context of a single transaction, multiple requests for the same secret should always - * return the same secret data for each request. Cached data does not expire and there is no - * option to reload data. - * - * TODO: Refactor to support additional use cases - * - Provide ESDL script operation support so mysql (now) and http-post-xml (eventually) - * can benefit secret reuse. - * - Encapsulate non-jsecrets access to support non-standard secret sources, unless - * jsecrets is changed to provide this encapsulation. - */ - class TransactionSecrets - { - private: - using Key = std::tuple; - using Cache = std::map>; - Cache cache; - public: - IPTree* getVaultSecret(const char* category, const char* vaultId, const char* name); - IPTree* getSecret(const char* category, const char* name); - protected: - IPTree* lookup(const char* category, const char* vaultId, const char* name) const; - void store(IPTree& secret, const char* category, const char* vaultId, const char* name); - }; - private: /** * @brief Abstraction of a per-transaction gateway updater. @@ -115,7 +87,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService */ interface IGatewayUpdater : public IInterface { - virtual void updateGateway(IPTree& gw, const char* requiredUsage) = 0; + virtual void updateGateway(IPTree& gw) = 0; }; using GatewayUpdaters = std::map>; @@ -143,6 +115,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService interface IUpdatableGateway : public IInterface { virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const = 0; + virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const = 0; }; using UpdatableGateways = std::map>; @@ -164,11 +137,13 @@ class EsdlServiceImpl : public CInterface, implements IEspService { public: virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const override; + virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const override; protected: std::string updatersKey; protected: CUpdatableGateway(const char* gwName); virtual IGatewayUpdater* getUpdater(TransactionSecrets& secrets) const = 0; + virtual IGatewayUpdater* getUpdater(IEsdlScriptContext* scriptContext) const = 0; bool updateURLCredentials(StringBuffer& url, const char* username, const char* password) const; }; @@ -189,8 +164,9 @@ class EsdlServiceImpl : public CInterface, implements IEspService { protected: // CUpdatableGateway virtual CLegacyUrlGateway* getUpdater(TransactionSecrets&) const override; + virtual CLegacyUrlGateway* getUpdater(IEsdlScriptContext*) const override; public: // IGatewayUpdater - virtual void updateGateway(IPTree& gw, const char*) override; + virtual void updateGateway(IPTree& gw) override; protected: StringBuffer url; public: @@ -202,47 +178,74 @@ class EsdlServiceImpl : public CInterface, implements IEspService * @brief Concrete extension of `CUpdatableGateway` for handling local secrets that is always * extended with secret usage-specific logic. * - * - Ensures `Gateway/@url` matches the pattern `"local-secret:" [ vault-id ":" ] secret-name".` + * - Ensures `Gateway/@url` matches the pattern `"local-secret:" [ vault-id "::" ] secret-name [ "::" version ]`. + * - `secret-name` is required always + * - `vault-id` is required before `version` can be specified * - Defines a standard updater separating dynamically changing secrets from the configuration. - * - Enforces a requirement for a local secret to explicitly allow its data to be shared with - * a backend roxie service. * * Support is provided as an alternative to inline definitions when secret names cannot be * passed to the target roxie for resolution. Use is discouraged unless it is unavoidable. */ - class CLocalSecretGateway : public CUpdatableGateway + template + class TLocalSecretGateway : public CUpdatableGateway { protected: - class CUpdater : public CInterfaceOf + template + class TUpdater : public CInterfaceOf { - friend class CLocalSecretGateway; + friend class TLocalSecretGateway; + static constexpr const char* category = "espUser"; public: // IGatewayUpdater - virtual void updateGateway(IPTree& gw, const char* requiredUsage) override; + virtual void updateGateway(IPTree& gw) override + { + if (!secret) + throw makeStringExceptionV(-1, "gateway %s: '%s' category secret '%s' not found", "?", entry->identifier.str(), category); + entry->doUpdate(gw, *secret); + } protected: - Linked entry; + Linked> entry; Owned secret; public: - CUpdater(const CLocalSecretGateway& _entry, TransactionSecrets& secrets); + TUpdater(const TLocalSecretGateway& _entry, secret_source_t& secrets) + { + entry.set(&_entry); + secret.setown(secrets.getSecret(category, entry->identity)); + } }; protected: // CUpdatableGateway - virtual IGatewayUpdater* getUpdater(TransactionSecrets& secrets) const override; + virtual IGatewayUpdater* getUpdater(TransactionSecrets& secrets) const override + { + return new TUpdater(*this, secrets); + } + virtual IGatewayUpdater* getUpdater(IEsdlScriptContext* scriptContext) const override + { + if (!scriptContext) + return nullptr; + return new TUpdater(*this, *scriptContext); + } protected: - StringBuffer secretId; - StringAttr vaultId; - StringBuffer secretName; + StringBuffer identifier; + secret_identity_t identity; protected: - CLocalSecretGateway(const IPTree& gw, const char* gwName, const char* gwUrl, const char* classPrefix); + TLocalSecretGateway(const IPTree& gw, const char* gwName, const char* gwUrl) + : CUpdatableGateway(gwName) + , identifier(gwUrl + gwLocalSecretPrefixLength) + , identity(identifier) + { + } protected: - virtual void doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const; + virtual void doUpdate(IPTree& gw, const IPTree& secret) const + { + } }; /** - * @brief Concete extension of `CLocalSecretGateway` to construct a new `Gateway/@url` value + * @brief Concete extension of `TLocalSecretGateway<>` to construct a new `Gateway/@url` value * from secret-defined values. * * - A secret name must begin with "http-connect-". Configurations should omit this prefix, * but its presence is acceptable. - * - A secret must satisfy the requirements defined in `CLocalSecretGateway`. + * - A secret must satisfy the requirements defined in `TLocalSecretGateway<>`. * - A secret must define a non-empty `url` property. * - A secret must define either a non-empty `username` property or set the `omitCredentials` * property to true. @@ -250,10 +253,10 @@ class EsdlServiceImpl : public CInterface, implements IEspService * - A secret must not define `username` and set `omitCredentials` to true. * - A secret must not define a `password` property if `username` is not defined. */ - class CHttpConnectGateway : public CLocalSecretGateway + class CHttpConnectGateway : public TLocalSecretGateway { - protected: // CLocalSecretGateway - void doUpdate(IPTree& gw, const IPTree& secret, const char* requiredUsage) const override; + protected: // TLocalSecretGateway + void doUpdate(IPTree& gw, const IPTree& secret) const override; public: CHttpConnectGateway(const IPTree& gw, const char* gwName, const char* gwUrl); }; @@ -489,7 +492,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService * @param updaters cache of updaters used in the current transaction * @param secrets cache of secrets used in the current transaction */ - void applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, TransactionSecrets& secrets) const; + void applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const; /** * @brief Implementation of legacy gateway transformation invoked only during preparation of From 512cc33f66db5963a9b67e8a08262c222ecacd20 Mon Sep 17 00:00:00 2001 From: Tim Klemm Date: Thu, 16 Nov 2023 15:36:09 -0500 Subject: [PATCH 9/9] Secrets cache abstraction --- esp/services/esdl_svc_engine/esdl_binding.cpp | 32 +++------- esp/services/esdl_svc_engine/esdl_binding.hpp | 61 ++++++++++++------- 2 files changed, 48 insertions(+), 45 deletions(-) diff --git a/esp/services/esdl_svc_engine/esdl_binding.cpp b/esp/services/esdl_svc_engine/esdl_binding.cpp index e31d00007e2..ad4a8b10f41 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.cpp +++ b/esp/services/esdl_svc_engine/esdl_binding.cpp @@ -750,6 +750,7 @@ void EsdlServiceImpl::configureTargets(IPropertyTree *cfg, const char *service) bool updateInlines = gateways->getPropBool("@updateInline"); bool doLegacyTransform = !isEmptyString(gateways->queryProp("@legacyTransformTarget")); TransactionSecrets secrets; + TTransactionSecretsWrapper wrappedSecrets(secrets); Owned gwIt(gateways->getElements("Gateway")); ForEach(*gwIt) { @@ -777,7 +778,7 @@ void EsdlServiceImpl::configureTargets(IPropertyTree *cfg, const char *service) gce.targetContext[key].set(handler.get()); // sanity check that a secret is identified; the updater is not to be retained GatewayUpdaters updaters; - Owned updater(handler->getUpdater(updaters, secrets)); + Owned updater(handler->getUpdater(updaters, wrappedSecrets)); updater.clear(); } else if (hasHttpPrefix(url)) @@ -1394,7 +1395,7 @@ EsdlServiceImpl::IUpdatableGateway* EsdlServiceImpl::createInlineGateway(const I return new CLegacyUrlGateway(gw, gwName, gwUrl); } -void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const +void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, ITransactionSecretsWrapper& secrets) const { ForEach(gwIt) { @@ -1405,7 +1406,7 @@ void EsdlServiceImpl::applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableG UpdatableGateways::const_iterator ugIt = updatables.find(name); if (updatables.end() == ugIt) continue; - Owned updater(ugIt->second->getUpdater(updaters, scriptContext)); + Owned updater(ugIt->second->getUpdater(updaters, secrets)); if (updater) updater->updateGateway(gw); } @@ -1787,7 +1788,7 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, GatewaysCache::const_iterator mgcIt = m_methodGatewaysCache.find(mthName); Owned gwIt; GatewayUpdaters updaters; - TransactionSecrets secrets; + TTransactionSecretsWrapper wrappedSecrets(*scriptContext); if (tgtctx) { if (mgcIt != m_methodGatewaysCache.end() && !mgcIt->second.targetContext.empty()) @@ -1799,7 +1800,7 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, if (ctxGateways) { gwIt.setown(ctxGateways->getElements("Gateway")); - applyGatewayUpdates(*gwIt, mgcIt->second.targetContext, updaters, scriptContext); + applyGatewayUpdates(*gwIt, mgcIt->second.targetContext, updaters, wrappedSecrets); } } toXML(tgtctx.get(), reqProcessed); @@ -1822,7 +1823,7 @@ void EsdlServiceImpl::prepareFinalRequest(IEspContext &context, // used in subsequent transactions. Owned copy(createPTreeFromIPT(cfgGateways)); gwIt.setown(copy->getElements("Gateway")); - applyGatewayUpdates(*gwIt, mgcIt->second.legacyTransform, updaters, scriptContext); + applyGatewayUpdates(*gwIt, mgcIt->second.legacyTransform, updaters, wrappedSecrets); } else gwIt.setown(cfgGateways->getElements("Gateway")); @@ -1905,7 +1906,7 @@ EsdlServiceImpl::~EsdlServiceImpl() } } -EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const +EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, ITransactionSecretsWrapper& secrets) const { GatewayUpdaters::iterator it = updaters.find(updatersKey); if (it != updaters.end()) @@ -1915,16 +1916,6 @@ EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater return spawned.getClear(); } -EsdlServiceImpl::IGatewayUpdater* EsdlServiceImpl::CUpdatableGateway::getUpdater(GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const -{ - GatewayUpdaters::iterator it = updaters.find(updatersKey); - if (it != updaters.end()) - return it->second.getLink(); - Owned spawned(getUpdater(scriptContext)); - updaters[updatersKey].setown(spawned.getLink()); - return spawned.getClear(); -} - EsdlServiceImpl::CUpdatableGateway::CUpdatableGateway(const char* gwName) { updatersKey.assign(gwName); @@ -1963,12 +1954,7 @@ bool EsdlServiceImpl::CUpdatableGateway::updateURLCredentials(StringBuffer& url, return false; } -EsdlServiceImpl::CLegacyUrlGateway* EsdlServiceImpl::CLegacyUrlGateway::getUpdater(TransactionSecrets&) const -{ - return LINK(const_cast(this)); -} - -EsdlServiceImpl::CLegacyUrlGateway* EsdlServiceImpl::CLegacyUrlGateway::getUpdater(IEsdlScriptContext*) const +EsdlServiceImpl::CLegacyUrlGateway* EsdlServiceImpl::CLegacyUrlGateway::getUpdater(ITransactionSecretsWrapper&) const { return LINK(const_cast(this)); } diff --git a/esp/services/esdl_svc_engine/esdl_binding.hpp b/esp/services/esdl_svc_engine/esdl_binding.hpp index 5b7ac65d52e..63d96aeef2a 100755 --- a/esp/services/esdl_svc_engine/esdl_binding.hpp +++ b/esp/services/esdl_svc_engine/esdl_binding.hpp @@ -73,6 +73,33 @@ typedef int (*cpp_service_method_t)(const char* CtxXML, const char* ReqXML, Stri class EsdlServiceImpl : public CInterface, implements IEspService { private: + /** + * @brief Utility wrapper interface for secret access. + * + * At service load, secrets are cached directly in a TransactionSecrets instance. When + * processing requests, secrets are cached in the script context. Both provide the same + * interface to obtain a secret. This wrapper decouples gateway secret updating from the + * cache in use. + */ + interface ITransactionSecretsWrapper + { + virtual IPTree* getSecret(const char* category, const SecretId& id) const = 0; + }; + + /** + * @brief Concrete implementation of `ITransactionSecretsWrapper` wrapping multiple caches. + * + * @tparam secret_cache_t + */ + template + struct TTransactionSecretsWrapper : public ITransactionSecretsWrapper + { + TTransactionSecretsWrapper(secret_cache_t& _wrapped) : wrapped(_wrapped) {} + virtual IPTree* getSecret(const char* category, const SecretId& id) const override { return wrapped.getSecret(category, id); } + private: + secret_cache_t& wrapped; + }; + /** * @brief Abstraction of a per-transaction gateway updater. * @@ -114,8 +141,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService */ interface IUpdatableGateway : public IInterface { - virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const = 0; - virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const = 0; + virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, ITransactionSecretsWrapper& secrets) const = 0; }; using UpdatableGateways = std::map>; @@ -130,20 +156,19 @@ class EsdlServiceImpl : public CInterface, implements IEspService * - Insertion (or replacement) of user credentials in a URL string is available to any * subclass that needs it.. * - * All extensions must implement `getUpdater(TransactionSecrets&)`. In some cases this may entail creation - * of a new updater instance. In other cases a single instance may be reused. + * All extensions must implement `getUpdater(ITransactionSecretsWrapper&)`. In some cases + * this may entail creation of a new updater instance. In other cases a single instance may + * be reused. */ class CUpdatableGateway : public CInterfaceOf { public: - virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, TransactionSecrets& secrets) const override; - virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const override; + virtual IGatewayUpdater* getUpdater(GatewayUpdaters& updaters, ITransactionSecretsWrapper& secrets) const override; protected: std::string updatersKey; protected: CUpdatableGateway(const char* gwName); - virtual IGatewayUpdater* getUpdater(TransactionSecrets& secrets) const = 0; - virtual IGatewayUpdater* getUpdater(IEsdlScriptContext* scriptContext) const = 0; + virtual IGatewayUpdater* getUpdater(ITransactionSecretsWrapper& secrets) const = 0; bool updateURLCredentials(StringBuffer& url, const char* username, const char* password) const; }; @@ -163,8 +188,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService class CLegacyUrlGateway : public CUpdatableGateway, public IGatewayUpdater { protected: // CUpdatableGateway - virtual CLegacyUrlGateway* getUpdater(TransactionSecrets&) const override; - virtual CLegacyUrlGateway* getUpdater(IEsdlScriptContext*) const override; + virtual CLegacyUrlGateway* getUpdater(ITransactionSecretsWrapper&) const override; public: // IGatewayUpdater virtual void updateGateway(IPTree& gw) override; protected: @@ -190,8 +214,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService class TLocalSecretGateway : public CUpdatableGateway { protected: - template - class TUpdater : public CInterfaceOf + class CUpdater : public CInterfaceOf { friend class TLocalSecretGateway; static constexpr const char* category = "espUser"; @@ -206,22 +229,16 @@ class EsdlServiceImpl : public CInterface, implements IEspService Linked> entry; Owned secret; public: - TUpdater(const TLocalSecretGateway& _entry, secret_source_t& secrets) + CUpdater(const TLocalSecretGateway& _entry, ITransactionSecretsWrapper& secrets) { entry.set(&_entry); secret.setown(secrets.getSecret(category, entry->identity)); } }; protected: // CUpdatableGateway - virtual IGatewayUpdater* getUpdater(TransactionSecrets& secrets) const override - { - return new TUpdater(*this, secrets); - } - virtual IGatewayUpdater* getUpdater(IEsdlScriptContext* scriptContext) const override + virtual IGatewayUpdater* getUpdater(ITransactionSecretsWrapper& secrets) const override { - if (!scriptContext) - return nullptr; - return new TUpdater(*this, *scriptContext); + return new CUpdater(*this, secrets); } protected: StringBuffer identifier; @@ -492,7 +509,7 @@ class EsdlServiceImpl : public CInterface, implements IEspService * @param updaters cache of updaters used in the current transaction * @param secrets cache of secrets used in the current transaction */ - void applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, IEsdlScriptContext* scriptContext) const; + void applyGatewayUpdates(IPTreeIterator& gwIt, const UpdatableGateways& updatables, GatewayUpdaters& updaters, ITransactionSecretsWrapper& secrets) const; /** * @brief Implementation of legacy gateway transformation invoked only during preparation of