From 471d62926d6299202abd7a9d215da6db6e2ad0d8 Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Fri, 27 Mar 2026 20:05:49 +0100 Subject: [PATCH] plugin/tsig: add require_opcode directive for opcode-based TSIG (#7828) Extend the tsig plugin to require TSIG signatures based on DNS opcodes, similar to the existing qtype-based requirement. The new require_opcode directive accepts opcode names (QUERY, IQUERY, STATUS, NOTIFY, UPDATE) or the special values "all" and "none". This is useful for requiring TSIG on dynamic update (UPDATE) or zone transfer notification (NOTIFY) requests while allowing unsigned queries. Example: ``` tsig { secret key. NoTCJU+DMqFWywaPyxSijrDEA/eC3nK0xi3AMEZuPVk= require_opcode UPDATE NOTIFY } ``` Signed-off-by: Seena Fallah --- plugin/tsig/README.md | 21 +++++++- plugin/tsig/setup.go | 24 ++++++++- plugin/tsig/setup_test.go | 86 +++++++++++++++++++++++++----- plugin/tsig/tsig.go | 31 ++++++----- plugin/tsig/tsig_test.go | 108 +++++++++++++++++++++++++++++++++----- 5 files changed, 228 insertions(+), 42 deletions(-) diff --git a/plugin/tsig/README.md b/plugin/tsig/README.md index d73b9ca9c..ed4a8f15f 100644 --- a/plugin/tsig/README.md +++ b/plugin/tsig/README.md @@ -19,6 +19,7 @@ tsig [ZONE...] { secret NAME KEY secrets FILE require [QTYPE...] + require_opcode [OPCODE...] } ~~~ @@ -36,10 +37,15 @@ tsig [ZONE...] { ``` 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 + * `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. + * `require_opcode` **OPCODE...** - the opcodes that must be TSIG'd. Requests with the specified opcodes + will be `REFUSED` if they are not signed. Valid opcodes are: `QUERY`, `IQUERY`, `STATUS`, `NOTIFY`, `UPDATE`. + `require_opcode all` will require requests with all opcodes to be signed. `require_opcode none` will not + require requests with any opcode to be signed. Default behavior is to not require. + ## Examples Require TSIG signed transactions for transfer requests to `example.zone`. @@ -68,6 +74,17 @@ auth.zone { } ``` +Require TSIG signed transactions for UPDATE and NOTIFY operations to `dynamic.zone`. + +``` +dynamic.zone { + tsig { + secret dynamic.zone.key. NoTCJU+DMqFWywaPyxSijrDEA/eC3nK0xi3AMEZuPVk= + require_opcode UPDATE NOTIFY + } +} +``` + ## Bugs ### Secondary diff --git a/plugin/tsig/setup.go b/plugin/tsig/setup.go index a187a4b4a..ef328549b 100644 --- a/plugin/tsig/setup.go +++ b/plugin/tsig/setup.go @@ -43,6 +43,7 @@ func parse(c *caddy.Controller) (*TSIGServer, error) { t := &TSIGServer{ secrets: make(map[string]string), types: defaultQTypes, + opcodes: defaultOpCodes, } for i := 0; c.Next(); i++ { @@ -89,7 +90,7 @@ func parse(c *caddy.Controller) (*TSIGServer, error) { return nil, c.ArgErr() } if args[0] == "all" { - t.all = true + t.allTypes = true continue } if args[0] == "none" { @@ -102,6 +103,26 @@ func parse(c *caddy.Controller) (*TSIGServer, error) { } t.types[qt] = struct{}{} } + case "require_opcode": + t.opcodes = opCodes{} + args := c.RemainingArgs() + if len(args) == 0 { + return nil, c.ArgErr() + } + if args[0] == "all" { + t.allOpcodes = true + continue + } + if args[0] == "none" { + continue + } + for _, str := range args { + op, ok := dns.StringToOpcode[str] + if !ok { + return nil, c.Errf("unknown opcode '%s'", str) + } + t.opcodes[op] = struct{}{} + } default: return nil, c.Errf("unknown property '%s'", c.Val()) } @@ -166,3 +187,4 @@ func parseKeyFile(f io.Reader) (map[string]string, error) { } var defaultQTypes = qTypes{} +var defaultOpCodes = opCodes{} diff --git a/plugin/tsig/setup_test.go b/plugin/tsig/setup_test.go index 0d7433996..e766601c1 100644 --- a/plugin/tsig/setup_test.go +++ b/plugin/tsig/setup_test.go @@ -32,49 +32,85 @@ key "name2.key." { defer cleanup() tests := []struct { - input string - shouldErr bool - expectedZones []string - expectedQTypes qTypes - expectedSecrets map[string]string - expectedAll bool + input string + shouldErr bool + expectedZones []string + expectedQTypes qTypes + expectedOpCodes opCodes + expectedSecrets map[string]string + expectedAllTypes bool + expectedAllOpcodes bool }{ { input: "tsig {\n " + secretConfig + "}", expectedZones: []string{"."}, expectedQTypes: defaultQTypes, + expectedOpCodes: defaultOpCodes, expectedSecrets: secrets, }, { input: "tsig {\n secrets " + secretsFile + "\n}", expectedZones: []string{"."}, expectedQTypes: defaultQTypes, + expectedOpCodes: defaultOpCodes, expectedSecrets: secrets, }, { input: "tsig example.com {\n " + secretConfig + "}", expectedZones: []string{"example.com."}, expectedQTypes: defaultQTypes, + expectedOpCodes: defaultOpCodes, expectedSecrets: secrets, }, { - input: "tsig {\n " + secretConfig + " require all \n}", - expectedZones: []string{"."}, - expectedQTypes: qTypes{}, - expectedAll: true, - expectedSecrets: secrets, + input: "tsig {\n " + secretConfig + " require all \n}", + expectedZones: []string{"."}, + expectedQTypes: qTypes{}, + expectedOpCodes: defaultOpCodes, + expectedAllTypes: true, + expectedSecrets: secrets, }, { input: "tsig {\n " + secretConfig + " require none \n}", expectedZones: []string{"."}, expectedQTypes: qTypes{}, - expectedAll: false, + expectedOpCodes: defaultOpCodes, expectedSecrets: secrets, }, { input: "tsig {\n " + secretConfig + " \n require A AAAA \n}", expectedZones: []string{"."}, expectedQTypes: qTypes{dns.TypeA: {}, dns.TypeAAAA: {}}, + expectedOpCodes: defaultOpCodes, + expectedSecrets: secrets, + }, + { + input: "tsig {\n " + secretConfig + " \n require_opcode UPDATE NOTIFY \n}", + expectedZones: []string{"."}, + expectedQTypes: defaultQTypes, + expectedOpCodes: opCodes{dns.OpcodeUpdate: {}, dns.OpcodeNotify: {}}, + expectedSecrets: secrets, + }, + { + input: "tsig {\n " + secretConfig + " \n require_opcode all \n}", + expectedZones: []string{"."}, + expectedQTypes: defaultQTypes, + expectedOpCodes: opCodes{}, + expectedAllOpcodes: true, + expectedSecrets: secrets, + }, + { + input: "tsig {\n " + secretConfig + " \n require_opcode none \n}", + expectedZones: []string{"."}, + expectedQTypes: defaultQTypes, + expectedOpCodes: opCodes{}, + expectedSecrets: secrets, + }, + { + input: "tsig {\n " + secretConfig + " \n require AXFR \n require_opcode UPDATE \n}", + expectedZones: []string{"."}, + expectedQTypes: qTypes{dns.TypeAXFR: {}}, + expectedOpCodes: opCodes{dns.OpcodeUpdate: {}}, expectedSecrets: secrets, }, { @@ -93,6 +129,14 @@ key "name2.key." { input: "tsig {\n require invalid-qtype \n}", shouldErr: true, }, + { + input: "tsig {\n require_opcode \n}", + shouldErr: true, + }, + { + input: "tsig {\n require_opcode INVALID \n}", + shouldErr: true, + }, } serverBlockKeys := []string{"."} @@ -121,8 +165,12 @@ key "name2.key." { } } - if test.expectedAll != ts.all { - t.Errorf("Test %d expected require all to be '%v', but got '%v'.", i, test.expectedAll, ts.all) + if test.expectedAllTypes != ts.allTypes { + t.Errorf("Test %d expected require all types to be '%v', but got '%v'.", i, test.expectedAllTypes, ts.allTypes) + } + + if test.expectedAllOpcodes != ts.allOpcodes { + t.Errorf("Test %d expected require all opcodes to be '%v', but got '%v'.", i, test.expectedAllOpcodes, ts.allOpcodes) } if len(test.expectedQTypes) != len(ts.types) { @@ -135,6 +183,16 @@ key "name2.key." { } } + if len(test.expectedOpCodes) != len(ts.opcodes) { + t.Fatalf("Test %d expected required opcodes '%v', but got '%v'.", i, test.expectedOpCodes, ts.opcodes) + } + for op := range test.expectedOpCodes { + if _, ok := ts.opcodes[op]; !ok { + t.Errorf("Test %d required opcodes '%v', but got '%v'.", i, test.expectedOpCodes, ts.opcodes) + break + } + } + if len(test.expectedSecrets) != len(ts.secrets) { t.Fatalf("Test %d expected secrets '%v', but got '%v'.", i, test.expectedSecrets, ts.secrets) } diff --git a/plugin/tsig/tsig.go b/plugin/tsig/tsig.go index de15558ac..458b765d9 100644 --- a/plugin/tsig/tsig.go +++ b/plugin/tsig/tsig.go @@ -15,14 +15,17 @@ import ( // 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 + Zones []string + secrets map[string]string // [key-name]secret + types qTypes + opcodes opCodes + allTypes bool + allOpcodes bool + Next plugin.Handler } type qTypes map[uint16]struct{} +type opCodes map[int]struct{} // Name implements plugin.Handler func (t TSIGServer) Name() string { return pluginName } @@ -37,7 +40,7 @@ func (t *TSIGServer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns. var tsigRR = r.IsTsig() rcode := dns.RcodeSuccess - if !t.tsigRequired(state.QType()) && tsigRR == nil { + if !t.tsigRequired(state.QType(), r.Opcode) && tsigRR == nil { return plugin.NextOrFailure(t.Name(), t.Next, ctx, w, r) } @@ -88,14 +91,18 @@ func (t *TSIGServer) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns. return dns.RcodeSuccess, nil } -func (t *TSIGServer) tsigRequired(qtype uint16) bool { - if t.all { - return true +func (t *TSIGServer) tsigRequired(qtype uint16, opcode int) bool { + typeMatches := t.allTypes + if !typeMatches { + _, typeMatches = t.types[qtype] } - if _, ok := t.types[qtype]; ok { - return true + + opcodeMatches := t.allOpcodes + if !opcodeMatches { + _, opcodeMatches = t.opcodes[opcode] } - return false + + return typeMatches || opcodeMatches } // restoreTsigWriter Implement Response Writer, and adds a TSIG RR to a response diff --git a/plugin/tsig/tsig_test.go b/plugin/tsig/tsig_test.go index b84b239b4..422340596 100644 --- a/plugin/tsig/tsig_test.go +++ b/plugin/tsig/tsig_test.go @@ -17,40 +17,48 @@ func TestServeDNS(t *testing.T) { cases := []struct { zones []string reqTypes qTypes + reqOpCodes opCodes qType uint16 - qTsig, all bool + opcode int + qTsig bool + allTypes bool + allOpcodes bool expectRcode int expectTsig bool statusError bool }{ { zones: []string{"."}, - all: true, + allTypes: true, qType: dns.TypeA, + opcode: dns.OpcodeQuery, qTsig: true, expectRcode: dns.RcodeSuccess, expectTsig: true, }, { zones: []string{"."}, - all: true, + allTypes: true, qType: dns.TypeA, + opcode: dns.OpcodeQuery, qTsig: false, expectRcode: dns.RcodeRefused, expectTsig: false, }, { zones: []string{"another.domain."}, - all: true, + allTypes: true, qType: dns.TypeA, + opcode: dns.OpcodeQuery, qTsig: false, expectRcode: dns.RcodeSuccess, expectTsig: false, }, { zones: []string{"another.domain."}, - all: true, + allTypes: true, qType: dns.TypeA, + opcode: dns.OpcodeQuery, qTsig: true, expectRcode: dns.RcodeSuccess, expectTsig: false, @@ -59,6 +67,7 @@ func TestServeDNS(t *testing.T) { zones: []string{"."}, reqTypes: qTypes{dns.TypeAXFR: {}}, qType: dns.TypeAXFR, + opcode: dns.OpcodeQuery, qTsig: true, expectRcode: dns.RcodeSuccess, expectTsig: true, @@ -67,6 +76,7 @@ func TestServeDNS(t *testing.T) { zones: []string{"."}, reqTypes: qTypes{}, qType: dns.TypeA, + opcode: dns.OpcodeQuery, qTsig: false, expectRcode: dns.RcodeSuccess, expectTsig: false, @@ -75,28 +85,99 @@ func TestServeDNS(t *testing.T) { zones: []string{"."}, reqTypes: qTypes{}, qType: dns.TypeA, + opcode: dns.OpcodeQuery, qTsig: true, expectRcode: dns.RcodeSuccess, expectTsig: true, }, { zones: []string{"."}, - all: true, + allTypes: true, qType: dns.TypeA, + opcode: dns.OpcodeQuery, qTsig: true, expectRcode: dns.RcodeNotAuth, expectTsig: true, statusError: true, }, + // Opcode-based tests + { + zones: []string{"."}, + reqOpCodes: opCodes{dns.OpcodeUpdate: {}}, + qType: dns.TypeSOA, + opcode: dns.OpcodeUpdate, + qTsig: true, + expectRcode: dns.RcodeSuccess, + expectTsig: true, + }, + { + zones: []string{"."}, + reqOpCodes: opCodes{dns.OpcodeUpdate: {}}, + qType: dns.TypeSOA, + opcode: dns.OpcodeUpdate, + qTsig: false, + expectRcode: dns.RcodeRefused, + expectTsig: false, + }, + { + zones: []string{"."}, + reqOpCodes: opCodes{dns.OpcodeUpdate: {}}, + qType: dns.TypeA, + opcode: dns.OpcodeQuery, + qTsig: false, + expectRcode: dns.RcodeSuccess, + expectTsig: false, + }, + { + zones: []string{"."}, + reqOpCodes: opCodes{dns.OpcodeNotify: {}}, + qType: dns.TypeSOA, + opcode: dns.OpcodeNotify, + qTsig: true, + expectRcode: dns.RcodeSuccess, + expectTsig: true, + }, + // Combined qtype and opcode requirement + { + zones: []string{"."}, + reqTypes: qTypes{dns.TypeAXFR: {}}, + reqOpCodes: opCodes{dns.OpcodeUpdate: {}}, + qType: dns.TypeA, + opcode: dns.OpcodeUpdate, + qTsig: true, + expectRcode: dns.RcodeSuccess, + expectTsig: true, + }, + // allOpcodes test + { + zones: []string{"."}, + allOpcodes: true, + qType: dns.TypeA, + opcode: dns.OpcodeQuery, + qTsig: true, + expectRcode: dns.RcodeSuccess, + expectTsig: true, + }, + { + zones: []string{"."}, + allOpcodes: true, + qType: dns.TypeA, + opcode: dns.OpcodeQuery, + qTsig: false, + expectRcode: dns.RcodeRefused, + expectTsig: false, + }, } 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(), + Zones: tc.zones, + allTypes: tc.allTypes, + allOpcodes: tc.allOpcodes, + types: tc.reqTypes, + opcodes: tc.reqOpCodes, + Next: testHandler(), } ctx := context.TODO() @@ -109,6 +190,7 @@ func TestServeDNS(t *testing.T) { } r := new(dns.Msg) r.SetQuestion("test.example.", tc.qType) + r.Opcode = tc.opcode if tc.qTsig { r.SetTsig("test.key.", dns.HmacSHA256, 300, time.Now().Unix()) } @@ -170,9 +252,9 @@ func TestServeDNSTsigErrors(t *testing.T) { } tsig := TSIGServer{ - Zones: []string{"."}, - all: true, - Next: testHandler(), + Zones: []string{"."}, + allTypes: true, + Next: testHandler(), } for _, tc := range cases {