Skip to content
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

Allow users without certificate to send messages, display messages #1646

Closed
vinkabuki opened this issue Jul 19, 2023 · 7 comments
Closed

Allow users without certificate to send messages, display messages #1646

vinkabuki opened this issue Jul 19, 2023 · 7 comments
Assignees

Comments

@vinkabuki
Copy link
Contributor

vinkabuki commented Jul 19, 2023

We should not implement any new schema, but use CSRs for that and replace them with certificates once signed.

@vinkabuki vinkabuki added this to Quiet Jul 19, 2023
@vinkabuki vinkabuki converted this from a draft issue Jul 19, 2023
@vinkabuki
Copy link
Contributor Author

  • User without certificate should be able to send a message (maybe we should just simply make signature optional and mark messages as verified and unverified?). ideally we sign messages anyway, but do not check certs, so we have kind of backward compatibility once user got its certificate we know that previous messages are valid.

  • it's hard, because users will produce both signed and unsigned messages (my intuition is some UI indicator e.g 'message not verified with some explanation that message was sent before user got his cert)

  • All users should display messages from unregistered users. (remove signature and schema checks at different layers of the app) and maybe let's do general solution and focus message validation into single place so it's maintainable.

@siepra
Copy link
Contributor

siepra commented Sep 4, 2023

Here is how messages verification works right now:

  1. In backend/storage.service.ts:402 there's a method for verifying message's signature
  2. Result of verification is being stored as a message object's property (boolean)
  3. All the messages are being passed down to state-manager
  4. State-manager stores the information about message verification status in redux (state-manager/verifyMessages.saga.ts)
  5. Unverified messages are being filtered out by the selector (state-manager/messages.selectors.ts:67)

@siepra
Copy link
Contributor

siepra commented Sep 4, 2023

This is my proposition of what we should do:

  1. Distinguish whether the signature is valid or unconfirmed (should it be backend's or state-manager's responsibility?)
  2. Do not filter out unconfirmed messages by the selector

I think the first point is mainly about renaming the properties. Although, if we want to keep the verification logic in backend, then we can probably get rid of state-manager's structure for verification statuses and stick to the object's prop only.

Is there any difference between unconfirmed messages and potential spoofing?

It is a certificate that holds the information about username, right? Will we be able to relate a username to the message using only CSR data? If the answer is yes, then there's probably an edge case while someone tries to sign a message using already taken username...

@vinkabuki

@vinkabuki
Copy link
Contributor Author

CSR contains same data as certificate (dm key, onion address, peerid, nickname)

@holmesworcester
Copy link
Contributor

holmesworcester commented Sep 5, 2023

Is there any difference between unconfirmed messages and potential spoofing?

In case this is helpful, we have two classes of "Potential spoofing" for unverified messages (messages from unregistered users):

  1. the username of an unregistered user matches another unregistered or registered user. Warn user of duplicate unregistered names #1559
  2. the username of a registered user matches the username of another unregistered user (the mapping of keys:users is not 1:1) Registered usernames that are duplicates or invalid should trigger aggressive warning #119

should it be backend's or state-manager's responsibility?

FWIW, to me it seems that this should be backend's responsibility.

@holmesworcester
Copy link
Contributor

Also, is this issue a duplicate of #1759 ?

@vinkabuki vinkabuki moved this from Next Sprint to Waiting for review in Quiet Sep 12, 2023
@vinkabuki vinkabuki moved this from Waiting for review to In progress in Quiet Sep 12, 2023
@Kacper-RF Kacper-RF moved this from In progress to Waiting for review in Quiet Sep 14, 2023
@Kacper-RF Kacper-RF moved this from Waiting for review to Merged (develop) in Quiet Sep 14, 2023
@Kacper-RF Kacper-RF moved this from Merged (develop) to Ready for QA in Quiet Sep 14, 2023
@kingalg
Copy link
Collaborator

kingalg commented Oct 23, 2023

Version: 2.0.1-alpha.7
mobile@2.0.0-alpha.21 (iOS 319)

It's working on both mobile and desktop. All bugs related to certificates are reported in separate issues.

@kingalg kingalg closed this as completed Oct 23, 2023
@kingalg kingalg moved this from Ready for QA to Done in Quiet Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

5 participants