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

Set messageKeysLimit to unlimited if communicating with our devices #1348

Merged
merged 2 commits into from
Aug 4, 2017

Conversation

scottnonnenberg
Copy link
Contributor

@scottnonnenberg scottnonnenberg commented Aug 4, 2017

Note: Dependent on the work in this PR: signalapp/libsignal-protocol-javascript#29. Need to merge that PR and pull dist/libsignal-protocol.js from that project into this PR.

This PR uses the new ability to set the messageKeysLimit on SessionCipher to remove the limit when communicating with your own devices. We only do this for SessionCipher objects which are used for encryption/decryption - there are a number of other instantiations which are used entirely to do session management.

A number of open issues reference this error "Too many message keys for chain": https://github.com/WhisperSystems/Signal-Desktop/issues?utf8=%E2%9C%93&q=is%3Aissue%20is%3Aopen%20%22Too%20many%20message%20keys%20for%20chain%22

Hopefully these changes will fix that issue. At the very least it will help us get at the root problem.

@scottnonnenberg scottnonnenberg force-pushed the message-keys-limit branch 2 times, most recently from 443e30f to 592b7c6 Compare August 4, 2017 18:30
@scottnonnenberg scottnonnenberg changed the base branch from combined to master August 4, 2017 19:03
var sessionCipher = new libsignal.SessionCipher(textsecure.storage.protocol, address);

var ourNumber = textsecure.storage.user.getNumber();
var theirNumber = address.toString().split('.')[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this just be theirNumber = number?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that it is generated from the plain 'address', I suppose. My attempt at extreme clarity made things harder to read?? Whaa? :0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants