Add tmkms yubihsm keys import command
#107
Conversation
Cargo.toml
Outdated
| @@ -19,6 +19,7 @@ circle-ci = { repository = "tendermint/kms" } | |||
| abscissa = "0.0.6" | |||
| abscissa_derive = "0.0.2" | |||
| byteorder = "1.2" | |||
| base64 = "0.10.0" | |||
There was a problem hiding this comment.
You can use base64 from subtle_encoding, which is already a dependency (should be a drop-in replacement):
https://docs.rs/subtle-encoding/latest/subtle_encoding/base64/index.html
There was a problem hiding this comment.
Oh thanks. Should have asked directly. I spent half an hour or so searching for a light weight base64 crate.
There was a problem hiding this comment.
That crate is probably fine now, but had some issues in the past. Regardless, it'd probably be good to avoid adding additional dependencies.
Replacing it should be as easy as:
use subtle_encoding::base64...and it should otherwise be (mostly) API-compatible.
There was a problem hiding this comment.
Done! And yes, agree about avoiding dependencies.
There was a problem hiding this comment.
Awesome, looks good!
Also: this is literally the use case subtle-encoding is designed for: decoding private keys in constant time.
src/commands/yubihsm/keys/import.rs
Outdated
| yubihsm::AsymmetricAlg::Ed25519, | ||
| key, | ||
| ) { | ||
| status_err!("couldn't generate key #{}: {}", self.key_id.unwrap(), e); |
There was a problem hiding this comment.
Should probably be "couldn't import key" or thereabouts
There was a problem hiding this comment.
Whoops. that parts was actually copy&pasted from the generate command (as they almost do the same thing).
| process::exit(1); | ||
| }); | ||
| let v: Value = serde_json::from_slice(&contents).unwrap(); | ||
| let s = v["priv_key"]["value"].as_str().unwrap_or_else(|| { |
There was a problem hiding this comment.
One alternative to consider for this sort of thing is using serde_derive to derive Deserialize for the structure of priv_validator.json.
I'm not sure it matters as this code is probably effectively "done" in that the priv_validator.json format is unlikely to change and this is literally the only use case which matters for tmkms, but I thought I'd at least throw it out there.
There was a problem hiding this comment.
Thanks! I also thought about this. But I think we can keep this for now. If we ever need other fields, we can change to using serde_derive.
|
A couple nits, otherwise LGTM. |
Can someone with a yubihsm test if this actually works?