From 13238cb6adfeaa10002578a7fa18fc5947702e30 Mon Sep 17 00:00:00 2001 From: Tom Thorogood Date: Sat, 30 Jan 2021 23:35:25 +1030 Subject: [PATCH] Support parsing known RR types in RFC 3597 format (#1211) * Support parsing known RR types in RFC 3597 format This is the format used for "Unknown DNS Resource Records", but it's also useful to support parsing known RR types in this way. RFC 3597 says: An implementation MAY also choose to represent some RRs of known type using the above generic representations for the type, class and/or RDATA, which carries the benefit of making the resulting master file portable to servers where these types are unknown. Using the generic representation for the RDATA of an RR of known type can also be useful in the case of an RR type where the text format varies depending on a version, protocol, or similar field (or several) embedded in the RDATA when such a field has a value for which no text format is known, e.g., a LOC RR [RFC1876] with a VERSION other than 0. Even though an RR of known type represented in the \# format is effectively treated as an unknown type for the purpose of parsing the RDATA text representation, all further processing by the server MUST treat it as a known type and take into account any applicable type- specific rules regarding compression, canonicalization, etc. * Correct mistakes in TestZoneParserAddressAAAA This was spotted when writing TestParseKnownRRAsRFC3597. * Eliminate canParseAsRR This has the advantage that concrete types will now be returned for parsed ANY, NULL, OPT and TSIG records. * Expand TestDynamicUpdateParsing for RFC 3597 This ensures we're properly handling empty RDATA for RFC 3597 parsed records. --- dns.go | 25 ++++++++++++++++++++++--- edns.go | 4 ++-- parse_test.go | 4 ++-- scan.go | 51 +++++++++++++++++++++++++++++++------------------- scan_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++--- tsig.go | 4 ++-- types.go | 8 ++++---- update_test.go | 27 +++++++++++++++++++------- 8 files changed, 129 insertions(+), 42 deletions(-) diff --git a/dns.go b/dns.go index ad83a27e..33c93b2b 100644 --- a/dns.go +++ b/dns.go @@ -1,6 +1,9 @@ package dns -import "strconv" +import ( + "encoding/hex" + "strconv" +) const ( year68 = 1 << 31 // For RFC1982 (Serial Arithmetic) calculations in 32 bits. @@ -111,7 +114,7 @@ func (h *RR_Header) parse(c *zlexer, origin string) *ParseError { // ToRFC3597 converts a known RR to the unknown RR representation from RFC 3597. func (rr *RFC3597) ToRFC3597(r RR) error { - buf := make([]byte, Len(r)*2) + buf := make([]byte, Len(r)) headerEnd, off, err := packRR(r, buf, 0, compressionMap{}, false) if err != nil { return err @@ -126,9 +129,25 @@ func (rr *RFC3597) ToRFC3597(r RR) error { } _, err = rr.unpack(buf, headerEnd) + return err +} + +// fromRFC3597 converts an unknown RR representation from RFC 3597 to the known RR type. +func (rr *RFC3597) fromRFC3597(r RR) error { + *r.Header() = rr.Hdr + + if len(rr.Rdata) == 0 { + // Dynamic update. + return nil + } + + // rr.pack requires an extra allocation and a copy so we just decode Rdata + // manually, it's simpler anyway. + msg, err := hex.DecodeString(rr.Rdata) if err != nil { return err } - return nil + _, err = r.unpack(msg, 0) + return err } diff --git a/edns.go b/edns.go index 04808d57..f3fb1c68 100644 --- a/edns.go +++ b/edns.go @@ -88,8 +88,8 @@ func (rr *OPT) len(off int, compression map[string]struct{}) int { return l } -func (rr *OPT) parse(c *zlexer, origin string) *ParseError { - panic("dns: internal error: parse should never be called on OPT") +func (*OPT) parse(c *zlexer, origin string) *ParseError { + return &ParseError{err: "OPT records do not have a presentation format"} } func (r1 *OPT) isDuplicate(r2 RR) bool { return false } diff --git a/parse_test.go b/parse_test.go index d839562a..886f46cc 100644 --- a/parse_test.go +++ b/parse_test.go @@ -1208,8 +1208,8 @@ func TestTypeXXXX(t *testing.T) { t.Errorf("this should not work, for TYPE655341") } _, err = NewRR("example.com IN TYPE1 \\# 4 0a000001") - if err == nil { - t.Errorf("this should not work") + if err != nil { + t.Errorf("failed to parse TYPE1 RR: %v", err) } } diff --git a/scan.go b/scan.go index aa2840ef..67161de2 100644 --- a/scan.go +++ b/scan.go @@ -577,10 +577,23 @@ func (zp *ZoneParser) Next() (RR, bool) { st = zExpectRdata case zExpectRdata: - var rr RR - if newFn, ok := TypeToRR[h.Rrtype]; ok && canParseAsRR(h.Rrtype) { + var ( + rr RR + parseAsRFC3597 bool + ) + if newFn, ok := TypeToRR[h.Rrtype]; ok { rr = newFn() *rr.Header() = *h + + // We may be parsing a known RR type using the RFC3597 format. + // If so, we handle that here in a generic way. + // + // This is also true for PrivateRR types which will have the + // RFC3597 parsing done for them and the Unpack method called + // to populate the RR instead of simply deferring to Parse. + if zp.c.Peek().token == "\\#" { + parseAsRFC3597 = true + } } else { rr = &RFC3597{Hdr: *h} } @@ -600,13 +613,18 @@ func (zp *ZoneParser) Next() (RR, bool) { return zp.setParseError("unexpected newline", l) } - if err := rr.parse(zp.c, zp.origin); err != nil { + parseAsRR := rr + if parseAsRFC3597 { + parseAsRR = &RFC3597{Hdr: *h} + } + + if err := parseAsRR.parse(zp.c, zp.origin); err != nil { // err is a concrete *ParseError without the file field set. // The setParseError call below will construct a new // *ParseError with file set to zp.file. - // If err.lex is nil than we have encounter an unknown RR type - // in that case we substitute our current lex token. + // err.lex may be nil in which case we substitute our current + // lex token. if err.lex == (lex{}) { return zp.setParseError(err.err, l) } @@ -614,6 +632,13 @@ func (zp *ZoneParser) Next() (RR, bool) { return zp.setParseError(err.err, err.lex) } + if parseAsRFC3597 { + err := parseAsRR.(*RFC3597).fromRFC3597(rr) + if err != nil { + return zp.setParseError(err.Error(), l) + } + } + return rr, true } } @@ -623,18 +648,6 @@ func (zp *ZoneParser) Next() (RR, bool) { return nil, false } -// canParseAsRR returns true if the record type can be parsed as a -// concrete RR. It blacklists certain record types that must be parsed -// according to RFC 3597 because they lack a presentation format. -func canParseAsRR(rrtype uint16) bool { - switch rrtype { - case TypeANY, TypeNULL, TypeOPT, TypeTSIG: - return false - default: - return true - } -} - type zlexer struct { br io.ByteReader @@ -1290,7 +1303,7 @@ func appendOrigin(name, origin string) string { // LOC record helper function func locCheckNorth(token string, latitude uint32) (uint32, bool) { - if latitude > 90 * 1000 * 60 * 60 { + if latitude > 90*1000*60*60 { return latitude, false } switch token { @@ -1304,7 +1317,7 @@ func locCheckNorth(token string, latitude uint32) (uint32, bool) { // LOC record helper function func locCheckEast(token string, longitude uint32) (uint32, bool) { - if longitude > 180 * 1000 * 60 * 60 { + if longitude > 180*1000*60*60 { return longitude, false } switch token { diff --git a/scan_test.go b/scan_test.go index 578deba2..08424e09 100644 --- a/scan_test.go +++ b/scan_test.go @@ -145,10 +145,10 @@ func TestZoneParserAddressAAAA(t *testing.T) { } aaaa, ok := got.(*AAAA) if !ok { - t.Fatalf("expected *AAAA RR, but got %T", aaaa) + t.Fatalf("expected *AAAA RR, but got %T", got) } - if g, w := aaaa.AAAA, tc.want.AAAA; !g.Equal(w) { - t.Fatalf("expected AAAA with IP %v, but got %v", g, w) + if !aaaa.AAAA.Equal(tc.want.AAAA) { + t.Fatalf("expected AAAA with IP %v, but got %v", tc.want.AAAA, aaaa.AAAA) } } } @@ -229,6 +229,48 @@ example.com. 60 PX ( } } +func TestParseKnownRRAsRFC3597(t *testing.T) { + t.Run("with RDATA", func(t *testing.T) { + rr, err := NewRR("example. 3600 CLASS1 TYPE1 \\# 4 7f000001") + if err != nil { + t.Fatalf("failed to parse RFC3579 format: %v", err) + } + + if rr.Header().Rrtype != TypeA { + t.Errorf("expected TypeA (1) Rrtype, but got %v", rr.Header().Rrtype) + } + + a, ok := rr.(*A) + if !ok { + t.Fatalf("expected *A RR, but got %T", rr) + } + + localhost := net.IPv4(127, 0, 0, 1) + if !a.A.Equal(localhost) { + t.Fatalf("expected A with IP %v, but got %v", localhost, a.A) + } + }) + t.Run("without RDATA", func(t *testing.T) { + rr, err := NewRR("example. 3600 CLASS1 TYPE1 \\# 0") + if err != nil { + t.Fatalf("failed to parse RFC3579 format: %v", err) + } + + if rr.Header().Rrtype != TypeA { + t.Errorf("expected TypeA (1) Rrtype, but got %v", rr.Header().Rrtype) + } + + a, ok := rr.(*A) + if !ok { + t.Fatalf("expected *A RR, but got %T", rr) + } + + if len(a.A) != 0 { + t.Fatalf("expected A with empty IP, but got %v", a.A) + } + }) +} + func BenchmarkNewRR(b *testing.B) { const name1 = "12345678901234567890123456789012345.12345678.123." const s = name1 + " 3600 IN MX 10 " + name1 diff --git a/tsig.go b/tsig.go index 5b52e1c4..b49562d8 100644 --- a/tsig.go +++ b/tsig.go @@ -106,8 +106,8 @@ func (rr *TSIG) String() string { return s } -func (rr *TSIG) parse(c *zlexer, origin string) *ParseError { - panic("dns: internal error: parse should never be called on TSIG") +func (*TSIG) parse(c *zlexer, origin string) *ParseError { + return &ParseError{err: "TSIG records do not have a presentation format"} } // The following values must be put in wireformat, so that the MAC can be calculated. diff --git a/types.go b/types.go index 1f385bd2..9e379eb3 100644 --- a/types.go +++ b/types.go @@ -245,8 +245,8 @@ type ANY struct { func (rr *ANY) String() string { return rr.Hdr.String() } -func (rr *ANY) parse(c *zlexer, origin string) *ParseError { - panic("dns: internal error: parse should never be called on ANY") +func (*ANY) parse(c *zlexer, origin string) *ParseError { + return &ParseError{err: "ANY records do not have a presentation format"} } // NULL RR. See RFC 1035. @@ -260,8 +260,8 @@ func (rr *NULL) String() string { return ";" + rr.Hdr.String() + rr.Data } -func (rr *NULL) parse(c *zlexer, origin string) *ParseError { - panic("dns: internal error: parse should never be called on NULL") +func (*NULL) parse(c *zlexer, origin string) *ParseError { + return &ParseError{err: "NULL records do not have a presentation format"} } // CNAME RR. See RFC 1034. diff --git a/update_test.go b/update_test.go index 5dba413a..380376c9 100644 --- a/update_test.go +++ b/update_test.go @@ -6,15 +6,28 @@ import ( ) func TestDynamicUpdateParsing(t *testing.T) { - prefix := "example.com. IN " - for _, typ := range TypeToString { - if typ == "OPT" || typ == "AXFR" || typ == "IXFR" || typ == "ANY" || typ == "TKEY" || - typ == "TSIG" || typ == "ISDN" || typ == "UNSPEC" || typ == "NULL" || typ == "ATMA" || - typ == "Reserved" || typ == "None" || typ == "NXT" || typ == "MAILB" || typ == "MAILA" { + const prefix = "example.com. IN " + + for typ, name := range TypeToString { + switch typ { + case TypeNone, TypeReserved: + continue + case TypeANY: + // ANY is ambiguous here and ends up parsed as a CLASS. + // + // TODO(tmthrgd): Using TYPE255 here doesn't seem to work and also + // seems to fail for some other record types. Investigate. continue } - if _, err := NewRR(prefix + typ); err != nil { - t.Errorf("failure to parse: %s %s: %v", prefix, typ, err) + + s := prefix + name + if _, err := NewRR(s); err != nil { + t.Errorf("failure to parse: %s: %v", s, err) + } + + s += " \\# 0" + if _, err := NewRR(s); err != nil { + t.Errorf("failure to parse: %s: %v", s, err) } } }