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

fix(cmds/keystore): do not allow to import keys we don't generate #8733

Merged
merged 1 commit into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions core/commands/keystore.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ const (
keyFormatOptionName = "format"
keyFormatPemCleartextOption = "pem-pkcs8-cleartext"
keyFormatLibp2pCleartextOption = "libp2p-protobuf-cleartext"
keyAllowAnyTypeOptionName = "allow-any-key-type"
)

var keyExportCmd = &cmds.Command{
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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
Expand Down
18 changes: 18 additions & 0 deletions test/sharness/t0165-keystore-data/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
schomatis marked this conversation as resolved.
Show resolved Hide resolved
```
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.
6 changes: 6 additions & 0 deletions test/sharness/t0165-keystore-data/openssl_secp384r1.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
-----BEGIN PRIVATE KEY-----
MIG2AgEAMBAGByqGSM49AgEGBSuBBAAiBIGeMIGbAgEBBDC8DksLZwPKGQS8tuWI
w+dNYiSHUyw30NkrK9YGmjgp84sVVa5NGrv0QniAnNWG1DqhZANiAATq0d5KV1MF
IIpF4beNX+YsmFdqB2oDLhznO/4xNsFCFKE39oGQmuMwnQhDNZZ2CQA8csfgZmuF
OSooe/Ru6ubyeGVKafcHJrOMBvl8hIg0tVWgIAhuXiTHq0UL0QTv9Vk=
-----END PRIVATE KEY-----
12 changes: 12 additions & 0 deletions test/sharness/t0165-keystore.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down