Merge pull request #14793 from afishbeck/roxieUDPKey

HPCC-25680 Use cert-manager generated key for Roxie encryption in transit

Reviewed-By: Gavin Halliday <gavin.halliday@lexisnexis.com>
Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

helm/hpcc/templates/_helpers.tpl

@@ -61,6 +61,9 @@ Pass in root as .
 {{- $spillStoragePlane := ($spillStorage.plane | default "hpcc-spill-plane") -}}
 {{- $daliStoragePlane := ($daliStorage.plane | default "hpcc-dali-plane") -}}
 {{- $dllStoragePlane := ($dllStorage.plane | default "hpcc-dlls-plane") -}}
+{{- $certificates := (.Values.certificates | default dict) -}}
+{{- $issuers := ($certificates.issuers | default dict) -}}
+mtls: {{ and ($certificates.enabled) (hasKey $issuers "local") }}
 imageVersion: {{ .Values.global.image.version | default .Chart.Version }}
 singleNode: {{ .Values.global.singleNode | default false }}
 {{ if .Values.global.defaultEsp -}}

helm/hpcc/templates/roxie.yaml

@@ -10,10 +10,22 @@ data:
   {{ .me.name }}.yaml: |
     version: 1.0
     roxie:
-{{ toYaml ( omit .me "logging" "topoServer") | indent 6 }}
+{{ toYaml ( omit .me "logging" "topoServer" "encryptInTransit") | indent 6 }}
       numChannels: {{ .numChannels }}
       topologyServers: "{{ .toponame }}:{{ .topoport }}"
       resolveLocally: false
+{{- $certificates := (.root.Values.certificates | default dict) -}}
+{{- $issuers := ($certificates.issuers | default dict) -}}
+{{- $mtlsEnabled := (and ($certificates.enabled) (hasKey $issuers "local")) -}}
+{{/* By default use encryption if local certificates are enabled, but allow it to be turned off via roxie .encryptInTransit value */}}
+{{- if (hasKey .me "encryptInTransit") -}}
+  {{- if and (.me.encryptInTransit) (not $mtlsEnabled) -}}
+    {{- $_ := fail (printf "Roxie %s encryptInTransit requires local cert-manager configuration." .me.name ) }}
+  {{- end }}
+      encryptInTransit: {{ .me.encryptInTransit }}
+{{ else }}
+      encryptInTransit: {{ $mtlsEnabled }}
+{{ end -}}
 {{- include "hpcc.generateLoggingConfig" (dict "root" .root "me" .me) | indent 6 }}
 {{ include "hpcc.generateVaultConfig" . | indent 6 }}
     global:
@@ -89,9 +101,9 @@ spec:
 {{ include "hpcc.addConfigMapVolume" $toposerver | indent 6 }}
 {{ include "hpcc.addCertificateVolume" (dict "root" $ "component" "topo" "name" $commonCtx.toponame "external" false) | indent 6 }}
 
+---
 {{ include "hpcc.addCertificate" (dict "root" $ "name" $commonCtx.toponame "servicename" $commonCtx.toponame "component" "topo" "external" false) }}
 {{ include "hpcc.addUDPCertificate" (dict "root" $ "name" $udpkeyname "component" "udpkey") }}
----
 
 ---
 {{- range $service := $roxie.services }}

roxie/ccd/ccdmain.cpp

@@ -657,6 +657,8 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml)
         saveTopology();
         localAgent = topology->getPropBool("@localAgent", topology->getPropBool("@localSlave", false));  // legacy name
         encryptInTransit = topology->getPropBool("@encryptInTransit", false) && !localAgent;
+        if (encryptInTransit)
+            initSecretUdpKey();
         numChannels = topology->getPropInt("@numChannels", 0);
 #ifdef _CONTAINERIZED
         if (!numChannels)

roxie/ccd/ccdqueue.cpp

@@ -23,6 +23,7 @@
 #include <jlog.hpp>
 #include "jisem.hpp"
 #include "jencrypt.hpp"
+#include "jsecrets.hpp"
 
 #include "udplib.hpp"
 #include "udptopo.hpp"
@@ -493,14 +494,6 @@ public:
 
 };
 
-// MORE - this is for TESTING ONLY - do not release with this key here like this!
-
-static byte key[32] = {
-    0xf7, 0xe8, 0x79, 0x40, 0x44, 0x16, 0x66, 0x18, 0x52, 0xb8, 0x18, 0x6e, 0x76, 0xd1, 0x68, 0xd3,
-    0x87, 0x47, 0x01, 0xe6, 0x66, 0x62, 0x2f, 0xbe, 0xc1, 0xd5, 0x9f, 0x4a, 0x53, 0x27, 0xae, 0xa1,
-};
-
-
 class CRoxieQueryPacket : public CRoxieQueryPacketBase, implements IRoxieQueryPacket
 {
 protected:
@@ -632,7 +625,8 @@ public:
             plainData += traceLength;
             unsigned plainLen = length - sizeof(RoxiePacketHeader) - traceLength;
             mb.append(sizeof(RoxiePacketHeader)+traceLength, data);  // Header and traceInfo are unencrypted
-            aesEncrypt(key, sizeof(key), plainData, plainLen, mb);   // Encrypt everything else
+            const MemoryAttr &udpkey = getSecretUdpKey(true);
+            aesEncrypt(udpkey.get(), udpkey.length(), plainData, plainLen, mb);   // Encrypt everything else
             RoxiePacketHeader *newHeader = (RoxiePacketHeader *) mb.toByteArray();
             newHeader->packetlength = mb.length();
         }
@@ -734,7 +728,8 @@ public:
             encryptedData += traceLength;
             unsigned encryptedLen = length - sizeof(RoxiePacketHeader) - traceLength;
             mb.append(sizeof(RoxiePacketHeader)+traceLength, data);         // Header and traceInfo are unencrypted
-            aesDecrypt(key, sizeof(key), encryptedData, encryptedLen, mb);  // Decrypt everything else
+            const MemoryAttr &udpkey = getSecretUdpKey(true);
+            aesDecrypt(udpkey.get(), udpkey.length(), encryptedData, encryptedLen, mb);  // Decrypt everything else
             RoxiePacketHeader *newHeader = (RoxiePacketHeader *) mb.toByteArray();
             newHeader->packetlength = mb.length();
         }
@@ -782,7 +777,8 @@ extern IRoxieQueryPacket *deserializeCallbackPacket(MemoryBuffer &m)
         MemoryBuffer decrypted;
         decrypted.append(sizeof(RoxiePacketHeader), header);
         decrypted.ensureCapacity(encryptedLen);  // May be up to 16 bytes smaller...
-        aesDecrypt(key, sizeof(key), encryptedData, encryptedLen, decrypted);
+        const MemoryAttr &udpkey = getSecretUdpKey(true);
+        aesDecrypt(udpkey.get(), udpkey.length(), encryptedData, encryptedLen, decrypted);
         unsigned length = decrypted.length();
         RoxiePacketHeader *newHeader = (RoxiePacketHeader *) decrypted.detachOwn();
         newHeader->packetlength = length;

roxie/udplib/udpmsgpk.cpp

@@ -29,6 +29,7 @@
 #include "jlog.hpp"
 #include "jisem.hpp"
 #include "jencrypt.hpp"
+#include "jsecrets.hpp"
 
 #include "udplib.hpp"
 #include "udptrr.hpp"
@@ -57,11 +58,6 @@ int g_sequence_compare(const void *arg1, const void *arg2 )
     return 0;
 }
 
-static byte key[32] = {
-    0xf7, 0xe8, 0x79, 0x40, 0x44, 0x16, 0x66, 0x18, 0x52, 0xb8, 0x18, 0x6e, 0x76, 0xd1, 0x68, 0xd3,
-    0x87, 0x47, 0x01, 0xe6, 0x66, 0x62, 0x2f, 0xbe, 0xc1, 0xd5, 0x9f, 0x4a, 0x53, 0x27, 0xae, 0xa1,
-};
-
 class PackageSequencer : public CInterface, implements IInterface
 {
     DataBuffer *firstPacket;
@@ -245,7 +241,8 @@ public:
         {
             // MORE - This is decrypting in-place. Is that ok?? Seems to be with the code we currently use, but if that changed
             // might need to rethink this
-            size_t decryptedSize = aesDecrypt(key, sizeof(key), pktHdr+1, pktHdr->length-sizeof(UdpPacketHeader), pktHdr+1, DATA_PAYLOAD-sizeof(UdpPacketHeader));
+            const MemoryAttr &udpkey = getSecretUdpKey(true);
+            size_t decryptedSize = aesDecrypt(udpkey.get(), udpkey.length(), pktHdr+1, pktHdr->length-sizeof(UdpPacketHeader), pktHdr+1, DATA_PAYLOAD-sizeof(UdpPacketHeader));
             if (checkTraceLevel(TRACE_MSGPACK, 5))
                 DBGLOG("Decrypted %u bytes at %p resulting in %u bytes", (unsigned) (pktHdr->length-sizeof(UdpPacketHeader)), pktHdr+1, (unsigned) decryptedSize);
             pktHdr->length = decryptedSize + sizeof(UdpPacketHeader);

roxie/udplib/udptrs.cpp

@@ -23,6 +23,7 @@
 #include "jsocket.hpp"
 #include "jlog.hpp"
 #include "jencrypt.hpp"
+#include "jsecrets.hpp"
 #include "roxie.hpp"
 #ifdef _WIN32
 #include <winsock.h>
@@ -190,11 +191,6 @@ public:
 
 };
 
-static byte key[32] = {
-    0xf7, 0xe8, 0x79, 0x40, 0x44, 0x16, 0x66, 0x18, 0x52, 0xb8, 0x18, 0x6e, 0x76, 0xd1, 0x68, 0xd3,
-    0x87, 0x47, 0x01, 0xe6, 0x66, 0x62, 0x2f, 0xbe, 0xc1, 0xd5, 0x9f, 0x4a, 0x53, 0x27, 0xae, 0xa1,
-};
-
 class UdpReceiverEntry : public IUdpReceiverEntry
 {
     UdpReceiverEntry() = delete;
@@ -428,7 +424,8 @@ public:
                     encryptBuffer.append(sizeof(UdpPacketHeader), header);    // We don't encrypt the header
                     length -= sizeof(UdpPacketHeader);
                     const char *data = buffer->data + sizeof(UdpPacketHeader);
-                    aesEncrypt(key, sizeof(key), data, length, encryptBuffer);
+                    const MemoryAttr &udpkey = getSecretUdpKey(true);
+                    aesEncrypt(udpkey.get(), udpkey.length(), data, length, encryptBuffer);
                     header->length = encryptBuffer.length();
                     encryptBuffer.writeDirect(0, sizeof(UdpPacketHeader), header);   // Only really need length updating
                     assert(length <= DATA_PAYLOAD);

system/jlib/jsecrets.cpp

@@ -73,6 +73,8 @@ static Owned<IPropertyTree> secretCache;
 static CriticalSection mtlsInfoCacheCS;
 static Owned<IPropertyTree> mtlsInfoCache;
 static Owned<IVaultManager> vaultManager;
+static MemoryAttr udpKey;
+static bool udpKeyInitialized = false;
 
 MODULE_INIT(INIT_PRIORITY_SYSTEM)
 {
@@ -86,6 +88,7 @@ MODULE_EXIT()
     vaultManager.clear();
     secretCache.clear();
     mtlsInfoCache.clear();
+    udpKey.clear();
 }
 
 static void splitUrlAddress(const char *address, size_t len, StringBuffer &host, StringBuffer *port)
@@ -554,10 +557,15 @@ static const char *ensureSecretDirectory()
     return secretDirectory;
 }
 
+static StringBuffer &buildSecretPath(StringBuffer &path, const char *category, const char * name)
+{
+    return addPathSepChar(path.append(ensureSecretDirectory())).append(category).append(PATHSEPCHAR).append(name).append(PATHSEPCHAR);
+}
+
 static IPropertyTree *loadLocalSecret(const char *category, const char * name)
 {
     StringBuffer path;
-    addPathSepChar(path.append(ensureSecretDirectory())).append(category).append(PATHSEPCHAR).append(name).append(PATHSEPCHAR);
+    buildSecretPath(path, category, name);
     Owned<IDirectoryIterator> entries = createDirectoryIterator(path);
     if (!entries || !entries->first())
         return nullptr;
@@ -719,14 +727,17 @@ extern jlib_decl bool getSecretValue(StringBuffer & result, const char *category
     return true;
 }
 
-bool getSecretUdpKey(MemoryAttr &updkey)
+void initSecretUdpKey()
 {
-    bool ret = false;
-    updkey.clear();
-#if defined(_CONTAINERIZED) && defined(_USE_OPENSSL)
-    BIO *in = BIO_new_file("/opt/HPCCSystems/secrets/certificates/udp/tls.key", "r");
+    if (udpKeyInitialized)
+        return;
+
+//can find alternatives for old openssl in the future if necessary
+#if defined(_USE_OPENSSL) && (OPENSSL_VERSION_NUMBER >= 0x10100000L)
+    StringBuffer path;
+    BIO *in = BIO_new_file(buildSecretPath(path, "certificates", "udp").append("tls.key"), "r");
     if (in == nullptr)
-        return false;
+        return;
     EC_KEY *eckey = PEM_read_bio_ECPrivateKey(in, nullptr, nullptr, nullptr);
     if (eckey)
     {
@@ -734,15 +745,23 @@ bool getSecretUdpKey(MemoryAttr &updkey)
         size_t privlen = EC_KEY_priv2buf(eckey, &priv);
         if (privlen != 0)
         {
-            updkey.set(privlen, priv);
+            udpKey.set(privlen, priv);
             OPENSSL_clear_free(priv, privlen);
-            ret = true;
         }
         EC_KEY_free(eckey);
     }
     BIO_free(in);
 #endif
-    return ret;
+    udpKeyInitialized = true;
+}
+
+const MemoryAttr &getSecretUdpKey(bool required)
+{
+    if (!udpKeyInitialized)
+        throw makeStringException(-1, "UDP Key not initialized.");
+    if (required && !udpKey.length())
+        throw makeStringException(-1, "UDP Key not found, cert-manager integration/configuration required.");
+    return udpKey;
 }
 
 IPropertyTree *queryMtlsSecretInfo(const char *name)
@@ -757,7 +776,7 @@ IPropertyTree *queryMtlsSecretInfo(const char *name)
     StringBuffer filepath;
     StringBuffer secretpath;
 
-    addPathSepChar(secretpath.append(ensureSecretDirectory())).append("certificates").append(PATHSEPCHAR).append(name).append(PATHSEPCHAR);
+    buildSecretPath(secretpath, "certificates", name);
 
     filepath.set(secretpath).append("tls.crt");
     if (!checkFileExists(filepath))
@@ -782,7 +801,6 @@ IPropertyTree *queryMtlsSecretInfo(const char *name)
         verify->setPropBool("@address_match", false);
         verify->setPropBool("@accept_selfsigned", false);
         verify->setProp("trusted_peers", "anyone");
-
     }
     return info;
-}
+}

system/jlib/jsecrets.hpp

@@ -32,7 +32,10 @@ extern jlib_decl IPropertyTree *getSecret(const char *category, const char * nam
 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 getSecretValue(StringBuffer & result, const char *category, const char * name, const char * key, bool required);
-extern jlib_decl bool getSecretUdpKey(MemoryAttr &updkey);
+
+extern jlib_decl void initSecretUdpKey();
+extern jlib_decl const MemoryAttr &getSecretUdpKey(bool required);
+
 extern jlib_decl IPropertyTree *queryMtlsSecretInfo(const char *name);
 
 extern jlib_decl  void splitFullUrl(const char *url, bool &https, StringBuffer &user, StringBuffer &password, StringBuffer &host, StringBuffer &port, StringBuffer &fullpath);