Various fixes (#339)

* Update builder submodule

* Move g_key global variable into the WforceReplication class

* Append dist tag to Release in wforce spec file

* Add setKey() Lua function to trackalert (it was moved out of common-lua.cc)

* Coverity issue: Initialize sodium nonce before using it

* Make getEncryptionKey() a const method

* Make all the methods in WforceReplication virtual except the basic set/get methods

* Make setKey() Lua function return boolean to indicate status and update docs

builder-support/specs/wforce.spec

@@ -23,7 +23,7 @@
 Summary: Weakforce daemon for detecting brute force attacts
 Name: wforce
 Version: %{getenv:BUILDER_RPM_VERSION}
-Release: %{getenv:BUILDER_RPM_RELEASE}
+Release: %{getenv:BUILDER_RPM_RELEASE}%{?dist}
 License: GPLv3
 Group: System Environment/Daemons
 URL: http://www.open-xchange.com/

common/common-lua.cc

@@ -340,56 +340,11 @@ void setupCommonLua(bool client,
 
   if (!multi_lua) {
     c_lua.writeFunction("makeKey", []() {
-	g_outputBuffer="setKey("+newKey()+")\n";
-      });
-  }
-  else {
-    c_lua.writeFunction("makeKey", []() { });    
-  }
-
-  if (!multi_lua) {
-    c_lua.writeFunction("setKey", [](const std::string& key) {
-	string newkey;
-	if(B64Decode(key, newkey) < 0) {
-	  g_outputBuffer=string("Unable to decode ")+key+" as Base64";
-	  errlog("%s", g_outputBuffer);
-	}
-	else
-	  g_key = newkey;
-      });
-  }
-  else {
-    c_lua.writeFunction("setKey", [](const std::string& key) { });
-  }
-
-  if (!multi_lua) {
-    c_lua.writeFunction("testCrypto", [](const std::string& testmsg)
-			{
-			  try {
-			    SodiumNonce sn, sn2;
-			    sn.init();
-			    sn2=sn;
-			    string encrypted = sodEncryptSym(testmsg, g_key, sn);
-			    string decrypted = sodDecryptSym(encrypted, g_key, sn2);
-
-			    sn.increment();
-			    sn2.increment();
-
-			    encrypted = sodEncryptSym(testmsg, g_key, sn);
-			    decrypted = sodDecryptSym(encrypted, g_key, sn2);
-
-			    if(testmsg == decrypted)
-			      g_outputBuffer="Everything is ok!\n";
-			    else
-			      g_outputBuffer="Crypto failed..\n";
-
-			  }
-			  catch(...) {
-			    g_outputBuffer="Crypto failed..\n";
-			  }});
+      g_outputBuffer="setKey("+newKey()+")\n";
+    });
   }
   else {
-    c_lua.writeFunction("testCrypto", [](const string& testmsg) {});
+    c_lua.writeFunction("makeKey", []() { });
   }
 
 #ifdef HAVE_GEOIP

common/common-lua.hh

@@ -52,8 +52,6 @@ extern WebHookDB g_custom_webhook_db;
 
 extern int g_num_luastates;
 
-extern std::string g_key; // in theory needs locking
-
 void setupCommonLua(bool client,
                     bool multi_lua,
                     LuaContext& c_lua,

docs/manpages/trackalert.conf.5.md

@@ -64,9 +64,13 @@ cannot be called inside the report or background functions:
 * setKey(\<key\>) - Use the specified key for authenticating 
   connections from siblings. The key must be generated with makeKey()
   from the console. See *trackalert(1)* for instructions on
-  running a console client. For example:
-
-		setKey("Ay9KXgU3g4ygK+qWT0Ut4gH8PPz02gbtPeXWPdjD0HE=")
+  running a console client.
+  Returns false if the key could not be set (e.g. invalid base64).
+  For example:
+
+        if not setKey("Ay9KXgU3g4ygK+qWT0Ut4gH8PPz02gbtPeXWPdjD0HE=")
+            then
+              ...
 
 * setNumLuaStates(\<num states\>) - Set the number of Lua Contexts that
   will be created to run report commands. Defaults to 10

docs/manpages/wforce.conf.5.md

@@ -159,11 +159,15 @@ cannot be called inside the allow/report/reset functions:
 		controlSocket("0.0.0.0:4004")
 
 * setKey(\<key\>) - Use the specified key for authenticating 
-  connections from siblings. The key must be generated with makeKey()
+  connections from siblings. The key can be generated with makeKey()
   from the console. See *wforce(1)* for instructions on
-  running a console client. For example:
-
-		setKey("Ay9KXgU3g4ygK+qWT0Ut4gH8PPz02gbtPeXWPdjD0HE=")
+  running a console client.
+  Returns false if the key could not be set (e.g. invalid base64).
+  For example:
+
+        if not setKey("Ay9KXgU3g4ygK+qWT0Ut4gH8PPz02gbtPeXWPdjD0HE=")
+            then
+              ...
 
 * setNumLuaStates(\<num states\>) - Set the number of Lua Contexts that
   will be created to run allow/report/reset commands. Defaults to 10
@@ -1083,4 +1087,4 @@ a Netmask. For example:
 # SEE ALSO
 wforce(1) wforce_webhook(5)
 
-<!-- {% endraw %} -->
+<!-- {% endraw %} -->

trackalert/trackalert-lua.cc

@@ -297,6 +297,24 @@ vector<std::function<void(void)>> setupLua(bool client, bool multi_lua,
     c_lua.writeFunction("showVersion", []() { });
   }
 
+  if (!multi_lua) {
+    c_lua.writeFunction("setKey", [](const std::string& key) -> bool {
+      string newkey;
+      if(B64Decode(key, newkey) < 0) {
+        g_outputBuffer=string("Unable to decode ")+key+" as Base64";
+        errlog("%s", g_outputBuffer);
+        return false;
+      }
+      else {
+        g_key = newkey;
+        return true;
+      }
+    });
+  }
+  else {
+    c_lua.writeFunction("setKey", [](const std::string& key) { });
+  }
+
   if (!multi_lua) {
     c_lua.writeFunction("testCrypto", [](string testmsg)
     {

wforce/wforce-lua.cc

@@ -1002,21 +1002,40 @@ vector<std::function<void(void)>> setupLua(bool client, bool multi_lua, LuaConte
     c_lua.writeFunction("showVersion", []() { });
   }
 
+  if (!multi_lua) {
+    c_lua.writeFunction("setKey", [](const std::string& key) -> bool {
+      string newkey;
+      if(B64Decode(key, newkey) < 0) {
+        g_outputBuffer=string("Unable to decode ")+key+" as Base64";
+        errlog("%s", g_outputBuffer);
+        return false;
+      }
+      else {
+        g_replication.setEncryptionKey(newkey);
+        return true;
+      }
+    });
+  }
+  else {
+    c_lua.writeFunction("setKey", [](const std::string& key) { });
+  }
+
   if (!multi_lua) {
     c_lua.writeFunction("testCrypto", [](string testmsg)
     {
       try {
         SodiumNonce sn, sn2;
         sn.init();
         sn2=sn;
-        string encrypted = sodEncryptSym(testmsg, g_key, sn);
-        string decrypted = sodDecryptSym(encrypted, g_key, sn2);
+        std::string key = g_replication.getEncryptionKey();
+        string encrypted = sodEncryptSym(testmsg, key, sn);
+        string decrypted = sodDecryptSym(encrypted, key, sn2);
 
         sn.increment();
         sn2.increment();
 
-        encrypted = sodEncryptSym(testmsg, g_key, sn);
-        decrypted = sodDecryptSym(encrypted, g_key, sn2);
+        encrypted = sodEncryptSym(testmsg, key, sn);
+        decrypted = sodDecryptSym(encrypted, key, sn2);
 
         if(testmsg == decrypted)
           g_outputBuffer="Everything is ok!\n";

wforce/wforce-replication.cc

@@ -22,7 +22,6 @@
 
 #include "config.h"
 #include <stddef.h>
-#include "wforce.hh"
 #include "wforce_ns.hh"
 #include "sstuff.hh"
 #include "misc.hh"
@@ -49,7 +48,7 @@ void WforceReplication::encryptMsg(const std::string& msg, std::string& packet)
 {
     std::lock_guard<std::mutex> lock(d_sod_mutx);
     packet=d_sodnonce.toString();
-    packet+=sodEncryptSym(msg, g_key, d_sodnonce);
+    packet+=sodEncryptSym(msg, d_key, d_sodnonce);
 }
 
 void WforceReplication::encryptMsgWithKey(const std::string& msg, std::string& packet, const std::string& key, SodiumNonce& nonce, std::mutex& mutex)
@@ -70,7 +69,7 @@ bool WforceReplication::decryptMsg(const char* buf, size_t len, std::string& msg
   memcpy((char*)&nonce, buf, crypto_secretbox_NONCEBYTES);
   string packet(buf + crypto_secretbox_NONCEBYTES, buf+len);
   try {
-    msg=sodDecryptSym(packet, g_key, nonce);
+    msg=sodDecryptSym(packet, d_key, nonce);
   }
   catch (std::runtime_error& e) {
     errlog("Could not decrypt replication operation: %s", e.what());
@@ -90,7 +89,7 @@ void WforceReplication::replicateOperation(const ReplicationOperation& rep_op)
   for(auto& s : *siblings) {
     bool use_sibling_packet = false;
     if (s->d_has_key) {
-      if (s->d_key != g_key) {
+      if (s->d_key != d_key) {
         encryptMsgWithKey(msg, sibling_packet, s->d_key, s->d_nonce, s->mutx);
         use_sibling_packet = true;
       }

wforce/wforce-replication.hh

@@ -22,7 +22,6 @@
 
 #pragma once
 
-#include "luastate.hh"
 #include "sholder.hh"
 #include "iputils.hh"
 #include "sodcrypto.hh"
@@ -35,29 +34,32 @@ public:
   }
   virtual ~WforceReplication() = default;
 
-  void receiveReplicationOperationsTCP(const ComboAddress& local);
-  void receiveReplicationOperations(const ComboAddress& local);
-  void startReplicationWorkerThreads();
-  void encryptMsg(const std::string& msg, std::string& packet);
-  void encryptMsgWithKey(const std::string& msg, std::string& packet, const std::string& key, SodiumNonce& nonce,
+  virtual void receiveReplicationOperationsTCP(const ComboAddress& local);
+  virtual void receiveReplicationOperations(const ComboAddress& local);
+
+  virtual void startReplicationWorkerThreads();
+  virtual void encryptMsg(const std::string& msg, std::string& packet);
+  virtual void encryptMsgWithKey(const std::string& msg, std::string& packet, const std::string& key, SodiumNonce& nonce,
                          std::mutex& mutex);
-  bool decryptMsg(const char* buf, size_t len, std::string& msg);
+  virtual bool decryptMsg(const char* buf, size_t len, std::string& msg);
   void setMaxSiblingRecvQueueSize(size_t size);
   void setNumSiblingThreads(unsigned int num_threads) { d_num_sibling_threads = num_threads; }
   GlobalStateHolder<vector<shared_ptr<Sibling>>>& getSiblings() { return d_siblings; }
-  void replicateOperation(const ReplicationOperation& rep_op);
+  virtual void replicateOperation(const ReplicationOperation& rep_op);
+  void setEncryptionKey(const std::string& key) { d_key = key; }
+  std::string getEncryptionKey() const { return d_key; }
 protected:
-  bool checkConnFromSibling(const ComboAddress& remote, shared_ptr<Sibling>& recv_sibling);
-  void parseTCPReplication(std::shared_ptr<Socket> sockp, const ComboAddress& remote, std::shared_ptr<Sibling> recv_sibling);
-  void parseReceivedReplicationMsg(const std::string& msg, const ComboAddress& remote, std::shared_ptr<Sibling> recv_sibling);
-private:
+  virtual bool checkConnFromSibling(const ComboAddress& remote, shared_ptr<Sibling>& recv_sibling);
+  virtual void parseTCPReplication(std::shared_ptr<Socket> sockp, const ComboAddress& remote, std::shared_ptr<Sibling> recv_sibling);
+  virtual void parseReceivedReplicationMsg(const std::string& msg, const ComboAddress& remote, std::shared_ptr<Sibling> recv_sibling);
   struct SiblingQueueItem {
     std::string msg;
     ComboAddress remote;
     std::shared_ptr<Sibling> recv_sibling;
   };
   GlobalStateHolder<vector<shared_ptr<Sibling>>> d_siblings;
   SodiumNonce d_sodnonce;
+  std::string d_key; // The default key to use if no per-sibling key
   std::mutex d_sod_mutx;
   std::mutex d_sibling_queue_mutex;
   std::queue<SiblingQueueItem> d_sibling_queue;

wforce/wforce-sibling.cc

@@ -103,6 +103,7 @@ Sibling::Sibling(const ComboAddress& ca,
   if (!d_key.empty()) {
     d_has_key = true;
   }
+  d_nonce.init();
   if (proto != Protocol::NONE) {
     {
       std::lock_guard<std::mutex> lock(mutx);

wforce/wforce-web.cc

@@ -239,7 +239,7 @@ void parseSyncCmd(const YaHTTP::Request& req, YaHTTP::Response& resp, const std:
         encryption_key = msg["encryption_key"].string_value();
       }
       else {
-        encryption_key = g_key;
+        encryption_key = g_replication.getEncryptionKey();
       }
       thread t(syncDBThread, replication_ca, callback_url, callback_pw, encryption_key);
       t.detach();
@@ -398,7 +398,7 @@ void parseSetSiblingsCmd(const YaHTTP::Request& req, YaHTTP::Response& resp, con
           encryption_key = i["encryption_key"].string_value();
         }
         else {
-          encryption_key = Base64Encode(g_key);
+          encryption_key = Base64Encode(g_replication.getEncryptionKey());
         }
 
         Sibling::Protocol proto = Sibling::Protocol::UDP;