signalapp / libsignal

Home to the Signal Protocol as well as other cryptographic primitives which make Signal possible.
GNU Affero General Public License v3.0
3.05k stars 359 forks source link

Java method SenderKeyDistributionMessage.getDistributionId() returns wrong value #543

Closed imb591 closed 6 months ago

imb591 commented 7 months ago

For example:

var distribution = java.util.UUID.fromString("01234567-89ab-cdef-fedc-ba9087654321");
var senderKeyStore = new org.signal.libsignal.protocol.groups.state.InMemorySenderKeyStore();
var groupSessionBuilder = new org.signal.libsignal.protocol.groups.GroupSessionBuilder(senderKeyStore);
var address = new org.signal.libsignal.protocol.SignalProtocolAddress("addr", 1);
var message = groupSessionBuilder.create(address, distribution);
System.out.println(String.format("distribution = %s, message.getDistributionId() = %s, store has distribution %s, store has message.getDistributionId() %s",
                distribution,
                message.getDistributionId(),
                senderKeyStore.loadSenderKey(address, distribution) != null,
                senderKeyStore.loadSenderKey(address, message.getDistributionId()) != null));

As far as I understand, it should print

distribution = 01234567-89ab-cdef-fedc-ba9087654321, 
message.getDistributionId() = 01234567-89ab-cdef-fedc-ba9087654321,
store has distribution true,
store has message.getDistributionId() true

but with libsignal 0.35.0 it seems to read something at a wrong offset and prints

distribution = 01234567-89ab-cdef-fedc-ba9087654321,
message.getDistributionId() = 60000000-0000-0000-0123-456789abcdef,
store has distribution true,
store has message.getDistributionId() false
jrose-signal commented 7 months ago

Aw shoot, you're right. We have a mistake in the untyped bridge layer, never noticed because the Android app doesn't need to use that API. Will fix in the next release.

jrose-signal commented 6 months ago

Fixed in v0.37.0, thank you!