Cosmos-sdk: Remove all key management from the LCD

Created on 5 Jun 2018  路  16Comments  路  Source: cosmos/cosmos-sdk

I discussed this with @ebuchman today.
The LCD should not expose any keys, instead the UI should handle keys. The LCD can help to assemble a valid transaction and accepts a signed transaction to be submitted to a full-node.

The LCD should only be responsible for handling light-client verification and not also double as a signing oracle. A lot of the early Ethereum vulnerabilities came from the fact that a full-node could expose unlocked private keys to potentially any network interface.

By removing key management from the LCD we make the code cleaner and make it easier in the future to migrate the LCD to Rust or JavaScript to be used in websites. The problem is in any case that we cannot start a go process from within a website.

gaiacli should ideally connect to the LCD, and gaiacli handles the key management. We need to ensure that it is easy enough to integrate the ledger with an electron app.

@nylira @faboweb What are the effects on Voyager of this?

@cwgoes @zmanian @rigelrozanski What are your thoughts on this?

discussion security

Most helpful comment

though i understand the merits of the different arguments here - i want to weigh in by saying that a couple weeks ago, the voyager team agreed to work on LCD.js _after_ launch. most of the changes discussed here would be in line with the requirements for making LCD.js to happen - which is great.

however, making major changes like this, this close to launch probably isn't the right move. unless there are known bugs or vulnerabilities with the current implementation and unless these changes will provide tangible user value - it really seems like this is not a good time to make a change like this.

if we feel like we need to make all these changes a pre-requisite for launch, we are realistically looking at adding months of development time to our project.

All 16 comments

Strongly in favor.

I think we should target removing the light client daemon as a dependency for wallets completely. It introduces an inefficient blocking dependency of frontend code on the SDK daemon, makes writing a secure LCD harder, and will never work for any third-party in-browser wallets (or even local wallets which aren't willing to ship the gaiacli binary). A hypothetical Rust-to-WASM LCD is an interesting solution, but I think that will be way more work in the near term than just porting transaction construction, light client verification, and key management logic to common Javascript libraries (which Voyager can use) - and the latter is likely to result in easier-to-use libraries for developers in any case.

Not sure how viable this is on the Voyager end, open to brainstorming ways the SDK team might be able to help. Maybe separating gaiacli is an impractical requirement for launch right now, but I think we should aim for this in the near term.

Another point is that the TokenAPI will not leverage any of the key management functionality of the LCD anyway since it only accepts signed transactions.

ref https://github.com/cosmos/cosmos-sdk/issues/1137

Okay so what happens if I don't want to use HSM to sign messages? Would the key management occur in Voyager? If so, I think this is a great idea - it introduces a bit of code dup - but as long as we're explicit enough in our keys-spec then this shouldn't be a problem :)

Isn't our ledger integration via LCD tho?

I agree that key management does not belong in the LCD.

Jessy posted an excellent presentation by Google in #security that talks about the growing importance of building stronger isolation into our system components: https://docs.google.com/presentation/d/17bKudNDduvN-7hWv7S84MiHUj2AnOPNbwjTM8euDC8w/edit#slide=id.p1

For the same reason, key management doesn't belong in the UI either. This is, in fact, the whole point of an HSM: It's isolated from the rest of the system.

If we're going to do key management in software then we should create a software version of an HSM, i.e., single-purpose.

@jaekwon already argued this point well in https://github.com/cosmos/cosmos-sdk/issues/324#issuecomment-358892297.

would voyager be responsible for writing txs or just signing them? would this be a sec issue if/when voyager moved to the browser?

Okay cool - what we need is a detailed spec for ckeystore described in #324 - ultimately I see there being 3 implementations:

  • HSM
  • cli (in golang)
  • UI, seperate from voyager, but hopefully very closely linked, I'd imagine maybe a popup or something, however the application would still be separate and easily audit-able

CC: @faboweb

though i understand the merits of the different arguments here - i want to weigh in by saying that a couple weeks ago, the voyager team agreed to work on LCD.js _after_ launch. most of the changes discussed here would be in line with the requirements for making LCD.js to happen - which is great.

however, making major changes like this, this close to launch probably isn't the right move. unless there are known bugs or vulnerabilities with the current implementation and unless these changes will provide tangible user value - it really seems like this is not a good time to make a change like this.

if we feel like we need to make all these changes a pre-requisite for launch, we are realistically looking at adding months of development time to our project.

most of the changes discussed here would be in line with the requirements for making LCD.js to happen

This proposal would not require LCD.js. The LCD would still do all the querying and proofs and could still actually create transactions.

All we're talking about here is moving the signing itself elsewhere.

Isn't our ledger integration via LCD tho?

Currently yes, its the same code used by gaiacli. We certainly still want the command line tools to be able to do signing, eg for offline signing too.

Basically this proposal would require adding support in voyager for creating keys (either in the JS or by connecting to an external HSM from the JS) and signing txs with them (again, either in the JS or by connecting to an external HSM from the JS).

would voyager be responsible for writing txs or just signing them? would this be a sec issue if/when voyager moved to the browser?

For this proposal, just signing them. The LCD can still create the txs

If we're going to do key management in software then we should create a software version of an HSM, i.e., single-purpose.

I think part of the rationale here is to not have a separate process for key-mgmt that binds a socket because of the inevitable issues of people accidentally not restricting access to that socket. So having a separate go process just for signing doesn't really address the issue, even though it does help on the separation of concerns front.

I think part of the rationale here is to not have a separate process for key-mgmt that binds a socket because of the inevitable issues of people accidentally not restricting access to that socket. So having a separate go process just for signing doesn't really address the issue, even though it does help on the separation of concerns front.

Does this concern apply equally well to using an HSM?

Does this concern apply equally well to using an HSM?

No, the ideal is for everyone to use an HSM. If folks are using an HSM, I think it matters much less if it's managed from the LCD or from Voyager or a third process. Third process would be great from separation of concerns front, and wouldn't have the other problems associated with it re binding sockets since the user authorization is required on the HSM regardless of who can talk to the process,

The problem as I see it is really only when the keys are not on a separate machine.

I see.

[it] wouldn't have the other problems associated with it re binding sockets since the user authorization is required on the HSM regardless of who can talk to the process ...

This could be true for a software HSM as well if it has its own UI as proposed by @rigelrozanski.

The proposal is to slowly migrate to not create unexpected work prelaunch. We would implement the /txs/broadcast endpoint for general purpose tx transmission and endpoints to build transactions as planned. Then later we remove the build -> sign -> send endpoints in favor of extracting the KMS for separation of concerns in cost of more implementation needed in integrating applications.

ICS1 (KeyAPI) will implement the key management. It is an optional feature of the LCD and clients can choose to use it or not. No other APIs depend on ICS1.

ICS1 will support HMS and plain-text keys.

I'll close this in favour of #1314 .

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ValarDragon picture ValarDragon  路  3Comments

rigelrozanski picture rigelrozanski  路  3Comments

fedekunze picture fedekunze  路  3Comments

hendrikhofstadt picture hendrikhofstadt  路  3Comments

rigelrozanski picture rigelrozanski  路  3Comments