near / near-wallet

Web wallet for NEAR Protocol which stores keys in browser's localStorage
https://wallet.near.org
MIT License
214 stars 174 forks source link

Create Account with 2FA or Ledger #630

Closed kcole16 closed 3 years ago

kcole16 commented 4 years ago

Overview

For any wallet account account, there are either one (Ledger) or at least three keys (2FA)

2FA 1 key in localStorage 1 key in recovery 1 key on our server

TXs require 2-of-3 multi-sig

Stories

As a user of the NEAR wallet, I want to know if I should choose 2FA or Ledger/hardware wallet, so I can make the correct security and usability tradeoffs.

As a user without a Ledger device, I want to secure my account with two factor authentication, so I can safely use my account with larger token amounts (> $100 worth of assets).

As a user with a Ledger device, I want to secure my account by only using my Ledger to custody the key, so I can safely use my account with larger token amounts (> $100 worth of assets).

Note: We should highly recommend users choose Ledger over 2FA when storing large quantities of tokens. cc @corwinharrell

Acceptance Criteria

Flow

https://docs.google.com/document/d/1dUSbtaykXTZY00umNiO78Qb-PGGxA7CzWQRTl1lIbMA/edit

UI Design

https://www.figma.com/file/tH6dad9CulATLPo36UCCyg/630-Create-Account-w-2FA?node-id=0%3A1

mattlockyer commented 4 years ago

Updates on linkdrop contract and JS progress: https://github.com/near-examples/rust-linkdrop-app

Ledger Flow: this code should be enough since it's a 1 limited access key (LAK) flow and the num_confirmations is an argument the client side passes in when calling create_multisig_and_claim.

2FA Flow: unfinished is passing in the recovery and 2nd factor "confirmation" LAK from the server.

Some details missing in point (4) @kcole16 @vgrichina we should discuss:

4) can't call this until user confirms their 2FA method with server (code) 4 i b) after 4 4 ii) agree with the whole flow. This is the time to call create_multisig_and_claim (not createAccount from (4)) in the contract and the contract performs the batch transaction; creates account, deploys contract, instantiates w/ num_confirmations, adds all keys.

vgrichina commented 4 years ago

@mattlockyer

can't call this until user confirms their 2FA method with server (code)

Not sure what do you mean. This flow is to create account from the link. You just need to sign with whatever key in the link (same as now), no 2FA is needed here.

Is you second question about calling create_multisig_and_claim vs createAccount or smth else? If so – then for sure we need to call whatever method will create account with multisig contract, it doesn't have to be literally called createAccount.

mattlockyer commented 4 years ago

can't call this until user confirms their 2FA method with server (code)

I don't think we should create account before user has confirmed their 2FA method so we have all keys and we know they have access to that method. i.e. mistyped email/phone number.

Overall, the flow should complete and confirm everything that is off-chain first. Then in 1 call to the linkdrop contract: account is created, multisig deployed and keys added.

kcole16 commented 4 years ago

Overall, the flow should complete and confirm everything that is off-chain first. Then in 1 call to the linkdrop contract: account is created, multisig deployed and keys added.

Agreed. Added to the flow above

heycorwin commented 4 years ago

Here are the UI designs for the effected views. Feel free to leave comments in Figma.

Throwing together a quick prototype to show things in motion which I will attach shortly, in addition to some small changes to the profile view which will now display your active 2FA method.

A couple things to note and/or discuss:

  1. Should the user elect to secure their account with Ledger when setting it up for the first time, they will be kicked into the ledger flow that we recently implemented. The primary difference will be the copy and CTA of the last confirmation screen, which instead of suggesting that they remove existing recovery methods (which they won’t have at this point), simply directs them to their dashboard.

  2. 2FA Backup Codes: @kcole16 and I had a breif discussion about presenting the user with backup codes as they setup 2FA. This is standard practice in most applications (even for SMS based 2FA, not just app-based, which I’ve confirmed). We wanted to open up the discussion to ask if anyone has any opinions on this. Should we go with what is pretty common practice and offer backup codes? Is there any reason for us to omit this step?

  3. Removal of phone as an account recovery method: There are a couple justifications for the removal of phone # as a recovery method:

    a. If the user sets up SMS based 2FA upon account creation, and then is presented with the option to also use their phone # as an account recovery method, they will encounter a sort of UX deja-vu in the sense that they would be able to select phone receovery as a recovery method even after using it to set up their 2FA. This is a potential cause for confusion.

    b. Seperation of concerns. If the user elects to use their phone # for 2FA, then they can instead rely on email or seed phrase as account recovery methods. This improves security in the sense that now, phone # is only tied to a single key, vs 2 if they were to use phone # for both 2FA and account recovery. cc @kcole16 If I missed anything here.

heycorwin commented 4 years ago

It’s also worth noting that I’ll be making some changes to the design in relation to https://github.com/near/near-wallet/issues/442#, but these should primarily affect the copy in this flow.

kcole16 commented 4 years ago

Thoughts from today's convo:

  1. Backup codes will require a bit of work to get running, and also act similarly to a seed phrase for the user (we cannot show it again, and the user cannot lose it). The reason for this is that, if a user can view the backup codes from the frontend, then 2FA loses it's value as anyone with access to the frontend can also gain 2FA access.

My recommendation is to omit these for now, as most users will be able to regain access to their phone number, and many will be able to do the same for email. We can consider adding backup codes, but will need to consider the implementation more thoroughly.

  1. I agree that an optimal 2FA setup is SMS + email backup (or seed phrase). With SMS + SMS backup, users may lose access entirely if they lose their phone (localstorage key on phone, then both 2FA and backup key lost). They can regain access to their number, and potentially go through their phone records, but not ideal. Additionally, a SIM swapper can gain both 2FA access and potentially phone record access as well, compromising both key.

@Patrick1904 Suggested we add the option for email 2FA, in case users do not have SMS access. This opens up a similar vector to SMS + SMS, but may be worthwhile as a backup (we recommend phone and have that as default for 2FA) if users do not have SMS or Ledger (or cannot receive international SMS for some reason).

vgrichina commented 4 years ago

@corwinharrell @kcole16 I agree that seed phrase (or recovery link in e-mail/SMS) is our equivalent of backup codes, so we don't really need additional codes for 2FA. We expect users to setup 3 factors, but only need 2 at any given time for authorization.

vgrichina commented 4 years ago

Additionally, a SIM swapper can gain both 2FA access and potentially phone record access as well, compromising both key.

@kcole16 what do you mean by phone record access here? SIM swapping by itself shouldn't give attacker access to my actual physical phone.

kcole16 commented 4 years ago

what do you mean by phone record access here? SIM swapping by itself shouldn't give attacker access to my actual physical phone.

The attacker will have convinced your carrier that they are you in order to SIM swap, and theoretically can then request your SMS records, which will contain the recovery link. I do think that's somewhat unlikely (maybe extremely unlikely), but seems like a potential risk if it's a motivated attacker.

kcole16 commented 4 years ago

@vgrichina @ilblackdragon Was discussing potentially making this flow an entirely separate route from the current flow, and having two account creation experiences depending on the targeted users cc @Patrick1904 @corwinharrell @mattlockyer.

Details:

Benefits:

Note: This is specifically pre-Phase 2 (as we should combine the flows after that, allowing users to manage/upgrade/downgrade from 2FA)

Thoughts?

vgrichina commented 4 years ago

and theoretically can then request your SMS records

hmm, is it like a normal thing I can do? Like I can ask my cellular operator to give me history of my texts?

vgrichina commented 4 years ago

@kcole16 what would be the difference for 2 flows though? Currently seems like the only difference is seed phrase backup option, as for e-mail/SMS flow we in any case do message code confirmation (i.e. basically same flow as required for 2FA).

kcole16 commented 4 years ago

@vgrichina Take a look at @corwinharrell’s designs a few posts above. The 2FA/Ledger flow has several extra steps

mattlockyer commented 4 years ago

If I understand correctly, the entirety of #630 would be in it's own endpoint / flow.

We would still allow some people to create full access accounts that have no contract deployed and with mainnet Near?

kcole16 commented 4 years ago

If I understand correctly, the entirety of #630 would be in it's own endpoint / flow.

Yep, that's correct (as I'm proposing it).

We would still allow some people to create full access accounts that have no contract deployed and with mainnet Near?

Yes, as it simplifies the flow quite a bit for low value users (or rather, keeps it simple).

These users will still need lockup contracts deployed though, as no one will have unlocked NEAR pre-Phase 2. Just that they will interface with them through full access keys.

vgrichina commented 4 years ago

@kcole16 @corwinharrell where's the best place to look? Following the link in comment above (https://www.figma.com/file/tH6dad9CulATLPo36UCCyg/630-Create-Account-w-2FA?node-id=1%3A13) gives me File not found

heycorwin commented 4 years ago

@vgrichina link should be fixed. If you still can’t access let me know. I noticed your team seat invitation for Figma hadn’t been accepted so I resent it as well.

vgrichina commented 4 years ago

@corwinharrell @kcole16 do I understand correctly that type of confirmation (SMS/e-mail) will depend on which recovery method is selected? Or is the plan to actually do SMS always?

I think in general e-mail might be preferred option:

kcole16 commented 4 years ago

We've discussed offering SMS as the default, with the option to switch to email if SMS is unavailable for the user for whatever reason.

mattlockyer commented 4 years ago

I'm ok with the multisig being single TX and multiple actions because that's all the infra supports right now and probably will support for the next 6-12mo.

However, as I learn more about how rigid upgrades could be, especially since the namespace is limited, we might have some unhappy users who have to change and transfer all their funds to a new accountId.

  1. I create mattlock.near
  2. I deploy multisig
  3. new cool multisig comes out, or I want to switch wallets from web wallet to some third party
  4. what happens to mattlock.near???

As far I can tell, deploying another multisig contract on top of the one that's there is REALLY hard... lots of little details and state migration issues. I'm not expert on this enough to say.

Anyone else have thoughts about upgrading / switching wallets, multiple accounts per "human", etc...?

kcole16 commented 4 years ago

There is the potential for a user to delete their account entirely, transfer assets to a temporary account, create a new account with the original account name, then deploy the new contract and transfer the assets to it.

Definitely suboptimal, but doable. And we can likely abstract the vast majority of this.

bowenwang1996 commented 4 years ago

There is the potential for a user to delete their account entirely, transfer assets to a temporary account, create a new account with the original account name, then deploy the new contract and transfer the assets to it.

This is not completely bullet-proof as someone can grab that name in between.

kcole16 commented 4 years ago

@bowenwang1996 Couldn’t the above be done in a single batch TX?

bowenwang1996 commented 4 years ago

@kcole16 I don't think you can switch receiver in a batched transaction. cc @evgenykuzyakov

mattlockyer commented 4 years ago

Update on this issue after going over the wallet flows with @Patrick1904

Detailed flows are here: https://docs.google.com/document/d/1dUSbtaykXTZY00umNiO78Qb-PGGxA7CzWQRTl1lIbMA/edit (near account access only)

The following comment will provide some high level goals for the wallet. If we don't agree on these goals, we will end up going in circles with each person's opinion based on individual threat models.

Explanation of the "Key Points" in the document above.

  1. Recovery links always have to recover to a full access key A recover link is a full access key and can recover the account on it's own (see 3 for more on this). It's analogous to seed phrases and even recovery methods for hardware wallets. Most in our industry are comfortable with a seed phrase as their recovery option. The document details how a recovery key is used to first create a full access key on the new device and IF the user enabled 2FA, exchange that new full access key with a limited access key.

  2. The recovery method creation should be done first - without it, you can lose access to your account Accounts can be lost if the user creates an account with a linkdrop and then deletes their browser storage without first setting up a recovery method. This is pretty clear. So before the linkdrop is claimed and the account is created, a recovery option must be selected. 2FA setup is optional and comes after this.

  3. 2FA is device/browser specific - every time you recover your account, you will be asked if you want that device/browser to be 2FA enabled This seems to be a bit contentious, but it helps to separate the 2FA functionality from the account. The account does not have 2FA enabled. 2FA is the ability to add and confirm requests on the smart contract deployed to the account. The smart contract does not stop a full access key from doing any other actions. TLDR; 2FA is a pattern of contract + limited access key usage. Yes, there is the option to have a "pure" 2FA account with only limited access keys, but that should not be how we use 2FA. It requires extra case on the part of the user to preserve 2 out of 3 limited access keys at all times. e.g. I lose phone and haven't used my browser for months where local storage is deleted.

What is our goal? Our goal is protecting the user from a potentially malicious browser/extension/http/device compromise. Our goal is not to handicap their recovery options or require 2FA if they recover. By having a full access key as a recovery key (1) and allowing the user to choose 2FA each time they recover (3) we have a nice pattern that's quite secure; provided the user protects their recovery key.

Why is the recovery key a full access key? Scenario A - You lose your phone, and your browser storage is wiped out, you lost everything. Your phone and browser storage are arguably not 100% within your control at all times. Scenario B - If you lose your seed phrase, you also lose everything. We have to make a choice. We should make the choice our industry is familiar with: the seed phrase.

Why can I choose 2FA every time I recover? Recovery is simply adding a key to the current device browser storage. We (@Patrick1904) felt that this is a good opportunity to check if they still have their second device for 2FA. If they want 2FA on, they exchange the new full access key for a limited access key in browser storage; or if they want to recover and disabled 2FA, we leave them with a full access key in browser storage. They can re-enable 2FA at any time by going through the 2FA flow.

vgrichina commented 4 years ago

Recovery links always have to recover to a full access key Our goal is not to handicap their recovery options or require 2FA if they recover.

We want to require 2FA when they recover, at least for phone and e-mail, as sending recovery link there isn't exactly most secure thing. It can leak e.g. through Gmail integration into some sales tool.

Note that there is also orthogonal reason why we need to support working without any full access keys – while transfers are still limited on mainnet, most users won't ever get a hold of full access keys.

We have to make a choice. We should make the choice our industry is familiar with: the seed phrase.

We aren't building for crypto industry, though arguably seed phrase is targeted for more crypto-savvy people (vs say e-mail/phone recovery which for sure shouldn't assume any familiarity with crypto).

kcole16 commented 4 years ago
  1. If we go with DeFi focus, we will have many users with large $$$ of assets. Even the average DeFi user has > $100 worth
  2. 2FA is designed for security over everything. We of course do not want users losing keys, which is why we have a recovery key in the first place. The likelihood of losing access to SMS or email is low. OTOH, if we allow users to setup email for both, the threat model is similar to making the recovery email a FAK
  3. Seed phrases are pretty universally hated UX-wise, even by crypto vets. Argent gets continual praise for not even offering it. I’m still onboard with offering it due to its trade offs relative to other options, but our goal should be to remove it eventually. Most users will not use seed phrase, and we shouldn’t make that the most prominent option (instead email/phone).

I agree it’s worthwhile to make 2FA optional, as the ~30 N deposit makes it difficult to use for low value and newer users. That being said, it’s designed to maximize security when it is turned on, and we should do what we can to protect the user from that point on.

Note: An upside to requiring 2FA is we signal to the community we are serious about security, despite being a web wallet (regarded as insecure).

kcole16 commented 4 years ago

Reiterating the following:

  1. Our users will be attacked. This is a matter of IF not WHEN, and the first question we should ask ourselves before implementing any feature should be "How can this be abused/compromised?"
  2. Almost all wallet users in crypto store their seed phrases insecurely. Additionally, many of them will lose their seed phrase. This is the most common way users lose access to their wallets in crypto (losing their seed phrase). We should not treat seed phrases as "access to my account is secure", we should treat is as the opposite.

We should set a NEAR limit (and eventually other asset limit) above which we prompt the user to setup 2FA every single time they login to their account (no opt-out). They can choose to ignore this message, but they will have to ignore it every single time, unless they drop below the NEAR/asset threshold.

I think we should enforce 2FA on everything once it is setup. I’m mixed on making the recovery phrase a FAK, as I can see the benefits of both choices.

As we are currently the only wallet for NEAR, I propose we require 2FA on recovery for now, as security from attacks is overwhelmingly a priority. Security of access is of course equally important, but as there is a low probability of users losing access to email and/or SMS, I believe it is worth it for now.

mattlockyer commented 4 years ago

I think we should enforce 2FA on everything once it is setup. I’m mixed on making the recovery phrase a FAK, as I can see the benefits of both choices.

As we are currently the only wallet for NEAR, I propose we require 2FA on recovery for now, as security from attacks is overwhelmingly a priority. Security of access is of course equally important, but as there is a low probability of users losing access to email and/or SMS, I believe it is worth it for now.

Ok that's fair and thanks @vgrichina for your points as well. It's still possible to do everything @Patrick1904 and myself discussed, while keeping the recovery key as a LAK to multisig requests and requiring 2/3.

Summary: security over convenience / safety.

Recommendation: option to generate additional seed phrases so user can have 2/4 multisig where keys are all limited but arranged like so:

Implementing only limited access keys on a multisig contract account:

There is one scenario that's different and I'll do my best to document it here:

  1. user chooses accountId
  2. chooses email recovery
  3. email confirmed
  4. recovery key send (this is a FAK)
  5. 2FA flow initiated
  6. sms chosen
  7. sms confirmed
  8. 2 LAKs created (one for device, one for backend confirming requests only)

At this point the user's recovery key is still a FAK. We can send them a new recovery key that is a LAK, but we most likely want to remove the FAK from the account. It's either:

  1. Easy if the recovery FAK is still in JS memory because they're still signing up and a FAK can delete itself immediately
  2. We require 2FA once after enabling 2FA to accept the "DeleteKey" multisig request
  3. At any point in time when the user is using the wallet, we can warn them their account is potentially insecure by querying the keys they have and initiating a DeleteKey request for FAKs as in point (2)

Thoughts?

Patrick1904 commented 4 years ago

Some thoughts:

I agree that the risk of an attack is high and we should help prevent this, especially for users with a higher balance (>$100). The most important thing however, is that I as a user, should always be able to fully recover my account using the recovery method. There should not be any restrictions, requiring 2FA when sending etc unless I am explicitly agreeing to that. Even if the chance of losing access to ones phone is low, it will happen if we get a lot of users, especially in lower income countries where it’s common to use cheaper pre-paid phones, or simply can’t receive SMS from the US for any reason. If my recovery link does not give me full access, without restrictions to my account, then what’s the point of having it. When recovering the account, opting out of 2FA can be an option, rather than opting in. And if the balance is above X amount, we can continue to bug the user that they should either accept 2FA on the device, or setup 2FA for a different number etc if they’ve lost access.

kcole16 commented 4 years ago

@Patrick1904 You’re basically describing 2FA backup codes.

@vgrichina Let’s assume we allow a user to setup 2FA with email, and also send the recovery link to email. Do you think we can consider this any safer than a full access key recovery link? This is the biggest argument for me to make recovery links FAK, as I do agree that SMS is less reliable for users in developing nations (or even potentially prepaid users in US).

Patrick1904 commented 4 years ago

@Patrick1904 You’re basically describing 2FA backup codes.

@kcole16 one scenario would be that I lose access to my phone number and I'm not logged in on any device. In that case, I'd need a seed phrase / link, and a 2FA backup code in order to re-gain full access to my account.

If I'm a high value user, I want to know that I'm always in full control of my account. Scenarios:

  1. If only seed phrase / link is required for unlimited access, then I know that as long as I have that, I'm ok. But it also makes it easier for potential attacker since the attacker only needs my seed phrase / link to gain full access to my account.
  2. If seed phrase / link and 2FA backup code is required for unlimited access, then that needs to be clearly communicated. If I store these two things separately, I make it harder for an attacker, but increase the risk of me losing one. If I store them together, any extra security value is lost.

As a user, I want to know that there's one powerful phrase/link/whatever that I can store securely that will give me unrestricted access to my account should other methods fail for any reason.

vgrichina commented 4 years ago

@kcole16 threat model for email recovery with email 2FA: your existing emails leaked for whatever reason (e.g. you used contact import service by LinkedIn and they somehow didn’t scrub the data), but attacker doesn’t have live access to email. If you use standalone email client - your emails can be leaked from your PC as well, but saved passwords usually have better protection.

vgrichina commented 4 years ago

but we most likely want to remove the FAK from the account. It's either:

@mattlockyer I think the way to do it is to have user send a transaction through 2FA flow to remove the FAK. So that we make sure auth flow works for user. This can replace separate flow to confirm SMS.

vgrichina commented 4 years ago

especially in lower income countries where it’s common to use cheaper pre-paid phones

Cheap pre-paid phone plans are pretty popular in Ukraine, but you can still regain access to your number. I think generally it's relatively easy to regain access to phone number irregardless of contract/pre-paid (otherwise SIM cloning won't be a problem).

heycorwin commented 4 years ago

Figma link added to the issue description for updated UI 🚀 Comments and feedback welcome, It's likely I may have missed something small due to all the back and forth 🙏

mattlockyer commented 4 years ago

This doc has settled down (internal only, no new comments for 4-5 days): https://docs.google.com/document/d/1dUSbtaykXTZY00umNiO78Qb-PGGxA7CzWQRTl1lIbMA/edit#heading=h.eds6nf10m530

Here's the flow outline for wallet 2fa account creation (apologies for screen grab) documenting for historical / external partners who are developing their own wallets.

Orange highlights additional logic / UI on top of what was testnet/betanet wallet prior to July 2020.

image