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

Secure /keys #864

Closed
jaekwon opened this issue Apr 15, 2018 · 5 comments
Closed

Secure /keys #864

jaekwon opened this issue Apr 15, 2018 · 5 comments

Comments

@jaekwon
Copy link
Contributor

jaekwon commented Apr 15, 2018

Right now /keys just passes passwords around like it wants to get hacked.
We must make sure that the default behavior of the Cosmos-SDK server doesn't allow for such unsafe operations. We'll need to discuss how to properly handshake between the server and client (e.g. TLS or secret_connection or other). We discussed this a bit during SDK standup meetings a month ago.

@jaekwon jaekwon added the T:Bug label Apr 15, 2018
@martindale martindale added this to the 1.0 Code Freeze milestone Apr 24, 2018
@cwgoes cwgoes modified the milestone: 1.0 Code Freeze Apr 27, 2018
@ValarDragon
Copy link
Contributor

Should we punt this to postlaunch. Its not clear to me how to implement this / what concretely is being proposed. Whatever we do should be fully backwards compatible, or could be upgraded in a backwards-compatible manner. (Since we are using a custom file decryption scheme, we have to know the key used in the file at some point, granted the key comes out of bcrypt, so if the OS keychain supports bcrypt than we could have it come from there)

Sends aren't going to be enabled at launch, so this gives us plenty of time to improve gaiacli keys. I think #617 is a more viable first step in the right direction, along with a keybase refactor, so that the passphrase never leaves the crypto packages.

The exploit here requires full memory access (within a single process!), so it will be quite costly to exploit.

@alexanderbez
Copy link
Contributor

I think #617 is a more viable first step in the right direction, along with a keybase refactor, so that the passphrase never leaves the crypto packages.

++

@ValarDragon not sure if you have an open issue for the keybase refactor? @jaekwon and I briefly spoke about it and thought it might not be worth the time overhead #prelaunch. Is this something you foresee #postlaunch? Mind jotting down some of your thoughts for the refactor and what is needed?

@ValarDragon
Copy link
Contributor

Something actionable would be to ensure we clear the memory, though I'm unsure if golang provides a robust way to do that.

@alessio
Copy link
Contributor

alessio commented Sep 22, 2018

FTR LCD REST server now serves requests over HTTPS by default, see #595.

@jackzampolin
Copy link
Member

This is decidedly #postlaunch, but we have address the transport sec issue. Further security updates will require a much broader discussion about architecture. I don't think this is the right forum for that. Going to close this issue as addressed, if there are additional concerns to come out of this discussion that were unaddressed please open another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants