Encrypted Key Store #1078

connormullett · 2021-04-21T04:46:16Z

Summary of changes
Changes introduced in this pull request:

This PR introduces methods to encrypt and decrypt messages and payloads symmetrically. This will further be utilized in the keystore to securely save keys.

Reference issue to close (if applicable)

Closes #1033

Thank you 🔥

CLAassistant · 2021-04-21T04:46:54Z

All committers have signed the CLA.

cryptoquick · 2021-04-21T15:48:35Z

In addition to key_info I think we should have two additional properties on the MemKeyStore struct:

unlocked: bool: has it been unlocked yet
encrypted_key: bool: it's an encrypted key

@ec2 I'd like your opinions on these:

We may have to introduce checks wherever the keystore is read from to throw if an encrypted key needs to be decrypted. I'm not sure if we should have a default key that's read-only.
Should we introduce checks or requirements for encrypted keys to only work if an HTTP connection is secure?
Should we introduce a requirement for admin, sign, and write keys to require encryption?

I think 2 & 3 these are nice-to-haves, maybe I can put them in a separate issue, when we have the option for running in "production" mode vs "development" mode.

cryptoquick · 2021-04-21T16:17:31Z

Also @connormullett , check this out:

target/release/forest -c ../config.toml -p 1235
FULLNODE_API_INFO="/ip4/127.0.0.1/tcp/1235/http" lotus auth create-token --perm admin
Take the resulting admin JWT, and replace the one here:
FULLNODE_API_INFO="eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJBbGxvdyI6WyJyZWFkIiwid3JpdGUiLCJzaWduIiwiYWRtaW4iXX0.PFlC4lndH_Dw8GitzmTS6nxxGOia_kuefCWHd5FwtR0:/ip4/127.0.0.1/tcp/1235/http" lotus wallet new
Then check keystore.json
All these private keys should be capable of being encrypted

Though, should we be able to encrypt a private key that's already be written to the keystore, or should we only write freshly encrypted keys?

forest/src/daemon.rs

key_management/src/keystore.rs

* WIP RPC API for encrypted keystore unlock method. * Updates based on PR review feedback.

…e/forest into connor/1033-encrypted-key-store

key_management/src/keystore.rs

forest/src/daemon.rs

key_management/src/keystore.rs

… KDF. Replace hostname-based salt with random generated salt.

loop on incorrect passphrase

fix conflicts; refactor tests for new keystore API;

cryptoquick

I just tested it out, it works great!
Great work, @connormullett, and a fantastic first contribution!

creativcoder · 2021-04-29T05:18:05Z

key_management/src/keystore.rs

-        }
-    }
+pub enum KeyStoreConfig {
+    Memory(),


Nit: we don't need the ()

Yeah, that's fair. I just figured it looked more consistent that way.

Refactored.

creativcoder · 2021-04-29T05:18:13Z

key_management/src/keystore.rs

+                encryption: None,
+            }),
+            KeyStoreConfig::Persistent(location) => {
+                let file_path = location.join(Path::new(KEYSTORE_NAME));


We can just pass KEYSTORE_NAME to join, as it accepts anything that implements AsRef and &str does: https://doc.rust-lang.org/std/path/struct.PathBuf.html#method.join

LGTM. It does seem like if you use encrypted mode, you cant access your unencrypted keys. I wonder if we want that functionality in the future...

While the daemon is running, yes; However, the file still exists. Switching the daemon off to use another set of keys doesn't sound ideal so I agree.

creativcoder · 2021-04-29T05:18:23Z

key_management/src/keystore.rs

+    ) -> Result<Vec<u8>, EncryptedKeyStoreError> {
+        let ciphertext = &msg[..msg.len() - 24];
+
+        let nonce = match secretbox::Nonce::from_slice(&msg[msg.len() - 24..]) {


ok_or is perfect for these situations where we want to return an Err in None match arm.

This makes it much cleaner (cause ? op). Thanks for this.

Yeah, i love how std lib APIs compose well together

creativcoder · 2021-04-29T15:30:58Z

Awesome work! Let's get this merged

ec2

LGTM. It does seem like if you use encrypted mode, you cant access your unencrypted keys. I wonder if we want that functionality in the future...

connormullett added 7 commits April 20, 2021 17:50

scaffold out using encryption for key store

6de778e

generate key using hostname and passphrase with pbkd2f

6231f2d

comment the test

78a0533

more scaffolding and checking potential dependencies

a327739

generate encrypted message and append nonce as last 24 bytes

c5ea8b5

cleanup

a79eaa3

encryption/decryption works

6c6f092

assert 2x keypasses create the same key

5c8434f

cryptoquick changed the title ~~Connor/1033 encrypted key store~~ Encrypted Key Store Apr 21, 2021

cryptoquick changed the title ~~Encrypted Key Store~~ Encrypted Key Store (Encryption Methods) Apr 21, 2021

connormullett added 4 commits April 21, 2021 16:36

add is_encrypted to keyinfo

a38d659

Merge branch 'main' into connor/1033-encrypted-key-store

6610852

add optional passphrase for encrypt/decrypt

27084ce

revert for new impl

367ed8b

cryptoquick mentioned this pull request Apr 22, 2021

Encrypted Key Store (RPC API & CLI) #1080

Merged

1 task

connormullett added 3 commits April 22, 2021 14:17

add encrypt keystore config option

9454947

if encrypt_keystore enabled, block to get passphrase

08ab041

clippy cleanup

c274021

cryptoquick reviewed Apr 22, 2021

View reviewed changes

forest/src/daemon.rs Show resolved Hide resolved

cryptoquick suggested changes Apr 22, 2021

View reviewed changes

key_management/src/keystore.rs Outdated Show resolved Hide resolved

cryptoquick and others added 4 commits April 22, 2021 16:12

Encrypted Key Store (RPC API & CLI) (#1080)

c012c31

* WIP RPC API for encrypted keystore unlock method. * Updates based on PR review feedback.

use serde cbor

48e560f

Merge branch 'connor/1033-encrypted-key-store' of github.com:ChainSaf…

3fd9148

…e/forest into connor/1033-encrypted-key-store

add unlock to key stores

a14f44b

cryptoquick reviewed Apr 22, 2021

View reviewed changes

key_management/src/keystore.rs Outdated Show resolved Hide resolved

use new errors; implement hunters additions

ff1c3a3

ghost reviewed Apr 22, 2021

View reviewed changes

forest/src/daemon.rs Outdated Show resolved Hide resolved

ghost reviewed Apr 22, 2021

View reviewed changes

forest/src/daemon.rs Outdated Show resolved Hide resolved

ghost reviewed Apr 22, 2021

View reviewed changes

key_management/src/keystore.rs Outdated Show resolved Hide resolved

connormullett added 2 commits April 22, 2021 17:03

suggested changes; scaffold encrypted keystore tests

cc918c8

EOD push; working serializing encrypted keystore

1be4d9c

cryptoquick changed the title ~~Encrypted Key Store (Encryption Methods)~~ Encrypted Key Store Apr 22, 2021

connormullett and others added 8 commits April 23, 2021 00:56

read/write encrypted keystore

1387aa0

new test; incorrect passphrase panics

6191db4

fix bug with init new encrypted keystores

46cbe74

usize cannot be negative

0cd741c

Refactor KeyStore. Replace ring PBKDF2 with sodiumoxide Argon2id hash…

a4ac5a7

… KDF. Replace hostname-based salt with random generated salt.

Cleanup.

b1e31f3

Prepend encrypted bytes with encryption salt.

8dd4f58

Fix path issue. Fix linter and unit tests.

5620ac8

connormullett force-pushed the connor/1033-encrypted-key-store branch from 72c2764 to 5620ac8 Compare April 27, 2021 18:01

connormullett added 3 commits April 27, 2021 14:24

revert keystore to hunters changes (not needed)

0a0c08d

loop on incorrect passphrase

ask user to confirm password

6ad06aa

Merge branch 'main' into connor/1033-encrypted-key-store

e48d967

fix conflicts; refactor tests for new keystore API;

connormullett marked this pull request as ready for review April 28, 2021 18:51

connormullett requested a review from creativcoder as a code owner April 28, 2021 18:51

cryptoquick approved these changes Apr 28, 2021

View reviewed changes

cryptoquick requested review from a user and ec2 April 28, 2021 20:27

creativcoder reviewed Apr 29, 2021

View reviewed changes

ec2 approved these changes Apr 29, 2021

View reviewed changes

connormullett added 2 commits April 29, 2021 11:43

requested changes

bc53c81

Merge branch 'main' into connor/1033-encrypted-key-store

ee254ea

connormullett merged commit d245f14 into main Apr 29, 2021

connormullett deleted the connor/1033-encrypted-key-store branch April 29, 2021 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Encrypted Key Store #1078

Encrypted Key Store #1078

connormullett commented Apr 21, 2021

CLAassistant commented Apr 21, 2021 •

edited

cryptoquick commented Apr 21, 2021

cryptoquick commented Apr 21, 2021

cryptoquick left a comment

creativcoder Apr 29, 2021

cryptoquick Apr 29, 2021

connormullett Apr 29, 2021

creativcoder Apr 29, 2021

connormullett Apr 29, 2021

creativcoder Apr 29, 2021 •

edited

connormullett Apr 29, 2021

creativcoder Apr 29, 2021

creativcoder commented Apr 29, 2021

ec2 left a comment

Encrypted Key Store #1078

Encrypted Key Store #1078

Conversation

connormullett commented Apr 21, 2021

CLAassistant commented Apr 21, 2021 • edited

cryptoquick commented Apr 21, 2021

cryptoquick commented Apr 21, 2021

cryptoquick left a comment

Choose a reason for hiding this comment

creativcoder Apr 29, 2021

Choose a reason for hiding this comment

cryptoquick Apr 29, 2021

Choose a reason for hiding this comment

connormullett Apr 29, 2021

Choose a reason for hiding this comment

creativcoder Apr 29, 2021

Choose a reason for hiding this comment

connormullett Apr 29, 2021

Choose a reason for hiding this comment

creativcoder Apr 29, 2021 • edited

Choose a reason for hiding this comment

connormullett Apr 29, 2021

Choose a reason for hiding this comment

creativcoder Apr 29, 2021

Choose a reason for hiding this comment

creativcoder commented Apr 29, 2021

ec2 left a comment

Choose a reason for hiding this comment

CLAassistant commented Apr 21, 2021 •

edited

creativcoder Apr 29, 2021 •

edited