security: E2EE and transport correctness (issue #147) #173
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
120
src/common.c
@@ -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
|
||||
|
jabber.developer
commented
Can curl realistically fallback to HTTP? If not, then this code provides no benefit. The string is defined in const variable Can curl realistically fallback to HTTP? If not, then this code provides no benefit. The string is defined in const variable `url` which we do not touch later, so the protocol always remains the same.
|
||||
|
||||
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) {
|
||||
|
jabber.developer
commented
Can we actually detect this overflow postfactum? Can we actually detect this overflow postfactum?
|
||||
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");
|
||||
|
jabber.developer
commented
Why such complicated setup? Earlier setup with https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/grand.c#L172
Why such complicated setup? Earlier setup with `g_rand_new()` and `g_rand_int_range()` utilized `/dev/urandom` as well. It even had a fallback mechanism.
https://gitlab.gnome.org/GNOME/glib/-/blob/main/glib/grand.c#L172
`get_random_string()` is used only to create stanza ids, download ids, and unique profanity identifier. There is no need for actual strong randomness. But I see the potential of strong random in future applications, such as password generation.
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
jabber.developer
commented
TODO: probably it's a bug with diff, but the only change I see here is adding this condtion. However, for me, the lines below are highlighted as well. TODO: probably it's a bug with diff, but the only change I see here is adding this condtion. However, for me, the lines below are highlighted as well.
|
||||
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
|
||||
{
|
||||
|
||||
@@ -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()));
|
||||
|
jabber.developer
commented
Do we even need to guard NULL-strings with Do we even need to guard NULL-strings with `STR_MAYBE_NULL`? Typically all `%s` for us show `(null)` if they are null.
|
||||
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);
|
||||
|
||||
@@ -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
|
||||
|
jabber.developer
commented
The comment is nice, but I feel that it can be placed slightly higher, since inline comment for log message looks less readable. The comment is nice, but I feel that it can be placed slightly higher, since inline comment for log message looks less readable.
|
||||
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];
|
||||
|
||||
@@ -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
|
||||
|
jabber.developer
commented
Nice catch! I really dislike Nice catch! I really dislike `message_send_chat_otr`, as it's really misleading: unlike other `message_send_chat_` `ENCRYPTION_NAME` methods, the method sends plaintext message and just marks the message as "otr".
|
||||
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);
|
||||
|
||||
@@ -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,
|
||||
|
jabber.developer
commented
We need We need `theme_warning`
|
||||
"Security warning: TLS certificate not verified for '%s' "
|
||||
"(tls.policy=trust).",
|
||||
url);
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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.");
|
||||
|
jabber.developer
commented
We need We need `cons_show_warning`
|
||||
}
|
||||
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.");
|
||||
|
jabber.developer
commented
Nice actionable warning. Nice actionable warning.
|
||||
}
|
||||
|
||||
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");
|
||||
|
jabber.developer
commented
Here and below shouldn't happen. We don't need true or strong randomness for simple things like that. Falling back on current time, as GLib does, is perfectly acceptable in these scenarios. Here and below shouldn't happen. We don't need true or strong randomness for simple things like that. Falling back on current time, as GLib does, is perfectly acceptable in these scenarios.
|
||||
}
|
||||
}
|
||||
|
||||
free_keyfile(&keyfile);
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -977,6 +977,8 @@ migration_incremental_accumulation(void** state)
|
||||
assert_true(prof_output_regex("Database backend switched to 'flatfile'\\."));
|
||||
|
||||
prof_input("/msg buddy1@localhost");
|
||||
/* full repaint: incremental redraw may emit only partial line text */
|
||||
prof_input("/redraw");
|
||||
assert_true(prof_output_exact("incr-batch1-first"));
|
||||
assert_true(prof_output_exact("incr-batch1-second"));
|
||||
assert_true(prof_output_exact("incr-batch2-third"));
|
||||
|
||||
@@ -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
|
||||
|
jabber.developer
commented
What about What about `...`, `.`, `..`?
|
||||
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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
We just overwrite defaults here.