vanitasvitae / smack Goto Github PK

Those messages can be sent to repair broken sessions (as PreKeyMessages) or to simply forward the ratchet in case the client has not send messages for an extended amount of time to preserve forward secrecy.

According to this bug report there might be offline messages lost, since the stanzalistener for OMEMO messages gets registered after authentication, while it probably should be registered after connection.

Background: The client should generate and publish a new singedPreKey in their bundle every now and then (7-14 days). The old signedPreKey must be kept for ~a month in order to decrypt delayed preKeyMessages that were encrypted using the old signedPreKey. In order to strengthen forward secrecy, the old key should be deleted after a month though.

Possible solutions:

• Rotate keys every n days and ensure that maximal floor(28/n) keys are kept in storage.
• Store a creation date for every key and rotate/delete when keys are too old.

Manual rotation of the signedPreKey is already possible

MUC has to be private, deanonymized and the user must have bidirectional presence subscription with all participants.

This is not reliable reproducible, which makes it very hard to debug. I assume, that OmemoService.processBundle() writes the identityKey lazy to disk, so the when the key is requested for trusting, it is null. I'm not yet sure though.

Restarting the client helps.

I updated to the latest smack-omemo libraries and they changed something related to parsing message keys and auth tags. If you don't send a 32 byte long message + auth key or something jt throws a runtime error and crashes. The old code that was replaced has a link to
ChatSecure/ChatSecure-iOS#647

All in all, it seems ChatSecure iOS is using the "legacy format" whatever that means. This is what @chrisballinger had to say:

"The legacy format shouldn't crash smack-omemo, that's a bug. Consuming both legacy and new format should be supported simultaneously. Likely enough people are using new format that we can move away from sending legacy, but it's not top priority."

@n8fr8 I just made some changes to the API. Beginning with 14013a7 one OmemoService can handle multiple OmemoManagers/Devices. That enables you to have multiple devices active on the same XMPPConnection (useful for eg. testing).

This results in a slightly different setup. First, the client has to take care of the deviceId, since the OmemoStore doesn't do that any longer (store it eg. in androids shared preferences). Second, OmemoManagers.getInstanceFor() now takes two arguments, first the Connection and second the id of either an existing device, or null if you want to create a new device.

In short, you should have the following composition now:

• One OmemoService object per used OMEMO implementation (typically one SignalOmemoService).
• One OmemoManager per device (now you can use multiple devices per account).
• One OmemoStore per OmemoManager.
• Each OmemoManager has a reference to the used OmemoService (in theory you could in the future have eg. 2 OmemoManagers, one using a SignalOmemoService, and one using a Olm/XYZOmemoService at the same time!)

I hope, I did not miss anything important. I also updated the documentation. If you have any questions, don't hesitate to ask :)
Since the API is likely undergoing some more changes until it is merged, I'll leave this one open for now to document the development :)

Reported here.
This happens, when the OmemoManager was first created using OmemoManager.getIntanceFor(Connection) before the user logged in. In that case, there are two new jid folders in the OMEMO store: "[email protected]" and "user".

When the client than tries to get an instance of the OmemoManager via getInstanceFor after the user logged in, it gets an instance, that has no key material, so processBundle will throw an exception.

I suggest to either only create the OmemoManager after login, or keep one instance of the OmemoManager around as a local variable. Nevertheless, this must be fixed.

Testing against ChatSecure iOS, it receives and decrypts my omemo message successfully, but it also receives and shows the "I sent you an OMEMO encrypt message but you client doesn't seem to support that...".

Not sure, if its an upstream bug in new PubSubManager.getOrCreateNode() by @Flowdalic or if it was introduced when adopting those changes.

Anyway it seams like the ID gets sent to the DeviceList Node, but DeviceList updates still do not contain it afterwards.

Also I have to rewrite deviceID publishing, since at the moment its not really perfect.

This should be done in order to generate a valid session

like OmemoManager.conntactSupportsOmemo(BareJid contact)

It leads to confusion, that you cannot send a message to a muc by using its barejid by now.
This should get simplyfied by checking, if the provided jid is from a muc. In that case all members shall be added to the list of recipients.

This MUST NOT use code from smack-omemo-signal.

Otherwise the method fails

When my account is on prosody and their account is on ejabberd 16.09, I cannot fetch their device bundle, because the iq result only arrives 1 sec after the timeout. This is true with timeout 10 secs as well as 50 or 60 secs. Strangely enough fetching device lists works (same procedure, only difference I spotted is missing "max_items=1" in the query. Fetching bundles from prosody <-> prosody works (tested on one server).

I'm not sure, if this is an upstream issue of smack, prosody or ejabberd or just a faulty configuration. Maybe @Flowdalic has heard of something like this/has an idea how to debug this?

Ratchet update messages should not trigger decryption since the payload is null. They also should not be forwarded to the client as decrypted omemo messages.

This was proposed by @Flowdalic.
I'll have to refactor most methods so that every method gets called with the deviceId/the OmemoManager. Should be no big deal.

Edit: The FileBasedOmemoStore isn't that beautiful anyway. There is a lot of IO that should be made safer.

Otherwise there will be a nullpointer exception

We are seeing this error: https://prosody.im/issues/issue/805

against our home.zom.im server.

Was this resolved? Do we need to upgrade mod_pep?

Make sure, the Base64 provider is available in Android for example

Could be done by eg. stopping to decrypt after 50 failures (like Conversations), or similar.
The library should do this, because when the device created fresh keys, it will not be able to decrypt all messages in MAM, since none will be encrypted for the freshly generated key.

can be <element/> instead of <element></element>

Might already be done by libsignal.

That way a client can display, whether the sender of the decrypted message was trusted/untrusted/undecided.

This comes in handy when the user wants to do eg. integration tests.
Is needed when multiple devices share the same connection (very rare edge case)

deviceList node contains deviceIds of older tests despite the fact all items are deleted as well as the node itself.

Maybe this is caused by delayed PEP updates (race condition)?

Its not fatal, but its ugly to have all those exceptions in the log.

This is currently only logged, but not thrown as Exceptions.

Maybe we should get these guys to take a look!
https://pwnaccelerator.github.io/2017/signal-part3.html