From 20a7da0f6b201de46927da90ad8d80066679a419 Mon Sep 17 00:00:00 2001 From: Gavin Halliday Date: Wed, 25 Oct 2023 17:10:47 +0100 Subject: [PATCH] Add option to disable mtls for dafilesrv Signed-off-by: Gavin Halliday --- fs/dafsserver/dafsserver.cpp | 13 +---------- roxie/ccd/ccdmain.cpp | 2 +- system/jlib/jsecrets.cpp | 23 ++++++++++--------- system/jlib/jsecrets.hpp | 6 +++-- system/jlib/jsmartsock.cpp | 2 +- system/security/securesocket/securesocket.cpp | 2 +- 6 files changed, 20 insertions(+), 28 deletions(-) diff --git a/fs/dafsserver/dafsserver.cpp b/fs/dafsserver/dafsserver.cpp index 4184d99d80b..9bee295c8dd 100644 --- a/fs/dafsserver/dafsserver.cpp +++ b/fs/dafsserver/dafsserver.cpp @@ -140,20 +140,9 @@ static ISecureSocket *createSecureSocket(ISocket *sock, bool disableClientCertVe */ const char *certScope = strsame("cluster", getComponentConfigSP()->queryProp("service/@visibility")) ? "local" : "public"; - Owned info = getIssuerTlsSyncedConfig(certScope); + Owned info = getIssuerTlsSyncedConfig(certScope, nullptr, disableClientCertVerification); if (!info || !info->isValid()) throw makeStringException(-1, "createSecureSocket() : missing MTLS configuration"); - Owned cloneInfo; - if (disableClientCertVerification) - { - // do not insist clients provide a certificate for verification. - // This is used when the connection is TLS, but the authentication is done via other means - // e.g. in the case of the streaming service a opaque signed blob is transmitted and must - // be verified before proceeding. - cloneInfo.setown(createPTreeFromIPT(info)); - cloneInfo->setPropBool("verify/@enable", false); - info.set(cloneInfo); - } secureContextServer.setown(createSecureSocketContextSynced(info, ServerSocket)); #else secureContextServer.setown(createSecureSocketContextEx2(securitySettings.getSecureConfig(), ServerSocket)); diff --git a/roxie/ccd/ccdmain.cpp b/roxie/ccd/ccdmain.cpp index c0f13791c97..40a81cfcc13 100644 --- a/roxie/ccd/ccdmain.cpp +++ b/roxie/ccd/ccdmain.cpp @@ -1501,7 +1501,7 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml) { roxiePort = port; if (roxieFarm.getPropBool("@tls")) - roxiePortTlsClientConfig = createIssuerTlsConfig(roxieFarm.queryProp("@issuer"), nullptr, true, roxieFarm.getPropBool("@selfSigned"), true); + roxiePortTlsClientConfig = createIssuerTlsConfig(roxieFarm.queryProp("@issuer"), nullptr, true, roxieFarm.getPropBool("@selfSigned"), true, false); debugEndpoint.set(roxiePort, ip); } bool suspended = roxieFarm.getPropBool("@suspended", false); diff --git a/system/jlib/jsecrets.cpp b/system/jlib/jsecrets.cpp index 3d761b59c55..012b86bd72e 100644 --- a/system/jlib/jsecrets.cpp +++ b/system/jlib/jsecrets.cpp @@ -1430,8 +1430,8 @@ void CSyncedCertificateBase::updateCertificateAuthorityFromSecret(const IPropert class CIssuerConfig final : public CSyncedCertificateBase { public: - CIssuerConfig(const char *_issuer, const char * _trustedPeers, bool _isClientConnection, bool _acceptSelfSigned, bool _addCACert) - : CSyncedCertificateBase(_issuer), trustedPeers(_trustedPeers), isClientConnection(_isClientConnection), acceptSelfSigned(_acceptSelfSigned), addCACert(_addCACert) + CIssuerConfig(const char *_issuer, const char * _trustedPeers, bool _isClientConnection, bool _acceptSelfSigned, bool _addCACert, bool _disableMTLS) + : CSyncedCertificateBase(_issuer), trustedPeers(_trustedPeers), isClientConnection(_isClientConnection), acceptSelfSigned(_acceptSelfSigned), addCACert(_addCACert), disableMTLS(_disableMTLS) { secret.setown(resolveSecret("certificates", issuer, nullptr, nullptr)); createConfig(); @@ -1444,6 +1444,7 @@ class CIssuerConfig final : public CSyncedCertificateBase bool isClientConnection; // required in constructor bool acceptSelfSigned; // required in constructor bool addCACert; // required in constructor + bool disableMTLS; }; @@ -1457,7 +1458,7 @@ void CIssuerConfig::updateConfigFromSecret(const IPropertyTree * secretInfo) con IPropertyTree *verify = config->queryPropTree("verify"); //For now only the "public" issuer implies client certificates are not required - verify->setPropBool("@enable", isClientConnection || !strieq(issuer, "public")); + verify->setPropBool("@enable", !disableMTLS && (isClientConnection || !strieq(issuer, "public"))); verify->setPropBool("@address_match", false); verify->setPropBool("@accept_selfsigned", isClientConnection && acceptSelfSigned); if (trustedPeers) // Allow blank string to mean none, null means anyone @@ -1467,9 +1468,9 @@ void CIssuerConfig::updateConfigFromSecret(const IPropertyTree * secretInfo) con } -ISyncedPropertyTree * createIssuerTlsConfig(const char * issuer, const char * optTrustedPeers, bool isClientConnection, bool acceptSelfSigned, bool addCACert) +ISyncedPropertyTree * createIssuerTlsConfig(const char * issuer, const char * optTrustedPeers, bool isClientConnection, bool acceptSelfSigned, bool addCACert, bool disableMTLS) { - return new CIssuerConfig(issuer, optTrustedPeers, isClientConnection, acceptSelfSigned, addCACert); + return new CIssuerConfig(issuer, optTrustedPeers, isClientConnection, acceptSelfSigned, addCACert, disableMTLS); } //--------------------------------------------------------------------------------------------------------------------- @@ -1508,16 +1509,16 @@ ISyncedPropertyTree * createStorageTlsConfig(const char * secretName, bool addCA } -const ISyncedPropertyTree * getIssuerTlsSyncedConfig(const char * issuer, const char * optTrustedPeers) +const ISyncedPropertyTree * getIssuerTlsSyncedConfig(const char * issuer, const char * optTrustedPeers, bool disableMTLS) { if (isEmptyString(issuer)) return nullptr; const char * key; StringBuffer temp; - if (!isEmptyString(optTrustedPeers)) + if (!isEmptyString(optTrustedPeers) || disableMTLS) { - temp.append(issuer).append("/").append(optTrustedPeers); + temp.append(issuer).append("/").append(optTrustedPeers).append('/').append(disableMTLS); key = temp.str(); } else @@ -1528,20 +1529,20 @@ const ISyncedPropertyTree * getIssuerTlsSyncedConfig(const char * issuer, const if (match != mtlsInfoCache.cend()) return LINK(match->second); - Owned config = createIssuerTlsConfig(issuer, optTrustedPeers, false, false, true); + Owned config = createIssuerTlsConfig(issuer, optTrustedPeers, false, false, true, disableMTLS); mtlsInfoCache.emplace(key, config); return config.getClear(); } bool hasIssuerTlsConfig(const char *issuer) { - Owned match = getIssuerTlsSyncedConfig(issuer, nullptr); + Owned match = getIssuerTlsSyncedConfig(issuer, nullptr, false); return match && match->isValid(); } const IPropertyTree *getIssuerTlsServerConfigWithTrustedPeers(const char *issuer, const char *trusted_peers) { - Owned match = getIssuerTlsSyncedConfig(issuer, trusted_peers); + Owned match = getIssuerTlsSyncedConfig(issuer, trusted_peers, false); if (!match) return nullptr; return match->getTree(); diff --git a/system/jlib/jsecrets.hpp b/system/jlib/jsecrets.hpp index 1172e453c54..888000cef67 100644 --- a/system/jlib/jsecrets.hpp +++ b/system/jlib/jsecrets.hpp @@ -40,11 +40,13 @@ extern jlib_decl const MemoryAttr &getSecretUdpKey(bool required); extern jlib_decl bool containsEmbeddedKey(const char *certificate); //getIssuerTlsConfig must return owned because the internal cache could be updated internally and the return will become invalid, so must be linked -extern jlib_decl const ISyncedPropertyTree * getIssuerTlsSyncedConfig(const char * issuer, const char * optTrustedPeers = nullptr); +extern jlib_decl const ISyncedPropertyTree * getIssuerTlsSyncedConfig(const char * issuer, const char * optTrustedPeers, bool disableMTLS); +inline const ISyncedPropertyTree * getIssuerTlsSyncedConfig(const char * issuer) { return getIssuerTlsSyncedConfig(issuer, nullptr, false); } + extern jlib_decl bool hasIssuerTlsConfig(const char *issuer); extern jlib_decl const IPropertyTree *getIssuerTlsServerConfigWithTrustedPeers(const char *issuer, const char *trusted_peers); -extern jlib_decl ISyncedPropertyTree * createIssuerTlsConfig(const char * issuer, const char * optTrustedPeers, bool isClientConnection, bool acceptSelfSigned, bool addCACert); +extern jlib_decl ISyncedPropertyTree * createIssuerTlsConfig(const char * issuer, const char * optTrustedPeers, bool isClientConnection, bool acceptSelfSigned, bool addCACert, bool disableMTLS); extern jlib_decl ISyncedPropertyTree * createStorageTlsConfig(const char * secretName, bool addCACert); extern jlib_decl void splitFullUrl(const char *url, bool &https, StringBuffer &user, StringBuffer &password, StringBuffer &host, StringBuffer &port, StringBuffer &fullpath); diff --git a/system/jlib/jsmartsock.cpp b/system/jlib/jsmartsock.cpp index 1bfddd7d9fb..e8644827297 100644 --- a/system/jlib/jsmartsock.cpp +++ b/system/jlib/jsmartsock.cpp @@ -218,7 +218,7 @@ CSmartSocketFactory::CSmartSocketFactory(IPropertyTree &service, bool _retry, un tlsService = service.getPropBool("@tls"); issuer.set(service.queryProp("@issuer")); if (tlsService) - tlsConfig.setown(createIssuerTlsConfig(issuer, nullptr, true, service.getPropBool("@selfSigned"), service.getPropBool("@caCert"))); + tlsConfig.setown(createIssuerTlsConfig(issuer, nullptr, true, service.getPropBool("@selfSigned"), service.getPropBool("@caCert"), false)); StringBuffer s; s.append(name).append(':').append(port); diff --git a/system/security/securesocket/securesocket.cpp b/system/security/securesocket/securesocket.cpp index 36710efc161..e0558ae0c42 100644 --- a/system/security/securesocket/securesocket.cpp +++ b/system/security/securesocket/securesocket.cpp @@ -186,7 +186,7 @@ class CSecureSocket : implements ISecureSocket, public CInterface //Check if a new ssl context should be created. //No need for a critical section because the socket functions are never accessed by multiple threads at the same time //It is possible that createActiveSSL() may be for a later version - but that will only mean that the same context - //is recreated when the version number is seen to have chanegd. + //is recreated when the version number is seen to have changed. unsigned activeVersion = contextCallback->getVersion(); if (activeVersion != contextVersion) {