Add pubkey importing / seed generation #172

mriise · 2023-06-28T03:34:35Z

Description

Add support for importing ed25519 keypairs and generating a new keypair from seed bytes.

Link to issue

#163

Type of change

New feature (non-breaking change that adds functionality)
This change requires a documentation update

Test plan (required)

TODO

Demonstrate the code is solid. Which commands did you test with and what are the expected results?
Which tests have you added or updated? Do the tests cover all of the changes included in this PR?

Screenshots/Screencaps

Please add previews of any UI Changes.

zeeshanlakhani

One comment thus far, but awesome to see this @mriise! We should also have this PR use the setting upon swarm startup, right?

homestar-runtime/src/settings.rs

codecov · 2023-06-28T03:46:40Z

Codecov Report

Merging #172 (b9eb51f) into main (af9b1d1) will decrease coverage by 0.35%.
The diff coverage is 29.54%.

@@            Coverage Diff             @@
##             main     #172      +/-   ##
==========================================
- Coverage   47.97%   47.62%   -0.35%     
==========================================
  Files          50       50              
  Lines        2193     2236      +43     
  Branches      539      557      +18     
==========================================
+ Hits         1052     1065      +13     
- Misses        817      834      +17     
- Partials      324      337      +13

Impacted Files	Coverage Δ
homestar-runtime/src/network/swarm.rs	`0.00% <0.00%> (ø)`
homestar-runtime/src/settings.rs	`44.44% <30.95%> (-26.99%)`	⬇️

zeeshanlakhani · 2023-06-28T03:50:30Z

To the CI errors, @mriise if you install the pre-commit hook and such, you'll get the cargo fmt stuff for free too. LMK if you want to chat through that or the nix flake.

homestar-runtime/Cargo.toml

homestar-runtime/config/testkey.pem

homestar-runtime/src/settings.rs

zeeshanlakhani · 2023-06-28T11:20:52Z

homestar-runtime/src/settings.rs

+                };
+                // TODO convert err
+                let pem = pem::parse(pem_file).unwrap();
+                // we only take ed25519


Must we only take ed25519 btw? We could support others, i.e. https://libp2p.github.io/rust-libp2p/libp2p_identity/index.html? Maybe this is part of the configuration.

we can support Secp256k1 keys as it will likely be need for blockchains, but I would prefer to stay away from RSA. Am neutral on the random variant.

Yep. Good with that/those choice(es)

added support for secp256k1, however importing is a bit weird. for the moment i am just importing a raw key bytes in PEM format, but it should probably be pkcs#8 or something.

homestar-runtime/config/settings.toml

zeeshanlakhani

Some super minor comments @mriise, but this is shaping up really nicely.

homestar-runtime/Cargo.toml

zeeshanlakhani · 2023-07-04T02:07:03Z

homestar-runtime/fixtures/settings-import-secp256k1.toml

+[node]
+
+[node.network.keypair_config]
+# `openssl ecparam -genkey -name secp256k1 -outform DER -o secp256k1_key.der`


so great to have the docs/comments here!

homestar-runtime/src/network/swarm.rs

zeeshanlakhani · 2023-07-04T02:07:45Z

homestar-runtime/src/settings.rs

+                match key_type {
+                    KeyType::Ed25519 => {
+                        identity::Keypair::ed25519_from_bytes(new_key).map_err(|e| {
+                            anyhow!("Failed to generate ed25519 key from random: {:?}", e)


minor: {:#?} instead of {:?} will pretty-print debug logs.

yeah we can pretty print these. though if we are going to be showing these errors to a lib that displays errors better in general like https://docs.rs/color-eyre/latest/color_eyre/

@mriise yep, we'll prob bring in color_eyre for local dev. Nonetheless, this is more so for logs that will be picked up by grafana/other-simialr tool (or logging service), especially as we use logfmt.

zeeshanlakhani

Great work @mriise. I had a super minor comment on the tests below, i.e. assert vs unwrap.

With that, this is mergeable! => First PR in the books!!!!

zeeshanlakhani · 2023-07-06T03:17:13Z

homestar-runtime/src/settings.rs

+    fn import_secp256k1_key() {
+        let settings = Settings::build("fixtures/settings-import-secp256k1.toml".into()).unwrap();
+
+        settings.node.network.keypair_config.keypair().unwrap();


minor:

Suggested change

settings.node.network.keypair_config.keypair().unwrap();

assert!(settings.node.network.keypair_config.keypair().is_ok())

panic on unwrap arguably give better info on failure than an assert of is_ok here since the unwrap will print out the actual error and is_ok will just print the bool

@mriise thinking about it, then expect is what we'd want (as it will also print the actual error, and the context for the error given).

zeeshanlakhani · 2023-07-06T03:17:21Z

homestar-runtime/src/settings.rs

+    fn seeded_secp256k1_key() {
+        let settings = Settings::build("fixtures/settings-random-secp256k1.toml".into()).unwrap();
+
+        settings.node.network.keypair_config.keypair().unwrap();


same as above.

zeeshanlakhani · 2023-07-06T03:17:59Z

homestar-runtime/src/settings.rs

+
+                match key_type {
+                    KeyType::Ed25519 => {
+                        const PEM_HEADER: &str = "PRIVATE KEY";


minor: I tend to put consts up top, though doesn't matter much.

Didn't want it conflated when we have the secp PEM headers, so would pref to keep as is.

zeeshanlakhani · 2023-07-06T18:43:45Z

Great work @mriise. Last thing, @mriise can you sign your commits? May be easier to rebase here and sign the singular commit. Thanks!

… not specified in config

…r function & warn about filesystem access.

mriise requested a review from a team as a code owner June 28, 2023 03:34

zeeshanlakhani reviewed Jun 28, 2023

View reviewed changes

homestar-runtime/src/settings.rs Outdated Show resolved Hide resolved

zeeshanlakhani reviewed Jun 28, 2023

View reviewed changes

zeeshanlakhani reviewed Jul 4, 2023

View reviewed changes

zeeshanlakhani mentioned this pull request Jul 4, 2023

refactor: first-pass at runtime interface, signals, shutdown, and more #180

Merged

mriise force-pushed the ident-import branch from e6b5efd to 446d50b Compare July 6, 2023 01:55

zeeshanlakhani approved these changes Jul 6, 2023

View reviewed changes

mriise force-pushed the ident-import branch 2 times, most recently from 06313f3 to 20a80a9 Compare July 6, 2023 07:46

mriise added 16 commits July 6, 2023 14:33

add pubkey config field to settings

1038d2f

add pem processing & way too many panics

37c7ccf

fix: use a zeroed out secret key for example instead of a generated one

e6880e7

fix: move secret key to fixtures folder

148f814

fix: add serde default macro so that fields use the default impl when…

5f1d9be

… not specified in config

feat: use an actual RNG/seed phrase

a90cafd

feat: add test for importing a key, use anyhow errors

8b93914

feat: add support for secp256k1 keys

9b81dc7

fix: take raw bytes for secp key imports for now

42fe7b9

fix: use different lib for PEM and DER file parsing

3b951c2

docs: add a bit more docs

8bde951

fix: ride patch releases for new crates

c2a61ce

fix: dont caps error messages as per review suggestion, rename keypai…

432349d

…r function & warn about filesystem access.

feat: add info logs for key generation

56d8158

fix: pretty format some errors

c45ef1f

fix: use expect() in tests instead of unwrap()

3f0b887

mriise force-pushed the ident-import branch from b9eb51f to 3f0b887 Compare July 6, 2023 21:33

mriise merged commit 938ac61 into main Jul 6, 2023

mriise deleted the ident-import branch July 6, 2023 21:35

zeeshanlakhani mentioned this pull request Jul 8, 2023

Add option to import keypair to runtime at startup for libp2p network/swarm setup #163

Closed

3 tasks

release-plz-ipvm-wg bot mentioned this pull request Oct 9, 2023

chore: release #372

Closed

This was referenced Jan 19, 2024

chore: release #511

Closed

chore: release #516

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pubkey importing / seed generation #172

Add pubkey importing / seed generation #172

mriise commented Jun 28, 2023

zeeshanlakhani left a comment

codecov bot commented Jun 28, 2023 •

edited

Loading

zeeshanlakhani commented Jun 28, 2023 •

edited

Loading

zeeshanlakhani Jun 28, 2023

mriise Jun 30, 2023

zeeshanlakhani Jun 30, 2023

mriise Jun 30, 2023

zeeshanlakhani left a comment •

edited

Loading

zeeshanlakhani Jul 4, 2023

zeeshanlakhani Jul 4, 2023

mriise Jul 6, 2023

zeeshanlakhani Jul 6, 2023

zeeshanlakhani left a comment

zeeshanlakhani Jul 6, 2023

mriise Jul 6, 2023

zeeshanlakhani Jul 6, 2023

zeeshanlakhani Jul 6, 2023

zeeshanlakhani Jul 6, 2023

mriise Jul 6, 2023

zeeshanlakhani commented Jul 6, 2023

	settings.node.network.keypair_config.keypair().unwrap();
	assert!(settings.node.network.keypair_config.keypair().is_ok())

Add pubkey importing / seed generation #172

Add pubkey importing / seed generation #172

Conversation

mriise commented Jun 28, 2023

Description

Link to issue

Type of change

Test plan (required)

Screenshots/Screencaps

zeeshanlakhani left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 28, 2023 • edited Loading

Codecov Report

zeeshanlakhani commented Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeeshanlakhani left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeeshanlakhani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeeshanlakhani commented Jul 6, 2023

codecov bot commented Jun 28, 2023 •

edited

Loading

zeeshanlakhani commented Jun 28, 2023 •

edited

Loading

zeeshanlakhani left a comment •

edited

Loading