Skip to content

Commit

Permalink
Merge pull request #17788 from ghalliday/issue30304
Browse files Browse the repository at this point in the history 
HPCC-30304 Introduce ISecret and simplify the secret calling code

Reviewed-By: Anthony Fishbeck <[email protected]>
Merged-by: Gavin Halliday <[email protected]>
  • Loading branch information
@ghalliday
ghalliday authored Sep 21, 2023
2 parents 44bc305 + e2aadb3 commit 2078c1b
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 34 deletions.
2 changes: 1 addition & 1 deletion common/thorhelper/thorsoapcall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ class CWSCHelper : implements IWSCHelper, public CInterface
}
StringBuffer secretName("http-connect-");
secretName.append(finger);
Owned<IPropertyTree> secret = (vaultId.isEmpty()) ? getSecret("ecl", secretName) : getVaultSecret("ecl", vaultId, secretName, nullptr);
Owned<IPropertyTree> secret = getSecret("ecl", secretName, vaultId, nullptr);
if (!secret)
throw MakeStringException(0, "%sCALL %s SECRET not found", wscType == STsoap ? "SOAP" : "HTTP", secretName.str());

Expand Down
4 changes: 1 addition & 3 deletions esp/esdlscriptlib/esdl_script.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -929,9 +929,7 @@ class CEsdlTransformOperationMySqlCall : public CEsdlTransformOperationBase
StringBuffer vault;
if (m_vaultName)
sourceContext->evaluateAsString(m_vaultName, vault);
if (vault.isEmpty())
return getSecret("esp", name);
return getVaultSecret("esp", vault, name);
return getSecret("esp", name, vault);
}
void appendOption(StringBuffer &options, const char *name, const char *value, bool required)
{
Expand Down
171 changes: 156 additions & 15 deletions system/jlib/jsecrets.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ MODULE_EXIT()
udpKey.clear();
}

static IPropertyTree *getLocalSecret(const char *category, const char * name);

//based on kubernetes secret / key names. Even if some vault backends support additional characters we'll restrict to this subset for now

static const char *validSecretNameChrs = ".-";
Expand Down Expand Up @@ -272,14 +274,19 @@ extern jlib_decl void setSecretMount(const char * path)
secretDirectory.set(path);
}

static inline bool checkSecretExpired(unsigned created)
static bool checkSecretExpired(unsigned created)
{
if (!created)
return false;
unsigned age = msTick() - created;
return age > getSecretTimeout();
}

static bool hasCacheExpired(const IPropertyTree * secret)
{
return checkSecretExpired((unsigned)secret->getPropInt("@created"));
}

enum class VaultAuthType {unknown, k8s, appRole, token};

static void setTimevalMS(timeval &tv, time_t ms)
Expand Down Expand Up @@ -570,7 +577,7 @@ class CVault
IPropertyTree *envelope = tree->queryPropTree(vername);
if (!envelope)
return false;
if (checkSecretExpired((unsigned) envelope->getPropInt("@created")))
if (hasCacheExpired(envelope))
{
tree->removeTree(envelope);
return false;
Expand Down Expand Up @@ -813,7 +820,7 @@ static IPropertyTree *getCachedLocalSecret(const char *category, const char *nam
secret.setown(tree->getPropTree(name));
if (secret)
{
if (checkSecretExpired((unsigned) secret->getPropInt("@created")))
if (hasCacheExpired(secret))
{
secretCache->removeProp(name);
return nullptr;
Expand Down Expand Up @@ -877,7 +884,7 @@ static IPropertyTree *loadLocalSecret(const char *category, const char * name)
return tree.getClear();
}

extern jlib_decl IPropertyTree *getLocalSecret(const char *category, const char * name)
static IPropertyTree *getLocalSecret(const char *category, const char * name)
{
validateCategoryName(category);
validateSecretName(name);
Expand Down Expand Up @@ -944,11 +951,8 @@ static IPropertyTree *requestVaultSecret(const char *category, const char *vault
return createPTreeFromVaultSecret(json.str(), kind);
}

extern jlib_decl IPropertyTree *getVaultSecret(const char *category, const char *vaultId, const char * name, const char *version)
static IPropertyTree *getVaultSecret(const char *category, const char * name, const char *vaultId, const char *version)
{
validateCategoryName(category);
validateSecretName(name);

CVaultKind kind;
StringBuffer json;
IVaultManager *vaultmgr = ensureVaultManager();
Expand All @@ -965,10 +969,10 @@ extern jlib_decl IPropertyTree *getVaultSecret(const char *category, const char
return createPTreeFromVaultSecret(json.str(), kind);
}

extern jlib_decl IPropertyTree *getSecret(const char *category, const char * name)
IPropertyTree *getSecretTree(const char *category, const char * name, const char * optVaultId, const char * optVersion)
{
validateCategoryName(category);
validateSecretName(name);
if (!isEmptyString(optVaultId))
return getVaultSecret(category, name, optVaultId, optVersion);

//check for any chached first
Owned<IPropertyTree> secret = getCachedLocalSecret(category, name);
Expand All @@ -982,20 +986,35 @@ extern jlib_decl IPropertyTree *getSecret(const char *category, const char * nam
return secret.getClear();
}

extern jlib_decl bool getSecretKeyValue(MemoryBuffer & result, IPropertyTree *secret, const char * key)
IPropertyTree *getSecret(const char *category, const char * name, const char * optVaultId, const char * optVersion)
{
validateCategoryName(category);
validateSecretName(name);

return getSecretTree(category, name, optVaultId, optVersion);
}


bool getSecretKeyValue(MemoryBuffer & result, const IPropertyTree *secret, const char * key)
{
validateKeyName(key);

if (!secret)
return false;

IPropertyTree *tree = secret->queryPropTree(key);
if (tree)
return tree->getPropBin(nullptr, result);
return false;
}

extern jlib_decl bool getSecretKeyValue(StringBuffer & result, IPropertyTree *secret, const char * key)
bool getSecretKeyValue(StringBuffer & result, const IPropertyTree *secret, const char * key)
{
validateKeyName(key);

if (!secret)
return false;

IPropertyTree *tree = secret->queryPropTree(key);
if (!tree)
return false;
Expand All @@ -1018,8 +1037,6 @@ extern jlib_decl bool getSecretKeyValue(StringBuffer & result, IPropertyTree *se

extern jlib_decl bool getSecretValue(StringBuffer & result, const char *category, const char * name, const char * key, bool required)
{
validateKeyName(key); //name and category validated in getSecret

Owned<IPropertyTree> secret = getSecret(category, name);
if (required && !secret)
throw MakeStringException(-1, "secret %s.%s not found", category, name);
Expand All @@ -1029,6 +1046,130 @@ extern jlib_decl bool getSecretValue(StringBuffer & result, const char *category
return true;
}

//---------------------------------------------------------------------------------------------------------------------

class CSecret final : public CInterfaceOf<ISecret>
{
public:
CSecret(const char *_category, const char * _name, const char * _vaultId, const char * _version, const IPropertyTree * _secret)
: category(_category), name(_name), vaultId(_vaultId), version(_version), secret(_secret)
{
updateHash();
}

virtual const IPropertyTree * getTree() const;

virtual bool getKeyValue(MemoryBuffer & result, const char * key) const
{
CriticalBlock block(secretCs);
checkStale();
return getSecretKeyValue(result, secret, key);
}
virtual bool getKeyValue(StringBuffer & result, const char * key) const
{
CriticalBlock block(secretCs);
checkStale();
return getSecretKeyValue(result, secret, key);
}
virtual bool isStale() const
{
return secret && hasCacheExpired(secret);
}
virtual unsigned getVersion() const
{
return secretHash;
}

protected:
void checkStale() const;
void updateHash() const;

protected:
StringAttr category;
StringAttr name;
StringAttr vaultId;
StringAttr version;
mutable CriticalSection secretCs;
mutable Linked<const IPropertyTree> secret;
mutable unsigned secretHash = 0;
};


const IPropertyTree * CSecret::getTree() const
{
CriticalBlock block(secretCs);
checkStale();
return LINK(secret);
}

void CSecret::checkStale() const
{
if (isStale())
{
//MORE: This could block or fail - in roxie especially it would be better to return the old value
try
{
secret.setown(getSecretTree(category, name, vaultId, version));
updateHash();
}
catch (IException * e)
{
VStringBuffer msg("Failed to update secret %s.%s", category.str(), name.str());
EXCLOG(e, msg.str());
e->Release();
}
}
}

//This should probably move to jptree.?pp as a generally useful function
static unsigned calculateTreeHash(const IPropertyTree & source, unsigned hashcode)
{
if (source.isBinary())
{
MemoryBuffer mb;
source.getPropBin(nullptr, mb);
hashcode = hashc((const byte *)mb.bufferBase(), mb.length(), hashcode);
}
else
{
const char * value = source.queryProp(nullptr);
if (value)
hashcode = hashcz((const byte *)value, hashcode);
}

Owned<IAttributeIterator> aiter = source.getAttributes();
ForEach(*aiter)
{
hashcode = hashcz((const byte *)aiter->queryName(), hashcode);
hashcode = hashcz((const byte *)aiter->queryValue(), hashcode);
}

Owned<IPropertyTreeIterator> iter = source.getElements("*");
ForEach(*iter)
{
IPropertyTree & child = iter->query();
hashcode = hashcz((const byte *)child.queryName(), hashcode);
hashcode = calculateTreeHash(child, hashcode);
}
return hashcode;
}

void CSecret::updateHash() const
{
if (secret)
secretHash = calculateTreeHash(*secret.get(), 0x811C9DC5);
else
secretHash = 0;
}

ISecret * resolveSecret(const char *category, const char * name, const char * optVaultId, const char * optVersion)
{
Owned<IPropertyTree> resolved = getSecret(category, name, optVaultId, optVersion);
return new CSecret(category, name, optVaultId, optVersion, resolved);
}

//---------------------------------------------------------------------------------------------------------------------

void initSecretUdpKey()
{
if (udpKeyInitialized)
Expand Down
20 changes: 15 additions & 5 deletions system/jlib/jsecrets.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,25 @@
#include "jlib.hpp"
#include "jstring.hpp"

interface ISecret : extends IInterface
{
virtual const IPropertyTree * getTree() const = 0;
virtual bool getKeyValue(MemoryBuffer & result, const char * key) const = 0;
virtual bool getKeyValue(StringBuffer & result, const char * key) const = 0;
virtual bool isStale() const = 0;
//Return a sequence number which changes whenever the secret actually changes - so that a caller can determine
//whether it needs to reload the certificates.
virtual unsigned getVersion() const = 0;
};

extern jlib_decl void setSecretMount(const char * path);
extern jlib_decl void setSecretTimeout(unsigned timeoutMs);

extern jlib_decl IPropertyTree *getLocalSecret(const char *category, const char * name);
extern jlib_decl IPropertyTree *getVaultSecret(const char *category, const char *vaultId, const char * name, const char *version=nullptr);
extern jlib_decl IPropertyTree *getSecret(const char *category, const char * name);
extern jlib_decl IPropertyTree *getSecret(const char *category, const char * name, const char * optVaultId = nullptr, const char * optVersion = nullptr);
extern jlib_decl ISecret * resolveSecret(const char *category, const char * name, const char * optRequiredVault);

extern jlib_decl bool getSecretKeyValue(MemoryBuffer & result, IPropertyTree *secret, const char * key);
extern jlib_decl bool getSecretKeyValue(StringBuffer & result, IPropertyTree *secret, const char * key);
extern jlib_decl bool getSecretKeyValue(MemoryBuffer & result, const IPropertyTree *secret, const char * key);
extern jlib_decl bool getSecretKeyValue(StringBuffer & result, const IPropertyTree *secret, const char * key);
extern jlib_decl bool getSecretValue(StringBuffer & result, const char *category, const char * name, const char * key, bool required);

extern jlib_decl void initSecretUdpKey();
Expand Down
13 changes: 3 additions & 10 deletions system/security/LdapSecurity/ldapconnection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,11 +408,8 @@ class CLdapConfig : implements ILdapConfig, public CInterface
cfg->getProp(".//@ldapAdminVaultId", vaultId);//optional HashiCorp vault ID

DBGLOG("Retrieving LDAP Admin username/password from secrets repo: %s %s", !vaultId.isEmpty() ? vaultId.str() : "", adminUserSecretKey.str());
Owned<IPropertyTree> secretTree;
if (!vaultId.isEmpty())
secretTree.setown(getVaultSecret("authn", vaultId, adminUserSecretKey.str(), nullptr));
else
secretTree.setown(getSecret("authn", adminUserSecretKey.str()));

Owned<IPropertyTree> secretTree(getSecret("authn", adminUserSecretKey.str(), vaultId, nullptr));
if (!secretTree)
throw MakeStringException(-1, "Error retrieving LDAP Admin username/password");

Expand Down Expand Up @@ -497,11 +494,7 @@ class CLdapConfig : implements ILdapConfig, public CInterface
cfg->getProp(".//@hpccAdminVaultId", vaultId);//optional HashiCorp vault ID

DBGLOG("Retrieving optional HPCC Admin username/password from secrets repo: %s %s", !vaultId.isEmpty() ? vaultId.str() : "", adminUserSecretKey.str());
Owned<IPropertyTree> secretTree;
if (!vaultId.isEmpty())
secretTree.setown(getVaultSecret("authn", vaultId, adminUserSecretKey.str(), nullptr));
else
secretTree.setown(getSecret("authn", adminUserSecretKey.str()));
Owned<IPropertyTree> secretTree(getSecret("authn", adminUserSecretKey.str(), vaultId, nullptr));
if (secretTree)
{
getSecretKeyValue(m_HPCCAdminUser_username, secretTree, "username");
Expand Down

0 comments on commit 2078c1b

Please sign in to comment.