Ryan Malloy
6ab2b6af6d
H6 — TSIG replay-window test. New TestCheckTSIG_BadStatus_Refused verifies that when miekg/dns reports a TSIG verification failure via ResponseWriter.TsigStatus (the channel for fudge-window violations, bad MACs, expired timestamps), our plugin refuses. The fudge tolerance itself is miekg/dns's default (300s); documented in tsig.go so operators know the dependency. H7 — No-op UPDATE policy: documented explicitly in update.go. We do NOT bump the SOA on a no-op (deduped) UPDATE — forcing downstream secondaries to AXFR identical content wastes bandwidth and contradicts RFC 2136's intent. Callers wanting to force a serial bump can send a throwaway add+delete pair (touch-UPDATE pattern). M3 — Delete-by-exact-match ignores TTL and class per RFC 2136 §2.5.4. The previous rr.String() comparison included TTL, so an UPDATE with CLASS=NONE TTL=0 (the protocol-required encoding for a delete) failed to match stored RRs at CLASS=IN with non-zero TTL. Now we normalize both sides (TTL=0, class=IN) before invoking dns.IsDuplicate. M4 — validateZoneFiles now actually parses each zone at startup (loadRRs invocation). Previously it only stat()'d the file; corrupt zone content sailed through startup and produced SERVFAIL on the first UPDATE with no startup-time signal. Combined with H3+H4's invariant checks, this turns silent zone corruption into immediate startup failure. M7 — Commit-message sanitization. RR names are attacker-controlled (TSIG only authenticates the sender; the payload is hostile by default). Control characters in commit messages could inject newlines into git log or ANSI sequences into downstream log renderers. New sanitizeForCommitMessage escapes \n, \r, \t, and other C0 controls. New tests: - TestCheckTSIG_BadStatus_Refused (H6) - TestUpdate_DeleteRR_IgnoresTTL (M3) - TestSanitizeForCommitMessage (M7)
445 lines
15 KiB
Go
445 lines
15 KiB
Go
package rfc2136
|
|
|
|
import (
|
|
"net"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/miekg/dns"
|
|
)
|
|
|
|
// captureWriter implements dns.ResponseWriter and stashes the message
|
|
// passed to WriteMsg so tests can inspect it after handleUpdate returns.
|
|
//
|
|
// tsigErr, if non-nil, is what TsigStatus() returns — letting tests
|
|
// simulate TSIG-verification failure (bad MAC, fudge-window violation,
|
|
// unknown key etc.).
|
|
type captureWriter struct {
|
|
msg *dns.Msg
|
|
tsigErr error
|
|
}
|
|
|
|
func (cw *captureWriter) WriteMsg(m *dns.Msg) error { cw.msg = m; return nil }
|
|
func (cw *captureWriter) Write([]byte) (int, error) { return 0, nil }
|
|
func (cw *captureWriter) Close() error { return nil }
|
|
func (cw *captureWriter) TsigStatus() error { return cw.tsigErr }
|
|
func (cw *captureWriter) TsigTimersOnly(bool) {}
|
|
func (cw *captureWriter) Hijack() {}
|
|
func (cw *captureWriter) LocalAddr() net.Addr { return &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)} }
|
|
func (cw *captureWriter) RemoteAddr() net.Addr { return &net.IPAddr{IP: net.IPv4(127, 0, 0, 1)} }
|
|
func (cw *captureWriter) Network() string { return "udp" }
|
|
|
|
// newTestPluginWithZone builds an RFC2136 backed by a temp zones-dir.
|
|
// AutoCommit is disabled by default (tests don't need git side-effects).
|
|
func newTestPluginWithZone(t *testing.T, zone string) *RFC2136 {
|
|
t.Helper()
|
|
dir := withTempZonesDir(t, zone)
|
|
zone = dns.Fqdn(zone)
|
|
|
|
p := &RFC2136{
|
|
Zones: []string{zone},
|
|
TTL: 60,
|
|
ZonesDir: dir,
|
|
zones: map[string]*zoneFile{
|
|
zone: openZoneFile(filepath.Join(dir, strings.TrimSuffix(zone, ".")+".zone"), zone),
|
|
},
|
|
}
|
|
p.zones[zone].AutoCommit = false
|
|
return p
|
|
}
|
|
|
|
// newUpdate builds an UPDATE message for `zone` with zero prereqs and
|
|
// the given update RRs. Matches what caddy-dns/rfc2136 sends.
|
|
func newUpdate(zone string, updates ...dns.RR) *dns.Msg {
|
|
m := new(dns.Msg)
|
|
m.SetUpdate(dns.Fqdn(zone))
|
|
m.Ns = append(m.Ns, updates...)
|
|
return m
|
|
}
|
|
|
|
// runUpdate sends msg through the handler with TSIG auth bypassed
|
|
// (calling handleUpdate directly with verified=true instead of
|
|
// ServeDNS). Tests that exercise post-auth logic use this; tests that
|
|
// assert auth-failure behavior should call handleUpdate(w, msg, false)
|
|
// directly.
|
|
func runUpdate(t *testing.T, p *RFC2136, msg *dns.Msg) (rcode int) {
|
|
t.Helper()
|
|
w := &captureWriter{}
|
|
rcode, _ = p.handleUpdate(w, msg, true)
|
|
return rcode
|
|
}
|
|
|
|
// readZoneRecords loads the zone file and returns the RRs for inspection.
|
|
func readZoneRecords(t *testing.T, p *RFC2136, zone string) []dns.RR {
|
|
t.Helper()
|
|
zf := p.zones[dns.Fqdn(zone)]
|
|
rrs, _, err := zf.loadRRs()
|
|
if err != nil {
|
|
t.Fatalf("loadRRs: %v", err)
|
|
}
|
|
return rrs
|
|
}
|
|
|
|
func TestUpdate_AddSingleTXT_PersistsToFile(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
upd := newUpdate("auth.example.com",
|
|
mustRR(t, `token-1.auth.example.com. 60 IN TXT "validation-1"`),
|
|
)
|
|
if rcode := runUpdate(t, p, upd); rcode != dns.RcodeSuccess {
|
|
t.Fatalf("rcode = %d, want NOERROR", rcode)
|
|
}
|
|
|
|
// Verify by re-reading the file.
|
|
rrs := readZoneRecords(t, p, "auth.example.com")
|
|
for _, rr := range rrs {
|
|
if txt, ok := rr.(*dns.TXT); ok && txt.Hdr.Name == "token-1.auth.example.com." {
|
|
if txt.Txt[0] == "validation-1" {
|
|
return // success
|
|
}
|
|
}
|
|
}
|
|
t.Errorf("added TXT not found in re-read zone file")
|
|
}
|
|
|
|
func TestUpdate_AddBumpsSerial(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
before := readZoneRecords(t, p, "auth.example.com")
|
|
var beforeSerial uint32
|
|
for _, rr := range before {
|
|
if soa, ok := rr.(*dns.SOA); ok {
|
|
beforeSerial = soa.Serial
|
|
break
|
|
}
|
|
}
|
|
|
|
upd := newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`),
|
|
)
|
|
runUpdate(t, p, upd)
|
|
|
|
after := readZoneRecords(t, p, "auth.example.com")
|
|
var afterSerial uint32
|
|
for _, rr := range after {
|
|
if soa, ok := rr.(*dns.SOA); ok {
|
|
afterSerial = soa.Serial
|
|
break
|
|
}
|
|
}
|
|
if afterSerial <= beforeSerial {
|
|
t.Errorf("Serial did not advance: before=%d after=%d", beforeSerial, afterSerial)
|
|
}
|
|
}
|
|
|
|
func TestUpdate_DeleteRRset_RemovesAllOfType(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
// First add two TXTs at the same name.
|
|
runUpdate(t, p, newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "a"`),
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "b"`),
|
|
))
|
|
|
|
// Now delete the whole TXT RRset.
|
|
del := &dns.ANY{Hdr: dns.RR_Header{
|
|
Name: "foo.auth.example.com.",
|
|
Rrtype: dns.TypeTXT,
|
|
Class: dns.ClassANY,
|
|
}}
|
|
if rcode := runUpdate(t, p, newUpdate("auth.example.com", del)); rcode != dns.RcodeSuccess {
|
|
t.Fatalf("delete rcode = %d", rcode)
|
|
}
|
|
|
|
for _, rr := range readZoneRecords(t, p, "auth.example.com") {
|
|
if rr.Header().Rrtype == dns.TypeTXT && rr.Header().Name == "foo.auth.example.com." {
|
|
t.Errorf("TXT %s should be gone, still present: %s", rr.Header().Name, rr.String())
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestCheckTSIG_BadStatus_Refused covers H6 — the path where miekg/dns
|
|
// has reported a TSIG verification failure (bad MAC, fudge-window
|
|
// violation, expired timestamp, etc.) via dns.ResponseWriter.TsigStatus.
|
|
// checkTSIG must surface this as an error so ServeDNS refuses the
|
|
// UPDATE. This is the test that would catch a regression in our
|
|
// reliance on miekg/dns's default fudge window.
|
|
func TestCheckTSIG_BadStatus_Refused(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
// Configure a key so checkTSIG doesn't short-circuit on "no keys".
|
|
p.TSIGKeys = map[string]tsigKey{
|
|
"acme-update-key.": {Algorithm: dns.HmacSHA256, Secret: []byte("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")},
|
|
}
|
|
upd := newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`),
|
|
)
|
|
upd.SetTsig("acme-update-key.", dns.HmacSHA256, 300, 0)
|
|
|
|
w := &captureWriter{
|
|
// Simulate miekg/dns reporting "TSIG outside fudge window."
|
|
tsigErr: dns.ErrTime,
|
|
}
|
|
if err := p.checkTSIG(w, upd); err == nil {
|
|
t.Errorf("checkTSIG accepted request despite TsigStatus reporting %v", dns.ErrTime)
|
|
}
|
|
}
|
|
|
|
// TestUpdate_UnverifiedCaller_Refused proves the C2 defense-in-depth
|
|
// contract: handleUpdate refuses any call that doesn't assert TSIG
|
|
// verification, even if the rest of the message is well-formed and the
|
|
// zone is authoritative. This guards against a future internal caller
|
|
// that bypasses the wire-level TSIG check in ServeDNS.
|
|
func TestUpdate_UnverifiedCaller_Refused(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
upd := newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "should-not-apply"`),
|
|
)
|
|
w := &captureWriter{}
|
|
rcode, _ := p.handleUpdate(w, upd, false) // <- the security boundary
|
|
if rcode != dns.RcodeRefused {
|
|
t.Errorf("rcode = %d, want REFUSED for unverified caller", rcode)
|
|
}
|
|
// Zone file must NOT have been modified.
|
|
for _, rr := range readZoneRecords(t, p, "auth.example.com") {
|
|
if txt, ok := rr.(*dns.TXT); ok && txt.Hdr.Name == "foo.auth.example.com." {
|
|
t.Errorf("unverified UPDATE applied state mutation: %s", rr.String())
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestUpdate_DeleteRR_IgnoresTTL covers M3: an UPDATE's delete-RR
|
|
// specifies a different TTL than the stored RR, but per RFC 2136
|
|
// §3.4.2.4 the deletion must match by owner/class/type/rdata only.
|
|
// The old String()-based comparison would silently fail to match.
|
|
func TestUpdate_DeleteRR_IgnoresTTL(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
// Add a TXT with TTL 60.
|
|
runUpdate(t, p, newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "match-me"`),
|
|
))
|
|
// Issue a CLASS=NONE delete with a different TTL (per RFC 2136
|
|
// §2.5.4, the TTL on a CLASS=NONE delete is supposed to be 0; some
|
|
// clients get this wrong, and even when they get it right, an
|
|
// implementation must ignore the value).
|
|
delRR := mustRR(t, `foo.auth.example.com. 0 IN TXT "match-me"`)
|
|
delRR.Header().Class = dns.ClassNONE
|
|
if rcode := runUpdate(t, p, newUpdate("auth.example.com", delRR)); rcode != dns.RcodeSuccess {
|
|
t.Fatalf("rcode = %d, want NOERROR", rcode)
|
|
}
|
|
for _, rr := range readZoneRecords(t, p, "auth.example.com") {
|
|
if txt, ok := rr.(*dns.TXT); ok && txt.Hdr.Name == "foo.auth.example.com." && txt.Txt[0] == "match-me" {
|
|
t.Errorf("TTL-differing delete did not remove the RR: %s", rr.String())
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestSanitizeForCommitMessage covers M7: attacker-controlled RR names
|
|
// (TSIG only authenticates the sender; the payload is hostile by
|
|
// default) must not inject control characters into commit messages or
|
|
// downstream log renderers.
|
|
func TestSanitizeForCommitMessage(t *testing.T) {
|
|
cases := []struct {
|
|
in, want string
|
|
}{
|
|
{"plain", "plain"},
|
|
{"with\nnewline", "with\\nnewline"},
|
|
{"tab\there", "tab\\there"},
|
|
{"esc\x1b[31mred\x1b[0m", "esc\\x1b[31mred\\x1b[0m"},
|
|
{"del\x7fchar", "del\\x7fchar"},
|
|
}
|
|
for _, tc := range cases {
|
|
got := sanitizeForCommitMessage(tc.in)
|
|
if got != tc.want {
|
|
t.Errorf("sanitize(%q) = %q, want %q", tc.in, got, tc.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestUpdate_OutOfZone_Refused(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
upd := newUpdate("auth.example.com",
|
|
mustRR(t, `evil.other.tld. 60 IN TXT "nope"`),
|
|
)
|
|
if rcode := runUpdate(t, p, upd); rcode != dns.RcodeNotZone {
|
|
t.Errorf("rcode = %d, want NOTZONE (%d)", rcode, dns.RcodeNotZone)
|
|
}
|
|
}
|
|
|
|
func TestUpdate_UnknownZone_NOTAUTH(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
// Build UPDATE for a zone we don't manage.
|
|
m := new(dns.Msg)
|
|
m.SetUpdate("other.zone.")
|
|
m.Ns = []dns.RR{mustRR(t, `x.other.zone. 60 IN TXT "y"`)}
|
|
|
|
if rcode := runUpdate(t, p, m); rcode != dns.RcodeNotAuth {
|
|
t.Errorf("rcode = %d, want NOTAUTH (%d)", rcode, dns.RcodeNotAuth)
|
|
}
|
|
}
|
|
|
|
func TestUpdate_ApexSOADeletion_Refused(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
// CLASS=ANY type=SOA at apex → would wipe the SOA.
|
|
del := &dns.ANY{Hdr: dns.RR_Header{
|
|
Name: "auth.example.com.",
|
|
Rrtype: dns.TypeSOA,
|
|
Class: dns.ClassANY,
|
|
}}
|
|
if rcode := runUpdate(t, p, newUpdate("auth.example.com", del)); rcode != dns.RcodeRefused {
|
|
t.Errorf("rcode = %d, want REFUSED", rcode)
|
|
}
|
|
|
|
// SOA must still be in the file.
|
|
hasSOA := false
|
|
for _, rr := range readZoneRecords(t, p, "auth.example.com") {
|
|
if _, ok := rr.(*dns.SOA); ok {
|
|
hasSOA = true
|
|
break
|
|
}
|
|
}
|
|
if !hasSOA {
|
|
t.Errorf("SOA was deleted despite REFUSED rcode")
|
|
}
|
|
}
|
|
|
|
func TestUpdate_ApexWipe_Refused(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
del := &dns.ANY{Hdr: dns.RR_Header{
|
|
Name: "auth.example.com.",
|
|
Rrtype: dns.TypeANY,
|
|
Class: dns.ClassANY,
|
|
}}
|
|
if rcode := runUpdate(t, p, newUpdate("auth.example.com", del)); rcode != dns.RcodeRefused {
|
|
t.Errorf("rcode = %d, want REFUSED", rcode)
|
|
}
|
|
}
|
|
|
|
func TestUpdate_PrereqRRsetMustNotExist_YXRRSET(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
// Seed a TXT then send an UPDATE whose prereq says "no TXT here".
|
|
runUpdate(t, p, newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "present"`),
|
|
))
|
|
|
|
prereq := &dns.ANY{Hdr: dns.RR_Header{
|
|
Name: "foo.auth.example.com.",
|
|
Rrtype: dns.TypeTXT,
|
|
Class: dns.ClassNONE,
|
|
}}
|
|
m := new(dns.Msg)
|
|
m.SetUpdate("auth.example.com.")
|
|
m.Answer = []dns.RR{prereq}
|
|
m.Ns = []dns.RR{mustRR(t, `foo.auth.example.com. 60 IN TXT "should-not-be-added"`)}
|
|
|
|
if rcode := runUpdate(t, p, m); rcode != dns.RcodeYXRrset {
|
|
t.Errorf("rcode = %d, want YXRRSET", rcode)
|
|
}
|
|
}
|
|
|
|
func TestUpdate_NoOpAdd_DoesntRewriteFile(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
// Add a record once.
|
|
runUpdate(t, p, newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`),
|
|
))
|
|
|
|
path := p.zones["auth.example.com."].Path
|
|
beforeStat, _ := os.Stat(path)
|
|
|
|
// Same UPDATE again — should be a no-op (dedupe).
|
|
runUpdate(t, p, newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`),
|
|
))
|
|
|
|
afterStat, _ := os.Stat(path)
|
|
if afterStat.ModTime().After(beforeStat.ModTime()) {
|
|
t.Errorf("file was rewritten despite no-op update (mtime advanced)")
|
|
}
|
|
}
|
|
|
|
// TestUpdate_SignedRequest_ResponseGetsTSIG verifies the RFC 8945 §5.4.2
|
|
// requirement that a server's response to a TSIG-signed request must
|
|
// itself carry TSIG so the client can authenticate the answer. Before
|
|
// this fix, BIND nsupdate complained "expected a TSIG or SIG(0)" because
|
|
// the response went back without any TSIG record attached.
|
|
func TestUpdate_SignedRequest_ResponseGetsTSIG(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
upd := newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`),
|
|
)
|
|
// Attach a TSIG to the request — the values mimic what nsupdate
|
|
// or caddy-dns/rfc2136 emit. handleUpdate runs WITHOUT MAC
|
|
// verification in this test (that's ServeDNS's job and is
|
|
// covered separately), so the TSIG record's MAC contents don't
|
|
// matter — only the presence/algorithm/name do.
|
|
upd.SetTsig("acme-update-key.", dns.HmacSHA256, 300, 0)
|
|
|
|
w := &captureWriter{}
|
|
if _, err := p.handleUpdate(w, upd, true); err != nil {
|
|
t.Fatalf("handleUpdate: %v", err)
|
|
}
|
|
if w.msg == nil {
|
|
t.Fatal("response was not written")
|
|
}
|
|
got := w.msg.IsTsig()
|
|
if got == nil {
|
|
t.Fatal("response missing TSIG — nsupdate would reject as 'expected a TSIG or SIG(0)'")
|
|
}
|
|
if !strings.EqualFold(got.Hdr.Name, "acme-update-key.") {
|
|
t.Errorf("response TSIG key = %q, want acme-update-key.", got.Hdr.Name)
|
|
}
|
|
if !strings.EqualFold(got.Algorithm, dns.HmacSHA256) {
|
|
t.Errorf("response TSIG alg = %q, want %s", got.Algorithm, dns.HmacSHA256)
|
|
}
|
|
}
|
|
|
|
// TestUpdate_UnsignedRequest_ResponseStaysUnsigned guards the no-op
|
|
// branch — if the client didn't sign, we don't synthesize a TSIG on
|
|
// the response (we couldn't pick a key anyway).
|
|
func TestUpdate_UnsignedRequest_ResponseStaysUnsigned(t *testing.T) {
|
|
p := newTestPluginWithZone(t, "auth.example.com")
|
|
|
|
upd := newUpdate("auth.example.com",
|
|
mustRR(t, `foo.auth.example.com. 60 IN TXT "x"`),
|
|
)
|
|
// No SetTsig — request is plain.
|
|
|
|
w := &captureWriter{}
|
|
if _, err := p.handleUpdate(w, upd, true); err != nil {
|
|
t.Fatalf("handleUpdate: %v", err)
|
|
}
|
|
if w.msg.IsTsig() != nil {
|
|
t.Error("response carries TSIG despite unsigned request")
|
|
}
|
|
}
|
|
|
|
func TestFindZone_LongestSuffixWins(t *testing.T) {
|
|
p := &RFC2136{Zones: []string{"example.com.", "auth.example.com."}}
|
|
if got := p.findZone("foo.auth.example.com."); got != "auth.example.com." {
|
|
t.Errorf("findZone returned %q, expected longest match", got)
|
|
}
|
|
}
|
|
|
|
func TestFindZone_OutsideAllZones(t *testing.T) {
|
|
p := &RFC2136{Zones: []string{"auth.example.com."}}
|
|
if got := p.findZone("other.tld."); got != "" {
|
|
t.Errorf("findZone returned %q, want empty", got)
|
|
}
|
|
}
|
|
|
|
func TestFindZone_CaseInsensitive(t *testing.T) {
|
|
p := &RFC2136{Zones: []string{"auth.example.com."}}
|
|
if got := p.findZone("Foo.AUTH.example.COM."); got != "auth.example.com." {
|
|
t.Errorf("case-insensitive findZone returned %q", got)
|
|
}
|
|
}
|