Skip to content

[#493] modify identity store to store one identity key per contact

aguestuser requested to merge 493-modify-identity-store-impl into main

Closes #493 (closed)

context

  • we are getting ready for a big ETL job #463 and want to get our data model in good shape
  • in prepearing for the ETL and looking at existing signald data, we noticed that signald only stores one identity record per contact, whereas signalc currently (improperly and unlike singal-android) stores one per device
  • this MR refactors the data model to accomodate storing one identity per contact, and makes some related refactors to our identity-trusting code made possible by this change (and making future work to migrate the identity store to redis more tractable)

changes

data model

  • remove device_id from identities table
  • collapse all previous identity records (of which there used to be one for each device and thus N per contact) into one per contact
  • leverage this change to create a new ContactRecord interface that looks up records by their accountId-contactId 2-tuple (rather than their accountId-contactId-deviceId 3-tuple as with DeviceRecords)
  • modify the interface overriding-functions in SignalcIdentityStore to leverage this lookup interface
  • bonus: add created_at and updated_at fields to help us sleuth any bugs we might run into w.r.t. safety number changes

logic

  • having modified the shape of the identity store to disregard device id's, we no longer need to query for identity records by fingerprint.
  • yay! so: we modify (and rename) #trustFingerprint and add #untrustFingerprint (the latter of which will be needed in our ETL job) to lookup identity records by their address, and (as a layer of safety), throw if we are trying to trust a fingerprint that does not match the one we have stored for a given identity
  • we then do a couple of data model tricks that will pay off in a protocol store refactor:
    • first, we add VerifiedAccount#identifier and SignalcAccount#identifier (and allow them to default to uuids, which will soon be safe once we have a ProtocolStore#resolveId function in place to disambiguiate btw/ uuids and e164 numbers).
    • then, we leverage this to (pretty) safely convert from a SignalcAddress to a ProtocolStoreAddress
    • finally, we use the final converter to allow us to call the libsignal-defined saveIdentity (which requires a SignalProtocolAddress) from SocketReceiver, thus eliminating the need for IdentityStore#saveFingerprintForAllIdentities, which we delete
  • to make our confidence in our tests slightly higher, we modify StringGen#genFingerprint to run the same code that signal and signalc run when generating the fingerprint we send to signalboost and back to signalc on the trust path

[1] this was a workaround to not have to lookup all fingerprints for a given contact id, which is now unnecessary. moreover, it necessitated looking up an indexed field, which will become impossible in the case where the identity store is a redis-backed key-value store)

archiving sessions

  • to match signal-android, we archive all sessions for a contact after updating its identity key (in SocketReceiver)
  • note, in signal-android this is performed by:
    • calling archiveAllSiblings (to archive all sessions than the session with the same device id as the SignalProtocolAddress provided by the caller) in the body of #saveIdentity... IFF that call performed an update to a new key.
    • see: TextSecureIdentityStore#saveIdentity
  • then archiving the session for the device corresponding to the address provided by the caller IFF the #saveIdentity call performed an update
    • see: IdentityUtils#saveIdentity
  • we can accomplish this more cleanly/efficiently by simply archiving all the sessions at the same time IFF a given call to #saveIdentity performs an update. so we do that!

misc

  • we were (unnecessarily) placing a size limitation on session record byte arrays in our sql table definition. these byte arrays can get quite long (by design). since the size restriction peforms no useful validation but could cause errors, we remove it!
Edited by aguestuser

Merge request reports