Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: save CNAME into archival data format #877

Merged
merged 3 commits into from
Aug 23, 2022
Merged

feat: save CNAME into archival data format #877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1471161
feat: save CNAME into archival data format
bassosimone Aug 23, 2022
d062300
doc: add design note
bassosimone Aug 23, 2022
2aa2110
doc: document the expected growth of extraction function
bassosimone Aug 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions internal/measurexlite/dns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ type DNSNetworkAddresser interface {
func NewArchivalDNSLookupResultFromRoundTrip(index int64, started time.Duration, reso DNSNetworkAddresser, query model.DNSQuery,
response model.DNSResponse, addrs []string, err error, finished time.Duration) *model.ArchivalDNSLookupResult {
return &model.ArchivalDNSLookupResult{
Answers: archivalAnswersFromAddrs(addrs),
Answers: newArchivalDNSAnswers(addrs, response),
Engine: reso.Network(),
Failure: tracex.NewFailure(err),
Hostname: query.Domain(),
Expand All @@ -125,8 +125,16 @@ func NewArchivalDNSLookupResultFromRoundTrip(index int64, started time.Duration,
}
}

// archivalAnswersFromAddrs generates model.ArchivalDNSAnswer from an array of addresses
func archivalAnswersFromAddrs(addrs []string) (out []model.ArchivalDNSAnswer) {
// newArchivalDNSAnswers generates []model.ArchivalDNSAnswer from [addrs] and [resp].
func newArchivalDNSAnswers(addrs []string, resp model.DNSResponse) (out []model.ArchivalDNSAnswer) {
// Design note: in principle we might want to extract everything from the
// response but, when we're called by netxlite, netxlite has already extracted
// the addresses to return them to the caller, so I think it's fine to keep
// this extraction code as such rather than suppressing passing the addrs from
// netxlite. Also, a wrong IP address is a bug because netxlite should not
// return invalid IP addresses from its resolvers, so we want to know about that.

// Include IP addresses extracted by netxlite
for _, addr := range addrs {
ipv6, err := netxlite.IsIPv6(addr)
if err != nil {
Expand All @@ -142,6 +150,7 @@ func archivalAnswersFromAddrs(addrs []string) (out []model.ArchivalDNSAnswer) {
AnswerType: "A",
Hostname: "",
IPv4: addr,
IPv6: "",
TTL: nil,
})
case true:
Expand All @@ -150,11 +159,32 @@ func archivalAnswersFromAddrs(addrs []string) (out []model.ArchivalDNSAnswer) {
ASOrgName: org,
AnswerType: "AAAA",
Hostname: "",
IPv4: "",
IPv6: addr,
TTL: nil,
})
}
}

// Include additional answer types when a response is available
if resp != nil {

// Include CNAME if available
if cname, err := resp.DecodeCNAME(); err == nil && cname != "" {
out = append(out, model.ArchivalDNSAnswer{
ASN: 0,
ASOrgName: "",
AnswerType: "CNAME",
Hostname: cname,
IPv4: "",
IPv6: "",
TTL: nil,
})
}

// TODO(bassosimone): what other fields generally present inside A/AAAA replies
// would it be useful to extract here? Perhaps, the SoA field?
}
return
}

Expand Down
213 changes: 172 additions & 41 deletions internal/measurexlite/dns_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ func TestNewResolver(t *testing.T) {
}
return []string{"1.1.1.1"}, nil
},
MockDecodeCNAME: func() (string, error) {
return "dns.google.", nil
},
}
return response, nil
},
Expand Down Expand Up @@ -177,7 +180,7 @@ func TestNewResolver(t *testing.T) {
if ev.Engine != "mocked" {
t.Fatal("unexpected engine")
}
if len(ev.Answers) != 1 {
if len(ev.Answers) != 2 {
t.Fatal("expected single answer in DNSLookup event")
}
if ev.QueryType == "A" && ev.Answers[0].IPv4 != "1.1.1.1" {
Expand All @@ -186,6 +189,9 @@ func TestNewResolver(t *testing.T) {
if ev.QueryType == "AAAA" && ev.Answers[0].IPv6 != "fe80::a00:20ff:feb9:4c54" {
t.Fatal("unexpected AAAA query result")
}
if ev.Answers[1].AnswerType != "CNAME " && ev.Answers[1].Hostname != "dns.google." {
t.Fatal("unexpected second answer (expected CNAME)", ev.Answers[1])
}
}
})
})
Expand All @@ -205,6 +211,9 @@ func TestNewResolver(t *testing.T) {
}
return []string{"1.1.1.1"}, nil
},
MockDecodeCNAME: func() (string, error) {
return "dns.google.", nil
},
}
return response, nil
},
Expand Down Expand Up @@ -350,8 +359,12 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) {
addrs := []string{"1.1.1.1"}
finished := trace.TimeNow()
// 1. fill the trace
err := trace.OnDelayedDNSResponse(started, txp, query, &mocks.DNSResponse{},
addrs, nil, finished)
dnsResponse := &mocks.DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "", netxlite.ErrOODNSNoAnswer
},
}
err := trace.OnDelayedDNSResponse(started, txp, query, dnsResponse, addrs, nil, finished)
// 2. read the trace
got := trace.DelayedDNSResponseWithTimeout(context.Background(), time.Second)
if err != nil {
Expand Down Expand Up @@ -388,8 +401,12 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) {
addrs := []string{"1.1.1.1"}
finished := trace.TimeNow()
// 1. attempt to write into the trace
err := trace.OnDelayedDNSResponse(started, txp, query, &mocks.DNSResponse{},
addrs, nil, finished)
dnsResponse := &mocks.DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "", netxlite.ErrOODNSNoAnswer
},
}
err := trace.OnDelayedDNSResponse(started, txp, query, dnsResponse, addrs, nil, finished)
if !errors.Is(err, ErrDelayedDNSResponseBufferFull) {
t.Fatal("unexpected error", err)
}
Expand Down Expand Up @@ -427,10 +444,15 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) {
addrs := []string{"1.1.1.1"}
finished := trace.TimeNow()
events := 4
dnsResponse := &mocks.DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "", netxlite.ErrOODNSNoAnswer
},
}
for i := 0; i < events; i++ {
// fill the trace
trace.delayedDNSResponse <- NewArchivalDNSLookupResultFromRoundTrip(trace.Index, started.Sub(trace.ZeroTime),
txp, query, &mocks.DNSResponse{}, addrs, nil, finished.Sub(trace.ZeroTime))
txp, query, dnsResponse, addrs, nil, finished.Sub(trace.ZeroTime))
}
ctx, cancel := context.WithCancel(context.Background())
cancel() // we ensure that the context cancels before draining all the events
Expand Down Expand Up @@ -465,8 +487,13 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) {
}
addrs := []string{"1.1.1.1"}
finished := trace.TimeNow()
dnsResponse := &mocks.DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "", netxlite.ErrOODNSNoAnswer
},
}
trace.delayedDNSResponse <- NewArchivalDNSLookupResultFromRoundTrip(trace.Index, started.Sub(trace.ZeroTime),
txp, query, &mocks.DNSResponse{}, addrs, nil, finished.Sub(trace.ZeroTime))
txp, query, dnsResponse, addrs, nil, finished.Sub(trace.ZeroTime))
got := trace.DelayedDNSResponseWithTimeout(context.Background(), time.Second)
if len(got) != 1 {
t.Fatal("unexpected output from trace")
Expand All @@ -475,52 +502,156 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) {
})
}

func TestAnswersFromAddrs(t *testing.T) {
func TestNewArchivalDNSAnswers(t *testing.T) {
tests := []struct {
name string
args []string
name string
addrs []string
resp model.DNSResponse
expected []model.ArchivalDNSAnswer
}{{
name: "with valid input",
args: []string{"1.1.1.1", "fe80::a00:20ff:feb9:4c54"},
addrs: []string{
"8.8.4.4",
"2001:4860:4860::8844",
},
resp: nil,
expected: []model.ArchivalDNSAnswer{{
ASN: 15169,
ASOrgName: "Google LLC",
AnswerType: "A",
Hostname: "",
IPv4: "8.8.4.4",
IPv6: "",
TTL: nil,
}, {
ASN: 15169,
ASOrgName: "Google LLC",
AnswerType: "AAAA",
Hostname: "",
IPv4: "",
IPv6: "2001:4860:4860::8844",
TTL: nil,
}},
}, {
name: "with invalid IPv4 address",
args: []string{"1.1.1.1.1", "fe80::a00:20ff:feb9:4c54"},
addrs: []string{
"1.1.1.1.1", // invalid because it has five dots
"2001:4860:4860::8844",
},
resp: nil,
expected: []model.ArchivalDNSAnswer{{
ASN: 15169,
ASOrgName: "Google LLC",
AnswerType: "AAAA",
Hostname: "",
IPv4: "",
IPv6: "2001:4860:4860::8844",
TTL: nil,
}},
}, {
name: "with invalid IPv6 address",
args: []string{"1.1.1.1", "fe80::a00:20ff:feb9:::4c54"},
addrs: []string{
"8.8.4.4",
"fe80::a00:20ff:feb9:::4c54", // invalid because it has :::
},
resp: nil,
expected: []model.ArchivalDNSAnswer{{
ASN: 15169,
ASOrgName: "Google LLC",
AnswerType: "A",
Hostname: "",
IPv4: "8.8.4.4",
IPv6: "",
TTL: nil,
}},
}, {
name: "with empty input",
addrs: []string{},
resp: nil,
expected: nil,
}, {
name: "with empty input",
args: []string{},
name: "with nil input",
addrs: nil,
resp: nil,
expected: nil,
}, {
name: "with nil input",
args: nil,
name: "with valid IPv4 address and CNAME",
addrs: []string{
"8.8.8.8",
},
resp: &mocks.DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "dns.google.", nil
},
},
expected: []model.ArchivalDNSAnswer{{
ASN: 15169,
ASOrgName: "Google LLC",
AnswerType: "A",
Hostname: "",
IPv4: "8.8.8.8",
IPv6: "",
TTL: nil,
}, {
ASN: 0,
ASOrgName: "",
AnswerType: "CNAME",
Hostname: "dns.google.",
IPv4: "",
IPv6: "",
TTL: nil,
}},
}, {
name: "with valid IPv6 address and CNAME",
addrs: []string{
"2001:4860:4860::8844",
},
resp: &mocks.DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "dns.google.", nil
},
},
expected: []model.ArchivalDNSAnswer{{
ASN: 15169,
ASOrgName: "Google LLC",
AnswerType: "AAAA",
Hostname: "",
IPv4: "",
IPv6: "2001:4860:4860::8844",
TTL: nil,
}, {
ASN: 0,
ASOrgName: "",
AnswerType: "CNAME",
Hostname: "dns.google.",
IPv4: "",
IPv6: "",
TTL: nil,
}},
}, {
name: "with DecodeCNAME error",
addrs: []string{},
resp: &mocks.DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "", errors.New("mocked errorr")
},
},
expected: nil,
}, {
name: "with DecodeCNAME success and no CNAME",
addrs: []string{},
resp: &mocks.DNSResponse{
MockDecodeCNAME: func() (string, error) {
return "", nil
},
},
expected: nil,
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := archivalAnswersFromAddrs(tt.args)
var idx int
for _, inp := range tt.args {
ip6, err := netxlite.IsIPv6(inp)
if err != nil {
continue
}
if idx >= len(got) {
t.Fatal("unexpected array length")
}
answer := got[idx]
if ip6 {
if answer.AnswerType != "AAAA" || answer.IPv6 != inp {
t.Fatal("unexpected output", answer)
}
} else {
if answer.AnswerType != "A" || answer.IPv4 != inp {
t.Fatal("unexpected output", answer)
}
}
idx++
}
if idx != len(got) {
t.Fatal("unexpected array length", len(got))
got := newArchivalDNSAnswers(tt.addrs, tt.resp)
if diff := cmp.Diff(tt.expected, got); diff != "" {
t.Fatal(diff)
}
})
}
Expand Down