Merge branch '25-fish-stale-keys'

* 25-fish-stale-keys:
  If users messages with an invalid shared key, delete it (#25)
  Rename fish_data_t.timestamp -> key_created_at (#25)
  Only update FishKeys timestamp when creating the key (#25)
  Expire shared keys every 60 minutes (#25)
  Expire key exchange if not completed in 7 seconds (#25)
  Detect if FiSH decryption failed or not (#25)
  Don't decrypt NULL-padding bytes from blowfish

doc/UPDATES

@@ -14,6 +14,8 @@
     * Auto FiSH key-exchange when accepting users via callerid (controllable with set 'fish-auto-keyx')
     * Automatic re-key exchange after every message to avoid replay attacks (controllable with set 'fish-paranoid')
     * Set FiSH key via cmd_setkey and 'chanset fish-key'
+    * Bot expires key exchange if there's no response in 7 seconds.
+    * Bot expires key-exchanged keys after 60 minutes.
   * When 'mdop' protection is on, re-op all previously opped clients automatically.
   * When 'mop' protection is on, deop all previously regular clients automatically.
   * Add './wraith -V' which will display the packconfig that the bot is using.

src/chanprog.c

@@ -930,7 +930,7 @@ void keyx(const bd::String &target) {
   notice(target, "DH1080_INIT " + myPublicKeyB64, DP_HELP);
   fishData->myPublicKeyB64 = myPublicKeyB64;
   fishData->myPrivateKey = myPrivateKey;
-  fishData->timestamp = now;
+  fishData->key_created_at = now;
   FishKeys[target] = fishData;
 }
 
@@ -958,7 +958,6 @@ void set_fish_key(char *target, bd::String key)
     }
 
     // Set the key
-    fishData->timestamp = now;
     FishKeys[target] = fishData;
   }
 }

src/crypto/bf_util.c

@@ -122,8 +122,20 @@ bd::String egg_bf_decrypt(bd::String in, const bd::String& key)
       data.lr.left |= val << part * 6;
     }
     BF_decrypt(&data.bf_long, &bf_d_key);
-    for (part = 0; part < 4; part++) out += char((data.lr.left & (0xff << ((3 - part) * 8))) >> ((3 - part) * 8));
-    for (part = 0; part < 4; part++) out += char((data.lr.right & (0xff << ((3 - part) * 8))) >> ((3 - part) * 8));
+    for (part = 0; part < 4; part++) {
+      const char decrypted_char = char((data.lr.left & (0xff << ((3 - part) * 8))) >> ((3 - part) * 8));
+      // Don't write NULLs into the string
+      if (decrypted_char) {
+        out += decrypted_char;
+      }
+    }
+    for (part = 0; part < 4; part++) {
+      const char decrypted_char = char((data.lr.right & (0xff << ((3 - part) * 8))) >> ((3 - part) * 8));
+      // Don't write NULLs into the string
+      if (decrypted_char) {
+        out += decrypted_char;
+      }
+    }
   }
 
   return out;

src/mod/server.mod/server.c

@@ -1030,32 +1030,53 @@ static void server_secondly()
       nick_available(1, 0);
     }
 
-    if (!loading) {
-      static int cnt_10 = 0;
-
-      // Every 10 seconds
-      if (cnt_10 == 9) {
-        // Ensure that +D/+f are not conflicting
-
-        if (deaf_char) {
-          // +f or auth bots in used need to see channel chatter.
-          bool need_chatter = doflood(NULL) || (Auth::ht_host.size() && auth_chan && strlen(auth_prefix));
-
-          // In +D but am +f, need to -D
-          if (in_deaf && (need_chatter || !use_deaf)) {
-            dprintf(DP_SERVER, "MODE %s -%c\n", botname, deaf_char);
-            in_deaf = 0;
-          } else if (!in_deaf && use_deaf && !need_chatter) {
-            // Not +D but should be, probably had +f removed.
-            dprintf(DP_SERVER, "MODE %s +%c\n", botname, deaf_char);
-            in_deaf = 1;
-          }
+    static int cnt_10 = 0;
+
+    // Every 10 seconds
+    if (cnt_10 == 9) {
+
+      // Ensure that +D/+f are not conflicting
+      if (!loading && deaf_char) {
+        // +f or auth bots in used need to see channel chatter.
+        bool need_chatter = doflood(NULL) || (Auth::ht_host.size() && auth_chan && strlen(auth_prefix));
+
+        // In +D but am +f, need to -D
+        if (in_deaf && (need_chatter || !use_deaf)) {
+          dprintf(DP_SERVER, "MODE %s -%c\n", botname, deaf_char);
+          in_deaf = 0;
+        } else if (!in_deaf && use_deaf && !need_chatter) {
+          // Not +D but should be, probably had +f removed.
+          dprintf(DP_SERVER, "MODE %s +%c\n", botname, deaf_char);
+          in_deaf = 1;
         }
+      }
 
-        cnt_10 = 0;
-      } else
-        ++cnt_10;
+      // Clear expired key exchanges that aren't finished (7 seconds)
+      const bd::Array<bd::String> fish_targets(FishKeys.keys());
+      for (size_t i = 0; i < fish_targets.length(); ++i) {
+        const bd::String target(fish_targets[i]);
+        fish_data_t* fishData = FishKeys[target];
+        bool should_delete = false;
+        if (fishData->key_created_at && !fishData->sharedKey && ((now - 7) >= fishData->key_created_at)) {
+          putlog(LOG_DEBUG, "*", "Deleting expired DH1080 FiSH exchange with %s", target.c_str());
+          should_delete = true;
+        } else if (fishData->key_created_at && fishData->sharedKey.length() && ((now - 3600) >= fishData->key_created_at)) {
+          putlog(LOG_DEBUG, "*", "Deleting expired (60 min) FiSH key with %s", target.c_str());
+          should_delete = true;
+        }
+
+        if (should_delete) {
+          FishKeys.remove(target);
+          delete fishData;
+        }
+      }
+
+
+      cnt_10 = 0;
+    } else {
+      ++cnt_10;
     }
+
     if (connect_bursting && (now - SERVER_CONNECT_BURST_TIME) >= connect_bursting) {
       end_burstmode();
       putlog(LOG_DEBUG, "*", "Ending server burst mode");

src/mod/server.mod/server.h

@@ -51,7 +51,7 @@ typedef struct {
   bd::String sharedKey;
   bd::String myPrivateKey;
   bd::String myPublicKeyB64;
-  time_t timestamp;
+  time_t key_created_at;
 } fish_data_t;
 
 extern bind_table_t	*BT_ctcp, *BT_ctcr;

src/mod/server.mod/servmsg.c

@@ -165,19 +165,21 @@ static int check_bind_raw(char *from, char *code, char *msg)
     if (colon) {
       if (!strncmp(colon, "+OK ", 4)) {
         char *p = strchr(from, '!');
+        const bool target_is_chan = strchr(CHANMETA, target[0]);
         bd::String ciphertext(colon), sharedKey, nick(from, p - from), key_target;
 
         // If this is a channel msg, decrypt with the channel key
-        if (strchr(CHANMETA, target[0])) {
+        if (target_is_chan) {
           key_target = target;
         } else {
           // Otherwise decrypt with the nick's key
           key_target = nick;
         }
 
-        if (FishKeys.contains(key_target)) {
+        const bool have_shared_key = FishKeys.contains(key_target);
+
+        if (have_shared_key) {
           sharedKey = FishKeys[key_target]->sharedKey;
-          FishKeys[key_target]->timestamp = now;
         } else {
           struct userrec *u = get_user_by_host(from);
           if (u) {
@@ -187,8 +189,24 @@ static int check_bind_raw(char *from, char *code, char *msg)
 
         if (sharedKey.length()) {
           // Decrypt the message before passing along to the binds
-          bd::String cleartext(bd::String(msg, colon - msg) + egg_bf_decrypt(ciphertext, sharedKey));
-          mymsg = p2 = strdup(cleartext.c_str());
+          const bd::String decrypted(egg_bf_decrypt(ciphertext, sharedKey));
+          // Does the decrypted text make sense? If not, the key is probably invalid, reset it.
+          bool isValidCipherText = true;
+          for (size_t i = 0; i < decrypted.length(); ++i) {
+            if (!isprint(decrypted[i])) {
+              isValidCipherText = false;
+              break;
+            }
+          }
+          if (isValidCipherText) {
+            bd::String cleartext(bd::String(msg, colon - msg) + decrypted);
+            mymsg = p2 = strdup(cleartext.c_str());
+          } else if (!target_is_chan && have_shared_key) {
+            // Delete the shared key
+            fish_data_t* fishData = FishKeys[key_target];
+            FishKeys.remove(key_target);
+            delete fishData;
+          }
         }
       }
     }
@@ -803,7 +821,7 @@ void handle_DH1080_init(const char* nick, const char* uhost, const char* from, s
   fishData->myPublicKeyB64 = myPublicKeyB64;
   fishData->myPrivateKey = myPrivateKey;
   fishData->sharedKey = sharedKey;
-  fishData->timestamp = now;
+  fishData->key_created_at = now;
   FishKeys[nick] = fishData;
   sdprintf("Set key for %s: %s", nick, sharedKey.c_str());
   return;