-
-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhanced zone signing. #418
base: initial-nsec3-generation
Are you sure you want to change the base?
Conversation
- FIX: Clear the signing buffer between uses. - Output signed DNSKEY RRs from sign().
…e unsigned delegation NSEC3 RRs in the output.
/// If only one key has a supported algorithm and has the ZONE flag set | ||
/// AND has the SEP flag set, it will be used as a CSK (i.e. both KSK and | ||
/// ZSK). |
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.
This isn't quite true. At the moment, if only KSKs or only ZSKs are found, then they are all used as CSKs. If multiple KSKs or multiple ZSKs are found, they will all be used. Do you want to update the documentation or the code?
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.
Thanks. Note: None of this behaviour / interface is set in stone yet as I'm still working my way through the support needed for all of the ldns-signzone
arguments and behaviours. Once I'm done with that I'll start reconsidering the API and then update comments to match.
#[allow(clippy::type_complexity)] | ||
pub fn sign<Octets, ConcreteSecretKey>( | ||
&self, | ||
apex: &FamilyName<N>, | ||
expiration: Timestamp, | ||
inception: Timestamp, | ||
key: SigningKey<Octets, ConcreteSecretKey>, | ||
) -> Result<Vec<Record<N, Rrsig<Octets, N>>>, ErrorTypeToBeDetermined> | ||
keys: &[SigningKey<Octets, ConcreteSecretKey>], |
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.
Better to take an iterator, maybe?
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.
There's a version of this code that takes an iterator and works with a domain ZoneTree, so yes changes to this API are intended at some point, but at the moment I'm just making stupid minimal changes to the API and will return to the topic of API design later.
This PR builds upon #416 to extend the signing support in
domain
to support signing with multiple keys and key types using the new key management functionality in #406.