diff --git a/internal/cmd/minipipeline/main.go b/internal/cmd/minipipeline/main.go index ede5f9e4fb..4b3d966f21 100644 --- a/internal/cmd/minipipeline/main.go +++ b/internal/cmd/minipipeline/main.go @@ -6,7 +6,9 @@ import ( "os" "path/filepath" + "github.com/ooni/probe-cli/v3/internal/geoipx" "github.com/ooni/probe-cli/v3/internal/minipipeline" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/must" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -56,8 +58,9 @@ func main() { must.UnmarshalJSON(must.ReadFile(*measurementFlag), &parsed) // generate and write observations + lookupper := model.GeoIPASNLookupperFunc(geoipx.LookupASN) observationsPath := filepath.Join(*destdirFlag, *prefixFlag+"observations.json") - container := runtimex.Try1(minipipeline.IngestWebMeasurement(&parsed)) + container := runtimex.Try1(minipipeline.IngestWebMeasurement(lookupper, &parsed)) mustWriteFileFn(observationsPath, must.MarshalAndIndentJSON(container, "", " "), 0600) // generate and write classic observations @@ -67,11 +70,11 @@ func main() { // generate and write observations analysis analysisPath := filepath.Join(*destdirFlag, *prefixFlag+"analysis.json") - analysis := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(container) + analysis := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(lookupper, container) mustWriteFileFn(analysisPath, must.MarshalAndIndentJSON(analysis, "", " "), 0600) // generate and write the classic analysis classicAnalysisPath := filepath.Join(*destdirFlag, *prefixFlag+"analysis_classic.json") - analysisClassic := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(containerClassic) + analysisClassic := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(lookupper, containerClassic) mustWriteFileFn(classicAnalysisPath, must.MarshalAndIndentJSON(analysisClassic, "", " "), 0600) } diff --git a/internal/cmd/qatool/main.go b/internal/cmd/qatool/main.go index 1b08d47856..2041608a05 100644 --- a/internal/cmd/qatool/main.go +++ b/internal/cmd/qatool/main.go @@ -9,7 +9,9 @@ import ( "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivitylte" "github.com/ooni/probe-cli/v3/internal/experiment/webconnectivityqa" + "github.com/ooni/probe-cli/v3/internal/geoipx" "github.com/ooni/probe-cli/v3/internal/minipipeline" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/must" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -73,8 +75,11 @@ func runWebConnectivityLTE(tc *webconnectivityqa.TestCase) { var webMeasurement minipipeline.WebMeasurement must.UnmarshalJSON(rawData, &webMeasurement) + // create the GeoIP ASN lookupper we're going to use + lookupper := model.GeoIPASNLookupperFunc(geoipx.LookupASN) + // ingest web measurement - observationsContainer := runtimex.Try1(minipipeline.IngestWebMeasurement(&webMeasurement)) + observationsContainer := runtimex.Try1(minipipeline.IngestWebMeasurement(lookupper, &webMeasurement)) // serialize the observations mustSerializeMkdirAllAndWriteFile(actualDestdir, "observations.json", observationsContainer) @@ -86,13 +91,13 @@ func runWebConnectivityLTE(tc *webconnectivityqa.TestCase) { mustSerializeMkdirAllAndWriteFile(actualDestdir, "observations_classic.json", observationsContainerClassic) // analyze the observations - analysis := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(observationsContainer) + analysis := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(lookupper, observationsContainer) // serialize the observations analysis mustSerializeMkdirAllAndWriteFile(actualDestdir, "analysis.json", analysis) // perform the classic analysis - analysisClassic := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(observationsContainerClassic) + analysisClassic := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(lookupper, observationsContainerClassic) // serialize the classic analysis results mustSerializeMkdirAllAndWriteFile(actualDestdir, "analysis_classic.json", analysisClassic) diff --git a/internal/experiment/webconnectivitylte/analysisclassic.go b/internal/experiment/webconnectivitylte/analysisclassic.go index 22e2048bf9..2c687eab07 100644 --- a/internal/experiment/webconnectivitylte/analysisclassic.go +++ b/internal/experiment/webconnectivitylte/analysisclassic.go @@ -8,6 +8,7 @@ package webconnectivitylte // import ( + "github.com/ooni/probe-cli/v3/internal/geoipx" "github.com/ooni/probe-cli/v3/internal/minipipeline" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/optional" @@ -18,17 +19,17 @@ import ( // results that are backward compatible with Web Connectivity v0.4 while also // procuding more fine-grained blocking flags. func analysisEngineClassic(tk *TestKeys, logger model.Logger) { - tk.analysisClassic(logger) + tk.analysisClassic(model.GeoIPASNLookupperFunc(geoipx.LookupASN), logger) } -func (tk *TestKeys) analysisClassic(logger model.Logger) { +func (tk *TestKeys) analysisClassic(lookupper model.GeoIPASNLookupper, logger model.Logger) { // Since we run after all tasks have completed (or so we assume) we're // not going to use any form of locking here. // 1. produce web observations container := minipipeline.NewWebObservationsContainer() - container.IngestDNSLookupEvents(tk.Queries...) - container.IngestTCPConnectEvents(tk.TCPConnect...) + container.IngestDNSLookupEvents(lookupper, tk.Queries...) + container.IngestTCPConnectEvents(lookupper, tk.TCPConnect...) container.IngestTLSHandshakeEvents(tk.TLSHandshakes...) container.IngestHTTPRoundTripEvents(tk.Requests...) @@ -40,14 +41,14 @@ func (tk *TestKeys) analysisClassic(logger model.Logger) { } // 2. compute extended analysis flags - analysisExtMain(tk, container) + analysisExtMain(lookupper, tk, container) // 3. filter observations to only include results collected by the // system resolver, which approximates v0.4's results classic := minipipeline.ClassicFilter(container) // 3. produce a web observations analysis based on the web observations - woa := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(classic) + woa := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(lookupper, classic) // 5. determine the DNS consistency tk.DNSConsistency = analysisClassicDNSConsistency(woa) diff --git a/internal/experiment/webconnectivitylte/analysisclassic_test.go b/internal/experiment/webconnectivitylte/analysisclassic_test.go new file mode 100644 index 0000000000..5d6e27bdbc --- /dev/null +++ b/internal/experiment/webconnectivitylte/analysisclassic_test.go @@ -0,0 +1,87 @@ +package webconnectivitylte + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/mocks" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/tailscale/hujson" +) + +// This test is related to https://github.com/ooni/probe/issues/2517. +// +// It shows how blocking flags could vary depending on the ASN distribution based on +// a real case where we observed false positives caused by that. +func TestTestKeys_analysisDNSToplevel(t *testing.T) { + + // testcase is a test case in this test + type testcase struct { + // name is the name of the test case + name string + + // tkFile is the name of the JSONC file containing the test keys + tkFile string + + // geoInfo contains a static mapping of geoip info + geoInfo map[string]*model.LocationASN + + // expectBlockingFlags contains the expected BlockingFlags + expecteBlockingFlags int64 + } + + testcases := []testcase{{ + name: "https://github.com/ooni/probe/issues/2517", + tkFile: filepath.Join("testdata", "20230706183840.201925_PK_webconnectivity_19f5e0d803cbaea7.jsonc"), + geoInfo: map[string]*model.LocationASN{ + "172.224.19.10": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "172.224.19.5": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "172.224.19.9": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "17.248.248.101": {ASNumber: 714, Organization: "Apple Inc."}, + "2a01:b740:a41:212::8": {ASNumber: 714, Organization: "Apple Inc."}, + "2a01:b740:a41:212::7": {ASNumber: 714, Organization: "Apple Inc."}, + "2a01:b740:a41:213::7": {ASNumber: 714, Organization: "Apple Inc."}, + "172.224.19.3": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "172.224.19.12": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "17.248.248.103": {ASNumber: 714, Organization: "Apple Inc."}, + "17.248.248.119": {ASNumber: 714, Organization: "Apple Inc."}, + "2a01:b740:a41:213::5": {ASNumber: 714, Organization: "Apple Inc."}, + "172.224.19.4": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "172.224.19.6": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "172.224.19.11": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "2a01:b740:a41:212::4": {ASNumber: 714, Organization: "Apple Inc."}, + "172.224.19.7": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "17.248.248.117": {ASNumber: 714, Organization: "Apple Inc."}, + "17.248.248.121": {ASNumber: 714, Organization: "Apple Inc."}, + "2a01:b740:a41:212::5": {ASNumber: 714, Organization: "Apple Inc."}, + "17.248.248.104": {ASNumber: 714, Organization: "Apple Inc."}, + "2a01:b740:a41:213::9": {ASNumber: 714, Organization: "Apple Inc."}, + "172.224.19.14": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "172.224.19.15": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "2a01:b740:a41:212::6": {ASNumber: 714, Organization: "Apple Inc."}, + "172.224.19.17": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "172.224.19.13": {ASNumber: 36183, Organization: "Akamai Technologies, Inc."}, + "17.248.248.105": {ASNumber: 714, Organization: "Apple Inc."}, + "17.248.248.100": {ASNumber: 714, Organization: "Apple Inc."}, + }, + expecteBlockingFlags: AnalysisBlockingFlagDNSBlocking, + }} + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + data := runtimex.Try1(os.ReadFile(tc.tkFile)) + data = runtimex.Try1(hujson.Standardize(data)) + var tk TestKeys + runtimex.Try0(json.Unmarshal(data, &tk)) + log.SetLevel(log.DebugLevel) + tk.analysisClassic(mocks.NewGeoIPASNLookupper(tc.geoInfo), log.Log) + if tc.expecteBlockingFlags != tk.BlockingFlags { + t.Fatal("expected", tc.expecteBlockingFlags, "got", tk.BlockingFlags) + } + }) + } +} diff --git a/internal/experiment/webconnectivitylte/analysisext.go b/internal/experiment/webconnectivitylte/analysisext.go index f23d359cd1..5c9f5941b8 100644 --- a/internal/experiment/webconnectivitylte/analysisext.go +++ b/internal/experiment/webconnectivitylte/analysisext.go @@ -13,14 +13,19 @@ import ( "strings" "github.com/ooni/probe-cli/v3/internal/minipipeline" + "github.com/ooni/probe-cli/v3/internal/model" ) // analysisExtMain computes the extended analysis. // // This function MUTATES the [*TestKeys]. -func analysisExtMain(tk *TestKeys, container *minipipeline.WebObservationsContainer) { +func analysisExtMain( + lookupper model.GeoIPASNLookupper, + tk *TestKeys, + container *minipipeline.WebObservationsContainer, +) { // compute the web analysis - analysis := minipipeline.AnalyzeWebObservationsWithoutLinearAnalysis(container) + analysis := minipipeline.AnalyzeWebObservationsWithoutLinearAnalysis(lookupper, container) // prepare for emitting informational messages var info strings.Builder @@ -234,6 +239,16 @@ func analysisExtExpectedFailures(tk *TestKeys, analysis *minipipeline.WebAnalysi // Also note that these cases ARE NOT MUTUALLY EXCLUSIVE meaning that these conditions // could actually happen simultaneously in a bunch of cases. + // See https://github.com/ooni/probe/issues/2290 for further + // documentation about the issue we're solving here. + // + // It would be tempting to check specifically for NXDOMAIN here, but we + // know it is problematic do that. In fact, on Android the getaddrinfo + // resolver always returns EAI_NODATA on error, regardless of the actual + // error that may have occurred in the Android DNS backend. + // + // See https://github.com/ooni/probe/issues/2029 for more information + // on Android's getaddrinfo behavior. if expected := analysis.DNSLookupExpectedFailure; expected.Len() > 0 { tk.NullNullFlags |= AnalysisFlagNullNullExpectedDNSLookupFailure fmt.Fprintf( @@ -242,6 +257,10 @@ func analysisExtExpectedFailures(tk *TestKeys, analysis *minipipeline.WebAnalysi ) } + // See https://explorer.ooni.org/measurement/20220911T105037Z_webconnectivity_IT_30722_n1_ruzuQ219SmIO9SrT?input=https://doh.centraleu.pi-dns.com/dns-query?dns=q80BAAABAAAAAAAAA3d3dwdleGFtcGxlA2NvbQAAAQAB + // for an example measurement with this behavior. + // + // See also https://github.com/ooni/probe/issues/2299. if expected := analysis.TCPConnectExpectedFailure; expected.Len() > 0 { tk.NullNullFlags |= AnalysisFlagNullNullExpectedTCPConnectFailure fmt.Fprintf( @@ -250,6 +269,7 @@ func analysisExtExpectedFailures(tk *TestKeys, analysis *minipipeline.WebAnalysi ) } + // See https://github.com/ooni/probe/issues/2300. if expected := analysis.TLSHandshakeExpectedFailure; expected.Len() > 0 { tk.NullNullFlags |= AnalysisFlagNullNullExpectedTLSHandshakeFailure fmt.Fprintf( diff --git a/internal/minipipeline/analysis.go b/internal/minipipeline/analysis.go index 5f2fe2caf4..986aef0cd0 100644 --- a/internal/minipipeline/analysis.go +++ b/internal/minipipeline/analysis.go @@ -3,6 +3,7 @@ package minipipeline import ( "sort" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/optional" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -94,13 +95,14 @@ func NewLinearWebAnalysis(input *WebObservationsContainer) (output []*WebObserva // AnalyzeWebObservationsWithoutLinearAnalysis generates a [*WebAnalysis] from a [*WebObservationsContainer] // but avoids calling [NewLinearyAnalysis] to generate a linear analysis. -func AnalyzeWebObservationsWithoutLinearAnalysis(container *WebObservationsContainer) *WebAnalysis { +func AnalyzeWebObservationsWithoutLinearAnalysis( + lookupper model.GeoIPASNLookupper, container *WebObservationsContainer) *WebAnalysis { analysis := &WebAnalysis{ ControlExpectations: container.ControlExpectations, } - analysis.dnsComputeSuccessMetrics(container) - analysis.dnsComputeSuccessMetricsClassic(container) + analysis.dnsComputeSuccessMetrics(lookupper, container) + analysis.dnsComputeSuccessMetricsClassic(lookupper, container) analysis.dnsComputeFailureMetrics(container) analysis.tcpComputeMetrics(container) @@ -113,8 +115,9 @@ func AnalyzeWebObservationsWithoutLinearAnalysis(container *WebObservationsConta // AnalyzeWebObservationsWithLinearAnalysis generates a [*WebAnalysis] from a [*WebObservationsContainer] // and ensures we also use [NewLinearyAnalysis] to generate a linear analysis. -func AnalyzeWebObservationsWithLinearAnalysis(container *WebObservationsContainer) *WebAnalysis { - analysis := AnalyzeWebObservationsWithoutLinearAnalysis(container) +func AnalyzeWebObservationsWithLinearAnalysis( + lookupper model.GeoIPASNLookupper, container *WebObservationsContainer) *WebAnalysis { + analysis := AnalyzeWebObservationsWithoutLinearAnalysis(lookupper, container) analysis.Linear = NewLinearWebAnalysis(container) return analysis } @@ -262,7 +265,8 @@ type WebAnalysis struct { Linear []*WebObservation } -func (wa *WebAnalysis) dnsComputeSuccessMetrics(c *WebObservationsContainer) { +func (wa *WebAnalysis) dnsComputeSuccessMetrics( + lookupper model.GeoIPASNLookupper, c *WebObservationsContainer) { // fill the invalid set var already Set[int64] for _, obs := range c.DNSLookupSuccesses { @@ -304,7 +308,7 @@ func (wa *WebAnalysis) dnsComputeSuccessMetrics(c *WebObservationsContainer) { } // this lookup is good if there is ASN intersection - if DNSDiffFindCommonASNsIntersection(measurement, control).Len() > 0 { + if DNSDiffFindCommonASNsIntersection(lookupper, measurement, control).Len() > 0 { wa.DNSLookupSuccessWithValidAddress.Add(obs.DNSTransactionID.Unwrap()) continue } @@ -332,7 +336,8 @@ func (wa *WebAnalysis) dnsComputeSuccessMetrics(c *WebObservationsContainer) { } } -func (wa *WebAnalysis) dnsComputeSuccessMetricsClassic(c *WebObservationsContainer) { +func (wa *WebAnalysis) dnsComputeSuccessMetricsClassic( + lookupper model.GeoIPASNLookupper, c *WebObservationsContainer) { var already Set[int64] for _, obs := range c.DNSLookupSuccesses { @@ -363,7 +368,7 @@ func (wa *WebAnalysis) dnsComputeSuccessMetricsClassic(c *WebObservationsContain } // this lookup is good if there is ASN intersection - if DNSDiffFindCommonASNsIntersection(measurement, control).Len() > 0 { + if DNSDiffFindCommonASNsIntersection(lookupper, measurement, control).Len() > 0 { wa.DNSLookupSuccessWithValidAddressClassic.Add(obs.DNSTransactionID.Unwrap()) continue } @@ -421,7 +426,18 @@ func (wa *WebAnalysis) dnsComputeFailureMetrics(c *WebObservationsContainer) { continue } - // handle the case where both failed + // Handle the case where both failed. + // + // See https://github.com/ooni/probe/issues/2290 for further + // documentation about the issue we're solving here. + // + // It would be tempting to check specifically for NXDOMAIN here, but we + // know it is problematic do that. In fact, on Android the getaddrinfo + // resolver always returns EAI_NODATA on error, regardless of the actual + // error that may have occurred in the Android DNS backend. + // + // See https://github.com/ooni/probe/issues/2029 for more information + // on Android's getaddrinfo behavior. if obs.DNSLookupFailure.Unwrap() != "" && obs.ControlDNSLookupFailure.Unwrap() != "" { wa.DNSLookupExpectedFailure.Add(obs.DNSTransactionID.Unwrap()) continue @@ -481,7 +497,12 @@ func (wa *WebAnalysis) tcpComputeMetrics(c *WebObservationsContainer) { // handle the case where the control fails if obs.ControlTCPConnectFailure.Unwrap() != "" { - // if also the probe failed mark this failure as expected + // If also the probe failed mark this failure as expected. + // + // See https://explorer.ooni.org/measurement/20220911T105037Z_webconnectivity_IT_30722_n1_ruzuQ219SmIO9SrT?input=https://doh.centraleu.pi-dns.com/dns-query?dns=q80BAAABAAAAAAAAA3d3dwdleGFtcGxlA2NvbQAAAQAB + // for an example measurement with this behavior. + // + // See also https://github.com/ooni/probe/issues/2299. if obs.TCPConnectFailure.Unwrap() != "" { wa.TCPConnectExpectedFailure.Add(obs.EndpointTransactionID.Unwrap()) } @@ -540,7 +561,9 @@ func (wa *WebAnalysis) tlsComputeMetrics(c *WebObservationsContainer) { // handle the case where the control fails if obs.ControlTLSHandshakeFailure.Unwrap() != "" { - // if also the probe failed mark this failure as expected + // If also the probe failed mark this failure as expected. + // + // See https://github.com/ooni/probe/issues/2300. if obs.TLSHandshakeFailure.Unwrap() != "" { wa.TLSHandshakeExpectedFailure.Add(obs.EndpointTransactionID.Unwrap()) } diff --git a/internal/minipipeline/dnsdiff.go b/internal/minipipeline/dnsdiff.go index 7f7c5fc1fe..c0a0ec8cb3 100644 --- a/internal/minipipeline/dnsdiff.go +++ b/internal/minipipeline/dnsdiff.go @@ -1,6 +1,6 @@ package minipipeline -import "github.com/ooni/probe-cli/v3/internal/geoipx" +import "github.com/ooni/probe-cli/v3/internal/model" // DNSDiffFindCommonIPAddressIntersection returns the set of IP addresses that // belong to both the measurement and the control sets. @@ -32,7 +32,8 @@ func DNSDiffFindCommonIPAddressIntersection(measurement, control Set[string]) Se // DNSDiffFindCommonIPAddressIntersection returns the set of ASNs that belong to both the set of ASNs // obtained from the measurement and the one obtained from the control. -func DNSDiffFindCommonASNsIntersection(measurement, control Set[string]) Set[int64] { +func DNSDiffFindCommonASNsIntersection( + lookupper model.GeoIPASNLookupper, measurement, control Set[string]) Set[int64] { const ( inMeasurement = 1 << 0 inControl = 1 << 1 @@ -41,12 +42,12 @@ func DNSDiffFindCommonASNsIntersection(measurement, control Set[string]) Set[int asnmap := make(map[int64]int) for _, ipAddr := range measurement.Keys() { - if asn, _, err := geoipx.LookupASN(ipAddr); err == nil && asn > 0 { + if asn, _, err := lookupper.LookupASN(ipAddr); err == nil && asn > 0 { asnmap[int64(asn)] |= inMeasurement } } for _, ipAddr := range control.Keys() { - if asn, _, err := geoipx.LookupASN(ipAddr); err == nil && asn > 0 { + if asn, _, err := lookupper.LookupASN(ipAddr); err == nil && asn > 0 { asnmap[int64(asn)] |= inControl } } diff --git a/internal/minipipeline/observation.go b/internal/minipipeline/observation.go index c78a839102..32b67d7041 100644 --- a/internal/minipipeline/observation.go +++ b/internal/minipipeline/observation.go @@ -22,15 +22,15 @@ var ErrNoTestKeys = errors.New("minipipeline: no test keys") // XControlRequestFields: in such a case, this function will just avoid using the test helper // (aka control) information for generating flat [*WebObservation]. This function returns an // error if the [*WebMeasurement] TestKeys are empty or Input is not a valid URL. -func IngestWebMeasurement(meas *WebMeasurement) (*WebObservationsContainer, error) { +func IngestWebMeasurement(lookupper model.GeoIPASNLookupper, meas *WebMeasurement) (*WebObservationsContainer, error) { tk := meas.TestKeys.UnwrapOr(nil) if tk == nil { return nil, ErrNoTestKeys } container := NewWebObservationsContainer() - container.IngestDNSLookupEvents(tk.Queries...) - container.IngestTCPConnectEvents(tk.TCPConnect...) + container.IngestDNSLookupEvents(lookupper, tk.Queries...) + container.IngestTCPConnectEvents(lookupper, tk.TCPConnect...) container.IngestTLSHandshakeEvents(tk.TLSHandshakes...) container.IngestHTTPRoundTripEvents(tk.Requests...) @@ -303,9 +303,10 @@ func NewWebObservationsContainer() *WebObservationsContainer { // IngestDNSLookupEvents ingests DNS lookup events from a OONI measurement. You MUST // ingest DNS lookup events before ingesting any other kind of event. -func (c *WebObservationsContainer) IngestDNSLookupEvents(evs ...*model.ArchivalDNSLookupResult) { +func (c *WebObservationsContainer) IngestDNSLookupEvents( + lookupper model.GeoIPASNLookupper, evs ...*model.ArchivalDNSLookupResult) { c.ingestDNSLookupFailures(evs...) - c.ingestDNSLookupSuccesses(evs...) + c.ingestDNSLookupSuccesses(lookupper, evs...) } func (c *WebObservationsContainer) ingestDNSLookupFailures(evs ...*model.ArchivalDNSLookupResult) { @@ -334,7 +335,8 @@ func (c *WebObservationsContainer) ingestDNSLookupFailures(evs ...*model.Archiva } } -func (c *WebObservationsContainer) ingestDNSLookupSuccesses(evs ...*model.ArchivalDNSLookupResult) { +func (c *WebObservationsContainer) ingestDNSLookupSuccesses( + lookupper model.GeoIPASNLookupper, evs ...*model.ArchivalDNSLookupResult) { for _, ev := range evs { // skip all the failed queries if ev.Failure != nil { @@ -356,7 +358,7 @@ func (c *WebObservationsContainer) ingestDNSLookupSuccesses(evs ...*model.Archiv DNSEngine: optional.Some(ev.Engine), DNSResolvedAddrs: optional.Some(addrs), IPAddress: optional.Some(ipAddr), - IPAddressASN: utilsGeoipxLookupASN(ipAddr), + IPAddressASN: utilsGeoipxLookupASN(lookupper, ipAddr), IPAddressBogon: optional.Some(netxlite.IsBogon(ipAddr)), TagDepth: utilsExtractTagDepth(ev.Tags), } @@ -374,14 +376,15 @@ func (c *WebObservationsContainer) ingestDNSLookupSuccesses(evs ...*model.Archiv // IngestTCPConnectEvents ingests TCP connect events from a OONI measurement. You MUST ingest // these events after DNS events and before any other kind of events. -func (c *WebObservationsContainer) IngestTCPConnectEvents(evs ...*model.ArchivalTCPConnectResult) { +func (c *WebObservationsContainer) IngestTCPConnectEvents( + lookupper model.GeoIPASNLookupper, evs ...*model.ArchivalTCPConnectResult) { for _, ev := range evs { // create or fetch a record obs, found := c.knownIPAddresses[ev.IP] if !found { obs = &WebObservation{ IPAddress: optional.Some(ev.IP), - IPAddressASN: utilsGeoipxLookupASN(ev.IP), + IPAddressASN: utilsGeoipxLookupASN(lookupper, ev.IP), IPAddressBogon: optional.Some(netxlite.IsBogon(ev.IP)), } } diff --git a/internal/minipipeline/observation_test.go b/internal/minipipeline/observation_test.go index 93d3e5e74b..fbd6923aa1 100644 --- a/internal/minipipeline/observation_test.go +++ b/internal/minipipeline/observation_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/ooni/probe-cli/v3/internal/geoipx" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/optional" @@ -13,7 +14,10 @@ import ( func TestLoadWebObservations(t *testing.T) { t.Run("we handle the case where the test keys are nil", func(t *testing.T) { meas := &WebMeasurement{ /* empty */ } - container, err := IngestWebMeasurement(meas) + container, err := IngestWebMeasurement( + model.GeoIPASNLookupperFunc(geoipx.LookupASN), + meas, + ) if !errors.Is(err, ErrNoTestKeys) { t.Fatal("expected", ErrNoTestKeys, "got", err) } @@ -41,7 +45,10 @@ func TestLoadWebObservations(t *testing.T) { }), }), } - container, err := IngestWebMeasurement(meas) + container, err := IngestWebMeasurement( + model.GeoIPASNLookupperFunc(geoipx.LookupASN), + meas, + ) if err == nil || !strings.HasSuffix(err.Error(), "invalid control character in URL") { t.Fatal("unexpected err", err) } diff --git a/internal/minipipeline/qa_test.go b/internal/minipipeline/qa_test.go index d9d755420c..627b2dbc44 100644 --- a/internal/minipipeline/qa_test.go +++ b/internal/minipipeline/qa_test.go @@ -6,7 +6,9 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/ooni/probe-cli/v3/internal/geoipx" "github.com/ooni/probe-cli/v3/internal/minipipeline" + "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/must" "github.com/ooni/probe-cli/v3/internal/optional" "github.com/ooni/probe-cli/v3/internal/runtimex" @@ -63,7 +65,10 @@ func testMustRunAllWebTestCases(t *testing.T, topdir string) { must.UnmarshalJSON(expectedClassicAnalysisRaw, &expectedClassicAnalysisData) // load the measurement into the pipeline - gotContainerData, err := minipipeline.IngestWebMeasurement(&measurementData) + gotContainerData, err := minipipeline.IngestWebMeasurement( + model.GeoIPASNLookupperFunc(geoipx.LookupASN), + &measurementData, + ) if err != nil { t.Fatal(err) } @@ -72,10 +77,16 @@ func testMustRunAllWebTestCases(t *testing.T, topdir string) { gotClassicContainerData := minipipeline.ClassicFilter(gotContainerData) // analyze the measurement - gotAnalysisData := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(gotContainerData) + gotAnalysisData := minipipeline.AnalyzeWebObservationsWithLinearAnalysis( + model.GeoIPASNLookupperFunc(geoipx.LookupASN), + gotContainerData, + ) // perform the classic web-connectivity-v0.4-like analysis - gotClassicAnalysisData := minipipeline.AnalyzeWebObservationsWithLinearAnalysis(gotClassicContainerData) + gotClassicAnalysisData := minipipeline.AnalyzeWebObservationsWithLinearAnalysis( + model.GeoIPASNLookupperFunc(geoipx.LookupASN), + gotClassicContainerData, + ) // // Note: if tests fail, you likely need to regenerate the static test diff --git a/internal/minipipeline/utils.go b/internal/minipipeline/utils.go index 87ed880d2c..887fffa45f 100644 --- a/internal/minipipeline/utils.go +++ b/internal/minipipeline/utils.go @@ -4,7 +4,6 @@ import ( "strconv" "strings" - "github.com/ooni/probe-cli/v3/internal/geoipx" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/optional" @@ -17,8 +16,8 @@ func utilsStringPointerToString(failure *string) (out string) { return } -func utilsGeoipxLookupASN(ipAddress string) optional.Value[int64] { - if asn, _, err := geoipx.LookupASN(ipAddress); err == nil && asn > 0 { +func utilsGeoipxLookupASN(lookupper model.GeoIPASNLookupper, ipAddress string) optional.Value[int64] { + if asn, _, err := lookupper.LookupASN(ipAddress); err == nil && asn > 0 { return optional.Some(int64(asn)) } return optional.None[int64]() @@ -111,6 +110,10 @@ func utilsDNSEngineIsDNSOverHTTPS(obs *WebObservation) bool { return obs.DNSEngine.UnwrapOr("") == "doh" } +// utilsTCPConnectFailureSeemsMisconfiguredIPv6 returns whether IPv6 seems to be +// misconfigured for this specific TCP connect attempt. +// +// See https://github.com/ooni/probe/issues/2284 for more info. func utilsTCPConnectFailureSeemsMisconfiguredIPv6(obs *WebObservation) bool { switch obs.TCPConnectFailure.UnwrapOr("") { case netxlite.FailureNetworkUnreachable, netxlite.FailureHostUnreachable: