diff --git a/commands/types/dnscontrol.d.ts b/commands/types/dnscontrol.d.ts index 876f339807..5b4b4fc36b 100644 --- a/commands/types/dnscontrol.d.ts +++ b/commands/types/dnscontrol.d.ts @@ -3465,6 +3465,7 @@ declare function SOA(name: string, ns: string, mbox: string, refresh: number, re * * `txtMaxSize` The maximum size for each TXT record. Values over 255 will result in [multiple strings][multi-string]. General recommendation is to [not go higher than 450][record-size] so that DNS responses will still fit in a UDP packet. (Optional. Default: `"255"`) * * `parts:` The individual parts of the SPF settings. * * `flatten:` Which includes should be inlined. For safety purposes the flattening is done on an opt-in basis. If `"*"` is listed, all includes will be flattened... this might create more problems than is solves due to length limitations. + * * `keepIgnoredRedirects:` Inlining an include can move its `redirect=` modifier into a record that has an `all` mechanism, where [RFC 7208 Section 6.1](https://tools.ietf.org/html/rfc7208#section-6.1) requires it to be ignored. Such modifiers are removed. Set this to `true` to keep them. (Optional. Default: `false`) * * [multi-string]: https://tools.ietf.org/html/rfc4408#section-3.1.3 [record-size]: https://tools.ietf.org/html/rfc4408#section-3.1.4 * @@ -3590,7 +3591,7 @@ declare function SOA(name: string, ns: string, mbox: string, refresh: number, re * * @see https://docs.dnscontrol.org/language-reference/domain-modifiers/spf_builder */ -declare function SPF_BUILDER(opts: { label?: string; overflow?: string; overhead1?: string; raw?: string; ttl?: Duration; txtMaxSize?: number; parts: string[]; flatten?: string[] }): DomainModifier; +declare function SPF_BUILDER(opts: { label?: string; overflow?: string; overhead1?: string; raw?: string; ttl?: Duration; txtMaxSize?: number; parts: string[]; flatten?: string[]; keepIgnoredRedirects?: boolean }): DomainModifier; /** * `SRV` adds a [Service locator record](https://www.rfc-editor.org/rfc/rfc2782) to a domain. The name should be the relative label for the record. diff --git a/documentation/language-reference/domain-modifiers/SPF_BUILDER.md b/documentation/language-reference/domain-modifiers/SPF_BUILDER.md index 17ee406560..fc8420cd59 100644 --- a/documentation/language-reference/domain-modifiers/SPF_BUILDER.md +++ b/documentation/language-reference/domain-modifiers/SPF_BUILDER.md @@ -9,6 +9,7 @@ parameters: - txtMaxSize - parts - flatten + - keepIgnoredRedirects parameters_object: true parameter_types: label: string? @@ -19,6 +20,7 @@ parameter_types: txtMaxSize: number? parts: string[] flatten: string[]? + keepIgnoredRedirects: boolean? --- DNSControl can optimize the SPF settings on a domain by flattening (inlining) includes and removing duplicates. DNSControl also makes it easier to document your SPF configuration. @@ -128,6 +130,7 @@ The parameters are: * `txtMaxSize` The maximum size for each TXT record. Values over 255 will result in [multiple strings][multi-string]. General recommendation is to [not go higher than 450][record-size] so that DNS responses will still fit in a UDP packet. (Optional. Default: `"255"`) * `parts:` The individual parts of the SPF settings. * `flatten:` Which includes should be inlined. For safety purposes the flattening is done on an opt-in basis. If `"*"` is listed, all includes will be flattened... this might create more problems than is solves due to length limitations. +* `keepIgnoredRedirects:` Inlining an include can move its `redirect=` modifier into a record that has an `all` mechanism, where [RFC 7208 Section 6.1](https://tools.ietf.org/html/rfc7208#section-6.1) requires it to be ignored. Such modifiers are removed. Set this to `true` to keep them. (Optional. Default: `false`) [multi-string]: https://tools.ietf.org/html/rfc4408#section-3.1.3 [record-size]: https://tools.ietf.org/html/rfc4408#section-3.1.4 diff --git a/pkg/js/helpers.js b/pkg/js/helpers.js index b305bc2064..6bb13a4905 100644 --- a/pkg/js/helpers.js +++ b/pkg/js/helpers.js @@ -1247,6 +1247,7 @@ function LOC_builder_push(value, dms) { // flatten: A list of domains to be flattened. // overhead1: Amout of "buffer room" to reserve on the first item in the spf chain. // txtMaxSize: The maximum size for each TXT string. Values over 255 will result in multiple strings (default: '255') +// keepIgnoredRedirects: Keep redirect= modifiers that flattening moved into a record with an "all" mechanism, which RFC 7208 requires to be ignored. (default: false) function SPF_BUILDER(value) { if (!value.parts || value.parts.length < 2) { @@ -1290,6 +1291,10 @@ function SPF_BUILDER(value) { p.txtMaxSize = value.txtMaxSize; } + if (value.keepIgnoredRedirects) { + p.keepIgnoredRedirects = 'true'; + } + // Generate a TXT record with the metaparameters. if (value.ttl) { r.push(TXT(value.label, rawspf, p, TTL(value.ttl))); diff --git a/pkg/normalize/flatten.go b/pkg/normalize/flatten.go index a42552c21e..06e5cb257a 100644 --- a/pkg/normalize/flatten.go +++ b/pkg/normalize/flatten.go @@ -47,7 +47,11 @@ func flattenSPFs(cfg *models.DNSConfig) []error { } } if flatten, ok := txt.Metadata["flatten"]; ok && strings.HasPrefix(txtTarget, "v=spf1") { - rec = rec.Flatten(flatten) + var opts []spflib.FlattenOption + if txt.Metadata["keepIgnoredRedirects"] == "true" { + opts = append(opts, spflib.KeepIgnoredRedirects()) + } + rec = rec.Flatten(flatten, opts...) err = txt.SetTargetTXT(rec.TXT()) if err != nil { errs = append(errs, err) diff --git a/pkg/spflib/flatten.go b/pkg/spflib/flatten.go index 4ca10a942f..1e96b8b85e 100644 --- a/pkg/spflib/flatten.go +++ b/pkg/spflib/flatten.go @@ -107,8 +107,31 @@ func (s *SPFRecord) split(thisfqdn string, pattern string, nextIdx int, m map[st newRec.split(nextFQDN, pattern, nextIdx+1, m, 0, txtMaxSize) } +// FlattenOption alters the behavior of Flatten. +type FlattenOption func(*flattenOptions) + +type flattenOptions struct { + keepIgnoredRedirects bool +} + +// KeepIgnoredRedirects makes Flatten retain redirect= modifiers that are +// ignored because the record contains an "all" mechanism. +func KeepIgnoredRedirects() FlattenOption { + return func(o *flattenOptions) { + o.keepIgnoredRedirects = true + } +} + // Flatten optimizes s. -func (s *SPFRecord) Flatten(spec string) *SPFRecord { +func (s *SPFRecord) Flatten(spec string, opts ...FlattenOption) *SPFRecord { + var o flattenOptions + for _, opt := range opts { + opt(&o) + } + return s.flatten(spec, o) +} + +func (s *SPFRecord) flatten(spec string, opts flattenOptions) *SPFRecord { newRec := &SPFRecord{} for _, p := range s.Parts { if p.IncludeRecord == nil { @@ -119,7 +142,7 @@ func (s *SPFRecord) Flatten(spec string) *SPFRecord { newRec.Parts = append(newRec.Parts, p) } else { // flatten child recursively - flattenedChild := p.IncludeRecord.Flatten(spec) + flattenedChild := p.IncludeRecord.flatten(spec, opts) // include their parts (skipping final all term) parts := flattenedChild.Parts if n := len(parts); n > 0 && isAllMechanism(parts[n-1].Text) { @@ -128,17 +151,34 @@ func (s *SPFRecord) Flatten(spec string) *SPFRecord { newRec.Parts = append(newRec.Parts, parts...) } } + if !opts.keepIgnoredRedirects { + newRec.dropIgnoredRedirects() + } return newRec } -func isAllMechanism(text string) bool { - if text == "" { - return false +// dropIgnoredRedirects removes the redirect= modifiers of s. RFC 7208 Section +// 6.1 requires them to be ignored when the record has an "all" mechanism. +func (s *SPFRecord) dropIgnoredRedirects() { + if !slices.ContainsFunc(s.Parts, func(p *SPFPart) bool { return isAllMechanism(p.Text) }) { + return } - if qualifiers[text[0]] { - text = text[1:] + s.Parts = slices.DeleteFunc(s.Parts, func(p *SPFPart) bool { + return strings.HasPrefix(trimQualifier(p.Text), "redirect=") + }) +} + +// trimQualifier removes the leading qualifier of a term, as Parse does before +// deciding what the term is. +func trimQualifier(text string) string { + if text != "" && qualifiers[text[0]] { + return text[1:] } - return strings.EqualFold(text, "all") + return text +} + +func isAllMechanism(text string) bool { + return strings.EqualFold(trimQualifier(text), "all") } func matchesFlatSpec(spec, fqdn string) bool { diff --git a/pkg/spflib/flatten_test.go b/pkg/spflib/flatten_test.go index da57e23353..2efd93b044 100644 --- a/pkg/spflib/flatten_test.go +++ b/pkg/spflib/flatten_test.go @@ -127,6 +127,121 @@ func TestFlattenTrailingAll(t *testing.T) { } } +func TestFlattenIgnoredRedirect(t *testing.T) { + tests := []struct { + description string + dnsres fakeResolver + input string + spec string + opts []FlattenOption + want string + }{ + { + description: "redirect of a flattened include is dropped when an all mechanism ignores it", + dnsres: fakeResolver{ + "child.example.net": "v=spf1 ip4:1.2.3.4 redirect=other.example.org", + "other.example.org": "v=spf1 ip4:9.9.9.9 -all", + }, + input: "v=spf1 include:child.example.net -all", + spec: "child.example.net", + want: "v=spf1 ip4:1.2.3.4 -all", + }, + { + description: "redirect is kept when no all mechanism ignores it", + dnsres: fakeResolver{ + "child.example.net": "v=spf1 ip4:1.2.3.4 redirect=other.example.org", + "other.example.org": "v=spf1 ip4:9.9.9.9 -all", + }, + input: "v=spf1 include:child.example.net", + spec: "child.example.net", + want: "v=spf1 ip4:1.2.3.4 redirect=other.example.org", + }, + { + description: "KeepIgnoredRedirects retains a redirect an all mechanism ignores", + dnsres: fakeResolver{ + "child.example.net": "v=spf1 ip4:1.2.3.4 redirect=other.example.org", + "other.example.org": "v=spf1 ip4:9.9.9.9 -all", + }, + input: "v=spf1 include:child.example.net -all", + spec: "child.example.net", + opts: []FlattenOption{KeepIgnoredRedirects()}, + want: "v=spf1 ip4:1.2.3.4 redirect=other.example.org -all", + }, + { + description: "qualified redirect is dropped when an all mechanism ignores it", + dnsres: fakeResolver{ + "child.example.net": "v=spf1 ip4:1.2.3.4 ~redirect=other.example.org", + "other.example.org": "v=spf1 ip4:9.9.9.9 -all", + }, + input: "v=spf1 include:child.example.net -all", + spec: "child.example.net", + want: "v=spf1 ip4:1.2.3.4 -all", + }, + { + description: "qualified redirect is kept when no all mechanism ignores it", + dnsres: fakeResolver{ + "child.example.net": "v=spf1 ip4:1.2.3.4 ~redirect=other.example.org", + "other.example.org": "v=spf1 ip4:9.9.9.9 -all", + }, + input: "v=spf1 include:child.example.net", + spec: "child.example.net", + want: "v=spf1 ip4:1.2.3.4 ~redirect=other.example.org", + }, + { + description: "redirect ignored inside a nested include does not become live in the parent", + dnsres: fakeResolver{ + "a.example.net": "v=spf1 include:b.example.net ~all", + "b.example.net": "v=spf1 ip4:1.2.3.4 redirect=other.example.org", + "other.example.org": "v=spf1 ip4:9.9.9.9 -all", + }, + input: "v=spf1 include:a.example.net", + spec: "a.example.net,b.example.net", + want: "v=spf1 ip4:1.2.3.4", + }, + { + description: "redirect that matches the flatten spec is still inlined", + dnsres: fakeResolver{ + "child.example.net": "v=spf1 ip4:1.2.3.4 redirect=other.example.org", + "other.example.org": "v=spf1 ip4:9.9.9.9 -all", + }, + input: "v=spf1 include:child.example.net -all", + spec: "*", + want: "v=spf1 ip4:1.2.3.4 ip4:9.9.9.9 -all", + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + rec, err := Parse(test.input, test.dnsres) + if err != nil { + t.Fatal(err) + } + if got := rec.Flatten(test.spec, test.opts...).TXT(); got != test.want { + t.Errorf("got %s want %s", got, test.want) + } + }) + } +} + +func TestFlattenOutputParses(t *testing.T) { + for _, qualifier := range []string{"", "+", "~", "-", "?"} { + t.Run(qualifier+"redirect", func(t *testing.T) { + dnsres := fakeResolver{ + "child.example.net": "v=spf1 ip4:1.2.3.4 " + qualifier + "redirect=other.example.org", + "other.example.org": "v=spf1 ip4:9.9.9.9 -all", + } + rec, err := Parse("v=spf1 include:child.example.net -all", dnsres) + if err != nil { + t.Fatal(err) + } + got := rec.Flatten("child.example.net").TXT() + if _, err := Parse(got, dnsres); err != nil { + t.Errorf("Parse(%q) returned %v", got, err) + } + }) + } +} + // each test is array of strings. // first item is unsplit input // next is @ spf record