From f39fa611dc9d972a119cbee1a772c0064cfddd30 Mon Sep 17 00:00:00 2001
From: Lucas Molas <schomatis@gmail.com>
Date: Thu, 10 Feb 2022 11:25:37 -0300
Subject: [PATCH] fix(cmds/keystore): do not allow to import keys we don't
 generate

---
 core/commands/keystore.go                      | 16 ++++++++++++++++
 test/sharness/t0165-keystore-data/README.md    | 18 ++++++++++++++++++
 .../t0165-keystore-data/openssl_secp384r1.pem  |  6 ++++++
 test/sharness/t0165-keystore.sh                | 12 ++++++++++++
 4 files changed, 52 insertions(+)
 create mode 100644 test/sharness/t0165-keystore-data/openssl_secp384r1.pem

diff --git a/core/commands/keystore.go b/core/commands/keystore.go
index 11f62fe5046..5596ceee03a 100644
--- a/core/commands/keystore.go
+++ b/core/commands/keystore.go
@@ -143,6 +143,7 @@ const (
 	keyFormatOptionName            = "format"
 	keyFormatPemCleartextOption    = "pem-pkcs8-cleartext"
 	keyFormatLibp2pCleartextOption = "libp2p-protobuf-cleartext"
+	keyAllowAnyTypeOptionName      = "allow-any-key-type"
 )
 
 var keyExportCmd = &cmds.Command{
@@ -319,6 +320,7 @@ The PEM format allows for key generation outside of the IPFS node:
 	Options: []cmds.Option{
 		ke.OptionIPNSBase,
 		cmds.StringOption(keyFormatOptionName, "f", "The format of the private key to import, libp2p-protobuf-cleartext or pem-pkcs8-cleartext.").WithDefault(keyFormatLibp2pCleartextOption),
+		cmds.BoolOption(keyAllowAnyTypeOptionName, "Allow importing any key type.").WithDefault(false),
 	},
 	Arguments: []cmds.Argument{
 		cmds.StringArg("name", true, false, "name to associate with key in keychain"),
@@ -391,6 +393,20 @@ The PEM format allows for key generation outside of the IPFS node:
 			return fmt.Errorf("unrecognized import format: %s", importFormat)
 		}
 
+		// We only allow importing keys of the same type we generate (see list in
+		// https://github.com/ipfs/interface-go-ipfs-core/blob/1c3d8fc/options/key.go#L58-L60),
+		// unless explicitly stated by the user.
+		allowAnyKeyType, _ := req.Options[keyAllowAnyTypeOptionName].(bool)
+		if !allowAnyKeyType {
+			switch t := sk.(type) {
+			case *crypto.RsaPrivateKey, *crypto.Ed25519PrivateKey:
+			default:
+				return fmt.Errorf("key type %T is not allowed to be imported, only RSA or Ed25519;"+
+					" use flag --%s if you are sure of what you're doing",
+					t, keyAllowAnyTypeOptionName)
+			}
+		}
+
 		cfgRoot, err := cmdenv.GetConfigRoot(env)
 		if err != nil {
 			return err
diff --git a/test/sharness/t0165-keystore-data/README.md b/test/sharness/t0165-keystore-data/README.md
index 33c77fbd376..4c0a68b5169 100644
--- a/test/sharness/t0165-keystore-data/README.md
+++ b/test/sharness/t0165-keystore-data/README.md
@@ -6,3 +6,21 @@ Created with commands:
 openssl genpkey -algorithm ED25519 > openssl_ed25519.pem
 openssl genpkey -algorithm RSA -pkeyopt rsa_keygen_bits:2048 > openssl_rsa.pem
 ```
+
+secp key used in the 'restrict import key' test.
+From: https://www.openssl.org/docs/man1.1.1/man1/openssl-genpkey.html
+```bash
+openssl genpkey -genparam -algorithm EC -out ecp.pem \
+        -pkeyopt ec_paramgen_curve:secp384r1 \
+        -pkeyopt ec_param_enc:named_curve
+openssl genpkey -paramfile ecp.pem -out openssl_secp384r1.pem
+rm ecp.pem
+```
+Note: The Bitcoin `secp256k1` curve which is what `go-libp2p-core/crypto`
+actually generates and would be of interest to test against is not
+recognized by the Go library:
+```
+Error: parsing PKCS8 format: x509: failed to parse EC private key embedded
+ in PKCS#8: x509: unknown elliptic curve
+```
+We keep the `secp384r1` type instead from the original openssl example.
diff --git a/test/sharness/t0165-keystore-data/openssl_secp384r1.pem b/test/sharness/t0165-keystore-data/openssl_secp384r1.pem
new file mode 100644
index 00000000000..d9000993526
--- /dev/null
+++ b/test/sharness/t0165-keystore-data/openssl_secp384r1.pem
@@ -0,0 +1,6 @@
+-----BEGIN PRIVATE KEY-----
+MIG2AgEAMBAGByqGSM49AgEGBSuBBAAiBIGeMIGbAgEBBDC8DksLZwPKGQS8tuWI
+w+dNYiSHUyw30NkrK9YGmjgp84sVVa5NGrv0QniAnNWG1DqhZANiAATq0d5KV1MF
+IIpF4beNX+YsmFdqB2oDLhznO/4xNsFCFKE39oGQmuMwnQhDNZZ2CQA8csfgZmuF
+OSooe/Ru6ubyeGVKafcHJrOMBvl8hIg0tVWgIAhuXiTHq0UL0QTv9Vk=
+-----END PRIVATE KEY-----
diff --git a/test/sharness/t0165-keystore.sh b/test/sharness/t0165-keystore.sh
index 4729a7e2e20..60089ecd7f2 100755
--- a/test/sharness/t0165-keystore.sh
+++ b/test/sharness/t0165-keystore.sh
@@ -74,6 +74,18 @@ ipfs key rm key_ed25519
 
   test_openssl_compatibility_all_types
 
+  INVALID_KEY=../t0165-keystore-data/openssl_secp384r1.pem
+  test_expect_success "import key type we don't generate fails" '
+    test_must_fail ipfs key import restricted-type -f pem-pkcs8-cleartext $INVALID_KEY 2>&1 | tee key_exp_out &&
+    grep -q "Error: key type \*crypto.ECDSAPrivateKey is not allowed to be imported" key_exp_out &&
+    rm key_exp_out
+  '
+
+  test_expect_success "import key type we don't generate succeeds with flag" '
+    ipfs key import restricted-type --allow-any-key-type -f pem-pkcs8-cleartext $INVALID_KEY  > /dev/null  &&
+    ipfs key rm restricted-type
+  '
+
   test_expect_success "test export file option" '
     ipfs key export generated_rsa_key -o=named_rsa_export_file &&
     test_cmp generated_rsa_key.key named_rsa_export_file &&