feat: serve pre-signed DNSSEC zones (DNSKEY/RRSIG/NSEC* + PRESIGNED metadata)#4
Conversation
|
Thank you for the PR and the work you put into it. I have to walk through your changes thoroughly and comment them. I screened your changes at a whole and see some points worth discussing. Please give me some time to do so. I have already begun to implement DNSSEC live signing (currently not commit-ready, because it uses transactions and it is challenging to make it work). But I like the idea to first implement support for the pre-signed model, because it does not rely on transactions and apparently would make at least you happy :-) ; but I have to coordinate that work with the live-signing model (which is definitely on the planned features list). Before the next "big change" (which will be some DNSSEC support) I would like to release the next development version (0.4.0), which has some important changes worth releasing. It should take place in a few days. |
|
Can confirm: solving my own problems does, in fact, make me happy 🙂 — happiness levels measurably rising on the operational side already. No rush at all. Take whatever time you need; I'd rather have this land cleanly alongside the live-signing work than rushed. I'm happy to rebase on top of 0.4.0 once it's out and to revisit anything that conflicts with where you're heading on the transactional side. |
|
I have a question about signing, because I do not yet understand it yet in much detail and did not dive deep into documentation yet: When (pre-)signing the zone, is the serial part of the signature or not? You have added a special handling or override for the SOA serial in the PR; is it because the serial is part of the signature and would invalidate the signature when changed automatically due to some unrelated or even effectless changes (e.g. change an |
|
Yes — the SOA serial is part of the signature, but only of the SOA RRSIG. DNSSEC signs each RRset independently, so RRSIG(A), RRSIG(MX), RRSIG(NS) etc. cover only their own RRset's RDATA and are unaffected by serial changes. Only RRSIG(SOA) breaks if the served serial differs from the one the signer saw — but that's enough for validating resolvers to reject the whole answer. A correction on what you read: the PR does not actually add a serial override. The doc note in To close the gap I'd add a real override: accept a numeric |
|
Finally I have sorted and committed most of my local work, only the DNSSEC transactions stuff for live signing is still WIP. But this should not affect your PR, so you could now rebase it on the latest code. This current committed state should be the base for the release 0.4.0, your PR would be applied afterwards. Some comments on your proposed changes (in no particular order):
|
3f57448 to
c9bd733
Compare
|
Rebased on latest master and reworked per feedback. Force-push incoming.
Diff vs master: +57/-3 across 4 files plus `dnssec_test.go`. Unit tests, vet, and golangci-lint v2.9 clean (remaining lint warnings are all preexisting in master). |
nixn
left a comment
There was a problem hiding this comment.
Please review my suggestions and comments.
as described in PR #4 it is more flaky the more CPUs are available to it
|
Thanks for the thorough review — all six points applied in 0ec6cb6:
Pushed as a separate commit on top of the rebased branch so the delta from your review is easy to see. |
When set on a zone, this metadata key replaces the automatic zoneRev-derived serial in the SOA record (parsed as uint32, per RFC 1035). Invalid/missing values fall back to zoneRev() unchanged. Motivation: pre-signed DNSSEC needs the served SOA serial to match the one baked into RRSIG(SOA); otherwise validators reject the answer. Also useful without DNSSEC for manual serial control. Pre-signed zones otherwise need no new code: PRESIGNED=1 rides the existing metadata mechanism, and DNSSEC record types (DNSKEY, RRSIG, NSEC*, DS, CDS, CDNSKEY) fall through the default plain-string path and are served verbatim.
- move soaSerial out of the *rrParams receiver into a free function (SOA-specific logic shouldn't hang off the receiver common to all RRs) - drop the redundant plain-string passthrough test (verbatim serving of alphanumeric-leading plain strings is already covered by the default path) - docs: tighten the X-PE3-MINIMUM-SERIAL note, the pre-signed intro and the NSEC-chain wording; trim the README Features bullet
0ec6cb6 to
26e1a68
Compare
|
Thank you for your work. There should be a test for DNSSEC in pre-signed mode, but (again) the main work was implementing the support, which is now merged. I have already put work into live signing (heads up: it seems to basically work) and also added testing for it, so I would write the test myself, because the base work for it is done already (unfortunately that was not straightforward); no need for you to waste time on that. I really admire your courage to use pdns-etcd3 in production, please feel free to report any further issue with that. |
Summary
Pre-signed DNSSEC support: any zone whose apex carries a
DNSKEYrecord is served asPRESIGNED=1withDNSKEY,RRSIG,NSEC,NSEC3,NSEC3PARAM,DS,CDSandCDNSKEYrecords passed back to PowerDNS verbatim. Detection is automatic (no config switch). Online signing is intentionally out of scope.Motivation
Today, configuring
remote-dnssec=yesagainst this backend produces a steady stream ofunknown/unimplemented request: getDomainKeyserrors and breaksANYqueries on apex names with anALIAS(PowerDNS treats the empty-key answer as a sign the backend cannot serve the zone). The fix is small: implement the three DNSSEC-related backend methods PowerDNS probes (getDomainMetadata,getAllDomainMetadata,getDomainKeys) and accept the DNSSEC record types as opaque presentation-format strings that are stored once by an external signer and served unchanged.This unlocks the pre-signed deployment model — sign offline (
ldns-signzone,dnssec-signzone, OpenDNSSEC, ...), push the resulting records into ETCD with the same naming scheme, and let pdns-etcd3 serve them — without taking on the much larger surface of online signing (key storage, KASP, NSEC chain navigation, key rollover) or modifying any existing record path.What changed
src/rr.go—DNSKEY,RRSIG,NSEC,NSEC3,NSEC3PARAM,DS,CDSandCDNSKEYare added to bothparses(regex that captures the entire content) andrrFuncs(a newpassthrough(key)rrFunc that emits the value unchanged).src/data.go— two helpers on*dataNode:hasDNSKEY()checks a single node,hasDNSKEYForZone(name)walks down from the data root with name normalisation (case-insensitive, trailing-dot tolerant) and returns whether the matching node has any DNSKEY at its apex.src/pdns-etcd3.go—handleRequestnow produces real responses forgetalldomainmetadata(returns{"PRESIGNED":["1"]}when the zone is pre-signed,{}otherwise),getdomainmetadata(returns["1"]only when the requested kind matches a present metadata key), andgetdomainkeys(returns[], no longer an "unknown/unimplemented" error). The detection helpercollectMetadata(zoneName)is the single source of truth.src/dnssec_test.go— unit tests under the existingunitbuild tag covering the registration of the new types in both maps, presentation-format round-trip viaparseContentfor each type, the data-node helpers (with case-insensitive / trailing-dot variants), and the metadata producer (signed / unsigned / missing / empty-name).README.mdanddoc/ETCD-structure.md— new "Pre-signed DNSSEC" subsections explaining the supported types, detection rule, a minimal example layout and the operational notes (re-sign cadence, atomic NSEC chain updates, serial bump). The Features list and the Planned section are adjusted accordingly.Net diff: ~340 lines (~145 of which are tests and docs).
Out of scope (deliberate)
getBeforeAndAfterNamesAbsolute— denial-of-existence proofs for non-present names are not synthesised. NSEC3 with opt-out and pre-computed chain entries in ETCD covers the common case for static zones.getTSIGKeyand friends remain unimplemented (orthogonal to pre-signing).Tests
make unit-tests— green, including the four new test functions.make integration-tests— passes for everything exceptTestParallelRequests, which is pre-existing flakiness on this branch's base (upstream/masteralso fails the same test ~2 of 3 runs in the same environment; the thresholddataRoot.readers.max < nCPU/2is borderline on machines with many CPUs).DNSKEYs + anRRSIG+ anNSECreturns the records correctly vialookup,getalldomainmetadatareturns{"PRESIGNED":["1"]}for the signed zone and{}for an unsigned sibling,getdomainmetadata PRESIGNEDreturns["1"]for the signed zone,getdomainkeysreturns[].Backwards compatibility
getalldomainmetadatareturns{},getdomainmetadatareturns[]).getdomainkeyscall now returns[]cleanly, removing the long-standing log noise even on deployments that do not enable DNSSEC.