Skip to content
This repository has been archived by the owner on Aug 5, 2021. It is now read-only.

Only checking against the SignalProtocolAddress name when calling isTrustedIdentity in SessionBuilder #83

Open
fstracke opened this issue Jul 8, 2021 · 1 comment

Comments

@fstracke
Copy link

fstracke commented Jul 8, 2021

Hey! I've recently started implementing End-to-end encryption using this library.
However when implementing a multi-device scenario I constantly got errors about changing IdentityKeys. When I checked the source code of the SessionBuilder class I found following line causing the error:

return this.storage.isTrustedIdentity(
this.remoteAddress.getName(), device.identityKey, this.storage.Direction.SENDING
).then(function(trusted) {

Here in line 10 the IdentityKeyStore is only checked for the name of the SignalProtocolAddress not the complete tuple of name.device. In the following lines the IdentityKeys are also only access through the address name.

Reading the documentation of the Signal Protocol, it states:

Sesame supports two different models for key pairs: With per-user identity keys, all devices under a user share the same key pair. With per-device identity keys, each device may have a different key pair.

With per-user identity keys, identity public keys for other devices are stored in UserRecords. With per-device identity keys, identity public keys for other devices are stored in DeviceRecords.

My question simply would be if this implementation of the Signal Protocol is indeed based on saving identity keys in the UserRecord, requiring them to be shared over multiple devices, and whether it would introduce security concerns to (privately) change the implementation to storing identity keys in DeviceRecords?

@fstracke
Copy link
Author

Furthermore, I've noticed how in different parts of the library, the toString() function is used when referencing IdentityKeys:
https://github.com/signalapp/libsignal-protocol-javascript/blob/master/src/SessionBuilder.js#L52-L55

Doesn't this cause the isTrustedIdentity() function to always return true, as long as a valid identityKey is passed to the function?

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

No branches or pull requests

1 participant