plugin/sign: a plugin that signs zone

[miekg]

Sign is a plugin that signs zone data (on disk). The README.md details
what exactly happens to should be accurate related to the code.

There are a couple of things missing, documented in TODO; these mostly
deal with setup and tear down. But also some more important things like
adding NSEC and not signing glue records. Also more tests need be added.

The current test signs db.miek.nl and puts the result in
db.miek.nl.signed in the current directory.

The change to file/* need to be backed out, once some other changes in
another PR are in - these are just here to make things work now.

---

Pushing this now if someone feels inclined to review as new plugins are
usually a lot of code at once - currently this is still manageable, I
hope.

[corbot]

Thank you for your contribution. I've just checked the OWNERS files to find a suitable reviewer. This search was successful and I've asked superq (via /OWNERS) for a review.

If you have questions or suggestions for this bot, please file an issue against the miekg/dreck repository.

The bot understands the commands that are listed here.

[stp-ip]

Still want to go deeper on the code, currently swamped with other stuff, but added a few first thoughts and ideas.

[miekg]

@stp-ip think I fixed your comments. PTAL

[miekg]

[ Quoting <notifications@github.com> in "Re: [coredns/coredns] plugin/sign: ..." ]

+Or use a single zone file for *multiple* zones, note that the **ZONES** are repeated for both plugins. +Also note this outputs *multiple* signed output files. Here we use the default output directory +`/var/lib/coredns`. + +~~~ corefile +. { + file /var/lib/coredns/db.example.org.signed example.org + file /var/lib/coredns/db.example.net example.net ```suggestion file /var/lib/coredns/db.example.net.signed example.net ```

applied. thanks!

[codecov-io]

Codecov Report

Merging #2993 into master will decrease coverage by 0.06%.
The diff coverage is 54.24%.

@@            Coverage Diff             @@
##           master    #2993      +/-   ##
==========================================
- Coverage   55.83%   55.76%   -0.07%     
==========================================
  Files         206      213       +7     
  Lines       10417    10747     +330     
==========================================
+ Hits         5816     5993     +177     
- Misses       4183     4298     +115     
- Partials      418      456      +38

Impacted Files	Coverage Δ
plugin/sign/sign.go	0% <0%> (ø)
plugin/sign/dnssec.go	100% <100%> (ø)
plugin/sign/keys.go	40.32% <40.32%> (ø)
plugin/sign/signer.go	51.3% <51.3%> (ø)
plugin/sign/file.go	57.14% <57.14%> (ø)
plugin/sign/setup.go	64.17% <64.17%> (ø)
plugin/sign/nsec.go	72.22% <72.22%> (ø)
plugin/file/reload.go	69.44% <0%> (-5.56%)	⬇️
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eec24cb...2779e4e. Read the comment docs.

[miekg]

OK, this should be relatively ready. Code without test is about 600 lines, which is not too bad. The meat of the thing is in signer.go.

I'm running this branch on miek.nl (just for that zone, but I'll add the rest) to see how things go (eyeballing the log). I didn't change the default timers so this will take some while).

[miekg]

It also needs a test to check that if, after startup, the zone needs resigning because the signatures are almost expired. Currently I believe this falls back on signer.last being zero, which is not the intent

…

On Sun, 21 Jul 2019, 20:25 Codecov, ***@***.***> wrote: Codecov <https://codecov.io/gh/coredns/coredns/pull/2993?src=pr&el=h1> Report Merging #2993 <https://codecov.io/gh/coredns/coredns/pull/2993?src=pr&el=desc> into master <https://codecov.io/gh/coredns/coredns/commit/01e13c622e087779d5e0e1e82378333ccca9b1fb?src=pr&el=desc> will *decrease* coverage by 0.07%. The diff coverage is 54.6%. [image: Impacted file tree graph] <https://codecov.io/gh/coredns/coredns/pull/2993?src=pr&el=tree> @@ Coverage Diff @@ ## master #2993 +/- ## ========================================== - Coverage 56.2% 56.12% -0.08% ========================================== Files 204 211 +7 Lines 10136 10462 +326 ========================================== + Hits 5697 5872 +175 - Misses 4025 4139 +114 - Partials 414 451 +37 Impacted Files <https://codecov.io/gh/coredns/coredns/pull/2993?src=pr&el=tree> Coverage Δ plugin/sign/sign.go <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree#diff-cGx1Z2luL3NpZ24vc2lnbi5nbw==> 0% <0%> (ø) plugin/sign/dnssec.go <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree#diff-cGx1Z2luL3NpZ24vZG5zc2VjLmdv> 100% <100%> (ø) plugin/sign/keys.go <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree#diff-cGx1Z2luL3NpZ24va2V5cy5nbw==> 40.32% <40.32%> (ø) plugin/sign/signer.go <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree#diff-cGx1Z2luL3NpZ24vc2lnbmVyLmdv> 52.67% <52.67%> (ø) plugin/sign/file.go <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree#diff-cGx1Z2luL3NpZ24vZmlsZS5nbw==> 54.16% <54.16%> (ø) plugin/sign/setup.go <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree#diff-cGx1Z2luL3NpZ24vc2V0dXAuZ28=> 64.7% <64.7%> (ø) plugin/sign/nsec.go <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree#diff-cGx1Z2luL3NpZ24vbnNlYy5nbw==> 72.22% <72.22%> (ø) plugin/route53/route53.go <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree#diff-cGx1Z2luL3JvdXRlNTMvcm91dGU1My5nbw==> 84.28% <0%> (-2.15%) ⬇️ ... and 5 more <https://codecov.io/gh/coredns/coredns/pull/2993/diff?src=pr&el=tree-more> ------------------------------ Continue to review full report at Codecov <https://codecov.io/gh/coredns/coredns/pull/2993?src=pr&el=continue>. *Legend* - Click here to learn more <https://docs.codecov.io/docs/codecov-delta> Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov <https://codecov.io/gh/coredns/coredns/pull/2993?src=pr&el=footer>. Last update 01e13c6...12aad48 <https://codecov.io/gh/coredns/coredns/pull/2993?src=pr&el=lastupdated>. Read the comment docs <https://docs.codecov.io/docs/pull-request-comments> . — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2993?email_source=notifications&email_token=AACWIWZDCRUQTQDVCIPRFI3QASZZDA5CNFSM4IAWMGO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2OJ5TI#issuecomment-513580749>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACWIW5LCL2SL6ZMC7AOZE3QASZZDANCNFSM4IAWMGOQ> .

[miekg]

Seeing a lot of transfers happing

Jul 26 04:25:18 deb coredns[720]: 2019-07-26T04:25:18.910Z [INFO] plugin/file: Outgoing transfer of 53 records of zone miek.nl. to 37.97.149.87 started
Jul 26 04:27:18 deb coredns[720]: 2019-07-26T04:27:18.944Z [INFO] plugin/file: Outgoing transfer of 53 records of zone miek.nl. to 37.97.149.87 started
Jul 26 04:29:18 deb coredns[720]: 2019-07-26T04:29:18.993Z [INFO] plugin/file: Outgoing transfer of 53 records of zone miek.nl. to 37.97.149.87 started
Jul 26 05:25:46 deb coredns[720]: 2019-07-26T05:25:46.060Z [INFO] plugin/file: Outgoing transfer of 53 records of zone miek.nl. to 37.97.149.87 started
Jul 26 05:27:46 deb coredns[720]: 2019-07-26T05:27:46.136Z [INFO] plugin/file: Outgoing transfer of 53 records of zone miek.nl. to 37.97.149.87 started
Jul 26 05:29:46 deb coredns[720]: 2019-07-26T05:29:46.161Z [INFO] plugin/file: Outgoing transfer of 53 records of zone miek.nl. to 37.97.149.87 started
Jul 26 06:28:38 deb coredns[720]: 2019-07-26T06:28:38.221Z [INFO] plugin/file: Outgoing transfer of 53 records of zone miek.nl. to 37.97.149.87 started
Jul 26 06:30:38 deb coredns[720]: 2019-07-26T06:30:38.257Z [INFO] plugin/file: Outgoing transfer of 53 records of zone miek.nl. to 37.97.149.87 started

[miekg]

after some simplifications I've now moved all my signed zones over:

Aug 03 18:45:58 deb coredns[4637]: 2019-08-03T18:45:58.158Z [INFO] plugin/sign: Skipping signing zone "miek.nl." in "/var/lib/coredns/db.miek.nl.signed", signatures are valid
Aug 03 18:45:58 deb coredns[4637]: 2019-08-03T18:45:58.160Z [INFO] plugin/sign: Skipping signing zone "dnssex.nl." in "/var/lib/coredns/db.dnssex.nl.signed", signatures are valid
Aug 03 18:45:58 deb coredns[4637]: 2019-08-03T18:45:58.161Z [INFO] plugin/sign: Skipping signing zone "atoom.net." in "/var/lib/coredns/db.atoom.net.signed", signatures are valid
Aug 03 18:45:58 deb coredns[4637]: 2019-08-03T18:45:58.161Z [INFO] plugin/sign: Skipping signing zone "coredns.io." in "/var/lib/coredns/db.coredns.io.signed", signatures are valid

We'll see what happens

[miekg]

any problems with merging this? (@stp-ip)

Has been running stable on my server.

(as said, this needs followup PRs to make delegations work)

[stp-ip]

/lgtm

Added a few comments.

Will try to carve out some additional time to do another run over signer.go and test a few more things.

[stp-ip]

Why specifically 100 records?

[miekg]

it should be 1, but the RFC doesn't mandate the SOA being first. 100 seemed like a crazy enough number that makes sense? Open to suggestions.
(I'll add comment on why the current value)

[stp-ip]

100 seems reasonable high to catch the edge cases of the SOA not being first I agree.
Comment for the reasoning sounds good. 100 were too arbitrary without a comment in my view.

[stp-ip]

Did a few more manual tests and another run on the signer.go file. /lgtm

[miekg]

lgtm at the end of a line isn't detected. But noted and thanks :)

[stp-ip]

Damn that was supposed to have a newline ;)