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 <mknyszek@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

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 (file)
index 0000000..3d1af6c
--- /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 c4e5194a34084598f81d51cf3822db0e627f7688..826b4daba1e7fd71f4bb5cab97b161fca4108b49 100644 (file)
--- 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 f94fd8cefaab1838514aab57c866ec9cfb557a73..d4be8eaa0e10c3786fd99a4358daa0b71f94c504 100644 (file)
--- 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)
        }