diff --git a/internal/enginenetx/bridgespolicy.go b/internal/enginenetx/bridgespolicy.go index 23e7cfaf9..0bbc1e9fc 100644 --- a/internal/enginenetx/bridgespolicy.go +++ b/internal/enginenetx/bridgespolicy.go @@ -8,7 +8,6 @@ package enginenetx import ( "context" "math/rand" - "slices" "time" ) @@ -32,66 +31,6 @@ func (p *bridgesPolicyV2) LookupTactics(ctx context.Context, domain, port string return bridgesTacticsForDomain(domain, port) } -// bridgesPolicy is a policy where we use bridges for communicating -// with the OONI backend, i.e., api.ooni.io. -// -// A bridge is an IP address that can route traffic from and to -// the OONI backend and accepts any SNI. -// -// The zero value is invalid; please, init MANDATORY fields. -type bridgesPolicy struct { - // Fallback is the MANDATORY fallback policy. - Fallback httpsDialerPolicy -} - -var _ httpsDialerPolicy = &bridgesPolicy{} - -// LookupTactics implements httpsDialerPolicy. -func (p *bridgesPolicy) LookupTactics(ctx context.Context, domain, port string) <-chan *httpsDialerTactic { - return mixSequentially( - // emit bridges related tactics first which are empty if there are - // no bridges for the givend domain and port - bridgesTacticsForDomain(domain, port), - - // now fallback to get more tactics (typically here the fallback - // uses the DNS and obtains some extra tactics) - // - // we wrap whatever the underlying policy returns us with some - // extra logic for better communicating with test helpers - p.maybeRewriteTestHelpersTactics(p.Fallback.LookupTactics(ctx, domain, port)), - ) -} - -func (p *bridgesPolicy) maybeRewriteTestHelpersTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic { - out := make(chan *httpsDialerTactic) - - go func() { - defer close(out) // tell the parent when we're done - - for tactic := range input { - // When we're not connecting to a TH, pass the policy down the chain unmodified - if !slices.Contains(testHelpersDomains, tactic.VerifyHostname) { - out <- tactic - continue - } - - // This is the case where we're connecting to a test helper. Let's try - // to produce policies hiding the SNI to censoring middleboxes. - for _, sni := range bridgesDomainsInRandomOrder() { - out <- &httpsDialerTactic{ - Address: tactic.Address, - InitialDelay: 0, // set when dialing - Port: tactic.Port, - SNI: sni, - VerifyHostname: tactic.VerifyHostname, - } - } - } - }() - - return out -} - func bridgesTacticsForDomain(domain, port string) <-chan *httpsDialerTactic { out := make(chan *httpsDialerTactic) diff --git a/internal/enginenetx/bridgespolicy_test.go b/internal/enginenetx/bridgespolicy_test.go index 27d2430e1..243ac4828 100644 --- a/internal/enginenetx/bridgespolicy_test.go +++ b/internal/enginenetx/bridgespolicy_test.go @@ -2,11 +2,7 @@ package enginenetx import ( "context" - "errors" "testing" - - "github.com/ooni/probe-cli/v3/internal/mocks" - "github.com/ooni/probe-cli/v3/internal/model" ) func TestBridgesPolicyV2(t *testing.T) { @@ -63,239 +59,3 @@ func TestBridgesPolicyV2(t *testing.T) { } }) } - -func TestBridgesPolicy(t *testing.T) { - t.Run("for domains for which we don't have bridges and DNS failure", func(t *testing.T) { - expected := errors.New("mocked error") - p := &bridgesPolicy{ - Fallback: &dnsPolicy{ - Logger: model.DiscardLogger, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return nil, expected - }, - }, - }, - } - - ctx := context.Background() - tactics := p.LookupTactics(ctx, "www.example.com", "443") - - var count int - for range tactics { - count++ - } - - if count != 0 { - t.Fatal("expected to see zero tactics") - } - }) - - t.Run("for domains for which we don't have bridges and DNS success", func(t *testing.T) { - p := &bridgesPolicy{ - Fallback: &dnsPolicy{ - Logger: model.DiscardLogger, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return []string{"93.184.216.34"}, nil - }, - }, - }, - } - - ctx := context.Background() - tactics := p.LookupTactics(ctx, "www.example.com", "443") - - var count int - for tactic := range tactics { - count++ - - if tactic.Port != "443" { - t.Fatal("the port should always be 443") - } - if tactic.Address != "93.184.216.34" { - t.Fatal("the host should always be 93.184.216.34") - } - - if tactic.InitialDelay != 0 { - t.Fatal("unexpected InitialDelay") - } - - if tactic.SNI != "www.example.com" { - t.Fatal("the SNI field should always be like `www.example.com`") - } - - if tactic.VerifyHostname != "www.example.com" { - t.Fatal("the VerifyHostname field should always be like `www.example.com`") - } - } - - if count != 1 { - t.Fatal("expected to see one tactic") - } - }) - - t.Run("for the api.ooni.io domain with DNS failure", func(t *testing.T) { - expected := errors.New("mocked error") - p := &bridgesPolicy{ - Fallback: &dnsPolicy{ - Logger: model.DiscardLogger, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return nil, expected - }, - }, - }, - } - - ctx := context.Background() - tactics := p.LookupTactics(ctx, "api.ooni.io", "443") - - // since the DNS fails, we should only see tactics generated by bridges - var count int - for tactic := range tactics { - count++ - - if tactic.Port != "443" { - t.Fatal("the port should always be 443") - } - if tactic.Address != "162.55.247.208" { - t.Fatal("the host should always be 162.55.247.208") - } - - if tactic.InitialDelay != 0 { - t.Fatal("unexpected InitialDelay") - } - - if tactic.SNI == "api.ooni.io" { - t.Fatal("we should not see the `api.ooni.io` SNI on the wire") - } - - if tactic.VerifyHostname != "api.ooni.io" { - t.Fatal("the VerifyHostname field should always be like `api.ooni.io`") - } - } - - if count <= 0 { - t.Fatal("expected to see at least one tactic") - } - }) - - t.Run("for the api.ooni.io domain with DNS success", func(t *testing.T) { - p := &bridgesPolicy{ - Fallback: &dnsPolicy{ - Logger: model.DiscardLogger, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return []string{"130.192.91.211"}, nil - }, - }, - }, - } - - ctx := context.Background() - tactics := p.LookupTactics(ctx, "api.ooni.io", "443") - - // since the DNS succeeds we should see bridge tactics mixed with DNS tactics - var ( - bridgesCount int - dnsCount int - overallCount int - ) - const expectedDNSEntryCount = 153 // yikes! - for tactic := range tactics { - overallCount++ - - t.Log(overallCount, tactic) - - if tactic.Port != "443" { - t.Fatal("the port should always be 443") - } - - switch { - case overallCount == expectedDNSEntryCount: - if tactic.Address != "130.192.91.211" { - t.Fatal("the host should be 130.192.91.211 for count ==", expectedDNSEntryCount) - } - - if tactic.SNI != "api.ooni.io" { - t.Fatal("we should see the `api.ooni.io` SNI on the wire for count ==", expectedDNSEntryCount) - } - - dnsCount++ - - default: - if tactic.Address != "162.55.247.208" { - t.Fatal("the host should be 162.55.247.208 for count !=", expectedDNSEntryCount) - } - - if tactic.SNI == "api.ooni.io" { - t.Fatal("we should not see the `api.ooni.io` SNI on the wire for count !=", expectedDNSEntryCount) - } - - bridgesCount++ - } - - if tactic.InitialDelay != 0 { - t.Fatal("unexpected InitialDelay") - } - - if tactic.VerifyHostname != "api.ooni.io" { - t.Fatal("the VerifyHostname field should always be like `api.ooni.io`") - } - } - - if overallCount <= 0 { - t.Fatal("expected to see at least one tactic") - } - if dnsCount != 1 { - t.Fatal("expected to see exactly one DNS based tactic") - } - if bridgesCount <= 0 { - t.Fatal("expected to see at least one bridge tactic") - } - }) - - t.Run("for test helper domains", func(t *testing.T) { - for _, domain := range testHelpersDomains { - t.Run(domain, func(t *testing.T) { - expectedAddrs := []string{"164.92.180.7"} - - p := &bridgesPolicy{ - Fallback: &dnsPolicy{ - Logger: model.DiscardLogger, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return expectedAddrs, nil - }, - }, - }, - } - - ctx := context.Background() - for tactic := range p.LookupTactics(ctx, domain, "443") { - - if tactic.Address != "164.92.180.7" { - t.Fatal("unexpected .Address") - } - - if tactic.InitialDelay != 0 { - t.Fatal("unexpected .InitialDelay") - } - - if tactic.Port != "443" { - t.Fatal("unexpected .Port") - } - - if tactic.SNI == domain { - t.Fatal("unexpected .Domain") - } - - if tactic.VerifyHostname != domain { - t.Fatal("unexpected .VerifyHostname") - } - } - }) - } - }) -} diff --git a/internal/enginenetx/mix.go b/internal/enginenetx/mix.go deleted file mode 100644 index 1cac673ca..000000000 --- a/internal/enginenetx/mix.go +++ /dev/null @@ -1,87 +0,0 @@ -package enginenetx - -import "sync" - -// mixSequentially mixes entries from primary followed by entries from fallback. -// -// This function returns a channel where we emit the edited -// tactics, and which we clone when we're done. -func mixSequentially(primary, fallback <-chan *httpsDialerTactic) <-chan *httpsDialerTactic { - output := make(chan *httpsDialerTactic) - go func() { - defer close(output) - for tx := range primary { - output <- tx - } - for tx := range fallback { - output <- tx - } - }() - return output -} - -// mixDeterministicThenRandomConfig contains config for [mixDeterministicThenRandom]. -type mixDeterministicThenRandomConfig struct { - // C is the channel to mix from. - C <-chan *httpsDialerTactic - - // N is the number of entries to read from at the - // beginning before starting random mixing. - N int -} - -// mixDeterministicThenRandom reads the first N entries from primary, if any, then the first N -// entries from fallback, if any, and then randomly mixes the entries. -func mixDeterministicThenRandom(primary, fallback *mixDeterministicThenRandomConfig) <-chan *httpsDialerTactic { - output := make(chan *httpsDialerTactic) - go func() { - defer close(output) - mixTryEmitN(primary.C, primary.N, output) - mixTryEmitN(fallback.C, fallback.N, output) - for tx := range mixRandomly(primary.C, fallback.C) { - output <- tx - } - }() - return output -} - -func mixTryEmitN(input <-chan *httpsDialerTactic, numToRead int, output chan<- *httpsDialerTactic) { - for idx := 0; idx < numToRead; idx++ { - tactic, good := <-input - if !good { - return - } - output <- tactic - } -} - -func mixRandomly(left, right <-chan *httpsDialerTactic) <-chan *httpsDialerTactic { - output := make(chan *httpsDialerTactic) - go func() { - // read from left - waitg := &sync.WaitGroup{} - waitg.Add(1) - go func() { - defer waitg.Done() - for tx := range left { - output <- tx - } - }() - - // read from right - waitg.Add(1) - go func() { - defer waitg.Done() - for tx := range right { - output <- tx - } - }() - - // close when done - go func() { - waitg.Wait() - close(output) - }() - }() - return output -} diff --git a/internal/enginenetx/mix_test.go b/internal/enginenetx/mix_test.go deleted file mode 100644 index 990373af8..000000000 --- a/internal/enginenetx/mix_test.go +++ /dev/null @@ -1,227 +0,0 @@ -package enginenetx - -import ( - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/ooni/probe-cli/v3/internal/testingx" -) - -func TestMixSequentially(t *testing.T) { - primary := []*httpsDialerTactic{} - fallback := []*httpsDialerTactic{} - - ff := &testingx.FakeFiller{} - ff.Fill(&primary) - ff.Fill(&fallback) - - expect := append([]*httpsDialerTactic{}, primary...) - expect = append(expect, fallback...) - - var output []*httpsDialerTactic - for tx := range mixSequentially(streamTacticsFromSlice(primary), streamTacticsFromSlice(fallback)) { - output = append(output, tx) - } - - if diff := cmp.Diff(expect, output); diff != "" { - t.Fatal(diff) - } -} - -func TestMixDeterministicThenRandom(t *testing.T) { - // define primary data source - primary := []*httpsDialerTactic{{ - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a1.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a2.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a3.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a4.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a5.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a6.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a7.com", - VerifyHostname: "api.ooni.io", - }} - - // define fallback data source - fallback := []*httpsDialerTactic{{ - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b1.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b2.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b3.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b4.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b5.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b6.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b7.com", - VerifyHostname: "api.ooni.io", - }} - - // define the expectations for the beginning of the result - expectBeginning := []*httpsDialerTactic{{ - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a1.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "a2.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b1.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b2.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "130.192.91.211", - InitialDelay: 0, - Port: "443", - SNI: "b3.com", - VerifyHostname: "api.ooni.io", - }} - - // remix - outch := mixDeterministicThenRandom( - &mixDeterministicThenRandomConfig{ - C: streamTacticsFromSlice(primary), - N: 2, - }, - &mixDeterministicThenRandomConfig{ - C: streamTacticsFromSlice(fallback), - N: 3, - }, - ) - var output []*httpsDialerTactic - for tx := range outch { - output = append(output, tx) - } - - // make sure we have the expected number of entries - if len(output) != 14 { - t.Fatal("we need 14 entries") - } - if diff := cmp.Diff(expectBeginning, output[:5]); diff != "" { - t.Fatal(diff) - } - - // make sure each entry is represented - const ( - inprimary = 1 << 0 - infallback - inoutput - ) - mapping := make(map[string]int) - for _, entry := range primary { - mapping[entry.tacticSummaryKey()] |= inprimary - } - for _, entry := range fallback { - mapping[entry.tacticSummaryKey()] |= infallback - } - for _, entry := range output { - mapping[entry.tacticSummaryKey()] |= inoutput - } - for entry, flags := range mapping { - if flags != (inprimary|inoutput) && flags != (infallback|inoutput) { - t.Fatal("unexpected flags", flags, "for entry", entry) - } - } -} - -func TestMixTryEmitNWithClosedChannel(t *testing.T) { - // create an already closed channel - inputch := make(chan *httpsDialerTactic) - close(inputch) - - // create channel for collecting the results - outputch := make(chan *httpsDialerTactic) - - go func() { - // Implementation note: mixTryEmitN does not close the channel - // when done, therefore we need to close it ourselves. - mixTryEmitN(inputch, 10, outputch) - close(outputch) - }() - - // read the output channel - var output []*httpsDialerTactic - for tx := range outputch { - output = append(output, tx) - } - - // make sure we didn't read anything - if len(output) != 0 { - t.Fatal("expected zero entries") - } -} diff --git a/internal/enginenetx/statspolicy.go b/internal/enginenetx/statspolicy.go index 74f42f18e..cf8e41231 100644 --- a/internal/enginenetx/statspolicy.go +++ b/internal/enginenetx/statspolicy.go @@ -12,34 +12,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/runtimex" ) -// statsPolicy is a policy that schedules tactics already known -// to work based on statistics and defers to a fallback policy -// once it has generated all the tactics known to work. -// -// The zero value of this struct is invalid; please, make sure you -// fill all the fields marked as MANDATORY. -type statsPolicy struct { - // Fallback is the MANDATORY fallback policy. - Fallback httpsDialerPolicy - - // Stats is the MANDATORY stats manager. - Stats *statsManager -} - -var _ httpsDialerPolicy = &statsPolicy{} - -// LookupTactics implements HTTPSDialerPolicy. -func (p *statsPolicy) LookupTactics(ctx context.Context, domain string, port string) <-chan *httpsDialerTactic { - // avoid emitting nil tactics and duplicate tactics - return filterOnlyKeepUniqueTactics(filterOutNilTactics(mixSequentially( - // give priority to what we know from stats - streamTacticsFromSlice(statsPolicyFilterStatsTactics(p.Stats.LookupTactics(domain, port))), - - // fallback to the secondary policy - p.Fallback.LookupTactics(ctx, domain, port), - ))) -} - // statsPolicyV2 is a policy that schedules tactics already known // to work based on the previously collected stats. // diff --git a/internal/enginenetx/statspolicy_test.go b/internal/enginenetx/statspolicy_test.go index 19ea48ddb..264a012fc 100644 --- a/internal/enginenetx/statspolicy_test.go +++ b/internal/enginenetx/statspolicy_test.go @@ -9,9 +9,7 @@ import ( "github.com/apex/log" "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/kvstore" - "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/netemx" - "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -197,293 +195,6 @@ func TestStatsPolicyV2(t *testing.T) { }) } -func TestStatsPolicyWorkingAsIntended(t *testing.T) { - // prepare the content of the stats - twentyMinutesAgo := time.Now().Add(-20 * time.Minute) - - const bridgeAddress = netemx.AddressApiOONIIo - - expectTacticsStats := []*statsTactic{{ - CountStarted: 5, - CountTCPConnectError: 0, - CountTCPConnectInterrupt: 0, - CountTLSHandshakeError: 0, - CountTLSHandshakeInterrupt: 0, - CountTLSVerificationError: 0, - CountSuccess: 5, // this one always succeeds, so it should be there - HistoTCPConnectError: map[string]int64{}, - HistoTLSHandshakeError: map[string]int64{}, - HistoTLSVerificationError: map[string]int64{}, - LastUpdated: twentyMinutesAgo, - Tactic: &httpsDialerTactic{ - Address: bridgeAddress, - InitialDelay: 0, - Port: "443", - SNI: "www.repubblica.it", - VerifyHostname: "api.ooni.io", - }, - }, { - CountStarted: 3, - CountTCPConnectError: 0, - CountTCPConnectInterrupt: 0, - CountTLSHandshakeError: 1, - CountTLSHandshakeInterrupt: 0, - CountTLSVerificationError: 0, - CountSuccess: 2, // this one sometimes succeded so it should be added - HistoTCPConnectError: map[string]int64{}, - HistoTLSHandshakeError: map[string]int64{}, - HistoTLSVerificationError: map[string]int64{}, - LastUpdated: twentyMinutesAgo, - Tactic: &httpsDialerTactic{ - Address: bridgeAddress, - InitialDelay: 0, - Port: "443", - SNI: "www.kernel.org", - VerifyHostname: "api.ooni.io", - }, - }, { - CountStarted: 3, - CountTCPConnectError: 0, - CountTCPConnectInterrupt: 0, - CountTLSHandshakeError: 3, // this one always failed, so should not be added - CountTLSHandshakeInterrupt: 0, - CountTLSVerificationError: 0, - CountSuccess: 0, - HistoTCPConnectError: map[string]int64{}, - HistoTLSHandshakeError: map[string]int64{}, - HistoTLSVerificationError: map[string]int64{}, - LastUpdated: twentyMinutesAgo, - Tactic: &httpsDialerTactic{ - Address: bridgeAddress, - InitialDelay: 0, - Port: "443", - SNI: "theconversation.com", - VerifyHostname: "api.ooni.io", - }, - }, { - CountStarted: 4, - CountTCPConnectError: 0, - CountTCPConnectInterrupt: 0, - CountTLSHandshakeError: 0, - CountTLSHandshakeInterrupt: 0, - CountTLSVerificationError: 0, - CountSuccess: 4, - HistoTCPConnectError: map[string]int64{}, - HistoTLSHandshakeError: map[string]int64{}, - HistoTLSVerificationError: map[string]int64{}, - LastUpdated: twentyMinutesAgo, - Tactic: nil, // the nil policy here should cause this entry to be filtered out - }, { - CountStarted: 0, - CountTCPConnectError: 0, - CountTCPConnectInterrupt: 0, - CountTLSHandshakeError: 0, - CountTLSHandshakeInterrupt: 0, - CountTLSVerificationError: 0, - CountSuccess: 0, - HistoTCPConnectError: map[string]int64{}, - HistoTLSHandshakeError: map[string]int64{}, - HistoTLSVerificationError: map[string]int64{}, - LastUpdated: time.Time{}, // the zero time should exclude this one - Tactic: &httpsDialerTactic{ - Address: bridgeAddress, - InitialDelay: 0, - Port: "443", - SNI: "ilpost.it", - VerifyHostname: "api.ooni.io", - }, - }} - - // createStatsManager creates a stats manager given some baseline stats - createStatsManager := func(domainEndpoint string, tactics ...*statsTactic) *statsManager { - container := &statsContainer{ - DomainEndpoints: map[string]*statsDomainEndpoint{ - domainEndpoint: { - Tactics: map[string]*statsTactic{}, - }, - }, - Version: statsContainerVersion, - } - - for _, tx := range tactics { - if tx.Tactic != nil { - container.DomainEndpoints[domainEndpoint].Tactics[tx.Tactic.tacticSummaryKey()] = tx - } - } - - kvStore := &kvstore.Memory{} - if err := kvStore.Set(statsKey, runtimex.Try1(json.Marshal(container))); err != nil { - t.Fatal(err) - } - - const trimInterval = 30 * time.Second - return newStatsManager(kvStore, log.Log, trimInterval) - } - - t.Run("when we have unique statistics", func(t *testing.T) { - // create stats manager - stats := createStatsManager("api.ooni.io:443", expectTacticsStats...) - defer stats.Close() - - // create the composed policy - policy := &statsPolicy{ - Fallback: &dnsPolicy{ - Logger: log.Log, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - switch domain { - case "api.ooni.io": - return []string{bridgeAddress}, nil - default: - return nil, netxlite.ErrOODNSNoSuchHost - } - }, - }, - }, - Stats: stats, - } - - // obtain the tactics from the saved stats - var tactics []*httpsDialerTactic - for entry := range policy.LookupTactics(context.Background(), "api.ooni.io", "443") { - tactics = append(tactics, entry) - } - - // compute the list of results we expect to see from the stats data - var expect []*httpsDialerTactic - for _, entry := range expectTacticsStats { - if entry.CountSuccess <= 0 || entry.Tactic == nil { - continue // we SHOULD NOT include entries that systematically failed - } - t := entry.Tactic.Clone() - t.InitialDelay = 0 - expect = append(expect, t) - } - - // extend the expected list to include DNS results - expect = append(expect, &httpsDialerTactic{ - Address: bridgeAddress, - InitialDelay: 0, - Port: "443", - SNI: "api.ooni.io", - VerifyHostname: "api.ooni.io", - }) - - // perform the actual comparison - if diff := cmp.Diff(expect, tactics); diff != "" { - t.Fatal(diff) - } - }) - - t.Run("when we have duplicates", func(t *testing.T) { - // add each entry twice to create obvious duplicates - statsWithDupes := []*statsTactic{} - for _, entry := range expectTacticsStats { - statsWithDupes = append(statsWithDupes, entry.Clone()) - statsWithDupes = append(statsWithDupes, entry.Clone()) - } - - // create stats manager - stats := createStatsManager("api.ooni.io:443", statsWithDupes...) - defer stats.Close() - - // create the composed policy - policy := &statsPolicy{ - Fallback: &dnsPolicy{ - Logger: log.Log, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - switch domain { - case "api.ooni.io": - // Twice so we try to cause duplicate entries also with the DNS policy - return []string{bridgeAddress, bridgeAddress}, nil - default: - return nil, netxlite.ErrOODNSNoSuchHost - } - }, - }, - }, - Stats: stats, - } - - // obtain the tactics from the saved stats - var tactics []*httpsDialerTactic - for entry := range policy.LookupTactics(context.Background(), "api.ooni.io", "443") { - tactics = append(tactics, entry) - } - - // compute the list of results we expect to see from the stats data - var expect []*httpsDialerTactic - for _, entry := range expectTacticsStats { - if entry.CountSuccess <= 0 || entry.Tactic == nil { - continue // we SHOULD NOT include entries that systematically failed - } - t := entry.Tactic.Clone() - t.InitialDelay = 0 - expect = append(expect, t) - } - - // extend the expected list to include DNS results - expect = append(expect, &httpsDialerTactic{ - Address: bridgeAddress, - InitialDelay: 0, - Port: "443", - SNI: "api.ooni.io", - VerifyHostname: "api.ooni.io", - }) - - // perform the actual comparison - if diff := cmp.Diff(expect, tactics); diff != "" { - t.Fatal(diff) - } - }) - - t.Run("we avoid manipulating nil tactics", func(t *testing.T) { - // create stats manager - stats := createStatsManager("api.ooni.io:443", expectTacticsStats...) - defer stats.Close() - - // create the composed policy - policy := &statsPolicy{ - Fallback: &mocksPolicy{ - MockLookupTactics: func(ctx context.Context, domain, port string) <-chan *httpsDialerTactic { - out := make(chan *httpsDialerTactic) - go func() { - defer close(out) - - // explicitly send nil on the channel - out <- nil - }() - return out - }, - }, - Stats: stats, - } - - // obtain the tactics from the saved stats - var tactics []*httpsDialerTactic - for entry := range policy.LookupTactics(context.Background(), "api.ooni.io", "443") { - tactics = append(tactics, entry) - } - - // compute the list of results we expect to see from the stats data - var expect []*httpsDialerTactic - for _, entry := range expectTacticsStats { - if entry.CountSuccess <= 0 || entry.Tactic == nil { - continue // we SHOULD NOT include entries that systematically failed - } - t := entry.Tactic.Clone() - t.InitialDelay = 0 - expect = append(expect, t) - } - - // perform the actual comparison - if diff := cmp.Diff(expect, tactics); diff != "" { - t.Fatal(diff) - } - }) -} - func TestStatsPolicyFilterStatsTactics(t *testing.T) { t.Run("we do nothing when good is false", func(t *testing.T) { tactics := statsPolicyFilterStatsTactics(nil, false) diff --git a/internal/enginenetx/userpolicy.go b/internal/enginenetx/userpolicy.go index b782b8313..e4e664649 100644 --- a/internal/enginenetx/userpolicy.go +++ b/internal/enginenetx/userpolicy.go @@ -95,61 +95,12 @@ func (ldp *userPolicyV2) LookupTactics(ctx context.Context, domain string, port return out } -// userPolicy is an [httpsDialerPolicy] incorporating verbatim -// a user policy loaded from the engine's key-value store. -// -// This policy is very useful for exploration and experimentation. -type userPolicy struct { - // Fallback is the fallback policy in case the user one does not - // contain a rule for a specific domain. - Fallback httpsDialerPolicy - - // Root is the root of the user policy loaded from disk. - Root *userPolicyRoot -} - // userPolicyKey is the kvstore key used to retrieve the user policy. const userPolicyKey = "bridges.conf" // errUserPolicyWrongVersion means that the user policy document has the wrong version number. var errUserPolicyWrongVersion = errors.New("wrong user policy version") -// newUserPolicy attempts to constructs a user policy using a given fallback -// policy and either returns a good policy or an error. The typical error case is the one -// in which there's no httpsDialerUserPolicyKey in the key-value store. -func newUserPolicy( - kvStore model.KeyValueStore, fallback httpsDialerPolicy) (*userPolicy, error) { - // attempt to read the user policy bytes from the kvstore - data, err := kvStore.Get(userPolicyKey) - if err != nil { - return nil, err - } - - // attempt to parse the user policy using human-readable JSON - var root userPolicyRoot - if err := hujsonx.Unmarshal(data, &root); err != nil { - return nil, err - } - - // make sure the version is OK - if root.Version != userPolicyVersion { - err := fmt.Errorf( - "%s: %w: expected=%d got=%d", - userPolicyKey, - errUserPolicyWrongVersion, - userPolicyVersion, - root.Version, - ) - return nil, err - } - - out := &userPolicy{ - Fallback: fallback, - Root: &root, - } - return out, nil -} - // userPolicyVersion is the current version of the user policy file. const userPolicyVersion = 3 @@ -162,36 +113,6 @@ type userPolicyRoot struct { Version int } -var _ httpsDialerPolicy = &userPolicy{} - -// LookupTactics implements httpsDialerPolicy. -func (ldp *userPolicy) LookupTactics( - ctx context.Context, domain string, port string) <-chan *httpsDialerTactic { - // check whether an entry exists in the user-provided map, which MAY be nil - // if/when the user has chosen their policy to be as such - tactics, found := ldp.Root.DomainEndpoints[net.JoinHostPort(domain, port)] - if !found { - return ldp.Fallback.LookupTactics(ctx, domain, port) - } - - // note that we also need to fallback when the tactics contains an empty list - // or a list that only contains nil entries - tactics = userPolicyRemoveNilEntries(tactics) - if len(tactics) <= 0 { - return ldp.Fallback.LookupTactics(ctx, domain, port) - } - - // emit the results, which may possibly be empty - out := make(chan *httpsDialerTactic) - go func() { - defer close(out) // let the caller know we're done - for _, tactic := range tactics { - out <- tactic - } - }() - return out -} - func userPolicyRemoveNilEntries(input []*httpsDialerTactic) (output []*httpsDialerTactic) { for _, entry := range input { if entry != nil { diff --git a/internal/enginenetx/userpolicy_test.go b/internal/enginenetx/userpolicy_test.go index 2f4215e9c..20f3861e5 100644 --- a/internal/enginenetx/userpolicy_test.go +++ b/internal/enginenetx/userpolicy_test.go @@ -6,10 +6,8 @@ import ( "testing" "time" - "github.com/apex/log" "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/kvstore" - "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -284,303 +282,3 @@ func TestUserPolicyV2(t *testing.T) { }) }) } - -func TestUserPolicy(t *testing.T) { - t.Run("newUserPolicy", func(t *testing.T) { - // testcase is a test case implemented by this function - type testcase struct { - // name is the test case name - name string - - // key is the key to use for settings the input inside the kvstore - key string - - // input contains the serialized input bytes - input []byte - - // expectErr contains the expected error string or the empty string on success - expectErr string - - // expectRoot contains the expected policy we loaded or nil - expectedPolicy *userPolicy - } - - fallback := &dnsPolicy{} - - cases := []testcase{{ - name: "when there is no key in the kvstore", - key: "", - input: []byte{}, - expectErr: "no such key", - expectedPolicy: nil, - }, { - name: "with nil input", - key: userPolicyKey, - input: nil, - expectErr: "hujson: line 1, column 1: parsing value: unexpected EOF", - expectedPolicy: nil, - }, { - name: "with invalid serialized JSON", - key: userPolicyKey, - input: []byte(`{`), - expectErr: "hujson: line 1, column 2: parsing value: unexpected EOF", - expectedPolicy: nil, - }, { - name: "with empty JSON", - key: userPolicyKey, - input: []byte(`{}`), - expectErr: "bridges.conf: wrong user policy version: expected=3 got=0", - expectedPolicy: nil, - }, { - name: "with real serialized policy", - key: userPolicyKey, - input: (func() []byte { - return runtimex.Try1(json.Marshal(&userPolicyRoot{ - DomainEndpoints: map[string][]*httpsDialerTactic{ - - // Please, note how the input includes explicitly nil entries - // with the purpose of making sure the code can handle them - "api.ooni.io:443": {{ - Address: "162.55.247.208", - InitialDelay: 0, - Port: "443", - SNI: "api.ooni.io", - VerifyHostname: "api.ooni.io", - }, nil, { - Address: "46.101.82.151", - InitialDelay: 300 * time.Millisecond, - Port: "443", - SNI: "api.ooni.io", - VerifyHostname: "api.ooni.io", - }, { - Address: "2a03:b0c0:1:d0::ec4:9001", - InitialDelay: 600 * time.Millisecond, - Port: "443", - SNI: "api.ooni.io", - VerifyHostname: "api.ooni.io", - }, nil, { - Address: "46.101.82.151", - InitialDelay: 3000 * time.Millisecond, - Port: "443", - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "2a03:b0c0:1:d0::ec4:9001", - InitialDelay: 3300 * time.Millisecond, - Port: "443", - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }, nil}, - // - - }, - Version: userPolicyVersion, - })) - })(), - expectErr: "", - expectedPolicy: &userPolicy{ - Fallback: fallback, - Root: &userPolicyRoot{ - DomainEndpoints: map[string][]*httpsDialerTactic{ - "api.ooni.io:443": {{ - Address: "162.55.247.208", - InitialDelay: 0, - Port: "443", - SNI: "api.ooni.io", - VerifyHostname: "api.ooni.io", - }, nil, { - Address: "46.101.82.151", - InitialDelay: 300 * time.Millisecond, - Port: "443", - SNI: "api.ooni.io", - VerifyHostname: "api.ooni.io", - }, { - Address: "2a03:b0c0:1:d0::ec4:9001", - InitialDelay: 600 * time.Millisecond, - Port: "443", - SNI: "api.ooni.io", - VerifyHostname: "api.ooni.io", - }, nil, { - Address: "46.101.82.151", - InitialDelay: 3000 * time.Millisecond, - Port: "443", - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }, { - Address: "2a03:b0c0:1:d0::ec4:9001", - InitialDelay: 3300 * time.Millisecond, - Port: "443", - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - }, nil}, - }, - Version: userPolicyVersion, - }, - }, - }} - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - kvStore := &kvstore.Memory{} - runtimex.Try0(kvStore.Set(tc.key, tc.input)) - - policy, err := newUserPolicy(kvStore, fallback) - - switch { - case err != nil && tc.expectErr == "": - t.Fatal("expected", tc.expectErr, "got", err) - - case err == nil && tc.expectErr != "": - t.Fatal("expected", tc.expectErr, "got", err) - - case err != nil && tc.expectErr != "": - if diff := cmp.Diff(tc.expectErr, err.Error()); diff != "" { - t.Fatal(diff) - } - - case err == nil && tc.expectErr == "": - // all good - } - - if diff := cmp.Diff(tc.expectedPolicy, policy); diff != "" { - t.Fatal(diff) - } - }) - } - }) - - t.Run("LookupTactics", func(t *testing.T) { - expectedTactic := &httpsDialerTactic{ - Address: "162.55.247.208", - InitialDelay: 0, - Port: "443", - SNI: "www.example.com", - VerifyHostname: "api.ooni.io", - } - userPolicyRoot := &userPolicyRoot{ - DomainEndpoints: map[string][]*httpsDialerTactic{ - // Note that here we're adding explicitly nil entries - // to make sure that the code correctly handles 'em - "api.ooni.io:443": { - nil, - expectedTactic, - nil, - }, - - // We add additional entries to make sure that in those - // cases we are going to fallback as they're basically empty - // and so non-actionable for us. - "api.ooni.xyz:443": nil, - "api.ooni.org:443": {}, - "api.ooni.com:443": {nil, nil, nil}, - }, - Version: userPolicyVersion, - } - kvStore := &kvstore.Memory{} - rawUserPolicyRoot := runtimex.Try1(json.Marshal(userPolicyRoot)) - if err := kvStore.Set(userPolicyKey, rawUserPolicyRoot); err != nil { - t.Fatal(err) - } - - t.Run("with user policy", func(t *testing.T) { - ctx := context.Background() - - policy, err := newUserPolicy(kvStore, nil /* explictly to crash if used */) - if err != nil { - t.Fatal(err) - } - - tactics := policy.LookupTactics(ctx, "api.ooni.io", "443") - got := []*httpsDialerTactic{} - for tactic := range tactics { - t.Logf("%+v", tactic) - got = append(got, tactic) - } - - expect := []*httpsDialerTactic{expectedTactic} - - if diff := cmp.Diff(expect, got); diff != "" { - t.Fatal(diff) - } - }) - - t.Run("we fallback if there is no entry in the user policy", func(t *testing.T) { - ctx := context.Background() - - fallback := &dnsPolicy{ - Logger: log.Log, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return []string{"93.184.216.34"}, nil - }, - }, - } - - policy, err := newUserPolicy(kvStore, fallback) - if err != nil { - t.Fatal(err) - } - - tactics := policy.LookupTactics(ctx, "www.example.com", "443") - got := []*httpsDialerTactic{} - for tactic := range tactics { - t.Logf("%+v", tactic) - got = append(got, tactic) - } - - expect := []*httpsDialerTactic{{ - Address: "93.184.216.34", - InitialDelay: 0, - Port: "443", - SNI: "www.example.com", - VerifyHostname: "www.example.com", - }} - - if diff := cmp.Diff(expect, got); diff != "" { - t.Fatal(diff) - } - }) - - t.Run("we fallback if the entry in the user policy is ~empty", func(t *testing.T) { - ctx := context.Background() - - fallback := &dnsPolicy{ - Logger: log.Log, - Resolver: &mocks.Resolver{ - MockLookupHost: func(ctx context.Context, domain string) ([]string, error) { - return []string{"93.184.216.34"}, nil - }, - }, - } - - policy, err := newUserPolicy(kvStore, fallback) - if err != nil { - t.Fatal(err) - } - - // these cases are specially constructed to be empty/invalid user policies - for _, domain := range []string{"api.ooni.xyz", "api.ooni.org", "api.ooni.com"} { - t.Run(domain, func(t *testing.T) { - tactics := policy.LookupTactics(ctx, domain, "443") - got := []*httpsDialerTactic{} - for tactic := range tactics { - t.Logf("%+v", tactic) - got = append(got, tactic) - } - - expect := []*httpsDialerTactic{{ - Address: "93.184.216.34", - InitialDelay: 0, - Port: "443", - SNI: domain, - VerifyHostname: domain, - }} - - if diff := cmp.Diff(expect, got); diff != "" { - t.Fatal(diff) - } - }) - } - }) - }) -}