Skip to content

Commit ca96ca8

Browse files
TomOnTimeTomOnTime
authored
REFACTOR: CAA/DNSKEY/DS/PopulateFromString and others now use modern methods (#4426)
* Rewrite `models.backfill()` to set legacy fields directly. Do not use `SetTarget*()` helpers. This prevents accidental infinite recursion. * `MakeCAA()` now validates the tag string. * BUGFIX: `MyNewData()` appended wrong domain. * Create `models.legacySetTargetArgs()` and `models.legacySetTargetParse()` to make it easier to moderize `models/t_*.go` files * Add `dc.AddTestRCParse()` * Moderize `models.t_caa.go` * Moderize `models/t_dnskey.go` * Modernize `models/t_ds.go` * Rewrite `t_parse.go` to use `legacySetTargetParse()` * Rewrite `PopulateFromString()` to use `PopulateFromStringFunc()` * Modernize `pkg/diff2/analyze_test.go` * Convert `pkg/diff2/handsoff_test.go` to use dnsv2 * Rewrite `pkg/dnsrr/dnsrr.go` to use RecordConfig factories, not `SetTarget*()` helpers. This prevents accidental infinite recursion.
1 parent 8f1283c commit ca96ca8

16 files changed

Lines changed: 385 additions & 331 deletions

File tree

‎models/backfill.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ func backfill(rc *RecordConfig) error {
2828
// no-op
2929

3030
case dnsrdatav2.CAA:
31-
rc.SetTargetCAA(rd.Flag, rd.Tag, rd.Value)
31+
rc.CaaFlag = rd.Flag
32+
rc.CaaTag = rd.Tag
33+
rc.SetTarget(rd.Value)
3234
case privatetypesrdata.CFWORKERROUTE:
3335
rc.SetTarget(fmt.Sprintf("%s,%s", rd.When, rd.Then))
3436
case privatetypesrdata.CLOUDFLAREAPISINGLEREDIRECT:
@@ -41,10 +43,9 @@ func backfill(rc *RecordConfig) error {
4143
case dnsrdatav2.DNAME:
4244
rc.SetTarget(rd.Target)
4345
case dnsrdatav2.DS:
44-
rc.SetTargetDS(rd.KeyTag, rd.Algorithm, rd.DigestType, rd.Digest)
46+
rc.DsKeyTag, rc.DsAlgorithm, rc.DsDigestType, rc.DsDigest = rd.KeyTag, rd.Algorithm, rd.DigestType, rd.Digest
4547
case dnsrdatav2.DNSKEY:
46-
rc.SetTargetDNSKEY(rd.Flags, rd.Protocol, rd.Algorithm, rd.PublicKey)
47-
48+
rc.DnskeyFlags, rc.DnskeyProtocol, rc.DnskeyAlgorithm, rc.DnskeyPublicKey = rd.Flags, rd.Protocol, rd.Algorithm, rd.PublicKey
4849
case privatetypesrdata.FRAME:
4950
rc.SetTarget(rd.Target)
5051

‎models/makers.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package models
55

66
import (
77
"fmt"
8+
"slices"
89
"strings"
910

1011
dnsv2 "codeberg.org/miekg/dns"
@@ -81,7 +82,14 @@ func MakeCAA(origin string, metadata map[string]string, args ...any) (dnsv2.RDAT
8182
}
8283
return dnsrdatav2.CAA{Flag: mustbe.Uint8(flag), Tag: mustbe.RawString(args[0]), Value: mustbe.RawString(args[1])}, nil
8384
}
84-
return dnsrdatav2.CAA{Flag: mustbe.Uint8(args[0]), Tag: mustbe.RawString(args[1]), Value: mustbe.RawString(args[2])}, nil
85+
86+
tag := mustbe.RawString(args[1])
87+
allowedTags := []string{"issue", "issuewild", "iodef", "contactemail", "contactphone", "issuemail", "issuevmc"}
88+
if !slices.Contains(allowedTags, tag) {
89+
return nil, fmt.Errorf("CAA tag (%v) is not one of the valid types", tag)
90+
}
91+
92+
return dnsrdatav2.CAA{Flag: mustbe.Uint8(args[0]), Tag: tag, Value: mustbe.RawString(args[2])}, nil
8593

8694
}
8795
func MakeCNAME(origin string, _ map[string]string, args ...any) (dnsv2.RDATA, error) {

‎models/rdata.go‎

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,22 +51,45 @@ func (rc *RecordConfig) ValidateRDATA() {
5151
}
5252

5353
func MyNewData(typeNum uint16, contents string, origin string) (dnsv2.RDATA, error) {
54-
rd, err := dnsv2.NewData(typeNum, contents, origin)
54+
rd, err := dnsv2.NewData(typeNum, contents, origin+".")
5555
if err != nil {
5656
return nil, err
5757
}
5858

5959
rd2 := assureNotPointerRDATA(rd)
6060

61-
// DNSControl stores TXT data as a single string (see models/t_txt.go); the
62-
// provider is responsible for splitting it into 255-octet segments on the
63-
// wire. The presentation-format parser, however, yields one Txt element per
64-
// segment, so a >255-octet TXT round-trips as multiple strings and its
65-
// RDATA.String() (used for ComparableV3) would differ from the same value
66-
// built via MakeTXT, causing a spurious diff. Rejoin into a single string.
67-
if txt, ok := rd2.(dnsrdatav2.TXT); ok && len(txt.Txt) > 1 {
68-
txt.Txt = []string{strings.Join(txt.Txt, "")}
69-
rd2 = txt
61+
// TODO(tlim): This duplicates code in the MakeTYPE() functions, but
62+
// sadly those functions aren't called by dnsv2.NewData(). It is
63+
// unclear what would be better. Maybe privatetypes.RegisterMaker() can
64+
// also register a function that does this cleanup, and this
65+
// function would call privatetypes.PostParseCleanup(rd)?
66+
67+
switch v := rd2.(type) {
68+
69+
case dnsrdatav2.DS:
70+
v.Digest = strings.ToUpper(v.Digest)
71+
rd2 = v
72+
73+
case dnsrdatav2.SSHFP:
74+
v.FingerPrint = strings.ToUpper(v.FingerPrint)
75+
rd2 = v
76+
77+
case dnsrdatav2.TLSA:
78+
v.Certificate = strings.ToUpper(v.Certificate)
79+
rd2 = v
80+
81+
case dnsrdatav2.TXT:
82+
// DNSControl stores TXT data as a single string (see models/t_txt.go); the
83+
// provider is responsible for splitting it into 255-octet segments on the
84+
// wire. The presentation-format parser, however, yields one Txt element per
85+
// segment, so a >255-octet TXT round-trips as multiple strings and its
86+
// RDATA.String() (used for ComparableV3) would differ from the same value
87+
// built via MakeTXT, causing a spurious diff. Rejoin into a single string.
88+
if len(v.Txt) > 1 {
89+
v.Txt = []string{strings.Join(v.Txt, "")}
90+
rd2 = v
91+
}
92+
7093
}
7194

7295
return rd2, nil

‎models/record_factory.go‎

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@ func (dc *DomainConfig) NewRecordConfigForRRv2toRC(name string, ttl uint32, type
6565
func NewRecordConfigForRRtoRC(origin, name string, ttl uint32, typeNum uint16, args ...any) (*RecordConfig, error) {
6666
mustbe.ValidArgs(args)
6767

68+
// Make sure label is a shortname.
69+
name = strings.ToLower(name)
70+
if before, found := strings.CutSuffix(name, "."+origin+"."); found {
71+
name = before
72+
}
73+
if name == origin+"." {
74+
name = "@"
75+
}
76+
6877
rd, err := privatetypes.TypeToMakeRDATA[typeNum](origin, nil, args...)
6978
if err != nil {
7079
log.Fatalf("NewRecordConfigForRRtoRC: Failed to create RDATA for type %s: %v", dnsutilv2.TypeToString(typeNum), err)
@@ -122,24 +131,89 @@ func newRecordConfigHelper(origin, name string, ttl uint32, typeNum uint16, rd d
122131
return rc, nil
123132
}
124133

125-
func newRecordConfigHelperRC(rc *RecordConfig, typeName string, contents string, origin string) error {
126-
typeNum, err := dnsutilv2.StringToType(typeName)
134+
// func newRecordConfigHelperRC(rc *RecordConfig, typeName string, contents string, origin string) error {
135+
// typeNum, err := dnsutilv2.StringToType(typeName)
136+
// if err != nil {
137+
// return err
138+
// }
139+
// rc.TypeNum = typeNum
140+
// rc.Type = typeName
141+
142+
// rd, err := MyNewData(typeNum, contents, origin)
143+
// if err != nil {
144+
// return err
145+
// }
146+
// rc.SetRDATA(rd)
147+
// rc.FixUp(origin) // Add .ComparableV3
148+
// err = backfill(rc)
149+
// if err != nil {
150+
// return err
151+
// }
152+
// return nil
153+
// }
154+
155+
func legacySetTargetArgs(rc *RecordConfig, typeNum uint16, args ...any) error {
156+
typeStr := dnsutilv2.TypeToString(typeNum)
157+
158+
// Make sure .Type isn't already set to something else.
159+
if rc.Type != "" && rc.Type != typeStr {
160+
return fmt.Errorf("legacySetTargetArgs(%s) called with .Type set to %s", typeStr, rc.Type)
161+
}
162+
163+
rc.TypeNum = typeNum
164+
rc.Type = typeStr
165+
166+
if rc.Metadata == nil {
167+
rc.Metadata = map[string]string{}
168+
}
169+
170+
f, ok := privatetypes.TypeToMakeRDATA[typeNum]
171+
if !ok {
172+
fmt.Printf("NewRecordConfig: failed TypeToMakeRDATA[%d] == nil", typeNum)
173+
return fmt.Errorf("legacySetTargetArgs: failed TypeToMakeRDATA[%d] == nil", typeNum)
174+
}
175+
rd, err := f("", nil, args...)
176+
if err != nil {
177+
log.Fatalf("legacySetTargetArgs: Failed to create RDATA for type %s: %+v", rc.Type, err)
178+
}
179+
rc.SetRDATA(rd)
180+
181+
rc.FixUp("") // Add .ComparableV3
182+
err = backfill(rc) // Port .RDATA to legacy fields.
127183
if err != nil {
128184
return err
129185
}
186+
187+
return nil
188+
}
189+
190+
func legacySetTargetParse(rc *RecordConfig, typeNum uint16, contents string) error {
191+
typeStr := dnsutilv2.TypeToString(typeNum)
192+
193+
// Make sure .Type isn't already set to something else.
194+
if rc.Type != "" && rc.Type != typeStr {
195+
return fmt.Errorf("legacySetTargetArgs(%s) called with .Type set to %s", typeStr, rc.Type)
196+
}
197+
130198
rc.TypeNum = typeNum
131-
rc.Type = typeName
199+
rc.Type = typeStr
132200

133-
rd, err := MyNewData(typeNum, contents, origin)
201+
if rc.Metadata == nil {
202+
rc.Metadata = map[string]string{}
203+
}
204+
205+
rd, err := MyNewData(typeNum, contents, "")
134206
if err != nil {
135207
return err
136208
}
137209
rc.SetRDATA(rd)
138-
rc.FixUp(origin) // Add .ComparableV3
139-
err = backfill(rc)
210+
211+
rc.FixUp("") // Add .ComparableV3
212+
err = backfill(rc) // Port .RDATA to legacy fields.
140213
if err != nil {
141214
return err
142215
}
216+
143217
return nil
144218
}
145219

‎models/record_maketestdata.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,12 @@ func (dc *DomainConfig) AddTestRC(t *testing.T, label string, ttl uint32, typeNu
1717
dc.AddRecordConfig(rc)
1818
return rc
1919
}
20+
21+
func (dc *DomainConfig) AddTestRCParse(label string, ttl uint32, typeNum uint16, contents string) *RecordConfig {
22+
rc, err := dc.NewRecordConfigParse(label, ttl, typeNum, contents)
23+
if err != nil {
24+
panic(fmt.Sprintf("dc.NewRecordConfigParse() returned %v", err))
25+
}
26+
dc.AddRecordConfig(rc)
27+
return rc
28+
}

‎models/t_caa.go‎

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,24 @@
11
package models
22

33
import (
4-
"fmt"
5-
"slices"
6-
"strconv"
4+
dnsv2 "codeberg.org/miekg/dns"
75
)
86

97
// SetTargetCAA sets the CAA fields.
8+
// Deprecated. Use models.NewRecordConfig() instead.
109
func (rc *RecordConfig) SetTargetCAA(flag uint8, tag string, target string) error {
11-
rc.CaaTag = tag
12-
rc.CaaFlag = flag
13-
if err := rc.SetTarget(target); err != nil {
14-
return err
15-
}
16-
if rc.Type == "" {
17-
rc.Type = "CAA"
18-
}
19-
if rc.Type != "CAA" {
20-
panic("assertion failed: SetTargetCAA called when .Type is not CAA")
21-
}
22-
23-
// Per: https://www.iana.org/assignments/pkix-parameters/pkix-parameters.xhtml#caa-properties excluding reserved tags
24-
allowedTags := []string{"issue", "issuewild", "iodef", "contactemail", "contactphone", "issuemail", "issuevmc"}
25-
if !slices.Contains(allowedTags, tag) {
26-
return fmt.Errorf("CAA tag (%v) is not one of the valid types", tag)
27-
}
28-
29-
return nil
10+
return legacySetTargetArgs(rc, dnsv2.TypeCAA, flag, tag, target)
3011
}
3112

3213
// SetTargetCAAStrings is like SetTargetCAA but accepts strings.
14+
// Deprecated. Use models.NewRecordConfig() instead.
3315
func (rc *RecordConfig) SetTargetCAAStrings(flag, tag, target string) error {
34-
i64flag, err := strconv.ParseUint(flag, 10, 8)
35-
if err != nil {
36-
return fmt.Errorf("CAA flag does not fit in 8 bits: %w", err)
37-
}
38-
return rc.SetTargetCAA(uint8(i64flag), tag, target)
16+
return legacySetTargetArgs(rc, dnsv2.TypeCAA, flag, tag, target)
3917
}
4018

4119
// SetTargetCAAString is like SetTargetCAA but accepts one big string.
4220
// Ex: `0 issue "letsencrypt.org"`.
21+
// Deprecated. Use models.NewRecordConfigParse() instead.
4322
func (rc *RecordConfig) SetTargetCAAString(s string) error {
44-
part, err := ParseQuotedFields(s)
45-
if err != nil {
46-
return err
47-
}
48-
if len(part) != 3 {
49-
return fmt.Errorf("CAA value does not contain 3 fields: (%#v)", s)
50-
}
51-
return rc.SetTargetCAAStrings(part[0], part[1], part[2])
23+
return legacySetTargetParse(rc, dnsv2.TypeCAA, s)
5224
}

‎models/t_dnskey.go‎

Lines changed: 13 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,23 @@
11
package models
22

33
import (
4-
"strconv"
5-
"strings"
6-
7-
"github.com/pkg/errors"
4+
dnsv2 "codeberg.org/miekg/dns"
85
)
96

10-
// SetTargetDNSKEY sets the DNSKEY fields.
11-
func (rc *RecordConfig) SetTargetDNSKEY(flags uint16, protocol, algorithm uint8, publicKey string) error {
12-
rc.DnskeyFlags = flags
13-
rc.DnskeyProtocol = protocol
14-
rc.DnskeyAlgorithm = algorithm
15-
rc.DnskeyPublicKey = publicKey
16-
17-
if rc.Type == "" {
18-
rc.Type = "DNSKEY"
19-
}
20-
if rc.Type != "DNSKEY" {
21-
panic("assertion failed: SetTargetDNSKEY called when .Type is not DNSKEY")
22-
}
23-
24-
return nil
25-
}
26-
27-
// SetTargetDNSKEYStrings is like SetTargetDNSKEY but accepts strings.
28-
func (rc *RecordConfig) SetTargetDNSKEYStrings(flags, protocol, algorithm, publicKey string) error {
29-
u16flags, err := strconv.ParseUint(flags, 10, 16)
30-
if err != nil {
31-
return errors.Wrap(err, "DNSKEY Flags can't fit in 16 bits")
32-
}
33-
u8protocol, err := strconv.ParseUint(protocol, 10, 8)
34-
if err != nil {
35-
return errors.Wrap(err, "DNSKEY Protocol can't fit in 8 bits")
36-
}
37-
u8algorithm, err := strconv.ParseUint(algorithm, 10, 8)
38-
if err != nil {
39-
return errors.Wrap(err, "DNSKEY Algorithm can't fit in 8 bits")
40-
}
7+
// // SetTargetDNSKEY sets the DNSKEY fields.
8+
// // Deprecated. Use models.NewRecordConfig() instead.
9+
// func (rc *RecordConfig) SetTargetDNSKEY(flags uint16, protocol, algorithm uint8, publicKey string) error {
10+
// return legacySetTargetArgs(rc, dnsv2.TypeDNSKEY, flags, protocol, algorithm, publicKey)
11+
// }
4112

42-
return rc.SetTargetDNSKEY(uint16(u16flags), uint8(u8protocol), uint8(u8algorithm), publicKey)
43-
}
13+
// // SetTargetDNSKEYStrings is like SetTargetDNSKEY but accepts strings.
14+
// // Deprecated. Use models.NewRecordConfig() instead.
15+
// func (rc *RecordConfig) SetTargetDNSKEYStrings(flags, protocol, algorithm, publicKey string) error {
16+
// return legacySetTargetArgs(rc, dnsv2.TypeDNSKEY, flags, protocol, algorithm, publicKey)
17+
// }
4418

4519
// SetTargetDNSKEYString is like SetTargetDNSKEY but accepts one big string.
20+
// Deprecated. Use models.NewRecordConfigParse() instead.
4621
func (rc *RecordConfig) SetTargetDNSKEYString(s string) error {
47-
part := strings.Fields(s)
48-
if len(part) != 4 {
49-
return errors.Errorf("DNSKEY value does not contain 4 fieldnskey: (%#v)", s)
50-
}
51-
return rc.SetTargetDNSKEYStrings(part[0], part[1], part[2], part[3])
22+
return legacySetTargetParse(rc, dnsv2.TypeDNSKEY, s)
5223
}

0 commit comments

Comments
 (0)