-
Notifications
You must be signed in to change notification settings - Fork 33
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
Implement device name encryption and decryption #119
Conversation
Just leaving a note: there's still a merge commit in here that you should easily be able to rebase away :-) |
Yeah, it's just me being 😴 I'll push force the branch with the final result anyways. FYI: It doesn't work at all 😂 |
If you need a second set of eyes on something, let me know. |
I'll probably ping you tomorrow on the chat, I really can't figure it out - the crypto is sound, I have tested with both a round-trip test and decrypting some data obtained in EDIT: I believe it could be because this request is send over the websocket 😢 |
415f76d
to
295c67c
Compare
I don't know what the current state of this PR is (or if it is even worked on currently), but I would find that feature really useful. |
295c67c
to
5109b7b
Compare
Thanks a lot for digging this out @Schmiddiii I'm happy it works, I believe it didn't work before because it required the updates done as part of #145. I updated the PR and intend to merge it ASAP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, as long as you or @Schmiddiii say it works. One suggestion in line for a KAT.
Sadly, it does not seem to work completly. I was just trying to port this to presage again, it works fine if the config store is already set up correctly, but when deleting the config store and trying again, it fails and I get a As far as I interpret the logs, it does not correctly get the other devices to sent the account attributes to. Here are the logs. Failed
Successfull
|
Seems like it should include some retry logic for missing devices, like the one we have for sending messages. Specifically, this is the |
The reason behind <whisperfish#119 (comment)> was a rouge second `Self::json`. This would lead to the whole response being consumed by the first `Self::json` to print the response, the second `Self::json` would then not have to decode a empty response.
This enables the fix in <whisperfish/libsignal-service-rs#119>. I am not completly sure what name to give the device when registering as primary device and I am also unsure what device-capabilities presage has, so I mostly left it at default in combination with the previous device capabilities.
This was introduced in Signal-Desktop a few months ago. I'll implement encryption and decryption. In the meantime to make sure people can properly register their devices, we should stop passing a value to the
name
field.TODO:
Resolves #124