When setting up OMEMO for the first time via `/omemo gen` one had
to reconnect in order to make OMEMO work. This is fixed now.
Fixes: 5b6b5130 ("Fix OMEMO keyfile loading")
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
Several users have reported segfaults when starting up profanity which
has OMEMO support, but OMEMO is not set up yet.
@StefanKropp has been able to reproduce this and tracked it down to
`_load_identity()` calling `omemo_known_devices_keyfile_save()`.
The latter then calls `save_keyfile()` which calls
`g_key_file_save_to_file()`. This can then fail if one of the first two
strings is NULL and won't set the `error` on return. In its error handling
`save_keyfile()` unconditionally dereferences `error` which leads to the
segfault.
Fix this and also go through the entire codebase and verify that the usage
of `GError` is done correctly.
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
Instead of adding stuff to `_shutdown()`, we can now register a shutdown
routine from the respective `init()` function of our modules.
This also has the advantage, that we're sure they're called in reverse
order from how the initialization happened.
I didn't simply use `atexit()` because POSIX says that the number of
possible callbacks is limited (min 32) and I was not sure whether
we will maybe extend this number at one point.
Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
This reverts commit 3b099e9403.
This resulted in:
```
==5285== Invalid read of size 16
==5285== at 0x4FA80FC: ec_public_key_serialize (in /usr/lib64/libsignal-protocol-c.so.2.3.3)
==5285== by 0x4E5E76: omemo_identity_key (omemo.c:419)
==5285== by 0x4EBB7E: omemo_bundle_publish (omemo.c:129)
==5285== by 0x4E5BD9: omemo_publish_crypto_materials (omemo.c:335)
==5285== by 0x460407: sv_ev_connection_features_received (server_events.c:202)
==5285== by 0x43AA87: connection_features_received (connection.c:779)
==5285== by 0x4418C9: _disco_info_response_id_handler_onconnect (iq.c:2423)
==5285== by 0x43B9F1: _iq_handler (iq.c:241)
==5285== by 0x5163848: ??? (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x516A224: ??? (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x5E4FE43: ??? (in /usr/lib64/libxml2.so.2.12.8)
==5285== by 0x5E54927: xmlParseChunk (in /usr/lib64/libxml2.so.2.12.8)
==5285== by 0x5163450: xmpp_run_once (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x439797: connection_check_events (connection.c:162)
==5285== by 0x43894E: session_process_events (session.c:256)
==5285== by 0x4319FF: prof_run (profanity.c:128)
==5285== by 0x4EDAE6: main (main.c:174)
==5285== Address 0xa1cb1e0 is 16 bytes inside a block of size 72 free'd
==5285== at 0x484875B: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==5285== by 0x4395D0: _xfree (connection.c:110)
==5285== by 0x516A1A7: xmpp_stanza_release (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x516A16C: xmpp_stanza_release (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x516A16C: xmpp_stanza_release (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x516A230: ??? (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x5E4FE43: ??? (in /usr/lib64/libxml2.so.2.12.8)
==5285== by 0x5E54927: xmlParseChunk (in /usr/lib64/libxml2.so.2.12.8)
==5285== by 0x5163450: xmpp_run_once (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x439797: connection_check_events (connection.c:162)
==5285== by 0x43894E: session_process_events (session.c:256)
==5285== by 0x4319FF: prof_run (profanity.c:128)
==5285== by 0x4EDAE6: main (main.c:174)
==5285== Block was alloc'd at
==5285== at 0x4845794: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==5285== by 0x43958A: _xmalloc (connection.c:102)
==5285== by 0x516A0D1: xmpp_stanza_new (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x516BF34: ??? (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x5F10A17: ??? (in /usr/lib64/libxml2.so.2.12.8)
==5285== by 0x5E5481F: xmlParseChunk (in /usr/lib64/libxml2.so.2.12.8)
==5285== by 0x5163450: xmpp_run_once (in /usr/lib64/libstrophe.so.0.13.1)
==5285== by 0x439797: connection_check_events (connection.c:162)
==5285== by 0x43894E: session_process_events (session.c:256)
==5285== by 0x4319FF: prof_run (profanity.c:128)
==5285== by 0x4EDAE6: main (main.c:174)
```
Tested via sending OMEMO messages via 1:1 and in MUC.
libsignal does this properly, so there wouldn't be a real double free, but
it will `abort()`.
Instead of destroying the identity key on disconnect, already destroy it
after it has been put into the libsignal 'ratchet identity key pair'.
In the case where the key pair is initially generated, the public
and private parts are only `ref()`'ed once in [0].
In the case where the key pair is read from the disk, the public
and private parts are `ref()`'ed twice, first when decoded in [1] resp.
[2] and a second time in [3].
When `omemo_on_disconnect()` is called we were `unref()`'ing the parts
twice, before this patch. First in [4], a second time in [5] resp. [6].
Now we do the second `unref()` already when loading.
[0] `signal_protocol_key_helper_generate_identity_key_pair()`
[1] `curve_decode_point()`
[2] `curve_decode_private_point()`
[3] `ratchet_identity_key_pair_create()`
[4] `ratchet_identity_key_pair_destroy()`
[5] `ec_private_key_destroy()`
[6] `ec_public_key_destroy()`
Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
* 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 <jaeckel-floss@eyet-services.de>
Use a singleton `Jid` inside the connection instead of always re-creating
a `Jid` from the same string.
Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
We have to lazy load the keyfiles as they will maybe be written at one
point and should be initialized by that time.
Fixup of ca2df180d8
Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
As all parts of the code invoking the `files_get_account_data_path()`
function did the same afterwards, a function has been added with the same
behavior.
1. create path
2. `mkdir` of that path
3. return final path
Signed-off-by: Steffen Jaeckel <jaeckel-floss@eyet-services.de>
Remove support for libsignal-protocol-c < 2.3.2.
Debian 10 uses 2.3.2, Debian 11 and 12 use 2.3.3.
openSUSE from 15.2 onward uses 2.3.3.
Fedora since 28 uses 2.3.2.
We should be good.
After generation of an identity we observe that `omemo_ctx.device_list`
has an entry for sender's jid. But on application restart it is absent
thus messages are not encrypted for the rest set of sender devices.
This commit fixes this by applying code for acquiring the aforementioned device list after the connection.
In profanity are two handlers for device list:
* _handle_own_device_list
* _handle_device_list_start_session
I seems both handler will start a session via
omemo_start_device_session_handle_bundle
_handle_own_device_list will also make sure, that the own device is withing the
omemo device list. If we add the _handle_device_list_start_session into the
hashmap, we are not going the republish ourself, in case we clean-up the device
list from another client.
This will maybe fix#1552
99 Little Bugs in my Code.
Take one down.
Patch it around.
113 Bugs in my Code
