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

Rework support for XEP-0384: OMEMO Encryption #177

Merged
merged 1 commit into from Jun 13, 2018

Conversation

Projects
None yet
6 participants
@vanitasvitae
Contributor

vanitasvitae commented Oct 24, 2017

This PR ditches trust management from the OMEMO store. The store now only stores keys and the users ratchet. By doing so, the OmemoManager can be created while the client is offline. Once it comes online, it retrieves the users BareJid for usage with the OmemoStore.

THIS IS STILL WORK IN PROGRESS
Feeback appreciated.

Changes:

  • Rework integration tests
  • New structure of base integration test classes
  • bump dependency on signal-protocol-java from 2.4.0 to 2.6.2
  • Introduced CachingOmemoStore implementations
  • Use CachingOmemoStore classes in integration tests
  • Removed OmemoSession classes (replaced with more logical OmemoRatchet classes)
  • Consequently also removed load/storeOmemoSession methods from OmemoStore
  • Removed some clutter from KeyUtil classes
  • Moved trust decision related code from OmemoStore to TrustCallback
  • Require authenticated connection for many functions
  • Add async initialization function in OmemoStore
  • Refactor omemo test package (/java/org/jivesoftware/smack/omemo -> /java/org/jivesoftware/smackx)
  • Remove OmemoStore method isFreshInstallation() as well as defaultDeviceId related stuff
  • FileBasedOmemoStore: Add cleaner methods to store/load base data types (Using tryWithResource, only for future releases, once Android API gets bumped)
  • Attempt to make OmemoManager thread safe
  • new logic for getInstanceFor() deviceId determination
  • OmemoManagers encrypt methods now don't throw exceptions when encryption for some devices fails. Instead message gets encrypted when possible and more information about failures gets returned alongside the message itself
  • Added OmemoMessage class for that purpose
  • Reworked entire OmemoService class
  • Use safer logic for creating trust-ignoring messages (like ratchet-update messages)
  • Restructure elements/provider in order to prepare for OMEMO namespace bumps
  • Remove OmemoManager.regenerate() methods in favor of getInstanceFor(connection, randomDeviceId)
  • Removed some unnecessary configuration options
  • Prepare for support of more AES message key types
  • Simplify session creation
  • Where possible, avoid side effects in methods
  • Add UntrustedOmemoIdentityException
  • Add TrustState enum
  • More improved tests

TODO:

  • Reimplement message decryption
  • Reimplement stanza listening
  • Reimplement PEP listeners
  • Reimplement support for MAM queries
  • Reimplement some more convenience methods
  • More testing
  • Implement client side support for publish options

  • Document changes
  • Create migration guide
  • Rant about OMEMOs quirks
@IgniteRealtime-Bot

This comment has been minimized.

Show comment
Hide comment
@IgniteRealtime-Bot

IgniteRealtime-Bot commented Nov 17, 2017

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/smack-omemoservice-subscribetodevicelists-registerdevicelistlistener-happen-too-late-to-listen-for-pepevent-to-take-actions/73413/5

@IgniteRealtime-Bot

This comment has been minimized.

Show comment
Hide comment
@IgniteRealtime-Bot

IgniteRealtime-Bot Nov 17, 2017

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/omemomanager-does-not-responding-sometimes/79211/2

IgniteRealtime-Bot commented Nov 17, 2017

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/omemomanager-does-not-responding-sometimes/79211/2

@IgniteRealtime-Bot

This comment has been minimized.

Show comment
Hide comment
@IgniteRealtime-Bot

IgniteRealtime-Bot commented Nov 17, 2017

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/smack-4-2-1-beta2-snapshot-omemoservice-subscribetodevicelists-starts-up-too-late-to-process-pep-node-device-list-notify/73416/4

@IgniteRealtime-Bot

This comment has been minimized.

Show comment
Hide comment
@IgniteRealtime-Bot

IgniteRealtime-Bot commented Nov 24, 2017

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/getting-array-index-out-of-bound-exception-in-authentication-listener-during-initializing-omemomanger/62662/5

@IgniteRealtime-Bot

This comment has been minimized.

Show comment
Hide comment
@IgniteRealtime-Bot

IgniteRealtime-Bot commented Nov 24, 2017

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/smack-omemo-undecidedomemoidentityexception-getuntrusteddevices-returns-only-undecidedomemoidentity/73410/4

@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Nov 28, 2017

There were the following issues with your Pull Request

  • Commit: 80db752
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

GitCop commented Nov 28, 2017

There were the following issues with your Pull Request

  • Commit: 80db752
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Nov 28, 2017

There were the following issues with your Pull Request

  • Commit: 4ed3adf
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

GitCop commented Nov 28, 2017

There were the following issues with your Pull Request

  • Commit: 4ed3adf
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

@bansiRohit

This comment has been minimized.

Show comment
Hide comment
@bansiRohit

bansiRohit Dec 6, 2017

Hi ,
I want to implement xmpp chat , so please guide me

bansiRohit commented Dec 6, 2017

Hi ,
I want to implement xmpp chat , so please guide me

@vanitasvitae

This comment has been minimized.

Show comment
Hide comment
@vanitasvitae

vanitasvitae Dec 6, 2017

Contributor

@bansiRohit this is not a support forum. Please visit https://discourse.igniterealtime.org/ instead. Also please think about a better question to ask ;)

Contributor

vanitasvitae commented Dec 6, 2017

@bansiRohit this is not a support forum. Please visit https://discourse.igniterealtime.org/ instead. Also please think about a better question to ask ;)

@vanitasvitae vanitasvitae changed the title from OmemoManager offline rework to Smack-omemo rework Jan 2, 2018

@IgniteRealtime-Bot

This comment has been minimized.

Show comment
Hide comment

IgniteRealtime-Bot commented Jan 12, 2018

@cmeng-git

This comment has been minimized.

Show comment
Hide comment
@cmeng-git

cmeng-git Jan 12, 2018

Whenever user acquired new device, regenerate omemo identity or reinstall the app, a new omemoDevice is created and published to the server. Overtime, this deviceList gets larger and becomes unmanageable. In aTalk when user starts new to setup a omemo session with this buddy for the first time, he/she is presented with a long list of omemo identities to trust, including the unused identities. This really confuse the user.
Does your new release take care or would consider to implement the removal of unused omemoDevice that were previously published on the server.

cmeng-git commented Jan 12, 2018

Whenever user acquired new device, regenerate omemo identity or reinstall the app, a new omemoDevice is created and published to the server. Overtime, this deviceList gets larger and becomes unmanageable. In aTalk when user starts new to setup a omemo session with this buddy for the first time, he/she is presented with a long list of omemo identities to trust, including the unused identities. This really confuse the user.
Does your new release take care or would consider to implement the removal of unused omemoDevice that were previously published on the server.

@vanitasvitae

This comment has been minimized.

Show comment
Hide comment
@vanitasvitae

vanitasvitae Jan 13, 2018

Contributor

@cmeng-git devices, which do not send a message for an extended period of time will get ignored after a while and even deleted from the list when no message is received for some more time. Also there is (and has been) the method purgeDeviceList(), which deletes all but the device of the user from the list.

Contributor

vanitasvitae commented Jan 13, 2018

@cmeng-git devices, which do not send a message for an extended period of time will get ignored after a while and even deleted from the list when no message is received for some more time. Also there is (and has been) the method purgeDeviceList(), which deletes all but the device of the user from the list.

@IgniteRealtime-Bot

This comment has been minimized.

Show comment
Hide comment
@IgniteRealtime-Bot

IgniteRealtime-Bot commented Jan 31, 2018

@IgniteRealtime-Bot

This comment has been minimized.

Show comment
Hide comment
@IgniteRealtime-Bot

IgniteRealtime-Bot Mar 26, 2018

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/smack-omemo-rework-177-the-omemoservice-throws-concurrent-modification-exception/81134/3

IgniteRealtime-Bot commented Mar 26, 2018

This pull request has been mentioned on Ignite Realtime Community Forums. There might be relevant details there:

https://discourse.igniterealtime.org/t/smack-omemo-rework-177-the-omemoservice-throws-concurrent-modification-exception/81134/3

@vanitasvitae vanitasvitae changed the title from Smack-omemo rework to Rework support for XEP-0384: OMEMO Encryption May 22, 2018

@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Jun 9, 2018

There were the following issues with your Pull Request

  • Commit: b70e80b
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

GitCop commented Jun 9, 2018

There were the following issues with your Pull Request

  • Commit: b70e80b
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

@vanitasvitae vanitasvitae changed the base branch from 4.2 to master Jun 9, 2018

@vanitasvitae

This comment has been minimized.

Show comment
Hide comment
@vanitasvitae

vanitasvitae Jun 9, 2018

Contributor

Rebased on and now targeting master branch

Contributor

vanitasvitae commented Jun 9, 2018

Rebased on and now targeting master branch

@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Jun 9, 2018

There were the following issues with your Pull Request

  • Commit: a07a0ac
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

GitCop commented Jun 9, 2018

There were the following issues with your Pull Request

  • Commit: a07a0ac
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Jun 12, 2018

There were the following issues with your Pull Request

  • Commit: 3851b95
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

GitCop commented Jun 12, 2018

There were the following issues with your Pull Request

  • Commit: 3851b95
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

Rework support for XEP-0384: OMEMO Encryption
    Changes:

    Rework integration tests
    New structure of base integration test classes
    bump dependency on signal-protocol-java from 2.4.0 to 2.6.2
    Introduced CachingOmemoStore implementations
    Use CachingOmemoStore classes in integration tests
    Removed OmemoSession classes (replaced with more logical OmemoRatchet classes)
    Consequently also removed load/storeOmemoSession methods from OmemoStore
    Removed some clutter from KeyUtil classes
    Moved trust decision related code from OmemoStore to TrustCallback
    Require authenticated connection for many functions
    Add async initialization function in OmemoStore
    Refactor omemo test package (/java/org/jivesoftware/smack/omemo -> /java/org/jivesoftware/smackx)
    Remove OmemoStore method isFreshInstallation() as well as defaultDeviceId related stuff
    FileBasedOmemoStore: Add cleaner methods to store/load base data types (Using tryWithResource, only for future releases, once Android API gets bumped)
    Attempt to make OmemoManager thread safe
    new logic for getInstanceFor() deviceId determination
    OmemoManagers encrypt methods now don't throw exceptions when encryption for some devices fails. Instead message gets encrypted when possible and more information about failures gets returned alongside the message itself
    Added OmemoMessage class for that purpose
    Reworked entire OmemoService class
    Use safer logic for creating trust-ignoring messages (like ratchet-update messages)
    Restructure elements/provider in order to prepare for OMEMO namespace bumps
    Remove OmemoManager.regenerate() methods in favor of getInstanceFor(connection, randomDeviceId)
    Removed some unnecessary configuration options
    Prepare for support of more AES message key types
    Simplify session creation
    Where possible, avoid side effects in methods
    Add UntrustedOmemoIdentityException
    Add TrustState enum
    More improved tests
@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Jun 13, 2018

There were the following issues with your Pull Request

  • Commit: 1f731f6
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

GitCop commented Jun 13, 2018

There were the following issues with your Pull Request

  • Commit: 1f731f6
  • Your commit message body contains a line that is longer than 72 characters

Guidelines are available at https://github.com/igniterealtime/Smack/wiki/Guidelines-for-Smack-Developers-and-Contributors


This message was auto-generated by https://gitcop.com

@Flowdalic Flowdalic merged commit 153473c into igniterealtime:master Jun 13, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.3%) to 37.111%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment