From d19b5e0118fdf807e33999aa9e88ed10f83ebcc8 Mon Sep 17 00:00:00 2001 From: frcroth Date: Thu, 19 Dec 2024 12:59:42 +0100 Subject: [PATCH 1/5] Add ddr experiment --- internal/experiment/ddr/ddr.go | 174 ++++++++++++++++++++++++++++ internal/experiment/ddr/ddr_test.go | 15 +++ internal/registry/ddr.go | 28 +++++ 3 files changed, 217 insertions(+) create mode 100644 internal/experiment/ddr/ddr.go create mode 100644 internal/experiment/ddr/ddr_test.go create mode 100644 internal/registry/ddr.go diff --git a/internal/experiment/ddr/ddr.go b/internal/experiment/ddr/ddr.go new file mode 100644 index 0000000000..c12ba5ac49 --- /dev/null +++ b/internal/experiment/ddr/ddr.go @@ -0,0 +1,174 @@ +package ddr + +import ( + "context" + "errors" + "fmt" + "net" + "time" + + "github.com/apex/log" + "github.com/miekg/dns" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" +) + +const ( + testName = "ddr" + testVersion = "0.1.0" +) + +type Config struct { +} + +// Measurer performs the measurement. +type Measurer struct { + config Config +} + +// ExperimentName implements ExperimentMeasurer.ExperimentName. +func (m *Measurer) ExperimentName() string { + return testName +} + +// ExperimentVersion implements ExperimentMeasurer.ExperimentVersion. +func (m *Measurer) ExperimentVersion() string { + return testVersion +} + +type DDRResponse struct { + Priority int `json:"priority"` + Target string `json:"target"` + Keys map[string]string `json:"keys"` +} + +type TestKeys struct { + // DDRResponse is the DDR response. + DDRResponse []DDRResponse `json:"ddr_responses"` + + // SupportsDDR is true if DDR is supported. + SupportsDDR bool `json:"supports_ddr"` + + // Resolver is the resolver used (the system resolver of the host). + Resolver string `json:"resolver"` + + // Failure is the failure that occurred, or nil. + Failure *string `json:"failure"` +} + +func (m *Measurer) Run( + ctx context.Context, + args *model.ExperimentArgs) error { + + log.SetLevel(log.DebugLevel) + measurement := args.Measurement + + tk := &TestKeys{} + measurement.TestKeys = tk + + timeout := 60 * time.Second + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + systemResolver := getSystemResolverAddress() + if systemResolver == "" { + return errors.New("could not get system resolver") + } + log.Infof("Using system resolver: %s", systemResolver) + tk.Resolver = systemResolver + + // DDR queries are queries of the SVCB type for the _dns.resolver.arpa. domain. + + netx := &netxlite.Netx{} + dialer := netx.NewDialerWithoutResolver(log.Log) + transport := netxlite.NewUnwrappedDNSOverUDPTransport( + dialer, systemResolver) + encoder := &netxlite.DNSEncoderMiekg{} + query := encoder.Encode( + "_dns.resolver.arpa.", // As specified in RFC 9462 + dns.TypeSVCB, + true) + resp, err := transport.RoundTrip(ctx, query) + if err != nil { + failure := err.Error() + tk.Failure = &failure + return nil + } + + reply := &dns.Msg{} + err = reply.Unpack(resp.Bytes()) + if err != nil { + unpackError := err.Error() + tk.Failure = &unpackError + return nil + } + + ddrResponse, err := decodeResponse(reply.Answer) + + if err != nil { + decodingError := err.Error() + tk.Failure = &decodingError + } else { + tk.DDRResponse = ddrResponse + } + + tk.SupportsDDR = len(tk.DDRResponse) > 0 + + log.Infof("Gathered DDR Responses: %+v", tk.DDRResponse) + return nil +} + +// decodeResponse decodes the response from the DNS query. +// DDR is only concerned with SVCB records, so we only decode those. +func decodeResponse(responseFields []dns.RR) ([]DDRResponse, error) { + responses := make([]DDRResponse, 0) + for _, rr := range responseFields { + switch rr := rr.(type) { + case *dns.SVCB: + parsed, err := parseSvcb(rr) + if err != nil { + return nil, err + } + responses = append(responses, parsed) + default: + return nil, fmt.Errorf("unknown RR type: %T", rr) + } + } + return responses, nil +} + +func parseSvcb(rr *dns.SVCB) (DDRResponse, error) { + keys := make(map[string]string) + for _, kv := range rr.Value { + value := kv.String() + key := kv.Key().String() + keys[key] = value + } + + return DDRResponse{ + Priority: int(rr.Priority), + Target: rr.Target, + Keys: keys, + }, nil +} + +// Get the system resolver address from /etc/resolv.conf +// This should also be possible via querying the system resolver and checking the response +func getSystemResolverAddress() string { + resolverConfig, err := dns.ClientConfigFromFile("/etc/resolv.conf") + if err != nil { + return "" + } + + if len(resolverConfig.Servers) > 0 { + return net.JoinHostPort(resolverConfig.Servers[0], resolverConfig.Port) + } + + return "" +} + +func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { + return &Measurer{ + config: config, + } +} diff --git a/internal/experiment/ddr/ddr_test.go b/internal/experiment/ddr/ddr_test.go new file mode 100644 index 0000000000..ee848efeb8 --- /dev/null +++ b/internal/experiment/ddr/ddr_test.go @@ -0,0 +1,15 @@ +package ddr + +import ( + "testing" +) + +func TestMeasurerExperimentNameVersion(t *testing.T) { + measurer := NewExperimentMeasurer(Config{}) + if measurer.ExperimentName() != "ddr" { + t.Fatal("unexpected ExperimentName") + } + if measurer.ExperimentVersion() != "0.1.0" { + t.Fatal("unexpected ExperimentVersion") + } +} diff --git a/internal/registry/ddr.go b/internal/registry/ddr.go new file mode 100644 index 0000000000..9ab7f84542 --- /dev/null +++ b/internal/registry/ddr.go @@ -0,0 +1,28 @@ +package registry + +// +// Registers the `ddr' experiment. +// + +import ( + "github.com/ooni/probe-cli/v3/internal/experiment/ddr" + "github.com/ooni/probe-cli/v3/internal/model" +) + +func init() { + const canonicalName = "ddr" + AllExperiments[canonicalName] = func() *Factory { + return &Factory{ + build: func(config interface{}) model.ExperimentMeasurer { + return ddr.NewExperimentMeasurer( + *config.(*ddr.Config), + ) + }, + canonicalName: canonicalName, + config: &ddr.Config{}, + enabledByDefault: true, + interruptible: true, + inputPolicy: model.InputNone, + } + } +} From 8ddec182caff7c1f9d49aa76a69d2fd10733e613 Mon Sep 17 00:00:00 2001 From: frcroth Date: Thu, 19 Dec 2024 15:59:47 +0100 Subject: [PATCH 2/5] Add tests to DDR experiment --- internal/experiment/ddr/ddr.go | 19 ++++--- internal/experiment/ddr/ddr_test.go | 78 +++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/internal/experiment/ddr/ddr.go b/internal/experiment/ddr/ddr.go index c12ba5ac49..946d121786 100644 --- a/internal/experiment/ddr/ddr.go +++ b/internal/experiment/ddr/ddr.go @@ -19,6 +19,9 @@ const ( ) type Config struct { + // CustomResolver is the custom resolver to use. + // If empty, the system resolver is used. + CustomResolver *string } // Measurer performs the measurement. @@ -70,19 +73,23 @@ func (m *Measurer) Run( ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - systemResolver := getSystemResolverAddress() - if systemResolver == "" { - return errors.New("could not get system resolver") + if m.config.CustomResolver == nil { + systemResolver := getSystemResolverAddress() + if systemResolver == "" { + return errors.New("could not get system resolver") + } + log.Infof("Using system resolver: %s", systemResolver) + tk.Resolver = systemResolver + } else { + tk.Resolver = *m.config.CustomResolver } - log.Infof("Using system resolver: %s", systemResolver) - tk.Resolver = systemResolver // DDR queries are queries of the SVCB type for the _dns.resolver.arpa. domain. netx := &netxlite.Netx{} dialer := netx.NewDialerWithoutResolver(log.Log) transport := netxlite.NewUnwrappedDNSOverUDPTransport( - dialer, systemResolver) + dialer, tk.Resolver) encoder := &netxlite.DNSEncoderMiekg{} query := encoder.Encode( "_dns.resolver.arpa.", // As specified in RFC 9462 diff --git a/internal/experiment/ddr/ddr_test.go b/internal/experiment/ddr/ddr_test.go index ee848efeb8..28d930621b 100644 --- a/internal/experiment/ddr/ddr_test.go +++ b/internal/experiment/ddr/ddr_test.go @@ -1,7 +1,12 @@ package ddr import ( + "context" "testing" + + "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/mocks" + "github.com/ooni/probe-cli/v3/internal/model" ) func TestMeasurerExperimentNameVersion(t *testing.T) { @@ -13,3 +18,76 @@ func TestMeasurerExperimentNameVersion(t *testing.T) { t.Fatal("unexpected ExperimentVersion") } } + +func TestMeasurerRun(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + + oneOneOneOneResolver := "1.1.1.1:53" + + measurer := NewExperimentMeasurer(Config{ + CustomResolver: &oneOneOneOneResolver, + }) + args := &model.ExperimentArgs{ + Callbacks: model.NewPrinterCallbacks(log.Log), + Measurement: new(model.Measurement), + Session: &mocks.Session{ + MockLogger: func() model.Logger { + return log.Log + }, + }, + } + if err := measurer.Run(context.Background(), args); err != nil { + t.Fatal(err) + } + tk := args.Measurement.TestKeys.(*TestKeys) + if tk.Failure != nil { + t.Fatal("unexpected Failure") + } + + if tk.Resolver != oneOneOneOneResolver { + t.Fatal("Resolver should be written to TestKeys") + } + + // 1.1.1.1 supports DDR + if tk.SupportsDDR != true { + t.Fatal("unexpected value for Supports DDR") + } +} + +// This test fails because the resolver is a domain name and not an IP address. +func TestMeasurerFailsWithDomainResolver(t *testing.T) { + invalidResolver := "invalid-resolver.example:53" + + tk, _ := runExperiment(invalidResolver) + if tk.Failure == nil { + t.Fatal("expected Failure") + } +} + +func TestMeasurerFailsWithNoPort(t *testing.T) { + invalidResolver := "1.1.1.1" + + tk, _ := runExperiment(invalidResolver) + if tk.Failure == nil { + t.Fatal("expected Failure") + } +} + +func runExperiment(resolver string) (*TestKeys, error) { + measurer := NewExperimentMeasurer(Config{ + CustomResolver: &resolver, + }) + args := &model.ExperimentArgs{ + Callbacks: model.NewPrinterCallbacks(log.Log), + Measurement: new(model.Measurement), + Session: &mocks.Session{ + MockLogger: func() model.Logger { + return log.Log + }, + }, + } + err := measurer.Run(context.Background(), args) + return args.Measurement.TestKeys.(*TestKeys), err +} From 8739b76162bb288cc1f322c420abb84ec4478bb6 Mon Sep 17 00:00:00 2001 From: frcroth Date: Fri, 31 Jan 2025 11:31:12 +0100 Subject: [PATCH 3/5] Use DNS data format in DDR experiment --- internal/experiment/ddr/ddr.go | 88 +++++++++++++++++++++-------- internal/experiment/ddr/ddr_test.go | 8 ++- internal/model/archival.go | 22 +++++--- 3 files changed, 85 insertions(+), 33 deletions(-) diff --git a/internal/experiment/ddr/ddr.go b/internal/experiment/ddr/ddr.go index 946d121786..81682f7c6f 100644 --- a/internal/experiment/ddr/ddr.go +++ b/internal/experiment/ddr/ddr.go @@ -9,6 +9,7 @@ import ( "github.com/apex/log" "github.com/miekg/dns" + "github.com/ooni/probe-cli/v3/internal/geoipx" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -39,22 +40,13 @@ func (m *Measurer) ExperimentVersion() string { return testVersion } -type DDRResponse struct { - Priority int `json:"priority"` - Target string `json:"target"` - Keys map[string]string `json:"keys"` -} - type TestKeys struct { - // DDRResponse is the DDR response. - DDRResponse []DDRResponse `json:"ddr_responses"` + // DNS Queries and results (as specified in https://github.com/ooni/spec/blob/master/data-formats/df-002-dnst.md#dns-data-format) + Queries model.ArchivalDNSLookupResult `json:"queries"` // SupportsDDR is true if DDR is supported. SupportsDDR bool `json:"supports_ddr"` - // Resolver is the resolver used (the system resolver of the host). - Resolver string `json:"resolver"` - // Failure is the failure that occurred, or nil. Failure *string `json:"failure"` } @@ -73,15 +65,16 @@ func (m *Measurer) Run( ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() + resolver := "" if m.config.CustomResolver == nil { systemResolver := getSystemResolverAddress() if systemResolver == "" { return errors.New("could not get system resolver") } log.Infof("Using system resolver: %s", systemResolver) - tk.Resolver = systemResolver + resolver = systemResolver } else { - tk.Resolver = *m.config.CustomResolver + resolver = *m.config.CustomResolver } // DDR queries are queries of the SVCB type for the _dns.resolver.arpa. domain. @@ -89,12 +82,13 @@ func (m *Measurer) Run( netx := &netxlite.Netx{} dialer := netx.NewDialerWithoutResolver(log.Log) transport := netxlite.NewUnwrappedDNSOverUDPTransport( - dialer, tk.Resolver) + dialer, resolver) encoder := &netxlite.DNSEncoderMiekg{} query := encoder.Encode( "_dns.resolver.arpa.", // As specified in RFC 9462 dns.TypeSVCB, true) + t0 := time.Since(measurement.MeasurementStartTimeSaved).Seconds() resp, err := transport.RoundTrip(ctx, query) if err != nil { failure := err.Error() @@ -115,20 +109,19 @@ func (m *Measurer) Run( if err != nil { decodingError := err.Error() tk.Failure = &decodingError - } else { - tk.DDRResponse = ddrResponse } + t := time.Since(measurement.MeasurementStartTimeSaved).Seconds() + tk.Queries = createResult(t, t0, tk.Failure, resp, resolver, ddrResponse) - tk.SupportsDDR = len(tk.DDRResponse) > 0 + tk.SupportsDDR = len(ddrResponse) > 0 - log.Infof("Gathered DDR Responses: %+v", tk.DDRResponse) return nil } // decodeResponse decodes the response from the DNS query. // DDR is only concerned with SVCB records, so we only decode those. -func decodeResponse(responseFields []dns.RR) ([]DDRResponse, error) { - responses := make([]DDRResponse, 0) +func decodeResponse(responseFields []dns.RR) ([]model.SVCBData, error) { + responses := make([]model.SVCBData, 0) for _, rr := range responseFields { switch rr := rr.(type) { case *dns.SVCB: @@ -144,7 +137,7 @@ func decodeResponse(responseFields []dns.RR) ([]DDRResponse, error) { return responses, nil } -func parseSvcb(rr *dns.SVCB) (DDRResponse, error) { +func parseSvcb(rr *dns.SVCB) (model.SVCBData, error) { keys := make(map[string]string) for _, kv := range rr.Value { value := kv.String() @@ -152,10 +145,10 @@ func parseSvcb(rr *dns.SVCB) (DDRResponse, error) { keys[key] = value } - return DDRResponse{ - Priority: int(rr.Priority), - Target: rr.Target, - Keys: keys, + return model.SVCBData{ + Priority: rr.Priority, + TargetName: rr.Target, + Params: keys, }, nil } @@ -174,6 +167,51 @@ func getSystemResolverAddress() string { return "" } +func createResult(t float64, t0 float64, failure *string, resp model.DNSResponse, resolver string, svcbRecords []model.SVCBData) model.ArchivalDNSLookupResult { + resolverHost, _, err := net.SplitHostPort(resolver) + if err != nil { + log.Warnf("Could not split resolver address %s: %s", resolver, err) + resolverHost = resolver + } + asn, org, err := geoipx.LookupASN(resolverHost) + if err != nil { + log.Warnf("Could not lookup ASN for resolver %s: %s", resolverHost, err) + asn = 0 + org = "" + } + + answers := make([]model.ArchivalDNSAnswer, 0) + for _, record := range svcbRecords { + // Create an ArchivalDNSAnswer for each SVCB record + // for this experiment, only the SVCB key is relevant. + answers = append(answers, model.ArchivalDNSAnswer{ + ASN: int64(asn), + ASOrgName: org, + AnswerType: "SVCB", + Hostname: "", + IPv4: "", + IPv6: "", + SVCB: &record, + }) + } + + return model.ArchivalDNSLookupResult{ + Answers: answers, + Engine: "udp", + Failure: failure, + GetaddrinfoError: 0, + Hostname: "_dns.resolver.arpa.", + QueryType: "SVCB", + RawResponse: resp.Bytes(), + Rcode: int64(resp.Rcode()), + ResolverAddress: resolverHost, + T0: t0, + T: t, + Tags: nil, + TransactionID: 0, + } +} + func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{ config: config, diff --git a/internal/experiment/ddr/ddr_test.go b/internal/experiment/ddr/ddr_test.go index 28d930621b..766f27060c 100644 --- a/internal/experiment/ddr/ddr_test.go +++ b/internal/experiment/ddr/ddr_test.go @@ -46,7 +46,13 @@ func TestMeasurerRun(t *testing.T) { t.Fatal("unexpected Failure") } - if tk.Resolver != oneOneOneOneResolver { + firstAnswer := tk.Queries.Answers[0] + + if firstAnswer.AnswerType != "SVCB" { + t.Fatal("unexpected AnswerType") + } + + if tk.Queries.ResolverAddress != oneOneOneOneResolver { t.Fatal("Resolver should be written to TestKeys") } diff --git a/internal/model/archival.go b/internal/model/archival.go index 1930dc53c5..94032aead8 100644 --- a/internal/model/archival.go +++ b/internal/model/archival.go @@ -198,13 +198,21 @@ type ArchivalDNSLookupResult struct { // ArchivalDNSAnswer is a DNS answer. type ArchivalDNSAnswer struct { - ASN int64 `json:"asn,omitempty"` - ASOrgName string `json:"as_org_name,omitempty"` - AnswerType string `json:"answer_type"` - Hostname string `json:"hostname,omitempty"` - IPv4 string `json:"ipv4,omitempty"` - IPv6 string `json:"ipv6,omitempty"` - TTL *uint32 `json:"ttl"` + ASN int64 `json:"asn,omitempty"` + ASOrgName string `json:"as_org_name,omitempty"` + AnswerType string `json:"answer_type"` + Hostname string `json:"hostname,omitempty"` + IPv4 string `json:"ipv4,omitempty"` + IPv6 string `json:"ipv6,omitempty"` + TTL *uint32 `json:"ttl"` + SVCB *SVCBData `json:"svcb,omitempty"` // SVCB-specific data +} + +// SVCBData represents details of an SVCB record. +type SVCBData struct { + Priority uint16 `json:"priority"` + TargetName string `json:"target_name"` + Params map[string]string `json:"params,omitempty"` // SvcParams key-value pairs } // From 27a2ebc715bd0f3476273e176e59f04450821665 Mon Sep 17 00:00:00 2001 From: frcroth Date: Tue, 4 Feb 2025 16:47:44 +0100 Subject: [PATCH 4/5] Add more tests for DDR experiment --- internal/experiment/ddr/ddr.go | 25 ++++----- internal/experiment/ddr/ddr_test.go | 84 ++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 15 deletions(-) diff --git a/internal/experiment/ddr/ddr.go b/internal/experiment/ddr/ddr.go index 81682f7c6f..cbf050c2e0 100644 --- a/internal/experiment/ddr/ddr.go +++ b/internal/experiment/ddr/ddr.go @@ -51,10 +51,7 @@ type TestKeys struct { Failure *string `json:"failure"` } -func (m *Measurer) Run( - ctx context.Context, - args *model.ExperimentArgs) error { - +func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { log.SetLevel(log.DebugLevel) measurement := args.Measurement @@ -77,20 +74,22 @@ func (m *Measurer) Run( resolver = *m.config.CustomResolver } - // DDR queries are queries of the SVCB type for the _dns.resolver.arpa. domain. - netx := &netxlite.Netx{} dialer := netx.NewDialerWithoutResolver(log.Log) - transport := netxlite.NewUnwrappedDNSOverUDPTransport( - dialer, resolver) + transport := netxlite.NewUnwrappedDNSOverUDPTransport(dialer, resolver) encoder := &netxlite.DNSEncoderMiekg{} - query := encoder.Encode( - "_dns.resolver.arpa.", // As specified in RFC 9462 - dns.TypeSVCB, - true) + // As specified in RFC 9462 a DDR Query is a SVCB query for the _dns.resolver.arpa. domain + query := encoder.Encode("_dns.resolver.arpa.", dns.TypeSVCB, true) t0 := time.Since(measurement.MeasurementStartTimeSaved).Seconds() + resp, err := transport.RoundTrip(ctx, query) if err != nil { + // Since we are using a custom transport, we need to check for context errors manually + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + failure := "interrupted" + tk.Failure = &failure + return nil + } failure := err.Error() tk.Failure = &failure return nil @@ -105,14 +104,12 @@ func (m *Measurer) Run( } ddrResponse, err := decodeResponse(reply.Answer) - if err != nil { decodingError := err.Error() tk.Failure = &decodingError } t := time.Since(measurement.MeasurementStartTimeSaved).Seconds() tk.Queries = createResult(t, t0, tk.Failure, resp, resolver, ddrResponse) - tk.SupportsDDR = len(ddrResponse) > 0 return nil diff --git a/internal/experiment/ddr/ddr_test.go b/internal/experiment/ddr/ddr_test.go index 766f27060c..b150d12871 100644 --- a/internal/experiment/ddr/ddr_test.go +++ b/internal/experiment/ddr/ddr_test.go @@ -3,6 +3,7 @@ package ddr import ( "context" "testing" + "time" "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/mocks" @@ -52,7 +53,7 @@ func TestMeasurerRun(t *testing.T) { t.Fatal("unexpected AnswerType") } - if tk.Queries.ResolverAddress != oneOneOneOneResolver { + if tk.Queries.ResolverAddress != "1.1.1.1" { t.Fatal("Resolver should be written to TestKeys") } @@ -81,6 +82,87 @@ func TestMeasurerFailsWithNoPort(t *testing.T) { } } +func TestMeasurerFailsWithInvalidResolver(t *testing.T) { + invalidResolver := "256.256.256.256:53" + + tk, _ := runExperiment(invalidResolver) + if tk.Failure == nil { + t.Fatal("expected Failure") + } +} + +func TestMeasurerRunWithSystemResolver(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + + measurer := NewExperimentMeasurer(Config{}) + args := &model.ExperimentArgs{ + Callbacks: model.NewPrinterCallbacks(log.Log), + Measurement: new(model.Measurement), + Session: &mocks.Session{ + MockLogger: func() model.Logger { + return log.Log + }, + }, + } + if err := measurer.Run(context.Background(), args); err != nil { + t.Fatal(err) + } + tk := args.Measurement.TestKeys.(*TestKeys) + if tk.Failure != nil { + t.Fatal("unexpected Failure") + } +} + +func TestMeasurerRunWithCancelledContext(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // immediately cancel the context + + measurer := NewExperimentMeasurer(Config{}) + args := &model.ExperimentArgs{ + Callbacks: model.NewPrinterCallbacks(log.Log), + Measurement: new(model.Measurement), + Session: &mocks.Session{ + MockLogger: func() model.Logger { + return log.Log + }, + }, + } + err := measurer.Run(ctx, args) + if err != nil { + t.Fatal("expected no error due to cancelled context") + } + tk := args.Measurement.TestKeys.(*TestKeys) + if tk.Failure == nil || *tk.Failure != "interrupted" { + t.Fatal("expected interrupted failure") + } +} + +func TestMeasurerRunWithTimeout(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond) + defer cancel() + + measurer := NewExperimentMeasurer(Config{}) + args := &model.ExperimentArgs{ + Callbacks: model.NewPrinterCallbacks(log.Log), + Measurement: new(model.Measurement), + Session: &mocks.Session{ + MockLogger: func() model.Logger { + return log.Log + }, + }, + } + err := measurer.Run(ctx, args) + if err != nil { + t.Fatal("expected no error due to context timeout") + } + tk := args.Measurement.TestKeys.(*TestKeys) + if tk.Failure == nil || *tk.Failure != "interrupted" { + t.Fatal("expected interrupted failure") + } +} + func runExperiment(resolver string) (*TestKeys, error) { measurer := NewExperimentMeasurer(Config{ CustomResolver: &resolver, From 47a97b925efcbc9f94f8e30ac075cdd3621f7904 Mon Sep 17 00:00:00 2001 From: frcroth Date: Tue, 25 Mar 2025 16:39:35 +0100 Subject: [PATCH 5/5] Implement LookupSVCB in DNS resolvers --- internal/bytecounter/resolver.go | 13 ++ internal/bytecounter/resolver_test.go | 78 +++++++++++ internal/cmd/oohelper/internal/fake_test.go | 4 + internal/engineresolver/resolver.go | 5 + internal/experiment/ddr/ddr.go | 137 ++++---------------- internal/experiment/ddr/ddr_test.go | 29 +---- internal/experiment/dnscheck/dnscheck.go | 2 +- internal/legacy/measurex/resolver.go | 27 ++++ internal/legacy/tracex/archival.go | 38 ++++++ internal/legacy/tracex/event.go | 3 + internal/legacy/tracex/resolver.go | 25 ++++ internal/measurexlite/dns.go | 7 + internal/mocks/dnsresponse.go | 5 + internal/mocks/resolver.go | 6 + internal/model/archival.go | 16 +-- internal/model/netx.go | 38 +++++- internal/netxlite/bogon.go | 6 + internal/netxlite/dnsdecoder.go | 45 +++++++ internal/netxlite/dnsdecoder_test.go | 95 ++++++++++++++ internal/netxlite/dnsovergetaddrinfo.go | 4 + internal/netxlite/resolvercache.go | 5 + internal/netxlite/resolvercore.go | 56 ++++++++ internal/netxlite/resolvercore_test.go | 11 ++ internal/netxlite/resolverparallel.go | 12 ++ internal/netxlite/resolverserial.go | 11 ++ internal/registry/factory_test.go | 2 +- 26 files changed, 528 insertions(+), 152 deletions(-) diff --git a/internal/bytecounter/resolver.go b/internal/bytecounter/resolver.go index 453ea931f4..e8bf2727d5 100644 --- a/internal/bytecounter/resolver.go +++ b/internal/bytecounter/resolver.go @@ -41,6 +41,11 @@ func (r *ContextAwareSystemResolver) LookupHTTPS(ctx context.Context, domain str return r.wrap(ctx).LookupHTTPS(ctx, domain) } +// LookupSVCB implements model.Resolver. +func (r *ContextAwareSystemResolver) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + return r.wrap(ctx).LookupSVCB(ctx, domain) +} + // LookupHost implements model.Resolver. func (r *ContextAwareSystemResolver) LookupHost(ctx context.Context, hostname string) (addrs []string, err error) { return r.wrap(ctx).LookupHost(ctx, hostname) @@ -108,6 +113,14 @@ func (r *resolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPS return out, err } +// LookupSVCB implements model.Resolver +func (r *resolver) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + r.updateCounterBytesSent(domain, 1) + out, err := r.Resolver.LookupSVCB(ctx, domain) + r.updateCounterBytesRecv(err) + return out, err +} + // LookupHost implements model.Resolver func (r *resolver) LookupHost(ctx context.Context, hostname string) (addrs []string, err error) { r.updateCounterBytesSent(hostname, 2) diff --git a/internal/bytecounter/resolver_test.go b/internal/bytecounter/resolver_test.go index 37ab4f0498..1644352035 100644 --- a/internal/bytecounter/resolver_test.go +++ b/internal/bytecounter/resolver_test.go @@ -122,6 +122,84 @@ func TestMaybeWrapSystemResolver(t *testing.T) { }) }) + t.Run("LookupSVCB works as intended", func(t *testing.T) { + t.Run("on success", func(t *testing.T) { + expected := []*model.SVCB{ + {Priority: 1, TargetName: "target1"}, + {Priority: 2, TargetName: "target2"}, + {Priority: 3, TargetName: "target3"}, + } + underlying := &mocks.Resolver{ + MockLookupSVCB: func(ctx context.Context, domain string) ([]*model.SVCB, error) { + return expected, nil + }, + } + counter := New() + reso := MaybeWrapSystemResolver(underlying, counter) + got, err := reso.LookupSVCB(context.Background(), "dns.google") + if err != nil { + t.Fatal("unexpected error", err) + } + if len(got) != 3 { + t.Fatal("invalid result") + } + if nsent := counter.BytesSent(); nsent != 10 { + t.Fatal("unexpected nsent", nsent) + } + if nrecv := counter.BytesReceived(); nrecv != 256 { + t.Fatal("unexpected nrecv") + } + }) + + t.Run("on non-DNS failure", func(t *testing.T) { + expected := errors.New("mocked error") + underlying := &mocks.Resolver{ + MockLookupSVCB: func(ctx context.Context, domain string) ([]*model.SVCB, error) { + return nil, expected + }, + } + counter := New() + reso := MaybeWrapSystemResolver(underlying, counter) + got, err := reso.LookupSVCB(context.Background(), "dns.google") + if !errors.Is(err, expected) { + t.Fatal("unexpected error", err) + } + if got != nil { + t.Fatal("invalid result") + } + if nsent := counter.BytesSent(); nsent != 10 { + t.Fatal("unexpected nsent", nsent) + } + if nrecv := counter.BytesReceived(); nrecv != 0 { + t.Fatal("unexpected nrecv") + } + }) + + t.Run("on DNS failure", func(t *testing.T) { + expected := errors.New(netxlite.FailureDNSNXDOMAINError) + underlying := &mocks.Resolver{ + MockLookupSVCB: func(ctx context.Context, domain string) ([]*model.SVCB, error) { + return nil, expected + }, + } + counter := New() + reso := MaybeWrapSystemResolver(underlying, counter) + got, err := reso.LookupSVCB(context.Background(), "dns.google") + if !errors.Is(err, expected) { + t.Fatal("unexpected error", err) + } + if got != nil { + t.Fatal("invalid result") + } + if nsent := counter.BytesSent(); nsent != 10 { + t.Fatal("unexpected nsent", nsent) + } + if nrecv := counter.BytesReceived(); nrecv != 128 { + t.Fatal("unexpected nrecv") + } + }) + }) + t.Run("LookupNS works as intended", func(t *testing.T) { t.Run("on success", func(t *testing.T) { underlying := &mocks.Resolver{ diff --git a/internal/cmd/oohelper/internal/fake_test.go b/internal/cmd/oohelper/internal/fake_test.go index dd1888b4c4..41edaab780 100644 --- a/internal/cmd/oohelper/internal/fake_test.go +++ b/internal/cmd/oohelper/internal/fake_test.go @@ -56,6 +56,10 @@ func (c FakeResolver) LookupHTTPS(ctx context.Context, domain string) (*model.HT return nil, errors.New("not implemented") } +func (c FakeResolver) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + return nil, errors.New("not implemented") +} + func (c FakeResolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { return nil, errors.New("not implemented") } diff --git a/internal/engineresolver/resolver.go b/internal/engineresolver/resolver.go index d203cd702a..4de789fd39 100644 --- a/internal/engineresolver/resolver.go +++ b/internal/engineresolver/resolver.go @@ -88,6 +88,11 @@ func (r *Resolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPS return nil, errLookupNotImplemented } +// LookupSVCB implements Resolver.LookupSVCB. +func (r *Resolver) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + return nil, errLookupNotImplemented +} + // LookupNS implements Resolver.LookupNS. func (r *Resolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { return nil, errLookupNotImplemented diff --git a/internal/experiment/ddr/ddr.go b/internal/experiment/ddr/ddr.go index cbf050c2e0..de32815cf2 100644 --- a/internal/experiment/ddr/ddr.go +++ b/internal/experiment/ddr/ddr.go @@ -9,7 +9,8 @@ import ( "github.com/apex/log" "github.com/miekg/dns" - "github.com/ooni/probe-cli/v3/internal/geoipx" + "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/legacy/tracex" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" ) @@ -42,7 +43,7 @@ func (m *Measurer) ExperimentVersion() string { type TestKeys struct { // DNS Queries and results (as specified in https://github.com/ooni/spec/blob/master/data-formats/df-002-dnst.md#dns-data-format) - Queries model.ArchivalDNSLookupResult `json:"queries"` + Queries []model.ArchivalDNSLookupResult `json:"queries"` // SupportsDDR is true if DDR is supported. SupportsDDR bool `json:"supports_ddr"` @@ -62,93 +63,46 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - resolver := "" + resolverAddress := "" if m.config.CustomResolver == nil { systemResolver := getSystemResolverAddress() if systemResolver == "" { return errors.New("could not get system resolver") } log.Infof("Using system resolver: %s", systemResolver) - resolver = systemResolver + resolverAddress = systemResolver } else { - resolver = *m.config.CustomResolver + resolverAddress = *m.config.CustomResolver } - netx := &netxlite.Netx{} - dialer := netx.NewDialerWithoutResolver(log.Log) - transport := netxlite.NewUnwrappedDNSOverUDPTransport(dialer, resolver) - encoder := &netxlite.DNSEncoderMiekg{} - // As specified in RFC 9462 a DDR Query is a SVCB query for the _dns.resolver.arpa. domain - query := encoder.Encode("_dns.resolver.arpa.", dns.TypeSVCB, true) - t0 := time.Since(measurement.MeasurementStartTimeSaved).Seconds() + netxlite := &netxlite.Netx{} + evsaver := new(tracex.Saver) + dialer := netxlite.NewDialerWithoutResolver(log.Log) + baseResolver := netxlite.NewParallelUDPResolver(log.Log, dialer, resolverAddress) + resolver := netx.NewResolver(netx.Config{ + BaseResolver: baseResolver, + Saver: evsaver, + }) - resp, err := transport.RoundTrip(ctx, query) + // As specified in RFC 9462 a DDR Query is a SVCB query for the _dns.resolver.arpa. domain + resp, err := resolver.LookupSVCB(ctx, "_dns.resolver.arpa.") if err != nil { - // Since we are using a custom transport, we need to check for context errors manually - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - failure := "interrupted" - tk.Failure = &failure - return nil - } - failure := err.Error() - tk.Failure = &failure + tk.Failure = new(string) + *tk.Failure = err.Error() return nil } + queries := tracex.NewDNSQueriesList(measurement.MeasurementStartTimeSaved, evsaver.Read()) - reply := &dns.Msg{} - err = reply.Unpack(resp.Bytes()) - if err != nil { - unpackError := err.Error() - tk.Failure = &unpackError - return nil + for r := range resp { + log.Debug(fmt.Sprintf("Got SVCB record: %v", r)) } - ddrResponse, err := decodeResponse(reply.Answer) - if err != nil { - decodingError := err.Error() - tk.Failure = &decodingError - } - t := time.Since(measurement.MeasurementStartTimeSaved).Seconds() - tk.Queries = createResult(t, t0, tk.Failure, resp, resolver, ddrResponse) - tk.SupportsDDR = len(ddrResponse) > 0 + tk.Queries = queries + tk.SupportsDDR = len(resp) > 0 return nil } -// decodeResponse decodes the response from the DNS query. -// DDR is only concerned with SVCB records, so we only decode those. -func decodeResponse(responseFields []dns.RR) ([]model.SVCBData, error) { - responses := make([]model.SVCBData, 0) - for _, rr := range responseFields { - switch rr := rr.(type) { - case *dns.SVCB: - parsed, err := parseSvcb(rr) - if err != nil { - return nil, err - } - responses = append(responses, parsed) - default: - return nil, fmt.Errorf("unknown RR type: %T", rr) - } - } - return responses, nil -} - -func parseSvcb(rr *dns.SVCB) (model.SVCBData, error) { - keys := make(map[string]string) - for _, kv := range rr.Value { - value := kv.String() - key := kv.Key().String() - keys[key] = value - } - - return model.SVCBData{ - Priority: rr.Priority, - TargetName: rr.Target, - Params: keys, - }, nil -} - // Get the system resolver address from /etc/resolv.conf // This should also be possible via querying the system resolver and checking the response func getSystemResolverAddress() string { @@ -164,51 +118,6 @@ func getSystemResolverAddress() string { return "" } -func createResult(t float64, t0 float64, failure *string, resp model.DNSResponse, resolver string, svcbRecords []model.SVCBData) model.ArchivalDNSLookupResult { - resolverHost, _, err := net.SplitHostPort(resolver) - if err != nil { - log.Warnf("Could not split resolver address %s: %s", resolver, err) - resolverHost = resolver - } - asn, org, err := geoipx.LookupASN(resolverHost) - if err != nil { - log.Warnf("Could not lookup ASN for resolver %s: %s", resolverHost, err) - asn = 0 - org = "" - } - - answers := make([]model.ArchivalDNSAnswer, 0) - for _, record := range svcbRecords { - // Create an ArchivalDNSAnswer for each SVCB record - // for this experiment, only the SVCB key is relevant. - answers = append(answers, model.ArchivalDNSAnswer{ - ASN: int64(asn), - ASOrgName: org, - AnswerType: "SVCB", - Hostname: "", - IPv4: "", - IPv6: "", - SVCB: &record, - }) - } - - return model.ArchivalDNSLookupResult{ - Answers: answers, - Engine: "udp", - Failure: failure, - GetaddrinfoError: 0, - Hostname: "_dns.resolver.arpa.", - QueryType: "SVCB", - RawResponse: resp.Bytes(), - Rcode: int64(resp.Rcode()), - ResolverAddress: resolverHost, - T0: t0, - T: t, - Tags: nil, - TransactionID: 0, - } -} - func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return &Measurer{ config: config, diff --git a/internal/experiment/ddr/ddr_test.go b/internal/experiment/ddr/ddr_test.go index b150d12871..1ee246b296 100644 --- a/internal/experiment/ddr/ddr_test.go +++ b/internal/experiment/ddr/ddr_test.go @@ -3,7 +3,6 @@ package ddr import ( "context" "testing" - "time" "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/mocks" @@ -47,13 +46,13 @@ func TestMeasurerRun(t *testing.T) { t.Fatal("unexpected Failure") } - firstAnswer := tk.Queries.Answers[0] + firstAnswer := tk.Queries[0].Answers[0] if firstAnswer.AnswerType != "SVCB" { t.Fatal("unexpected AnswerType") } - if tk.Queries.ResolverAddress != "1.1.1.1" { + if tk.Queries[0].ResolverAddress != "1.1.1.1:53" { t.Fatal("Resolver should be written to TestKeys") } @@ -139,30 +138,6 @@ func TestMeasurerRunWithCancelledContext(t *testing.T) { } } -func TestMeasurerRunWithTimeout(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Nanosecond) - defer cancel() - - measurer := NewExperimentMeasurer(Config{}) - args := &model.ExperimentArgs{ - Callbacks: model.NewPrinterCallbacks(log.Log), - Measurement: new(model.Measurement), - Session: &mocks.Session{ - MockLogger: func() model.Logger { - return log.Log - }, - }, - } - err := measurer.Run(ctx, args) - if err != nil { - t.Fatal("expected no error due to context timeout") - } - tk := args.Measurement.TestKeys.(*TestKeys) - if tk.Failure == nil || *tk.Failure != "interrupted" { - t.Fatal("expected interrupted failure") - } -} - func runExperiment(resolver string) (*TestKeys, error) { measurer := NewExperimentMeasurer(Config{ CustomResolver: &resolver, diff --git a/internal/experiment/dnscheck/dnscheck.go b/internal/experiment/dnscheck/dnscheck.go index 815ab2511a..dcee12b437 100644 --- a/internal/experiment/dnscheck/dnscheck.go +++ b/internal/experiment/dnscheck/dnscheck.go @@ -172,7 +172,7 @@ func (m *Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { return ErrUnsupportedURLScheme } - // Implementation note: we must not return an error from now now. Returning an + // Implementation note: we must not return an error from now. Returning an // error means that we don't have a measurement to submit. // 4. possibly expand a domain to a list of IP addresses. diff --git a/internal/legacy/measurex/resolver.go b/internal/legacy/measurex/resolver.go index 86c5dd10dd..3aa76af98c 100644 --- a/internal/legacy/measurex/resolver.go +++ b/internal/legacy/measurex/resolver.go @@ -187,3 +187,30 @@ func (r *resolverDB) computeOddityHTTPSSvc(https *model.HTTPSSvc, err error) Odd addrs = append(addrs, https.IPv6...) return r.computeOddityLookupHost(addrs, nil) } + +func (r *resolverDB) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + started := time.Since(r.begin).Seconds() + svcb, err := r.Resolver.LookupSVCB(ctx, domain) + finished := time.Since(r.begin).Seconds() + + // DNS lookup event does not really support SVCB + ev := &DNSLookupEvent{ + Network: tracex.ResolverNetworkAdaptNames(r.Resolver.Network()), + Address: r.Resolver.Address(), + Domain: domain, + QueryType: "SVCB", + Started: started, + Finished: finished, + Failure: NewFailure(err), + } + + if err == nil { + for _, svc := range svcb { + for _, alpn := range svc.ALPN { + ev.ALPN = append(ev.ALPN, alpn) + } + } + } + r.db.InsertIntoLookupHTTPSSvc(ev) + return svcb, err +} diff --git a/internal/legacy/tracex/archival.go b/internal/legacy/tracex/archival.go index 55f58714ba..365d28a30f 100644 --- a/internal/legacy/tracex/archival.go +++ b/internal/legacy/tracex/archival.go @@ -156,6 +156,44 @@ func NewDNSQueriesList(begin time.Time, events []Event) (out []DNSQueryEntry) { } out = append(out, entry) } + // Handle SVCB records + if ev.DNSQueryType == "SVCB" { + entry := DNSQueryEntry{ + Engine: ev.Proto, + Failure: ev.Err.ToFailure(), + Hostname: ev.Hostname, + QueryType: ev.DNSQueryType, + ResolverAddress: ev.Address, + T: ev.Time.Sub(begin).Seconds(), + } + svcbResponses := make([]model.SVCBData, 0) + for _, record := range ev.DNSSVCBRespones { + svcb := model.SVCBData{Priority: record.Priority, + TargetName: record.TargetName} + svcb.Params = make(map[string]string) + + svcb.Params["alpn"] = strings.Join(record.ALPN, ",") + svcb.Params["ipv4hint"] = strings.Join(record.IPv4, ",") + svcb.Params["ipv6hint"] = strings.Join(record.IPv6, ",") + if record.DoHPath != "" { + svcb.Params["dohpath"] = record.DoHPath + } + if record.OHttp { + svcb.Params["ohttp"] = strconv.FormatBool(record.OHttp) + } + if record.Port != 0 { + svcb.Params["port"] = strconv.Itoa(int(record.Port)) + } + //svcb.Params["echconfig"] = record.ECHConfig + svcbResponses = append(svcbResponses, svcb) + } + entry.Answers = append(entry.Answers, DNSAnswerEntry{ + AnswerType: "SVCB", + SVCB: svcbResponses, + }) + out = append(out, entry) + } + } return } diff --git a/internal/legacy/tracex/event.go b/internal/legacy/tracex/event.go index f593928cc1..84936c9445 100644 --- a/internal/legacy/tracex/event.go +++ b/internal/legacy/tracex/event.go @@ -9,6 +9,7 @@ import ( "net/http" "time" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -276,8 +277,10 @@ func (ev *EventWriteOperation) Name() string { type EventValue struct { Addresses []string `json:",omitempty"` Address string `json:",omitempty"` + DNSQueryType string `json:",omitempty"` DNSQuery []byte `json:",omitempty"` DNSResponse []byte `json:",omitempty"` + DNSSVCBRespones []*model.SVCB `json:",omitempty"` Data []byte `json:",omitempty"` Duration time.Duration `json:",omitempty"` Err FailureStr `json:",omitempty"` diff --git a/internal/legacy/tracex/resolver.go b/internal/legacy/tracex/resolver.go index 363bec8415..004124f214 100644 --- a/internal/legacy/tracex/resolver.go +++ b/internal/legacy/tracex/resolver.go @@ -92,6 +92,31 @@ func (r *ResolverSaver) LookupHTTPS(ctx context.Context, domain string) (*model. return r.Resolver.LookupHTTPS(ctx, domain) } +func (r *ResolverSaver) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + start := time.Now() + r.Saver.Write(&EventResolveStart{&EventValue{ + Address: r.Resolver.Address(), + DNSQueryType: "SVCB", + Hostname: domain, + Proto: r.Network(), + Time: start, + }}) + resp, err := r.Resolver.LookupSVCB(ctx, domain) + + stop := time.Now() + r.Saver.Write(&EventResolveDone{&EventValue{ + Address: r.Resolver.Address(), + DNSQueryType: "SVCB", + DNSSVCBRespones: resp, + Duration: stop.Sub(start), + Err: NewFailureStr(err), + Hostname: domain, + Proto: r.Network(), + Time: stop, + }}) + return resp, err +} + func (r *ResolverSaver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { // TODO(bassosimone): we should probably implement this method return r.Resolver.LookupNS(ctx, domain) diff --git a/internal/measurexlite/dns.go b/internal/measurexlite/dns.go index 662942089d..11633ee15b 100644 --- a/internal/measurexlite/dns.go +++ b/internal/measurexlite/dns.go @@ -85,6 +85,13 @@ func (r *resolverTrace) LookupHTTPS(ctx context.Context, domain string) (*model. return r.r.LookupHTTPS(netxlite.ContextWithTrace(ctx, r.tx), domain) } +// LookupSVCB implements model.Resolver.LookupSVCB +func (r *resolverTrace) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + defer r.emiteResolveDone() + r.emitResolveStart() + return r.r.LookupSVCB(netxlite.ContextWithTrace(ctx, r.tx), domain) +} + // LookupNS implements model.Resolver.LookupNS func (r *resolverTrace) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { defer r.emiteResolveDone() diff --git a/internal/mocks/dnsresponse.go b/internal/mocks/dnsresponse.go index d2dcf96292..872aceb4be 100644 --- a/internal/mocks/dnsresponse.go +++ b/internal/mocks/dnsresponse.go @@ -16,6 +16,7 @@ type DNSResponse struct { MockBytes func() []byte MockRcode func() int MockDecodeHTTPS func() (*model.HTTPSSvc, error) + MockDecodeSVCB func() ([]*model.SVCB, error) MockDecodeLookupHost func() ([]string, error) MockDecodeNS func() ([]*net.NS, error) MockDecodeCNAME func() (string, error) @@ -39,6 +40,10 @@ func (r *DNSResponse) DecodeHTTPS() (*model.HTTPSSvc, error) { return r.MockDecodeHTTPS() } +func (r *DNSResponse) DecodeSVCB() ([]*model.SVCB, error) { + return r.MockDecodeSVCB() +} + func (r *DNSResponse) DecodeLookupHost() ([]string, error) { return r.MockDecodeLookupHost() } diff --git a/internal/mocks/resolver.go b/internal/mocks/resolver.go index d9879fdd90..1ebd38fe5b 100644 --- a/internal/mocks/resolver.go +++ b/internal/mocks/resolver.go @@ -14,6 +14,7 @@ type Resolver struct { MockAddress func() string MockCloseIdleConnections func() MockLookupHTTPS func(ctx context.Context, domain string) (*model.HTTPSSvc, error) + MockLookupSVCB func(ctx context.Context, domain string) ([]*model.SVCB, error) MockLookupNS func(ctx context.Context, domain string) ([]*net.NS, error) } @@ -42,6 +43,11 @@ func (r *Resolver) LookupHTTPS(ctx context.Context, domain string) (*model.HTTPS return r.MockLookupHTTPS(ctx, domain) } +// LookupSVCB calls MockLookupSVCB. +func (r *Resolver) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + return r.MockLookupSVCB(ctx, domain) +} + // LookupNS calls MockLookupNS. func (r *Resolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { return r.MockLookupNS(ctx, domain) diff --git a/internal/model/archival.go b/internal/model/archival.go index 94032aead8..6e59f60776 100644 --- a/internal/model/archival.go +++ b/internal/model/archival.go @@ -198,14 +198,14 @@ type ArchivalDNSLookupResult struct { // ArchivalDNSAnswer is a DNS answer. type ArchivalDNSAnswer struct { - ASN int64 `json:"asn,omitempty"` - ASOrgName string `json:"as_org_name,omitempty"` - AnswerType string `json:"answer_type"` - Hostname string `json:"hostname,omitempty"` - IPv4 string `json:"ipv4,omitempty"` - IPv6 string `json:"ipv6,omitempty"` - TTL *uint32 `json:"ttl"` - SVCB *SVCBData `json:"svcb,omitempty"` // SVCB-specific data + ASN int64 `json:"asn,omitempty"` + ASOrgName string `json:"as_org_name,omitempty"` + AnswerType string `json:"answer_type"` + Hostname string `json:"hostname,omitempty"` + IPv4 string `json:"ipv4,omitempty"` + IPv6 string `json:"ipv6,omitempty"` + TTL *uint32 `json:"ttl"` + SVCB []SVCBData `json:"svcb,omitempty"` // SVCB-specific data } // SVCBData represents details of an SVCB record. diff --git a/internal/model/netx.go b/internal/model/netx.go index f6f4bc78ff..32711515fa 100644 --- a/internal/model/netx.go +++ b/internal/model/netx.go @@ -29,10 +29,14 @@ type DNSResponse interface { // Rcode returns the response's Rcode. Rcode() int - // DecodeHTTPS returns information gathered from all the HTTPS + // DecodeHTTPS returns information gathered from all the HTTPS/SVCB // records found inside of this response. DecodeHTTPS() (*HTTPSSvc, error) + // DecodeSVCB returns information gathered from all the SVCB records + // found inside of this response. + DecodeSVCB() ([]*SVCB, error) + // DecodeLookupHost returns the addresses in the response matching // the original query type (one of A and AAAA). DecodeLookupHost() ([]string, error) @@ -186,6 +190,35 @@ type HTTPSSvc struct { IPv6 []string } +// SVCB is the reply to an SVCB DNS query. +type SVCB struct { + // Priority is the priority of the SVCB record. + Priority uint16 + + // TargetName is the target name of the SVCB record. + TargetName string + + // ALPN contains the ALPNs inside the SVCB reply. + ALPN []string + + // IPv4 contains the IPv4 hint (which may be empty). + IPv4 []string + + // IPv6 contains the IPv6 hint (which may be empty). + IPv6 []string + + // Port is the port to use for the connection. + Port uint16 + + // DoHPath is the path to use for DNS-over-HTTPS. + DoHPath string + + // OHttp denotes whether oblivious DNS over HTTPS is supported. + OHttp bool + + //This could also include ECH and other SVCB parameters +} + // MeasuringNetwork defines the constructors required for implementing OONI experiments. All // these constructors MUST guarantee proper error wrapping to map Go errors to OONI errors // as documented by the [netxlite] package. The [*netxlite.Netx] type is currently the default @@ -307,6 +340,9 @@ type Resolver interface { // LookupNS issues a NS query for a domain. LookupNS(ctx context.Context, domain string) ([]*net.NS, error) + + //LookupSVCB issues a SVCB query for a domain. + LookupSVCB(ctx context.Context, domain string) ([]*SVCB, error) } // TLSConn is the type of connection that oohttp expects from diff --git a/internal/netxlite/bogon.go b/internal/netxlite/bogon.go index a31f57119e..4701a6fc39 100644 --- a/internal/netxlite/bogon.go +++ b/internal/netxlite/bogon.go @@ -59,6 +59,12 @@ func (r *bogonResolver) LookupHTTPS(ctx context.Context, hostname string) (*mode return nil, ErrNoDNSTransport } +// LookupSVCB implements Resolver.LookupSVCB +func (r *bogonResolver) LookupSVCB(ctx context.Context, hostname string) ([]*model.SVCB, error) { + // TODO: decide whether we want to implement this method or not + return nil, ErrNoDNSTransport +} + // LookupNS implements Resolver.LookupNS func (r *bogonResolver) LookupNS(ctx context.Context, hostname string) ([]*net.NS, error) { // TODO(bassosimone): decide whether we want to implement this method or not diff --git a/internal/netxlite/dnsdecoder.go b/internal/netxlite/dnsdecoder.go index d00ef1ce26..95d4ed50d1 100644 --- a/internal/netxlite/dnsdecoder.go +++ b/internal/netxlite/dnsdecoder.go @@ -127,6 +127,51 @@ func (r *dnsResponse) DecodeHTTPS() (*model.HTTPSSvc, error) { return out, nil } +// DecodeSVCB implements model.DNSResponse.DecodeSVCB. +func (r *dnsResponse) DecodeSVCB() ([]*model.SVCB, error) { + if err := r.rcodeToError(); err != nil { + return nil, err // error already wrapped + } + out := []*model.SVCB{} + for _, answer := range r.msg.Answer { + switch record := answer.(type) { + case *dns.SVCB: + svcb := &model.SVCB{ + ALPN: []string{}, // ensure it's not nil + IPv4: []string{}, // ensure it's not nil + IPv6: []string{}, // ensure it's not nil + } + for _, v := range record.Value { + switch extv := v.(type) { + case *dns.SVCBAlpn: + svcb.ALPN = extv.Alpn + case *dns.SVCBIPv4Hint: + for _, ip := range extv.Hint { + svcb.IPv4 = append(svcb.IPv4, ip.String()) + } + case *dns.SVCBIPv6Hint: + for _, ip := range extv.Hint { + svcb.IPv6 = append(svcb.IPv6, ip.String()) + } + case *dns.SVCBPort: + svcb.Port = extv.Port + case *dns.SVCBOhttp: + svcb.OHttp = true + case *dns.SVCBDoHPath: + svcb.DoHPath = extv.String() + } + } + svcb.Priority = record.Priority + svcb.TargetName = record.Target + out = append(out, svcb) + } + } + if len(out) <= 0 { + return nil, dnsDecoderWrapError(ErrOODNSNoAnswer) + } + return out, nil +} + // DecodeLookupHost implements model.DNSResponse.DecodeLookupHost. func (r *dnsResponse) DecodeLookupHost() ([]string, error) { if err := r.rcodeToError(); err != nil { diff --git a/internal/netxlite/dnsdecoder_test.go b/internal/netxlite/dnsdecoder_test.go index 931819b291..9ccb4b2d75 100644 --- a/internal/netxlite/dnsdecoder_test.go +++ b/internal/netxlite/dnsdecoder_test.go @@ -271,6 +271,65 @@ func TestDNSDecoderMiekg(t *testing.T) { }) }) + t.Run("dnsResponse.DecodeSVCB", func(t *testing.T) { + t.Run("with failure", func(t *testing.T) { + // Ensure that we're not trying to decode if rcode != 0 + d := &DNSDecoderMiekg{} + queryID := dns.Id() + rawQuery := dnsGenQuery(dns.TypeSVCB, queryID) + rawResponse := dnsGenReplyWithError(rawQuery, dns.RcodeRefused) + query := &mocks.DNSQuery{ + MockID: func() uint16 { + return queryID + }, + } + resp, err := d.DecodeResponse(rawResponse, query) + if err != nil { + t.Fatal(err) + } + svcb, err := resp.DecodeSVCB() + if !errors.Is(err, ErrOODNSRefused) { + t.Fatal("unexpected err", err) + } + if !dnsDecoderErrorIsWrapped(err) { + t.Fatal("unwrapped error", err) + } + if svcb != nil { + t.Fatal("expected nil svcb result") + } + }) + + t.Run("with full answer", func(t *testing.T) { + d := &DNSDecoderMiekg{} + queryID := dns.Id() + rawQuery := dnsGenQuery(dns.TypeSVCB, queryID) + rawResponse := dnsGenSVCBReplySuccess(rawQuery, "example.com.", 443, []string{"sample"}) + query := &mocks.DNSQuery{ + MockID: func() uint16 { + return queryID + }, + } + resp, err := d.DecodeResponse(rawResponse, query) + if err != nil { + t.Fatal(err) + } + svcb, err := resp.DecodeSVCB() + if err != nil { + t.Fatal(err) + } + firstSvcb := svcb[0] + if firstSvcb.TargetName != "example.com." { + t.Fatal("unexpected target name") + } + if firstSvcb.Port != 443 { + t.Fatal("unexpected port") + } + if firstSvcb.ALPN[0] != "sample" { + t.Fatal("unexpected protocol") + } + }) + }) + t.Run("dnsResponse.DecodeNS", func(t *testing.T) { t.Run("with failure", func(t *testing.T) { // Ensure that we're not trying to decode if rcode != 0 @@ -755,6 +814,42 @@ func dnsGenHTTPSReplySuccess(rawQuery []byte, alpns, ipv4s, ipv6s []string) []by return data } +// dnsGenSVCBReplySuccess generates a successful SVCB response containing +// the given host, port, and alpns. +func dnsGenSVCBReplySuccess(rawQuery []byte, target string, port int, alpns []string) []byte { + query := new(dns.Msg) + err := query.Unpack(rawQuery) + runtimex.PanicOnError(err, "query.Unpack failed") + runtimex.Assert(len(query.Question) == 1, "expected just a single question") + question := query.Question[0] + runtimex.Assert(question.Qtype == dns.TypeSVCB, "expected SVCB query") + reply := new(dns.Msg) + reply.Compress = true + reply.MsgHdr.RecursionAvailable = true + reply.SetReply(query) + answer := &dns.SVCB{ + Hdr: dns.RR_Header{ + Name: dns.Fqdn("x.org"), + Rrtype: dns.TypeSVCB, + Class: dns.ClassINET, + Ttl: 100, + }, + Target: dns.Fqdn(target), + Priority: 0, + Value: []dns.SVCBKeyValue{}, + } + reply.Answer = append(reply.Answer, answer) + if port != 0 { + answer.Value = append(answer.Value, &dns.SVCBPort{Port: uint16(port)}) + } + if len(alpns) > 0 { + answer.Value = append(answer.Value, &dns.SVCBAlpn{Alpn: alpns}) + } + data, err := reply.Pack() + runtimex.PanicOnError(err, "reply.Pack failed") + return data +} + // dnsGenNSReplySuccess generates a successful NS reply using the given names. func dnsGenNSReplySuccess(rawQuery []byte, names ...string) []byte { query := new(dns.Msg) diff --git a/internal/netxlite/dnsovergetaddrinfo.go b/internal/netxlite/dnsovergetaddrinfo.go index 9ad4195fd4..a88bde8357 100644 --- a/internal/netxlite/dnsovergetaddrinfo.go +++ b/internal/netxlite/dnsovergetaddrinfo.go @@ -148,6 +148,10 @@ func (r *dnsOverGetaddrinfoResponse) DecodeHTTPS() (*model.HTTPSSvc, error) { return nil, ErrNoDNSTransport } +func (r *dnsOverGetaddrinfoResponse) DecodeSVCB() ([]*model.SVCB, error) { + return nil, ErrNoDNSTransport +} + func (r *dnsOverGetaddrinfoResponse) DecodeLookupHost() ([]string, error) { if len(r.addrs) <= 0 { return nil, ErrOODNSNoAnswer diff --git a/internal/netxlite/resolvercache.go b/internal/netxlite/resolvercache.go index 76ff5ae5b4..ad9a937e6a 100644 --- a/internal/netxlite/resolvercache.go +++ b/internal/netxlite/resolvercache.go @@ -113,6 +113,11 @@ func (r *cacheResolver) LookupHTTPS(ctx context.Context, domain string) (*model. return nil, ErrNoDNSTransport } +// LookupSVCB implements model.Resolver.LookupSVCB. +func (r *cacheResolver) LookupSVCB(ctx context.Context, domain string) ([]*model.SVCB, error) { + return nil, ErrNoDNSTransport +} + // LookupNS implements model.Resolver.LookupNS. func (r *cacheResolver) LookupNS(ctx context.Context, domain string) ([]*net.NS, error) { return nil, ErrNoDNSTransport diff --git a/internal/netxlite/resolvercore.go b/internal/netxlite/resolvercore.go index 6a46beaa07..5098b278f8 100644 --- a/internal/netxlite/resolvercore.go +++ b/internal/netxlite/resolvercore.go @@ -145,6 +145,11 @@ func (r *resolverSystem) LookupHTTPS( return nil, ErrNoDNSTransport } +func (r *resolverSystem) LookupSVCB( + ctx context.Context, domain string) ([]*model.SVCB, error) { + return nil, ErrNoDNSTransport +} + func (r *resolverSystem) LookupNS( ctx context.Context, domain string) ([]*net.NS, error) { return nil, ErrNoDNSTransport @@ -190,6 +195,21 @@ func (r *resolverLogger) LookupHTTPS( return https, nil } +func (r *resolverLogger) LookupSVCB( + ctx context.Context, domain string) ([]*model.SVCB, error) { + prefix := fmt.Sprintf("resolve[SVCB] %s with %s (%s)", domain, r.Network(), r.Address()) + r.Logger.Debugf("%s...", prefix) + start := time.Now() + svcb, err := r.Resolver.LookupSVCB(ctx, domain) + elapsed := time.Since(start) + if err != nil { + r.Logger.Debugf("%s... %s in %s", prefix, err, elapsed) + return nil, err + } + r.Logger.Debugf("%s... %+v in %s", prefix, svcb, elapsed) + return svcb, nil +} + func (r *resolverLogger) Address() string { return r.Resolver.Address() } @@ -243,6 +263,14 @@ func (r *resolverIDNA) LookupHTTPS( return r.Resolver.LookupHTTPS(ctx, host) } +func (r *resolverIDNA) LookupSVCB( + ctx context.Context, domain string) ([]*model.SVCB, error) { + // This does not work here, since we need to query + // for _dns.resolver.arpa., which results in + // error idna: disallowed rune U+005F because of the underscore. + return r.Resolver.LookupSVCB(ctx, domain) +} + func (r *resolverIDNA) Network() string { return r.Resolver.Network() } @@ -292,6 +320,20 @@ func (r *ResolverShortCircuitIPAddr) LookupHTTPS(ctx context.Context, hostname s return r.Resolver.LookupHTTPS(ctx, hostname) } +func (r *ResolverShortCircuitIPAddr) LookupSVCB(ctx context.Context, hostname string) ([]*model.SVCB, error) { + if net.ParseIP(hostname) != nil { + svcb := &model.SVCB{} + if isIPv6(hostname) { + svcb.IPv6 = append(svcb.IPv6, hostname) + } else { + svcb.IPv4 = append(svcb.IPv4, hostname) + } + + return []*model.SVCB{svcb}, nil + } + return r.Resolver.LookupSVCB(ctx, hostname) +} + func (r *ResolverShortCircuitIPAddr) Network() string { return r.Resolver.Network() } @@ -363,6 +405,11 @@ func (r *NullResolver) LookupHTTPS( return nil, ErrNoResolver } +func (r *NullResolver) LookupSVCB( + ctx context.Context, domain string) ([]*model.SVCB, error) { + return nil, ErrNoResolver +} + func (r *NullResolver) LookupNS( ctx context.Context, domain string) ([]*net.NS, error) { return nil, ErrNoResolver @@ -392,6 +439,15 @@ func (r *resolverErrWrapper) LookupHTTPS( return out, nil } +func (r *resolverErrWrapper) LookupSVCB( + ctx context.Context, domain string) ([]*model.SVCB, error) { + out, err := r.Resolver.LookupSVCB(ctx, domain) + if err != nil { + return nil, NewErrWrapper(ClassifyResolverError, ResolveOperation, err) + } + return out, nil +} + func (r *resolverErrWrapper) Network() string { return r.Resolver.Network() } diff --git a/internal/netxlite/resolvercore_test.go b/internal/netxlite/resolvercore_test.go index 43df93aa58..79ed6bdd28 100644 --- a/internal/netxlite/resolvercore_test.go +++ b/internal/netxlite/resolvercore_test.go @@ -198,6 +198,17 @@ func TestResolverSystem(t *testing.T) { } }) + t.Run("LookupSVCB", func(t *testing.T) { + r := &resolverSystem{} + svcb, err := r.LookupSVCB(context.Background(), "x.org") + if !errors.Is(err, ErrNoDNSTransport) { + t.Fatal("not the error we expected") + } + if svcb != nil { + t.Fatal("expected nil result") + } + }) + t.Run("LookupNS", func(t *testing.T) { r := &resolverSystem{} ns, err := r.LookupNS(context.Background(), "x.org") diff --git a/internal/netxlite/resolverparallel.go b/internal/netxlite/resolverparallel.go index 129fe15e6d..948568ef9c 100644 --- a/internal/netxlite/resolverparallel.go +++ b/internal/netxlite/resolverparallel.go @@ -87,6 +87,18 @@ func (r *ParallelResolver) LookupHTTPS( return response.DecodeHTTPS() } +// LookupSVCB implements Resolver.LookupSVCB. +func (r *ParallelResolver) LookupSVCB( + ctx context.Context, hostname string) ([]*model.SVCB, error) { + encoder := &DNSEncoderMiekg{} + query := encoder.Encode(hostname, dns.TypeSVCB, r.Txp.RequiresPadding()) + response, err := r.Txp.RoundTrip(ctx, query) + if err != nil { + return nil, err + } + return response.DecodeSVCB() +} + // parallelResolverResult is the internal representation of a // lookup using either the A or the AAAA query type. type parallelResolverResult struct { diff --git a/internal/netxlite/resolverserial.go b/internal/netxlite/resolverserial.go index e1c7e38b40..53274220f2 100644 --- a/internal/netxlite/resolverserial.go +++ b/internal/netxlite/resolverserial.go @@ -94,6 +94,17 @@ func (r *SerialResolver) LookupHTTPS( return response.DecodeHTTPS() } +func (r *SerialResolver) LookupSVCB( + ctx context.Context, hostname string) ([]*model.SVCB, error) { + encoder := &DNSEncoderMiekg{} + query := encoder.Encode(hostname, dns.TypeSVCB, r.Txp.RequiresPadding()) + response, err := r.Txp.RoundTrip(ctx, query) + if err != nil { + return nil, err + } + return response.DecodeSVCB() +} + func (r *SerialResolver) lookupHostWithRetry( ctx context.Context, hostname string, qtype uint16) ([]string, error) { // QUIRK: retrying has been there since the beginning so we need to diff --git a/internal/registry/factory_test.go b/internal/registry/factory_test.go index 425cc5269a..c83172250b 100644 --- a/internal/registry/factory_test.go +++ b/internal/registry/factory_test.go @@ -594,7 +594,7 @@ func TestNewFactory(t *testing.T) { }, "echcheck": { enabledByDefault: true, - inputPolicy: model.InputOptional, + inputPolicy: model.InputOptional, }, "example": { enabledByDefault: true,