Skip to content

Commit

Permalink
[release-branch.go1.15] net: filter bad names from Lookup functions i…
Browse files Browse the repository at this point in the history 
…nstead 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 #47012

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/+/333331
Run-TryBot: Roland Shoemaker <[email protected]>
  • Loading branch information
@rolandshoemaker
rolandshoemaker committed Jul 8, 2021
1 parent 0579cf1 commit 3d1d066
Show file tree
Hide file tree
Showing 2 changed files with 235 additions and 68 deletions.
223 changes: 173 additions & 50 deletions src/net/dnsclient_unix_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1846,6 +1846,17 @@ func TestCVE202133195(t *testing.T) {
Target: dnsmessage.MustNewName("<html>.golang.org."),
},
},
dnsmessage.Resource{
Header: dnsmessage.ResourceHeader{
Name: n,
Type: dnsmessage.TypeSRV,
Class: dnsmessage.ClassINET,
Length: 4,
},
Body: &dnsmessage.SRVResource{
Target: dnsmessage.MustNewName("good.golang.org."),
},
},
)
case dnsmessage.TypeMX:
r.Answers = append(r.Answers,
Expand All @@ -1860,6 +1871,17 @@ func TestCVE202133195(t *testing.T) {
MX: dnsmessage.MustNewName("<html>.golang.org."),
},
},
dnsmessage.Resource{
Header: dnsmessage.ResourceHeader{
Name: dnsmessage.MustNewName("good.golang.org."),
Type: dnsmessage.TypeMX,
Class: dnsmessage.ClassINET,
Length: 4,
},
Body: &dnsmessage.MXResource{
MX: dnsmessage.MustNewName("good.golang.org."),
},
},
)
case dnsmessage.TypeNS:
r.Answers = append(r.Answers,
Expand All @@ -1874,6 +1896,17 @@ func TestCVE202133195(t *testing.T) {
NS: dnsmessage.MustNewName("<html>.golang.org."),
},
},
dnsmessage.Resource{
Header: dnsmessage.ResourceHeader{
Name: dnsmessage.MustNewName("good.golang.org."),
Type: dnsmessage.TypeNS,
Class: dnsmessage.ClassINET,
Length: 4,
},
Body: &dnsmessage.NSResource{
NS: dnsmessage.MustNewName("good.golang.org."),
},
},
)
case dnsmessage.TypePTR:
r.Answers = append(r.Answers,
Expand All @@ -1888,6 +1921,17 @@ func TestCVE202133195(t *testing.T) {
PTR: dnsmessage.MustNewName("<html>.golang.org."),
},
},
dnsmessage.Resource{
Header: dnsmessage.ResourceHeader{
Name: dnsmessage.MustNewName("good.golang.org."),
Type: dnsmessage.TypePTR,
Class: dnsmessage.ClassINET,
Length: 4,
},
Body: &dnsmessage.PTRResource{
PTR: dnsmessage.MustNewName("good.golang.org."),
},
},
)
}
return r, nil
Expand All @@ -1903,58 +1947,137 @@ func TestCVE202133195(t *testing.T) {
defer func(orig string) { testHookHostsPath = orig }(testHookHostsPath)
testHookHostsPath = "testdata/hosts"

_, err := r.LookupCNAME(context.Background(), "golang.org")
if expected := "lookup golang.org: CNAME target is invalid"; err == nil || err.Error() != expected {
t.Errorf("Resolver.LookupCNAME returned unexpected error, got %q, want %q", err, expected)
}
_, err = LookupCNAME("golang.org")
if expected := "lookup golang.org: CNAME target is invalid"; err == nil || err.Error() != expected {
t.Errorf("LookupCNAME returned unexpected error, got %q, want %q", err, expected)
}

_, _, err = r.LookupSRV(context.Background(), "target", "tcp", "golang.org")
if expected := "lookup golang.org: SRV target is invalid"; err == nil || err.Error() != expected {
t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err, expected)
}
_, _, err = LookupSRV("target", "tcp", "golang.org")
if expected := "lookup golang.org: SRV target is invalid"; err == nil || err.Error() != expected {
t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err, expected)
}

_, _, err = r.LookupSRV(context.Background(), "hdr", "tcp", "golang.org")
if expected := "lookup golang.org: SRV header name is invalid"; err == nil || err.Error() != expected {
t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err, expected)
}
_, _, err = LookupSRV("hdr", "tcp", "golang.org")
if expected := "lookup golang.org: SRV header name is invalid"; err == nil || err.Error() != expected {
t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err, expected)
}

_, err = r.LookupMX(context.Background(), "golang.org")
if expected := "lookup golang.org: MX target is invalid"; err == nil || err.Error() != expected {
t.Errorf("Resolver.LookupMX returned unexpected error, got %q, want %q", err, expected)
}
_, err = LookupMX("golang.org")
if expected := "lookup golang.org: MX target is invalid"; err == nil || err.Error() != expected {
t.Errorf("LookupMX returned unexpected error, got %q, want %q", err, expected)
}

_, err = r.LookupNS(context.Background(), "golang.org")
if expected := "lookup golang.org: NS target is invalid"; err == nil || err.Error() != expected {
t.Errorf("Resolver.LookupNS returned unexpected error, got %q, want %q", err, expected)
}
_, err = LookupNS("golang.org")
if expected := "lookup golang.org: NS target is invalid"; err == nil || err.Error() != expected {
t.Errorf("LookupNS returned unexpected error, got %q, want %q", err, expected)
tests := []struct {
name string
f func(*testing.T)
}{
{
name: "CNAME",
f: func(t *testing.T) {
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "golang.org"}
_, err := r.LookupCNAME(context.Background(), "golang.org")
if err.Error() != expectedErr.Error() {
t.Fatalf("unexpected error: %s", err)
}
_, err = LookupCNAME("golang.org")
if err.Error() != expectedErr.Error() {
t.Fatalf("unexpected error: %s", err)
}
},
},
{
name: "SRV (bad record)",
f: func(t *testing.T) {
expected := []*SRV{
{
Target: "good.golang.org.",
},
}
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "golang.org"}
_, records, err := r.LookupSRV(context.Background(), "target", "tcp", "golang.org")
if err.Error() != expectedErr.Error() {
t.Fatalf("unexpected error: %s", err)
}
if !reflect.DeepEqual(records, expected) {
t.Error("Unexpected record set")
}
_, records, err = LookupSRV("target", "tcp", "golang.org")
if err.Error() != expectedErr.Error() {
t.Errorf("unexpected error: %s", err)
}
if !reflect.DeepEqual(records, expected) {
t.Error("Unexpected record set")
}
},
},
{
name: "SRV (bad header)",
f: func(t *testing.T) {
_, _, err := r.LookupSRV(context.Background(), "hdr", "tcp", "golang.org.")
if expected := "lookup golang.org.: SRV header name is invalid"; err == nil || err.Error() != expected {
t.Errorf("Resolver.LookupSRV returned unexpected error, got %q, want %q", err, expected)
}
_, _, err = LookupSRV("hdr", "tcp", "golang.org.")
if expected := "lookup golang.org.: SRV header name is invalid"; err == nil || err.Error() != expected {
t.Errorf("LookupSRV returned unexpected error, got %q, want %q", err, expected)
}
},
},
{
name: "MX",
f: func(t *testing.T) {
expected := []*MX{
{
Host: "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")
}
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")
}
},
},
{
name: "NS",
f: func(t *testing.T) {
expected := []*NS{
{
Host: "good.golang.org.",
},
}
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "golang.org"}
records, err := r.LookupNS(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")
}
records, err = LookupNS("golang.org")
if err.Error() != expectedErr.Error() {
t.Fatalf("unexpected error: %s", err)
}
if !reflect.DeepEqual(records, expected) {
t.Error("Unexpected record set")
}
},
},
{
name: "Addr",
f: func(t *testing.T) {
expected := []string{"good.golang.org."}
expectedErr := &DNSError{Err: errMalformedDNSRecordsDetail, Name: "192.0.2.42"}
records, err := r.LookupAddr(context.Background(), "192.0.2.42")
if err.Error() != expectedErr.Error() {
t.Fatalf("unexpected error: %s", err)
}
if !reflect.DeepEqual(records, expected) {
t.Error("Unexpected record set")
}
records, err = LookupAddr("192.0.2.42")
if err.Error() != expectedErr.Error() {
t.Fatalf("unexpected error: %s", err)
}
if !reflect.DeepEqual(records, expected) {
t.Error("Unexpected record set")
}
},
},
}

_, err = r.LookupAddr(context.Background(), "192.0.2.42")
if expected := "lookup 192.0.2.42: PTR target is invalid"; err == nil || err.Error() != expected {
t.Errorf("Resolver.LookupAddr returned unexpected error, got %q, want %q", err, expected)
}
_, err = LookupAddr("192.0.2.42")
if expected := "lookup 192.0.2.42: PTR target is invalid"; err == nil || err.Error() != expected {
t.Errorf("LookupAddr returned unexpected error, got %q, want %q", err, expected)
for _, tc := range tests {
t.Run(tc.name, tc.f)
}
}

Expand Down
Loading

0 comments on commit 3d1d066

Please sign in to comment.