NSEC3 generation support. #416
Conversation
A private key converted into a 'KeyPair' can be exported in the conventional DNS format. This is an important step in implementing 'ldns-keygen' using 'domain'. It is up to the implementation modules to provide conversion to and from 'KeyPair'; some impls (e.g. for HSMs) won't support it at all.
'Sign' is a more generic version of 'sign::key::SigningKey' that does not provide public key information. It does not try to abstract over all the functionality of a keypair, since that can depend on the underlying cryptographic implementation.
There are probably lots of bugs in this implementation, I'll add some tests soon.
Also fixes 'cargo clippy' issues, particularly with the MSRV.
I'm going to add a corresponding 'PublicKey' type, at which point it becomes important to differentiate from the generic representations and actual cryptographic implementations.
Key generation, for now, will only be provided by the OpenSSL backend (coming soon). However, generic keys (for RSA/SHA-256 or Ed25519) can be imported into the Ring backend and used freely.
The OpenSSL backend supports import from and export to generic secret keys, making the formatting and parsing machinery for them usable. The next step is to implement generation of keys.
There were bugs in the Base64 encoding/decoding that are not worth trying to debug; there's a perfectly usable Base64 implementation in the crate already.
I had to swap out the RSA key since 'ring' found it to be too small.
- RSA signatures were being made with an unspecified padding scheme. - ECDSA signatures were being output in ASN.1 DER format, instead of the fixed-size format required by DNSSEC (and output by 'ring'). - Tests for signature failures are now added for both backends.
|
@ximon18: yeah, I think this should be moved under |
Done. |
src/sign/records.rs
Outdated
| // the apex and the original owner name." | ||
| let distance_to_root = name.owner().iter_labels().count(); | ||
| let distance_to_apex = distance_to_root - apex_label_count; | ||
| if distance_to_apex > 1 { |
There was a problem hiding this comment.
I don't think the if statement is necessary, the for loop will run for zero iterations if this condition is not true.
There was a problem hiding this comment.
True, but the if statement matches nicely with the RFC text and makes it clear that if we enter this block it is because of the condition identified by the RFC text.
src/sign/records.rs
Outdated
| // It will NOT construct the last name as that will be dealt | ||
| // with in the next outer loop iteration. | ||
| // - a.b.c.mail.example.com | ||
| for n in (1..distance_to_apex - 1).rev() { |
There was a problem hiding this comment.
If distance_to_apex is 2, then this loop will never execute. Could the outer condition have been distance_to_apex > 2, then? Is there an off-by-one error somewhere?
There was a problem hiding this comment.
Thanks! I was planning to add proper test cases but for now have extended my test input zone to cover this missing case and realize now why I had some additional logic in there before which I removed because I couldn't see what value it was adding... 😛 Looking at the comments again I see I even left in a description of the additional logic I removed, i.e. tracking the last non-empty non-terminal label.
…EC3." This reverts commit a04c917.
…ity fixes from the downstream multiple-signing-key branch.
… partial backport of changes from the downstream multiple-signing-key branch.
tertsdiepraam
force-pushed
the
initial-nsec3-generation
branch
from
cf2f450
to
e1c1db8
Compare
Currently lacks collision detection and tests, though has been manually tested using
ldns-verify-zone,dnssec-verifyandnamed-checkzoneboth with and without opt-out and also including both signed and unsigned delegations.I'm posting this here as a draft to allow for alignment and early feedback from the team working on various pieces of DNSSEC support for
domain.