feat: allow key export in online mode #8113
Conversation
Export does not require repo lock and it is safe to do even when ipfs daemon is running. This enables apps like Brave browser to do import/export without stopping/starting daemon. Ref. brave/brave-browser#15422
| @@ -150,7 +151,6 @@ path can be specified with '--output=<path>' or '-o=<path>'. | |||
| cmds.StringOption(outputOptionName, "o", "The path where the output should be stored."), | |||
| }, | |||
| NoRemote: true, | |||
| PreRun: DaemonNotRunning, | |||
There was a problem hiding this comment.
Removing this check is probably fine, but mostly as a function of the keystore currently living on the filesystem and hoping that Go + the OS properly obey filesystem semantics cross platform. If we try and move keystore into some database this could start to fall apart since the database might require a single access point.
As for the keeping the keys on the filesystem, I'm not really a fan, especially since they're currently unencrypted (although that's fairly fixable), so even though it's low on the priority list it'd be a nice thing to deal with. This is especially so because we have no way of revoking IPNS keys (although setting the sequence number to the max value might do the job) or pushing people onto new keys if the old ones are compromised. Thus far IPNS has not had super high usage due to performance issues but as those resolve (IPNS over PubSub, faster DHT lookups, etc.) the idea of protecting the keys better may come into focus.
Are we ok either boxing ourselves into using a keystore that is always readable by multiple processes, or with having online key export potentially not be available in the future?
cc @Stebalien @lidel
There was a problem hiding this comment.
So, I think we should be able to do this without boxing ourselves into a corner. We'd probably need something in the future that:
- Reads the config to find where the keys are actually stored.
- Possibly connects to the system keyring.
For now, the current code is likely fine. In the future, we'll likely need something to open a keystore without opening the rest of the repo (i.e., read the config to find where the keystore is, then read the keystore). Some keystores may not support this, but that's fine.
There was a problem hiding this comment.
Yep, it'll be a bit more complicated but it seems doable enough for us to go ahead with this now. Some things we may have to worry about are:
- making sure to control how the config fine is modified if it has keystone info in there
- checking the keystone type and for some of them returning something like "error cannot export keys from this keystone while the daemon is running"
There was a problem hiding this comment.
Yep. That means checking the repo version is going to be important.
There was a problem hiding this comment.
Added version check in 6b63de8
This ensures key export endpoint is never exposed over HTTP API
|
Added more tests and repo version check. |
This PR enables apps like Brave browser to do import/export without stopping/starting daemon.
Export does not require repo lock, and afaik it is safe to do even when ipfs daemon is running.
Ref. brave/brave-browser#15422 (cc @spylogsster)