-
Notifications
You must be signed in to change notification settings - Fork 266
hc-dpki cli for DeepKey #2117
base: develop
Are you sure you want to change the base?
hc-dpki cli for DeepKey #2117
Conversation
@zo-el I notice a bunch of tests are breaking, would like to review this after tests pass... |
* updated tests
@zippy this PR is ready for review |
quiet, | ||
); | ||
keygen_standalone(keystore_passphrase)? | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed a few panics in here even though we are returning an HCResult, is this because we cannot recover from them if they happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, these expects should be replaced with ?
.read_line(&mut input) | ||
.expect("Could not read from stdin"); | ||
match input.as_str() { | ||
"Y\n" => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we care about lowercase/uppercase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not Really
&mut self.inner.buf, | ||
)?; | ||
Ok(KeyBundle::new_from_seed_buf(&mut ref_seed_buf)?) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could probably generalize these two structs with an enum.
enum Seed
{ Auth,Revocation}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good but had a few questions about the number of expects we have in here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @AshantiMutinta's comments
quiet, | ||
); | ||
keygen_standalone(keystore_passphrase)? | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, these expects should be replaced with ?
PR summary
Adds some new commands to the CLI tool for managing DPKI keys.
hc dpki genroot
creates a new random root seedhc dpki keygen
is similar tohc keygen
but will use a dpki root seed mnemonic so that keys can be recoveredhc dpki genrevoke/genauth
produce revocation/auth seeds from which keys can be derived.hc dpki sign
use a revocation/auth seed to produce a key then sign a message (which can be an agent key). This is the process by which keys can be authorized and revokedEach time a mnemonic is produced it is optionally encrypted with a passphrase
testing/benchmarking notes
( if any manual testing or benchmarking was/should be done, add notes and/or screenshots here )
followups
( any new tickets/concerns that were discovered or created during this work but aren't in scope for review here )
changelog
documentation