mirror of
https://github.com/weechat/weechat.git
synced 2026-06-12 14:14:48 +02:00
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.
This commit is contained in:
@@ -33,6 +33,7 @@
|
|||||||
#include "../../weechat-plugin.h"
|
#include "../../weechat-plugin.h"
|
||||||
#include "../relay.h"
|
#include "../relay.h"
|
||||||
#include "relay-irc.h"
|
#include "relay-irc.h"
|
||||||
|
#include "../relay-auth.h"
|
||||||
#include "../relay-buffer.h"
|
#include "../relay-buffer.h"
|
||||||
#include "../relay-client.h"
|
#include "../relay-client.h"
|
||||||
#include "../relay-config.h"
|
#include "../relay-config.h"
|
||||||
@@ -1701,7 +1702,7 @@ relay_irc_recv (struct t_relay_client *client, const char *data)
|
|||||||
NULL, NULL, NULL);
|
NULL, NULL, NULL);
|
||||||
if (password)
|
if (password)
|
||||||
{
|
{
|
||||||
if (strcmp (password, pos_password) == 0)
|
if (relay_auth_password_equals (password, pos_password))
|
||||||
{
|
{
|
||||||
RELAY_IRC_DATA(client, password_ok) = 1;
|
RELAY_IRC_DATA(client, password_ok) = 1;
|
||||||
weechat_hook_signal_send ("relay_client_auth_ok",
|
weechat_hook_signal_send ("relay_client_auth_ok",
|
||||||
|
|||||||
@@ -102,6 +102,66 @@ relay_auth_generate_nonce (int size)
|
|||||||
return nonce_hexa;
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Check if password received as plain text is valid.
|
* Check if password received as plain text is valid.
|
||||||
*
|
*
|
||||||
@@ -116,12 +176,6 @@ relay_auth_check_password_plain (struct t_relay_client *client,
|
|||||||
const char *password,
|
const char *password,
|
||||||
const char *relay_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)
|
if (!client || !password || !relay_password)
|
||||||
return -2;
|
return -2;
|
||||||
|
|
||||||
@@ -133,47 +187,7 @@ relay_auth_check_password_plain (struct t_relay_client *client,
|
|||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
return relay_auth_password_equals (password, relay_password) ? 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;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
|||||||
@@ -39,6 +39,8 @@ extern char *relay_auth_password_hash_algo_name[];
|
|||||||
|
|
||||||
extern int relay_auth_password_hash_algo_search (const char *name);
|
extern int relay_auth_password_hash_algo_search (const char *name);
|
||||||
extern char *relay_auth_generate_nonce (int size);
|
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,
|
extern int relay_auth_check_password_plain (struct t_relay_client *client,
|
||||||
const char *password,
|
const char *password,
|
||||||
const char *relay_password);
|
const char *relay_password);
|
||||||
|
|||||||
@@ -109,6 +109,31 @@ TEST(RelayAuth, GenerateNonce)
|
|||||||
free (nonce);
|
free (nonce);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* Test 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"));
|
||||||
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Test functions:
|
* Test functions:
|
||||||
* relay_auth_check_password_plain
|
* relay_auth_check_password_plain
|
||||||
|
|||||||
Reference in New Issue
Block a user