diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index 195a5a9d5..3612d1b34 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -346,7 +346,7 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI for _, provider := range openvpnDefaultProviders { reply, err := il.vpnConfig(ctx, provider) if err != nil { - return nil, err + break } // here we're just collecting all the inputs. we also cache the configs so that // each experiment run can access the credentials for a given provider. @@ -364,7 +364,6 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI for _, endpoint := range openvpn.DefaultEndpoints { urls = append(urls, model.OOAPIURLInfo{URL: endpoint.AsInputURI()}) } - return nil, ErrNoURLsReturned } return urls, nil } diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 42f4a35a0..7b8ad86b4 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -11,6 +11,7 @@ import ( vpnconfig "github.com/ooni/minivpn/pkg/config" vpntracex "github.com/ooni/minivpn/pkg/tracex" + "github.com/ooni/probe-cli/v3/internal/model" ) var ( @@ -184,7 +185,11 @@ func isValidProvider(provider string) bool { // 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(tracer *vpntracex.Tracer, endpoint *endpoint, creds *vpnconfig.OpenVPNOptions) (*vpnconfig.Config, error) { +func getOpenVPNConfig( + tracer *vpntracex.Tracer, + logger model.Logger, + endpoint *endpoint, + creds *vpnconfig.OpenVPNOptions) (*vpnconfig.Config, error) { // TODO(ainghazal): use merge ability in vpnconfig.OpenVPNOptions merge (pending PR) provider := endpoint.Provider if !isValidProvider(provider) { @@ -193,6 +198,7 @@ func getOpenVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, creds *vpnco baseOptions := defaultOptionsByProvider[provider] cfg := vpnconfig.NewConfig( + vpnconfig.WithLogger(logger), vpnconfig.WithOpenVPNOptions( &vpnconfig.OpenVPNOptions{ // endpoint-specific options. @@ -210,9 +216,9 @@ func getOpenVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, creds *vpnco Key: creds.Key, }, ), - vpnconfig.WithHandshakeTracer(tracer)) + vpnconfig.WithHandshakeTracer(tracer), + ) - // TODO: sanity check (Remote, Port, Proto etc + missing certs) return cfg, nil } diff --git a/internal/experiment/openvpn/endpoint_test.go b/internal/experiment/openvpn/endpoint_test.go index 3ff61b39d..502fd379c 100644 --- a/internal/experiment/openvpn/endpoint_test.go +++ b/internal/experiment/openvpn/endpoint_test.go @@ -284,7 +284,7 @@ func Test_getVPNConfig(t *testing.T) { Key: []byte("key"), } - cfg, err := getOpenVPNConfig(tracer, e, creds) + cfg, err := getOpenVPNConfig(tracer, nil, e, creds) if err != nil { t.Fatalf("did not expect error, got: %v", err) } @@ -333,7 +333,7 @@ func Test_getVPNConfig_with_unknown_provider(t *testing.T) { Cert: []byte("cert"), Key: []byte("key"), } - _, err := getOpenVPNConfig(tracer, e, creds) + _, err := getOpenVPNConfig(tracer, nil, e, creds) if !errors.Is(err, ErrInvalidInput) { t.Fatalf("expected invalid input error, got: %v", err) } diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index b97cc3f4a..2dd0f9912 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -231,79 +231,54 @@ func (m *Measurer) GetCredentialsFromOptionsOrAPI( } -// Run implements model.ExperimentMeasurer.Run. -// A single run expects exactly ONE input (endpoint), but we can modify whether -// to test different transports by settings options. -func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { - callbacks := args.Callbacks - measurement := args.Measurement - sess := args.Session +// mergeOpenVPNConfig attempts to get credentials from Options or API, and then +// constructs a [vpnconfig.Config] instance after merging the credentials passed by options or API response. +// It also returns an error if the operation fails. +func (m *Measurer) mergeOpenVPNConfig( + ctx context.Context, + sess model.ExperimentSession, + endpoint *endpoint, + tracer *vpntracex.Tracer) (*vpnconfig.Config, error) { - endpoint, err := parseEndpoint(measurement) + logger := sess.Logger() + + credentials, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, endpoint.Provider) if err != nil { - return err + return nil, err } - tk := NewTestKeys() - sess.Logger().Infof("Probing endpoint %s", endpoint.String()) - - // TODO: separate pre-connection checks - connResult, err := m.connectAndHandshake(ctx, int64(1), time.Now(), sess, endpoint) + openvpnConfig, err := getOpenVPNConfig(tracer, logger, endpoint, credentials) if err != nil { - sess.Logger().Warn("Fatal error while attempting to connect to endpoint, aborting!") - return err - } - if connResult != nil { - tk.AddConnectionTestKeys(connResult) + return nil, err } - tk.Success = tk.AllConnectionsSuccessful() - - callbacks.OnProgress(1.0, "All endpoints probed") - measurement.TestKeys = tk - - // TODO(ainghazal): validate we have valid config for each endpoint. - // TODO(ainghazal): validate hostname is a valid IP (ipv4 or 6) - // TODO(ainghazal): decide what to do if we have expired certs (abort one measurement or abort the whole experiment?) - - // Note: if here we return an error, the parent code will assume - // something fundamental was wrong and we don't have a measurement - // to submit to the OONI collector. Keep this in mind when you - // are writing new experiments! - return nil + // TODO: sanity check (Remote, Port, Proto etc + missing certs) + return openvpnConfig, nil } // connectAndHandshake dials a connection and attempts an OpenVPN handshake using that dialer. -func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTime time.Time, sess model.ExperimentSession, endpoint *endpoint) (*SingleConnection, error) { - - logger := sess.Logger() +func (m *Measurer) connectAndHandshake( + ctx context.Context, + zeroTime time.Time, + index int64, + logger model.Logger, + endpoint *endpoint, + openvpnConfig *vpnconfig.Config, + handshakeTracer *vpntracex.Tracer) (*SingleConnection, error) { // create a trace for the network dialer trace := measurexlite.NewTrace(index, zeroTime) dialer := trace.NewDialerWithoutResolver(logger) + var failure string // create a vpn tun Device that attempts to dial and performs the handshake - handshakeTracer := vpntracex.NewTracerWithTransactionID(zeroTime, index) - - // TODO -- move to outer function ------ - credentials, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, endpoint.Provider) - if err != nil { - return nil, err - } - - openvpnConfig, err := getOpenVPNConfig(handshakeTracer, endpoint, credentials) - if err != nil { - return nil, err - } - // TODO -- move to outer function ------ - tun, err := tunnel.Start(ctx, dialer, openvpnConfig) - - var failure string if err != nil { failure = err.Error() } - defer tun.Close() + if tun != nil { + defer tun.Close() + } handshakeEvents := handshakeTracer.Trace() port, _ := strconv.Atoi(endpoint.Port) @@ -347,15 +322,63 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTim }, nil } -// TODO: get cached from session instead of fetching every time func (m *Measurer) FetchProviderCredentials( ctx context.Context, sess model.ExperimentSession, provider string) (*model.OOAPIVPNProviderConfig, error) { - // TODO(ainghazal): do pass country code, can be useful to orchestrate campaigns specific to areas + // TODO(ainghazal): pass real country code, can be useful to orchestrate campaigns specific to areas. config, err := sess.FetchOpenVPNConfig(ctx, provider, "XX") if err != nil { return &model.OOAPIVPNProviderConfig{}, err } return config, nil } + +// Run implements model.ExperimentMeasurer.Run. +// A single run expects exactly ONE input (endpoint), but we can modify whether +// to test different transports by settings options. +func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { + callbacks := args.Callbacks + measurement := args.Measurement + sess := args.Session + + endpoint, err := parseEndpoint(measurement) + if err != nil { + return err + } + + tk := NewTestKeys() + + zeroTime := time.Now() + idx := int64(1) + handshakeTracer := vpntracex.NewTracerWithTransactionID(zeroTime, idx) + + openvpnConfig, err := m.mergeOpenVPNConfig(ctx, sess, endpoint, handshakeTracer) + if err != nil { + return err + } + sess.Logger().Infof("Probing endpoint %s", endpoint.String()) + + connResult, err := m.connectAndHandshake(ctx, zeroTime, idx, sess.Logger(), endpoint, openvpnConfig, handshakeTracer) + if err != nil { + sess.Logger().Warn("Fatal error while attempting to connect to endpoint, aborting!") + return err + } + if connResult != nil { + tk.AddConnectionTestKeys(connResult) + } + tk.Success = tk.AllConnectionsSuccessful() + + callbacks.OnProgress(1.0, "All endpoints probed") + measurement.TestKeys = tk + + // TODO(ainghazal): validate we have valid config for each endpoint. + // TODO(ainghazal): validate hostname is a valid IP (ipv4 or 6) + // TODO(ainghazal): decide what to do if we have expired certs (abort one measurement or abort the whole experiment?) + + // Note: if here we return an error, the parent code will assume + // something fundamental was wrong and we don't have a measurement + // to submit to the OONI collector. Keep this in mind when you + // are writing new experiments! + return nil +} diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index aa984f6bc..2fff8bd9c 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -254,12 +254,6 @@ func TestGetCredentialsFromOptionsOrAPI(t *testing.T) { t.Fatalf("expected nil options, got %v", opts) } }) - /* - sess.MockFetchOpenVPNConfig = func(context.Context, string, string) (*model.OOAPIVPNProviderConfig, error) { - return nil, someError - } - */ - } func TestAddConnectionTestKeys(t *testing.T) { @@ -369,25 +363,6 @@ func TestVPNInput(t *testing.T) { // TODO -- do a real test, get credentials etc. } -func TestSuccess(t *testing.T) { - m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn") - ctx := context.Background() - sess := makeMockSession() - callbacks := model.NewPrinterCallbacks(sess.Logger()) - measurement := new(model.Measurement) - args := &model.ExperimentArgs{ - Callbacks: callbacks, - Measurement: measurement, - Session: sess, - } - fmt.Println(args, m, ctx) - // TODO: mock runner - // err := m.Run(ctx, args) - //if err != nil { - // t.Fatal(err) - //} -} - func TestMeasurer_FetchProviderCredentials(t *testing.T) { t.Run("Measurer.FetchProviderCredentials calls method in session", func(t *testing.T) { m := openvpn.NewExperimentMeasurer( @@ -422,3 +397,31 @@ func TestMeasurer_FetchProviderCredentials(t *testing.T) { }) } + +func TestSuccess(t *testing.T) { + m := openvpn.NewExperimentMeasurer(openvpn.Config{ + Provider: "riseup", + SafeCA: "base64:Zm9v", + SafeKey: "base64:Zm9v", + SafeCert: "base64:Zm9v", + }, "openvpn") + ctx := context.Background() + sess := makeMockSession() + callbacks := model.NewPrinterCallbacks(sess.Logger()) + measurement := new(model.Measurement) + measurement.Input = "openvpn://riseup.corp/?address=127.0.0.1:9989&transport=tcp" + args := &model.ExperimentArgs{ + Callbacks: callbacks, + Measurement: measurement, + Session: sess, + } + if sess.Logger() == nil { + t.Fatal("logger should not be nil") + } + fmt.Println(ctx, args, m) + + err := m.Run(ctx, args) + if err != nil { + t.Fatal(err) + } +}