Added persistence of node-key #84
Conversation
|
Thanks, we usually use openssh key format, for storing the keys, you can see how here: https://github.com/n0-computer/iroh-n0des/blob/main/src/client.rs#L64 |
|
Verified that the |
src/main.rs
Outdated
| .map(Cow::from) // Reference | ||
| .or_else(|| { | ||
| std::env::home_dir().map(|mut p| { | ||
| p.push(".auth"); |
There was a problem hiding this comment.
shouldn't this be stored in a platform-specific app folder?
eg. dirs::config_dir().join("dumbpipe").join("dumbpipe.key")
with https://crates.io/crates/dirs
| Ok(Some(result)) => return result, | ||
| Ok(None) => {} | ||
| Err(error) => { | ||
| error!("Error reading persisted dumbpipe key: [{:?}]", error); |
There was a problem hiding this comment.
Instead of logging the error and continuing with overwriting the keyfile, instead I'd propose to abort here and tell the user to delete the keyfile or set a different path. Silently overwriting the file is no good practice IMO, it might destroy an actual key that was copied to the wrong place or such.
There was a problem hiding this comment.
Thanks for spotting this. My intention was to continue with an ephemeral key without overwriting an existing file. I tried to continue with the normal operation of dumbpipe wherever I could, but maybe a fail-fast design would be beter. If I abort here, then it would be consistent to abort when there is a parse error when reconstructing the key or when the file can't be written to. What do you think most users of dumbpipe would prefer: permissive or fail-fast?
src/main.rs
Outdated
|
|
||
| #[derive(Debug, Snafu)] | ||
| #[non_exhaustive] | ||
| pub enum PersistError { |
There was a problem hiding this comment.
Because dumbpipe is an application, I think you can remove this error enum and instead just return n0_snafu::Error (n0_snafu::Error is similar to anyhow::Error)
It is scary when folks have to worry about secrets being printed. #84 will solve the key persistence in a much better way anyway.
|
I created a crate |
3 participants
I added the possibility to persists the node-key, so that the listener can be restarted after a reboot or container restart. I use this for a
dumbpipethat forwards a port from my development laptop to a dev-container that serves a Dioxus-app.Together with the similar persisted node-id in the Iroh-P2P enabled remote_server for Zed (courtesy of dignifiedquire) this enables me to remotely develop an app without having to copy and paste a new connection-string each time the dev-container is restarted. See also the definition of the dev-rust-dioxus container-image.
This is the first time I used
snafu(I am used toanyhow). The error-structs and associated trait-impls are quite verbose. Is this idiomatic, of do you have suggestions on how to improve it?