okTurtles / group-income

A decentralized and private (end-to-end encrypted) financial safety net for you and your friends.
https://groupincome.org
GNU Affero General Public License v3.0
331 stars 44 forks source link

Fix login after failure to sync when username exists #1371

Open taoeffect opened 2 years ago

taoeffect commented 2 years ago

Problem

This problem occurs in the following scenario:

  1. You've logged in on Browser A to your account and everything is working fine.
  2. The server has lost its data or deleted your group's contracts
  3. You've re-registered your account on Browser B
  4. Now Browser A will show the following error when it refreshes:

Group Dashboard copy

Solution

When the server resets and you’ve recreated your username - but none of the contracts exist (including a different identity contract for yourself!) - there needs to be a way to delete your local contracts and reset everything. And in the future there needs to be a way to migrate your data to another server if you'd like, but for now it's more important that your browser doesn't get "stuck" in an unrecoverable way.

Maybe use gi.ui/prompt to ask if they’d like to reset everything, and if they say yes, delete the contracts that cannot be found and reset your local state afresh.

Silver-IT commented 1 year ago

When the server resets and you’ve recreated your username - but none of the contracts exist (including a different identity contract for yourself!) - there needs to be a way to delete your local contracts and reset everything.

@taoeffect, you mentioned none of the contracts exist. But what can we do if only a few contracts don't exist? For example, UserA joined two groups; GroupA and GroupB. But accidently, only GroupB contract (or HEAD file) is deleted. What can we do in this situation? Should we allow him to login or reset everything?

In my opinion, if any of them doesn't exist, we should reset.

taoeffect commented 1 year ago

There is a problem with resetting everything, and that is we have to treat identity contracts differently from group contracts. It is a much bigger deal to reset an identity contract than it is to reset a group. In fact, at the moment we cannot reset the lookup from username to identity contracts at all because they are cached, and it might stay that way for security reasons.

This issue specifically focuses on recovering from a situation where the identity contract has been wiped, and then the username is re-registered (creating a new identity contract). Again - this is a serious problem because any users from before who have interacted with this user will no longer be able to interact with them, because of the way each user locally caches the username->identityContract lookup.

You're are right that we should also handle the case where group contracts become unavailable suddenly. And that should be handled in a totally different way I think. The UI should inform the user that their group is now unusable as it appears to no longer exist on the server, but it should give them the option to recreate the group (perhaps on a different server) with the same state.

I'm thinking that I need to think more about how to solve this issue, so I'm going to self-assign this until I come up with a good answer, since it relates to the security stuff that I'm working on as well.

Silver-IT commented 1 year ago

Let's think about this case. UserA was in GroupA and GroupB in DeviceA, but accidentally UserA's identity contract(I was testing by removing all the backend database) is deleted in the backend. So UserA re-registered his account in another device (DeviceB) and joined GroupC. A few days later, UserA tried to login his account in the previous device DeviceA.

What should be happening? 1) UserA should take part of only GroupC? 2) UserA should take part of GroupA and GroupB? 3) UserA should take part of GroupA, GroupB and GroupC?

I remember that we are going to treat identity contract differently from group contracts. And I thought user's profile in group contract is just like using computed in Vue whose input value is identity contract and the return value could be another profile. So if the identity contract does not exist, no more actions could be made by the user, and by it's profile in the group, I think.

And I think it's not possible to reset the username⇾identityContractID lookup in the backend at the moment but we can clear the locally cashed username⇾identityContractID lookup. You say It might stay that way for security reasons. Can you tell me the security reasons?

taoeffect commented 1 year ago

What should be happening?

I'm not entirely sure myself yet.

This might need some input from @corrideat.

With the way @corrideat currently has it designed, a server wipe + a re-registration with the same password results in different private keys being generated because a different salt is used to generate the private key. Normally using a salt is a good thing because it makes it very difficult to bruteforce account passwords, but in this case, it means that existing conversations between that user and others will now appear to be MITM attacks.

Here's a specific scenario:

Even if we got rid of the salt, this could still happen if they chose a different password. The key would still be different in that case.

So I think, to keep things secure, we need to somehow re-upload the identity contract from the client to the server, along with the original salt information. However, even if we do implement that, we also need to handle the case where the user first re-registers their own account instead of re-uploading their old account.

So, this is a massive headache and really complicated to address. I don't have a good answer yet to this problem and need to think more on it. Of course, suggestions/ideas are welcome.

You say It might stay that way for security reasons. Can you tell me the security reasons?

The point is to prevent the server from being able to impersonate someone. If we don't cache the lookup, then the server can at any point impersonate you by changing the username->identityContractID mapping to one it controls. By caching this value, we make it so that it doesn't matter if the server changes it - existing clients that have DMs with you will ignore the server's value and will use their cached value.

Silver-IT commented 1 year ago

To be honest, my opinion is that it's not secure to treat those two users the same. Let's think about this case.

  1. Alex registered user with the username testuser.
  2. The database is deleted in the backend accidentally. But Alex was on vacation so he wasn't active in GroupIncome and didn't notice yet.
  3. At that time, Greg registered with the username testuser, the same username which Alex was using.

Should we treat these two users the same for only the reason that they using same username? Actually, I suggest the first answer 'UserA should take part of only GroupC?' for the previous question.

taoeffect commented 1 year ago

I agree, they shouldn't be treated as the same. So the answer is, I think:

we need to somehow re-upload the identity contract from the client to the server, along with the original salt information

But if someone re-registers our username before we do that — even if it's us — we won't be able to do that. If we were the ones who re-registered our username, however, it will at least be possible to delete the account potentially, and then re-upload and re-register from the original device.

corrideat commented 1 year ago

IMHO what makes this issue difficult to reason about is the hybrid between having a centralised database and at the same time treating the same data as decentralised.

I'd discussed this point above with @taoeffect and, at the time at least, we concluded that the centralised database should be regarded mostly as an implementation detail, but not as a 'permanent' feature of the protocol. For this reason, and if this consideration still holds, I'd say that any solution to reconcile the states and which might result in permanently affecting the client state must be presented to the user and not be automatically resolved. A broken client might be preferrable if the server is misbehaving (whether maliciously or not).

With that in mind, I'll try to answer to the discussion points. Suffice it to say that most of the following is my opinion only. I'll explain my reasoning, but it's likely that other solutions exist which are better, especially under a different set of assumptions.

There is a problem with resetting everything, and that is we have to treat identity contracts differently from group contracts. It is a much bigger deal to reset an identity contract than it is to reset a group. In fact, at the moment we cannot reset the lookup from username to identity contracts at all because they are cached, and it might stay that way for security reasons.

Agreed. However, there might be good reasons to reset the mapping, such as users username-squatting. This might or might not be a separate issue. How to do the change is tricky, but likely (non-deleted) users would receive some kind of warning about this, and, after executing some consensus protocol they'd either agree to remove the user or to do something else (what this might be is tricky, but it could involve switching to a different server).

This issue specifically focuses on recovering from a situation where the identity contract has been wiped, and then the username is re-registered (creating a new identity contract). Again - this is a serious problem because any users from before who have interacted with this user will no longer be able to interact with them, because of the way each user locally caches the username->identityContract lookup.

Agreed here too. The server shouldn't be deleting data accidentally. But regardless data might still get deleted for various reasons, whether accidentally or because of not paying for hosting, malicious activity or just to free up disk space.

Normally, this wouldn't be too much of a concern, in the sense that contract IDs are unique. So, deleting group X only affects group X. In the case of special contracts that have a mapping between namespaces, like usernames do, this is a serious issue that simply cannot be silently ignored by clients, lest there be a MitM attack.

So, the solution in this case should involve at least some form of warning if the identity contract that's being referenced changes. An example (though perhaps not the best solution) could be the security warnings shown in e.g., FB Messenger, Signal or WhatsApp when the keys associated with a user change (from signing up on a different device).

You're are right that we should also handle the case where group contracts become unavailable suddenly. And that should be handled in a totally different way I think. The UI should inform the user that their group is now unusable as it appears to no longer exist on the server, but it should give them the option to recreate the group (perhaps on a different server) with the same state.

Agreed for the reasons stated above.


UserA was in GroupA and GroupB in DeviceA, but accidentally UserA's identity contract(I was testing by removing all the backend database) is deleted in the backend. So UserA re-registered his account in another device (DeviceB) and joined GroupC. A few days later, UserA tried to login his account in the previous device DeviceA.

What should be happening?

  1. UserA should take part of only GroupC?
  2. UserA should take part of GroupA and GroupB?
  3. UserA should take part of GroupA, GroupB and GroupC?

First of all, this (UserA tried to login his account in the previous device DeviceA) shouldn't be possible. But let's say that somehow it is. Then, UserA:DeviceA takes part in GroupA and GroupB and UserA:DeviceB takes part in GroupC. This because the crypto keys for the different devices are entirely different.

Now, a potential solution to this scenario could involve some 'identity merge' or 'identity consolidation' mechanism whereby UserA:DeviceA & UserA:DeviceB coordinate to have a single account taking part in all three groups.


So I think, to keep things secure, we need to somehow re-upload the identity contract from the client to the server, along with the original salt information. However, even if we do implement that, we also need to handle the case where the user first re-registers their own account instead of re-uploading their old account.

Probably. Or that could be considered 'lost' and the user re-joins the groups under the new username (following the normal procedures for joining).

With the way @corrideat currently has it designed, a server wipe + a re-registration with the same password results in different private keys being generated because a different salt is used to generate the private key. Normally using a salt is a good thing because it makes it very difficult to bruteforce account passwords, but in this case, it means that existing conversations between that user and others will now appear to be MITM attacks.

Correct.

Even if we got rid of the salt, this could still happen if they chose a different password. The key would still be different in that case.

Correct (and getting rid of the salt is a terrible idea as well; I know this was just a hypothetical).

So, this is a massive headache and really complicated to address. I don't have a good answer yet to this problem and need to think more on it. Of course, suggestions/ideas are welcome.

I think the re-upload / restore is impossible to get around. You can't use data you don't have, and in this case can't get it out of thin air either.


Should we treat these two users the same for only the reason that they using same username?

As mentioned above, likely not, with the caveat that previous users that had relied on that mapping should very much be warned of the change.

Actually, I suggest the first answer 'UserA should take part of only GroupC?' for the previous question.

Agreed, as I mentioned above.


taoeffect commented 10 months ago

I've created a related issue that I think will help us deal with these types of situation much better: #1814

It certainly addresses how to deal with this situation from the perspective of the user whose account was deleted, as from their perspective the "link" between their local account and the remote account is broken - "disconnected" - and they can decide what to do from there.

As far as how others will view the situation, for example: former chat participants in DMs, we can let them know that the user's account on the server is gone, and that they can no longer chat with this user because of it.

From both perspectives, the username is treated as far less important that the identityContractID, and the expected course of action is for the user to either start over with a new account, or re-register their local account under a new username on either the same server or a different server under any available username.