relay/irc: fix timing attack on PASS command (GHSA-vhv8-g2r9-cwcc)

The IRC relay protocol's PASS handler compared the server password with
the client-supplied value using strcmp, leaking the password byte-by-byte
via response timing. This is the same class of bug fixed for the api and
weechat protocols, on a separate code path that did not go through
relay_auth_check_password_plain.

Extract the HMAC-then-constant-time-compare logic from
relay_auth_check_password_plain into relay_auth_password_equals, then
use it in both the plain-auth wrapper and the IRC PASS handler.

src/plugins/relay/irc/relay-irc.c

@@ -31,6 +31,7 @@
 #include "../../weechat-plugin.h"
 #include "../relay.h"
 #include "relay-irc.h"
+#include "../relay-auth.h"
 #include "../relay-buffer.h"
 #include "../relay-client.h"
 #include "../relay-config.h"
@@ -1699,7 +1700,7 @@ relay_irc_recv (struct t_relay_client *client, const char *data)
                         NULL, NULL, NULL);
                     if (password)
                     {
-                        if (strcmp (password, pos_password) == 0)
+                        if (relay_auth_password_equals (password, pos_password))
                         {
                             RELAY_IRC_DATA(client, password_ok) = 1;
                             weechat_hook_signal_send ("relay_client_auth_ok",

src/plugins/relay/relay-auth.c

@@ -100,6 +100,66 @@ relay_auth_generate_nonce (int size)
     return nonce_hexa;
 }
 
+/*
+ * Compare two passwords for equality in constant time.
+ *
+ * HMAC both sides with a fresh random key, then compare the fixed-size
+ * MACs. This hides both the per-byte comparison and the password length
+ * from a timing-side-channel observer.
+ *
+ * Both messages are prefixed with a zero byte so empty passwords still
+ * produce a valid HMAC (the underlying crypto API rejects zero-length
+ * messages); the prefix is identical on both sides so equal inputs
+ * still yield equal MACs.
+ *
+ * Return:
+ *   1: passwords are equal
+ *   0: passwords are not equal (or an error occurred)
+ */
+
+int
+relay_auth_password_equals (const char *password1, const char *password2)
+{
+    unsigned char key[32];
+    char hmac1[64], hmac2[64];
+    char *buf1, *buf2;
+    int buf1_size, buf2_size, hmac1_size, hmac2_size, rc;
+
+    if (!password1 || !password2)
+        return 0;
+
+    rc = 0;
+    buf1_size = strlen (password1) + 1;
+    buf2_size = strlen (password2) + 1;
+    buf1 = malloc (buf1_size);
+    buf2 = malloc (buf2_size);
+    if (buf1 && buf2)
+    {
+        buf1[0] = 0;
+        memcpy (buf1 + 1, password1, buf1_size - 1);
+        buf2[0] = 0;
+        memcpy (buf2 + 1, password2, buf2_size - 1);
+        gcry_create_nonce (key, sizeof (key));
+        if (weechat_crypto_hmac (key, sizeof (key),
+                                 buf1, buf1_size,
+                                 "sha256",
+                                 hmac1, &hmac1_size)
+            && weechat_crypto_hmac (key, sizeof (key),
+                                    buf2, buf2_size,
+                                    "sha256",
+                                    hmac2, &hmac2_size)
+            && (hmac1_size == hmac2_size)
+            && (weechat_string_memcmp_constant_time (
+                    hmac1, hmac2, hmac1_size) == 0))
+        {
+            rc = 1;
+        }
+    }
+    free (buf1);
+    free (buf2);
+    return rc;
+}
+
 /*
  * Checks if password received as plain text is valid.
  *
@@ -114,12 +174,6 @@ relay_auth_check_password_plain (struct t_relay_client *client,
                                  const char *password,
                                  const char *relay_password)
 {
-    unsigned char key[32];
-    char hmac_password[64], hmac_relay[64];
-    char *buf_password, *buf_relay;
-    int buf_password_size, buf_relay_size;
-    int hmac_password_size, hmac_relay_size, rc;
-
     if (!client || !password || !relay_password)
         return -2;
 
@@ -131,47 +185,7 @@ relay_auth_check_password_plain (struct t_relay_client *client,
         return -1;
     }
 
-    /*
-     * Compare passwords in constant time to defeat timing attacks: HMAC
-     * both sides with a fresh random key, then compare the fixed-size
-     * MACs. This hides both the per-byte comparison and the password
-     * length from the attacker.
-     *
-     * Both messages are prefixed with a zero byte so that empty
-     * passwords still produce a valid HMAC (the underlying crypto API
-     * rejects zero-length messages); the prefix is identical on both
-     * sides so equal inputs still yield equal MACs.
-     */
-    rc = -2;
-    buf_password_size = strlen (password) + 1;
-    buf_relay_size = strlen (relay_password) + 1;
-    buf_password = malloc (buf_password_size);
-    buf_relay = malloc (buf_relay_size);
-    if (buf_password && buf_relay)
-    {
-        buf_password[0] = 0;
-        memcpy (buf_password + 1, password, buf_password_size - 1);
-        buf_relay[0] = 0;
-        memcpy (buf_relay + 1, relay_password, buf_relay_size - 1);
-        gcry_create_nonce (key, sizeof (key));
-        if (weechat_crypto_hmac (key, sizeof (key),
-                                 buf_password, buf_password_size,
-                                 "sha256",
-                                 hmac_password, &hmac_password_size)
-            && weechat_crypto_hmac (key, sizeof (key),
-                                    buf_relay, buf_relay_size,
-                                    "sha256",
-                                    hmac_relay, &hmac_relay_size)
-            && (hmac_password_size == hmac_relay_size)
-            && (weechat_string_memcmp_constant_time (
-                    hmac_password, hmac_relay, hmac_password_size) == 0))
-        {
-            rc = 0;
-        }
-    }
-    free (buf_password);
-    free (buf_relay);
-    return rc;
+    return relay_auth_password_equals (password, relay_password) ? 0 : -2;
 }
 
 /*

src/plugins/relay/relay-auth.h

@@ -37,6 +37,8 @@ extern char *relay_auth_password_hash_algo_name[];
 
 extern int relay_auth_password_hash_algo_search (const char *name);
 extern char *relay_auth_generate_nonce (int size);
+extern int relay_auth_password_equals (const char *password1,
+                                       const char *password2);
 extern int relay_auth_check_password_plain (struct t_relay_client *client,
                                             const char *password,
                                             const char *relay_password);

tests/unit/plugins/relay/test-relay-auth.cpp

@@ -107,6 +107,31 @@ TEST(RelayAuth, GenerateNonce)
     free (nonce);
 }
 
+/*
+ * Tests functions:
+ *   relay_auth_password_equals
+ */
+
+TEST(RelayAuth, PasswordEquals)
+{
+    /* invalid arguments */
+    LONGS_EQUAL(0, relay_auth_password_equals (NULL, NULL));
+    LONGS_EQUAL(0, relay_auth_password_equals ("abcd", NULL));
+    LONGS_EQUAL(0, relay_auth_password_equals (NULL, "abcd"));
+
+    /* different passwords */
+    LONGS_EQUAL(0, relay_auth_password_equals ("test", "password"));
+    LONGS_EQUAL(0, relay_auth_password_equals ("Password", "password"));
+    LONGS_EQUAL(0, relay_auth_password_equals ("", "password"));
+    LONGS_EQUAL(0, relay_auth_password_equals ("password", ""));
+
+    /* equal passwords */
+    LONGS_EQUAL(1, relay_auth_password_equals ("", ""));
+    LONGS_EQUAL(1, relay_auth_password_equals ("password", "password"));
+    LONGS_EQUAL(1, relay_auth_password_equals ("a really long password with spaces",
+                                               "a really long password with spaces"));
+}
+
 /*
  * Tests functions:
  *   relay_auth_check_password_plain