diff options
author | 2022-06-27 15:48:34 -0400 | |
---|---|---|
committer | 2022-06-27 15:48:34 -0400 | |
commit | 68e141eff28d2b0d6331684ef153d76902b4001c (patch) | |
tree | 32921a733b6851b8cabaef84d34fae27e5bc27e7 | |
parent | 64885950cc8ab59d26ae1df56e94a9f43e439787 (diff) | |
download | coredns-68e141eff28d2b0d6331684ef153d76902b4001c.tar.gz coredns-68e141eff28d2b0d6331684ef153d76902b4001c.tar.zst coredns-68e141eff28d2b0d6331684ef153d76902b4001c.zip |
plugin/tsig: new plugin TSIG (#4957)
* expose tsig secrets via dnsserver.Config
* add tsig plugin
Signed-off-by: Chris O'Haver <cohaver@infoblox.com>
-rw-r--r-- | core/dnsserver/config.go | 3 | ||||
-rw-r--r-- | core/dnsserver/register.go | 1 | ||||
-rw-r--r-- | core/dnsserver/server.go | 12 | ||||
-rw-r--r-- | core/dnsserver/zdirectives.go | 1 | ||||
-rw-r--r-- | core/plugin/zplugin.go | 1 | ||||
-rw-r--r-- | plugin.cfg | 1 | ||||
-rw-r--r-- | plugin/transfer/setup.go | 4 | ||||
-rw-r--r-- | plugin/transfer/transfer.go | 4 | ||||
-rw-r--r-- | plugin/tsig/README.md | 111 | ||||
-rw-r--r-- | plugin/tsig/setup.go | 168 | ||||
-rw-r--r-- | plugin/tsig/setup_test.go | 248 | ||||
-rw-r--r-- | plugin/tsig/tsig.go | 140 | ||||
-rw-r--r-- | plugin/tsig/tsig_test.go | 255 | ||||
-rw-r--r-- | test/tsig_test.go | 166 |
14 files changed, 1112 insertions, 3 deletions
diff --git a/core/dnsserver/config.go b/core/dnsserver/config.go index 4007d830f..c34398b39 100644 --- a/core/dnsserver/config.go +++ b/core/dnsserver/config.go @@ -43,6 +43,9 @@ type Config struct { // TLSConfig when listening for encrypted connections (gRPC, DNS-over-TLS). TLSConfig *tls.Config + // TSIG secrets, [name]key. + TsigSecret map[string]string + // Plugin stack. Plugin []plugin.Plugin diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go index ad311d323..86ab5cea3 100644 --- a/core/dnsserver/register.go +++ b/core/dnsserver/register.go @@ -156,6 +156,7 @@ func (h *dnsContext) MakeServers() ([]caddy.Server, error) { c.Debug = c.firstConfigInBlock.Debug c.Stacktrace = c.firstConfigInBlock.Stacktrace c.TLSConfig = c.firstConfigInBlock.TLSConfig + c.TsigSecret = c.firstConfigInBlock.TsigSecret } // we must map (group) each config to a bind address diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go index ec056ba68..fff6ebc9c 100644 --- a/core/dnsserver/server.go +++ b/core/dnsserver/server.go @@ -44,6 +44,8 @@ type Server struct { debug bool // disable recover() stacktrace bool // enable stacktrace in recover error log classChaos bool // allow non-INET class queries + + tsigSecret map[string]string } // NewServer returns a new CoreDNS server and compiles all plugins in to it. By default CH class @@ -54,6 +56,7 @@ func NewServer(addr string, group []*Config) (*Server, error) { Addr: addr, zones: make(map[string]*Config), graceTimeout: 5 * time.Second, + tsigSecret: make(map[string]string), } // We have to bound our wg with one increment @@ -73,6 +76,11 @@ func NewServer(addr string, group []*Config) (*Server, error) { // set the config per zone s.zones[site.Zone] = site + // copy tsig secrets + for key, secret := range site.TsigSecret { + s.tsigSecret[key] = secret + } + // compile custom plugin for everything var stack plugin.Handler for i := len(site.Plugin) - 1; i >= 0; i-- { @@ -115,7 +123,7 @@ func (s *Server) Serve(l net.Listener) error { ctx := context.WithValue(context.Background(), Key{}, s) ctx = context.WithValue(ctx, LoopKey{}, 0) s.ServeDNS(ctx, w, r) - })} + }), TsigSecret: s.tsigSecret} s.m.Unlock() return s.server[tcp].ActivateAndServe() @@ -129,7 +137,7 @@ func (s *Server) ServePacket(p net.PacketConn) error { ctx := context.WithValue(context.Background(), Key{}, s) ctx = context.WithValue(ctx, LoopKey{}, 0) s.ServeDNS(ctx, w, r) - })} + }), TsigSecret: s.tsigSecret} s.m.Unlock() return s.server[udp].ActivateAndServe() diff --git a/core/dnsserver/zdirectives.go b/core/dnsserver/zdirectives.go index bca217185..53168be86 100644 --- a/core/dnsserver/zdirectives.go +++ b/core/dnsserver/zdirectives.go @@ -34,6 +34,7 @@ var Directives = []string{ "any", "chaos", "loadbalance", + "tsig", "cache", "rewrite", "header", diff --git a/core/plugin/zplugin.go b/core/plugin/zplugin.go index a9167eeaf..45bfb5415 100644 --- a/core/plugin/zplugin.go +++ b/core/plugin/zplugin.go @@ -52,5 +52,6 @@ import ( _ "github.com/coredns/coredns/plugin/tls" _ "github.com/coredns/coredns/plugin/trace" _ "github.com/coredns/coredns/plugin/transfer" + _ "github.com/coredns/coredns/plugin/tsig" _ "github.com/coredns/coredns/plugin/whoami" ) diff --git a/plugin.cfg b/plugin.cfg index 628e71412..46a7df4c1 100644 --- a/plugin.cfg +++ b/plugin.cfg @@ -43,6 +43,7 @@ acl:acl any:any chaos:chaos loadbalance:loadbalance +tsig:tsig cache:cache rewrite:rewrite header:header diff --git a/plugin/transfer/setup.go b/plugin/transfer/setup.go index 604a26959..cd7d2091f 100644 --- a/plugin/transfer/setup.go +++ b/plugin/transfer/setup.go @@ -28,8 +28,10 @@ func setup(c *caddy.Controller) error { }) c.OnStartup(func() error { + config := dnsserver.GetConfig(c) + t.tsigSecret = config.TsigSecret // find all plugins that implement Transferer and add them to Transferers - plugins := dnsserver.GetConfig(c).Handlers() + plugins := config.Handlers() for _, pl := range plugins { tr, ok := pl.(Transferer) if !ok { diff --git a/plugin/transfer/transfer.go b/plugin/transfer/transfer.go index a9ad211df..792dd58cd 100644 --- a/plugin/transfer/transfer.go +++ b/plugin/transfer/transfer.go @@ -18,6 +18,7 @@ var log = clog.NewWithPlugin("transfer") type Transfer struct { Transferers []Transferer // List of plugins that implement Transferer xfrs []*xfr + tsigSecret map[string]string Next plugin.Handler } @@ -110,6 +111,9 @@ func (t *Transfer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Ms // Send response to client ch := make(chan *dns.Envelope) tr := new(dns.Transfer) + if r.IsTsig() != nil { + tr.TsigSecret = t.tsigSecret + } errCh := make(chan error) go func() { if err := tr.Out(w, r, ch); err != nil { diff --git a/plugin/tsig/README.md b/plugin/tsig/README.md new file mode 100644 index 000000000..4c5c9dc11 --- /dev/null +++ b/plugin/tsig/README.md @@ -0,0 +1,111 @@ +# tsig + +## Name + +*tsig* - validate TSIG requests and sign responses. + +## Description + +With *tsig*, you can define a set of TSIG secret keys for validating incoming TSIG requests and signing +responses. It can also require TSIG for certain query types, refusing requests that do not comply. + +## Syntax + +~~~ +tsig [ZONE...] { + secret NAME KEY + secrets FILE + require [QTYPE...] +} +~~~ + + * **ZONE** - the zones *tsig* will TSIG. By default, the zones from the server block are used. + + * `secret` **NAME** **KEY** - specifies a TSIG secret for **NAME** with **KEY**. Use this option more than once + to define multiple secrets. Secrets are global to the server instance, not just for the enclosing **ZONE**. + + * `secrets` **FILE** - same as `secret`, but load the secrets from a file. The file may define any number + of unique keys, each in the following `named.conf` format: + ```cgo + key "example." { + secret "X28hl0BOfAL5G0jsmJWSacrwn7YRm2f6U5brnzwWEus="; + }; + ``` + Each key may also specify an `algorithm` e.g. `algorithm hmac-sha256;`, but this is currently ignored by the plugin. + + * `require` **QTYPE...** - the query types that must be TSIG'd. Requests of the specified types + will be `REFUSED` if they are not signed.`require all` will require requests of all types to be + signed. `require none` will not require requests any types to be signed. Default behavior is to not require. + +## Examples + +Require TSIG signed transactions for transfer requests to `example.zone`. + +``` +example.zone { + tsig { + secret example.zone.key. NoTCJU+DMqFWywaPyxSijrDEA/eC3nK0xi3AMEZuPVk= + require AXFR IXFR + } + transfer { + to * + } +} +``` + +Require TSIG signed transactions for all requests to `auth.zone`. + +``` +auth.zone { + tsig { + secret auth.zone.key. NoTCJU+DMqFWywaPyxSijrDEA/eC3nK0xi3AMEZuPVk= + require all + } + forward . 10.1.0.2 +} +``` + +## Bugs + +### Zone Transfer Notifies + +With the transfer plugin, zone transfer notifications from CoreDNS are not TSIG signed. + +### Special Considerations for Forwarding Servers (RFC 8945 5.5) + +https://datatracker.ietf.org/doc/html/rfc8945#section-5.5 + +CoreDNS does not implement this section as follows ... + +* RFC requirement: + > If the name on the TSIG is not +of a secret that the server shares with the originator, the server +MUST forward the message unchanged including the TSIG. + + CoreDNS behavior: +If ths zone of the request matches the _tsig_ plugin zones, then the TSIG record +is always stripped. But even when the _tsig_ plugin is not involved, the _forward_ plugin +may alter the message with compression, which would cause validation failure +at the destination. + + +* RFC requirement: + > If the TSIG passes all checks, the forwarding +server MUST, if possible, include a TSIG of its own to the +destination or the next forwarder. + + CoreDNS behavior: +If ths zone of the request matches the _tsig_ plugin zones, _forward_ plugin will +proxy the request upstream without TSIG. + + +* RFC requirement: + > If no transaction security is +available to the destination and the message is a query, and if the +corresponding response has the AD flag (see RFC4035) set, the +forwarder MUST clear the AD flag before adding the TSIG to the +response and returning the result to the system from which it +received the query. + + CoreDNS behavior: +The AD flag is not cleared. diff --git a/plugin/tsig/setup.go b/plugin/tsig/setup.go new file mode 100644 index 000000000..a187a4b4a --- /dev/null +++ b/plugin/tsig/setup.go @@ -0,0 +1,168 @@ +package tsig + +import ( + "bufio" + "fmt" + "io" + "os" + "strings" + + "github.com/coredns/caddy" + "github.com/coredns/coredns/core/dnsserver" + "github.com/coredns/coredns/plugin" + + "github.com/miekg/dns" +) + +func init() { + caddy.RegisterPlugin(pluginName, caddy.Plugin{ + ServerType: "dns", + Action: setup, + }) +} + +func setup(c *caddy.Controller) error { + t, err := parse(c) + if err != nil { + return plugin.Error(pluginName, c.ArgErr()) + } + + config := dnsserver.GetConfig(c) + + config.TsigSecret = t.secrets + + config.AddPlugin(func(next plugin.Handler) plugin.Handler { + t.Next = next + return t + }) + + return nil +} + +func parse(c *caddy.Controller) (*TSIGServer, error) { + t := &TSIGServer{ + secrets: make(map[string]string), + types: defaultQTypes, + } + + for i := 0; c.Next(); i++ { + if i > 0 { + return nil, plugin.ErrOnce + } + + t.Zones = plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) + for c.NextBlock() { + switch c.Val() { + case "secret": + args := c.RemainingArgs() + if len(args) != 2 { + return nil, c.ArgErr() + } + k := plugin.Name(args[0]).Normalize() + if _, exists := t.secrets[k]; exists { + return nil, fmt.Errorf("key %q redefined", k) + } + t.secrets[k] = args[1] + case "secrets": + args := c.RemainingArgs() + if len(args) != 1 { + return nil, c.ArgErr() + } + f, err := os.Open(args[0]) + if err != nil { + return nil, err + } + secrets, err := parseKeyFile(f) + if err != nil { + return nil, err + } + for k, s := range secrets { + if _, exists := t.secrets[k]; exists { + return nil, fmt.Errorf("key %q redefined", k) + } + t.secrets[k] = s + } + case "require": + t.types = qTypes{} + args := c.RemainingArgs() + if len(args) == 0 { + return nil, c.ArgErr() + } + if args[0] == "all" { + t.all = true + continue + } + if args[0] == "none" { + continue + } + for _, str := range args { + qt, ok := dns.StringToType[str] + if !ok { + return nil, c.Errf("unknown query type '%s'", str) + } + t.types[qt] = struct{}{} + } + default: + return nil, c.Errf("unknown property '%s'", c.Val()) + } + } + } + return t, nil +} + +func parseKeyFile(f io.Reader) (map[string]string, error) { + secrets := make(map[string]string) + s := bufio.NewScanner(f) + for s.Scan() { + fields := strings.Fields(s.Text()) + if len(fields) == 0 { + continue + } + if fields[0] != "key" { + return nil, fmt.Errorf("unexpected token %q", fields[0]) + } + if len(fields) < 2 { + return nil, fmt.Errorf("expected key name %q", s.Text()) + } + key := strings.Trim(fields[1], "\"{") + if len(key) == 0 { + return nil, fmt.Errorf("expected key name %q", s.Text()) + } + key = plugin.Name(key).Normalize() + if _, ok := secrets[key]; ok { + return nil, fmt.Errorf("key %q redefined", key) + } + key: + for s.Scan() { + fields := strings.Fields(s.Text()) + if len(fields) == 0 { + continue + } + switch fields[0] { + case "algorithm": + continue + case "secret": + if len(fields) < 2 { + return nil, fmt.Errorf("expected secret key %q", s.Text()) + } + secret := strings.Trim(fields[1], "\";") + if len(secret) == 0 { + return nil, fmt.Errorf("expected secret key %q", s.Text()) + } + secrets[key] = secret + case "}": + fallthrough + case "};": + break key + default: + return nil, fmt.Errorf("unexpected token %q", fields[0]) + } + } + if _, ok := secrets[key]; !ok { + return nil, fmt.Errorf("expected secret for key %q", key) + } + } + return secrets, nil +} + +var defaultQTypes = qTypes{} diff --git a/plugin/tsig/setup_test.go b/plugin/tsig/setup_test.go new file mode 100644 index 000000000..00966bf09 --- /dev/null +++ b/plugin/tsig/setup_test.go @@ -0,0 +1,248 @@ +package tsig + +import ( + "fmt" + "strings" + "testing" + + "github.com/coredns/caddy" + "github.com/coredns/coredns/plugin/test" + + "github.com/miekg/dns" +) + +func TestParse(t *testing.T) { + + secrets := map[string]string{ + "name.key.": "test-key", + "name2.key.": "test-key-2", + } + secretConfig := "" + for k, s := range secrets { + secretConfig += fmt.Sprintf("secret %s %s\n", k, s) + } + secretsFile, cleanup, err := test.TempFile(".", `key "name.key." { + secret "test-key"; +}; +key "name2.key." { + secret "test-key2"; +};`) + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + defer cleanup() + + tests := []struct { + input string + shouldErr bool + expectedZones []string + expectedQTypes qTypes + expectedSecrets map[string]string + expectedAll bool + }{ + { + input: "tsig {\n " + secretConfig + "}", + expectedZones: []string{"."}, + expectedQTypes: defaultQTypes, + expectedSecrets: secrets, + }, + { + input: "tsig {\n secrets " + secretsFile + "\n}", + expectedZones: []string{"."}, + expectedQTypes: defaultQTypes, + expectedSecrets: secrets, + }, + { + input: "tsig example.com {\n " + secretConfig + "}", + expectedZones: []string{"example.com."}, + expectedQTypes: defaultQTypes, + expectedSecrets: secrets, + }, + { + input: "tsig {\n " + secretConfig + " require all \n}", + expectedZones: []string{"."}, + expectedQTypes: qTypes{}, + expectedAll: true, + expectedSecrets: secrets, + }, + { + input: "tsig {\n " + secretConfig + " require none \n}", + expectedZones: []string{"."}, + expectedQTypes: qTypes{}, + expectedAll: false, + expectedSecrets: secrets, + }, + { + input: "tsig {\n " + secretConfig + " \n require A AAAA \n}", + expectedZones: []string{"."}, + expectedQTypes: qTypes{dns.TypeA: {}, dns.TypeAAAA: {}}, + expectedSecrets: secrets, + }, + { + input: "tsig {\n blah \n}", + shouldErr: true, + }, + { + input: "tsig {\n secret name. too many parameters \n}", + shouldErr: true, + }, + { + input: "tsig {\n require \n}", + shouldErr: true, + }, + { + input: "tsig {\n require invalid-qtype \n}", + shouldErr: true, + }, + } + + serverBlockKeys := []string{"."} + for i, test := range tests { + c := caddy.NewTestController("dns", test.input) + c.ServerBlockKeys = serverBlockKeys + ts, err := parse(c) + + if err == nil && test.shouldErr { + t.Fatalf("Test %d expected errors, but got no error.", i) + } else if err != nil && !test.shouldErr { + t.Fatalf("Test %d expected no errors, but got '%v'", i, err) + } + + if test.shouldErr { + continue + } + + if len(test.expectedZones) != len(ts.Zones) { + t.Fatalf("Test %d expected zones '%v', but got '%v'.", i, test.expectedZones, ts.Zones) + } + for j := range test.expectedZones { + if test.expectedZones[j] != ts.Zones[j] { + t.Errorf("Test %d expected zones '%v', but got '%v'.", i, test.expectedZones, ts.Zones) + break + } + } + + if test.expectedAll != ts.all { + t.Errorf("Test %d expected require all to be '%v', but got '%v'.", i, test.expectedAll, ts.all) + } + + if len(test.expectedQTypes) != len(ts.types) { + t.Fatalf("Test %d expected required types '%v', but got '%v'.", i, test.expectedQTypes, ts.types) + } + for qt := range test.expectedQTypes { + if _, ok := ts.types[qt]; !ok { + t.Errorf("Test %d required types '%v', but got '%v'.", i, test.expectedQTypes, ts.types) + break + } + } + + if len(test.expectedSecrets) != len(ts.secrets) { + t.Fatalf("Test %d expected secrets '%v', but got '%v'.", i, test.expectedSecrets, ts.secrets) + } + for qt := range test.expectedSecrets { + secret, ok := ts.secrets[qt] + if !ok { + t.Errorf("Test %d required secrets '%v', but got '%v'.", i, test.expectedSecrets, ts.secrets) + break + } + if secret != ts.secrets[qt] { + t.Errorf("Test %d required secrets '%v', but got '%v'.", i, test.expectedSecrets, ts.secrets) + break + } + } + + } +} + +func TestParseKeyFile(t *testing.T) { + var reader = strings.NewReader(`key "foo" { + algorithm hmac-sha256; + secret "36eowrtmxceNA3T5AdE+JNUOWFCw3amtcyHACnrDVgQ="; +}; +key "bar" { + algorithm hmac-sha256; + secret "X28hl0BOfAL5G0jsmJWSacrwn7YRm2f6U5brnzwWEus="; +}; +key "baz" { + secret "BycDPXSx/5YCD44Q4g5Nd2QNxNRDKwWTXddrU/zpIQM="; +};`) + + secrets, err := parseKeyFile(reader) + if err != nil { + t.Fatalf("Unexpected error: %q", err) + } + expectedSecrets := map[string]string{ + "foo.": "36eowrtmxceNA3T5AdE+JNUOWFCw3amtcyHACnrDVgQ=", + "bar.": "X28hl0BOfAL5G0jsmJWSacrwn7YRm2f6U5brnzwWEus=", + "baz.": "BycDPXSx/5YCD44Q4g5Nd2QNxNRDKwWTXddrU/zpIQM=", + } + + if len(secrets) != len(expectedSecrets) { + t.Fatalf("result has %d keys. expected %d", len(secrets), len(expectedSecrets)) + } + + for k, sec := range secrets { + expectedSec, ok := expectedSecrets[k] + if !ok { + t.Errorf("unexpected key in result. %q", k) + continue + } + if sec != expectedSec { + t.Errorf("incorrect secret in result for key %q. expected %q got %q ", k, expectedSec, sec) + } + } +} + +func TestParseKeyFileErrors(t *testing.T) { + tests := []struct { + in string + err string + }{ + {in: `key {`, err: "expected key name \"key {\""}, + {in: `foo "key" {`, err: "unexpected token \"foo\""}, + { + in: `key "foo" { + secret "36eowrtmxceNA3T5AdE+JNUOWFCw3amtcyHACnrDVgQ="; + }; + key "foo" { + secret "X28hl0BOfAL5G0jsmJWSacrwn7YRm2f6U5brnzwWEus="; + }; `, + err: "key \"foo.\" redefined", + }, + {in: `key "foo" { + schmalgorithm hmac-sha256;`, + err: "unexpected token \"schmalgorithm\"", + }, + { + in: `key "foo" { + schmecret "36eowrtmxceNA3T5AdE+JNUOWFCw3amtcyHACnrDVgQ=";`, + err: "unexpected token \"schmecret\"", + }, + { + in: `key "foo" { + secret`, + err: "expected secret key \"\\tsecret\"", + }, + { + in: `key "foo" { + secret ;`, + err: "expected secret key \"\\tsecret ;\"", + }, + { + in: `key "foo" { + };`, + err: "expected secret for key \"foo.\"", + }, + } + for i, testcase := range tests { + _, err := parseKeyFile(strings.NewReader(testcase.in)) + if err == nil { + t.Errorf("Test %d: expected error, got no error", i) + continue + } + if err.Error() != testcase.err { + t.Errorf("Test %d: Expected error: %q, got %q", i, testcase.err, err.Error()) + } + + } +} diff --git a/plugin/tsig/tsig.go b/plugin/tsig/tsig.go new file mode 100644 index 000000000..6441c8a6b --- /dev/null +++ b/plugin/tsig/tsig.go @@ -0,0 +1,140 @@ +package tsig + +import ( + "context" + "encoding/binary" + "encoding/hex" + "time" + + "github.com/coredns/coredns/plugin" + "github.com/coredns/coredns/plugin/pkg/log" + "github.com/coredns/coredns/request" + + "github.com/miekg/dns" +) + +// TSIGServer verifies tsig status and adds tsig to responses +type TSIGServer struct { + Zones []string + secrets map[string]string // [key-name]secret + types qTypes + all bool + Next plugin.Handler +} + +type qTypes map[uint16]struct{} + +// Name implements plugin.Handler +func (t TSIGServer) Name() string { return pluginName } + +// ServeDNS implements plugin.Handler +func (t *TSIGServer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { + var err error + state := request.Request{Req: r, W: w} + if z := plugin.Zones(t.Zones).Matches(state.Name()); z == "" { + return plugin.NextOrFailure(t.Name(), t.Next, ctx, w, r) + } + + var tsigRR = r.IsTsig() + rcode := dns.RcodeSuccess + if !t.tsigRequired(state.QType()) && tsigRR == nil { + return plugin.NextOrFailure(t.Name(), t.Next, ctx, w, r) + } + + if tsigRR == nil { + log.Debugf("rejecting '%s' request without TSIG\n", dns.TypeToString[state.QType()]) + rcode = dns.RcodeRefused + } + + // wrap the response writer so the response will be TSIG signed. + w = &restoreTsigWriter{w, r, tsigRR} + + tsigStatus := w.TsigStatus() + if tsigStatus != nil { + log.Debugf("TSIG validation failed: %v %v", dns.TypeToString[state.QType()], tsigStatus) + rcode = dns.RcodeNotAuth + switch tsigStatus { + case dns.ErrSecret: + tsigRR.Error = dns.RcodeBadKey + case dns.ErrTime: + tsigRR.Error = dns.RcodeBadTime + default: + tsigRR.Error = dns.RcodeBadSig + } + resp := new(dns.Msg).SetRcode(r, rcode) + w.WriteMsg(resp) + return dns.RcodeSuccess, nil + } + + // strip the TSIG RR. Next, and subsequent plugins will not see the TSIG RRs. + // This violates forwarding cases (RFC 8945 5.5). See README.md Bugs + if len(r.Extra) > 1 { + r.Extra = r.Extra[0 : len(r.Extra)-1] + } else { + r.Extra = []dns.RR{} + } + + if rcode == dns.RcodeSuccess { + rcode, err = plugin.NextOrFailure(t.Name(), t.Next, ctx, w, r) + if err != nil { + log.Errorf("request handler returned an error: %v\n", err) + } + } + // If the plugin chain result was not an error, restore the TSIG and write the response. + if !plugin.ClientWrite(rcode) { + resp := new(dns.Msg).SetRcode(r, rcode) + w.WriteMsg(resp) + } + return dns.RcodeSuccess, nil +} + +func (t *TSIGServer) tsigRequired(qtype uint16) bool { + if t.all { + return true + } + if _, ok := t.types[qtype]; ok { + return true + } + return false +} + +// restoreTsigWriter Implement Response Writer, and adds a TSIG RR to a response +type restoreTsigWriter struct { + dns.ResponseWriter + req *dns.Msg // original request excluding TSIG if it has one + reqTSIG *dns.TSIG // original TSIG +} + +// WriteMsg adds a TSIG RR to the response +func (r *restoreTsigWriter) WriteMsg(m *dns.Msg) error { + // Make sure the response has an EDNS OPT RR if the request had it. + // Otherwise ScrubWriter would append it *after* TSIG, making it a non-compliant DNS message. + state := request.Request{Req: r.req, W: r.ResponseWriter} + state.SizeAndDo(m) + + repTSIG := m.IsTsig() + if r.reqTSIG != nil && repTSIG == nil { + repTSIG = new(dns.TSIG) + repTSIG.Hdr = dns.RR_Header{Name: r.reqTSIG.Hdr.Name, Rrtype: dns.TypeTSIG, Class: dns.ClassANY} + repTSIG.Algorithm = r.reqTSIG.Algorithm + repTSIG.OrigId = m.MsgHdr.Id + repTSIG.Error = r.reqTSIG.Error + repTSIG.MAC = r.reqTSIG.MAC + repTSIG.MACSize = r.reqTSIG.MACSize + if repTSIG.Error == dns.RcodeBadTime { + // per RFC 8945 5.2.3. client time goes into TimeSigned, server time in OtherData, OtherLen = 6 ... + repTSIG.TimeSigned = r.reqTSIG.TimeSigned + b := make([]byte, 8) + // TimeSigned is network byte order. + binary.BigEndian.PutUint64(b, uint64(time.Now().Unix())) + // truncate to 48 least significant bits (network order 6 rightmost bytes) + repTSIG.OtherData = hex.EncodeToString(b[2:]) + repTSIG.OtherLen = 6 + } + m.Extra = append(m.Extra, repTSIG) + } + + return r.ResponseWriter.WriteMsg(m) +} + +const pluginName = "tsig" diff --git a/plugin/tsig/tsig_test.go b/plugin/tsig/tsig_test.go new file mode 100644 index 000000000..f7ec1fdf1 --- /dev/null +++ b/plugin/tsig/tsig_test.go @@ -0,0 +1,255 @@ +package tsig + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/coredns/coredns/plugin/pkg/dnstest" + "github.com/coredns/coredns/plugin/test" + "github.com/coredns/coredns/request" + + "github.com/miekg/dns" +) + +func TestServeDNS(t *testing.T) { + cases := []struct { + zones []string + reqTypes qTypes + qType uint16 + qTsig, all bool + expectRcode int + expectTsig bool + statusError bool + }{ + { + zones: []string{"."}, + all: true, + qType: dns.TypeA, + qTsig: true, + expectRcode: dns.RcodeSuccess, + expectTsig: true, + }, + { + zones: []string{"."}, + all: true, + qType: dns.TypeA, + qTsig: false, + expectRcode: dns.RcodeRefused, + expectTsig: false, + }, + { + zones: []string{"another.domain."}, + all: true, + qType: dns.TypeA, + qTsig: false, + expectRcode: dns.RcodeSuccess, + expectTsig: false, + }, + { + zones: []string{"another.domain."}, + all: true, + qType: dns.TypeA, + qTsig: true, + expectRcode: dns.RcodeSuccess, + expectTsig: false, + }, + { + zones: []string{"."}, + reqTypes: qTypes{dns.TypeAXFR: {}}, + qType: dns.TypeAXFR, + qTsig: true, + expectRcode: dns.RcodeSuccess, + expectTsig: true, + }, + { + zones: []string{"."}, + reqTypes: qTypes{}, + qType: dns.TypeA, + qTsig: false, + expectRcode: dns.RcodeSuccess, + expectTsig: false, + }, + { + zones: []string{"."}, + reqTypes: qTypes{}, + qType: dns.TypeA, + qTsig: true, + expectRcode: dns.RcodeSuccess, + expectTsig: true, + }, + { + zones: []string{"."}, + all: true, + qType: dns.TypeA, + qTsig: true, + expectRcode: dns.RcodeNotAuth, + expectTsig: true, + statusError: true, + }, + } + + for i, tc := range cases { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + tsig := TSIGServer{ + Zones: tc.zones, + all: tc.all, + types: tc.reqTypes, + Next: testHandler(), + } + + ctx := context.TODO() + + var w *dnstest.Recorder + if tc.statusError { + w = dnstest.NewRecorder(&ErrWriter{err: dns.ErrSig}) + } else { + w = dnstest.NewRecorder(&test.ResponseWriter{}) + } + r := new(dns.Msg) + r.SetQuestion("test.example.", tc.qType) + if tc.qTsig { + r.SetTsig("test.key.", dns.HmacSHA256, 300, time.Now().Unix()) + } + + _, err := tsig.ServeDNS(ctx, w, r) + if err != nil { + t.Fatal(err) + } + + if w.Msg.Rcode != tc.expectRcode { + t.Fatalf("expected rcode %v, got %v", tc.expectRcode, w.Msg.Rcode) + } + + if ts := w.Msg.IsTsig(); ts == nil && tc.expectTsig { + t.Fatal("expected TSIG in response") + } + if ts := w.Msg.IsTsig(); ts != nil && !tc.expectTsig { + t.Fatal("expected no TSIG in response") + } + }) + } +} + +func TestServeDNSTsigErrors(t *testing.T) { + clientNow := time.Now().Unix() + + cases := []struct { + desc string + tsigErr error + expectRcode int + expectError int + expectOtherLength int + expectTimeSigned int64 + }{ + { + desc: "Unknown Key", + tsigErr: dns.ErrSecret, + expectRcode: dns.RcodeNotAuth, + expectError: dns.RcodeBadKey, + expectOtherLength: 0, + expectTimeSigned: 0, + }, + { + desc: "Bad Signature", + tsigErr: dns.ErrSig, + expectRcode: dns.RcodeNotAuth, + expectError: dns.RcodeBadSig, + expectOtherLength: 0, + expectTimeSigned: 0, + }, + { + desc: "Bad Time", + tsigErr: dns.ErrTime, + expectRcode: dns.RcodeNotAuth, + expectError: dns.RcodeBadTime, + expectOtherLength: 6, + expectTimeSigned: clientNow, + }, + } + + tsig := TSIGServer{ + Zones: []string{"."}, + all: true, + Next: testHandler(), + } + + for _, tc := range cases { + t.Run(tc.desc, func(t *testing.T) { + ctx := context.TODO() + + var w *dnstest.Recorder + + w = dnstest.NewRecorder(&ErrWriter{err: tc.tsigErr}) + + r := new(dns.Msg) + r.SetQuestion("test.example.", dns.TypeA) + r.SetTsig("test.key.", dns.HmacSHA256, 300, clientNow) + + // set a fake MAC and Size in request + rtsig := r.IsTsig() + rtsig.MAC = "0123456789012345678901234567890101234567890123456789012345678901" + rtsig.MACSize = 32 + + _, err := tsig.ServeDNS(ctx, w, r) + if err != nil { + t.Fatal(err) + } + + if w.Msg.Rcode != tc.expectRcode { + t.Fatalf("expected rcode %v, got %v", tc.expectRcode, w.Msg.Rcode) + } + + ts := w.Msg.IsTsig() + + if ts == nil { + t.Fatal("expected TSIG in response") + } + + if int(ts.Error) != tc.expectError { + t.Errorf("expected TSIG error code %v, got %v", tc.expectError, ts.Error) + } + + if len(ts.OtherData)/2 != tc.expectOtherLength { + t.Errorf("expected Other of length %v, got %v", tc.expectOtherLength, len(ts.OtherData)) + } + + if int(ts.OtherLen) != tc.expectOtherLength { + t.Errorf("expected OtherLen %v, got %v", tc.expectOtherLength, ts.OtherLen) + } + + if ts.TimeSigned != uint64(tc.expectTimeSigned) { + t.Errorf("expected TimeSigned to be %v, got %v", tc.expectTimeSigned, ts.TimeSigned) + } + }) + } +} + +func testHandler() test.HandlerFunc { + return func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { + state := request.Request{W: w, Req: r} + qname := state.Name() + m := new(dns.Msg) + rcode := dns.RcodeServerFailure + if qname == "test.example." { + m.SetReply(r) + rr := test.A("test.example. 300 IN A 1.2.3.48") + m.Answer = []dns.RR{rr} + m.Authoritative = true + rcode = dns.RcodeSuccess + } + m.SetRcode(r, rcode) + w.WriteMsg(m) + return rcode, nil + } +} + +// a test.ResponseWriter that always returns err as the TSIG status error +type ErrWriter struct { + err error + test.ResponseWriter +} + +// TsigStatus always returns an error. +func (t *ErrWriter) TsigStatus() error { return t.err } diff --git a/test/tsig_test.go b/test/tsig_test.go new file mode 100644 index 000000000..19d5d0379 --- /dev/null +++ b/test/tsig_test.go @@ -0,0 +1,166 @@ +package test + +import ( + "testing" + "time" + + "github.com/miekg/dns" +) + +var tsigKey = "tsig.key." +var tsigSecret = "i9M+00yrECfVZG2qCjr4mPpaGim/Bq+IWMiNrLjUO4Y=" + +var corefile = `.:0 { + tsig { + secret ` + tsigKey + ` ` + tsigSecret + ` + } + hosts { + 1.2.3.4 test + } + }` + +func TestTsig(t *testing.T) { + i, udp, _, err := CoreDNSServerAndPorts(corefile) + if err != nil { + t.Fatalf("Could not get CoreDNS serving instance: %s", err) + } + defer i.Stop() + + m := new(dns.Msg) + m.SetQuestion("test.", dns.TypeA) + m.SetTsig(tsigKey, dns.HmacSHA256, 300, time.Now().Unix()) + + client := dns.Client{Net: "udp", TsigSecret: map[string]string{tsigKey: tsigSecret}} + r, _, err := client.Exchange(m, udp) + if err != nil { + t.Fatalf("Could not send msg: %s", err) + } + if r.Rcode != dns.RcodeSuccess { + t.Fatalf("Rcode should be dns.RcodeSuccess") + } + tsig := r.IsTsig() + if tsig == nil { + t.Fatalf("Respose was not TSIG") + } + if tsig.Error != dns.RcodeSuccess { + t.Fatalf("TSIG Error code should be dns.RcodeSuccess") + } +} + +func TestTsigBadKey(t *testing.T) { + i, udp, _, err := CoreDNSServerAndPorts(corefile) + if err != nil { + t.Fatalf("Could not get CoreDNS serving instance: %s", err) + } + defer i.Stop() + + m := new(dns.Msg) + m.SetQuestion("test.", dns.TypeA) + m.SetTsig("bad.key.", dns.HmacSHA256, 300, time.Now().Unix()) + + // rename client key to a key name the server doesnt have + client := dns.Client{Net: "udp", TsigSecret: map[string]string{"bad.key.": tsigSecret}} + r, _, err := client.Exchange(m, udp) + + if err != dns.ErrAuth { + t.Fatalf("Expected \"dns: bad authentication\" error, got: %s", err) + } + if r.Rcode != dns.RcodeNotAuth { + t.Fatalf("Rcode should be dns.RcodeNotAuth") + } + tsig := r.IsTsig() + if tsig == nil { + t.Fatalf("Respose was not TSIG") + } + if tsig.Error != dns.RcodeBadKey { + t.Fatalf("TSIG Error code should be dns.RcodeBadKey") + } + if tsig.MAC != "" { + t.Fatalf("TSIG MAC should be empty") + } + if tsig.MACSize != 0 { + t.Fatalf("TSIG MACSize should be 0") + } + if tsig.TimeSigned != 0 { + t.Fatalf("TSIG TimeSigned should be 0") + } +} + +func TestTsigBadSig(t *testing.T) { + i, udp, _, err := CoreDNSServerAndPorts(corefile) + if err != nil { + t.Fatalf("Could not get CoreDNS serving instance: %s", err) + } + defer i.Stop() + + m := new(dns.Msg) + m.SetQuestion("test.", dns.TypeA) + m.SetTsig(tsigKey, dns.HmacSHA256, 300, time.Now().Unix()) + + // mangle the client secret so the sig wont match the server sig + client := dns.Client{Net: "udp", TsigSecret: map[string]string{tsigKey: "BADSIG00ECfVZG2qCjr4mPpaGim/Bq+IWMiNrLjUO4Y="}} + r, _, err := client.Exchange(m, udp) + + if err != dns.ErrAuth { + t.Fatalf("Expected \"dns: bad authentication\" error, got: %s", err) + } + if r.Rcode != dns.RcodeNotAuth { + t.Fatalf("Rcode should be dns.RcodeNotAuth") + } + tsig := r.IsTsig() + if tsig == nil { + t.Fatalf("Respose was not TSIG") + } + if tsig.Error != dns.RcodeBadSig { + t.Fatalf("TSIG Error code should be dns.RcodeBadSig") + } + if tsig.MAC != "" { + t.Fatalf("TSIG MAC should be empty") + } + if tsig.MACSize != 0 { + t.Fatalf("TSIG MACSize should be 0") + } + if tsig.TimeSigned != 0 { + t.Fatalf("TSIG TimeSigned should be 0") + } +} + +func TestTsigBadTime(t *testing.T) { + i, udp, _, err := CoreDNSServerAndPorts(corefile) + if err != nil { + t.Fatalf("Could not get CoreDNS serving instance: %s", err) + } + defer i.Stop() + + m := new(dns.Msg) + m.SetQuestion("test.", dns.TypeA) + + // set time to be older by > fudge seconds + m.SetTsig(tsigKey, dns.HmacSHA256, 300, time.Now().Unix()-600) + + client := dns.Client{Net: "udp", TsigSecret: map[string]string{tsigKey: tsigSecret}} + r, _, err := client.Exchange(m, udp) + + if err != dns.ErrAuth { + t.Fatalf("Expected \"dns: bad authentication\" error, got: %s", err) + } + if r.Rcode != dns.RcodeNotAuth { + t.Fatalf("Rcode should be dns.RcodeNotAuth") + } + tsig := r.IsTsig() + if tsig == nil { + t.Fatalf("Respose was not TSIG") + } + if tsig.Error != dns.RcodeBadTime { + t.Fatalf("TSIG Error code should be dns.RcodeBadTime") + } + if tsig.MAC == "" { + t.Fatalf("TSIG MAC should not be empty") + } + if tsig.MACSize != 32 { + t.Fatalf("TSIG MACSize should be 32") + } + if tsig.TimeSigned == 0 { + t.Fatalf("TSIG TimeSigned should not be 0") + } +} |