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 <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: ianlancetaylor

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.

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)
 				}
 			},
 		},

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)
 	}