From c9943dff38113358514dc9385af8092aaa75f80c Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 23 Aug 2022 16:12:04 +0200 Subject: [PATCH] feat(dns): expose more low-level fields (#873) This pull request started as a draft to enable users to see CNAME answers. It contained several patches which we merged separately (see https://github.com/ooni/probe-cli/pull/873#issuecomment-1222406732 and https://github.com/ooni/probe-cli/compare/2301a30630577637e5ec9912b027584e5ab4eebc...60b7d1f87be4b088929048edfb950dc93146a01f for details on what has actually changed, which is based on patches originally part of this PR). In its final form, however, this PR only deals with exposing more low-level DNS fields to the archival data format. Closes: https://github.com/ooni/probe/issues/2228 Related PR spec: https://github.com/ooni/spec/pull/256 --- internal/measurexlite/dns.go | 21 ++++++++++++++++ internal/measurexlite/dns_test.go | 36 +++++++++++++++++++++++++++ internal/model/archival.go | 4 +++ internal/netxlite/getaddrinfo.go | 4 +-- internal/netxlite/getaddrinfo_test.go | 6 ++++- 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/internal/measurexlite/dns.go b/internal/measurexlite/dns.go index 008b7d4..952613c 100644 --- a/internal/measurexlite/dns.go +++ b/internal/measurexlite/dns.go @@ -117,14 +117,35 @@ func NewArchivalDNSLookupResultFromRoundTrip(index int64, started time.Duration, Answers: newArchivalDNSAnswers(addrs, response), Engine: reso.Network(), Failure: tracex.NewFailure(err), + GetaddrinfoError: netxlite.ErrorToGetaddrinfoRetvalOrZero(err), Hostname: query.Domain(), QueryType: dns.TypeToString[query.Type()], + RawResponse: maybeRawResponse(response), + Rcode: maybeResponseRcode(response), ResolverHostname: nil, + ResolverPort: nil, ResolverAddress: reso.Address(), + T0: started.Seconds(), T: finished.Seconds(), } } +// maybeResponseRcode returns the response rcode (when available) +func maybeResponseRcode(resp model.DNSResponse) (out int64) { + if resp != nil { + out = int64(resp.Rcode()) + } + return +} + +// maybeRawResponse returns either the raw response (when available) or nil. +func maybeRawResponse(resp model.DNSResponse) (out []byte) { + if resp != nil { + out = resp.Bytes() + } + return +} + // 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 diff --git a/internal/measurexlite/dns_test.go b/internal/measurexlite/dns_test.go index 43cb25b..c1a4777 100644 --- a/internal/measurexlite/dns_test.go +++ b/internal/measurexlite/dns_test.go @@ -138,6 +138,12 @@ func TestNewResolver(t *testing.T) { MockDecodeCNAME: func() (string, error) { return "dns.google.", nil }, + MockRcode: func() int { + return 0 + }, + MockBytes: func() []byte { + return []byte{} + }, } return response, nil }, @@ -214,6 +220,12 @@ func TestNewResolver(t *testing.T) { MockDecodeCNAME: func() (string, error) { return "dns.google.", nil }, + MockRcode: func() int { + return 0 + }, + MockBytes: func() []byte { + return []byte{} + }, } return response, nil }, @@ -363,6 +375,12 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) { MockDecodeCNAME: func() (string, error) { return "", netxlite.ErrOODNSNoAnswer }, + MockRcode: func() int { + return 0 + }, + MockBytes: func() []byte { + return []byte{} + }, } err := trace.OnDelayedDNSResponse(started, txp, query, dnsResponse, addrs, nil, finished) // 2. read the trace @@ -405,6 +423,12 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) { MockDecodeCNAME: func() (string, error) { return "", netxlite.ErrOODNSNoAnswer }, + MockRcode: func() int { + return 0 + }, + MockBytes: func() []byte { + return []byte{} + }, } err := trace.OnDelayedDNSResponse(started, txp, query, dnsResponse, addrs, nil, finished) if !errors.Is(err, ErrDelayedDNSResponseBufferFull) { @@ -448,6 +472,12 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) { MockDecodeCNAME: func() (string, error) { return "", netxlite.ErrOODNSNoAnswer }, + MockRcode: func() int { + return 0 + }, + MockBytes: func() []byte { + return []byte{} + }, } for i := 0; i < events; i++ { // fill the trace @@ -491,6 +521,12 @@ func TestDelayedDNSResponseWithTimeout(t *testing.T) { MockDecodeCNAME: func() (string, error) { return "", netxlite.ErrOODNSNoAnswer }, + MockRcode: func() int { + return 0 + }, + MockBytes: func() []byte { + return []byte{} + }, } trace.delayedDNSResponse <- NewArchivalDNSLookupResultFromRoundTrip(trace.Index, started.Sub(trace.ZeroTime), txp, query, dnsResponse, addrs, nil, finished.Sub(trace.ZeroTime)) diff --git a/internal/model/archival.go b/internal/model/archival.go index 34b35df..d0c5f87 100644 --- a/internal/model/archival.go +++ b/internal/model/archival.go @@ -115,11 +115,15 @@ type ArchivalDNSLookupResult struct { Answers []ArchivalDNSAnswer `json:"answers"` Engine string `json:"engine"` Failure *string `json:"failure"` + GetaddrinfoError int64 `json:"getaddrinfo_error,omitempty"` Hostname string `json:"hostname"` QueryType string `json:"query_type"` + RawResponse []byte `json:"raw_response,omitempty"` + Rcode int64 `json:"rcode,omitempty"` ResolverHostname *string `json:"resolver_hostname"` ResolverPort *string `json:"resolver_port"` ResolverAddress string `json:"resolver_address"` + T0 float64 `json:"t0"` T float64 `json:"t"` } diff --git a/internal/netxlite/getaddrinfo.go b/internal/netxlite/getaddrinfo.go index 1d67457..ad775bb 100644 --- a/internal/netxlite/getaddrinfo.go +++ b/internal/netxlite/getaddrinfo.go @@ -29,10 +29,10 @@ func (err *ErrGetaddrinfo) Unwrap() error { return err.Underlying } -// ErrorToGetaddrinfoRetval converts an arbitrary error to +// ErrorToGetaddrinfoRetvalOrZero converts an arbitrary error to // the return value of getaddrinfo. If err is nil or is not // an instance of ErrGetaddrinfo, we just return zero. -func ErrorToGetaddrinfoRetval(err error) int64 { +func ErrorToGetaddrinfoRetvalOrZero(err error) int64 { var aierr *ErrGetaddrinfo if err != nil && errors.As(err, &aierr) { return aierr.Code diff --git a/internal/netxlite/getaddrinfo_test.go b/internal/netxlite/getaddrinfo_test.go index a763741..7b8c2ae 100644 --- a/internal/netxlite/getaddrinfo_test.go +++ b/internal/netxlite/getaddrinfo_test.go @@ -35,10 +35,14 @@ func TestErrorToGetaddrinfoRetval(t *testing.T) { name: "with another kind of error", args: args{io.EOF}, want: 0, + }, { + name: "with nil error", + args: args{nil}, + want: 0, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := ErrorToGetaddrinfoRetval(tt.args.err); got != tt.want { + if got := ErrorToGetaddrinfoRetvalOrZero(tt.args.err); got != tt.want { t.Errorf("ErrorToGetaddrinfoRetval() = %v, want %v", got, tt.want) } })