From 6790cdc5ee386208316c71add375bb6fc53b6881 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 8 Oct 2024 17:07:12 +0200 Subject: [PATCH 01/10] feat: add fallback domain names while working on this, I also gave more priority to possible oonirun descriptors passed in the command line. - Related: #2805 --- internal/experiment/openvpn/endpoint.go | 8 ++ internal/experiment/openvpn/openvpn.go | 2 +- internal/experiment/openvpn/openvpn_test.go | 2 +- internal/experiment/openvpn/richerinput.go | 78 ++++++++---- internal/experiment/openvpn/targets.go | 129 +++++++++++++++++--- internal/experiment/openvpn/targets_test.go | 26 ++-- 6 files changed, 182 insertions(+), 63 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index df255527d..9c41a62c4 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -25,6 +25,10 @@ type endpoint struct { // IPAddr is the IP Address for this endpoint. IPAddr string + // DomainName is an optional domain name that we use internally to get the IP address. + // This is just a convenience field, the experiments should always be done against a canonical IPAddr. + DomainName string + // Obfuscation is any obfuscation method use to connect to this endpoint. // Valid values are: obfs4, none. Obfuscation string @@ -32,6 +36,10 @@ type endpoint struct { // Port is the Port for this endpoint. Port string + // PreferredCountries is an optional array of country codes. Probes in these countries have preference on this + // endpoint. + PreferredCountries []string + // Protocol is the tunneling protocol (openvpn, openvpn+obfs4). Protocol string 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..f3c88fc14 100644 --- a/internal/experiment/openvpn/richerinput.go +++ b/internal/experiment/openvpn/richerinput.go @@ -84,6 +84,13 @@ type targetLoader struct { // Load implements model.ExperimentTargetLoader. func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, error) { + // 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 (fallthru) + if err != nil { + tl.loader.Logger.Warnf("Error loading OpenVPN targets from cli") + } + // 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) { @@ -91,16 +98,7 @@ func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, err 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 - inputs, err := targetloading.LoadStatic(tl.loader) - - // Handle the case where we couldn't load from CLI or files: - if err != nil { - return nil, err + tl.loader.Logger.Warnf("Error fetching OpenVPN targets from backend") } // Build the list of targets that we should measure. @@ -119,22 +117,52 @@ func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, err return tl.loadFromDefaultEndpoints() } -func (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) { - tl.loader.Logger.Warnf("Using default OpenVPN endpoints") +func makeTargetListPerProtocol(cc string, num int) []model.ExperimentTarget { targets := []model.ExperimentTarget{} - if udp, err := defaultOONIOpenVPNTargetUDP(); err == nil { - targets = append(targets, - &Target{ - Config: pickFromDefaultOONIOpenVPNConfig(), - URL: udp, - }) + var reverse bool + switch num { + case 1, 2: + // for single or few picks, we start the list in the natural order + reverse = false + default: + // for multiple picks, we start the list from the bottom, so that we can lookup + // custom country campaigns first. + reverse = true + } + if inputsUDP, err := pickOONIOpenVPNTargets("udp", cc, num, reverse); err == nil { + for _, t := range inputsUDP { + targets = append(targets, + &Target{ + Config: pickFromDefaultOONIOpenVPNConfig(), + URL: t, + }) + } } - if tcp, err := defaultOONIOpenVPNTargetTCP(); err == nil { - targets = append(targets, - &Target{ - Config: pickFromDefaultOONIOpenVPNConfig(), - URL: tcp, - }) + if inputsTCP, err := pickOONIOpenVPNTargets("tcp", cc, num, reverse); err == nil { + for _, t := range inputsTCP { + targets = append(targets, + &Target{ + Config: pickFromDefaultOONIOpenVPNConfig(), + URL: t, + }) + } + } + return targets +} + +func (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) { + cc := tl.session.ProbeCC() + + tl.loader.Logger.Warnf("Using default OpenVPN endpoints") + tl.loader.Logger.Warnf("Picking endpoints for %s", cc) + + var targets []model.ExperimentTarget + switch cc { + case "RU", "CN", "IR", "EG", "NL": + // we want to cover all of our bases for a few interest countries + targets = makeTargetListPerProtocol(cc, 20) + default: + targets = makeTargetListPerProtocol(cc, 1) } return targets, nil } @@ -160,8 +188,6 @@ func (tl *targetLoader) loadFromBackend(ctx context.Context) ([]model.Experiment 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", } diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go index 6a85b48ca..e797760a4 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -4,9 +4,87 @@ import ( "fmt" "math/rand" "net" + "slices" ) -const defaultOpenVPNEndpoint = "openvpn-server1.ooni.io" +// defaultOpenVPNEndpoints contain a list of all default endpoints +// to be tried, in the order that we want the name resolution to happen. +var defaultOpenVPNEndpoints = []endpoint{ + // default domain. this should work fine for most places. + { + IPAddr: "", + DomainName: "openvpn-server1.ooni.io", + Obfuscation: "none", + Port: "1194", + Protocol: "openvpn", + Provider: "oonivpn", + Transport: "tcp", + }, + { + IPAddr: "", + DomainName: "openvpn-server1.ooni.io", + Obfuscation: "none", + Port: "1194", + Protocol: "openvpn", + Provider: "oonivpn", + Transport: "udp", + }, + // alt domain 1. still same endpoint ports, one udp and one tcp. + // TODO(ain): update to real domain names + { + IPAddr: "", + DomainName: "alt-domain1.example.org", + Obfuscation: "none", + Port: "1194", + Protocol: "openvpn", + Provider: "oonivpn", + Transport: "udp", + }, + { + IPAddr: "", + DomainName: "alt-domain1.example.org", + Obfuscation: "none", + Port: "1194", + Protocol: "openvpn", + Provider: "oonivpn", + Transport: "tcp", + }, + // alt domain 2. still same endpoint ports, one udp and one tcp. + // TODO(ain): update to real domain names + { + IPAddr: "", + DomainName: "alt-domain2.example.org", + Obfuscation: "none", + Port: "53", + Protocol: "openvpn", + Provider: "oonivpn", + Transport: "udp", + }, + { + IPAddr: "", + DomainName: "alt-domain2.example.org", + Obfuscation: "none", + Port: "443", + Protocol: "openvpn", + Provider: "oonivpn", + Transport: "tcp", + }, + // alt domain 3. this is reserved. + // TODO(ain): update to real domain names + { + IPAddr: "", + DomainName: "alt-domain3.example.org", + Obfuscation: "none", + Port: "443", + Protocol: "openvpn", + Provider: "oonivpn", + Transport: "tcp", + PreferredCountries: []string{ + "AM", "AZ", "BY", "GE", "KZ", "KG", "LT", "MD", "RU", "TJ", "TM", "UA", "UZ", + "IR", "CN", "EG"}, + }, + // TODO: add more backup domains here +} // 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 @@ -15,6 +93,7 @@ const defaultOpenVPNEndpoint = "openvpn-server1.ooni.io" // 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. +// If the probe services ever gets deployed, this step will not be needed anymore. func resolveTarget(domain string) (string, error) { ips, err := net.LookupIP(domain) if err != nil { @@ -23,27 +102,43 @@ func resolveTarget(domain string) (string, error) { if len(ips) > 0 { return ips[0].String(), nil } - return "", fmt.Errorf("cannot resolve %v", defaultOpenVPNEndpoint) -} - -func defaultOONITargetURL(ip string) string { - return "openvpn://oonivpn.corp/?address=" + ip + ":1194" + return "", fmt.Errorf("cannot resolve %v", domain) } -func defaultOONIOpenVPNTargetUDP() (string, error) { - ip, err := resolveTarget(defaultOpenVPNEndpoint) - if err != nil { - return "", err +// pickOONIOpenVPNTargets returns an array of input URIs from the list of available endpoints, up to max, +// for the given transport. By default, we use the first endpoint that resolves to an IP. If reverseOrder +// is specified, we reverse the list before attempting resolution. +func pickOONIOpenVPNTargets(transport string, cc string, max int, reverseOrder bool) ([]string, error) { + endpoints := slices.Clone(defaultOpenVPNEndpoints)[:] + if reverseOrder { + slices.Reverse(endpoints) } - return defaultOONITargetURL(ip) + "&transport=udp", nil -} + targets := make([]string, 0) + for _, endpoint := range endpoints { + if endpoint.Transport != transport { + continue + } + if len(endpoint.PreferredCountries) > 0 && !slices.Contains(endpoint.PreferredCountries, cc) { + // not for us + continue + } + // Do note that this will get the wrong result if we got DNS poisoning. + // When analyzing this data, you should be careful about bogus IPs. + ip, err := resolveTarget(endpoint.DomainName) + if err != nil { + continue + } + endpoint.IPAddr = ip -func defaultOONIOpenVPNTargetTCP() (string, error) { - ip, err := resolveTarget(defaultOpenVPNEndpoint) - if err != nil { - return "", err + targets = append(targets, endpoint.AsInputURI()) + if len(targets) == max { + return targets, nil + } + } + if len(targets) > 0 { + return targets, nil } - return defaultOONITargetURL(ip) + "&transport=tcp", nil + return nil, fmt.Errorf("cannot find any endpoint for %s", transport) } func pickFromDefaultOONIOpenVPNConfig() *Config { diff --git a/internal/experiment/openvpn/targets_test.go b/internal/experiment/openvpn/targets_test.go index 480be65f5..704750bb8 100644 --- a/internal/experiment/openvpn/targets_test.go +++ b/internal/experiment/openvpn/targets_test.go @@ -1,6 +1,7 @@ package openvpn import ( + "net/url" "testing" "github.com/google/go-cmp/cmp" @@ -35,8 +36,8 @@ func Test_resolveTarget(t *testing.T) { } } -func Test_defaultOONIOpenVPNTargetUDP(t *testing.T) { - url, err := defaultOONIOpenVPNTargetUDP() +func Test_pickOpenVPNTargets(t *testing.T) { + urls, err := pickOONIOpenVPNTargets("udp", "IT", 1, false) if err != nil { if err.Error() == "connection_refused" { // connection_refused is raised when running this test @@ -46,25 +47,14 @@ func Test_defaultOONIOpenVPNTargetUDP(t *testing.T) { } 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) - } -} + expected := "openvpn://oonivpn.corp?address=37.218.243.98:1194&transport=udp" -func Test_defaultOONIOpenVPNTargetTCP(t *testing.T) { - url, err := defaultOONIOpenVPNTargetTCP() + got, err := url.QueryUnescape(urls[0]) 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") + t.Fatal(err) } - expected := "openvpn://oonivpn.corp/?address=37.218.243.98:1194&transport=tcp" - if diff := cmp.Diff(url, expected); diff != "" { + + if diff := cmp.Diff(got, expected); diff != "" { t.Fatal(diff) } } From 1bdabbbb7151012c80c329f7330bae2e8fdfc328 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 17 Oct 2024 00:19:50 +0200 Subject: [PATCH 02/10] adapt to latest dns resolution strategy --- internal/experiment/openvpn/endpoint.go | 8 -- internal/experiment/openvpn/richerinput.go | 143 +++++++------------ internal/experiment/openvpn/targets.go | 146 +++++++------------- internal/experiment/openvpn/targets_test.go | 55 -------- 4 files changed, 99 insertions(+), 253 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 9c41a62c4..df255527d 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -25,10 +25,6 @@ type endpoint struct { // IPAddr is the IP Address for this endpoint. IPAddr string - // DomainName is an optional domain name that we use internally to get the IP address. - // This is just a convenience field, the experiments should always be done against a canonical IPAddr. - DomainName string - // Obfuscation is any obfuscation method use to connect to this endpoint. // Valid values are: obfs4, none. Obfuscation string @@ -36,10 +32,6 @@ type endpoint struct { // Port is the Port for this endpoint. Port string - // PreferredCountries is an optional array of country codes. Probes in these countries have preference on this - // endpoint. - PreferredCountries []string - // Protocol is the tunneling protocol (openvpn, openvpn+obfs4). Protocol string diff --git a/internal/experiment/openvpn/richerinput.go b/internal/experiment/openvpn/richerinput.go index f3c88fc14..1cd7384c0 100644 --- a/internal/experiment/openvpn/richerinput.go +++ b/internal/experiment/openvpn/richerinput.go @@ -3,16 +3,26 @@ package openvpn import ( "context" "fmt" + "slices" + "time" "github.com/ooni/probe-cli/v3/internal/experimentconfig" + "github.com/ooni/probe-cli/v3/internal/legacy/netx" "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" +// defaultOONIHostnames 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. +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 // providerAuthentication is a map so that we know which kind of credentials we // need to fill in the openvpn options for each known provider. @@ -91,16 +101,6 @@ func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, err tl.loader.Logger.Warnf("Error loading OpenVPN targets from cli") } - // 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") - } - // Build the list of targets that we should measure. var targets []model.ExperimentTarget for _, input := range inputs { @@ -117,93 +117,50 @@ func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, err return tl.loadFromDefaultEndpoints() } -func makeTargetListPerProtocol(cc string, num int) []model.ExperimentTarget { - targets := []model.ExperimentTarget{} - var reverse bool - switch num { - case 1, 2: - // for single or few picks, we start the list in the natural order - reverse = false - default: - // for multiple picks, we start the list from the bottom, so that we can lookup - // custom country campaigns first. - reverse = true - } - if inputsUDP, err := pickOONIOpenVPNTargets("udp", cc, num, reverse); err == nil { - for _, t := range inputsUDP { - targets = append(targets, - &Target{ - Config: pickFromDefaultOONIOpenVPNConfig(), - URL: t, - }) - } - } - if inputsTCP, err := pickOONIOpenVPNTargets("tcp", cc, num, reverse); err == nil { - for _, t := range inputsTCP { - targets = append(targets, - &Target{ - Config: pickFromDefaultOONIOpenVPNConfig(), - URL: t, - }) - } - } - return targets +// TODO: move to targets. +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 (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) { - cc := tl.session.ProbeCC() - - tl.loader.Logger.Warnf("Using default OpenVPN endpoints") - tl.loader.Logger.Warnf("Picking endpoints for %s", cc) - - var targets []model.ExperimentTarget - switch cc { - case "RU", "CN", "IR", "EG", "NL": - // we want to cover all of our bases for a few interest countries - targets = makeTargetListPerProtocol(cc, 20) - default: - targets = makeTargetListPerProtocol(cc, 1) - } - return targets, nil -} - -func (tl *targetLoader) loadFromBackend(ctx context.Context) ([]model.ExperimentTarget, error) { - if tl.options.Provider == "" { - tl.options.Provider = defaultProvider + resolver := netx.NewResolver(netx.Config{ + BogonIsError: false, + Logger: tl.session.Logger(), + Saver: nil, + }) + + addrs := []string{} + + // 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 { + tl.loader.Logger.Warnf("Cannot resolve %s", hostname) + continue + } + for _, ipaddr := range resolved { + if !slices.Contains(addrs, ipaddr) { + addrs = append(addrs, ipaddr) + } + } } - targets := make([]model.ExperimentTarget, 0) - provider := tl.options.Provider + fmt.Println(">>> ADDRS", addrs) - apiConfig, err := tl.session.FetchOpenVPNConfig(ctx, provider, tl.session.ProbeCC()) - 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) - } + // TODO: filter bogons (here), return err if nil - for _, input := range apiConfig.Inputs { - config := &Config{ - Auth: "SHA512", - Cipher: "AES-256-GCM", - } - 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) + tl.loader.Logger.Warnf("Picking from default OpenVPN endpoints") + targets := []model.ExperimentTarget{} + if inputs, err := pickOONIOpenVPNTargets(addrs); err == nil { + for _, url := range inputs { + targets = append(targets, + &Target{ + Config: pickFromDefaultOONIOpenVPNConfig(), + URL: url, + }) } - targets = append(targets, &Target{ - URL: input, - Config: config, - }) } - return targets, nil } diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go index e797760a4..2770357f0 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -3,142 +3,94 @@ package openvpn import ( "fmt" "math/rand" - "net" - "slices" ) +// TODO: deprecate, move to function below // defaultOpenVPNEndpoints contain a list of all default endpoints -// to be tried, in the order that we want the name resolution to happen. +// to be tried. We will fill in the IP Addresses. var defaultOpenVPNEndpoints = []endpoint{ - // default domain. this should work fine for most places. { - IPAddr: "", - DomainName: "openvpn-server1.ooni.io", Obfuscation: "none", Port: "1194", Protocol: "openvpn", Provider: "oonivpn", - Transport: "tcp", - }, - { IPAddr: "", - DomainName: "openvpn-server1.ooni.io", - Obfuscation: "none", - Port: "1194", - Protocol: "openvpn", - Provider: "oonivpn", - Transport: "udp", + Transport: "", }, - // alt domain 1. still same endpoint ports, one udp and one tcp. - // TODO(ain): update to real domain names { IPAddr: "", - DomainName: "alt-domain1.example.org", Obfuscation: "none", - Port: "1194", - Protocol: "openvpn", - Provider: "oonivpn", - Transport: "udp", - }, - { - IPAddr: "", - DomainName: "alt-domain1.example.org", - Obfuscation: "none", - Port: "1194", + Port: "443", Protocol: "openvpn", Provider: "oonivpn", Transport: "tcp", }, - // alt domain 2. still same endpoint ports, one udp and one tcp. - // TODO(ain): update to real domain names { IPAddr: "", - DomainName: "alt-domain2.example.org", Obfuscation: "none", Port: "53", Protocol: "openvpn", Provider: "oonivpn", Transport: "udp", }, - { - IPAddr: "", - DomainName: "alt-domain2.example.org", +} + +// pickOONIOpenVPNTargets returns an array of input URIs from the list of available endpoints, up to max, +// for the given transport. By default, we use the first endpoint that resolves to an IP. If reverseOrder +// is specified, we reverse the list before attempting resolution. +func pickOONIOpenVPNTargets(ipaddrList []string) ([]string, error) { + // Step 1. Create endpoint list. + endpoints := []endpoint{} + for _, ipAddr := range ipaddrList { + // 1. 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 and sample on non-standard ports + // to confirm if this one goes through. + extra := ipaddrList[rand.Intn(len(ipaddrList))] + endpoints = append(endpoints, endpoint{ Obfuscation: "none", - Port: "443", Protocol: "openvpn", Provider: "oonivpn", - Transport: "tcp", - }, - // alt domain 3. this is reserved. - // TODO(ain): update to real domain names - { - IPAddr: "", - DomainName: "alt-domain3.example.org", + IPAddr: extra, + Port: "53", + Transport: "udp", + }) + endpoints = append(endpoints, endpoint{ Obfuscation: "none", - Port: "443", Protocol: "openvpn", Provider: "oonivpn", + IPAddr: extra, + Port: "443", Transport: "tcp", - PreferredCountries: []string{ - "AM", "AZ", "BY", "GE", "KZ", "KG", "LT", "MD", "RU", "TJ", "TM", "UA", "UZ", - "IR", "CN", "EG"}, - }, - // TODO: add more backup domains here -} - -// 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. -// If the probe services ever gets deployed, this step will not be needed anymore. -func resolveTarget(domain string) (string, error) { - ips, err := net.LookupIP(domain) - if err != nil { - return "", err - } - if len(ips) > 0 { - return ips[0].String(), nil - } - return "", fmt.Errorf("cannot resolve %v", domain) -} + }) -// pickOONIOpenVPNTargets returns an array of input URIs from the list of available endpoints, up to max, -// for the given transport. By default, we use the first endpoint that resolves to an IP. If reverseOrder -// is specified, we reverse the list before attempting resolution. -func pickOONIOpenVPNTargets(transport string, cc string, max int, reverseOrder bool) ([]string, error) { - endpoints := slices.Clone(defaultOpenVPNEndpoints)[:] - if reverseOrder { - slices.Reverse(endpoints) - } + // Step 2. Create targets for the selected endpoints. targets := make([]string, 0) - for _, endpoint := range endpoints { - if endpoint.Transport != transport { - continue - } - if len(endpoint.PreferredCountries) > 0 && !slices.Contains(endpoint.PreferredCountries, cc) { - // not for us - continue - } - // Do note that this will get the wrong result if we got DNS poisoning. - // When analyzing this data, you should be careful about bogus IPs. - ip, err := resolveTarget(endpoint.DomainName) - if err != nil { - continue - } - endpoint.IPAddr = ip - - targets = append(targets, endpoint.AsInputURI()) - if len(targets) == max { - return targets, nil - } + for _, e := range endpoints { + targets = append(targets, e.AsInputURI()) } if len(targets) > 0 { return targets, nil } - return nil, fmt.Errorf("cannot find any endpoint for %s", transport) + 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 704750bb8..54e202d7f 100644 --- a/internal/experiment/openvpn/targets_test.go +++ b/internal/experiment/openvpn/targets_test.go @@ -1,64 +1,9 @@ package openvpn import ( - "net/url" "testing" - - "github.com/google/go-cmp/cmp" ) -func Test_resolveTarget(t *testing.T) { - // TODO: mustHaveExternalNetwork() equivalent. - if testing.Short() { - t.Skip("skip test in short mode") - } - - _, err := resolveTarget("google.com") - - 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") - } - - _, err = resolveTarget("nothing.corp") - if err == nil { - t.Fatal("should not be able to resolve the target") - } - - _, err = resolveTarget("asfasfasfasfasfafs.ooni.io") - if err == nil { - t.Fatal("should not be able to resolve the target") - } -} - -func Test_pickOpenVPNTargets(t *testing.T) { - urls, err := pickOONIOpenVPNTargets("udp", "IT", 1, false) - 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" - - got, err := url.QueryUnescape(urls[0]) - if err != nil { - t.Fatal(err) - } - - if diff := cmp.Diff(got, expected); diff != "" { - t.Fatal(diff) - } -} - func Test_pickFromDefaultOONIOpenVPNConfig(t *testing.T) { pick := pickFromDefaultOONIOpenVPNConfig() From 8ca92e1d03fc3c9f07138d8d56efe2be845f2018 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 17 Oct 2024 01:41:16 +0200 Subject: [PATCH 03/10] remove bogons --- internal/experiment/openvpn/richerinput.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/experiment/openvpn/richerinput.go b/internal/experiment/openvpn/richerinput.go index 1cd7384c0..b84249181 100644 --- a/internal/experiment/openvpn/richerinput.go +++ b/internal/experiment/openvpn/richerinput.go @@ -2,13 +2,13 @@ package openvpn import ( "context" - "fmt" "slices" "time" "github.com/ooni/probe-cli/v3/internal/experimentconfig" "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" "github.com/ooni/probe-cli/v3/internal/targetloading" ) @@ -147,13 +147,19 @@ func (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, er } } - fmt.Println(">>> ADDRS", addrs) + // Remove the bogons - // TODO: filter bogons (here), return err if nil + validAddrs := []string{} + + for _, addr := range addrs { + if !netxlite.IsBogon(addr) { + validAddrs = append(validAddrs, addr) + } + } tl.loader.Logger.Warnf("Picking from default OpenVPN endpoints") targets := []model.ExperimentTarget{} - if inputs, err := pickOONIOpenVPNTargets(addrs); err == nil { + if inputs, err := pickOONIOpenVPNTargets(validAddrs); err == nil { for _, url := range inputs { targets = append(targets, &Target{ From dd895e91c547d4a73b2b829bff20235870cbc7e7 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 17 Oct 2024 11:43:36 +0200 Subject: [PATCH 04/10] remove unused test --- .../experiment/openvpn/richerinput_test.go | 45 ------------------- internal/experiment/openvpn/targets.go | 30 ------------- 2 files changed, 75 deletions(-) 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 2770357f0..8a322319a 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -5,36 +5,6 @@ import ( "math/rand" ) -// TODO: deprecate, move to function below -// defaultOpenVPNEndpoints contain a list of all default endpoints -// to be tried. We will fill in the IP Addresses. -var defaultOpenVPNEndpoints = []endpoint{ - { - Obfuscation: "none", - Port: "1194", - Protocol: "openvpn", - Provider: "oonivpn", - IPAddr: "", - Transport: "", - }, - { - IPAddr: "", - Obfuscation: "none", - Port: "443", - Protocol: "openvpn", - Provider: "oonivpn", - Transport: "tcp", - }, - { - IPAddr: "", - Obfuscation: "none", - Port: "53", - Protocol: "openvpn", - Provider: "oonivpn", - Transport: "udp", - }, -} - // pickOONIOpenVPNTargets returns an array of input URIs from the list of available endpoints, up to max, // for the given transport. By default, we use the first endpoint that resolves to an IP. If reverseOrder // is specified, we reverse the list before attempting resolution. From 86202ed689aab53be9d9d0813daf9847eb60a910 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 17 Oct 2024 12:02:08 +0200 Subject: [PATCH 05/10] sample a max of n targets --- internal/experiment/openvpn/richerinput.go | 51 ++---------- internal/experiment/openvpn/targets.go | 93 ++++++++++++++++++++++ 2 files changed, 98 insertions(+), 46 deletions(-) diff --git a/internal/experiment/openvpn/richerinput.go b/internal/experiment/openvpn/richerinput.go index b84249181..e84d5c41c 100644 --- a/internal/experiment/openvpn/richerinput.go +++ b/internal/experiment/openvpn/richerinput.go @@ -2,28 +2,13 @@ package openvpn import ( "context" - "slices" "time" "github.com/ooni/probe-cli/v3/internal/experimentconfig" - "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" "github.com/ooni/probe-cli/v3/internal/targetloading" ) -// defaultOONIHostnames 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. -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 - // 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{ @@ -125,41 +110,15 @@ func lookupHost(ctx context.Context, hostname string, r model.Resolver) ([]strin } func (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) { - resolver := netx.NewResolver(netx.Config{ - BogonIsError: false, - Logger: tl.session.Logger(), - Saver: nil, - }) - - addrs := []string{} - - // 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 { - tl.loader.Logger.Warnf("Cannot resolve %s", hostname) - continue - } - for _, ipaddr := range resolved { - if !slices.Contains(addrs, ipaddr) { - addrs = append(addrs, ipaddr) - } - } - } - - // Remove the bogons - - validAddrs := []string{} + targets := []model.ExperimentTarget{} - for _, addr := range addrs { - if !netxlite.IsBogon(addr) { - validAddrs = append(validAddrs, addr) - } + addrs, err := resolveOONIAddresses(tl.session.Logger()) + if err != nil { + return targets, err } tl.loader.Logger.Warnf("Picking from default OpenVPN endpoints") - targets := []model.ExperimentTarget{} - if inputs, err := pickOONIOpenVPNTargets(validAddrs); err == nil { + if inputs, err := pickOONIOpenVPNTargets(addrs); err == nil { for _, url := range inputs { targets = append(targets, &Target{ diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go index 8a322319a..f39cb43c8 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -1,10 +1,103 @@ package openvpn import ( + "context" "fmt" "math/rand" + "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" ) +// defaultOONIHostnames 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. +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) + } + + rand.Seed(time.Now().UnixNano()) + sampled := make([]string, 0) + + // Use a map to track indices we've already selected to avoid duplicates + picked := make(map[int]struct{}) + + for len(sampled) < n { + idx := rand.Intn(len(a)) // Random index + if _, exists := picked[idx]; !exists { + sampled = append(sampled, a[idx]) + picked[idx] = struct{}{} // Mark index as used + } + } + + return sampled +} + +// resolveOONIAddresses returns a max of maxDefaultOONIAddresses after +// performing DNS resolution. The returned IP addreses exclude possible +// bogons. +func resolveOONIAddresses(logger model.Logger) ([]string, error) { + 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) + } + } + } + + // 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 +} + // pickOONIOpenVPNTargets returns an array of input URIs from the list of available endpoints, up to max, // for the given transport. By default, we use the first endpoint that resolves to an IP. If reverseOrder // is specified, we reverse the list before attempting resolution. From ec9a637c2b72ea702f6b3eb9042b5d637ef7da30 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 17 Oct 2024 12:14:41 +0200 Subject: [PATCH 06/10] add test for resolution --- internal/experiment/openvpn/targets.go | 4 +- internal/experiment/openvpn/targets_test.go | 61 +++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go index f39cb43c8..d47801915 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -5,14 +5,13 @@ import ( "fmt" "math/rand" "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" ) -// defaultOONIHostnames is the array of hostnames that will return valid +// 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. var defaultOONIEndpoints = []string{ @@ -30,7 +29,6 @@ func sampleN(a []string, n int) []string { n = len(a) } - rand.Seed(time.Now().UnixNano()) sampled := make([]string, 0) // Use a map to track indices we've already selected to avoid duplicates diff --git a/internal/experiment/openvpn/targets_test.go b/internal/experiment/openvpn/targets_test.go index 54e202d7f..4b84682b6 100644 --- a/internal/experiment/openvpn/targets_test.go +++ b/internal/experiment/openvpn/targets_test.go @@ -2,6 +2,10 @@ package openvpn 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_pickFromDefaultOONIOpenVPNConfig(t *testing.T) { @@ -14,3 +18,60 @@ func Test_pickFromDefaultOONIOpenVPNConfig(t *testing.T) { t.Fatal("ca unexpected") } } + +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}, + } + + 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_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") + } + + got, err := resolveOONIAddresses(model.DiscardLogger) + if err != nil { + t.Errorf("resolveOONIAddresses() error = %v, wantErr %v", err, nil) + return + } + 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) + } + }) +} From 1b703679cbbdcf567aadcd66b3e21f59dbf66dfc Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 17 Oct 2024 12:19:29 +0200 Subject: [PATCH 07/10] x --- internal/experiment/openvpn/targets.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go index d47801915..873a286ab 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -13,7 +13,9 @@ import ( // 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. +// 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", @@ -96,20 +98,22 @@ func resolveOONIAddresses(logger model.Logger) ([]string, error) { return valid, nil } -// pickOONIOpenVPNTargets returns an array of input URIs from the list of available endpoints, up to max, -// for the given transport. By default, we use the first endpoint that resolves to an IP. If reverseOrder -// is specified, we reverse the list before attempting resolution. +// pickOONIOpenVPNTargets crafts targets from the passed array of IP addresses. func pickOONIOpenVPNTargets(ipaddrList []string) ([]string, error) { + // Step 1. Create endpoint list. + endpoints := []endpoint{} - for _, ipAddr := range ipaddrList { - // 1. Probe the canonical 1194/udp and 1194/tcp ports + 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, + IPAddr: ipaddr, Transport: "tcp", }) endpoints = append(endpoints, endpoint{ @@ -117,14 +121,15 @@ func pickOONIOpenVPNTargets(ipaddrList []string) ([]string, error) { Port: "1194", Protocol: "openvpn", Provider: "oonivpn", - IPAddr: ipAddr, + IPAddr: ipaddr, Transport: "udp", }) } - // Pick one IP and sample on non-standard ports - // to confirm if this one goes through. + // 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", @@ -144,6 +149,7 @@ func pickOONIOpenVPNTargets(ipaddrList []string) ([]string, error) { }) // Step 2. Create targets for the selected endpoints. + targets := make([]string, 0) for _, e := range endpoints { targets = append(targets, e.AsInputURI()) From bcab63d28f0dc98363de4916b25df109a4007d59 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 17 Oct 2024 12:24:14 +0200 Subject: [PATCH 08/10] x --- internal/experiment/openvpn/richerinput.go | 8 -------- internal/experiment/openvpn/targets.go | 7 +++++++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/internal/experiment/openvpn/richerinput.go b/internal/experiment/openvpn/richerinput.go index e84d5c41c..5cf212770 100644 --- a/internal/experiment/openvpn/richerinput.go +++ b/internal/experiment/openvpn/richerinput.go @@ -2,7 +2,6 @@ package openvpn import ( "context" - "time" "github.com/ooni/probe-cli/v3/internal/experimentconfig" "github.com/ooni/probe-cli/v3/internal/model" @@ -102,13 +101,6 @@ func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, err return tl.loadFromDefaultEndpoints() } -// TODO: move to targets. -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 (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) { targets := []model.ExperimentTarget{} diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go index 873a286ab..99fdb7dd5 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -5,6 +5,7 @@ import ( "fmt" "math/rand" "slices" + "time" "github.com/ooni/probe-cli/v3/internal/legacy/netx" "github.com/ooni/probe-cli/v3/internal/model" @@ -98,6 +99,12 @@ func resolveOONIAddresses(logger model.Logger) ([]string, error) { return valid, nil } +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) +} + // pickOONIOpenVPNTargets crafts targets from the passed array of IP addresses. func pickOONIOpenVPNTargets(ipaddrList []string) ([]string, error) { From 78ac661ee641e90267c677f0df01600f3a120071 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 22 Oct 2024 10:36:03 +0200 Subject: [PATCH 09/10] add docs, simplify function, some marginal perf gain --- go.mod | 2 +- internal/experiment/openvpn/richerinput.go | 16 +++++++++-- internal/experiment/openvpn/targets.go | 31 ++++++++++----------- internal/experiment/openvpn/targets_test.go | 30 ++++++++++++++++++++ 4 files changed, 60 insertions(+), 19 deletions(-) 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/internal/experiment/openvpn/richerinput.go b/internal/experiment/openvpn/richerinput.go index 5cf212770..ed2ce74b8 100644 --- a/internal/experiment/openvpn/richerinput.go +++ b/internal/experiment/openvpn/richerinput.go @@ -77,6 +77,9 @@ 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) { // First, attempt to load the static inputs from CLI and files inputs, err := targetloading.LoadStatic(tl.loader) @@ -97,8 +100,17 @@ func (tl *targetLoader) Load(ctx context.Context) ([]model.ExperimentTarget, err return targets, nil } - // Return the hardcoded endpoints. - return tl.loadFromDefaultEndpoints() + // 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 len(targets) == 0 { + tl.loader.Logger.Warnf("No targets loaded from default endpoints") + } + return targets, nil } func (tl *targetLoader) loadFromDefaultEndpoints() ([]model.ExperimentTarget, error) { diff --git a/internal/experiment/openvpn/targets.go b/internal/experiment/openvpn/targets.go index 99fdb7dd5..11522fa2d 100644 --- a/internal/experiment/openvpn/targets.go +++ b/internal/experiment/openvpn/targets.go @@ -31,27 +31,22 @@ func sampleN(a []string, n int) []string { if n > len(a) { n = len(a) } - - sampled := make([]string, 0) - - // Use a map to track indices we've already selected to avoid duplicates - picked := make(map[int]struct{}) - - for len(sampled) < n { - idx := rand.Intn(len(a)) // Random index - if _, exists := picked[idx]; !exists { - sampled = append(sampled, a[idx]) - picked[idx] = struct{}{} // Mark index as used - } - } - - return sampled + 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, @@ -62,7 +57,8 @@ func resolveOONIAddresses(logger model.Logger) ([]string, error) { var lastErr error - // get the set of all IPs for all the hostnames we have. + // 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 { @@ -107,6 +103,9 @@ func lookupHost(ctx context.Context, hostname string, r model.Resolver) ([]strin // pickOONIOpenVPNTargets crafts targets from the passed array of IP addresses. func pickOONIOpenVPNTargets(ipaddrList []string) ([]string, error) { + if len(ipaddrList) == 0 { + return []string{}, nil + } // Step 1. Create endpoint list. diff --git a/internal/experiment/openvpn/targets_test.go b/internal/experiment/openvpn/targets_test.go index 4b84682b6..615c9847e 100644 --- a/internal/experiment/openvpn/targets_test.go +++ b/internal/experiment/openvpn/targets_test.go @@ -75,3 +75,33 @@ func Test_resolveOONIAddresses(t *testing.T) { } }) } + +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)) + } + }) +} From ffbd670baa28b0e1ab8f49d6b9f39877b375c2e8 Mon Sep 17 00:00:00 2001 From: DecFox <33030671+DecFox@users.noreply.github.com> Date: Fri, 22 Nov 2024 00:51:14 +0530 Subject: [PATCH 10/10] update go.sum to use ooni/minivpn v0.0.7 --- go.sum | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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=