keys: add command warns on poor key practices #1809
Conversation
| if let Ok(secret_key) = std::env::var("SOROBAN_SECRET_KEY") { | ||
| print.infoln("Read secret key from environment variable SOROBAN_SECRET_KEY"); | ||
| Ok(Secret::SecretKey { secret_key }) |
There was a problem hiding this comment.
Just realized there is no check here that it is a valid secret key. That's what we get for using a String when we should just use PrivateKey directly.
There was a problem hiding this comment.
Yup, see:
|
Closing this because this warning is too aggressive. I had a moment and forgot that 12 word seed phrases are still 128-bit. |
| if let Ok(secret_key) = std::env::var("SOROBAN_SECRET_KEY") { | ||
| print.infoln("Read secret key from environment variable SOROBAN_SECRET_KEY"); |
There was a problem hiding this comment.
Nit
| print.infoln("Read secret key from environment variable SOROBAN_SECRET_KEY"); | |
| print.infoln("Reading secret key from environment variable SOROBAN_SECRET_KEY"); |
| .collect::<Vec<_>>(); | ||
| let seed_words_len = seed_words.len(); | ||
| if seed_words_len < 24 { | ||
| print.warnln("Warning, seed phrases containing less than 24 words may not be secure. It is safer to use a 24 word seed. To generate a new key and a 24 word seed phrase use the `stellar keys generate` command."); |
| .map(ToString::to_string) | ||
| .collect::<Vec<_>>() | ||
| .join(" "), | ||
| seed_phrase: seed_words.join(" "), | ||
| }) | ||
| } else { | ||
| Err(Error::PasswordRead {}) |
There was a problem hiding this comment.
I just noticed that when a user doesn't pass --secret-key or --seed-phrase, we get to this else arm, but the error isn't super informative:
$ stellar keys add my-new-key
❌ error: secret input error
Perhaps we can add this, or just add a new error message for this case.
| Err(Error::PasswordRead {}) | |
| print.errorln("No secret key or seed phrase provided. Please use one of these flags: `--secret-key` or `--seed-phrase`."); | |
| Err(Error::PasswordRead {}) |
There was a problem hiding this comment.
Ah, just noticed that this PR is closed. I can open a new PR with this small change.
There was a problem hiding this comment.
I'm removing the error in another PR anyway:
There was a problem hiding this comment.
Nice, that's even better!
What
Output a warning if word count is less than 24 when accepting seed phrases as a new key.
Also changed how the output of the add command is displayed to use the Print functionality to normalize the output like other newer commands.
Why
Close #1806
Known limitations
[TODO or N/A]