Skip to content

Commit 296ddf2

Browse files
rolandshoemakerrolandshoemaker
committed
net: filter bad names from Lookup functions instead of hard failing
Instead of hard failing on a single bad record, filter the bad records and return anything valid. This only applies to the methods which can return multiple records, LookupMX, LookupNS, LookupSRV, and LookupAddr. When bad results are filtered out, also return an error, indicating that this filtering has happened. Updates #46241 Fixes #46979 Change-Id: I6493e0002beaf89f5a9795333a93605abd30d171 Reviewed-on: https://go-review.googlesource.com/c/go/+/332549 Trust: Roland Shoemaker <roland@golang.org> Reviewed-by: Filippo Valsorda <filippo@golang.org>
1 parent ce76298 commit 296ddf2

File tree

2 files changed

+229
-67
lines changed

2 files changed

+229
-67
lines changed

‎src/net/dnsclient_unix_test.go‎

Lines changed: 173 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1846,6 +1846,17 @@ func TestCVE202133195(t *testing.T) {
18461846
Target: dnsmessage.MustNewName("<html>.golang.org."),
18471847
},
18481848
},
1849+
dnsmessage.Resource{
1850+
Header: dnsmessage.ResourceHeader{
1851+
Name: n,
1852+
Type: dnsmessage.TypeSRV,
1853+
Class: dnsmessage.ClassINET,
1854+
Length: 4,
1855+
},
1856+
Body: &dnsmessage.SRVResource{
1857+
Target: dnsmessage.MustNewName("good.golang.org."),
1858+
},
1859+
},
18491860
)
18501861
case dnsmessage.TypeMX:
18511862
r.Answers = append(r.Answers,
@@ -1860,6 +1871,17 @@ func TestCVE202133195(t *testing.T) {
18601871
MX: dnsmessage.MustNewName("<html>.golang.org."),
18611872
},
18621873
},
1874+
dnsmessage.Resource{
1875+
Header: dnsmessage.ResourceHeader{
1876+
Name: dnsmessage.MustNewName("good.golang.org."),
1877+
Type: dnsmessage.TypeMX,
1878+
Class: dnsmessage.ClassINET,
1879+
Length: 4,
1880+
},
1881+
Body: &dnsmessage.MXResource{
1882+
MX: dnsmessage.MustNewName("good.golang.org."),
1883+
},
1884+
},
18631885
)
18641886
case dnsmessage.TypeNS:
18651887
r.Answers = append(r.Answers,
@@ -1874,6 +1896,17 @@ func TestCVE202133195(t *testing.T) {
18741896
NS: dnsmessage.MustNewName("<html>.golang.org."),
18751897
},
18761898
},
1899+
dnsmessage.Resource{
1900+
Header: dnsmessage.ResourceHeader{
1901+
Name: dnsmessage.MustNewName("good.golang.org."),
1902+
Type: dnsmessage.TypeNS,
1903+
Class: dnsmessage.ClassINET,
1904+
Length: 4,
1905+
},
1906+
Body: &dnsmessage.NSResource{
1907+
NS: dnsmessage.MustNewName("good.golang.org."),
1908+
},
1909+
},
18771910
)
18781911
case dnsmessage.TypePTR:
18791912
r.Answers = append(r.Answers,
@@ -1888,6 +1921,17 @@ func TestCVE202133195(t *testing.T) {
18881921
PTR: dnsmessage.MustNewName("<html>.golang.org."),
18891922
},
18901923
},
1924+
dnsmessage.Resource{
1925+
Header: dnsmessage.ResourceHeader{
1926+
Name: dnsmessage.MustNewName("good.golang.org."),
1927+
Type: dnsmessage.TypePTR,
1928+
Class: dnsmessage.ClassINET,
1929+
Length: 4,
1930+
},
1931+
Body: &dnsmessage.PTRResource{
1932+
PTR: dnsmessage.MustNewName("good.golang.org."),
1933+
},
1934+
},
18911935
)
18921936
}
18931937
return r, nil
@@ -1903,59 +1947,139 @@ func TestCVE202133195(t *testing.T) {
19031947
defer func(orig string) { testHookHostsPath = orig }(testHookHostsPath)
19041948
testHookHostsPath = "testdata/hosts"
19051949

1906-
_, err := r.LookupCNAME(context.Background(), "golang.org")
1907-
if expected := "lookup golang.org: CNAME target is invalid"; err == nil || err.Error() != expected {
1908-
t.Errorf("Resolver.LookupCNAME returned unexpected error, got %q, want %q", err, expected)
1909-
}
1910-
_, err = LookupCNAME("golang.org")
1911-
if expected := "lookup golang.org: CNAME target is invalid"; err == nil || err.Error() != expected {
1912-
t.Errorf("LookupCNAME returned unexpected error, got %q, want %q", err, expected)
1913-
}
1914-
1915-
_, _, err = r.LookupSRV(context.Background(), "target", "tcp", "golang.org")
1916-
if expected := "lookup golang.org: SRV target is invalid"; err == nil || err.Error() != expected {
1917-
t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err, expected)
1918-
}
1919-
_, _, err = LookupSRV("target", "tcp", "golang.org")
1920-
if expected := "lookup golang.org: SRV target is invalid"; err == nil || err.Error() != expected {
1921-
t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err, expected)
1922-
}
1923-
1924-
_, _, err = r.LookupSRV(context.Background(), "hdr", "tcp", "golang.org.")
1925-
if expected := "lookup golang.org.: SRV header name is invalid"; err == nil || err.Error() != expected {
1926-
t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err, expected)
1927-
}
1928-
_, _, err = LookupSRV("hdr", "tcp", "golang.org.")
1929-
if expected := "lookup golang.org.: SRV header name is invalid"; err == nil || err.Error() != expected {
1930-
t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err, expected)
1931-
}
1932-
1933-
_, err = r.LookupMX(context.Background(), "golang.org")
1934-
if expected := "lookup golang.org: MX target is invalid"; err == nil || err.Error() != expected {
1935-
t.Errorf("Resolver.LookupMX returned unexpected error, got %q, want %q", err, expected)
1936-
}
1937-
_, err = LookupMX("golang.org")
1938-
if expected := "lookup golang.org: MX target is invalid"; err == nil || err.Error() != expected {
1939-
t.Errorf("LookupMX returned unexpected error, got %q, want %q", err, expected)
1950+
tests := []struct {
1951+
name string
1952+
f func(*testing.T)
1953+
}{
1954+
{
1955+
name: "CNAME",
1956+
f: func(t *testing.T) {
1957+
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "golang.org"}
1958+
_, err := r.LookupCNAME(context.Background(), "golang.org")
1959+
if err.Error() != expectedErr.Error() {
1960+
t.Fatalf("unexpected error: %s", err)
1961+
}
1962+
_, err = LookupCNAME("golang.org")
1963+
if err.Error() != expectedErr.Error() {
1964+
t.Fatalf("unexpected error: %s", err)
1965+
}
1966+
},
1967+
},
1968+
{
1969+
name: "SRV (bad record)",
1970+
f: func(t *testing.T) {
1971+
expected := []*SRV{
1972+
{
1973+
Target: "good.golang.org.",
1974+
},
1975+
}
1976+
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "golang.org"}
1977+
_, records, err := r.LookupSRV(context.Background(), "target", "tcp", "golang.org")
1978+
if err.Error() != expectedErr.Error() {
1979+
t.Fatalf("unexpected error: %s", err)
1980+
}
1981+
if !reflect.DeepEqual(records, expected) {
1982+
t.Error("Unexpected record set")
1983+
}
1984+
_, records, err = LookupSRV("target", "tcp", "golang.org")
1985+
if err.Error() != expectedErr.Error() {
1986+
t.Errorf("unexpected error: %s", err)
1987+
}
1988+
if !reflect.DeepEqual(records, expected) {
1989+
t.Error("Unexpected record set")
1990+
}
1991+
},
1992+
},
1993+
{
1994+
name: "SRV (bad header)",
1995+
f: func(t *testing.T) {
1996+
_, _, err := r.LookupSRV(context.Background(), "hdr", "tcp", "golang.org.")
1997+
if expected := "lookup golang.org.: SRV header name is invalid"; err == nil || err.Error() != expected {
1998+
t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err, expected)
1999+
}
2000+
_, _, err = LookupSRV("hdr", "tcp", "golang.org.")
2001+
if expected := "lookup golang.org.: SRV header name is invalid"; err == nil || err.Error() != expected {
2002+
t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err, expected)
2003+
}
2004+
},
2005+
},
2006+
{
2007+
name: "MX",
2008+
f: func(t *testing.T) {
2009+
expected := []*MX{
2010+
{
2011+
Host: "good.golang.org.",
2012+
},
2013+
}
2014+
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "golang.org"}
2015+
records, err := r.LookupMX(context.Background(), "golang.org")
2016+
if err.Error() != expectedErr.Error() {
2017+
t.Fatalf("unexpected error: %s", err)
2018+
}
2019+
if !reflect.DeepEqual(records, expected) {
2020+
t.Error("Unexpected record set")
2021+
}
2022+
records, err = LookupMX("golang.org")
2023+
if err.Error() != expectedErr.Error() {
2024+
t.Fatalf("unexpected error: %s", err)
2025+
}
2026+
if !reflect.DeepEqual(records, expected) {
2027+
t.Error("Unexpected record set")
2028+
}
2029+
},
2030+
},
2031+
{
2032+
name: "NS",
2033+
f: func(t *testing.T) {
2034+
expected := []*NS{
2035+
{
2036+
Host: "good.golang.org.",
2037+
},
2038+
}
2039+
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "golang.org"}
2040+
records, err := r.LookupNS(context.Background(), "golang.org")
2041+
if err.Error() != expectedErr.Error() {
2042+
t.Fatalf("unexpected error: %s", err)
2043+
}
2044+
if !reflect.DeepEqual(records, expected) {
2045+
t.Error("Unexpected record set")
2046+
}
2047+
records, err = LookupNS("golang.org")
2048+
if err.Error() != expectedErr.Error() {
2049+
t.Fatalf("unexpected error: %s", err)
2050+
}
2051+
if !reflect.DeepEqual(records, expected) {
2052+
t.Error("Unexpected record set")
2053+
}
2054+
},
2055+
},
2056+
{
2057+
name: "Addr",
2058+
f: func(t *testing.T) {
2059+
expected := []string{"good.golang.org."}
2060+
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "192.0.2.42"}
2061+
records, err := r.LookupAddr(context.Background(), "192.0.2.42")
2062+
if err.Error() != expectedErr.Error() {
2063+
t.Fatalf("unexpected error: %s", err)
2064+
}
2065+
if !reflect.DeepEqual(records, expected) {
2066+
t.Error("Unexpected record set")
2067+
}
2068+
records, err = LookupAddr("192.0.2.42")
2069+
if err.Error() != expectedErr.Error() {
2070+
t.Fatalf("unexpected error: %s", err)
2071+
}
2072+
if !reflect.DeepEqual(records, expected) {
2073+
t.Error("Unexpected record set")
2074+
}
2075+
},
2076+
},
19402077
}
19412078

1942-
_, err = r.LookupNS(context.Background(), "golang.org")
1943-
if expected := "lookup golang.org: NS target is invalid"; err == nil || err.Error() != expected {
1944-
t.Errorf("Resolver.LookupNS returned unexpected error, got %q, want %q", err, expected)
1945-
}
1946-
_, err = LookupNS("golang.org")
1947-
if expected := "lookup golang.org: NS target is invalid"; err == nil || err.Error() != expected {
1948-
t.Errorf("LookupNS returned unexpected error, got %q, want %q", err, expected)
2079+
for _, tc := range tests {
2080+
t.Run(tc.name, tc.f)
19492081
}
19502082

1951-
_, err = r.LookupAddr(context.Background(), "192.0.2.42")
1952-
if expected := "lookup 192.0.2.42: PTR target is invalid"; err == nil || err.Error() != expected {
1953-
t.Errorf("Resolver.LookupAddr returned unexpected error, got %q, want %q", err, expected)
1954-
}
1955-
_, err = LookupAddr("192.0.2.42")
1956-
if expected := "lookup 192.0.2.42: PTR target is invalid"; err == nil || err.Error() != expected {
1957-
t.Errorf("LookupAddr returned unexpected error, got %q, want %q", err, expected)
1958-
}
19592083
}
19602084

19612085
func TestNullMX(t *testing.T) {

0 commit comments

Comments
 (0)