From fdfe3e2ad96a4d401bb6263749057a4a14a9799c Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 16 Nov 2023 13:24:17 +0100 Subject: [PATCH] Rework OMEMO handling on disconnect * Fix missing destruction of `session_store` and mutex * Replace `glib_hash_table_free()` The glib API `g_hash_table_destroy()` does exactly the same. * Use the default libsignal `destroy_func` instead of doing that manually * Set internal state to `0` after everything is cleaned up Signed-off-by: Steffen Jaeckel --- src/common.c | 7 ----- src/common.h | 1 - src/omemo/omemo.c | 79 ++++++++++++++++++++++++----------------------- src/omemo/omemo.h | 2 -- src/omemo/store.c | 6 ++-- 5 files changed, 44 insertions(+), 51 deletions(-) diff --git a/src/common.c b/src/common.c index be065a49..cffb908d 100644 --- a/src/common.c +++ b/src/common.c @@ -774,13 +774,6 @@ unique_filename_from_url(const char* url, const char* path) return unique_filename; } -void -glib_hash_table_free(GHashTable* hash_table) -{ - g_hash_table_remove_all(hash_table); - g_hash_table_unref(hash_table); -} - /* build profanity version string. * example: 0.13.1dev.master.69d8c1f9 */ diff --git a/src/common.h b/src/common.h index 83260fe6..5996c5d5 100644 --- a/src/common.h +++ b/src/common.h @@ -193,7 +193,6 @@ gchar** format_call_external_argv(const char* template, const char* url, const c gchar* unique_filename_from_url(const char* url, const char* path); gchar* get_expanded_path(const char* path); -void glib_hash_table_free(GHashTable* hash_table); char* basename_from_url(const char* url); gchar* prof_get_version(void); diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index d7880dcc..0382de04 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -66,8 +66,6 @@ #define AESGCM_URL_NONCE_LEN (2 * OMEMO_AESGCM_NONCE_LENGTH) #define AESGCM_URL_KEY_LEN (2 * OMEMO_AESGCM_KEY_LENGTH) -static gboolean loaded; - static void _generate_pre_keys(int count); static void _generate_signed_pre_key(void); static gboolean _load_identity(void); @@ -87,10 +85,8 @@ static void _acquire_sender_devices_list(void); typedef gboolean (*OmemoDeviceListHandler)(const char* const jid, GList* device_list); -struct omemo_context_t +typedef struct omemo_context { - pthread_mutexattr_t attr; - pthread_mutex_t lock; signal_context* signal; uint32_t device_id; GHashTable* device_list; @@ -108,11 +104,18 @@ struct omemo_context_t prof_keyfile_t sessions; prof_keyfile_t knowndevices; GHashTable* known_devices; - GHashTable* fingerprint_ac; -}; + gboolean loaded; +} omemo_context; static omemo_context omemo_ctx; +static struct omemo_static_data +{ + pthread_mutexattr_t attr; + pthread_mutex_t lock; + GHashTable* fingerprint_ac; +} omemo_static_data; + void omemo_init(void) { @@ -121,20 +124,21 @@ omemo_init(void) cons_show("Error initializing OMEMO crypto: gcry_check_version() failed"); } - pthread_mutexattr_init(&omemo_ctx.attr); - pthread_mutexattr_settype(&omemo_ctx.attr, PTHREAD_MUTEX_RECURSIVE); - pthread_mutex_init(&omemo_ctx.lock, &omemo_ctx.attr); + pthread_mutexattr_init(&omemo_static_data.attr); + pthread_mutexattr_settype(&omemo_static_data.attr, PTHREAD_MUTEX_RECURSIVE); + pthread_mutex_init(&omemo_static_data.lock, &omemo_static_data.attr); - omemo_ctx.fingerprint_ac = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)autocomplete_free); + omemo_static_data.fingerprint_ac = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)autocomplete_free); } void omemo_close(void) { - if (omemo_ctx.fingerprint_ac) { - g_hash_table_destroy(omemo_ctx.fingerprint_ac); - omemo_ctx.fingerprint_ac = NULL; + if (omemo_static_data.fingerprint_ac) { + g_hash_table_destroy(omemo_static_data.fingerprint_ac); + omemo_static_data.fingerprint_ac = NULL; } + pthread_mutex_destroy(&omemo_static_data.lock); } void @@ -181,7 +185,7 @@ omemo_on_connect(ProfAccount* account) .contains_session_func = contains_session, .delete_session_func = delete_session, .delete_all_sessions_func = delete_all_sessions, - .destroy_func = NULL, + .destroy_func = (void (*)(void*))g_hash_table_destroy, .user_data = omemo_ctx.session_store }; signal_protocol_store_context_set_session_store(omemo_ctx.store, &session_store); @@ -192,7 +196,7 @@ omemo_on_connect(ProfAccount* account) .store_pre_key = store_pre_key, .contains_pre_key = contains_pre_key, .remove_pre_key = remove_pre_key, - .destroy_func = NULL, + .destroy_func = (void (*)(void*))g_hash_table_destroy, .user_data = omemo_ctx.pre_key_store }; signal_protocol_store_context_set_pre_key_store(omemo_ctx.store, &pre_key_store); @@ -203,7 +207,7 @@ omemo_on_connect(ProfAccount* account) .store_signed_pre_key = store_signed_pre_key, .contains_signed_pre_key = contains_signed_pre_key, .remove_signed_pre_key = remove_signed_pre_key, - .destroy_func = NULL, + .destroy_func = (void (*)(void*))g_hash_table_destroy, .user_data = omemo_ctx.signed_pre_key_store }; signal_protocol_store_context_set_signed_pre_key_store(omemo_ctx.store, &signed_pre_key_store); @@ -219,10 +223,10 @@ omemo_on_connect(ProfAccount* account) }; signal_protocol_store_context_set_identity_key_store(omemo_ctx.store, &identity_key_store); - loaded = FALSE; + omemo_ctx.loaded = FALSE; omemo_ctx.device_list = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_list_free); omemo_ctx.device_list_handler = g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL); - omemo_ctx.known_devices = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)glib_hash_table_free); + omemo_ctx.known_devices = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); auto_gchar gchar* omemo_dir = files_file_in_account_data_path(DIR_OMEMO, account->jid, NULL); if (!omemo_dir) { @@ -253,15 +257,13 @@ omemo_on_connect(ProfAccount* account) void omemo_on_disconnect(void) { - if (!loaded) { + if (!omemo_ctx.loaded) { return; } - glib_hash_table_free(omemo_ctx.signed_pre_key_store); - glib_hash_table_free(omemo_ctx.pre_key_store); - glib_hash_table_free(omemo_ctx.known_devices); - glib_hash_table_free(omemo_ctx.device_list_handler); - glib_hash_table_free(omemo_ctx.device_list); + g_hash_table_destroy(omemo_ctx.known_devices); + g_hash_table_destroy(omemo_ctx.device_list_handler); + g_hash_table_destroy(omemo_ctx.device_list); free_keyfile(&omemo_ctx.knowndevices); free_keyfile(&omemo_ctx.sessions); @@ -276,12 +278,13 @@ omemo_on_disconnect(void) ec_public_key_destroy((signal_type_base*)pub); signal_context_destroy(omemo_ctx.signal); + memset(&omemo_ctx, 0, sizeof(omemo_ctx)); } void omemo_generate_crypto_materials(ProfAccount* account) { - if (loaded) { + if (omemo_ctx.loaded) { return; } @@ -316,7 +319,7 @@ omemo_generate_crypto_materials(ProfAccount* account) omemo_identity_keyfile_save(); - loaded = TRUE; + omemo_ctx.loaded = TRUE; omemo_publish_crypto_materials(); omemo_start_sessions(); @@ -327,7 +330,7 @@ omemo_publish_crypto_materials(void) { log_debug("[OMEMO] publish crypto materials"); - if (loaded != TRUE) { + if (omemo_ctx.loaded != TRUE) { cons_show("OMEMO: cannot publish crypto materials before they are generated"); log_error("[OMEMO] cannot publish crypto materials before they are generated"); return; @@ -404,7 +407,7 @@ omemo_start_muc_sessions(const char* const roomjid) gboolean omemo_loaded(void) { - return loaded; + return omemo_ctx.loaded; } uint32_t @@ -1261,15 +1264,13 @@ omemo_untrust(const char* const jid, const char* const fingerprint_formatted) static void _lock(void* user_data) { - omemo_context* ctx = (omemo_context*)user_data; - pthread_mutex_lock(&ctx->lock); + pthread_mutex_lock(&omemo_static_data.lock); } static void _unlock(void* user_data) { - omemo_context* ctx = (omemo_context*)user_data; - pthread_mutex_unlock(&ctx->lock); + pthread_mutex_unlock(&omemo_static_data.lock); } static void @@ -1337,7 +1338,7 @@ omemo_key_free(omemo_key_t* key) char* omemo_fingerprint_autocomplete(const char* const search_str, gboolean previous, void* context) { - Autocomplete ac = g_hash_table_lookup(omemo_ctx.fingerprint_ac, context); + Autocomplete ac = g_hash_table_lookup(omemo_static_data.fingerprint_ac, context); if (ac != NULL) { return autocomplete_complete(ac, search_str, FALSE, previous); } else { @@ -1348,9 +1349,11 @@ omemo_fingerprint_autocomplete(const char* const search_str, gboolean previous, void omemo_fingerprint_autocomplete_reset(void) { + if (!omemo_static_data.fingerprint_ac) + return; gpointer value; GHashTableIter iter; - g_hash_table_iter_init(&iter, omemo_ctx.fingerprint_ac); + g_hash_table_iter_init(&iter, omemo_static_data.fingerprint_ac); while (g_hash_table_iter_next(&iter, NULL, &value)) { Autocomplete ac = value; @@ -1510,7 +1513,7 @@ _load_identity(void) _generate_signed_pre_key(); } - loaded = TRUE; + omemo_ctx.loaded = TRUE; omemo_identity_keyfile_save(); omemo_start_sessions(); @@ -1622,10 +1625,10 @@ _cache_device_identity(const char* const jid, uint32_t device_id, ec_public_key* g_key_file_set_string(omemo_ctx.knowndevices.keyfile, jid, device_id_str, fingerprint); omemo_known_devices_keyfile_save(); - Autocomplete ac = g_hash_table_lookup(omemo_ctx.fingerprint_ac, jid); + Autocomplete ac = g_hash_table_lookup(omemo_static_data.fingerprint_ac, jid); if (ac == NULL) { ac = autocomplete_new(); - g_hash_table_insert(omemo_ctx.fingerprint_ac, strdup(jid), ac); + g_hash_table_insert(omemo_static_data.fingerprint_ac, strdup(jid), ac); } char* formatted_fingerprint = omemo_format_fingerprint(fingerprint); diff --git a/src/omemo/omemo.h b/src/omemo/omemo.h index 624bec51..040ab4b7 100644 --- a/src/omemo/omemo.h +++ b/src/omemo/omemo.h @@ -51,8 +51,6 @@ typedef enum { PROF_OMEMOPOLICY_ALWAYS } prof_omemopolicy_t; -typedef struct omemo_context_t omemo_context; - typedef struct omemo_key { unsigned char* data; diff --git a/src/omemo/store.c b/src/omemo/store.c index 08291460..fd085a58 100644 --- a/src/omemo/store.c +++ b/src/omemo/store.c @@ -43,7 +43,7 @@ GHashTable* session_store_new(void) { - return g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)glib_hash_table_free); + return g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); } GHashTable* @@ -61,7 +61,7 @@ signed_pre_key_store_new(void) void identity_key_store_new(identity_key_store_t* identity_key_store) { - identity_key_store->trusted = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)glib_hash_table_free); + identity_key_store->trusted = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); identity_key_store->private = NULL; identity_key_store->public = NULL; } @@ -71,7 +71,7 @@ identity_key_store_destroy(identity_key_store_t* identity_key_store) { signal_buffer_bzero_free(identity_key_store->private); signal_buffer_bzero_free(identity_key_store->public); - glib_hash_table_free(identity_key_store->trusted); + g_hash_table_destroy(identity_key_store->trusted); } int