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

[4] Can we create a QR code for a private key? #444

Closed
xiangxn opened this issue Sep 24, 2017 · 29 comments
Closed

[4] Can we create a QR code for a private key? #444

xiangxn opened this issue Sep 24, 2017 · 29 comments
Assignees
Labels
[3] Feature Classification indicating the addition of novel functionality to the design [4c] High Priority Priority indicating significant impact to system/user -OR- workaround is prohibitivly expensive
Milestone

Comments

@xiangxn
Copy link
Contributor

xiangxn commented Sep 24, 2017

Can we create a QR code for a private key?
This makes it easy for third parties to read the import private key.
I can finish the job within a business day.

@xiangxn xiangxn changed the title Can we create a two-dimensional code for a private key? Can we create a QR code for a private key? Sep 24, 2017
@abitmore
Copy link
Member

Sounds like something like steemit/condenser#1364 .

For private keys, I think it's better to NOT show the QR code of private key directly, instead, we can define a workflow, for example:

  • user click "show QR code" button
  • the UI generate a random 6-digit number, shows to the user; or let the user input a number
  • the UI encrypt the private key with this number, generate QR code of the encrypted key
  • user scan the QR code with 3rd-party app
  • 3rd-party app get the encrypted key from the QR code
  • 3rd-party app asks for the random number
  • user input the number
  • 3rd-party app decrypt the key with the number, get the real private key

@abitmore
Copy link
Member

Hmm, just found it's the workflow adopted by YOYOW (https://wallet.yoyow.org/). This issue is opened by the developer.

@xiangxn
Copy link
Contributor Author

xiangxn commented Sep 24, 2017

The QR code is optional encryption, if the password is empty is not encrypt.

@wmbutler
Copy link
Contributor

This will be part of the UI rewrite as well. Fairly easy fix that needs UX.

@wmbutler wmbutler added this to the 171015 milestone Sep 24, 2017
@wmbutler wmbutler self-assigned this Sep 24, 2017
@wmbutler wmbutler added the [3] Feature Classification indicating the addition of novel functionality to the design label Sep 24, 2017
@xiangxn
Copy link
Contributor Author

xiangxn commented Sep 25, 2017

There are several third party developers in China who have a strong demand, so I have implemented the code. Of course, you can also quickly achieve.

@abitmore
Copy link
Member

@wmbutler IMHO this is high priority. AFAIK there is at least one mobile wallet project waiting for this feature.
@xiangxn if you have the code already, perhaps just create a pull request.

@wmbutler
Copy link
Contributor

wmbutler commented Oct 1, 2017

OK, I'll get this on the Sprint for 171015. I will UX it as well.

@wmbutler wmbutler added the [4c] High Priority Priority indicating significant impact to system/user -OR- workaround is prohibitivly expensive label Oct 1, 2017
@wmbutler
Copy link
Contributor

wmbutler commented Oct 2, 2017

I feel like this is being made more difficult than it actually is... Why can't we import a QR code library to handle this? We can display the QR code when the user decides to view their private key in each of owner/active/memo.

screen shot 2017-10-02 at 4 48 35 pm

@wmbutler wmbutler changed the title Can we create a QR code for a private key? [3] Can we create a QR code for a private key? Oct 2, 2017
@wmbutler
Copy link
Contributor

wmbutler commented Oct 2, 2017

@xiangxn would like to claim this issue? If so, let me know if you think this UX is a reasonable path. I assigned 3 hours to it. Let me know if you think it should be more or less.

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 3, 2017

Private key QR-code should not be display directly, and should be opened each time with a different password encryption, so I think it should be at least 4 hours, and can not use the above UX.

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 3, 2017

@wmbutler If this UX, 3 hours of time is possible.

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 3, 2017

@wmbutler
Copy link
Contributor

wmbutler commented Oct 3, 2017

I'm not the dev, so that doesn't explain anything meaningful to me. Submit a UX before you begin work. Upgraded to 4 hours but won't assign without knowing how this is going to look first.

@wmbutler wmbutler changed the title [3] Can we create a QR code for a private key? [4] Can we create a QR code for a private key? Oct 3, 2017
@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 4, 2017

@wmbutler Do you think this is okay?
1
2
3

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 4, 2017

@wmbutler If you decide to use your UX, I can finish it in 3 hours.

@wmbutler
Copy link
Contributor

wmbutler commented Oct 4, 2017

I don't understand the purpose of an encrypted QR code. @svk31 can you shed some light on this? Seems to me that we can just display to qr code that represents the users' private key.

@svk31
Copy link
Contributor

svk31 commented Oct 4, 2017

To be honest I don't really see the point either, unless you want to print and store that qr code, in which case it makes sense to encrypt it as long as you're confident you remember the password.

For the proposed use case of transferring a private key to an app, I would assume you would do so in the privacy of your home or somewhere that doesn't require you to encrypt the qr code, but maybe that's just me.

@abit care to comment on why it's necessary?

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 5, 2017

@wmbutler @svk31 Encryption is not for storage, but to prevent accidental storage.
Most users may print and store it.
The purpose of encryption is to, in the import process, the private key will not leak.

@wmbutler
Copy link
Contributor

wmbutler commented Oct 5, 2017

@xiangxn something is getting lost in translation. I don't understand your reasoning. The text value of the private key is already being exposed. The QR code is just a machine-readable version of that same piece of information.

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 5, 2017

@wmbutler
We just want to provide users with a qr code that can choose encryption to meet the needs of different users.
Because we do not guarantee that the user does not store qr code, it is also intended to improve the security awareness of the user to protect his private key.

@wmbutler
Copy link
Contributor

wmbutler commented Oct 5, 2017

I see that you are suggesting that the encryption aspect is optional. I'd suggest re UXing this in a way that allows for the unencrypted QR code by default with a checkbox that a more savvy user might choose to encrypt the QR code.

This really requires more than one screenshot to fully communicate. Would you like for me to take a shot at it now that I understand the encryption is optional? Other questions this brings to mind:

  • What encryption algorithm are you proposing?
  • Why are you choosing that one in particular?
  • How can a user on the other end be sure they are using the same encryption method you have chosen?

This baffles me because you are exposing your private key with the purpose of moving it to another device. What is the use case?

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 5, 2017

I use the AES encryption algorithm in bitsharesjs, and third-party developers can use bitsharesjs libraries, or they can implement AES decryption algorithm on their own.

We can force users to use encrypted qr codes for the purpose of reducing the possibility of private key leaks.

Because it is convenient to import the qr code, some users will choose to save the qr code, but the plaintext qr code is quite insecure, and any device can be read directly, so I provide encryption function.

@wmbutler
Copy link
Contributor

wmbutler commented Oct 5, 2017

ok, thanks for the explanation. I'll spend some time working out a UX for this. I think I understand all the requirements now.

@abitmore
Copy link
Member

abitmore commented Oct 5, 2017

I'd suggest re UXing this in a way that allows for the unencrypted QR code by default with a checkbox that a more savvy user might choose to encrypt the QR code.

@wmbutler: encrypted QR code should be default. By reading above conversation, I think you've got it. The code/password is to prevent scenarios that users accidentally stored the QR code (image) to her phone, or the image is cached by browser or another app, or synced to the cloud, or MITM, etc, lots of ways to be leaked if it's plain text. Actually I'd recommend NOT have the option to set an empty code/password.

Image that your new WIFI router has a default password; when pairing new blue-tooth devices the user will be prompted to input a short code, etc. It's not terrible UX.

@abitmore
Copy link
Member

abitmore commented Oct 5, 2017

@xiangxn I'd recommend NOT show the text format by default, which is plain text, thus insecure.

image

Do something like this:

image

@wmbutler
Copy link
Contributor

wmbutler commented Oct 5, 2017

The data over a wss:// socket is encrypted regardless of what's going on with WIFI. The raw private key is in the browser data whether it's being visibly displayed or not.

@abitmore
Copy link
Member

abitmore commented Oct 5, 2017

Ok, perhaps my example aren't so suitable here. I just mean we can use a simple code in this feature. Not meant to discuss how data is transferred via WIFI.

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 5, 2017

The text of the qr code is not a default display, and the password of the wallet will be entered before the display. A hide button will be displayed, and a view button will be encrypt the qr code, as shown in the figure above.

@xiangxn
Copy link
Contributor Author

xiangxn commented Oct 18, 2017

@wmbutler this is my BTS account: necklace

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[3] Feature Classification indicating the addition of novel functionality to the design [4c] High Priority Priority indicating significant impact to system/user -OR- workaround is prohibitivly expensive
Projects
None yet
Development

No branches or pull requests

4 participants