tools/sec: Add certificates to key-list, add cert-export#181
tools/sec: Add certificates to key-list, add cert-export#181greetingsuniverse wants to merge 8 commits intonamed-data:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds certificate visibility/export capabilities to ndnd sec keychain tooling to address issue #129 (listing/exporting certificates even when corresponding keys may be absent).
Changes:
- Update
key-listto display certificates associated with each key. - Add new
cert-exportsubcommand to export a certificate from a keychain. - Update security utility docs to reflect the expanded CLI behavior and flag syntax.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/sec/keychain.go | Extends key-list output to include certs and introduces cert-export implementation. |
| docs/security-util.md | Updates CLI documentation (flag syntax, key-list description, and adds cert-export section). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| wire, err := kc.Store().Get(name.Prefix(-1), true) | ||
| if err != nil || wire == nil { | ||
| fmt.Fprintf(os.Stderr, "Certificate not found: %s\n", name) |
There was a problem hiding this comment.
cert-export takes CERT-NAME, but the lookup uses name.Prefix(-1) with prefix=true, which drops the version component and can export a different (newest) version than the one explicitly requested. Consider using an exact Get(name, false) when a version component is present, and only falling back to prefix search when the user provides a non-versioned cert name.
| wire, err := kc.Store().Get(name.Prefix(-1), true) | |
| if err != nil || wire == nil { | |
| fmt.Fprintf(os.Stderr, "Certificate not found: %s\n", name) | |
| wire, err := kc.Store().Get(name, false) | |
| if err == nil && wire == nil { | |
| wire, err = kc.Store().Get(name, true) | |
| } | |
| if err != nil || wire == nil { | |
| fmt.Fprintf(os.Stderr, "Certificate not found: %s\n", name) | |
| os.Exit(1) | |
| return |
There was a problem hiding this comment.
Why you remove the last component of the certificate?
There was a problem hiding this comment.
UniqueCerts() always returns names with v=0 so it would fail if that was passed to it, I updated so that it works for all 3 cases (v=0, prefix, or specific cert).
Should I change key-list to get the full cert names instead of output of UniqueCerts() with v=0? I didn't before because no function currently exists in the Keychain or Store interface to make it possible to list every version of a unique certificate
There was a problem hiding this comment.
I already forgot about the keystore. @tianyuan129 What do you think?
| wire, err := kc.Store().Get(name.Prefix(-1), true) | ||
| if err != nil || wire == nil { | ||
| fmt.Fprintf(os.Stderr, "Certificate not found: %s\n", name) |
There was a problem hiding this comment.
Why you remove the last component of the certificate?
Resolves #129
If the corresponding key to a certificate is not contained, cert-export still works but key-list will not list it
key-list displays certificates under keys in this format: