mirror of
https://github.com/weechat/weechat.git
synced 2026-06-12 14:14:48 +02:00
relay: fix timing attack on password authentication (GHSA-vhv8-g2r9-cwcc)
The relay authentication used non-constant-time comparisons (strcasecmp, strcmp) to verify password hashes and plaintext passwords, allowing an attacker to derive the expected hash byte-by-byte from response timing and then authenticate without knowing the password. - SHA/PBKDF2 hex hash comparisons: normalize the client-supplied hash to uppercase and compare in constant time over the fixed expected length. - Plaintext password comparison: HMAC-SHA256 both passwords with a fresh per-call random key and compare the fixed-size MACs in constant time, hiding both per-byte timing and the password length. Add string_memcmp_constant_time helper in core, exposed via the plugin API. Bump WEECHAT_PLUGIN_API_VERSION accordingly.
This commit is contained in:
@@ -23,6 +23,7 @@ SPDX-License-Identifier: GPL-3.0-or-later
|
||||
- irc: fix tag in message with list of names when joining a channel
|
||||
- fset: remove error displayed in core buffer when clicking with the mouse below the last option displayed
|
||||
- relay: limit size of decompressed websocket frame with permessage-deflate to prevent memory exhaustion ([GHSA-v2v4-45wm-5cr3](https://github.com/weechat/weechat/security/advisories/GHSA-v2v4-45wm-5cr3))
|
||||
- relay: fix timing attack on password authentication ([GHSA-vhv8-g2r9-cwcc](https://github.com/weechat/weechat/security/advisories/GHSA-vhv8-g2r9-cwcc))
|
||||
|
||||
## Version 4.9.0 (2026-03-29)
|
||||
|
||||
|
||||
@@ -923,6 +923,43 @@ string_strcmp_ignore_chars (const char *string1, const char *string2,
|
||||
string_charcasecmp (string1, string2);
|
||||
}
|
||||
|
||||
/*
|
||||
* Compare two memory areas of the same size in constant time.
|
||||
*
|
||||
* Use to compare secrets (e.g. password hashes, MACs) without leaking
|
||||
* information through the comparison's running time. The loop always
|
||||
* walks the full "size" bytes and uses only bitwise operations on the
|
||||
* data, so the execution time depends on "size" alone, not on the
|
||||
* position of the first differing byte.
|
||||
*
|
||||
* If either pointer is NULL, the areas are considered different (the
|
||||
* NULL check itself is not constant time but does not look at any
|
||||
* secret content).
|
||||
*
|
||||
* Return:
|
||||
* 0: areas are equal
|
||||
* 1: areas differ
|
||||
*/
|
||||
|
||||
int
|
||||
string_memcmp_constant_time (const void *area1, const void *area2, size_t size)
|
||||
{
|
||||
const unsigned char *p1, *p2;
|
||||
unsigned char diff;
|
||||
size_t i;
|
||||
|
||||
if (!area1 || !area2)
|
||||
return (area1 == area2) ? 0 : 1;
|
||||
|
||||
p1 = (const unsigned char *)area1;
|
||||
p2 = (const unsigned char *)area2;
|
||||
diff = 0;
|
||||
for (i = 0; i < size; i++)
|
||||
diff |= p1[i] ^ p2[i];
|
||||
|
||||
return (diff == 0) ? 0 : 1;
|
||||
}
|
||||
|
||||
/*
|
||||
* Search for a string in another string (locale and case independent).
|
||||
*
|
||||
|
||||
@@ -69,6 +69,8 @@ extern int string_strcmp_ignore_chars (const char *string1,
|
||||
const char *string2,
|
||||
const char *chars_ignored,
|
||||
int case_sensitive);
|
||||
extern int string_memcmp_constant_time (const void *area1, const void *area2,
|
||||
size_t size);
|
||||
extern const char *string_strcasestr (const char *string, const char *search);
|
||||
extern int string_match (const char *string, const char *mask,
|
||||
int case_sensitive);
|
||||
|
||||
@@ -624,6 +624,7 @@ plugin_load (const char *filename, int init_plugin, int argc, char **argv)
|
||||
new_plugin->strncasecmp = &string_strncasecmp;
|
||||
new_plugin->strncasecmp_range = &string_strncasecmp_range;
|
||||
new_plugin->strcmp_ignore_chars = &string_strcmp_ignore_chars;
|
||||
new_plugin->string_memcmp_constant_time = &string_memcmp_constant_time;
|
||||
new_plugin->strcasestr = &string_strcasestr;
|
||||
new_plugin->strlen_screen = &gui_chat_strlen_screen;
|
||||
new_plugin->string_match = &string_match;
|
||||
|
||||
@@ -116,6 +116,12 @@ 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;
|
||||
|
||||
@@ -127,7 +133,47 @@ relay_auth_check_password_plain (struct t_relay_client *client,
|
||||
return -1;
|
||||
}
|
||||
|
||||
return (strcmp (password, relay_password) == 0) ? 0 : -2;
|
||||
/*
|
||||
* 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;
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -347,8 +393,9 @@ relay_auth_check_hash_sha (const char *hash_algo,
|
||||
const char *hash_sha,
|
||||
const char *relay_password)
|
||||
{
|
||||
char *salt_password, hash[512 / 8], hash_hexa[((512 / 8) * 2) + 1];
|
||||
int rc, length, hash_size;
|
||||
char *salt_password, *hash_sha_upper, hash[512 / 8];
|
||||
char hash_hexa[((512 / 8) * 2) + 1];
|
||||
int rc, length, hash_size, hash_hexa_len;
|
||||
|
||||
rc = 0;
|
||||
|
||||
@@ -366,11 +413,24 @@ relay_auth_check_hash_sha (const char *hash_algo,
|
||||
hash_algo,
|
||||
hash, &hash_size))
|
||||
{
|
||||
weechat_string_base_encode ("16", hash, hash_size,
|
||||
hash_hexa_len = weechat_string_base_encode ("16", hash, hash_size,
|
||||
hash_hexa);
|
||||
if (weechat_strcasecmp (hash_hexa, hash_sha) == 0)
|
||||
/*
|
||||
* Compare in constant time to defeat timing attacks: the
|
||||
* client-supplied hash is normalized to uppercase to match
|
||||
* the output of base16 encoding, then compared byte-for-byte
|
||||
* with no early exit.
|
||||
*/
|
||||
hash_sha_upper = weechat_string_toupper (hash_sha);
|
||||
if (hash_sha_upper
|
||||
&& ((int)strlen (hash_sha_upper) == hash_hexa_len)
|
||||
&& (weechat_string_memcmp_constant_time (
|
||||
hash_hexa, hash_sha_upper, hash_hexa_len) == 0))
|
||||
{
|
||||
rc = 1;
|
||||
}
|
||||
free (hash_sha_upper);
|
||||
}
|
||||
free (salt_password);
|
||||
}
|
||||
|
||||
@@ -393,8 +453,8 @@ relay_auth_check_hash_pbkdf2 (const char *hash_pbkdf2_algo,
|
||||
const char *hash_pbkdf2,
|
||||
const char *relay_password)
|
||||
{
|
||||
char hash[512 / 8], hash_hexa[((512 / 8) * 2) + 1];
|
||||
int rc, hash_size;
|
||||
char *hash_pbkdf2_upper, hash[512 / 8], hash_hexa[((512 / 8) * 2) + 1];
|
||||
int rc, hash_size, hash_hexa_len;
|
||||
|
||||
rc = 0;
|
||||
|
||||
@@ -407,10 +467,19 @@ relay_auth_check_hash_pbkdf2 (const char *hash_pbkdf2_algo,
|
||||
iterations,
|
||||
hash, &hash_size))
|
||||
{
|
||||
weechat_string_base_encode ("16", hash, hash_size, hash_hexa);
|
||||
if (weechat_strcasecmp (hash_hexa, hash_pbkdf2) == 0)
|
||||
hash_hexa_len = weechat_string_base_encode ("16", hash, hash_size,
|
||||
hash_hexa);
|
||||
/* see relay_auth_check_hash_sha for rationale */
|
||||
hash_pbkdf2_upper = weechat_string_toupper (hash_pbkdf2);
|
||||
if (hash_pbkdf2_upper
|
||||
&& ((int)strlen (hash_pbkdf2_upper) == hash_hexa_len)
|
||||
&& (weechat_string_memcmp_constant_time (
|
||||
hash_hexa, hash_pbkdf2_upper, hash_hexa_len) == 0))
|
||||
{
|
||||
rc = 1;
|
||||
}
|
||||
free (hash_pbkdf2_upper);
|
||||
}
|
||||
}
|
||||
|
||||
return rc;
|
||||
|
||||
@@ -76,7 +76,7 @@ struct t_weelist_item;
|
||||
* please change the date with current one; for a second change at same
|
||||
* date, increment the 01, otherwise please keep 01.
|
||||
*/
|
||||
#define WEECHAT_PLUGIN_API_VERSION "20251112-01"
|
||||
#define WEECHAT_PLUGIN_API_VERSION "20260530-01"
|
||||
|
||||
/* macros for defining plugin infos */
|
||||
#define WEECHAT_PLUGIN_NAME(__name) \
|
||||
@@ -348,6 +348,8 @@ struct t_weechat_plugin
|
||||
int max, int range);
|
||||
int (*strcmp_ignore_chars) (const char *string1, const char *string2,
|
||||
const char *chars_ignored, int case_sensitive);
|
||||
int (*string_memcmp_constant_time) (const void *area1, const void *area2,
|
||||
size_t size);
|
||||
const char *(*strcasestr) (const char *string, const char *search);
|
||||
int (*strlen_screen) (const char *string);
|
||||
int (*string_match) (const char *string, const char *mask,
|
||||
@@ -1348,6 +1350,9 @@ extern int weechat_plugin_end (struct t_weechat_plugin *plugin);
|
||||
(weechat_plugin->strcmp_ignore_chars)(__string1, __string2, \
|
||||
__chars_ignored, \
|
||||
__case_sensitive)
|
||||
#define weechat_string_memcmp_constant_time(__area1, __area2, __size) \
|
||||
(weechat_plugin->string_memcmp_constant_time)(__area1, __area2, \
|
||||
__size)
|
||||
#define weechat_strcasestr(__string, __search) \
|
||||
(weechat_plugin->strcasestr)(__string, __search)
|
||||
#define weechat_strlen_screen(__string) \
|
||||
|
||||
@@ -800,6 +800,38 @@ TEST(CoreString, StringComparison)
|
||||
LONGS_EQUAL(-2, string_strcmp_ignore_chars ("è", "ê", "", 1));
|
||||
}
|
||||
|
||||
/*
|
||||
* Test functions:
|
||||
* string_memcmp_constant_time
|
||||
*/
|
||||
|
||||
TEST(CoreString, MemcmpConstantTime)
|
||||
{
|
||||
/* NULL handling */
|
||||
LONGS_EQUAL(0, string_memcmp_constant_time (NULL, NULL, 0));
|
||||
LONGS_EQUAL(0, string_memcmp_constant_time (NULL, NULL, 4));
|
||||
LONGS_EQUAL(1, string_memcmp_constant_time (NULL, "abcd", 4));
|
||||
LONGS_EQUAL(1, string_memcmp_constant_time ("abcd", NULL, 4));
|
||||
|
||||
/* zero-size compare always equal */
|
||||
LONGS_EQUAL(0, string_memcmp_constant_time ("", "", 0));
|
||||
LONGS_EQUAL(0, string_memcmp_constant_time ("abc", "xyz", 0));
|
||||
|
||||
/* equal areas */
|
||||
LONGS_EQUAL(0, string_memcmp_constant_time ("abcd", "abcd", 4));
|
||||
LONGS_EQUAL(0, string_memcmp_constant_time ("\x00\x01\x02\xff",
|
||||
"\x00\x01\x02\xff", 4));
|
||||
|
||||
/* differing areas (first / middle / last byte) */
|
||||
LONGS_EQUAL(1, string_memcmp_constant_time ("Xbcd", "abcd", 4));
|
||||
LONGS_EQUAL(1, string_memcmp_constant_time ("aXcd", "abcd", 4));
|
||||
LONGS_EQUAL(1, string_memcmp_constant_time ("abcX", "abcd", 4));
|
||||
LONGS_EQUAL(1, string_memcmp_constant_time ("abcd", "abce", 4));
|
||||
|
||||
/* only compares "size" bytes, ignores trailing content */
|
||||
LONGS_EQUAL(0, string_memcmp_constant_time ("abcd", "abcz", 3));
|
||||
}
|
||||
|
||||
/*
|
||||
* Test functions:
|
||||
* string_strcasestr
|
||||
|
||||
Reference in New Issue
Block a user