From 506ea97b982835e9c94d9868eca9c56652ab0bc5 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Fri, 29 Mar 2024 22:52:38 +0100 Subject: [PATCH] wip --- internal/experiment/openvpn/endpoint.go | 2 +- internal/experiment/openvpn/openvpn.go | 54 +++++++++------------ internal/experiment/openvpn/openvpn_test.go | 41 ++++++++++++---- 3 files changed, 56 insertions(+), 41 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index cf4ab3a5a..d18b50680 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -133,7 +133,7 @@ func (e *endpoint) AsInputURI() string { type endpointList []*endpoint // DefaultEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment and -// the backend query fails. We risk distributing endpoints that can go stale, so we should be careful about +// the backend query fails for whatever reason. We risk distributing endpoints that can go stale, so we should be careful about // the stability of the endpoints selected here, but in restrictive environments it's useful to have something // to probe in absence of an useful OONI API. Valid credentials are still needed, though. var DefaultEndpoints = endpointList{ diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 495430e54..0c0bf3b42 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -4,7 +4,6 @@ package openvpn import ( "context" "errors" - "fmt" "strconv" "strings" "time" @@ -28,6 +27,7 @@ const ( // of this experiment. By tagging these variables with `ooni:"..."`, we allow // miniooni's -O flag to find them and set them. type Config struct { + // TODO(ainghazal): Provider is right now ignored. InputLoader should get the provider from options. Provider string `ooni:"VPN provider"` SafeKey string `ooni:"key to connect to the OpenVPN endpoint"` SafeCert string `ooni:"cert to connect to the OpenVPN endpoint"` @@ -90,8 +90,6 @@ type Measurer struct { // NewExperimentMeasurer creates a new ExperimentMeasurer. func NewExperimentMeasurer(config Config, testName string) model.ExperimentMeasurer { - // TODO(ainghazal): allow ooniprobe to override this. - config.Provider = "riseup" return Measurer{config: config, testName: testName} } @@ -109,19 +107,14 @@ var ( ErrInvalidInput = errors.New("invalid input") ) -// parseListOfInputs return an endpointlist from a comma-separated list of inputs, -// and any error if the endpoints could not be parsed properly. -func parseListOfInputs(inputs string) (endpointList, error) { - endpoints := make(endpointList, 0) - inputList := strings.Split(inputs, ",") - for _, i := range inputList { - e, err := newEndpointFromInputString(i) - if err != nil { - return endpoints, err - } - endpoints = append(endpoints, e) +func isValidProtocol(s string) bool { + if strings.HasPrefix(s, "openvpn://") { + return true } - return endpoints, nil + if strings.HasPrefix(s, "openvpn+obfs4://") { + return true + } + return false } // Run implements model.ExperimentMeasurer.Run. @@ -134,26 +127,26 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { var endpoint *endpoint - if measurement.Input == "" { + if measurement.Input != "" { + if ok := isValidProtocol(string(measurement.Input)); !ok { + return ErrInvalidInput + } + var err error + endpoint, err = newEndpointFromInputString(string(measurement.Input)) + if err != nil { + return err + } + } else { // if input is null, we get one from the hardcoded list of inputs. + // TODO(ainghazal): can input be empty at this stage? + // InputPolicy should ensure we have a hardcoded input, + // so this is probably non-reachable code. Move the shuffling there. sess.Logger().Info("No input given, picking one hardcoded endpoint at random") endpoint = DefaultEndpoints.Shuffle()[0] measurement.Input = model.MeasurementTarget(endpoint.AsInputURI()) - } else { - // otherwise, we expect a comma-separated value of inputs in - // the URI scheme defined for openvpn experiments. - endpoints, err := parseListOfInputs(string(measurement.Input)) - if err != nil { - return err - } - if len(endpoints) != 1 { - return fmt.Errorf("%w: only single input accepted", ErrInvalidInput) - } - endpoint = endpoints[0] } tk := NewTestKeys() - sess.Logger().Infof("Probing endpoint %s", endpoint.String()) // TODO: separate pre-connection checks @@ -221,10 +214,11 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI( return creds, nil } - // No options passed, let's hit OONI API for credential distribution. + // No options passed, so let's get the credentials that inputbuilder should have cached + // for us after hitting the OONI API. // We expect the credentials from the API response to be encoded as the direct PEM serialization. apiCreds, err := m.fetchProviderCredentials(ctx, sess, provider) - // TODO(ainghazal): validate + // TODO(ainghazal): validate credentials have the info we expect, certs are not expired etc. if err != nil { sess.Logger().Warnf("Error fetching credentials from API: %s", err.Error()) diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index 38792f85c..f4948edfb 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -6,11 +6,9 @@ import ( "testing" "time" - "github.com/apex/log" "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/experiment/example" "github.com/ooni/probe-cli/v3/internal/experiment/openvpn" - "github.com/ooni/probe-cli/v3/internal/legacy/mockable" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" @@ -59,7 +57,6 @@ func TestAddConnectionTestKeys(t *testing.T) { Provider: "unknown", OpenVPNOptions: openvpn.OpenVPNOptions{}, Status: openvpn.ArchivalOpenVPNConnectStatus{}, - StartTime: time.Now(), T0: 0, T: 0, Tags: []string{}, @@ -122,11 +119,33 @@ func TestSuccess(t *testing.T) { if m.ExperimentName() != "openvpn" { t.Fatal("invalid ExperimentName") } - if m.ExperimentVersion() != "0.1.0" { + if m.ExperimentVersion() != "0.1.1" { t.Fatal("invalid ExperimentVersion") } ctx := context.Background() - sess := &mocks.Session{} + sess := &mocks.Session{ + MockLogger: func() model.Logger { + return model.DiscardLogger + }, + MockFetchOpenVPNConfig: func(context.Context, string, string) (*model.OOAPIVPNProviderConfig, error) { + return &model.OOAPIVPNProviderConfig{ + Provider: "provider", + Config: &struct { + CA string "json:\"ca\"" + Cert string "json:\"cert,omitempty\"" + Key string "json:\"key,omitempty\"" + Username string "json:\"username,omitempty\"" + Password string "json:\"password,omitempty\"" + }{ + CA: "ca", + Cert: "cert", + Key: "key", + }, + Inputs: []string{}, + DateUpdated: time.Now(), + }, nil + }, + } callbacks := model.NewPrinterCallbacks(sess.Logger()) measurement := new(model.Measurement) args := &model.ExperimentArgs{ @@ -134,6 +153,7 @@ func TestSuccess(t *testing.T) { Measurement: measurement, Session: sess, } + // TODO: mock runner err := m.Run(ctx, args) if err != nil { t.Fatal(err) @@ -141,12 +161,13 @@ func TestSuccess(t *testing.T) { } func TestFailure(t *testing.T) { - m := example.NewExperimentMeasurer(example.Config{ - SleepTime: int64(2 * time.Millisecond), - ReturnError: true, - }, "example") + m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn") ctx := context.Background() - sess := &mockable.Session{MockableLogger: log.Log} + sess := &mocks.Session{ + MockLogger: func() model.Logger { + return model.DiscardLogger + }, + } callbacks := model.NewPrinterCallbacks(sess.Logger()) args := &model.ExperimentArgs{ Callbacks: callbacks,