diff --git a/go.mod b/go.mod index e88409c52..1608d4868 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,7 @@ require ( github.com/miekg/dns v1.1.59 github.com/mitchellh/go-wordwrap v1.0.1 github.com/montanaflynn/stats v0.7.1 - github.com/ooni/minivpn v0.0.6 + github.com/ooni/minivpn v0.0.7 github.com/ooni/netem v0.0.0-20240208095707-608dcbcd82b8 github.com/ooni/oocrypto v0.6.2 github.com/ooni/oohttp v0.7.3 diff --git a/go.sum b/go.sum index 75d055462..526953374 100644 --- a/go.sum +++ b/go.sum @@ -354,8 +354,8 @@ github.com/onsi/ginkgo/v2 v2.17.3/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/ github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.33.0 h1:snPCflnZrpMsy94p4lXVEkHo12lmPnc3vY5XBbreexE= github.com/onsi/gomega v1.33.0/go.mod h1:+925n5YtiFsLzzafLUHzVMBpvvRAzrydIBiSIxjX3wY= -github.com/ooni/minivpn v0.0.6 h1:pGTsYRtofEupMrJL28f1IXO1LJslSI7dEHxSadNgGik= -github.com/ooni/minivpn v0.0.6/go.mod h1:0KNwmK2Wg9lDbk936XjtxvCq4tPNbK4C3IJvyLwIMrE= +github.com/ooni/minivpn v0.0.7 h1:fRL6lOivKM+Q23HcN/FFiBftbKTAtz7U8r6cOypBAeM= +github.com/ooni/minivpn v0.0.7/go.mod h1:0KNwmK2Wg9lDbk936XjtxvCq4tPNbK4C3IJvyLwIMrE= github.com/ooni/netem v0.0.0-20240208095707-608dcbcd82b8 h1:kJ2wn19lIP/y9ng85BbFRdWKHK6Er116Bbt5uhqHVD4= github.com/ooni/netem v0.0.0-20240208095707-608dcbcd82b8/go.mod h1:b/wAvTR5n92Vk2b0SBmuMU0xO4ZGVrsXtU7zjTby7vw= github.com/ooni/oocrypto v0.6.2 h1:gAg24bVP03PNsOkMYGxllxmvlKlBOvyHmFAsdBlFJag= diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 17faa139c..f49396877 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -17,7 +17,7 @@ import ( const ( testName = "openvpn" - testVersion = "0.1.5" + testVersion = "0.1.6" openVPNProtocol = "openvpn" ) diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index 73467ddc5..b7d6a4b69 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -41,7 +41,7 @@ func TestNewExperimentMeasurer(t *testing.T) { if m.ExperimentName() != "openvpn" { t.Fatal("invalid ExperimentName") } - if m.ExperimentVersion() != "0.1.5" { + if m.ExperimentVersion() != "0.1.6" { t.Fatal("invalid ExperimentVersion") } } diff --git a/internal/experiment/openvpn/richerinput.go b/internal/experiment/openvpn/richerinput.go index 1439add60..ed2ce74b8 100644 --- a/internal/experiment/openvpn/richerinput.go +++ b/internal/experiment/openvpn/richerinput.go @@ -2,18 +2,12 @@ package openvpn import ( "context" - "fmt" "github.com/ooni/probe-cli/v3/internal/experimentconfig" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/probe-cli/v3/internal/reflectx" "github.com/ooni/probe-cli/v3/internal/targetloading" ) -// defaultProvider is the provider we will request from API in case we got no provider set -// in the CLI options. -var defaultProvider = "riseupvpn" - // providerAuthentication is a map so that we know which kind of credentials we // need to fill in the openvpn options for each known provider. var providerAuthentication = map[string]AuthMethod{ @@ -83,24 +77,15 @@ type targetLoader struct { } // Load implements model.ExperimentTargetLoader. +// Returning an empty ExperimentTarget slice here is equivalent to not +// passing any input to the experiment; in this case the `openvpn` experiment +// just does not probe any endpoint (no-op). func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { - // If inputs and files are all empty and there are no options, let's use the backend - if len(tl.loader.StaticInputs) <= 0 && len(tl.loader.SourceFiles) <= 0 && - reflectx.StructOrStructPtrIsZero(tl.options) { - targets, err := tl.loadFromBackend(ctx) - if err == nil { - return targets, nil - } - } - - tl.loader.Logger.Warnf("Error fetching OpenVPN targets from backend") - - // Otherwise, attempt to load the static inputs from CLI and files + // First, attempt to load the static inputs from CLI and files inputs, err := targetloading.LoadStatic(tl.loader) - - // Handle the case where we couldn't load from CLI or files: + // Handle the case where we couldn't load from CLI or files (fallthru) if err != nil { - return nil, err + tl.loader.Logger.Warnf("Error loading OpenVPN targets from cli") } // Build the list of targets that we should measure. @@ -115,69 +100,36 @@ func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, err return targets, nil } - // Return the hardcoded endpoints. - return tl.loadFromDefaultEndpoints() -} - -func (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) { - tl.loader.Logger.Warnf("Using default OpenVPN endpoints") - targets := []model.ExperimentTarget{} - if udp, err := defaultOONIOpenVPNTargetUDP(); err == nil { - targets = append(targets, - &Target{ - Config: pickFromDefaultOONIOpenVPNConfig(), - URL: udp, - }) + // As a fallback (no backend, no files, no input from cli) + // return the hardcoded endpoints. + targets, err = tl.loadFromDefaultEndpoints() + if err != nil { + tl.loader.Logger.Warnf("Error loading default endpoints: %v", err) + return targets, nil } - if tcp, err := defaultOONIOpenVPNTargetTCP(); err == nil { - targets = append(targets, - &Target{ - Config: pickFromDefaultOONIOpenVPNConfig(), - URL: tcp, - }) + if len(targets) == 0 { + tl.loader.Logger.Warnf("No targets loaded from default endpoints") } return targets, nil } -func (tl *targetLoader) loadFromBackend(ctx context.Context) ([]model.ExperimentTarget, error) { - if tl.options.Provider == "" { - tl.options.Provider = defaultProvider - } - - targets := make([]model.ExperimentTarget, 0) - provider := tl.options.Provider +func (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) { + targets := []model.ExperimentTarget{} - apiConfig, err := tl.session.FetchOpenVPNConfig(ctx, provider, tl.session.ProbeCC()) + addrs, err := resolveOONIAddresses(tl.session.Logger()) if err != nil { - tl.session.Logger().Warnf("Cannot fetch openvpn config: %v", err) - return nil, err - } - - auth, ok := providerAuthentication[provider] - if !ok { - return nil, fmt.Errorf("%w: unknown authentication for provider %s", targetloading.ErrInvalidInput, provider) + return targets, err } - for _, input := range apiConfig.Inputs { - config := &Config{ - // TODO(ainghazal): Auth and Cipher are hardcoded for now. - // Backend should provide them as richer input; and if empty we can use these as defaults. - Auth: "SHA512", - Cipher: "AES-256-GCM", + tl.loader.Logger.Warnf("Picking from default OpenVPN endpoints") + if inputs, err := pickOONIOpenVPNTargets(addrs); err == nil { + for _, url := range inputs { + targets = append(targets, + &Target{ + Config: pickFromDefaultOONIOpenVPNConfig(), + URL: url, + }) } - switch auth { - case AuthCertificate: - config.SafeCA = apiConfig.Config.CA - config.SafeCert = apiConfig.Config.Cert - config.SafeKey = apiConfig.Config.Key - case AuthUserPass: - // TODO(ainghazal): implement (surfshark, etc) - } - targets = append(targets, &Target{ - URL: input, - Config: config, - }) } - return targets, nil } diff --git a/internal/experiment/openvpn/richerinput_test.go b/internal/experiment/openvpn/richerinput_test.go index 2f155ffc9..44e0c64b7 100644 --- a/internal/experiment/openvpn/richerinput_test.go +++ b/internal/experiment/openvpn/richerinput_test.go @@ -3,9 +3,7 @@ package openvpn import ( "context" "errors" - "fmt" "testing" - "time" "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/mocks" @@ -167,46 +165,3 @@ func TestTargetLoaderLoad(t *testing.T) { }) } } - -func TestTargetLoaderLoadFromBackend(t *testing.T) { - loader := &targetloading.Loader{ - ExperimentName: "openvpn", - InputPolicy: model.InputOrQueryBackend, - Logger: model.DiscardLogger, - Session: &mocks.Session{}, - } - sess := &mocks.Session{} - sess.MockFetchOpenVPNConfig = func(context.Context, string, string) (*model.OOAPIVPNProviderConfig, error) { - return &model.OOAPIVPNProviderConfig{ - Provider: "riseupvpn", - Config: &model.OOAPIVPNConfig{}, - Inputs: []string{ - "openvpn://target0", - "openvpn://target1", - }, - DateUpdated: time.Now(), - }, nil - } - sess.MockProbeCC = func() string { - return "IT" - } - tl := &targetLoader{ - loader: loader, - options: &Config{}, - session: sess, - } - targets, err := tl.Load(context.Background()) - if err != nil { - t.Fatal("expected no error") - } - fmt.Println("targets", targets) - if len(targets) != 2 { - t.Fatal("expected 2 targets") - } - if targets[0].String() != "openvpn://target0" { - t.Fatal("expected openvpn://target0") - } - if targets[1].String() != "openvpn://target1" { - t.Fatal("expected openvpn://target1") - } -} diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go index 6a85b48ca..11522fa2d 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -1,49 +1,169 @@ package openvpn import ( + "context" "fmt" "math/rand" - "net" + "slices" + "time" + + "github.com/ooni/probe-cli/v3/internal/legacy/netx" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) -const defaultOpenVPNEndpoint = "openvpn-server1.ooni.io" - -// this is a safety toggle: it's on purpose that the experiment will receive no -// input if the resolution fails. This also implies that we have no way of knowing if this -// target has been blocked at the level of DNS. -// TODO(ain,mehul): we might want to try resolving with other techniques (DoT etc), -// and perhaps also transform DNS failure into a specific failure of the experiment, not -// a skip. -// TODO(ain): update the openvpn spec to reflect the CURRENT state of delivering the targets. -func resolveTarget(domain string) (string, error) { - ips, err := net.LookupIP(domain) - if err != nil { - return "", err +// defaultOONIEndpoints is the array of hostnames that will return valid +// endpoints to be probed. Do note that this is a workaround for the lack +// of a backend service; if you maintain this experiment in the future please +// feel free to remove this workaround after the probe-services for distributing +// endpoints has been deployed to production. +var defaultOONIEndpoints = []string{ + "a.composer-presenter.com", + "a.goodyear2dumpster.com", +} + +// maxDefaultOONIAddresses is how many IPs to use from the +// set of resolved IPs. +var maxDefaultOONIAddresses = 3 + +// sampleN takes max n elements sampled ramdonly from the array a. +func sampleN(a []string, n int) []string { + if n > len(a) { + n = len(a) } - if len(ips) > 0 { - return ips[0].String(), nil + rand.Shuffle(len(a), func(i, j int) { + a[i], a[j] = a[j], a[i] + }) + return a[:n] +} + +// resolveOONIAddresses returns a max of maxDefaultOONIAddresses after +// performing DNS resolution. The returned IP addreses exclude possible +// bogons. +func resolveOONIAddresses(logger model.Logger) ([]string, error) { + + // We explicitely resolve with BogonIsError set to false, and + // later remove bogons from the list. The reason is that in this way + // we are able to control the rate at which we run tests by adding bogon addresses to the + // domain records for the test. + + resolver := netx.NewResolver(netx.Config{ + BogonIsError: false, + Logger: logger, + Saver: nil, + }) + + addrs := []string{} + + var lastErr error + + // Get the set of all IPs for all the hostnames we have. + + for _, hostname := range defaultOONIEndpoints { + resolved, err := lookupHost(context.Background(), hostname, resolver) + if err != nil { + lastErr = err + continue + } + for _, ipaddr := range resolved { + if !slices.Contains(addrs, ipaddr) { + addrs = append(addrs, ipaddr) + } + } } - return "", fmt.Errorf("cannot resolve %v", defaultOpenVPNEndpoint) + + // Sample a max of maxDefaultOONIAddresses + + sampled := sampleN(addrs, maxDefaultOONIAddresses) + + // Remove the bogons + + valid := []string{} + + for _, addr := range sampled { + if !netxlite.IsBogon(addr) { + valid = append(valid, addr) + } + } + + // We only return error if the filtered list is zero len. + + if (len(valid) == 0) && (lastErr != nil) { + return valid, lastErr + } + + return valid, nil } -func defaultOONITargetURL(ip string) string { - return "openvpn://oonivpn.corp/?address=" + ip + ":1194" +func lookupHost(ctx context.Context, hostname string, r model.Resolver) ([]string, error) { + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + return r.LookupHost(ctx, hostname) } -func defaultOONIOpenVPNTargetUDP() (string, error) { - ip, err := resolveTarget(defaultOpenVPNEndpoint) - if err != nil { - return "", err +// pickOONIOpenVPNTargets crafts targets from the passed array of IP addresses. +func pickOONIOpenVPNTargets(ipaddrList []string) ([]string, error) { + if len(ipaddrList) == 0 { + return []string{}, nil } - return defaultOONITargetURL(ip) + "&transport=udp", nil -} -func defaultOONIOpenVPNTargetTCP() (string, error) { - ip, err := resolveTarget(defaultOpenVPNEndpoint) - if err != nil { - return "", err + // Step 1. Create endpoint list. + + endpoints := []endpoint{} + for _, ipaddr := range ipaddrList { + + // Probe the canonical 1194/udp and 1194/tcp ports + + endpoints = append(endpoints, endpoint{ + Obfuscation: "none", + Port: "1194", + Protocol: "openvpn", + Provider: "oonivpn", + IPAddr: ipaddr, + Transport: "tcp", + }) + endpoints = append(endpoints, endpoint{ + Obfuscation: "none", + Port: "1194", + Protocol: "openvpn", + Provider: "oonivpn", + IPAddr: ipaddr, + Transport: "udp", + }) + + } + + // Pick one IP from the list and sample on non-standard ports + // to check if the standard port was filtered. + + extra := ipaddrList[rand.Intn(len(ipaddrList))] + endpoints = append(endpoints, endpoint{ + Obfuscation: "none", + Protocol: "openvpn", + Provider: "oonivpn", + IPAddr: extra, + Port: "53", + Transport: "udp", + }) + endpoints = append(endpoints, endpoint{ + Obfuscation: "none", + Protocol: "openvpn", + Provider: "oonivpn", + IPAddr: extra, + Port: "443", + Transport: "tcp", + }) + + // Step 2. Create targets for the selected endpoints. + + targets := make([]string, 0) + for _, e := range endpoints { + targets = append(targets, e.AsInputURI()) + } + if len(targets) > 0 { + return targets, nil } - return defaultOONITargetURL(ip) + "&transport=tcp", nil + return nil, fmt.Errorf("cannot find any usable endpoint") } func pickFromDefaultOONIOpenVPNConfig() *Config { diff --git a/internal/experiment/openvpn/targets_test.go b/internal/experiment/openvpn/targets_test.go index 480be65f5..615c9847e 100644 --- a/internal/experiment/openvpn/targets_test.go +++ b/internal/experiment/openvpn/targets_test.go @@ -4,78 +4,104 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/ooni/probe-cli/v3/internal/model" ) -func Test_resolveTarget(t *testing.T) { - // TODO: mustHaveExternalNetwork() equivalent. - if testing.Short() { - t.Skip("skip test in short mode") - } - - _, err := resolveTarget("google.com") +func Test_pickFromDefaultOONIOpenVPNConfig(t *testing.T) { + pick := pickFromDefaultOONIOpenVPNConfig() - if err != nil { - if err.Error() == "connection_refused" { - // connection_refused is raised when running this test - // on the restricted network for coverage tests. - // so we bail out - return - } - t.Fatal("should be able to resolve the target") + if pick.Cipher != "AES-256-GCM" { + t.Fatal("cipher unexpected") } + if pick.SafeCA != defaultCA { + t.Fatal("ca unexpected") + } +} - _, err = resolveTarget("nothing.corp") - if err == nil { - t.Fatal("should not be able to resolve the target") +func TestSampleN(t *testing.T) { + // Table of test cases + tests := []struct { + name string + a []string + n int + expected int // Expected length of result + }{ + {"n less than slice length", []string{"a", "b", "c", "d", "e"}, 3, 3}, + {"n greater than slice length", []string{"a", "b", "c", "d", "e"}, 10, 5}, + {"n equal to zero", []string{"a", "b", "c", "d", "e"}, 0, 0}, + {"empty slice", []string{}, 3, 0}, } - _, err = resolveTarget("asfasfasfasfasfafs.ooni.io") - if err == nil { - t.Fatal("should not be able to resolve the target") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := sampleN(tt.a, tt.n) + + // Check the length of the result + if len(result) != tt.expected { + t.Errorf("Expected %d items, got %d", tt.expected, len(result)) + } + + // Check for duplicates + seen := make(map[string]struct{}) + for _, v := range result { + if _, exists := seen[v]; exists { + t.Errorf("Duplicate value %s found", v) + } + seen[v] = struct{}{} + } + }) } } -func Test_defaultOONIOpenVPNTargetUDP(t *testing.T) { - url, err := defaultOONIOpenVPNTargetUDP() - if err != nil { - if err.Error() == "connection_refused" { - // connection_refused is raised when running this test - // on the restricted network for coverage tests. - // so we bail out - return - } - t.Fatal("unexpected error") - } - expected := "openvpn://oonivpn.corp/?address=37.218.243.98:1194&transport=udp" - if diff := cmp.Diff(url, expected); diff != "" { - t.Fatal(diff) +func Test_resolveOONIAddresses(t *testing.T) { + expected := []string{ + "108.61.164.186", + "37.218.243.98", } -} + t.Run("check resolution is what we expect", func(t *testing.T) { + if testing.Short() { + // this test uses the real internet so we want to skip this in short mode + t.Skip("skip test in short mode") + } -func Test_defaultOONIOpenVPNTargetTCP(t *testing.T) { - url, err := defaultOONIOpenVPNTargetTCP() - if err != nil { - if err.Error() == "connection_refused" { - // connection_refused is raised when running this test - // on the restricted network for coverage tests. - // so we bail out + got, err := resolveOONIAddresses(model.DiscardLogger) + if err != nil { + t.Errorf("resolveOONIAddresses() error = %v, wantErr %v", err, nil) return } - t.Fatal("unexpected error") - } - expected := "openvpn://oonivpn.corp/?address=37.218.243.98:1194&transport=tcp" - if diff := cmp.Diff(url, expected); diff != "" { - t.Fatal(diff) - } + if diff := cmp.Diff(expected, got, cmpopts.SortSlices(func(x, y string) bool { return x < y })); diff != "" { + t.Errorf("Mismatch (-expected +got):\n%s", diff) + } + }) } -func Test_pickFromDefaultOONIOpenVPNConfig(t *testing.T) { - pick := pickFromDefaultOONIOpenVPNConfig() - - if pick.Cipher != "AES-256-GCM" { - t.Fatal("cipher unexpected") - } - if pick.SafeCA != defaultCA { - t.Fatal("ca unexpected") - } +func Test_pickOONIOpenVPNTargets(t *testing.T) { + t.Run("empty ip list produces empty targets", func(t *testing.T) { + endpoints, err := pickOONIOpenVPNTargets([]string{}) + if err != nil { + t.Fatal("expected nil error") + } + if len(endpoints) != 0 { + t.Fatal("expected empty endpoints") + } + }) + t.Run("single-item ip list produces valid targets", func(t *testing.T) { + endpoints, err := pickOONIOpenVPNTargets([]string{"1.1.1.1"}) + if err != nil { + t.Fatal("expected nil error") + } + if len(endpoints) != 4 { + t.Fatalf("expected 4 endpoints, got %d", len(endpoints)) + } + }) + t.Run("2-item ip list produces 6 targets", func(t *testing.T) { + endpoints, err := pickOONIOpenVPNTargets([]string{"1.1.1.1", "2.2.2.2"}) + if err != nil { + t.Fatal("expected nil error") + } + if len(endpoints) != 6 { + t.Fatalf("expected 6 endpoints, got %d", len(endpoints)) + } + }) }