Use generic encrypted payloads

[hackergrrl]

Hey @cblgh, this is per our earlier discussion! Here is my new proposal for the structure of private messages & encrypted messages in general:

1. Use a generic encryption format (no private/chat, just use text/chat so existing logic can be more easily reused).
2. Continue using 'type: "encrypted" on ciphertext payloads. This means encrypted messages won't affect any existing views, since it's a type unknown to them. Views can opt-in to supporting encrypted messages as needed.
3. I decided against a cipher_type key on the ciphertext message, since we didn't have a clear decision on the pros/cons of that. However, we can add later /wo breaking backwards compatibility.

1:1 private text message format

It's just the same as regular chat message, but encrypted, and the inner payload has a recipients key instead of channel key.

{
  type: 'chat/text',
  content: {
    text: 'hello',
    recipients: [keypair.publicKey.toString('hex')]
  }
}

Does this sound good to you? I kinda suspect that as you're implementing you'll discover any holes in this design. 😈

[cblgh]

thanks for putting up this pr @hackergrrl!

started looking at this a bit late so feedback may not be of the utmost quality at this stage, some thoughts follow regardless:

• if using chat/text might be nice to keep channel (and not use recipients) as-is for compat with existing logic for handling chat/text messages. at least in my current wip work on cabal-client i've got some logic to massage the previous private/text into the more generic format, and i think that is one of the parts that it massages (in addition to the type)

• we can add later /wo breaking backwards compatibility

  • great realization!

• will probably have more to say re: code once i rebase my existing work ontop of this!

some more thoughts re the private message usecase: i think it makes sense to start with only supporting 1-1 PMs atm; there's a lot more work that needs to happen for 1-many PMs to be deployed (in cli, in desktop, in cabal-client). additionally, i think there's a good case to make for the UX of 1-1 PMs being fairly well-understood by folx and users of chat apps in general, so starting there feels like a nice incremental step instead of a giant leap :3

1-many PMs with the secret box caveats and constraints (e.g. better make the max #recpts small as a tradeoff to make message overhead more reasonable) makes less sense, especially when thinking about how to communicate them to users of apps, than having a generic secret channel mechanism and using that for "group PMs"

a lot of words to say "let's keep channel: string and omit recipients: array 😊

[hackergrrl]

Ok, take a look at the latest changes.

My only concern with this approach is that private and public messages, after decryption, will look identical. I worry that there could be an exploit some day if we aren't careful where someone makes a channel with your public key to spoof a PM channel or something.

[cblgh]

thanks for the patch @hackergrrl!

regarding your concern: yup! totally valid. in my current cabal-client code i've taken some precautions wrt validation. for instance making sure nobody can create/join a channel if it conforms to the pub key regex && if there's a user known with that id—it probably needs to be stricter. this validation also doesn't help if you are building ontop of cabal-core directly of course.

i was thinking in bed earlier re the problems of having exactly the same structure as regular messages, and thought that maybe we can add a smol bit of metadata to private messages like private: true. using that bit of information we can make doubly sure in public (non-private) channels that we only ever display messages that are not marked as private (and only display private messages in channels that are marked as private). curious to hear what you think abt that!

[hackergrrl]

Yes, I'm very in favour of a private: true field as a precaution.

[cblgh]

@hackergrrl added the metadata (and am using it in my cabal-client code :)

also pushed the cabal-core fix i had locally for making sure the ready event fires when all indexes have been successfully loaded (and not only the sync ones)—hope that's not terribly out of scope, just felt a bit easier to do that since it was already in my working code

[hackergrrl]

Great. A last nice thing would be to update the test/private.js tests to check for private: true, but looks good to me. 👍🏻

[cblgh]

@hackergrrl thanks for the nudge! i'll get that done next week 😊

[cblgh]

@hackergrrl alright, there we go!

[cblgh]

@hackergrrl do you have any further comments? if not, i will go ahead and merge this + issue a new release (as i guess a new major? bc we changed the private messages api)

[hackergrrl]

@cblgh No comments!