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

JS Keystore #1339

Closed
5 tasks
faboweb opened this issue Sep 20, 2018 · 16 comments
Closed
5 tasks

JS Keystore #1339

faboweb opened this issue Sep 20, 2018 · 16 comments
Labels
epic low priority has been discussed, will be addressed later security 🛡️ split 🍌 issues that need to be split into several smaller issues with reduced scope

Comments

@faboweb
Copy link
Collaborator

faboweb commented Sep 20, 2018

We currently rely on the key store integrated in the gaia binary. This is troublesome as we i.e. need to wait until the REST server is up until we can sign in the user. This also blocks us from moving Voyager to the browser.

The key store would need to be able to:

  • create new key pairs that are compatible with the SDK
  • create seeds that are compatible with the SDK
  • import key pairs from seed phrases generated by the SDK
  • create signatures from bytes
  • store the keys safely from the browser
@faboweb faboweb added low priority has been discussed, will be addressed later security 🛡️ split 🍌 issues that need to be split into several smaller issues with reduced scope epic voyager-web labels Sep 20, 2018
This was referenced Sep 20, 2018
@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 1, 2018

Should we use WalletConnect for this?

I don't think the browser is secure enough to act as a key store.

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 2, 2018

I'd be down for pioneering in WalletConnect.

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 2, 2018

WalletConnect doesn't have Ledger support as far as I see. :/

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 2, 2018

So I propose having Ledger support via https://github.com/ZondaX/ledger-cosmos-js in the browser and using walletconnect for storing keys on mobile. Still I think for onboarding users and for smaller interactions with the chain a js keystore might be useful.

@jbibla
Copy link
Collaborator

jbibla commented Oct 2, 2018

i like how manager.balance.io handles this question. WalletConnect is more accessible - you don't have to buy a ledger.

screen shot 2018-10-02 at 8 36 58 am

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 2, 2018

Let's copy them :)

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 2, 2018

A mobile phone is not that secure as a Ledger though.

@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 2, 2018

The number of Ledger owners is a tiny minority compared to cell phone owners. We'd be designing for the 1% case instead of the 80% case.

There's an argument to be made that a key store made with open source software on a well-designed smart phone (such as iPhone & Android) is more secure than a Ledger. The answer is not obvious.

Still I think for onboarding users and for smaller interactions with the chain a js keystore might be useful.

This is problematic. Every time a user reloads the site, they'll get new code from the server. This means the server is trusted. Unfortunately, while we can sign the code, it's impractical for us to control the server and there's no way for the browser to verify our code signature. This means that any future server compromise could compromise all user wallets without warning. I think this is leading our users into too much risk.

See https://security.stackexchange.com/questions/173620/what-s-wrong-with-in-browser-cryptography-in-2017 for some thoughts on the subject.

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 3, 2018

The number of Ledger owners is a tiny minority compared to cell phone owners. We'd be designing for the 1% case instead of the 80% case.

nice argument!

This is problematic. Every time a user reloads the site, they'll get new code from the server. This means the server is trusted. Unfortunately, while we can sign the code, it's impractical for us to control the server and there's no way for the browser to verify our code signature. This means that any future server compromise could compromise all user wallets without warning. I think this is leading our users into too much risk.

wouldn't we control a chrome plugin? would unsafe code be able to attack our chrome plugin code?

@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 3, 2018

Oh, if you want to do it as a browser plugin then I'm happy. I thought you wanted to do it in JavaScript.

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 4, 2018

A "software wallet" as an extension is one solution. Maybe there are others. Any ideas?

@jbibla
Copy link
Collaborator

jbibla commented Oct 4, 2018

seems like taking the same approach as balance.io makes sense. however, there is no metamask for cosmos yet (we can build it) or speak with them about including cosmos.

next would be wallet connect in my books - will serve more people and is a better ux than ledger.

after that we can use ledger.

i would also propose that we connect with @mappum to see if he has any shorter term plans to build a solution for this problem as he has expressed a desire to do so in the past.

is this a satisfactory plan?

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 4, 2018

Great sumup @jbibla

@faboweb
Copy link
Collaborator Author

faboweb commented Oct 12, 2018

How about we create a QR-Code for a transaction containing the transaction and the endpoint where to send the signed transaction to? A companion app just adds the signature and posts it to the endpoint. Question: How to trigger an update of the UI? Or is this even necessary?

Note: this is common pattern in some other desktop wallets

An MVP of this could be build in less then a week.

@NodeGuy
Copy link
Contributor

NodeGuy commented Oct 12, 2018

Question: How to trigger an update of the UI? Or is this even necessary?

WebSocket and yes, I think it's necessary.

@faboweb
Copy link
Collaborator Author

faboweb commented Jan 15, 2019

Done

@faboweb faboweb closed this as completed Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic low priority has been discussed, will be addressed later security 🛡️ split 🍌 issues that need to be split into several smaller issues with reduced scope
Projects
None yet
Development

No branches or pull requests

3 participants