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

Implement private key / hardware wallet signing protocol for Electron #921

Closed
wbobeirne opened this issue Jan 25, 2018 · 3 comments
Closed
Labels
type: discussion Items that are primarily a discussion around a feature or issue. May evolve to be actionable.

Comments

@wbobeirne
Copy link
Contributor

wbobeirne commented Jan 25, 2018

The Problem

The current Electron build can't unlock Ledger wallets because of the lack of U2F support (Digital Bitbox would also not work, once that integration is in.) It also doesn't do anything to improve security for generated wallets. While we could do some in-app node integration, this would open us up to a bunch of potential security holes, should someone get access to the Javascript in the Electron window context.

The Proposal

We come up with a protocol that allows for asynchronous private key actions, like generation, transaction signing, message signing, all that good stuff. This would not be dissimilar to Metamask or hardware wallets, where the private key is never externally communicated.

Using an Electron custom protocol, we should implement a few methods that get communicated to via mycrypto:// urls or some such. When you want to do anything that requires a private key, you'll shoot a POST request to something like mycrypto://sign-tx, and it'll open up a separate Electron window that allows the user to enter a password / unlock their hardware wallet, verify the transaction data, and send back a signed transaction to the request.

Both Ledger and Digitial Bitbox both have HID implementations that could work with node-hid. So we can go directly to the devices without the U2F middleman in the Electron node environment.

What We'd Need To Do

  1. Come up with a list of wallets that will move out of web context, and into this Electron protocol
    • Private keys
    • Mnemonic phrases
    • Keystore files
    • Ledger
    • Digital Bitbox
  2. Draft up what methods the protocol will need, what the API looks like
  3. Figure out a way to conditionally use the protocol in Electron, but use the direct method for web
  4. Put together a new electron-side wallet-agnostic unlock and transaction verification UI
    • Different wallets may need different unlock flows, but transaction verification can be uniform
  5. Potentially do another security audit on the new protocol once it's complete

Credit where credit is due: This idea was inspired by Aragon's blog post about their Electron app + integrating MetaMask.

@wbobeirne wbobeirne added the type: discussion Items that are primarily a discussion around a feature or issue. May evolve to be actionable. label Jan 25, 2018
@wbobeirne wbobeirne changed the title Implement custom private key signing protocol for Electron Implement private key / hardware wallet signing protocol for Electron Jan 25, 2018
@wbobeirne
Copy link
Contributor Author

One other thought I had: Coming up with a standard protocol would also allow us to share all business logic involving signing and key generation with a native app (React Native or otherwise) that also implements the protocol as well.

@wbobeirne
Copy link
Contributor Author

I sketched out what this protocol might look like in this gist, which is a mockup readme for a project that adds an Electron protocol: https://gist.github.com/wbobeirne/88a4483525261d27c3b38ea1b598d011

The idea here is that the protocol, and the UI for interacting with wallets would be separate. This gist only covers the protocol, but there'd be a separate module than handles the UI.

Let me know what y'all think.

@dternyak
Copy link
Contributor

dternyak commented Apr 2, 2018

I would like to move forward with this as we get closer to Electron release. You've done a great job of documenting the interface and explaining the feature @wbobeirne, and I definitely agree it adds tremendous value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: discussion Items that are primarily a discussion around a feature or issue. May evolve to be actionable.
Projects
None yet
Development

No branches or pull requests

3 participants