Skip to content

Commit fb052db

Browse files
rolandshoemakerrolandshoemaker
committed
[release-branch.go1.16] 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 Updates #46979 Fixes #46999 Change-Id: I6493e0002beaf89f5a9795333a93605abd30d171 Reviewed-on: https://go-review.googlesource.com/c/go/+/332549 Trust: Roland Shoemaker <[email protected]> Reviewed-by: Filippo Valsorda <[email protected]> (cherry picked from commit 296ddf2) Reviewed-on: https://go-review.googlesource.com/c/go/+/333330 Run-TryBot: Roland Shoemaker <[email protected]> TryBot-Result: Go Bot <[email protected]>
1 parent a91ed83 commit fb052db

File tree

2 files changed

+235
-68
lines changed

2 files changed

+235
-68
lines changed

‎src/net/dnsclient_unix_test.go

+173-50
Original file line numberDiff line numberDiff line change
@@ -1845,6 +1845,17 @@ func TestCVE202133195(t *testing.T) {
18451845
Target: dnsmessage.MustNewName("<html>.golang.org."),
18461846
},
18471847
},
1848+
dnsmessage.Resource{
1849+
Header: dnsmessage.ResourceHeader{
1850+
Name: n,
1851+
Type: dnsmessage.TypeSRV,
1852+
Class: dnsmessage.ClassINET,
1853+
Length: 4,
1854+
},
1855+
Body: &dnsmessage.SRVResource{
1856+
Target: dnsmessage.MustNewName("good.golang.org."),
1857+
},
1858+
},
18481859
)
18491860
case dnsmessage.TypeMX:
18501861
r.Answers = append(r.Answers,
@@ -1859,6 +1870,17 @@ func TestCVE202133195(t *testing.T) {
18591870
MX: dnsmessage.MustNewName("<html>.golang.org."),
18601871
},
18611872
},
1873+
dnsmessage.Resource{
1874+
Header: dnsmessage.ResourceHeader{
1875+
Name: dnsmessage.MustNewName("good.golang.org."),
1876+
Type: dnsmessage.TypeMX,
1877+
Class: dnsmessage.ClassINET,
1878+
Length: 4,
1879+
},
1880+
Body: &dnsmessage.MXResource{
1881+
MX: dnsmessage.MustNewName("good.golang.org."),
1882+
},
1883+
},
18621884
)
18631885
case dnsmessage.TypeNS:
18641886
r.Answers = append(r.Answers,
@@ -1873,6 +1895,17 @@ func TestCVE202133195(t *testing.T) {
18731895
NS: dnsmessage.MustNewName("<html>.golang.org."),
18741896
},
18751897
},
1898+
dnsmessage.Resource{
1899+
Header: dnsmessage.ResourceHeader{
1900+
Name: dnsmessage.MustNewName("good.golang.org."),
1901+
Type: dnsmessage.TypeNS,
1902+
Class: dnsmessage.ClassINET,
1903+
Length: 4,
1904+
},
1905+
Body: &dnsmessage.NSResource{
1906+
NS: dnsmessage.MustNewName("good.golang.org."),
1907+
},
1908+
},
18761909
)
18771910
case dnsmessage.TypePTR:
18781911
r.Answers = append(r.Answers,
@@ -1887,6 +1920,17 @@ func TestCVE202133195(t *testing.T) {
18871920
PTR: dnsmessage.MustNewName("<html>.golang.org."),
18881921
},
18891922
},
1923+
dnsmessage.Resource{
1924+
Header: dnsmessage.ResourceHeader{
1925+
Name: dnsmessage.MustNewName("good.golang.org."),
1926+
Type: dnsmessage.TypePTR,
1927+
Class: dnsmessage.ClassINET,
1928+
Length: 4,
1929+
},
1930+
Body: &dnsmessage.PTRResource{
1931+
PTR: dnsmessage.MustNewName("good.golang.org."),
1932+
},
1933+
},
18901934
)
18911935
}
18921936
return r, nil
@@ -1902,58 +1946,137 @@ func TestCVE202133195(t *testing.T) {
19021946
defer func(orig string) { testHookHostsPath = orig }(testHookHostsPath)
19031947
testHookHostsPath = "testdata/hosts"
19041948

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

1950-
_, err = r.LookupAddr(context.Background(), "192.0.2.42")
1951-
if expected := "lookup 192.0.2.42: PTR target is invalid"; err == nil || err.Error() != expected {
1952-
t.Errorf("Resolver.LookupAddr returned unexpected error, got %q, want %q", err, expected)
1953-
}
1954-
_, err = LookupAddr("192.0.2.42")
1955-
if expected := "lookup 192.0.2.42: PTR target is invalid"; err == nil || err.Error() != expected {
1956-
t.Errorf("LookupAddr returned unexpected error, got %q, want %q", err, expected)
2078+
for _, tc := range tests {
2079+
t.Run(tc.name, tc.f)
19572080
}
19582081
}
19592082

0 commit comments

Comments
 (0)