omemo: ignore key contents if there is no payload#2147
Merged
jubalh merged 1 commit intoprofanity-im:masterfrom Apr 9, 2026
Merged
omemo: ignore key contents if there is no payload#2147jubalh merged 1 commit intoprofanity-im:masterfrom
jubalh merged 1 commit intoprofanity-im:masterfrom
Conversation
jubalh
requested changes
Apr 9, 2026
Member
jubalh
left a comment
jubalh
left a comment
There was a problem hiding this comment.
Makes sense. I'll review in detail later today. For now: Could you please change the commit message from omemo: ignore key contents if there is no payload to fix(omemo): ignore key contents if there is no payload or fix: ignore omemo key contents if there is no payload.
The reason for this is that we currently use the commit messages to generate the changelog entries. So the first word is about being fix/feature etc. Also see CONTRIBUTING.md for details :)
For key transport messages, Monal at least doesn't append the authentication tag to the plaintext key contents as that only makes sense if the key was used to encrypt something. This causes the key length check to fail and show the OMEMO message received but decryption failed. error to the user which is confusing because there is no user-originated message involved. Skip the length check for key transport messages as profanity only uses these to advance the ratchet and makes no use of the decrypted contents. Signed-off-by: Karel Balej <balejk@matfyz.cz>
balejk
force-pushed
the
omemo-heartbeats-fix
branch
from
2c266df to
1458b09
Compare
jubalh
approved these changes
Apr 9, 2026
Member
|
Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For key transport messages, Monal at least doesn't append the
authentication tag to the plaintext key contents as that only makes
sense if the key was used to encrypt something. This causes the key
length check to fail and show the
error to the user which is confusing because there is no user-originated
message involved.
Skip the length check for key transport messages as profanity only uses
these to advance the ratchet and makes no use of the decrypted contents.
How to test the functionality
I have not tested this as I know of no straightforward way to do so but it makes sense to me based on what I was able to find out. I suppose it could be tested by triggering an incoming heartbeat message by sending enough outgoing messages but it should also become apparent if it works over time based on whether the error still appears (right now with 0.17.0 I am seeing on average one every one or two days).