security: E2EE and transport correctness (issue #147)
Some checks failed
CI Code / Check spelling (pull_request) Successful in 13s
CI Code / Check coding style (pull_request) Successful in 23s
CI Code / Code Coverage (pull_request) Failing after 3m23s
CI Code / Linux (debian) (pull_request) Failing after 5m32s
CI Code / Linux (arch) (pull_request) Failing after 6m54s
CI Code / Linux (ubuntu) (pull_request) Failing after 8m20s

T04: promote security events to warnings — SASL auth failure, TLS
handshake failure, cert-failure details, see-other-host redirect;
DISABLE_TLS, TRUST_TLS and LEGACY_AUTH get a log warning plus a
console notice (REQ-LOG-01, REQ-LOG-02, REQ-AUTH-03)

T05: warn when a session ends up unencrypted without the user having
asked for it; refuse in-band registration on an unencrypted stream;
warn on each HTTP transfer with certificate verification disabled
(REQ-CRY-03, REQ-CFG-01)

T06: pin the update check to https with peer/host verification and no
redirects; strict N.N.N parser for the fetched version, which is
untrusted network input (REQ-VUL-02)

T09: no plaintext logging on failed MUC OMEMO sends; OTR opportunistic
first message passes allow_unencrypted_message(); get_random_string()
draws from a CSPRNG without modulo bias; guard the identity-key length
decrement against unsigned underflow (REQ-CRY-01, REQ-CRY-02,
REQ-CRY-07, REQ-MEM-05)

REQ-VUL-03 and REQ-CRY-09 are already satisfied on master and are
left unchanged.
This commit is contained in:
2026-07-25 15:43:59 +03:00
parent 2be16df905
commit 6dd1610e47
18 changed files with 260 additions and 23 deletions

View File

@@ -342,6 +342,12 @@ AS_IF([test "x$enable_gdk_pixbuf" != xno],
[AC_MSG_ERROR([gdk-pixbuf-2.0 >= 2.4 is required to scale avatars before uploading])],
[AC_MSG_NOTICE([gdk-pixbuf-2.0 >= 2.4 not found, GDK Pixbuf support not enabled])])])])
dnl libgcrypt: CSPRNG for stanza ids and instance identifiers (also pulled in by OMEMO/OTR)
AC_CHECK_LIB([gcrypt], [gcry_create_nonce],
[AC_DEFINE([HAVE_LIBGCRYPT], [1], [Have libgcrypt])
LIBS="-lgcrypt $LIBS"],
[AC_MSG_NOTICE([libgcrypt not found, identifiers will use the OS random device])])
dnl feature: omemo
AM_CONDITIONAL([BUILD_OMEMO], [false])
if test "x$enable_omemo" != xno; then

View File

@@ -9558,6 +9558,11 @@ _url_aesgcm_method(ProfWin* window, const char* cmd_template, gchar* url, gchar*
return;
AESGCMDownload* download = g_new0(AESGCMDownload, 1);
download->id = get_random_string(4);
if (!download->id) {
cons_show_error("Could not start download: no random source available.");
g_free(download);
return;
}
download->url = strdup(url);
download->filename = strdup(filename);
if (cmd_template != NULL) {
@@ -9580,6 +9585,11 @@ _download_install_plugin(ProfWin* window, gchar* url, gchar* path)
return FALSE;
HTTPDownload* download = g_new0(HTTPDownload, 1);
download->id = get_random_string(4);
if (!download->id) {
cons_show_error("Could not start download: no random source available.");
g_free(download);
return FALSE;
}
download->url = strdup(url);
download->filename = strdup(filename);
download->cmd_template = NULL;
@@ -9599,6 +9609,11 @@ _url_http_method(ProfWin* window, const char* cmd_template, gchar* url, gchar* p
return;
HTTPDownload* download = g_new0(HTTPDownload, 1);
download->id = get_random_string(4);
if (!download->id) {
cons_show_error("Could not start download: no random source available.");
g_free(download);
return;
}
download->url = strdup(url);
download->filename = strdup(filename);
download->cmd_template = cmd_template ? strdup(cmd_template) : NULL;

View File

@@ -23,6 +23,9 @@
#include <curl/curl.h>
#include <curl/easy.h>
#ifdef HAVE_LIBGCRYPT
#include <gcrypt.h>
#endif
#include <glib.h>
#include <gio/gio.h>
#include <glib/gstdio.h>
@@ -470,9 +473,26 @@ release_get_latest(void)
curl_easy_setopt(handle, CURLOPT_TIMEOUT, 2L);
curl_easy_setopt(handle, CURLOPT_WRITEDATA, (void*)&output);
curl_easy_perform(handle);
curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 1L);
curl_easy_setopt(handle, CURLOPT_SSL_VERIFYHOST, 2L);
curl_easy_setopt(handle, CURLOPT_FOLLOWLOCATION, 0L);
#if LIBCURL_VERSION_NUM >= 0x075500 // 7.85.0 introduced the string form
curl_easy_setopt(handle, CURLOPT_PROTOCOLS_STR, "https");
curl_easy_setopt(handle, CURLOPT_REDIR_PROTOCOLS_STR, "https");
#else
curl_easy_setopt(handle, CURLOPT_PROTOCOLS, CURLPROTO_HTTPS);
curl_easy_setopt(handle, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTPS);
#endif
CURLcode res = curl_easy_perform(handle);
curl_easy_cleanup(handle);
if (res != CURLE_OK) {
log_warning("Update check failed: %s", curl_easy_strerror(res));
free(output.buffer);
return NULL;
}
if (output.buffer) {
output.buffer[output.size++] = '\0';
return output.buffer;
@@ -481,6 +501,43 @@ release_get_latest(void)
}
}
// strict "major.minor.patch": the found version is untrusted network input
static gboolean
_parse_version(const char* const version, int* major, int* minor, int* patch)
{
if (!version) {
return FALSE;
}
int* out[3] = { major, minor, patch };
const char* curr = version;
for (int part = 0; part < 3; part++) {
if (part > 0) {
if (*curr != '.') {
return FALSE;
}
curr++;
}
if (!g_ascii_isdigit(*curr)) {
return FALSE;
}
gint64 value = 0;
while (g_ascii_isdigit(*curr)) {
value = value * 10 + (*curr - '0');
if (value > G_MAXINT) {
return FALSE;
}
curr++;
}
*out[part] = (int)value;
}
return *curr == '\0';
}
gboolean
release_is_new(const char* const curr_version, const char* const found_version)
{
@@ -490,12 +547,10 @@ release_is_new(const char* const curr_version, const char* const found_version)
int curr_maj, curr_min, curr_patch, found_maj, found_min, found_patch;
int parse_curr = sscanf(curr_version, "%d.%d.%d", &curr_maj, &curr_min,
&curr_patch);
int parse_found = sscanf(found_version, "%d.%d.%d", &found_maj, &found_min,
&found_patch);
gboolean parse_curr = _parse_version(curr_version, &curr_maj, &curr_min, &curr_patch);
gboolean parse_found = _parse_version(found_version, &found_maj, &found_min, &found_patch);
if (parse_found == 3 && parse_curr == 3) {
if (parse_found && parse_curr) {
if (found_maj > curr_maj) {
return TRUE;
} else if (found_maj == curr_maj && found_min > curr_min) {
@@ -699,22 +754,57 @@ get_file_paths_recursive(const char* path, GSList** contents)
}
}
static gboolean
_random_bytes(unsigned char* buf, size_t len)
{
#ifdef HAVE_LIBGCRYPT
// never initialise gcrypt here: omemo_crypto_init() must set up secure memory first
if (gcry_control(GCRYCTL_INITIALIZATION_FINISHED_P)) {
gcry_create_nonce(buf, len);
return TRUE;
}
#endif
FILE* urandom = fopen("/dev/urandom", "rb");
if (urandom) {
size_t got = fread(buf, 1, len, urandom);
fclose(urandom);
if (got == len) {
return TRUE;
}
}
return FALSE;
}
gchar*
get_random_string(size_t length)
{
GRand* prng;
gchar* rand;
gchar alphabet[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
int endrange = sizeof(alphabet) - 1;
static const gchar alphabet[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
const unsigned alphabet_len = sizeof(alphabet) - 1;
const unsigned limit = 256 - (256 % alphabet_len); // reject the uneven tail, it would bias early letters
rand = g_malloc0(length + 1);
gchar* rand = g_malloc0(length + 1);
prng = g_rand_new();
for (size_t i = 0; i < length;) {
unsigned char block[64];
size_t want = length - i;
if (want > sizeof(block)) {
want = sizeof(block);
}
for (size_t i = 0; i < length; i++) {
rand[i] = alphabet[g_rand_int_range(prng, 0, endrange)];
if (!_random_bytes(block, want)) {
log_error("No cryptographic random source available for identifier generation");
g_free(rand);
return NULL;
}
for (size_t j = 0; j < want && i < length; j++) {
if (block[j] < limit) {
rand[i++] = alphabet[block[j] % alphabet_len];
}
}
}
g_rand_free(prng);
return rand;
}

View File

@@ -198,9 +198,11 @@ cl_ev_send_muc_msg_corrected(ProfMucWin* mucwin, const char* const msg, const ch
#ifdef HAVE_OMEMO
if (mucwin->is_omemo) {
auto_char char* id = omemo_on_message_send((ProfWin*)mucwin, message, FALSE, TRUE, replace_id);
groupchat_log_omemo_msg_out(mucwin->roomjid, message);
log_database_add_outgoing_muc(id, mucwin->roomjid, message, replace_id, PROF_MSG_ENC_OMEMO);
mucwin_outgoing_msg(mucwin, message, id, PROF_MSG_ENC_OMEMO, replace_id);
if (id != NULL) { // nothing was sent, so don't persist or echo the plaintext
groupchat_log_omemo_msg_out(mucwin->roomjid, message);
log_database_add_outgoing_muc(id, mucwin->roomjid, message, replace_id, PROF_MSG_ENC_OMEMO);
mucwin_outgoing_msg(mucwin, message, id, PROF_MSG_ENC_OMEMO, replace_id);
}
} else
#endif
{

View File

@@ -205,7 +205,7 @@ void
sv_ev_failed_login(void)
{
cons_show_error("Login failed.");
log_info("Login failed");
log_warning("[SECURITY] Authentication failed for %s", STR_MAYBE_NULL(session_get_account_name()));
tlscerts_clear_current();
}
@@ -1145,6 +1145,11 @@ sv_ev_certfail(const char* const errormsg, const TLSCertificate* cert)
return 1;
}
log_warning("[SECURITY] TLS certificate verification failed: %s (subject: %s, fingerprint: %s)",
errormsg,
cert && cert->subjectname ? cert->subjectname : "(unknown)",
cert && cert->fingerprint ? cert->fingerprint : "(none)");
cons_show("");
cons_show_error("TLS certificate verification failed: %s", errormsg);
cons_show_tlscert(cert);

View File

@@ -1433,6 +1433,12 @@ _omemo_fingerprint(ec_public_key* identity, gboolean formatted)
size_t identity_public_key_len = signal_buffer_len(identity_public_key);
unsigned char* identity_public_key_data = signal_buffer_data(identity_public_key);
if (identity_public_key_len == 0) {
log_error("[OMEMO] cannot fingerprint an empty identity key"); // the decrement below would wrap
signal_buffer_free(identity_public_key);
return NULL;
}
/* Skip first byte corresponding to signal DJB_TYPE */
identity_public_key_len--;
identity_public_key_data = &identity_public_key_data[1];

View File

@@ -22,6 +22,7 @@
#include "config/files.h"
#include "otr/otr.h"
#include "otr/otrlib.h"
#include "event/client_events.h"
#include "ui/ui.h"
#include "ui/window_list.h"
#include "xmpp/chat_session.h"
@@ -331,6 +332,10 @@ otr_on_message_send(ProfChatWin* chatwin, const char* const message, gboolean re
// tag and send for policy opportunistic
if (policy == PROF_OTRPOLICY_OPPORTUNISTIC) {
// the tagged first message is still plaintext on the wire
if (!allow_unencrypted_message(chatwin, message)) {
return TRUE;
}
auto_char char* otr_tagged_msg = otr_tag_message(message);
id = message_send_chat_otr(chatwin->barejid, otr_tagged_msg, request_receipt, replace_id);
chatwin_outgoing_msg(chatwin, message, id, PROF_MSG_ENC_NONE, request_receipt, replace_id);

View File

@@ -14,6 +14,7 @@
#include <string.h>
#include <gio/gio.h>
#include "log.h"
#include "tools/http_common.h"
#define FALLBACK_MSG ""
@@ -51,3 +52,13 @@ http_print_transfer(ProfWin* window, char* id, theme_item_t theme_item, const ch
g_string_free(msg, TRUE);
}
void
http_warn_insecure_transfer(ProfWin* window, char* id, const char* const url)
{
log_warning("[SECURITY] TLS certificate verification disabled for transfer: %s", url);
http_print_transfer_update(window, id, THEME_ERROR, 0,
"Security warning: TLS certificate not verified for '%s' "
"(tls.policy=trust).",
url);
}

View File

@@ -17,5 +17,6 @@ G_GNUC_PRINTF(4, 5)
void http_print_transfer(ProfWin* window, char* id, theme_item_t theme_item, const char* fmt, ...);
G_GNUC_PRINTF(5, 6)
void http_print_transfer_update(ProfWin* window, char* id, theme_item_t theme_item, int flags, const char* fmt, ...);
void http_warn_insecure_transfer(ProfWin* window, char* id, const char* const url);
#endif

View File

@@ -25,6 +25,7 @@
#include "profanity.h"
#include "event/client_events.h"
#include "tools/http_download.h"
#include "tools/http_common.h"
#include "config/cafile.h"
#include "config/preferences.h"
#include "ui/ui.h"
@@ -153,6 +154,8 @@ http_file_get(void* userdata)
if (insecure) {
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0L);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L);
http_warn_insecure_transfer(download->window, download->id,
download->display_url ?: download->url);
}
if ((res = curl_easy_perform(curl)) != CURLE_OK) {

View File

@@ -22,6 +22,7 @@
#include "profanity.h"
#include "event/client_events.h"
#include "tools/http_upload.h"
#include "tools/http_common.h"
#include "config/cafile.h"
#include "config/preferences.h"
#include "ui/ui.h"
@@ -242,6 +243,7 @@ http_file_put(void* userdata)
if (insecure) {
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0L);
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L);
http_warn_insecure_transfer(upload->window, upload->put_url, upload->put_url);
}
curl_easy_setopt(curl, CURLOPT_READDATA, fh);

View File

@@ -46,6 +46,7 @@ typedef struct prof_conn_t
GHashTable* available_resources;
GHashTable* features_by_jid;
GHashTable* requested_features;
gboolean tls_disabled_by_user;
} ProfConnection;
typedef struct
@@ -212,6 +213,21 @@ _conn_apply_settings(const char* const jid, const char* const passwd, const char
#undef LOG_FLAG_IF_SET
}
conn.tls_disabled_by_user = (flags & XMPP_CONN_FLAG_DISABLE_TLS) ? TRUE : FALSE;
if (flags & XMPP_CONN_FLAG_DISABLE_TLS) {
log_warning("[SECURITY] TLS is disabled for this connection: traffic and credentials are sent in the clear");
cons_show_error("Security warning: TLS is disabled, this connection is unencrypted.");
}
if (flags & XMPP_CONN_FLAG_TRUST_TLS) {
log_warning("[SECURITY] TLS certificate verification is disabled for this connection");
cons_show_error("Security warning: TLS certificates are not verified for this connection.");
}
if (flags & XMPP_CONN_FLAG_LEGACY_AUTH) {
log_warning("[SECURITY] Legacy (XEP-0078) authentication enabled: the password is sent without SASL");
cons_show_error("Security warning: legacy authentication is enabled for this connection.");
}
if (xmpp_conn_set_flags(conn.xmpp_conn, flags)) {
log_error("libstrophe doesn't accept this combination of flags: 0x%lx", flags);
conn.conn_status = JABBER_DISCONNECTED;
@@ -364,7 +380,8 @@ _register_handle_proceedtls_default(xmpp_conn_t* xmpp_conn,
xmpp_handler_delete(xmpp_conn, _register_handle_error);
xmpp_conn_open_stream_default(xmpp_conn);
} else {
log_debug("TLS failed.");
log_warning("[SECURITY] TLS handshake failed during registration, aborting");
cons_show_error("Security warning: TLS handshake failed, registration aborted.");
/* failed tls spoils the connection, so disconnect */
xmpp_disconnect(xmpp_conn);
}
@@ -408,6 +425,14 @@ _register_handle_features(xmpp_conn_t* xmpp_conn, xmpp_stanza_t* stanza, void* u
return 0;
}
/* Registration sends the new password in this stream: refuse to do so in the clear */
if (!xmpp_conn_is_secured(xmpp_conn)) {
log_warning("[SECURITY] Refusing in-band registration over an unencrypted stream");
cons_show_error("Registration aborted: the connection is not encrypted and the password would be sent in the clear.");
xmpp_disconnect(xmpp_conn);
return 0;
}
/* check whether server supports in-band registration */
child = xmpp_stanza_get_child_by_name(stanza, "register");
if (!child) {
@@ -957,6 +982,12 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status
conn.features_by_jid = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy);
g_hash_table_insert(conn.features_by_jid, strdup(conn.domain), g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL));
// tls.policy=allow negotiates opportunistically, so an unrequested downgrade must not pass silently
if (!connection_is_secured() && !conn.tls_disabled_by_user) {
log_warning("[SECURITY] Logged in over an unencrypted connection to %s: the server offered no usable TLS", conn.domain);
cons_show_error("Security warning: this session is NOT encrypted, the server did not provide TLS. Use '/account set <account> tls force' to require it.");
}
session_login_success(connection_is_secured());
if (conn.queued_messages) {
@@ -1023,8 +1054,9 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status
int port;
if (stream_error && stream_error->stanza && _get_other_host(stream_error->stanza, &host, &port)) {
g_assert(port >= 0 && port <= UINT16_MAX);
log_warning("[SECURITY] Server redirected the connection (see-other-host) to \"%s\":%d", host, port);
cons_show_error("Security notice: the server redirected this connection to %s:%d.", host, port);
session_reconnect(host, (unsigned short)port);
log_debug("Connection handler: Forcing a re-connect to \"%s\"", host);
conn.conn_status = JABBER_RECONNECT;
return;
}
@@ -1121,8 +1153,12 @@ _random_bytes_init(void)
profanity_instance_id = g_key_file_get_string(keyfile.keyfile, "identifier", "random_bytes", NULL);
} else {
profanity_instance_id = get_random_string(10);
g_key_file_set_string(keyfile.keyfile, "identifier", "random_bytes", profanity_instance_id);
save_keyfile(&keyfile);
if (profanity_instance_id) {
g_key_file_set_string(keyfile.keyfile, "identifier", "random_bytes", profanity_instance_id);
save_keyfile(&keyfile);
} else {
log_error("Could not generate an instance identifier: no random source available");
}
}
free_keyfile(&keyfile);

View File

@@ -117,6 +117,7 @@ main(int argc, char* argv[])
PROF_FUNC_TEST(connect_jid_requests_bookmarks),
PROF_FUNC_TEST(connect_bad_password),
PROF_FUNC_TEST(connect_shows_presence_updates),
PROF_FUNC_TEST(connect_warns_on_insecure_transport),
/* Ping tests - XEP-0199 XMPP Ping */
PROF_FUNC_TEST(ping_server),

View File

@@ -92,3 +92,15 @@ connect_shows_presence_updates(void **state)
);
assert_true(prof_output_exact("Buddy1 (mobile) is xa, \"Gone :(\""));
}
void
connect_warns_on_insecure_transport(void **state)
{
/* the harness connects with "tls disable auth legacy": both downgrades must reach the console */
prof_connect();
prof_timeout(10);
assert_true(prof_output_exact("Security warning: TLS is disabled, this connection is unencrypted."));
assert_true(prof_output_exact("Security warning: legacy authentication is enabled for this connection."));
prof_timeout_reset();
}

View File

@@ -3,3 +3,4 @@ void connect_jid_sends_presence_after_receiving_roster(void **state);
void connect_jid_requests_bookmarks(void **state);
void connect_bad_password(void **state);
void connect_shows_presence_updates(void **state);
void connect_warns_on_insecure_transport(void **state);

View File

@@ -1089,6 +1089,45 @@ release_is_new__tests__various(void** state)
assert_false(release_is_new("0.16.0", ""));
assert_false(release_is_new(NULL, "1.0.0"));
assert_false(release_is_new("1.0.0", NULL));
// the found version is untrusted network input: nothing but strictly N.N.N may parse
assert_false(release_is_new("0.16.0", " 1.0.0")); // leading whitespace
assert_false(release_is_new("0.16.0", "+1.0.0")); // explicit sign
assert_false(release_is_new("0.16.0", "-1.0.0")); // negative major
assert_false(release_is_new("0.16.0", "1.0.0-rc1")); // trailing garbage
assert_false(release_is_new("0.16.0", "1.0.0\n")); // trailing newline
assert_false(release_is_new("0.16.0", "1..0")); // empty component
assert_false(release_is_new("0.16.0", "0x10.0.0")); // hex is not accepted
assert_false(release_is_new("0.16.0", "99999999999.0.0")); // out of int range, must not wrap
assert_false(release_is_new("0.16.0", "1.0.0 malicious"));
// Boundary: the largest still-parsable component is accepted
assert_true(release_is_new("0.16.0", "2147483647.0.0"));
}
void
get_random_string__generates_valid_ids(void** state)
{
const gchar* alphabet = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
for (size_t len = 1; len <= 40; len++) {
gchar* id = get_random_string(len);
assert_non_null(id);
assert_int_equal(len, strlen(id));
for (size_t i = 0; i < len; i++) {
assert_non_null(strchr(alphabet, id[i]));
}
g_free(id);
}
// a collision here would mean a dead random source
gchar* a = get_random_string(20);
gchar* b = get_random_string(20);
assert_non_null(a);
assert_non_null(b);
assert_string_not_equal(a, b);
g_free(a);
g_free(b);
}
void

View File

@@ -64,5 +64,6 @@ void valid_tls_policy_option__is__correct_for_various_inputs(void** state);
void get_mentions__tests__various(void** state);
void release_is_new__tests__various(void** state);
void str_xml_sanitize__strips_illegal_characters(void** state);
void get_random_string__generates_valid_ids(void** state);
#endif

View File

@@ -687,6 +687,7 @@ main(int argc, char* argv[])
cmocka_unit_test(get_mentions__tests__various),
cmocka_unit_test(release_is_new__tests__various),
cmocka_unit_test(str_xml_sanitize__strips_illegal_characters),
cmocka_unit_test(get_random_string__generates_valid_ids),
cmocka_unit_test_setup_teardown(plugins_get_command_names__returns__no_commands,
load_preferences,