From a158036deefaa1da2b17f4596ecc067df12a4b3d Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Fri, 7 May 2021 16:26:23 +0200 Subject: [PATCH 1/4] feat: allow key export in online mode 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. https://github.com/brave/brave-browser/issues/15422 --- core/commands/keystore.go | 10 ++++++---- test/sharness/t0165-keystore.sh | 9 +++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/core/commands/keystore.go b/core/commands/keystore.go index 85c6ae1a3df..8a806113c0f 100644 --- a/core/commands/keystore.go +++ b/core/commands/keystore.go @@ -12,6 +12,7 @@ import ( cmds "github.com/ipfs/go-ipfs-cmds" config "github.com/ipfs/go-ipfs-config" + keystore "github.com/ipfs/go-ipfs-keystore" oldcmds "github.com/ipfs/go-ipfs/commands" cmdenv "github.com/ipfs/go-ipfs/core/commands/cmdenv" "github.com/ipfs/go-ipfs/core/commands/e" @@ -150,7 +151,6 @@ path can be specified with '--output=' or '-o='. cmds.StringOption(outputOptionName, "o", "The path where the output should be stored."), }, NoRemote: true, - PreRun: DaemonNotRunning, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { name := req.Arguments[0] @@ -163,13 +163,15 @@ path can be specified with '--output=' or '-o='. return err } - r, err := fsrepo.Open(cfgRoot) + // Export is read-only: safe to read it without acquiring repo lock + // (this makes export work when ipfs daemon is already running) + ksp := filepath.Join(cfgRoot, "keystore") + ks, err := keystore.NewFSKeystore(ksp) if err != nil { return err } - defer r.Close() - sk, err := r.Keystore().Get(name) + sk, err := ks.Get(name) if err != nil { return fmt.Errorf("key with name '%s' doesn't exist", name) } diff --git a/test/sharness/t0165-keystore.sh b/test/sharness/t0165-keystore.sh index b3ae12fefd7..0bd8f74e69c 100755 --- a/test/sharness/t0165-keystore.sh +++ b/test/sharness/t0165-keystore.sh @@ -175,8 +175,13 @@ ipfs key rm key_ed25519 test_cmp rsa_key_id roundtrip_rsa_key_id ' - test_expect_success "online export rsa key" ' - test_must_fail ipfs key export generated_rsa_key + test_expect_success "export and import ed25519 key while daemon is running" ' + edhash=$(ipfs key gen exported_ed25519_key --type=ed25519) + echo $edhash > ed25519_key_id + ipfs key export exported_ed25519_key && + ipfs key rm exported_ed25519_key && + ipfs key import exported_ed25519_key exported_ed25519_key.key > roundtrip_ed25519_key_id && + test_cmp ed25519_key_id roundtrip_ed25519_key_id ' test_expect_success "online rotate rsa key" ' From 48bc5735fcd0477970644dcd3fbec2487816e71a Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 11 May 2021 18:07:32 +0200 Subject: [PATCH 2/4] test: confirm /api/v0/key/export is HTTP 404 This ensures key export endpoint is never exposed over HTTP API --- test/sharness/t0165-keystore.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/sharness/t0165-keystore.sh b/test/sharness/t0165-keystore.sh index 0bd8f74e69c..4a4378692f0 100755 --- a/test/sharness/t0165-keystore.sh +++ b/test/sharness/t0165-keystore.sh @@ -175,6 +175,7 @@ ipfs key rm key_ed25519 test_cmp rsa_key_id roundtrip_rsa_key_id ' + # export works directly on the keystore present in IPFS_PATH test_expect_success "export and import ed25519 key while daemon is running" ' edhash=$(ipfs key gen exported_ed25519_key --type=ed25519) echo $edhash > ed25519_key_id @@ -184,10 +185,15 @@ ipfs key rm key_ed25519 test_cmp ed25519_key_id roundtrip_ed25519_key_id ' + test_expect_success "key export over HTTP /api/v0/key/export is not possible" ' + ipfs key gen nohttpexporttest_key --type=ed25519 && + test_curl_resp_http_code "http://127.0.0.1:$API_PORT/api/v0/key/export&arg=nohttpexporttest_key" "HTTP/1.1 404 Not Found" + ' + test_expect_success "online rotate rsa key" ' test_must_fail ipfs key rotate ' - + test_kill_ipfs_daemon } From 6b63de8a5ad6b9063bd1b65a41f355543ead0511 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 11 May 2021 19:39:47 +0200 Subject: [PATCH 3/4] fix: check repo version before key export https://github.com/ipfs/go-ipfs/pull/8113#discussion_r628314443 --- core/commands/keystore.go | 10 ++++++++++ test/sharness/t0165-keystore.sh | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/core/commands/keystore.go b/core/commands/keystore.go index 8a806113c0f..56e7de18319 100644 --- a/core/commands/keystore.go +++ b/core/commands/keystore.go @@ -18,6 +18,7 @@ import ( "github.com/ipfs/go-ipfs/core/commands/e" ke "github.com/ipfs/go-ipfs/core/commands/keyencode" fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo" + migrations "github.com/ipfs/go-ipfs/repo/fsrepo/migrations" options "github.com/ipfs/interface-go-ipfs-core/options" "github.com/libp2p/go-libp2p-core/crypto" peer "github.com/libp2p/go-libp2p-core/peer" @@ -163,6 +164,15 @@ path can be specified with '--output=' or '-o='. return err } + // Check repo version, and error out if not matching + ver, err := migrations.RepoVersion(cfgRoot) + if err != nil { + return err + } + if ver > fsrepo.RepoVersion { + return fmt.Errorf("key export expects repo version (%d) but found (%d)", fsrepo.RepoVersion, ver) + } + // Export is read-only: safe to read it without acquiring repo lock // (this makes export work when ipfs daemon is already running) ksp := filepath.Join(cfgRoot, "keystore") diff --git a/test/sharness/t0165-keystore.sh b/test/sharness/t0165-keystore.sh index 4a4378692f0..ad4b6a6c7c7 100755 --- a/test/sharness/t0165-keystore.sh +++ b/test/sharness/t0165-keystore.sh @@ -187,7 +187,7 @@ ipfs key rm key_ed25519 test_expect_success "key export over HTTP /api/v0/key/export is not possible" ' ipfs key gen nohttpexporttest_key --type=ed25519 && - test_curl_resp_http_code "http://127.0.0.1:$API_PORT/api/v0/key/export&arg=nohttpexporttest_key" "HTTP/1.1 404 Not Found" + curl -X POST -sI "http://$API_ADDR/api/v0/key/export&arg=nohttpexporttest_key" | grep -q "^HTTP/1.1 404 Not Found" ' test_expect_success "online rotate rsa key" ' From 1d77f9d33d65b7948dbac79abc4776661345d6e1 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Tue, 11 May 2021 20:11:12 +0200 Subject: [PATCH 4/4] refactor: key export requires exact repo ver --- core/commands/keystore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/commands/keystore.go b/core/commands/keystore.go index 56e7de18319..bd3146ca57c 100644 --- a/core/commands/keystore.go +++ b/core/commands/keystore.go @@ -169,7 +169,7 @@ path can be specified with '--output=' or '-o='. if err != nil { return err } - if ver > fsrepo.RepoVersion { + if ver != fsrepo.RepoVersion { return fmt.Errorf("key export expects repo version (%d) but found (%d)", fsrepo.RepoVersion, ver) }