feat: add fallback domain names for openvpn experiment (#1654)

while working on this, I also gave more priority to possible oonirun
descriptors passed in the command line.

- Related: #2805

## Checklist

- [x] I have read the [contribution
guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request:
ooni/probe#2805
- [ ] if you changed anything related to how experiments work and you
need to reflect these changes in the ooni/spec repository, please link
to the related ooni/spec pull request: <!-- add URL here -->
- [x] if you changed code inside an experiment, make sure you bump its
version number

## Description

Add fallback domains for openvpn default endpoints to be probed.

---------

Co-authored-by: DecFox <[email protected]>

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

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=

internal/experiment/openvpn/openvpn.go

@@ -17,7 +17,7 @@ import (
 
 const (
 	testName        = "openvpn"
-	testVersion     = "0.1.5"
+	testVersion     = "0.1.6"
 	openVPNProtocol = "openvpn"
 )
 

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")
 	}
 }

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
 }

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")
-	}
-}