Automatic OMEMO key expiration is too strict

[chrisballinger]

Currently, if you haven't received a message from someone else's OMEMO key for over thirty days, it is considered "expired", and must be manually re-enabled in the contact's profile view. This expiration is currently silent, and can be confusing to end users.

The whole point of key expiration was that if you had like multiple keys for a contact, and some of them were for lost/stolen/unused devices, to eventually stop encrypting to ones that you haven't received a message from in a while.

I think we should always keep/trust at least one key regardless of its "expiration". If you have 5 keys for someone, and they are all expired, we should continue to trust the most recently seen key (in terms of receiving a message from them). Using this new approach if you've only ever had one key for someone, it should never expire.

The current behavior can result in a scenario where if two people haven't spoken in >30 days, and keys on both sides are considered expired, no messages can be sent until someone manually marks an expired key as trusted again.

[mightyBroccoli]

I am really not sure if this is the correct way to handle OMEMO keys all together. The ability to retire keys is really helpful but should not be automated at least not in that small time frame. The amount of people affected by the automated retirement I guess is small, but for those this "feature" is even worse. For those people this mechanism will tear down the user experience. It would be totally ok if there is a way to verify ones keys, which would make them unretireable. I would really like a system to verify keys with qr codes as in Conversation/ PixArt. CS would need some way to generate its own qr code to verify itself to others, and if there is time verify its own possible secondary keys too.

[Lyokovic]

I also often have issues because of that. I have multiple devices and I do not use some of them for some time. The keys got disabled on my contacts using ChatSecure and they receive my messages but when they answer, the message is not encrypted with this device key so I do not see the answer. As those contact are not "technical people" it is not easy to tell them to re-enable the keys each time.

Conversations approach is (as what I see using it) that keys expire after some time, so next messages are not encrypted with the expired keys (like ChatSecure). But when a new message is received with an expired key, the key is re-enabled automatically and answers are then visible on the concerned contact device.

This approach works quite well (I use Conversations a lot). Maybe it is something to consider for ChatSecure? Or is it too permissive?

[zuglufttier]

For me ChatSecure disables keys way too soon (less than two weeks) and I don't know why. It just seems to happen at random. OMEMO only works for contacts with exactly one key reliably.

@chrisballinger, please enable a way to disable this behaviour completely until this is resolved and keys expire when they are supposed to.

[a11fruitless]

OMEMO key expiration is a huge issue for us. By far our biggest issue. It really makes the whole experience extremely confusing to an unsuspecting user! I want to echo other people here in saying that until a better solution is found, I'd like a way to disable key expiration all together. That, or at least take the Conversations approach described by Lyokovic.

[kmq]

Is it possible that this is a bug instead of the deliberate key-deactivation ? The keys don't seem to be deactivated in line with the 30 day expiry.

Looking at storeDevices it looks like the following could be happening:

1. A DeviceList is published containing a certain key
2. For some reason (I really don't know enough about OMEMO) the next time the DeviceList is published the key is removed from that list
3. ChatSecure marks the key as "removed" but does not delete it
4. The key is re-added to the DeviceList and published.
5. ChatSecure sees that it already has the key, but because it's not marked as "new" but rather as "removed" it does not get activated but shows up again in the de-activated state

Can someone with more Swift and OMOEMO/XMPP smarts than me validate or disprove this theory ?

[chrisballinger]

@kmq I'm pretty sure that's exactly what's happening, and that some XMPP servers may be occasionally returning empty device lists or something, triggering that behavior.

[mimi89999]

Older versions of Prosody didn't save PEP nodes on the disk, so after a server restart all OMEMO keys were gone.

[zuglufttier]

I use ejabberd and only ChatSecure is behaving that way.

[mimi89999]

Conversations always keeps the last key active and never expires it. On top of that, I think they automatically enable expired keys when they notice them. That makes the entire thing usable.

[sindastra]

I have to manually enable some keys every time I launch the app... "removed by server". #1115

[Lyokovic]

I think the problem (or one problem) is that the previously trusted keys which were deleted from the server are not reenabled when they are re-added on the server. I quickly investigated this issue in the code, and I might have found where the issue is, but I do not have access to a Mac right now to try to open a PR.

In the file OTROMEMOStorageManager.swift, function storeDevices. On line 104, the list of devices to add is built by substracting the "already known devices" set to the server device set. The issue is that the removed devices are present in the "already known devices" set so they are not in the "devices to add" set. There should be another device set containing "already known removed" devices which are now back on the server, and then reenable each of them.

I cannot open a PR right now, but I'll try to update my old Mac and install Xcode to test a fix and PR. Or if someone wants to do it before, go on ;)

[afriedmanGlacier]

We had similar problems (we use eJabberd), and added a conditional on line 112 in the handling of devicesToRemove to check for expiration in addition to being removed from the server. This way if the server returns an empty device list or something weird happens, it doesn't immediately put devices in the trustLevel .removed state. This helped quite a bit.

if (device.isExpired()) { device = device.copy() as! OMEMODevice...

HOWEVER, we do still have occasional issues, and now I'm wondering, why is it set to "removed" trustLevel rather than actually removed from the list of previouslyStoredDevices like it is on line 99? device.remove(with: transaction)

At least if it was removed and then re-added, it wouldn't be in the "already known devices" set. Am I misunderstanding something? Is there a reason to keep it around if it is removed from the server? The comments say "mark them as removed for historical purposes" but I'm wondering what historical purpose it serves?

[Lyokovic]

If the "removed" trust level is seen as "previously trusted but then removed from server" it makes sense to keep it : when it comes back on the server, it can be re-added as trusted directly (without asking). I think that's how Conversations works, and what I would like to do with my fix proposal. There should be another "expired" status though, which would not be automatically re-trusted if it comes back in the server list (I did not check if it exists of not).

Not removing the keys removed from the server can help to workaround the issue, but should not be done as a fix. A contact can be manually clearing its own omemo device keys from the server, and it should be reflected on its contact clients. If chatsecure properly reenable keys, it should not cause any issue when this contact re-adds devices it uses.

[afriedmanGlacier]

Maybe something like this around line 94?

`

    let newDeviceSet = Set(devices)
    newDeviceSet.forEach({ (deviceId) in
        let deviceKey = OMEMODevice.yapKey(withDeviceId: deviceId, parentKey: parentYapKey, parentCollection: parentYapCollection)
        guard var device = transaction.object(forKey: deviceKey, inCollection: OMEMODevice.collection) as? OMEMODevice else {
            return
        }
        
        if device.trustLevel == .removed {
            device = device.copy() as! OMEMODevice
            device.trustLevel = .trustedUser
            transaction.setObject(device, forKey: device.uniqueId, inCollection: OMEMODevice.collection)
        }
    })

`

Though I guess this doesn't tell us if they were at one point trusted before being removed, and we'd have to assume (as you said) that removed means "previously trusted but then removed from server."

[sindastra]

The way I would like to have it, is an option to manually "pin" OMEMO keys permanently and bypass expiration for a verified key altogether: #1115

To clarify, I'm not talking about automatically doing so. I'm talking about a switch or checkbox or option to permanently pin a chosen key and never have it expire or "removed by server" or anything of that sorts.