Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/omemo #1039

Merged
merged 100 commits into from
Apr 11, 2019
Merged

Feature/omemo #1039

merged 100 commits into from
Apr 11, 2019

Conversation

paulfariello
Copy link
Contributor

@paulfariello paulfariello commented Mar 1, 2019

Add native support to OMEMO

TODO:

  • OMEMO crypto materials generation
  • send OMEMO encrypted messages
  • receive OMEMO encrypted messages
  • long term storage for identity keys
  • add long term storage for session, {signed,}prekeys
  • get rid of libsodium
  • handle carbons
  • handle device_list_request response
  • check if prekey can be avoid while sending message
  • trust key from command line
  • untrust key from command line
  • valgrind check
  • add support for old gcm tag format parsing (not being done for now since we are not aware of any client using it)
  • put gcm tag in key element (new format)
  • add support for signal pre v3 encryption (not being done for now since we are not aware of any client using it)
  • start session on reception of a message with prekey (and republish bundle)
  • add support for libsignal 2.3.2
  • muc support
  • ensure MUC has the right options (nonanonymous and access to fulljid)
  • Ensure PEP with bundle and device list are open (see publish option )
  • Check if publish-options is supported by server before attaching such node to query
  • Handle precondition-not-met error for publish-options
  • add autocompletion trust command
  • add autocompletion untrust command
  • add a way to list trusted keys
  • alert if sending message in a MUC with untrusted devices
  • add visual feedback when message has been sent on MUC

To test this PR you must install:

  • gcrypt
  • libsignal-protocol-c (>= v2.3.1)

On first run, ensure to create OMEMO cryptographic materials:

/omemo gen

To start an omemo session you must start it twice (issue about handling device_list_request response):

/omemo start buddy@example.org

Then you must trust some fingerprint:

/omemo fingerprint
/omemo trust <fingerprint>

@paulfariello paulfariello mentioned this pull request Mar 2, 2019
@paulfariello paulfariello force-pushed the feature/omemo branch 4 times, most recently from 5ce153e to 6a3113c Compare March 6, 2019 16:22
@philipflohr philipflohr mentioned this pull request Mar 6, 2019
Echolon added a commit to Echolon/omemo-top that referenced this pull request Mar 6, 2019
They are close to release as this sounds like a very prospering draft: profanity-im/profanity#1039

Cheers
bascht pushed a commit to bascht/omemo-top that referenced this pull request Mar 7, 2019
They are close to release as this sounds like a very prospering draft: profanity-im/profanity#1039

Cheers
@kaffeekanne
Copy link
Contributor

To build this PR with profanity i had to remove -Werror from EM_CF_FLAGS in configure.ac
i am no developer, how should it be actually built?

@paulfariello
Copy link
Contributor Author

@kaffeekanne that's an issue. It should build without error. Can you provide a log of failing build?

@kaffeekanne
Copy link
Contributor

kaffeekanne commented Mar 8, 2019

Sure:

src/omemo/omemo.c: In function ‘omemo_init’:
src/omemo/omemo.c:108:30: error: initialization of ‘int (*)(signal_buffer **, signal_buffer **, const signal_protocol_address *, void *)’ {aka ‘int (*)(struct signal_buffer **, struct signal_buffer **, const struct signal_protocol_address *, void *)’} from incompatible pointer type ‘int (*)(signal_buffer **, const signal_protocol_address *, void *)’ {aka ‘int (*)(struct signal_buffer **, const struct signal_protocol_address *, void *)’} [-Werror=incompatible-pointer-types]

         .load_session_func = load_session,
                              ^~~~~~~~~~~~
src/omemo/omemo.c:108:30: note: (near initialization for ‘session_store.load_session_func’)
src/omemo/omemo.c:110:31: error: initialization of ‘int (*)(const signal_protocol_address *, uint8_t *, size_t,  uint8_t *, size_t,  void *)’ {aka ‘int (*)(const struct signal_protocol_address *, unsigned char *, long unsigned int,  unsigned char *, long unsigned int,  void *)’} from incompatible pointer type ‘int (*)(const signal_protocol_address *, uint8_t *, size_t,  void *)’int (*)(const struct signal_protocol_address *, unsigned char *, long unsigned int,  void *)’} [-Werror=incompatible-pointer-types]
         .store_session_func = store_session,
                               ^~~~~~~~~~~~~
src/omemo/omemo.c:110:31: note: (near initialization for ‘session_store.store_session_func’)
cc1: all warnings being treated as errors
make[1]: *** [Makefile:1823: src/omemo/omemo.o] Error 1
make[1]: Leaving directory '/home/ta/profanity'
make: *** [Makefile:1158: all] Error 2

Steps to reproduce:

git clone https://github.com/boothj5/profanity.git
cd profanity
git pull origin pull/1039/head
./bootstrap.sh
./configure
make

Do you set your build flags somewhere outside that configure.ac? Also i noticed libsodium is still being checked although you mention only needing libgcrypt and libsignal-protocol-c.

Also i /omemo gen does not give me any feedback and trying to start a omemo session does not work, because no crypto was generated. Any hints? I know this is alpha, but i am very interested in helping out.

@NiklausHofer
Copy link

@kaffeekanne I have observed the same behaviour with /omemo gen. You can work around this by restarting profanity after generating the keys.

@kaffeekanne
Copy link
Contributor

@NiklausHofer ok, that did the trick for generation. Now i get the feedback, that everything has been created. Starting a OMEMO-Chat segfaults profanity.

@NiklausHofer
Copy link

@kaffeekanne I am getting this as well, yes. See my comment in the bug report: #658 (comment)

@paulfariello
Copy link
Contributor Author

@kaffeekanne can you tell which version of libsignal you have? I suspect it's 2.3.2. Currently the PR only support 2.3.1.

@paulfariello
Copy link
Contributor Author

Concerning the /omemo gen giving no feedback. I'll add this. And fix the issue of required restart after it.

@kaffeekanne
Copy link
Contributor

libsignal-protocol-c is 2.3.2 using Archlinux. So every involved component should have the version of upstream.

@paulfariello
Copy link
Contributor Author

Sadly libsignal broke the api between 2.3.1 and 2.3.2 😒

@kaffeekanne
Copy link
Contributor

kaffeekanne commented Mar 8, 2019

Building libsignal-protocol-c in version 2.3.1 results in other errors. That is somewhat out of my league. I can merely understand well written build instructions but not sort out detailed compiler errors.

@NiklausHofer
Copy link

I am using 2.3.1 (signalapp/libsignal-protocol-c@aac6c68) though and as mentioned I am also getting the segfault.

@paulfariello
Copy link
Contributor Author

@kaffeekanne just wait monday so I can add support for 2.3.2.

@NiklausHofer yes your issue doesn't seems to be related to libsignal.

@paulfariello
Copy link
Contributor Author

@kaffeekanne 61ec496 should add support for both 2.3.2 and 2.3.1

@paulfariello
Copy link
Contributor Author

@NiklausHofer 35a11bf should fix your segfault.

@kaffeekanne
Copy link
Contributor

I can compile with libsignal-protocol-2=2.3.2 now and don't get segfaults. But i can't establish a OMEMO-session, did /omemo start <jid>, /close, /omemo start <jid>. The recepient is using gajim, if that matters. Anything i do to analyse? Where would you want to discuss such issues @paulfariello ?

@paulfariello
Copy link
Contributor Author

You could give a look at logs and xmlconsole.
What happens when you send a message? The other end do not receive anything?
What happens when the other end tries to send you messages? You do not see anything?
I'm using gajim for some test and have no issue with it.

I'll have to add way more logs to debug this kind of issue.

@jubalh
Copy link
Member

jubalh commented Mar 13, 2019

I think add support for libsignal 2.3.2 is already done, right? So this can be checked off? :)

We can't keep it between two connection because signal context is
specific to a given account.
We try to reconfigure node and publish again.
If it fails again then we give up.
devicelist handler should be kept after trigger
Reflected messages can't be filtered by nick only otherwise you might
ignore messages comming from you on another devices.

Consequently we maintain a list of sent messages id in mucwin.
To be sure the id will be correctly reflected we use the origin-id
stanza.
Add sv_ev_connection_features_received for that purpose
Stop using "jid:device_id" keys. And move long term storage to its own
file: trust.txt.
When decrypting first message with prekey, libsignal wants to remove
used prekey from storage. Return value on success should be 0.

We used to return number of deleted keys. Thus libsignal was considering
we failed to remove the key and we were ignoring plaintext.
@jubalh jubalh merged commit 61df0c8 into profanity-im:master Apr 11, 2019
@jubalh
Copy link
Member

jubalh commented Apr 11, 2019

Basic OMEMO functionality merged!

@paulfariello some of the tasks in the first comment are unchecked. You you reply down here which ones and why we chose to leave them out for now?

Maybe we can also create individual issues for the one that make sense.

@paulfariello paulfariello deleted the feature/omemo branch April 12, 2019 14:26
@pasis
Copy link
Member

pasis commented Apr 17, 2019

Good job @paulfariello! @jubalh, sorry, I didn't have time to fully review the code. I will do it separately from the pull request.

@jubalh jubalh mentioned this pull request Apr 17, 2019
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants