From eba6313ec0d942ef8dbbf3d62b86db8d56e2e8e2 Mon Sep 17 00:00:00 2001 From: Ryan Malloy Date: Thu, 21 May 2026 10:31:22 -0600 Subject: [PATCH] =?UTF-8?q?Phase=201.2:=20wire=20parser=20=E2=86=92=20type?= =?UTF-8?q?d=20config=20+=2013=20unit=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Corefile parser now fully populates typed fields on RFC2136 instead of just recognising directives. Validation happens at parse-time so configuration errors fail loud at CoreDNS startup rather than silent at request time. Added: - config.go: tsigKey type, TSIG algorithm allowlist (rejects HMAC-MD5 deliberately), base64 secret decoder with 8-byte minimum length check, canonical-key-name normalisation (lowercase + trailing dot). - plugin.go: RFC2136 struct now carries TSIGKeys map, TTL uint32, PersistPath string. DefaultTTL=60. - setup.go: parse() validates and stores tsig-key/ttl/persist directives. Duplicate key names rejected. Multiple TSIG keys allowed (for rotation). At-least-one-zone is enforced. - setup_test.go: 13 table-driven cases (5 happy + 8 error paths) using caddy.NewTestController. All pass. ServeDNS still passes through — UPDATE handling lands in Phase 1.4. Module path: git.supported.systems/rsp2k/coredns-rfc2136 --- README.md | 4 +- config.go | 78 +++++++++++++++++++ go.mod | 2 +- plugin.go | 41 +++++++--- setup.go | 59 ++++++++++---- setup_test.go | 209 ++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 364 insertions(+), 29 deletions(-) create mode 100644 config.go create mode 100644 setup_test.go diff --git a/README.md b/README.md index be46c29..f594c7a 100644 --- a/README.md +++ b/README.md @@ -66,10 +66,10 @@ This plugin is consumed by a custom CoreDNS build via `plugin.cfg`: ``` # In CoreDNS source's plugin.cfg, BEFORE the `cache` plugin: -rfc2136:git.supportedsystems.net/rpm/coredns-rfc2136 +rfc2136:git.supported.systems/rsp2k/coredns-rfc2136 ``` -Then `go get git.supportedsystems.net/rpm/coredns-rfc2136 && make`. +Then `go get git.supported.systems/rsp2k/coredns-rfc2136 && make`. ## License diff --git a/config.go b/config.go new file mode 100644 index 0000000..41e21c7 --- /dev/null +++ b/config.go @@ -0,0 +1,78 @@ +package rfc2136 + +import ( + "encoding/base64" + "fmt" + "strings" + + "github.com/miekg/dns" +) + +// tsigKey is a single pre-shared TSIG credential. The plugin holds one +// per declared `tsig-key` directive, keyed in RFC2136.TSIGKeys by the +// canonical (lowercased, trailing-dot) key name. +// +// Phase 1.2 stores the algorithm and decoded secret. Phase 1.4 will +// use them for actual signature verification via dns.TsigSecret + +// dns.Msg.IsTsig() / dns.TsigVerify(). +type tsigKey struct { + // Algorithm is the canonical miekg/dns algorithm identifier, e.g. + // dns.HmacSHA256 ("hmac-sha256."). Stored in dns-library form so + // it's directly usable when wiring TSIG verification later. + Algorithm string + + // Secret is the base64-decoded raw bytes of the shared secret. + // Decoding at parse time means a malformed secret fails Corefile + // load (loud), not mid-query (silent). + Secret []byte +} + +// supportedTSIGAlgorithms maps user-facing algorithm names (as written +// in Corefile) to the canonical names recognised by miekg/dns. Only +// SHA-family entries are included — HMAC-MD5 is deprecated and not +// supported here on principle. +var supportedTSIGAlgorithms = map[string]string{ + "hmac-sha1": dns.HmacSHA1, + "hmac-sha224": dns.HmacSHA224, + "hmac-sha256": dns.HmacSHA256, + "hmac-sha384": dns.HmacSHA384, + "hmac-sha512": dns.HmacSHA512, +} + +// parseTSIGAlgorithm validates and canonicalises the algorithm name +// from a `tsig-key` directive. Returns the miekg/dns canonical form +// (which has a trailing dot per DNS-name convention). +func parseTSIGAlgorithm(s string) (string, error) { + canon, ok := supportedTSIGAlgorithms[strings.ToLower(s)] + if !ok { + return "", fmt.Errorf("unsupported TSIG algorithm %q (supported: hmac-sha1, hmac-sha224, hmac-sha256, hmac-sha384, hmac-sha512)", s) + } + return canon, nil +} + +// decodeTSIGSecret validates the base64-encoded TSIG secret from the +// Corefile and returns the raw bytes. Rejects empty secrets and any +// secret shorter than 8 bytes (well under the HMAC algorithm's block +// size for any supported algorithm — anything that short is almost +// certainly a typo, not deliberate). +func decodeTSIGSecret(s string) ([]byte, error) { + raw, err := base64.StdEncoding.DecodeString(s) + if err != nil { + return nil, fmt.Errorf("invalid base64 in TSIG secret: %w", err) + } + if len(raw) < 8 { + return nil, fmt.Errorf("TSIG secret too short (%d bytes after base64-decode; need >= 8)", len(raw)) + } + return raw, nil +} + +// canonicalKeyName normalises a TSIG key name to lowercase + trailing +// dot. miekg/dns expects the dot-terminated FQDN form internally, so +// we coerce here once at parse-time rather than every lookup. +func canonicalKeyName(name string) string { + name = strings.ToLower(name) + if !strings.HasSuffix(name, ".") { + name += "." + } + return name +} diff --git a/go.mod b/go.mod index 04bef8c..cf2e833 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module git.supportedsystems.net/rpm/coredns-rfc2136 +module git.supported.systems/rsp2k/coredns-rfc2136 go 1.25.0 diff --git a/plugin.go b/plugin.go index ae87aad..c2498de 100644 --- a/plugin.go +++ b/plugin.go @@ -6,7 +6,7 @@ // // Scope: // - Handles UPDATE messages (OPCODE=5) for configured zones. -// - Verifies TSIG signatures (HMAC-SHA256 today; algorithm-pluggable). +// - Verifies TSIG signatures (HMAC-SHA family; algorithm-pluggable). // - Stores records in memory; optional periodic snapshot to disk. // - Serves queries (SOA, NS, A, AAAA, TXT) for the configured zone // from the in-memory store plus a synthetic SOA/NS apex. @@ -15,10 +15,11 @@ // - General-purpose authoritative DNS (use `auto`/`file` for that). // - DNSSEC signing (add later via the `dnssec` plugin in front). // -// Phase 1 status: skeleton only. ServeDNS passes through to the next -// plugin; setup parses the Corefile but does not yet do anything with -// the parsed configuration. See plan at -// ~/.claude/plans/dood-does-coredns-offer-enumerated-piglet.md +// Phase 1.2 status: parser fully wires Corefile into typed config. +// ServeDNS still passes through to the next plugin — UPDATE handling +// and zone-serving land in Phase 1.3/1.4. See plan at +// +// ~/.claude/plans/dood-does-coredns-offer-enumerated-piglet.md package rfc2136 import ( @@ -28,6 +29,12 @@ import ( "github.com/miekg/dns" ) +// DefaultTTL is the TTL applied to dynamically-added records when the +// Corefile doesn't specify one. 60s matches the short-lived nature of +// ACME challenge TXT records and keeps stale answers from lingering in +// resolver caches. +const DefaultTTL uint32 = 60 + // RFC2136 is the plugin handler. One instance per Corefile server block. type RFC2136 struct { // Next is the downstream plugin in the chain. @@ -38,18 +45,28 @@ type RFC2136 struct { // zones pass through to Next. Zones []string - // Phase 2 will add: - // - tsigKeys map[string]tsigKey - // - store *recordStore - // - ttl uint32 + // TSIGKeys is keyed by canonical key name (lowercased, trailing + // dot). Empty means TSIG is disabled — UPDATEs without TSIG would + // be rejected unconditionally in Phase 1.4. + TSIGKeys map[string]tsigKey + + // TTL is applied to dynamically-injected records that don't carry + // an explicit TTL in the UPDATE message. + TTL uint32 + + // PersistPath, when non-empty, names a file the plugin writes a + // JSON snapshot of its in-memory store to on a periodic schedule. + // Empty means in-memory only (acceptable for ACME challenges, + // which are seconds-to-minutes lived and re-issued on restart). + PersistPath string } // Name implements plugin.Handler. func (p *RFC2136) Name() string { return "rfc2136" } -// ServeDNS implements plugin.Handler. Phase 1 is a pass-through so the -// plugin can register, parse config, and live in the chain without -// changing behavior. Phase 2 wires the UPDATE handler and zone serving. +// ServeDNS implements plugin.Handler. Phase 1.x is a pass-through so +// the plugin can register, parse config, and live in the chain without +// changing behavior. Phase 1.3 wires UPDATE handling + query serving. func (p *RFC2136) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { return plugin.NextOrFailure(p.Name(), p.Next, ctx, w, r) } diff --git a/setup.go b/setup.go index e3be760..958d980 100644 --- a/setup.go +++ b/setup.go @@ -1,6 +1,8 @@ package rfc2136 import ( + "strconv" + "github.com/coredns/caddy" "github.com/coredns/coredns/core/dnsserver" "github.com/coredns/coredns/plugin" @@ -28,22 +30,28 @@ func setup(c *caddy.Controller) error { return p }) - log.Infof("registered for zones: %v", p.Zones) + log.Infof("registered for zones=%v keys=%d ttl=%d persist=%q", + p.Zones, len(p.TSIGKeys), p.TTL, p.PersistPath) return nil } -// parse reads a single `rfc2136 { ... }` block from the Corefile. +// parse reads a single `rfc2136 [...] { ... }` block from +// the Corefile and returns a fully-populated RFC2136 handler with all +// values validated at parse time (so configuration errors fail fast at +// CoreDNS startup, not later mid-request). // -// Phase 1 grammar (only the surface is parsed; sub-directives are -// accepted but ignored — Phase 2 wires them): +// Grammar: // -// rfc2136 { -// tsig-key -// ttl -// persist +// rfc2136 [...] { +// tsig-key ; may repeat +// ttl ; default 60 +// persist ; default off (in-memory only) // } func parse(c *caddy.Controller) (*RFC2136, error) { - p := &RFC2136{} + p := &RFC2136{ + TSIGKeys: make(map[string]tsigKey), + TTL: DefaultTTL, + } for c.Next() { args := c.RemainingArgs() @@ -59,28 +67,47 @@ func parse(c *caddy.Controller) (*RFC2136, error) { for c.NextBlock() { switch c.Val() { + case "tsig-key": + // tsig-key kArgs := c.RemainingArgs() if len(kArgs) != 3 { return nil, c.Errf("tsig-key requires 3 args (name algorithm secret), got %d", len(kArgs)) } - // Phase 2: store in p.tsigKeys[name] = tsigKey{algo, secret} - log.Debugf("tsig-key parsed (storage NYI): name=%s alg=%s", kArgs[0], kArgs[1]) + keyName := canonicalKeyName(kArgs[0]) + algo, err := parseTSIGAlgorithm(kArgs[1]) + if err != nil { + return nil, c.Err(err.Error()) + } + secret, err := decodeTSIGSecret(kArgs[2]) + if err != nil { + return nil, c.Errf("tsig-key %q: %v", keyName, err) + } + if _, exists := p.TSIGKeys[keyName]; exists { + return nil, c.Errf("duplicate tsig-key %q", keyName) + } + p.TSIGKeys[keyName] = tsigKey{Algorithm: algo, Secret: secret} case "ttl": tArgs := c.RemainingArgs() if len(tArgs) != 1 { return nil, c.ArgErr() } - // Phase 2: parse uint32, validate range, store in p.ttl - log.Debugf("ttl parsed (storage NYI): %s", tArgs[0]) + ttl, err := strconv.ParseUint(tArgs[0], 10, 32) + if err != nil { + return nil, c.Errf("ttl must be a non-negative integer: %v", err) + } + // Anything over a week is almost certainly a mistake + // for ACME challenge records, but allow up to the + // uint32 max so we don't ship an arbitrary cap. + p.TTL = uint32(ttl) case "persist": pArgs := c.RemainingArgs() if len(pArgs) != 1 { return nil, c.ArgErr() } - log.Debugf("persist parsed (storage NYI): %s", pArgs[0]) + p.PersistPath = pArgs[0] default: return nil, c.Errf("unknown directive: %s", c.Val()) @@ -88,5 +115,9 @@ func parse(c *caddy.Controller) (*RFC2136, error) { } } + if len(p.Zones) == 0 { + return nil, c.Err("at least one zone must be specified") + } + return p, nil } diff --git a/setup_test.go b/setup_test.go new file mode 100644 index 0000000..0fd1008 --- /dev/null +++ b/setup_test.go @@ -0,0 +1,209 @@ +package rfc2136 + +import ( + "strings" + "testing" + + "github.com/coredns/caddy" + "github.com/miekg/dns" +) + +// A 32-byte HMAC-SHA256 secret, base64-encoded. Generated once and +// re-used across tests so failures are reproducible. NEVER use this +// value in production — it's literally in this file in plaintext. +const testSecret = "xTgset4zj7kHqniSslYFn+OcdCf419olek9MNmOvlUM=" + +// TestParse exercises the Corefile parser across the matrix of valid +// and invalid configurations. Each row is fully self-contained. +func TestParse(t *testing.T) { + tests := []struct { + name string + input string + shouldErr bool + errMatch string // substring to look for in the error (when shouldErr) + check func(t *testing.T, p *RFC2136) + }{ + { + name: "single zone, no block", + input: `rfc2136 auth.example.com.`, + check: func(t *testing.T, p *RFC2136) { + wantZone(t, p, "auth.example.com.") + if p.TTL != DefaultTTL { + t.Errorf("TTL = %d, want default %d", p.TTL, DefaultTTL) + } + if len(p.TSIGKeys) != 0 { + t.Errorf("expected 0 TSIG keys, got %d", len(p.TSIGKeys)) + } + }, + }, + { + name: "multiple zones in one directive", + input: `rfc2136 auth.example.com. acme.example.org.`, + check: func(t *testing.T, p *RFC2136) { + if len(p.Zones) != 2 { + t.Fatalf("Zones = %v, want 2", p.Zones) + } + wantZone(t, p, "auth.example.com.") + wantZone(t, p, "acme.example.org.") + }, + }, + { + name: "full block with all directives", + input: `rfc2136 auth.example.com. { + tsig-key acme-key. hmac-sha256 ` + testSecret + ` + ttl 120 + persist /var/lib/coredns/rfc2136/auth.db + }`, + check: func(t *testing.T, p *RFC2136) { + wantZone(t, p, "auth.example.com.") + if p.TTL != 120 { + t.Errorf("TTL = %d, want 120", p.TTL) + } + if p.PersistPath != "/var/lib/coredns/rfc2136/auth.db" { + t.Errorf("PersistPath = %q", p.PersistPath) + } + k, ok := p.TSIGKeys["acme-key."] + if !ok { + t.Fatalf("acme-key. not in TSIGKeys; have keys=%v", keysOf(p.TSIGKeys)) + } + if k.Algorithm != dns.HmacSHA256 { + t.Errorf("Algorithm = %q, want %q", k.Algorithm, dns.HmacSHA256) + } + if len(k.Secret) != 32 { + t.Errorf("Secret length = %d, want 32 (decoded SHA-256-sized)", len(k.Secret)) + } + }, + }, + { + name: "tsig-key name normalised to trailing-dot lowercase", + input: `rfc2136 auth.example.com. { + tsig-key Acme-Key hmac-sha256 ` + testSecret + ` + }`, + check: func(t *testing.T, p *RFC2136) { + if _, ok := p.TSIGKeys["acme-key."]; !ok { + t.Errorf("expected canonicalised key 'acme-key.', got keys=%v", keysOf(p.TSIGKeys)) + } + }, + }, + { + name: "multiple tsig-keys allowed", + input: `rfc2136 auth.example.com. { + tsig-key key-a. hmac-sha256 ` + testSecret + ` + tsig-key key-b. hmac-sha512 ` + testSecret + ` + }`, + check: func(t *testing.T, p *RFC2136) { + if len(p.TSIGKeys) != 2 { + t.Errorf("expected 2 keys, got %d (%v)", len(p.TSIGKeys), keysOf(p.TSIGKeys)) + } + if p.TSIGKeys["key-a."].Algorithm != dns.HmacSHA256 { + t.Errorf("key-a algorithm wrong") + } + if p.TSIGKeys["key-b."].Algorithm != dns.HmacSHA512 { + t.Errorf("key-b algorithm wrong") + } + }, + }, + + // ─── error cases ────────────────────────────────────────── + { + name: "no zone", + input: `rfc2136`, + shouldErr: true, + errMatch: "Wrong argument count", + }, + { + name: "unknown directive", + input: `rfc2136 auth.example.com. { bogus value }`, + shouldErr: true, + errMatch: "unknown directive", + }, + { + name: "tsig-key with too few args", + input: `rfc2136 auth.example.com. { + tsig-key only-name + }`, + shouldErr: true, + errMatch: "tsig-key requires 3 args", + }, + { + name: "unsupported TSIG algorithm", + input: `rfc2136 auth.example.com. { + tsig-key key. hmac-md5 ` + testSecret + ` + }`, + shouldErr: true, + errMatch: "unsupported TSIG algorithm", + }, + { + name: "malformed base64 secret", + input: `rfc2136 auth.example.com. { + tsig-key key. hmac-sha256 not_base64_at_all!!! + }`, + shouldErr: true, + errMatch: "invalid base64", + }, + { + name: "secret too short after decode", + input: `rfc2136 auth.example.com. { + tsig-key key. hmac-sha256 c2hvcnQ= + }`, // "short" → 5 bytes < 8 min + shouldErr: true, + errMatch: "too short", + }, + { + name: "duplicate tsig-key name", + input: `rfc2136 auth.example.com. { + tsig-key dup. hmac-sha256 ` + testSecret + ` + tsig-key dup. hmac-sha512 ` + testSecret + ` + }`, + shouldErr: true, + errMatch: "duplicate tsig-key", + }, + { + name: "ttl non-numeric", + input: `rfc2136 auth.example.com. { + ttl not-a-number + }`, + shouldErr: true, + errMatch: "ttl must be a non-negative integer", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := caddy.NewTestController("dns", tt.input) + p, err := parse(c) + if (err != nil) != tt.shouldErr { + t.Fatalf("parse() err = %v, shouldErr = %v", err, tt.shouldErr) + } + if err != nil { + if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { + t.Errorf("err = %q, expected substring %q", err.Error(), tt.errMatch) + } + return + } + if tt.check != nil { + tt.check(t, p) + } + }) + } +} + +// wantZone asserts that p.Zones contains the expected canonical zone. +func wantZone(t *testing.T, p *RFC2136, want string) { + t.Helper() + for _, z := range p.Zones { + if z == want { + return + } + } + t.Errorf("zone %q not in p.Zones=%v", want, p.Zones) +} + +// keysOf is a debug helper for failure messages. +func keysOf(m map[string]tsigKey) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + return out +}