CLI Key Management #61
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Philip-NLnetLabs
force-pushed
the
keyset
branch
from
a88e558 to
f8c1453
Compare
Contributor
|
This seems to be missing the file for the |
Co-authored-by: Terts Diepraam <[email protected]>
mozzieongit
reviewed
Aug 13, 2025
Member
mozzieongit
left a comment
mozzieongit
left a comment
There was a problem hiding this comment.
I know this is still a draft, but here my notes from reading this PR to understand the other PRs:
- Add comments on the subcommands, that say what they do and how to use them
- And a note about what to do after each command? e.g. after init / creating key, mention which command should be called next and when
- Split up big run function into sub functions (obsolete by #108)
- Consistently use
env.in_cwd()vs raw paths (e.g.read_to_string(priv_url.path())) or make clear why not usingenv.in_cwd() - Add tests? I'm sure not all can be tested easily, but maybe some can
- Explain to users which TTL value is expected in the propagation/cache subcommands
- adjust the actions output after each command, to something human-actionable, so the user knows what to do with that information
- Adds KMIP server based key generation, signing and destruction, equivalent to the existing Ring/OpenSSL functionality. - Adds new kmip subcommands for managing KMIP server configurations. - Adds support for referring to KMIP keys by a new KMIP URL scheme. - Add a feature for the KMIP crypto backend just like the Ring and OpenSSL crypto backends. - Adds support for storing sensitive credentials in files separate to the KMIP server configuration.
* Restructure roll commands. * Import public keys. * Import a public/private key pair from files. * Add a default TTL to config. Use that for DNSKEY/CDS/CDNSKEY/DS RRsets. * Cargo.lock. * Support for importing KMIP keys. * Import public/private keys in decoupled state * Add --private-key option to importing a public/private key pair from files. * Add remove-key command.
* Verbose status output. * Add TODO list. * Get rid of .map_err::<Error, _>. Just .map_err is fine if the .into() is removed.
| domain_name, | ||
| keyset_state, | ||
| } = self.cmd | ||
| { |
Contributor
There was a problem hiding this comment.
Maybe it'd be nice to split this function up a bit i.e. extract all the subcommands to separate functions.
Member
Author
There was a problem hiding this comment.
Yes. I want to introduce a struct State. Then I can see if those can be operations on State.
| Self::Notify(notify) => notify.execute(env), | ||
| Self::SignZone(signzone) => signzone.execute(env), | ||
| Self::Update(update) => update.execute(env), | ||
| // Self::Help(help) => help.execute(env), |
Contributor
There was a problem hiding this comment.
Nit but I think this stems from a conflict.
Member
Author
There was a problem hiding this comment.
As far as I know, help was disabled by Jannik until the help command can render man pages.
Member
Author
There was a problem hiding this comment.
Ah, I see, in commit 6915b81:
- // Self::Help(help) => help.execute(env),
+ Self::LdnsUpdate(ldnsupdate) => ldnsupdate.execute(env),
That was just too confusing.
Cascade uses this because it can't easily tell whether the keyset state is already initialized, in some cases.
This reverts commit 67b99a1.
Co-authored-by: Ximon Eighteen <[email protected]>
To avoid Cascade installing dnst and uninstalling ldns-utils that Cascade users may want to use such as ldns-verify-zone, which dnst doesn't provide yet. Also because this branch of dnst depends on an as yet unreleased version of the domain crate. Update Cargo.lock bcause cargo-deb complains that it needs updating. Co-authored-by: Jannik Peters <[email protected]>
Co-authored-by: Jannik Peters <[email protected]> Co-authored-by: Philip-NLnetLabs <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Demonstrate key management using a CLI tool. Initial interface between key management and signer.