From f77bba43aa223fc86fd223f3ea4ef60db8e0c583 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 20 Feb 2025 12:12:35 -0800 Subject: [PATCH] net: accept a valid IP address in LookupMX Fixes #56025 Change-Id: I202fdd0e11afeb22c5bc22d91fe4bfea8987e727 Reviewed-on: https://go-review.googlesource.com/c/go/+/651056 Reviewed-by: Michael Knyszek Reviewed-by: Roland Shoemaker Auto-Submit: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI --- doc/next/6-stdlib/99-minor/net/56025.md | 5 ++ src/net/dnsclient_unix_test.go | 72 ++++++++++++++++++++++--- src/net/lookup.go | 20 ++++--- 3 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 doc/next/6-stdlib/99-minor/net/56025.md diff --git a/doc/next/6-stdlib/99-minor/net/56025.md b/doc/next/6-stdlib/99-minor/net/56025.md new file mode 100644 index 0000000000..3d1af6c2b6 --- /dev/null +++ b/doc/next/6-stdlib/99-minor/net/56025.md @@ -0,0 +1,5 @@ +[LookupMX] and [(*Resolver).LookupMX] now return DNS names that look +like valid IP address, as well as valid domain names. +Previously if a name server returned an IP address as a DNS name, +LookupMX would discard it, as required by the RFCs. +However, name servers in practice do sometimes return IP addresses. diff --git a/src/net/dnsclient_unix_test.go b/src/net/dnsclient_unix_test.go index c4e5194a34..826b4daba1 100644 --- a/src/net/dnsclient_unix_test.go +++ b/src/net/dnsclient_unix_test.go @@ -2028,6 +2028,50 @@ func TestCVE202133195(t *testing.T) { MX: dnsmessage.MustNewName("good.golang.org."), }, }, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("127.0.0.1."), + Type: dnsmessage.TypeMX, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.MXResource{ + MX: dnsmessage.MustNewName("127.0.0.1."), + }, + }, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("1.2.3.4.5."), + Type: dnsmessage.TypeMX, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.MXResource{ + MX: dnsmessage.MustNewName("1.2.3.4.5."), + }, + }, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("2001:4860:0:2001::68."), + Type: dnsmessage.TypeMX, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.MXResource{ + MX: dnsmessage.MustNewName("2001:4860:0:2001::68."), + }, + }, + dnsmessage.Resource{ + Header: dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("2001:4860:0:2001::68%zone."), + Type: dnsmessage.TypeMX, + Class: dnsmessage.ClassINET, + Length: 4, + }, + Body: &dnsmessage.MXResource{ + MX: dnsmessage.MustNewName("2001:4860:0:2001::68%zone."), + }, + }, ) case dnsmessage.TypeNS: r.Answers = append(r.Answers, @@ -2152,25 +2196,37 @@ func TestCVE202133195(t *testing.T) { { name: "MX", f: func(t *testing.T) { - expected := []*MX{ - { - Host: "good.golang.org.", - }, + expected := []string{ + "127.0.0.1.", + "2001:4860:0:2001::68.", + "good.golang.org.", } expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "golang.org"} records, err := r.LookupMX(context.Background(), "golang.org") if err.Error() != expectedErr.Error() { t.Fatalf("unexpected error: %s", err) } - if !reflect.DeepEqual(records, expected) { - t.Error("Unexpected record set") + + hosts := func(records []*MX) []string { + var got []string + for _, mx := range records { + got = append(got, mx.Host) + } + slices.Sort(got) + return got + } + + got := hosts(records) + if !slices.Equal(got, expected) { + t.Errorf("Unexpected record set: got %v, want %v", got, expected) } records, err = LookupMX("golang.org") if err.Error() != expectedErr.Error() { t.Fatalf("unexpected error: %s", err) } - if !reflect.DeepEqual(records, expected) { - t.Error("Unexpected record set") + got = hosts(records) + if !slices.Equal(got, expected) { + t.Errorf("Unexpected record set: got %v, want %v", got, expected) } }, }, diff --git a/src/net/lookup.go b/src/net/lookup.go index f94fd8cefa..d4be8eaa0e 100644 --- a/src/net/lookup.go +++ b/src/net/lookup.go @@ -9,6 +9,7 @@ import ( "errors" "internal/nettrace" "internal/singleflight" + "internal/stringslite" "net/netip" "sync" @@ -535,9 +536,9 @@ func (r *Resolver) LookupSRV(ctx context.Context, service, proto, name string) ( // LookupMX returns the DNS MX records for the given domain name sorted by preference. // // The returned mail server names are validated to be properly -// formatted presentation-format domain names. If the response contains -// invalid names, those records are filtered out and an error -// will be returned alongside the remaining results, if any. +// formatted presentation-format domain names, or numeric IP addresses. +// If the response contains invalid names, those records are filtered out +// and an error will be returned alongside the remaining results, if any. // // LookupMX uses [context.Background] internally; to specify the context, use // [Resolver.LookupMX]. @@ -548,9 +549,9 @@ func LookupMX(name string) ([]*MX, error) { // LookupMX returns the DNS MX records for the given domain name sorted by preference. // // The returned mail server names are validated to be properly -// formatted presentation-format domain names. If the response contains -// invalid names, those records are filtered out and an error -// will be returned alongside the remaining results, if any. +// formatted presentation-format domain names, or numeric IP addresses. +// If the response contains invalid names, those records are filtered out +// and an error will be returned alongside the remaining results, if any. func (r *Resolver) LookupMX(ctx context.Context, name string) ([]*MX, error) { records, err := r.lookupMX(ctx, name) if err != nil { @@ -562,7 +563,12 @@ func (r *Resolver) LookupMX(ctx context.Context, name string) ([]*MX, error) { continue } if !isDomainName(mx.Host) { - continue + // Check for IP address. In practice we observe + // these with a trailing dot, so strip that. + ip, err := netip.ParseAddr(stringslite.TrimSuffix(mx.Host, ".")) + if err != nil || ip.Zone() != "" { + continue + } } filteredMX = append(filteredMX, mx) }