feat(openvpn): implement richer input

internal/engine/session.go

@@ -66,7 +66,6 @@ type Session struct {
 	softwareName             string
 	softwareVersion          string
 	tempDir                  string
-	vpnConfig                map[string]model.OOAPIVPNProviderConfig
 
 	// closeOnce allows us to call Close just once.
 	closeOnce sync.Once
@@ -178,7 +177,6 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) {
 		torArgs:                 config.TorArgs,
 		torBinary:               config.TorBinary,
 		tunnelDir:               config.TunnelDir,
-		vpnConfig:               make(map[string]model.OOAPIVPNProviderConfig),
 	}
 	proxyURL := config.ProxyURL
 	if proxyURL != nil {
@@ -381,9 +379,6 @@ func (s *Session) FetchTorTargets(
 // internal cache. We do this to avoid hitting the API for every input.
 func (s *Session) FetchOpenVPNConfig(
 	ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) {
-	if config, ok := s.vpnConfig[provider]; ok {
-		return &config, nil
-	}
 	clnt, err := s.newOrchestraClient(ctx)
 	if err != nil {
 		return nil, err
@@ -397,7 +392,6 @@ func (s *Session) FetchOpenVPNConfig(
 	if err != nil {
 		return nil, err
 	}
-	s.vpnConfig[provider] = config
 	return &config, nil
 }
 

internal/experiment/openvpn/endpoint.go

@@ -1,12 +1,12 @@
 package openvpn
 
 import (
-	"encoding/base64"
 	"errors"
 	"fmt"
 	"math/rand"
 	"net"
 	"net/url"
+	"slices"
 	"strings"
 
 	vpnconfig "github.com/ooni/minivpn/pkg/config"
@@ -178,16 +178,6 @@ func (e endpointList) Shuffle() endpointList {
 	return e
 }
 
-// defaultOptionsByProvider is a map containing base config for
-// all the known providers. We extend this base config with credentials coming
-// from the OONI API.
-var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{
-	"riseupvpn": {
-		Auth:   "SHA512",
-		Cipher: "AES-256-GCM",
-	},
-}
-
 // APIEnabledProviders is the list of providers that the stable API Endpoint knows about.
 // This array will be a subset of the keys in defaultOptionsByProvider, but it might make sense
 // to still register info about more providers that the API officially knows about.
@@ -196,40 +186,25 @@ var APIEnabledProviders = []string{
 	"riseupvpn",
 }
 
-// isValidProvider returns true if the provider is found as key in the registry of defaultOptionsByProvider.
-// TODO(ainghazal): consolidate with list of enabled providers from the API viewpoint.
+// isValidProvider returns true if the provider is found as key in the array of APIEnabledProviders
 func isValidProvider(provider string) bool {
-	_, ok := defaultOptionsByProvider[provider]
-	return ok
+	return slices.Contains(APIEnabledProviders, provider)
 }
 
-// getOpenVPNConfig gets a properly configured [*vpnconfig.Config] object for the given endpoint.
-// To obtain that, we merge the endpoint specific configuration with base options.
-// Base options are hardcoded for the moment, for comparability among different providers.
-// We can add them to the OONI API and as extra cli options if ever needed.
-func getOpenVPNConfig(
+// mergeOpenVPNConfig gets a properly configured [*vpnconfig.Config] object for the given endpoint.
+// To obtain that, we merge the endpoint specific configuration with the options passed as richer input targets.
+func mergeOpenVPNConfig(
 	tracer *vpntracex.Tracer,
 	logger model.Logger,
 	endpoint *endpoint,
-	creds *vpnconfig.OpenVPNOptions) (*vpnconfig.Config, error) {
+	config *Config) (*vpnconfig.Config, error) {
+
 	// TODO(ainghazal): use merge ability in vpnconfig.OpenVPNOptions merge (pending PR)
 	provider := endpoint.Provider
 	if !isValidProvider(provider) {
 		return nil, fmt.Errorf("%w: unknown provider: %s", ErrInvalidInput, provider)
 	}
 
-	baseOptions := defaultOptionsByProvider[provider]
-
-	if baseOptions == nil {
-		return nil, fmt.Errorf("empty baseOptions for provider: %s", provider)
-	}
-	if baseOptions.Cipher == "" {
-		return nil, fmt.Errorf("empty cipher for provider: %s", provider)
-	}
-	if baseOptions.Auth == "" {
-		return nil, fmt.Errorf("empty auth for provider: %s", provider)
-	}
-
 	cfg := vpnconfig.NewConfig(
 		vpnconfig.WithLogger(logger),
 		vpnconfig.WithOpenVPNOptions(
@@ -239,14 +214,13 @@ func getOpenVPNConfig(
 				Port:   endpoint.Port,
 				Proto:  vpnconfig.Proto(endpoint.Transport),
 
-				// options coming from the default known values.
-				Cipher: baseOptions.Cipher,
-				Auth:   baseOptions.Auth,
-
-				// auth coming from passed credentials.
-				CA:   creds.CA,
-				Cert: creds.Cert,
-				Key:  creds.Key,
+				// options and credentials come from the experiment
+				// richer input targets.
+				Cipher: config.Cipher,
+				Auth:   config.Auth,
+				CA:     []byte(config.SafeCA),
+				Cert:   []byte(config.SafeCert),
+				Key:    []byte(config.SafeKey),
 			},
 		),
 		vpnconfig.WithHandshakeTracer(tracer),
@@ -255,20 +229,6 @@ func getOpenVPNConfig(
 	return cfg, nil
 }
 
-// maybeExtractBase64Blob is used to pass credentials as command-line options.
-func maybeExtractBase64Blob(val string) (string, error) {
-	s := strings.TrimPrefix(val, "base64:")
-	if len(s) == len(val) {
-		// no prefix, so we'll treat this as a pem-encoded credential.
-		return s, nil
-	}
-	dec, err := base64.URLEncoding.DecodeString(strings.TrimSpace(s))
-	if err != nil {
-		return "", fmt.Errorf("%w: %s", ErrBadBase64Blob, err)
-	}
-	return string(dec), nil
-}
-
 func isValidProtocol(s string) bool {
 	if strings.HasPrefix(s, "openvpn://") {
 		return true

internal/experiment/openvpn/endpoint_test.go

@@ -8,7 +8,6 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
-	vpnconfig "github.com/ooni/minivpn/pkg/config"
 	vpntracex "github.com/ooni/minivpn/pkg/tracex"
 )
 
@@ -272,21 +271,24 @@ func Test_isValidProvider(t *testing.T) {
 	}
 }
 
-func Test_getVPNConfig(t *testing.T) {
+func Test_mergeVPNConfig(t *testing.T) {
 	tracer := vpntracex.NewTracer(time.Now())
 	e := &endpoint{
 		Provider:  "riseupvpn",
 		IPAddr:    "1.1.1.1",
 		Port:      "443",
 		Transport: "udp",
 	}
-	creds := &vpnconfig.OpenVPNOptions{
-		CA:   []byte("ca"),
-		Cert: []byte("cert"),
-		Key:  []byte("key"),
+
+	config := &Config{
+		Auth:     "SHA512",
+		Cipher:   "AES-256-GCM",
+		SafeCA:   "ca",
+		SafeCert: "cert",
+		SafeKey:  "key",
 	}
 
-	cfg, err := getOpenVPNConfig(tracer, nil, e, creds)
+	cfg, err := mergeOpenVPNConfig(tracer, nil, e, config)
 	if err != nil {
 		t.Fatalf("did not expect error, got: %v", err)
 	}
@@ -311,81 +313,37 @@ func Test_getVPNConfig(t *testing.T) {
 	if transport := cfg.OpenVPNOptions().Proto; string(transport) != e.Transport {
 		t.Errorf("expected transport %s, got %s", e.Transport, transport)
 	}
-	if diff := cmp.Diff(cfg.OpenVPNOptions().CA, creds.CA); diff != "" {
+	if diff := cmp.Diff(cfg.OpenVPNOptions().CA, []byte(config.SafeCA)); diff != "" {
 		t.Error(diff)
 	}
-	if diff := cmp.Diff(cfg.OpenVPNOptions().Cert, creds.Cert); diff != "" {
+	if diff := cmp.Diff(cfg.OpenVPNOptions().Cert, []byte(config.SafeCert)); diff != "" {
 		t.Error(diff)
 	}
-	if diff := cmp.Diff(cfg.OpenVPNOptions().Key, creds.Key); diff != "" {
+	if diff := cmp.Diff(cfg.OpenVPNOptions().Key, []byte(config.SafeKey)); diff != "" {
 		t.Error(diff)
 	}
 }
 
-func Test_getVPNConfig_with_unknown_provider(t *testing.T) {
+func Test_mergeOpenVPNConfig_with_unknown_provider(t *testing.T) {
 	tracer := vpntracex.NewTracer(time.Now())
 	e := &endpoint{
 		Provider:  "nsa",
 		IPAddr:    "1.1.1.1",
 		Port:      "443",
 		Transport: "udp",
 	}
-	creds := &vpnconfig.OpenVPNOptions{
-		CA:   []byte("ca"),
-		Cert: []byte("cert"),
-		Key:  []byte("key"),
+	cfg := &Config{
+		SafeCA:   "ca",
+		SafeCert: "cert",
+		SafeKey:  "key",
 	}
-	_, err := getOpenVPNConfig(tracer, nil, e, creds)
+	_, err := mergeOpenVPNConfig(tracer, nil, e, cfg)
 	if !errors.Is(err, ErrInvalidInput) {
 		t.Fatalf("expected invalid input error, got: %v", err)
 	}
 
 }
 
-func Test_extractBase64Blob(t *testing.T) {
-	t.Run("decode good blob", func(t *testing.T) {
-		blob := "base64:dGhlIGJsdWUgb2N0b3B1cyBpcyB3YXRjaGluZw=="
-		decoded, err := maybeExtractBase64Blob(blob)
-		if decoded != "the blue octopus is watching" {
-			t.Fatal("could not decoded blob correctly")
-		}
-		if err != nil {
-			t.Fatal("should not fail with first blob")
-		}
-	})
-	t.Run("try decode without prefix", func(t *testing.T) {
-		blob := "dGhlIGJsdWUgb2N0b3B1cyBpcyB3YXRjaGluZw=="
-		dec, err := maybeExtractBase64Blob(blob)
-		if err != nil {
-			t.Fatal("should fail without prefix")
-		}
-		if dec != blob {
-			t.Fatal("decoded should be the same")
-		}
-	})
-	t.Run("bad base64 blob should fail", func(t *testing.T) {
-		blob := "base64:dGhlIGJsdWUgb2N0b3B1cyBpcyB3YXRjaGluZw"
-		_, err := maybeExtractBase64Blob(blob)
-		if !errors.Is(err, ErrBadBase64Blob) {
-			t.Fatal("bad blob should fail without prefix")
-		}
-	})
-	t.Run("decode empty blob", func(t *testing.T) {
-		blob := "base64:"
-		_, err := maybeExtractBase64Blob(blob)
-		if err != nil {
-			t.Fatal("empty blob should not fail")
-		}
-	})
-	t.Run("illegal base64 data should fail", func(t *testing.T) {
-		blob := "base64:=="
-		_, err := maybeExtractBase64Blob(blob)
-		if !errors.Is(err, ErrBadBase64Blob) {
-			t.Fatal("bad base64 data should fail")
-		}
-	})
-}
-
 func Test_IsValidProtocol(t *testing.T) {
 	t.Run("openvpn is valid", func(t *testing.T) {
 		if !isValidProtocol("openvpn://foobar.bar") {