From 5c0a4151e09d0b95a2070a3eff941aed8d31ce98 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Wed, 6 Mar 2024 03:36:21 +0100 Subject: [PATCH 01/53] stub for openvpn experiment --- internal/experiment/openvpn/endpoint.go | 16 ++++ internal/experiment/openvpn/openvpn.go | 94 +++++++++++++++++++++ internal/experiment/openvpn/openvpn_test.go | 57 +++++++++++++ internal/registry/openvpn.go | 29 +++++++ 4 files changed, 196 insertions(+) create mode 100644 internal/experiment/openvpn/endpoint.go create mode 100644 internal/experiment/openvpn/openvpn.go create mode 100644 internal/experiment/openvpn/openvpn_test.go create mode 100644 internal/registry/openvpn.go diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go new file mode 100644 index 0000000000..e22743f478 --- /dev/null +++ b/internal/experiment/openvpn/endpoint.go @@ -0,0 +1,16 @@ +package openvpn + +// Endpoint is a single endpoint to be probed. +type Endpoint struct { + // Provider is a unique label identifying the provider maintaining this endpoint. + Provider string + + // IPAddr is the IP Address for this endpoint. + IPAddr string + + // Port is the Port for this endpoint. + Port string + + // Transport is the underlying transport used for this endpoint. Valid transports are `tcp` and `udp`. + Transport string +} diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go new file mode 100644 index 0000000000..1afc138d0e --- /dev/null +++ b/internal/experiment/openvpn/openvpn.go @@ -0,0 +1,94 @@ +// Package openvpn contains a generic openvpn experiment. +package openvpn + +import ( + "context" + "errors" + "time" + + "github.com/ooni/probe-cli/v3/internal/model" +) + +const ( + testVersion = "0.1.0" + openVPNProcol = "openvpn" +) + +// Config contains the experiment config. +// +// This contains all the settings that user can set to modify the behaviour +// of this experiment. By tagging these variables with `ooni:"..."`, we allow +// miniooni's -O flag to find them and set them. +type Config struct { + Message string `ooni:"Message to emit at test completion"` + ReturnError bool `ooni:"Toogle to return a mocked error"` + SleepTime int64 `ooni:"Amount of time to sleep for"` +} + +// TestKeys contains the experiment's result. +type TestKeys struct { + Success bool `json:"success"` + Provider string `json:"provider"` + VPNProtocol string `json:"vpn_protocol"` + Transport string `json:"transport"` + Remote string `json:"remote"` + Obfuscation string `json:"obfuscation"` + BootstrapTime float64 `json:"bootstrap_time"` +} + +// Measurer performs the measurement. +type Measurer struct { + config Config + testName string +} + +// ExperimentName implements model.ExperimentMeasurer.ExperimentName. +func (m Measurer) ExperimentName() string { + return m.testName +} + +// ExperimentVersion implements model.ExperimentMeasurer.ExperimentVersion. +func (m Measurer) ExperimentVersion() string { + return testVersion +} + +// ErrFailure is the error returned when you set the +// config.ReturnError field to true. +var ErrFailure = errors.New("mocked error") + +// Run implements model.ExperimentMeasurer.Run. +func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { + callbacks := args.Callbacks + measurement := args.Measurement + sess := args.Session + var err error + if m.config.ReturnError { + err = ErrFailure + } + testkeys := &TestKeys{ + Success: err == nil, + Provider: "unknown", + VPNProtocol: openVPNProcol, + Transport: "udp", + Remote: "127.0.0.1:1194", + Obfuscation: "none", + BootstrapTime: 0, + } + measurement.TestKeys = testkeys + sess.Logger().Warnf("%s", "Follow the white rabbit.") + ctx, cancel := context.WithTimeout(ctx, time.Duration(m.config.SleepTime)) + defer cancel() + <-ctx.Done() + sess.Logger().Infof("%s", "Knock, knock, Neo.") + callbacks.OnProgress(1.0, m.config.Message) + // 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 err +} + +// NewExperimentMeasurer creates a new ExperimentMeasurer. +func NewExperimentMeasurer(config Config, testName string) model.ExperimentMeasurer { + return Measurer{config: config, testName: testName} +} diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go new file mode 100644 index 0000000000..67c0089bf2 --- /dev/null +++ b/internal/experiment/openvpn/openvpn_test.go @@ -0,0 +1,57 @@ +package openvpn_test + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/experiment/example" + "github.com/ooni/probe-cli/v3/internal/legacy/mockable" + "github.com/ooni/probe-cli/v3/internal/model" +) + +func TestSuccess(t *testing.T) { + m := example.NewExperimentMeasurer(example.Config{ + SleepTime: int64(2 * time.Millisecond), + }, "example") + if m.ExperimentName() != "example" { + t.Fatal("invalid ExperimentName") + } + if m.ExperimentVersion() != "0.1.0" { + t.Fatal("invalid ExperimentVersion") + } + ctx := context.Background() + sess := &mockable.Session{MockableLogger: log.Log} + callbacks := model.NewPrinterCallbacks(sess.Logger()) + measurement := new(model.Measurement) + args := &model.ExperimentArgs{ + Callbacks: callbacks, + Measurement: measurement, + Session: sess, + } + err := m.Run(ctx, args) + if err != nil { + t.Fatal(err) + } +} + +func TestFailure(t *testing.T) { + m := example.NewExperimentMeasurer(example.Config{ + SleepTime: int64(2 * time.Millisecond), + ReturnError: true, + }, "example") + ctx := context.Background() + sess := &mockable.Session{MockableLogger: log.Log} + callbacks := model.NewPrinterCallbacks(sess.Logger()) + args := &model.ExperimentArgs{ + Callbacks: callbacks, + Measurement: new(model.Measurement), + Session: sess, + } + err := m.Run(ctx, args) + if !errors.Is(err, example.ErrFailure) { + t.Fatal("expected an error here") + } +} diff --git a/internal/registry/openvpn.go b/internal/registry/openvpn.go new file mode 100644 index 0000000000..a02cd2c9e5 --- /dev/null +++ b/internal/registry/openvpn.go @@ -0,0 +1,29 @@ +package registry + +// +// Registers the `openvpn' experiment. +// + +import ( + "time" + + "github.com/ooni/probe-cli/v3/internal/experiment/openvpn" + "github.com/ooni/probe-cli/v3/internal/model" +) + +func init() { + AllExperiments["openvpn"] = &Factory{ + build: func(config interface{}) model.ExperimentMeasurer { + return openvpn.NewExperimentMeasurer( + *config.(*openvpn.Config), "openvpn", + ) + }, + config: &openvpn.Config{ + Message: "This is not an experiment yet!", + SleepTime: int64(time.Second), + }, + enabledByDefault: true, + interruptible: true, + inputPolicy: model.InputNone, + } +} From 06624b09b05903dce51db2f1191f8c217aec084b Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Wed, 6 Mar 2024 03:54:12 +0100 Subject: [PATCH 02/53] add hardcoded endpoint --- internal/experiment/openvpn/endpoint.go | 22 ++++++++++++++++++++-- internal/experiment/openvpn/openvpn.go | 11 ++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index e22743f478..2380aec6ba 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -1,7 +1,7 @@ package openvpn -// Endpoint is a single endpoint to be probed. -type Endpoint struct { +// endpoint is a single endpoint to be probed. +type endpoint struct { // Provider is a unique label identifying the provider maintaining this endpoint. Provider string @@ -14,3 +14,21 @@ type Endpoint struct { // Transport is the underlying transport used for this endpoint. Valid transports are `tcp` and `udp`. Transport string } + +// allEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment. +var allEndpoints = []endpoint{ + { + Provider: "riseup", + IPAddr: "185.220.103.11", + Port: "1194", + Transport: "tcp", + }, +} + +// sampleRandomEndpoint is a placeholder for a proper sampling function. +func sampleRandomEndpoint(all []endpoint) endpoint { + // chosen by fair dice roll + // guaranteed to be random + // https://xkcd.com/221/ + return all[0] +} diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 1afc138d0e..a697f60b47 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -4,6 +4,7 @@ package openvpn import ( "context" "errors" + "fmt" "time" "github.com/ooni/probe-cli/v3/internal/model" @@ -65,15 +66,19 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { if m.config.ReturnError { err = ErrFailure } + + target := sampleRandomEndpoint(allEndpoints) + testkeys := &TestKeys{ Success: err == nil, - Provider: "unknown", + Provider: target.Provider, VPNProtocol: openVPNProcol, - Transport: "udp", - Remote: "127.0.0.1:1194", + Transport: target.Transport, + Remote: fmt.Sprintf("%s:%s", target.IPAddr, target.Port), Obfuscation: "none", BootstrapTime: 0, } + measurement.TestKeys = testkeys sess.Logger().Warnf("%s", "Follow the white rabbit.") ctx, cancel := context.WithTimeout(ctx, time.Duration(m.config.SleepTime)) From aaf69957c18a132ecae69966a000ba03e2a237d2 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Wed, 6 Mar 2024 22:35:04 +0100 Subject: [PATCH 03/53] update archival format --- internal/experiment/openvpn/archival.go | 37 +++++ internal/experiment/openvpn/endpoint.go | 16 ++- internal/experiment/openvpn/openvpn.go | 184 +++++++++++++++++++----- internal/registry/openvpn.go | 8 +- 4 files changed, 201 insertions(+), 44 deletions(-) create mode 100644 internal/experiment/openvpn/archival.go diff --git a/internal/experiment/openvpn/archival.go b/internal/experiment/openvpn/archival.go new file mode 100644 index 0000000000..57d3f3e493 --- /dev/null +++ b/internal/experiment/openvpn/archival.go @@ -0,0 +1,37 @@ +package openvpn + +// TODO(ainghazal): move to archiva package when consolidated. + +// ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake. +type ArchivalOpenVPNHandshakeResult struct { + BootstrapTime float64 `json:"bootstrap_time,omitempty"` + Endpoint string `json:"endpoint"` + IP string `json:"ip"` + Port int `json:"port"` + Provider string `json:"provider"` + Status ArchivalOpenVPNConnectStatus `json:"status"` + T0 float64 `json:"t0,omitempty"` + T float64 `json:"t"` + Tags []string `json:"tags"` + TransactionID int64 `json:"transaction_id,omitempty"` +} + +// ArchivalOpenVPNConnectStatus is the status of ArchivalOpenVPNConnectResult. +type ArchivalOpenVPNConnectStatus struct { + Blocked *bool `json:"blocked,omitempty"` + Failure *string `json:"failure"` + Success bool `json:"success"` +} + +type ArchivalNetworkEvent struct { + // TODO(ainghazal): need to properly propagate I/O errors during the handshake. + Address string `json:"address,omitempty"` + Failure *string `json:"failure"` + NumBytes int64 `json:"num_bytes,omitempty"` + Operation string `json:"operation"` + Proto string `json:"proto,omitempty"` + T0 float64 `json:"t0,omitempty"` + T float64 `json:"t"` + TransactionID int64 `json:"transaction_id,omitempty"` + Tags []string `json:"tags,omitempty"` +} diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 2380aec6ba..775cfec0d3 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -1,26 +1,36 @@ package openvpn +import "fmt" + // endpoint is a single endpoint to be probed. type endpoint struct { - // Provider is a unique label identifying the provider maintaining this endpoint. - Provider string - // IPAddr is the IP Address for this endpoint. IPAddr string // Port is the Port for this endpoint. Port string + // Protocol is the tunneling protocol (openvpn, openvpn+obfs4). + Protocol string + + // Provider is a unique label identifying the provider maintaining this endpoint. + Provider string + // Transport is the underlying transport used for this endpoint. Valid transports are `tcp` and `udp`. Transport string } +func (e *endpoint) String() string { + return fmt.Sprintf("%s://%s:%s/%s", e.Protocol, e.IPAddr, e.Port, e.Transport) +} + // allEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment. var allEndpoints = []endpoint{ { Provider: "riseup", IPAddr: "185.220.103.11", Port: "1194", + Protocol: "openvpn", Transport: "tcp", }, } diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index a697f60b47..4d37e6c5b2 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -4,10 +4,16 @@ package openvpn import ( "context" "errors" - "fmt" + "strconv" "time" + "github.com/ooni/probe-cli/v3/internal/measurexlite" "github.com/ooni/probe-cli/v3/internal/model" + + "github.com/ooni/minivpn/pkg/config" + vpnconfig "github.com/ooni/minivpn/pkg/config" + vpntracex "github.com/ooni/minivpn/pkg/tracex" + "github.com/ooni/minivpn/pkg/tunnel" ) const ( @@ -21,20 +27,47 @@ 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 { - Message string `ooni:"Message to emit at test completion"` - ReturnError bool `ooni:"Toogle to return a mocked error"` - SleepTime int64 `ooni:"Amount of time to sleep for"` + vpnOptions vpnconfig.OpenVPNOptions } // TestKeys contains the experiment's result. type TestKeys struct { - Success bool `json:"success"` - Provider string `json:"provider"` - VPNProtocol string `json:"vpn_protocol"` - Transport string `json:"transport"` - Remote string `json:"remote"` - Obfuscation string `json:"obfuscation"` - BootstrapTime float64 `json:"bootstrap_time"` + Success bool `json:"success"` + Connections []*SingleConnection `json:"connections"` + + // TODO move into singlehandshake + /* + Provider string `json:"provider"` + VPNProtocol string `json:"vpn_protocol"` + Transport string `json:"transport"` + Remote string `json:"remote"` + Obfuscation string `json:"obfuscation"` + BootstrapTime float64 `json:"bootstrap_time"` + */ +} + +// NewTestKeys creates new openvpn TestKeys. +func NewTestKeys() *TestKeys { + return &TestKeys{ + Success: false, + Connections: []*SingleConnection{}, + } +} + +// AddConnectionTestKeys adds the result of a single OpenVPN connection attempt to the +// corresponding array in the [TestKeys] object. +func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { + tk.Connections = append(tk.Connections, result) +} + +// allConnectionsSuccessful returns true if all the registered connections have Status.Success equal to true. +func (tk *TestKeys) allConnectionsSuccessful() bool { + for _, c := range tk.Connections { + if !c.OpenVPNHandshake.Status.Success { + return false + } + } + return true } // Measurer performs the measurement. @@ -43,6 +76,16 @@ type Measurer struct { testName string } +// NewExperimentMeasurer creates a new ExperimentMeasurer. +func NewExperimentMeasurer(config Config, testName string) model.ExperimentMeasurer { + // TODO(ainghazal): get these per-provider, as defaults. + config.vpnOptions = vpnconfig.OpenVPNOptions{ + Cipher: "AES-256-GCM", + Auth: "SHA512", + } + return Measurer{config: config, testName: testName} +} + // ExperimentName implements model.ExperimentMeasurer.ExperimentName. func (m Measurer) ExperimentName() string { return m.testName @@ -57,43 +100,114 @@ func (m Measurer) ExperimentVersion() string { // config.ReturnError field to true. var ErrFailure = errors.New("mocked error") +// SingleConnection contains the results of a single handshake. +type SingleConnection struct { + TCPConnect *model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` + OpenVPNHandshake *ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` + NetworkEvents []*vpntracex.Event `json:"network_events"` + // TODO(ainghazal): pass the transaction idx also to the event tracer for uniformity. + // TODO(ainghazal): make sure to document in the spec that these network events only cover the handshake. + // TODO(ainghazal): in the future, we will want to store more operations under this struct for a single connection, + // like pingResults or urlgetter calls. + + // TODO(ainghazal): look how to store the index that identifies each connection attempt. +} + // Run implements model.ExperimentMeasurer.Run. func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { callbacks := args.Callbacks measurement := args.Measurement sess := args.Session - var err error - if m.config.ReturnError { - err = ErrFailure - } - target := sampleRandomEndpoint(allEndpoints) + tk := NewTestKeys() - testkeys := &TestKeys{ - Success: err == nil, - Provider: target.Provider, - VPNProtocol: openVPNProcol, - Transport: target.Transport, - Remote: fmt.Sprintf("%s:%s", target.IPAddr, target.Port), - Obfuscation: "none", - BootstrapTime: 0, + sess.Logger().Info("Starting to measure OpenVPN endpoints.") + for idx, endpoint := range allEndpoints { + tk.AddConnectionTestKeys(m.connectAndHandshake(ctx, int64(idx+1), time.Now(), sess.Logger(), endpoint)) } + 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): decide what to do if we have expired certs (abort one measurement or abort the whole experiment?) - measurement.TestKeys = testkeys - sess.Logger().Warnf("%s", "Follow the white rabbit.") - ctx, cancel := context.WithTimeout(ctx, time.Duration(m.config.SleepTime)) - defer cancel() - <-ctx.Done() - sess.Logger().Infof("%s", "Knock, knock, Neo.") - callbacks.OnProgress(1.0, m.config.Message) // 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 err + return nil } -// NewExperimentMeasurer creates a new ExperimentMeasurer. -func NewExperimentMeasurer(config Config, testName string) model.ExperimentMeasurer { - return Measurer{config: config, testName: testName} +// connectAndHandshake dials a connection and attempts an OpenVPN handshake using that dialer. +func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, + zeroTime time.Time, logger model.Logger, endpoint endpoint) *SingleConnection { + + // create a trace for the network dialer + trace := measurexlite.NewTrace(index, zeroTime) + + // TODO(ainghazal): can I pass tags to this tracer? + dialer := trace.NewDialerWithoutResolver(logger) + + // create a vpn tun Device that attempts to dial and performs the handshake + handshakeTracer := vpntracex.NewTracerWithTransactionID(zeroTime, index) + _, err := tunnel.Start(ctx, dialer, getVPNConfig(handshakeTracer, &endpoint, &m.config.vpnOptions)) + + var failure string + if err != nil { + failure = err.Error() + } + handshakeEvents := handshakeTracer.Trace() + port, _ := strconv.Atoi(endpoint.Port) + + var ( + tFirst float64 + tLast float64 + bootstrapTime float64 + ) + + if len(handshakeEvents) != 0 { + tFirst = handshakeEvents[0].AtTime + tLast = handshakeEvents[len(handshakeEvents)-1].AtTime + bootstrapTime = time.Since(zeroTime).Seconds() + } + + return &SingleConnection{ + TCPConnect: trace.FirstTCPConnectOrNil(), + OpenVPNHandshake: &ArchivalOpenVPNHandshakeResult{ + BootstrapTime: bootstrapTime, + Endpoint: endpoint.String(), + IP: endpoint.IPAddr, + Port: port, + Provider: endpoint.Provider, + Status: ArchivalOpenVPNConnectStatus{ + Failure: &failure, + Success: err == nil, + }, + T0: tFirst, + T: tLast, + Tags: []string{}, + TransactionID: index, + }, + NetworkEvents: handshakeEvents, + } +} + +func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, opts *config.OpenVPNOptions) *config.Config { + cfg := config.NewConfig( + config.WithOpenVPNOptions( + &config.OpenVPNOptions{ + Remote: endpoint.IPAddr, + Port: endpoint.Port, + Proto: config.Proto(endpoint.Transport), + CA: opts.CA, + Cert: opts.Cert, + Key: opts.Key, + Cipher: opts.Cipher, + Auth: opts.Auth, + }, + ), + config.WithHandshakeTracer(tracer)) + return cfg } diff --git a/internal/registry/openvpn.go b/internal/registry/openvpn.go index a02cd2c9e5..c060fbf1e5 100644 --- a/internal/registry/openvpn.go +++ b/internal/registry/openvpn.go @@ -5,8 +5,6 @@ package registry // import ( - "time" - "github.com/ooni/probe-cli/v3/internal/experiment/openvpn" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -18,10 +16,8 @@ func init() { *config.(*openvpn.Config), "openvpn", ) }, - config: &openvpn.Config{ - Message: "This is not an experiment yet!", - SleepTime: int64(time.Second), - }, + // TODO(ainghazal): we can pass an array of providers here. + config: &openvpn.Config{}, enabledByDefault: true, interruptible: true, inputPolicy: model.InputNone, From d4b65c381a6c55561c270d30bdaffa7d100cf02f Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Wed, 6 Mar 2024 22:57:11 +0100 Subject: [PATCH 04/53] wip --- internal/experiment/openvpn/endpoint.go | 49 +++++++++++++++++++++- internal/experiment/openvpn/openvpn.go | 54 ++++++++----------------- 2 files changed, 64 insertions(+), 39 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 775cfec0d3..333a2972d4 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -1,6 +1,12 @@ package openvpn -import "fmt" +import ( + "fmt" + + "github.com/ooni/minivpn/pkg/config" + vpnconfig "github.com/ooni/minivpn/pkg/config" + vpntracex "github.com/ooni/minivpn/pkg/tracex" +) // endpoint is a single endpoint to be probed. type endpoint struct { @@ -42,3 +48,44 @@ func sampleRandomEndpoint(all []endpoint) endpoint { // https://xkcd.com/221/ return all[0] } + +var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ + "riseup": { + Cipher: "AES-256-GCM", + Auth: "SHA512", + CA: []byte{}, + Key: []byte{}, + Cert: []byte{}, + }, +} + +// getVPNConfig gets a properly configured [*vpnconfig.Config] object for the given endpoint. +// To obtain that, we merge the endpoint specific configuration with base options. +// These base options are for the moment hardcoded. In the future we will want to be smarter +// about getting information for different providers. +func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint) (*vpnconfig.Config, error) { + // TODO(ainghazal): use options merge + provider := endpoint.Provider + // TODO(ainghazal): return error if provider unknown. we're in the happy path for now. + baseOptions := defaultOptionsByProvider[provider] + + cfg := vpnconfig.NewConfig( + vpnconfig.WithOpenVPNOptions( + &vpnconfig.OpenVPNOptions{ + Remote: endpoint.IPAddr, + Port: endpoint.Port, + Proto: vpnconfig.Proto(endpoint.Transport), + + // options coming from the default values, + // to be changed by check-in API info in the future. + CA: baseOptions.CA, + Cert: baseOptions.Cert, + Key: baseOptions.Key, + Cipher: baseOptions.Cipher, + Auth: baseOptions.Auth, + }, + ), + config.WithHandshakeTracer(tracer)) + // TODO: validate here and return an error, maybe. + return cfg, nil +} diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 4d37e6c5b2..1215a6d3d3 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -10,8 +10,6 @@ import ( "github.com/ooni/probe-cli/v3/internal/measurexlite" "github.com/ooni/probe-cli/v3/internal/model" - "github.com/ooni/minivpn/pkg/config" - vpnconfig "github.com/ooni/minivpn/pkg/config" vpntracex "github.com/ooni/minivpn/pkg/tracex" "github.com/ooni/minivpn/pkg/tunnel" ) @@ -27,23 +25,13 @@ 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 { - vpnOptions vpnconfig.OpenVPNOptions + Provider string } // TestKeys contains the experiment's result. type TestKeys struct { Success bool `json:"success"` Connections []*SingleConnection `json:"connections"` - - // TODO move into singlehandshake - /* - Provider string `json:"provider"` - VPNProtocol string `json:"vpn_protocol"` - Transport string `json:"transport"` - Remote string `json:"remote"` - Obfuscation string `json:"obfuscation"` - BootstrapTime float64 `json:"bootstrap_time"` - */ } // NewTestKeys creates new openvpn TestKeys. @@ -78,11 +66,8 @@ type Measurer struct { // NewExperimentMeasurer creates a new ExperimentMeasurer. func NewExperimentMeasurer(config Config, testName string) model.ExperimentMeasurer { - // TODO(ainghazal): get these per-provider, as defaults. - config.vpnOptions = vpnconfig.OpenVPNOptions{ - Cipher: "AES-256-GCM", - Auth: "SHA512", - } + // TODO(ainghazal): allow ooniprobe to override this. + config.Provider = "riseup" return Measurer{config: config, testName: testName} } @@ -123,7 +108,11 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { sess.Logger().Info("Starting to measure OpenVPN endpoints.") for idx, endpoint := range allEndpoints { - tk.AddConnectionTestKeys(m.connectAndHandshake(ctx, int64(idx+1), time.Now(), sess.Logger(), endpoint)) + connResult := m.connectAndHandshake(ctx, int64(idx+1), time.Now(), sess.Logger(), endpoint) + // TODO: better catch error here. + if connResult != nil { + tk.AddConnectionTestKeys(connResult) + } } tk.Success = tk.allConnectionsSuccessful() @@ -152,7 +141,14 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, // create a vpn tun Device that attempts to dial and performs the handshake handshakeTracer := vpntracex.NewTracerWithTransactionID(zeroTime, index) - _, err := tunnel.Start(ctx, dialer, getVPNConfig(handshakeTracer, &endpoint, &m.config.vpnOptions)) + + openvpnOptions, err := getVPNConfig(handshakeTracer, &endpoint) + if err != nil { + // TODO: find a better way to return the error - this is not a test failure, + // it's a failure to start the measurement. we should abort + return nil + } + _, err = tunnel.Start(ctx, dialer, openvpnOptions) var failure string if err != nil { @@ -193,21 +189,3 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, NetworkEvents: handshakeEvents, } } - -func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, opts *config.OpenVPNOptions) *config.Config { - cfg := config.NewConfig( - config.WithOpenVPNOptions( - &config.OpenVPNOptions{ - Remote: endpoint.IPAddr, - Port: endpoint.Port, - Proto: config.Proto(endpoint.Transport), - CA: opts.CA, - Cert: opts.Cert, - Key: opts.Key, - Cipher: opts.Cipher, - Auth: opts.Auth, - }, - ), - config.WithHandshakeTracer(tracer)) - return cfg -} From c4bb706755f2d1807c4dc726a8847bd52d83a352 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Wed, 6 Mar 2024 23:44:41 +0100 Subject: [PATCH 05/53] hardcode credentials as step zero --- internal/experiment/openvpn/endpoint.go | 72 ++++++++++++++++++++++--- internal/experiment/openvpn/openvpn.go | 4 +- 2 files changed, 67 insertions(+), 9 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 333a2972d4..d647df18d2 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -3,7 +3,6 @@ package openvpn import ( "fmt" - "github.com/ooni/minivpn/pkg/config" vpnconfig "github.com/ooni/minivpn/pkg/config" vpntracex "github.com/ooni/minivpn/pkg/tracex" ) @@ -51,11 +50,66 @@ func sampleRandomEndpoint(all []endpoint) endpoint { var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ "riseup": { - Cipher: "AES-256-GCM", Auth: "SHA512", - CA: []byte{}, - Key: []byte{}, - Cert: []byte{}, + Cipher: "AES-256-GCM", + CA: []byte(`-----BEGIN CERTIFICATE----- +MIIBYjCCAQigAwIBAgIBATAKBggqhkjOPQQDAjAXMRUwEwYDVQQDEwxMRUFQIFJv +b3QgQ0EwHhcNMjExMTAyMTkwNTM3WhcNMjYxMTAyMTkxMDM3WjAXMRUwEwYDVQQD +EwxMRUFQIFJvb3QgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQxOXBGu+gf +pjHzVteGTWL6XnFxtEnKMFpKaJkA/VOHmESzoLsZRQxt88GssxaqC01J17idQiqv +zgNpedmtvFtyo0UwQzAOBgNVHQ8BAf8EBAMCAqQwEgYDVR0TAQH/BAgwBgEB/wIB +ATAdBgNVHQ4EFgQUZdoUlJrCIUNFrpffAq+LQjnwEz4wCgYIKoZIzj0EAwIDSAAw +RQIgfr3w4tnRG+NdI3LsGPlsRktGK20xHTzsB3orB0yC6cICIQCB+/9y8nmSStfN +VUMUyk2hNd7/kC8nL222TTD7VZUtsg== +-----END CERTIFICATE-----`), + // TODO(ainghazal): this is extremely hacky, but it's a first step + // until we manage to have the check-in API handing credentials. + // Do note that these certificates will expire ca. Apr 6 2024 + // OTOH, yes, I do understand the risks of exposing key material + // on a public git repo. Thanks for caring. + Key: []byte(`-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAqprWmGJKLgZBFbdJUEMzKpJkWnVLoALSTTZqmzX8vuQD7W2J +HbwptiD+a7qCvikpX+bsRb9b84VctYZq/tnLwqRVeDfoega+pGws0KGMo74KWlUZ +1k+AjCbqxWJPlaYKNkDXAInsc6alEv09ZbeuGGpWtQSVpP+sudgDf9JpIEsnTLSK +5t0i1QX/53Vltr+omLCqd52a2bUxK8WNIwtsSs9lLGrpKTVJ1zKDpVBNmNFgahpk +kX5KAkoS0TVzBLPwNNq14GLnTd6YnJ66m9k5iiUBbML81bnE3qbxG7C/qoXIP4eH +0Y7RDBB0dlZ8PTBjeg0pnEtPF5MrglVRVeUimQIDAQABAoIBAGVgMspEBa5Jmx0r +V44xEFNov+ccsf54Dr1A66IlN3W7CjZok0SvDd4ixuv+3TfgP6y0DIv5hMs04P0g +za14f+K+Qed42VTBc0FC4nJqvKaEA6Tf0sWNYmZlrbXykDXtfz3z046HZpDmYkrh +Xj12IyZw8esIuV9daibYnGO1BTDhXy/B53zDjx6wYMDC3DFVa2gLSRWONtMnCYY8 +Hw7FbaP1Jxs6sNS/AKVZZo4SyBL1te80HN9Wo2syDmdc3o3aBMkCY9+u560dj8+5 +4xvn+d8ojp91Ts3o33DB6PY88r2UTg00ejGMn8LN7dCnZDO2mQ98nczKfcpYL0nW +CKxG6AECgYEA0L9XBdfps60nKNS5n4+rNvtYvhkvHOKkz7wFmYSo6r8M1ID3m7g3 +x6wwTY9MrlSPPsF9x6GnrmGIGIZsc8lNRuYFq/yemNhKfMi6KU9wnjqVQYDSg9S2 +fq4lutPxbeiQmSx5WYtjeaJXzTAzx9jT6t8QiAUXM06QgPPjLK7G+ZkCgYEA0Tku +iSz8Y2uHyBWOYFTIaEvvyCEJqyZ+hMgVRRgN7QzDjP4VUVmQClwdK7JIPNBaIf6V +Gvi+CXgb/oDMrcduMM4ZGoVN1ttpC3htn7qn35+38VsYPD3hgmF7r3WFSxoBd0vj +Yh7rO4tQo91tm0DkCs+NZvNRrFr1yL/VAHnDEQECgYAi3XJpdXCBJBCAT1dZgSN1 +oXFm/snRp0EjuSGuTGvyGUrJS2kPxyr53JaMvbxu+YybTLH3X9aj14Jlpj4C8MJJ +by3PVfgfSzDVuqjtMWl75Aj90chXYHXCns+Kbs/KLafJDZaPECrjK+xCRyS+4kYy +2mLmdQM0/JBCGXn+AosVMQKBgEFgy/DjlM6AaIKWcdIaTDGDIR95a2sG8VwOpc7c +cGWVqnmhYAn2obMLC7Z+1GHkfXXH9tHhzoho9t51YwAepIktrdyCsUslbtK9xAu4 +qQKRB0qtO4p/j7tNOPggEhHgw3qCxUABB2Ko6v75j2mHQns6ViZIfEoOdmVPxICM +i+8BAoGBAIn2RfbrcbrH3jZKNkklUMg7fU8K0w7PQ7kCR8vql4chA6Lzf7BO2uUb ++KoDT6FZRI7vSZFqMmcqs/LEEPsBYtr0GKNmH3pcFHQ5HvfZdXkMILADHj0gxwZ0 +ng58SKQl8yU3B3wIoBOV+YEo8D+pLzlmH9XTRUl6sX0NvX1xeP/d +-----END RSA PRIVATE KEY-----`), + Cert: []byte(`-----BEGIN CERTIFICATE----- +MIICeDCCAh6gAwIBAgIRAPB+TCOgYy6vkH4CTz0UDdMwCgYIKoZIzj0EAwIwMzEx +MC8GA1UEAwwoTEVBUCBSb290IENBIChjbGllbnQgY2VydGlmaWNhdGVzIG9ubHkh +KTAeFw0yNDAyMjgyMTU4MTJaFw0yNDA0MDMyMTU4MTJaMBQxEjAQBgNVBAMTCVVO +TElNSVRFRDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKqa1phiSi4G +QRW3SVBDMyqSZFp1S6AC0k02aps1/L7kA+1tiR28KbYg/mu6gr4pKV/m7EW/W/OF +XLWGav7Zy8KkVXg36HoGvqRsLNChjKO+ClpVGdZPgIwm6sViT5WmCjZA1wCJ7HOm +pRL9PWW3rhhqVrUElaT/rLnYA3/SaSBLJ0y0iubdItUF/+d1Zba/qJiwqnedmtm1 +MSvFjSMLbErPZSxq6Sk1Sdcyg6VQTZjRYGoaZJF+SgJKEtE1cwSz8DTateBi503e +mJyeupvZOYolAWzC/NW5xN6m8Ruwv6qFyD+Hh9GO0QwQdHZWfD0wY3oNKZxLTxeT +K4JVUVXlIpkCAwEAAaNnMGUwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoGCCsG +AQUFBwMCMB0GA1UdDgQWBBSWz2IckC83grgDEuwOSfHtxBy3OTAfBgNVHSMEGDAW +gBR9SmLY/ytJxHm2orHcjj5jB1yo/jAKBggqhkjOPQQDAgNIADBFAiEA/J7Y0zfR +QxEBzQJEfSjT3Q9/cJkZJ11KwehMQYJTwGICIEMg3zaOg2XUlUg6jshTYx7S9xfE +vly8wNG42zeRWAXz +-----END CERTIFICATE-----`), }, } @@ -64,7 +118,7 @@ var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ // These base options are for the moment hardcoded. In the future we will want to be smarter // about getting information for different providers. func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint) (*vpnconfig.Config, error) { - // TODO(ainghazal): use options merge + // TODO(ainghazal): use options merge (pending PR) provider := endpoint.Provider // TODO(ainghazal): return error if provider unknown. we're in the happy path for now. baseOptions := defaultOptionsByProvider[provider] @@ -72,6 +126,7 @@ func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint) (*vpnconfig.Conf cfg := vpnconfig.NewConfig( vpnconfig.WithOpenVPNOptions( &vpnconfig.OpenVPNOptions{ + // endpoint-specific options. Remote: endpoint.IPAddr, Port: endpoint.Port, Proto: vpnconfig.Proto(endpoint.Transport), @@ -85,7 +140,8 @@ func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint) (*vpnconfig.Conf Auth: baseOptions.Auth, }, ), - config.WithHandshakeTracer(tracer)) - // TODO: validate here and return an error, maybe. + vpnconfig.WithHandshakeTracer(tracer)) + + // TODO: validate options here and return an error, maybe. return cfg, nil } diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 1215a6d3d3..936fe48884 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -148,12 +148,14 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, // it's a failure to start the measurement. we should abort return nil } - _, err = tunnel.Start(ctx, dialer, openvpnOptions) + tun, err := tunnel.Start(ctx, dialer, openvpnOptions) var failure string if err != nil { failure = err.Error() } + defer tun.Close() + handshakeEvents := handshakeTracer.Trace() port, _ := strconv.Atoi(endpoint.Port) From 87bbe21f532020cdb098281e2b0ffdf29fb78b99 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 7 Mar 2024 18:30:52 +0100 Subject: [PATCH 06/53] flatten test keys --- internal/experiment/openvpn/openvpn.go | 58 ++++++++++++++++---------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 936fe48884..3dc5547669 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -30,34 +30,62 @@ type Config struct { // TestKeys contains the experiment's result. type TestKeys struct { - Success bool `json:"success"` - Connections []*SingleConnection `json:"connections"` + AllSuccess bool `json:"success_all"` + AnySuccess bool `json:"success_any"` + TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` + OpenVPNHandshake []*ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` + NetworkEvents []*vpntracex.Event `json:"network_events"` } // NewTestKeys creates new openvpn TestKeys. func NewTestKeys() *TestKeys { return &TestKeys{ - Success: false, - Connections: []*SingleConnection{}, + AllSuccess: false, + AnySuccess: false, + TCPConnect: []*model.ArchivalTCPConnectResult{}, + OpenVPNHandshake: []*ArchivalOpenVPNHandshakeResult{}, + NetworkEvents: []*vpntracex.Event{}, } } +// SingleConnection contains the results of a single handshake. +type SingleConnection struct { + TCPConnect *model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` + OpenVPNHandshake *ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` + NetworkEvents []*vpntracex.Event `json:"network_events"` + // TODO(ainghazal): pass the transaction idx also to the event tracer for uniformity. + // TODO(ainghazal): make sure to document in the spec that these network events only cover the handshake. + // TODO(ainghazal): in the future, we will want to store more operations under this struct for a single connection, + // like pingResults or urlgetter calls. +} + // AddConnectionTestKeys adds the result of a single OpenVPN connection attempt to the // corresponding array in the [TestKeys] object. func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { - tk.Connections = append(tk.Connections, result) + tk.TCPConnect = append(tk.TCPConnect, result.TCPConnect) + tk.OpenVPNHandshake = append(tk.OpenVPNHandshake, result.OpenVPNHandshake) + tk.NetworkEvents = append(tk.NetworkEvents, result.NetworkEvents...) } // allConnectionsSuccessful returns true if all the registered connections have Status.Success equal to true. func (tk *TestKeys) allConnectionsSuccessful() bool { - for _, c := range tk.Connections { - if !c.OpenVPNHandshake.Status.Success { + for _, c := range tk.OpenVPNHandshake { + if !c.Status.Success { return false } } return true } +func (tk *TestKeys) anyConnectionSuccessful() bool { + for _, c := range tk.OpenVPNHandshake { + if !c.Status.Success { + return true + } + } + return false +} + // Measurer performs the measurement. type Measurer struct { config Config @@ -85,19 +113,6 @@ func (m Measurer) ExperimentVersion() string { // config.ReturnError field to true. var ErrFailure = errors.New("mocked error") -// SingleConnection contains the results of a single handshake. -type SingleConnection struct { - TCPConnect *model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` - OpenVPNHandshake *ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` - NetworkEvents []*vpntracex.Event `json:"network_events"` - // TODO(ainghazal): pass the transaction idx also to the event tracer for uniformity. - // TODO(ainghazal): make sure to document in the spec that these network events only cover the handshake. - // TODO(ainghazal): in the future, we will want to store more operations under this struct for a single connection, - // like pingResults or urlgetter calls. - - // TODO(ainghazal): look how to store the index that identifies each connection attempt. -} - // Run implements model.ExperimentMeasurer.Run. func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { callbacks := args.Callbacks @@ -114,7 +129,8 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { tk.AddConnectionTestKeys(connResult) } } - tk.Success = tk.allConnectionsSuccessful() + tk.AllSuccess = tk.allConnectionsSuccessful() + tk.AnySuccess = tk.anyConnectionSuccessful() callbacks.OnProgress(1.0, "All endpoints probed") measurement.TestKeys = tk From 8c2e23e7760da5f0bc88b98b767d623f94059666 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 7 Mar 2024 19:00:20 +0100 Subject: [PATCH 07/53] wip --- internal/experiment/openvpn/archival.go | 3 ++ internal/experiment/openvpn/endpoint.go | 37 +++++++++++++++++++------ internal/experiment/openvpn/openvpn.go | 30 ++++++++++++++------ 3 files changed, 54 insertions(+), 16 deletions(-) diff --git a/internal/experiment/openvpn/archival.go b/internal/experiment/openvpn/archival.go index 57d3f3e493..75b5096471 100644 --- a/internal/experiment/openvpn/archival.go +++ b/internal/experiment/openvpn/archival.go @@ -1,5 +1,7 @@ package openvpn +import "time" + // TODO(ainghazal): move to archiva package when consolidated. // ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake. @@ -10,6 +12,7 @@ type ArchivalOpenVPNHandshakeResult struct { Port int `json:"port"` Provider string `json:"provider"` Status ArchivalOpenVPNConnectStatus `json:"status"` + StartTime time.Time `json:"handshake_start_time"` T0 float64 `json:"t0,omitempty"` T float64 `json:"t"` Tags []string `json:"tags"` diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index d647df18d2..76ccad2357 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -2,6 +2,7 @@ package openvpn import ( "fmt" + "math/rand" vpnconfig "github.com/ooni/minivpn/pkg/config" vpntracex "github.com/ooni/minivpn/pkg/tracex" @@ -25,27 +26,47 @@ type endpoint struct { Transport string } +// String implements Stringer. This is a subset of the input URI scheme. func (e *endpoint) String() string { return fmt.Sprintf("%s://%s:%s/%s", e.Protocol, e.IPAddr, e.Port, e.Transport) } +// AsInput is a string representation of this endpoint. It contains more information than the endpoint itself. +func (e *endpoint) AsInput() string { + provider := e.Provider + if provider == "" { + provider = "unknown" + } + i := fmt.Sprintf("%s/?provider=%s", e.String(), provider) + return i +} + +type endpointList []endpoint + // allEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment. -var allEndpoints = []endpoint{ +var allEndpoints = endpointList{ { Provider: "riseup", - IPAddr: "185.220.103.11", + IPAddr: "51.15.187.53", Port: "1194", Protocol: "openvpn", Transport: "tcp", }, + { + Provider: "riseup", + IPAddr: "51.15.187.53", + Port: "1194", + Protocol: "openvpn", + Transport: "udp", + }, } -// sampleRandomEndpoint is a placeholder for a proper sampling function. -func sampleRandomEndpoint(all []endpoint) endpoint { - // chosen by fair dice roll - // guaranteed to be random - // https://xkcd.com/221/ - return all[0] +// Shuffle returns a shuffled copy of the endpointList. +func (e endpointList) Shuffle() endpointList { + rand.Shuffle(len(e), func(i, j int) { + e[i], e[j] = e[j], e[i] + }) + return e } var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 3dc5547669..41ddf78575 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -19,6 +19,12 @@ const ( openVPNProcol = "openvpn" ) +// +// The input URI is in the form: +// "openvpn://1.2.3.4:443/udp/&provider=tunnelbear&obfs=none" +// "openvpn+obfs4://1.2.3.4:443/tcp/&provider=riseup&obfs=obfs4&cert=deadbeef" +// + // Config contains the experiment config. // // This contains all the settings that user can set to modify the behaviour @@ -32,9 +38,10 @@ type Config struct { type TestKeys struct { AllSuccess bool `json:"success_all"` AnySuccess bool `json:"success_any"` + Inputs []string `json:"inputs"` + NetworkEvents []*vpntracex.Event `json:"network_events"` TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` OpenVPNHandshake []*ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` - NetworkEvents []*vpntracex.Event `json:"network_events"` } // NewTestKeys creates new openvpn TestKeys. @@ -42,9 +49,10 @@ func NewTestKeys() *TestKeys { return &TestKeys{ AllSuccess: false, AnySuccess: false, + Inputs: []string{}, + NetworkEvents: []*vpntracex.Event{}, TCPConnect: []*model.ArchivalTCPConnectResult{}, OpenVPNHandshake: []*ArchivalOpenVPNHandshakeResult{}, - NetworkEvents: []*vpntracex.Event{}, } } @@ -53,7 +61,6 @@ type SingleConnection struct { TCPConnect *model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` OpenVPNHandshake *ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` NetworkEvents []*vpntracex.Event `json:"network_events"` - // TODO(ainghazal): pass the transaction idx also to the event tracer for uniformity. // TODO(ainghazal): make sure to document in the spec that these network events only cover the handshake. // TODO(ainghazal): in the future, we will want to store more operations under this struct for a single connection, // like pingResults or urlgetter calls. @@ -62,12 +69,14 @@ type SingleConnection struct { // AddConnectionTestKeys adds the result of a single OpenVPN connection attempt to the // corresponding array in the [TestKeys] object. func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { - tk.TCPConnect = append(tk.TCPConnect, result.TCPConnect) + if result.TCPConnect != nil { + tk.TCPConnect = append(tk.TCPConnect, result.TCPConnect) + } tk.OpenVPNHandshake = append(tk.OpenVPNHandshake, result.OpenVPNHandshake) tk.NetworkEvents = append(tk.NetworkEvents, result.NetworkEvents...) } -// allConnectionsSuccessful returns true if all the registered connections have Status.Success equal to true. +// allConnectionsSuccessful returns true if all the registered handshakes have Status.Success equal to true. func (tk *TestKeys) allConnectionsSuccessful() bool { for _, c := range tk.OpenVPNHandshake { if !c.Status.Success { @@ -77,9 +86,10 @@ func (tk *TestKeys) allConnectionsSuccessful() bool { return true } +// anyConnectionSuccessful returns true if any of the registered handshakes have Status.Success equal to true. func (tk *TestKeys) anyConnectionSuccessful() bool { for _, c := range tk.OpenVPNHandshake { - if !c.Status.Success { + if c.Status.Success { return true } } @@ -121,8 +131,11 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { tk := NewTestKeys() - sess.Logger().Info("Starting to measure OpenVPN endpoints.") - for idx, endpoint := range allEndpoints { + // TODO(ainghazal): we could parallelize multiple probing. + for idx, endpoint := range allEndpoints.Shuffle() { + sess.Logger().Infof("Probing endpoint %s", endpoint.String()) + tk.Inputs = append(tk.Inputs, endpoint.AsInput()) + connResult := m.connectAndHandshake(ctx, int64(idx+1), time.Now(), sess.Logger(), endpoint) // TODO: better catch error here. if connResult != nil { @@ -199,6 +212,7 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, Failure: &failure, Success: err == nil, }, + StartTime: zeroTime, T0: tFirst, T: tLast, Tags: []string{}, From 54c9510b6f28a9eb48fa5b575a24367c1eb925fa Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 7 Mar 2024 19:42:27 +0100 Subject: [PATCH 08/53] wip --- internal/experiment/openvpn/archival.go | 19 +++++++---- internal/experiment/openvpn/endpoint.go | 43 +++++++++++++++++++------ 2 files changed, 47 insertions(+), 15 deletions(-) diff --git a/internal/experiment/openvpn/archival.go b/internal/experiment/openvpn/archival.go index 75b5096471..c1e0e1f561 100644 --- a/internal/experiment/openvpn/archival.go +++ b/internal/experiment/openvpn/archival.go @@ -2,7 +2,7 @@ package openvpn import "time" -// TODO(ainghazal): move to archiva package when consolidated. +// TODO(ainghazal): move to ooni internal archival package when consolidated. // ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake. type ArchivalOpenVPNHandshakeResult struct { @@ -13,10 +13,13 @@ type ArchivalOpenVPNHandshakeResult struct { Provider string `json:"provider"` Status ArchivalOpenVPNConnectStatus `json:"status"` StartTime time.Time `json:"handshake_start_time"` - T0 float64 `json:"t0,omitempty"` - T float64 `json:"t"` - Tags []string `json:"tags"` - TransactionID int64 `json:"transaction_id,omitempty"` + + // T0 can differ from StartTime because for TCP we take T0 *after* dialing has successfully completed. + // This might be counterintuitive, review. + T0 float64 `json:"t0,omitempty"` + T float64 `json:"t"` + Tags []string `json:"tags"` + TransactionID int64 `json:"transaction_id,omitempty"` } // ArchivalOpenVPNConnectStatus is the status of ArchivalOpenVPNConnectResult. @@ -27,7 +30,11 @@ type ArchivalOpenVPNConnectStatus struct { } type ArchivalNetworkEvent struct { - // TODO(ainghazal): need to properly propagate I/O errors during the handshake. + // TODO(ainghazal): could properly propagate I/O errors during the handshake. + // Right now the network events WILL NOT append any RW error, but I think following + // other experiments one would expect such errors to be appended as failures in the operation. + // However, getting the start boundary for the failed operation might be tricky, + // I need to think about it. Address string `json:"address,omitempty"` Failure *string `json:"failure"` NumBytes int64 `json:"num_bytes,omitempty"` diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 76ccad2357..61369390dc 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -9,10 +9,16 @@ import ( ) // endpoint is a single endpoint to be probed. +// The information contained in here is not generally not sufficient to complete a connection: +// we need more info, as cipher selection or obfuscating proxy credentials. type endpoint struct { // IPAddr is the IP Address for this endpoint. IPAddr string + // Obfuscation is any obfuscation method use to connect to this endpoint. + // Valid values are: obfs4, none. + Obfuscation string + // Port is the Port for this endpoint. Port string @@ -26,6 +32,17 @@ type endpoint struct { Transport string } +// newEndpointFromInputString constructs an endpoint after parsing an input string. +// +// The input URI is in the form: +// "openvpn://1.2.3.4:443/udp/&provider=tunnelbear&obfs=none" +// "openvpn+obfs4://1.2.3.4:443/tcp/&provider=riseup&obfs=obfs4&cert=deadbeef" +// TODO(ainghazal): implement +func newEndpointFromInputString(input string) endpoint { + fmt.Println("should parse:", input) + return endpoint{} +} + // String implements Stringer. This is a subset of the input URI scheme. func (e *endpoint) String() string { return fmt.Sprintf("%s://%s:%s/%s", e.Protocol, e.IPAddr, e.Port, e.Transport) @@ -37,13 +54,21 @@ func (e *endpoint) AsInput() string { if provider == "" { provider = "unknown" } - i := fmt.Sprintf("%s/?provider=%s", e.String(), provider) + obfs := e.Obfuscation + if obfs == "" { + obfs = "none" + } + i := fmt.Sprintf("%s/?provider=%s&obfs=%s", e.String(), provider, obfs) return i } +// endpointList is a list of endpoints. type endpointList []endpoint // allEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment. +// This is a hardcoded list for now, but the idea is that we can receive this from the check-in api in the future. +// In any case, having hardcoded endpoints is good as a fallback for the cases in which we cannot contact +// OONI's backend. var allEndpoints = endpointList{ { Provider: "riseup", @@ -69,6 +94,11 @@ func (e endpointList) Shuffle() endpointList { return e } +// TODO(ainghazal): this is extremely hacky, but it's a first step +// until we manage to have the check-in API handing credentials. +// Do note that these certificates will expire ca. Apr 6 2024 +// OTOH, yes, I do understand the risks of exposing key material +// on a public git repo. Thanks for caring. var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ "riseup": { Auth: "SHA512", @@ -83,11 +113,6 @@ ATAdBgNVHQ4EFgQUZdoUlJrCIUNFrpffAq+LQjnwEz4wCgYIKoZIzj0EAwIDSAAw RQIgfr3w4tnRG+NdI3LsGPlsRktGK20xHTzsB3orB0yC6cICIQCB+/9y8nmSStfN VUMUyk2hNd7/kC8nL222TTD7VZUtsg== -----END CERTIFICATE-----`), - // TODO(ainghazal): this is extremely hacky, but it's a first step - // until we manage to have the check-in API handing credentials. - // Do note that these certificates will expire ca. Apr 6 2024 - // OTOH, yes, I do understand the risks of exposing key material - // on a public git repo. Thanks for caring. Key: []byte(`-----BEGIN RSA PRIVATE KEY----- MIIEowIBAAKCAQEAqprWmGJKLgZBFbdJUEMzKpJkWnVLoALSTTZqmzX8vuQD7W2J HbwptiD+a7qCvikpX+bsRb9b84VctYZq/tnLwqRVeDfoega+pGws0KGMo74KWlUZ @@ -152,8 +177,8 @@ func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint) (*vpnconfig.Conf Port: endpoint.Port, Proto: vpnconfig.Proto(endpoint.Transport), - // options coming from the default values, - // to be changed by check-in API info in the future. + // options coming from the default values for the provider, + // to switch to values provided by the check-in API in the future. CA: baseOptions.CA, Cert: baseOptions.Cert, Key: baseOptions.Key, @@ -163,6 +188,6 @@ func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint) (*vpnconfig.Conf ), vpnconfig.WithHandshakeTracer(tracer)) - // TODO: validate options here and return an error, maybe. + // TODO: validate options here and return an error. return cfg, nil } From 1bc0764cd324c7f7e3c39d82ec7be3b52f15a8dd Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 7 Mar 2024 20:39:20 +0100 Subject: [PATCH 09/53] include openvpn options in handshake result --- internal/experiment/openvpn/archival.go | 23 ++++++++++++++++------- internal/experiment/openvpn/endpoint.go | 18 ++++++++++-------- internal/experiment/openvpn/openvpn.go | 21 ++++++++++----------- 3 files changed, 36 insertions(+), 26 deletions(-) diff --git a/internal/experiment/openvpn/archival.go b/internal/experiment/openvpn/archival.go index c1e0e1f561..939d15c63e 100644 --- a/internal/experiment/openvpn/archival.go +++ b/internal/experiment/openvpn/archival.go @@ -4,15 +4,24 @@ import "time" // TODO(ainghazal): move to ooni internal archival package when consolidated. +// OpenVPNOptions is a subset of [vpnconfig.OpenVPNOptions] that we want to include +// in the archived result. +type OpenVPNOptions struct { + Auth string `json:"auth,omitempty"` + Cipher string `json:"cipher,omitempty"` + Compression string `json:"compression,omitempty"` +} + // ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake. type ArchivalOpenVPNHandshakeResult struct { - BootstrapTime float64 `json:"bootstrap_time,omitempty"` - Endpoint string `json:"endpoint"` - IP string `json:"ip"` - Port int `json:"port"` - Provider string `json:"provider"` - Status ArchivalOpenVPNConnectStatus `json:"status"` - StartTime time.Time `json:"handshake_start_time"` + BootstrapTime float64 `json:"bootstrap_time,omitempty"` + Endpoint string `json:"endpoint"` + IP string `json:"ip"` + OpenVPNOptions OpenVPNOptions `json:"openvpn_options"` + Port int `json:"port"` + Provider string `json:"provider"` + Status ArchivalOpenVPNConnectStatus `json:"status"` + StartTime time.Time `json:"handshake_start_time"` // T0 can differ from StartTime because for TCP we take T0 *after* dialing has successfully completed. // This might be counterintuitive, review. diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 61369390dc..b5e0ffcab9 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -45,20 +45,22 @@ func newEndpointFromInputString(input string) endpoint { // String implements Stringer. This is a subset of the input URI scheme. func (e *endpoint) String() string { - return fmt.Sprintf("%s://%s:%s/%s", e.Protocol, e.IPAddr, e.Port, e.Transport) + var proto string + if e.Obfuscation == "obfs4" { + proto = e.Protocol + "+obfs4" + } else { + proto = e.Protocol + } + return fmt.Sprintf("%s://%s:%s/%s", proto, e.IPAddr, e.Port, e.Transport) } -// AsInput is a string representation of this endpoint. It contains more information than the endpoint itself. -func (e *endpoint) AsInput() string { +// AsInputURI is a string representation of this endpoint. It contains more information than the endpoint itself. +func (e *endpoint) AsInputURI() string { provider := e.Provider if provider == "" { provider = "unknown" } - obfs := e.Obfuscation - if obfs == "" { - obfs = "none" - } - i := fmt.Sprintf("%s/?provider=%s&obfs=%s", e.String(), provider, obfs) + i := fmt.Sprintf("%s/?provider=%s", e.String(), provider) return i } diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 41ddf78575..49db66c151 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -19,12 +19,6 @@ const ( openVPNProcol = "openvpn" ) -// -// The input URI is in the form: -// "openvpn://1.2.3.4:443/udp/&provider=tunnelbear&obfs=none" -// "openvpn+obfs4://1.2.3.4:443/tcp/&provider=riseup&obfs=obfs4&cert=deadbeef" -// - // Config contains the experiment config. // // This contains all the settings that user can set to modify the behaviour @@ -134,7 +128,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { // TODO(ainghazal): we could parallelize multiple probing. for idx, endpoint := range allEndpoints.Shuffle() { sess.Logger().Infof("Probing endpoint %s", endpoint.String()) - tk.Inputs = append(tk.Inputs, endpoint.AsInput()) + tk.Inputs = append(tk.Inputs, endpoint.AsInputURI()) connResult := m.connectAndHandshake(ctx, int64(idx+1), time.Now(), sess.Logger(), endpoint) // TODO: better catch error here. @@ -171,13 +165,13 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, // create a vpn tun Device that attempts to dial and performs the handshake handshakeTracer := vpntracex.NewTracerWithTransactionID(zeroTime, index) - openvpnOptions, err := getVPNConfig(handshakeTracer, &endpoint) + openvpnConfig, err := getVPNConfig(handshakeTracer, &endpoint) if err != nil { // TODO: find a better way to return the error - this is not a test failure, // it's a failure to start the measurement. we should abort return nil } - tun, err := tunnel.Start(ctx, dialer, openvpnOptions) + tun, err := tunnel.Start(ctx, dialer, openvpnConfig) var failure string if err != nil { @@ -206,8 +200,13 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, BootstrapTime: bootstrapTime, Endpoint: endpoint.String(), IP: endpoint.IPAddr, - Port: port, - Provider: endpoint.Provider, + OpenVPNOptions: OpenVPNOptions{ + Cipher: openvpnConfig.OpenVPNOptions().Cipher, + Auth: openvpnConfig.OpenVPNOptions().Auth, + Compression: string(openvpnConfig.OpenVPNOptions().Compress), + }, + Port: port, + Provider: endpoint.Provider, Status: ArchivalOpenVPNConnectStatus{ Failure: &failure, Success: err == nil, From 567ed14737551ec87ba6ab87e819f8c94b97ad7f Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 7 Mar 2024 22:42:15 +0100 Subject: [PATCH 10/53] add fields to archival struct for handshake result --- internal/experiment/openvpn/archival.go | 3 ++- internal/experiment/openvpn/openvpn.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/experiment/openvpn/archival.go b/internal/experiment/openvpn/archival.go index 939d15c63e..54e261a46e 100644 --- a/internal/experiment/openvpn/archival.go +++ b/internal/experiment/openvpn/archival.go @@ -17,9 +17,10 @@ type ArchivalOpenVPNHandshakeResult struct { BootstrapTime float64 `json:"bootstrap_time,omitempty"` Endpoint string `json:"endpoint"` IP string `json:"ip"` - OpenVPNOptions OpenVPNOptions `json:"openvpn_options"` Port int `json:"port"` + Transport string `json:"transport"` Provider string `json:"provider"` + OpenVPNOptions OpenVPNOptions `json:"openvpn_options"` Status ArchivalOpenVPNConnectStatus `json:"status"` StartTime time.Time `json:"handshake_start_time"` diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 49db66c151..8d342d035b 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -200,13 +200,14 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, BootstrapTime: bootstrapTime, Endpoint: endpoint.String(), IP: endpoint.IPAddr, + Port: port, + Transport: endpoint.Transport, + Provider: endpoint.Provider, OpenVPNOptions: OpenVPNOptions{ Cipher: openvpnConfig.OpenVPNOptions().Cipher, Auth: openvpnConfig.OpenVPNOptions().Auth, Compression: string(openvpnConfig.OpenVPNOptions().Compress), }, - Port: port, - Provider: endpoint.Provider, Status: ArchivalOpenVPNConnectStatus{ Failure: &failure, Success: err == nil, From 5974d036d4e4a9e7c3fa4e0b522a98150c54f97b Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Fri, 15 Mar 2024 15:20:40 +0100 Subject: [PATCH 11/53] wip: input --- internal/experiment/openvpn/endpoint.go | 131 ++++++++++++++++++++++-- internal/experiment/openvpn/openvpn.go | 46 ++++++++- internal/registry/openvpn.go | 5 +- 3 files changed, 166 insertions(+), 16 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index b5e0ffcab9..f085974753 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -1,13 +1,21 @@ package openvpn import ( + "encoding/base64" + "errors" "fmt" "math/rand" + "net/url" + "strings" vpnconfig "github.com/ooni/minivpn/pkg/config" vpntracex "github.com/ooni/minivpn/pkg/tracex" ) +var ( + ErrBadBase64Blob = errors.New("wrong base64 encoding") +) + // endpoint is a single endpoint to be probed. // The information contained in here is not generally not sufficient to complete a connection: // we need more info, as cipher selection or obfuscating proxy credentials. @@ -35,12 +43,64 @@ type endpoint struct { // newEndpointFromInputString constructs an endpoint after parsing an input string. // // The input URI is in the form: -// "openvpn://1.2.3.4:443/udp/&provider=tunnelbear&obfs=none" -// "openvpn+obfs4://1.2.3.4:443/tcp/&provider=riseup&obfs=obfs4&cert=deadbeef" -// TODO(ainghazal): implement -func newEndpointFromInputString(input string) endpoint { - fmt.Println("should parse:", input) - return endpoint{} +// "openvpn://1.2.3.4:443/udp/&provider=tunnelbear" +// "openvpn+obfs4://1.2.3.4:443/tcp/&provider=riseup&cert=deadbeef" +func newEndpointFromInputString(uri string) (*endpoint, error) { + parsedURL, err := url.Parse(uri) + if err != nil { + return nil, fmt.Errorf("%w: %s", ErrInvalidInput, err) + } + var obfuscation string + switch parsedURL.Scheme { + case "openvpn": + obfuscation = "openvpn" + case "openvpn+obfs4": + obfuscation = "obfs4" + default: + return nil, fmt.Errorf("%w: unknown scheme: %s", ErrInvalidInput, parsedURL.Scheme) + } + + host := parsedURL.Hostname() + if host == "" { + return nil, fmt.Errorf("%w: expected host: %s", ErrInvalidInput, parsedURL.Host) + } + + port := parsedURL.Port() + if port == "" { + return nil, fmt.Errorf("%w: expected port: %s", ErrInvalidInput, parsedURL.Port()) + } + + pathParts := strings.Split(parsedURL.Path, "/") + if len(pathParts) != 3 { + return nil, fmt.Errorf("%w: invalid path: %s (%d)", ErrInvalidInput, pathParts, len(pathParts)) + } + transport := pathParts[1] + if transport != "tcp" && transport != "udp" { + return nil, fmt.Errorf("%w: invalid transport: %s", ErrInvalidInput, transport) + } + + params := parsedURL.Query() + provider := params.Get("provider") + + if provider == "" { + return nil, fmt.Errorf("%w: please specify a provider as part of the input", ErrInvalidInput) + } + + if provider != "riseup" { + // because we are hardcoding at the moment. figure out a way to pass info for + // arbitrary providers as options instead + return nil, fmt.Errorf("%w: unknown provider: %s", ErrInvalidInput, provider) + } + + endpoint := &endpoint{ + IPAddr: host, + Obfuscation: obfuscation, + Port: port, + Protocol: "openvpn", + Provider: provider, + Transport: transport, + } + return endpoint, nil } // String implements Stringer. This is a subset of the input URI scheme. @@ -65,7 +125,7 @@ func (e *endpoint) AsInputURI() string { } // endpointList is a list of endpoints. -type endpointList []endpoint +type endpointList []*endpoint // allEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment. // This is a hardcoded list for now, but the idea is that we can receive this from the check-in api in the future. @@ -96,6 +156,15 @@ func (e endpointList) Shuffle() endpointList { return e } +func isValidProvider(provider string) bool { + switch provider { + case "riseup": + return true + default: + return false + } +} + // TODO(ainghazal): this is extremely hacky, but it's a first step // until we manage to have the check-in API handing credentials. // Do note that these certificates will expire ca. Apr 6 2024 @@ -165,12 +234,41 @@ vly8wNG42zeRWAXz // To obtain that, we merge the endpoint specific configuration with base options. // These base options are for the moment hardcoded. In the future we will want to be smarter // about getting information for different providers. -func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint) (*vpnconfig.Config, error) { +func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, experimentConfig *Config) (*vpnconfig.Config, error) { // TODO(ainghazal): use options merge (pending PR) + provider := endpoint.Provider - // TODO(ainghazal): return error if provider unknown. we're in the happy path for now. + if !isValidProvider(provider) { + return nil, fmt.Errorf("%w: unknown provider: %s", ErrInvalidInput, provider) + } + baseOptions := defaultOptionsByProvider[provider] + // We override any provider related options found in the config + if experimentConfig.SafeCA != "" { + ca, err := extractBase64Blob(experimentConfig.SafeCA) + if err != nil { + return nil, err + } + baseOptions.CA = []byte(ca) + } + + if experimentConfig.SafeKey != "" { + key, err := extractBase64Blob(experimentConfig.SafeKey) + if err != nil { + return nil, err + } + baseOptions.Key = []byte(key) + } + + if experimentConfig.SafeCert != "" { + cert, err := extractBase64Blob(experimentConfig.SafeCert) + if err != nil { + return nil, err + } + baseOptions.Key = []byte(cert) + } + cfg := vpnconfig.NewConfig( vpnconfig.WithOpenVPNOptions( &vpnconfig.OpenVPNOptions{ @@ -193,3 +291,18 @@ func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint) (*vpnconfig.Conf // TODO: validate options here and return an error. return cfg, nil } + +func extractBase64Blob(val string) (string, error) { + s := strings.TrimPrefix(val, "base64:") + if len(s) == len(val) { + return "", fmt.Errorf("%w: %s", ErrBadBase64Blob, "missing prefix") + } + dec, err := base64.URLEncoding.DecodeString(strings.TrimSpace(s)) + if err != nil { + return "", fmt.Errorf("%w: %s", ErrBadBase64Blob, err) + } + if len(dec) == 0 { + return "", nil + } + return string(dec), nil +} diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 8d342d035b..3c3f99a94c 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -5,6 +5,7 @@ import ( "context" "errors" "strconv" + "strings" "time" "github.com/ooni/probe-cli/v3/internal/measurexlite" @@ -25,7 +26,10 @@ 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 { - Provider string + 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"` + SafeCA string `ooni:"ca to connect to the OpenVPN endpoint"` } // TestKeys contains the experiment's result. @@ -113,6 +117,24 @@ func (m Measurer) ExperimentVersion() string { return testVersion } +var ( + ErrInvalidInput = errors.New("invalid input") +) + +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) + } + return endpoints, nil + +} + // ErrFailure is the error returned when you set the // config.ReturnError field to true. var ErrFailure = errors.New("mocked error") @@ -123,10 +145,25 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { measurement := args.Measurement sess := args.Session + var endpoints endpointList + var err error + + if measurement.Input == "" { + // if input is null, we get the hardcoded list of inputs. + endpoints = allEndpoints + } 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 + } + } + tk := NewTestKeys() // TODO(ainghazal): we could parallelize multiple probing. - for idx, endpoint := range allEndpoints.Shuffle() { + for idx, endpoint := range endpoints.Shuffle() { sess.Logger().Infof("Probing endpoint %s", endpoint.String()) tk.Inputs = append(tk.Inputs, endpoint.AsInputURI()) @@ -143,6 +180,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { 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 @@ -154,7 +192,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { // connectAndHandshake dials a connection and attempts an OpenVPN handshake using that dialer. func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, - zeroTime time.Time, logger model.Logger, endpoint endpoint) *SingleConnection { + zeroTime time.Time, logger model.Logger, endpoint *endpoint) *SingleConnection { // create a trace for the network dialer trace := measurexlite.NewTrace(index, zeroTime) @@ -165,7 +203,7 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, // create a vpn tun Device that attempts to dial and performs the handshake handshakeTracer := vpntracex.NewTracerWithTransactionID(zeroTime, index) - openvpnConfig, err := getVPNConfig(handshakeTracer, &endpoint) + openvpnConfig, err := getVPNConfig(handshakeTracer, endpoint, &m.config) if err != nil { // TODO: find a better way to return the error - this is not a test failure, // it's a failure to start the measurement. we should abort diff --git a/internal/registry/openvpn.go b/internal/registry/openvpn.go index c060fbf1e5..2a0f6b1e7f 100644 --- a/internal/registry/openvpn.go +++ b/internal/registry/openvpn.go @@ -16,10 +16,9 @@ func init() { *config.(*openvpn.Config), "openvpn", ) }, - // TODO(ainghazal): we can pass an array of providers here. config: &openvpn.Config{}, - enabledByDefault: true, + enabledByDefault: false, interruptible: true, - inputPolicy: model.InputNone, + inputPolicy: model.InputOptional, } } From 0852d1a9278d4b85753a384b45f4e9a172e5662c Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 19 Mar 2024 20:51:21 +0100 Subject: [PATCH 12/53] api call to fetch credentials --- internal/engine/session.go | 10 ++ internal/experiment/openvpn/endpoint.go | 116 +++--------------- internal/experiment/openvpn/openvpn.go | 152 +++++++++++++++++------- internal/model/experiment.go | 3 + internal/model/ooapi.go | 22 ++++ internal/probeservices/openvpn.go | 24 ++++ 6 files changed, 188 insertions(+), 139 deletions(-) create mode 100644 internal/probeservices/openvpn.go diff --git a/internal/engine/session.go b/internal/engine/session.go index 3539c1c5e5..ffff4d6b3a 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -371,6 +371,16 @@ func (s *Session) FetchTorTargets( return clnt.FetchTorTargets(ctx, cc) } +// FetchOpenVPNConfig fetches openvpn config from the API. +func (s *Session) FetchOpenVPNConfig( + ctx context.Context, cc string) (map[string]model.OOAPIOpenVPNConfig, error) { + clnt, err := s.NewOrchestraClient(ctx) + if err != nil { + return nil, err + } + return clnt.FetchOpenVPNConfig(ctx, cc) +} + // KeyValueStore returns the configured key-value store. func (s *Session) KeyValueStore() model.KeyValueStore { return s.kvStore diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index f085974753..4fafb6a86a 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -156,86 +156,28 @@ func (e endpointList) Shuffle() endpointList { return e } -func isValidProvider(provider string) bool { - switch provider { - case "riseup": - return true - default: - return false - } -} - -// TODO(ainghazal): this is extremely hacky, but it's a first step -// until we manage to have the check-in API handing credentials. -// Do note that these certificates will expire ca. Apr 6 2024 -// OTOH, yes, I do understand the risks of exposing key material -// on a public git repo. Thanks for caring. +// 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{ "riseup": { Auth: "SHA512", Cipher: "AES-256-GCM", - CA: []byte(`-----BEGIN CERTIFICATE----- -MIIBYjCCAQigAwIBAgIBATAKBggqhkjOPQQDAjAXMRUwEwYDVQQDEwxMRUFQIFJv -b3QgQ0EwHhcNMjExMTAyMTkwNTM3WhcNMjYxMTAyMTkxMDM3WjAXMRUwEwYDVQQD -EwxMRUFQIFJvb3QgQ0EwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQxOXBGu+gf -pjHzVteGTWL6XnFxtEnKMFpKaJkA/VOHmESzoLsZRQxt88GssxaqC01J17idQiqv -zgNpedmtvFtyo0UwQzAOBgNVHQ8BAf8EBAMCAqQwEgYDVR0TAQH/BAgwBgEB/wIB -ATAdBgNVHQ4EFgQUZdoUlJrCIUNFrpffAq+LQjnwEz4wCgYIKoZIzj0EAwIDSAAw -RQIgfr3w4tnRG+NdI3LsGPlsRktGK20xHTzsB3orB0yC6cICIQCB+/9y8nmSStfN -VUMUyk2hNd7/kC8nL222TTD7VZUtsg== ------END CERTIFICATE-----`), - Key: []byte(`-----BEGIN RSA PRIVATE KEY----- -MIIEowIBAAKCAQEAqprWmGJKLgZBFbdJUEMzKpJkWnVLoALSTTZqmzX8vuQD7W2J -HbwptiD+a7qCvikpX+bsRb9b84VctYZq/tnLwqRVeDfoega+pGws0KGMo74KWlUZ -1k+AjCbqxWJPlaYKNkDXAInsc6alEv09ZbeuGGpWtQSVpP+sudgDf9JpIEsnTLSK -5t0i1QX/53Vltr+omLCqd52a2bUxK8WNIwtsSs9lLGrpKTVJ1zKDpVBNmNFgahpk -kX5KAkoS0TVzBLPwNNq14GLnTd6YnJ66m9k5iiUBbML81bnE3qbxG7C/qoXIP4eH -0Y7RDBB0dlZ8PTBjeg0pnEtPF5MrglVRVeUimQIDAQABAoIBAGVgMspEBa5Jmx0r -V44xEFNov+ccsf54Dr1A66IlN3W7CjZok0SvDd4ixuv+3TfgP6y0DIv5hMs04P0g -za14f+K+Qed42VTBc0FC4nJqvKaEA6Tf0sWNYmZlrbXykDXtfz3z046HZpDmYkrh -Xj12IyZw8esIuV9daibYnGO1BTDhXy/B53zDjx6wYMDC3DFVa2gLSRWONtMnCYY8 -Hw7FbaP1Jxs6sNS/AKVZZo4SyBL1te80HN9Wo2syDmdc3o3aBMkCY9+u560dj8+5 -4xvn+d8ojp91Ts3o33DB6PY88r2UTg00ejGMn8LN7dCnZDO2mQ98nczKfcpYL0nW -CKxG6AECgYEA0L9XBdfps60nKNS5n4+rNvtYvhkvHOKkz7wFmYSo6r8M1ID3m7g3 -x6wwTY9MrlSPPsF9x6GnrmGIGIZsc8lNRuYFq/yemNhKfMi6KU9wnjqVQYDSg9S2 -fq4lutPxbeiQmSx5WYtjeaJXzTAzx9jT6t8QiAUXM06QgPPjLK7G+ZkCgYEA0Tku -iSz8Y2uHyBWOYFTIaEvvyCEJqyZ+hMgVRRgN7QzDjP4VUVmQClwdK7JIPNBaIf6V -Gvi+CXgb/oDMrcduMM4ZGoVN1ttpC3htn7qn35+38VsYPD3hgmF7r3WFSxoBd0vj -Yh7rO4tQo91tm0DkCs+NZvNRrFr1yL/VAHnDEQECgYAi3XJpdXCBJBCAT1dZgSN1 -oXFm/snRp0EjuSGuTGvyGUrJS2kPxyr53JaMvbxu+YybTLH3X9aj14Jlpj4C8MJJ -by3PVfgfSzDVuqjtMWl75Aj90chXYHXCns+Kbs/KLafJDZaPECrjK+xCRyS+4kYy -2mLmdQM0/JBCGXn+AosVMQKBgEFgy/DjlM6AaIKWcdIaTDGDIR95a2sG8VwOpc7c -cGWVqnmhYAn2obMLC7Z+1GHkfXXH9tHhzoho9t51YwAepIktrdyCsUslbtK9xAu4 -qQKRB0qtO4p/j7tNOPggEhHgw3qCxUABB2Ko6v75j2mHQns6ViZIfEoOdmVPxICM -i+8BAoGBAIn2RfbrcbrH3jZKNkklUMg7fU8K0w7PQ7kCR8vql4chA6Lzf7BO2uUb -+KoDT6FZRI7vSZFqMmcqs/LEEPsBYtr0GKNmH3pcFHQ5HvfZdXkMILADHj0gxwZ0 -ng58SKQl8yU3B3wIoBOV+YEo8D+pLzlmH9XTRUl6sX0NvX1xeP/d ------END RSA PRIVATE KEY-----`), - Cert: []byte(`-----BEGIN CERTIFICATE----- -MIICeDCCAh6gAwIBAgIRAPB+TCOgYy6vkH4CTz0UDdMwCgYIKoZIzj0EAwIwMzEx -MC8GA1UEAwwoTEVBUCBSb290IENBIChjbGllbnQgY2VydGlmaWNhdGVzIG9ubHkh -KTAeFw0yNDAyMjgyMTU4MTJaFw0yNDA0MDMyMTU4MTJaMBQxEjAQBgNVBAMTCVVO -TElNSVRFRDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKqa1phiSi4G -QRW3SVBDMyqSZFp1S6AC0k02aps1/L7kA+1tiR28KbYg/mu6gr4pKV/m7EW/W/OF -XLWGav7Zy8KkVXg36HoGvqRsLNChjKO+ClpVGdZPgIwm6sViT5WmCjZA1wCJ7HOm -pRL9PWW3rhhqVrUElaT/rLnYA3/SaSBLJ0y0iubdItUF/+d1Zba/qJiwqnedmtm1 -MSvFjSMLbErPZSxq6Sk1Sdcyg6VQTZjRYGoaZJF+SgJKEtE1cwSz8DTateBi503e -mJyeupvZOYolAWzC/NW5xN6m8Ruwv6qFyD+Hh9GO0QwQdHZWfD0wY3oNKZxLTxeT -K4JVUVXlIpkCAwEAAaNnMGUwDgYDVR0PAQH/BAQDAgeAMBMGA1UdJQQMMAoGCCsG -AQUFBwMCMB0GA1UdDgQWBBSWz2IckC83grgDEuwOSfHtxBy3OTAfBgNVHSMEGDAW -gBR9SmLY/ytJxHm2orHcjj5jB1yo/jAKBggqhkjOPQQDAgNIADBFAiEA/J7Y0zfR -QxEBzQJEfSjT3Q9/cJkZJ11KwehMQYJTwGICIEMg3zaOg2XUlUg6jshTYx7S9xfE -vly8wNG42zeRWAXz ------END CERTIFICATE-----`), }, } +func isValidProvider(provider string) bool { + _, ok := defaultOptionsByProvider[provider] + return ok +} + // getVPNConfig gets a properly configured [*vpnconfig.Config] object for the given endpoint. // To obtain that, we merge the endpoint specific configuration with base options. // These base options are for the moment hardcoded. In the future we will want to be smarter // about getting information for different providers. -func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, experimentConfig *Config) (*vpnconfig.Config, error) { - // TODO(ainghazal): use options merge (pending PR) +func getVPNConfig(tracer *vpntracex.Tracer, 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) { @@ -244,31 +186,6 @@ func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, experimentConfig baseOptions := defaultOptionsByProvider[provider] - // We override any provider related options found in the config - if experimentConfig.SafeCA != "" { - ca, err := extractBase64Blob(experimentConfig.SafeCA) - if err != nil { - return nil, err - } - baseOptions.CA = []byte(ca) - } - - if experimentConfig.SafeKey != "" { - key, err := extractBase64Blob(experimentConfig.SafeKey) - if err != nil { - return nil, err - } - baseOptions.Key = []byte(key) - } - - if experimentConfig.SafeCert != "" { - cert, err := extractBase64Blob(experimentConfig.SafeCert) - if err != nil { - return nil, err - } - baseOptions.Key = []byte(cert) - } - cfg := vpnconfig.NewConfig( vpnconfig.WithOpenVPNOptions( &vpnconfig.OpenVPNOptions{ @@ -277,13 +194,14 @@ func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, experimentConfig Port: endpoint.Port, Proto: vpnconfig.Proto(endpoint.Transport), - // options coming from the default values for the provider, - // to switch to values provided by the check-in API in the future. - CA: baseOptions.CA, - Cert: baseOptions.Cert, - Key: baseOptions.Key, + // 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, }, ), vpnconfig.WithHandshakeTracer(tracer)) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 3c3f99a94c..c796e444ac 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -4,6 +4,7 @@ package openvpn import ( "context" "errors" + "fmt" "strconv" "strings" "time" @@ -11,6 +12,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/measurexlite" "github.com/ooni/probe-cli/v3/internal/model" + vpnconfig "github.com/ooni/minivpn/pkg/config" vpntracex "github.com/ooni/minivpn/pkg/tracex" "github.com/ooni/minivpn/pkg/tunnel" ) @@ -34,9 +36,7 @@ type Config struct { // TestKeys contains the experiment's result. type TestKeys struct { - AllSuccess bool `json:"success_all"` - AnySuccess bool `json:"success_any"` - Inputs []string `json:"inputs"` + Success bool `json:"success"` NetworkEvents []*vpntracex.Event `json:"network_events"` TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` OpenVPNHandshake []*ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` @@ -45,9 +45,7 @@ type TestKeys struct { // NewTestKeys creates new openvpn TestKeys. func NewTestKeys() *TestKeys { return &TestKeys{ - AllSuccess: false, - AnySuccess: false, - Inputs: []string{}, + Success: true, NetworkEvents: []*vpntracex.Event{}, TCPConnect: []*model.ArchivalTCPConnectResult{}, OpenVPNHandshake: []*ArchivalOpenVPNHandshakeResult{}, @@ -84,16 +82,6 @@ func (tk *TestKeys) allConnectionsSuccessful() bool { return true } -// anyConnectionSuccessful returns true if any of the registered handshakes have Status.Success equal to true. -func (tk *TestKeys) anyConnectionSuccessful() bool { - for _, c := range tk.OpenVPNHandshake { - if c.Status.Success { - return true - } - } - return false -} - // Measurer performs the measurement. type Measurer struct { config Config @@ -121,6 +109,8 @@ 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, ",") @@ -132,7 +122,6 @@ func parseListOfInputs(inputs string) (endpointList, error) { endpoints = append(endpoints, e) } return endpoints, nil - } // ErrFailure is the error returned when you set the @@ -140,41 +129,46 @@ func parseListOfInputs(inputs string) (endpointList, error) { var ErrFailure = errors.New("mocked error") // Run implements model.ExperimentMeasurer.Run. +// A single run expects exactly ONE input (endpoint). func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { callbacks := args.Callbacks measurement := args.Measurement sess := args.Session - var endpoints endpointList - var err error + var endpoint *endpoint if measurement.Input == "" { - // if input is null, we get the hardcoded list of inputs. - endpoints = allEndpoints + // if input is null, we get one from the hardcoded list of inputs. + sess.Logger().Info("No input given, picking one endpoint at random") + endpoint = allEndpoints.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)) + 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() - // TODO(ainghazal): we could parallelize multiple probing. - for idx, endpoint := range endpoints.Shuffle() { - sess.Logger().Infof("Probing endpoint %s", endpoint.String()) - tk.Inputs = append(tk.Inputs, endpoint.AsInputURI()) + sess.Logger().Infof("Probing endpoint %s", endpoint.String()) - connResult := m.connectAndHandshake(ctx, int64(idx+1), time.Now(), sess.Logger(), endpoint) - // TODO: better catch error here. - if connResult != nil { - tk.AddConnectionTestKeys(connResult) - } + // TODO: separate pre-connection checks + connResult, err := m.connectAndHandshake(ctx, int64(1), time.Now(), sess, endpoint) + if err != nil { + sess.Logger().Warn("Fatal error while attempting to connect to endpoint, aborting!") + return err } - tk.AllSuccess = tk.allConnectionsSuccessful() - tk.AnySuccess = tk.anyConnectionSuccessful() + if connResult != nil { + tk.AddConnectionTestKeys(connResult) + } + tk.Success = tk.allConnectionsSuccessful() callbacks.OnProgress(1.0, "All endpoints probed") measurement.TestKeys = tk @@ -190,9 +184,78 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { return nil } +// getCredentialsFromOptionsOrAPI attempts to find valid credentials for the given provider, either +// from the passed Options (cli, oonirun), or from a remote call to the OONI API endpoint. +func (m *Measurer) getCredentialsFromOptionsOrAPI( + ctx context.Context, + sess model.ExperimentSession, + provider string) (*vpnconfig.OpenVPNOptions, error) { + // TODO(ainghazal): Ideally, we need to know which authentication methods each provider uses, and this is + // information that the experiment could hardcode. Sticking to Certificate-based auth for riseupvpn. + + // get an empty options object to fill with credentials + creds := &vpnconfig.OpenVPNOptions{} + + cfg := m.config + + if cfg.SafeCA != "" && cfg.SafeCert != "" && cfg.SafeKey != "" { + // We override authentication info with what user provided in options. + ca, err := extractBase64Blob(cfg.SafeCA) + if err != nil { + return nil, err + } + creds.CA = []byte(ca) + + key, err := extractBase64Blob(cfg.SafeKey) + if err != nil { + return nil, err + } + creds.Key = []byte(key) + + cert, err := extractBase64Blob(cfg.SafeCert) + if err != nil { + return nil, err + } + creds.Key = []byte(cert) + + // return options-based credentials + return creds, nil + } + + // No options passed, let's hit OONI API for credential distribution. + // TODO(ainghazal): cache credentials fetch? + configFromAPI, err := m.fetchProviderCredentials(ctx, sess) + if err != nil { + sess.Logger().Warnf("Error fetching credentials from API: %s", err.Error()) + return nil, err + } + apiCreds, ok := configFromAPI[provider] + if ok { + sess.Logger().Infof("Got credentials from provider: %s", provider) + + ca, err := extractBase64Blob(apiCreds.CA) + if err == nil { + creds.CA = []byte(ca) + } + cert, err := extractBase64Blob(apiCreds.Cert) + if err == nil { + creds.Cert = []byte(cert) + } + key, err := extractBase64Blob(apiCreds.Key) + if err == nil { + creds.Key = []byte(key) + } + } + + return creds, 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, logger model.Logger, endpoint *endpoint) *SingleConnection { +func (m *Measurer) connectAndHandshake( + ctx context.Context, index int64, + zeroTime time.Time, sess model.ExperimentSession, endpoint *endpoint) (*SingleConnection, error) { + + logger := sess.Logger() // create a trace for the network dialer trace := measurexlite.NewTrace(index, zeroTime) @@ -203,12 +266,16 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, // create a vpn tun Device that attempts to dial and performs the handshake handshakeTracer := vpntracex.NewTracerWithTransactionID(zeroTime, index) - openvpnConfig, err := getVPNConfig(handshakeTracer, endpoint, &m.config) + credentials, err := m.getCredentialsFromOptionsOrAPI(ctx, sess, endpoint.Provider) + if err != nil { + return nil, err + } + + openvpnConfig, err := getVPNConfig(handshakeTracer, endpoint, credentials) if err != nil { - // TODO: find a better way to return the error - this is not a test failure, - // it's a failure to start the measurement. we should abort - return nil + return nil, err } + tun, err := tunnel.Start(ctx, dialer, openvpnConfig) var failure string @@ -257,5 +324,10 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, TransactionID: index, }, NetworkEvents: handshakeEvents, - } + }, nil +} + +func (m *Measurer) fetchProviderCredentials(ctx context.Context, sess model.ExperimentSession) (map[string]model.OOAPIOpenVPNConfig, error) { + // TODO do pass country code, can be useful to orchestrate campaigns specific to areas + return sess.FetchOpenVPNConfig(ctx, "XX") } diff --git a/internal/model/experiment.go b/internal/model/experiment.go index 410233f6fb..3b490b0221 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -21,6 +21,9 @@ type ExperimentSession interface { // DefaultHTTPClient returns the default HTTPClient used by the session. DefaultHTTPClient() HTTPClient + // FetchOpenVPNConfig returns vpn config as a serialized JSON or an error. + FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]OOAPIOpenVPNConfig, error) + // FetchPsiphonConfig returns psiphon's config as a serialized JSON or an error. FetchPsiphonConfig(ctx context.Context) ([]byte, error) diff --git a/internal/model/ooapi.go b/internal/model/ooapi.go index 940bc4ce5a..2d1469064f 100644 --- a/internal/model/ooapi.go +++ b/internal/model/ooapi.go @@ -99,6 +99,28 @@ type OOAPICheckReportIDResponse struct { V int64 `json:"v"` } +// OOAPIOpenVPNConfig is a minimal valid configuration subset for the openvpn experiment; at the moment it provides +// only credentials valid for endpoints in a provider. +type OOAPIOpenVPNConfig struct { + // Provider is the label for this provider. + Provider string `json:"provider,omitempty"` + + // CA is the Certificate Authority for the endpoints by this provider. + CA string `json:"ca"` + + // Cert is a valid certificate, for providers that use x509 certificate authentication. + Cert string `json:"cert,omitempty"` + + // Key is a valid key, for providers that use x509 certificate authentication. + Key string `json:"key,omitempty"` + + // Username is a valid username, for providers that use password authentication. + Username string `json:"username,omitempty"` + + // Password is a valid password, for providers that use password authentication. + Password string `json:"password,omitempty"` +} + // OOAPIService describes a backend service. // // The fields of this struct have the meaning described in v2.0.0 of the OONI diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go new file mode 100644 index 0000000000..d001fb74c4 --- /dev/null +++ b/internal/probeservices/openvpn.go @@ -0,0 +1,24 @@ +package probeservices + +import ( + "context" + "fmt" + "net/url" + + "github.com/ooni/probe-cli/v3/internal/model" +) + +// FetchOpenVPNConfig returns valid configuration for the openvpn experiment. +func (c Client) FetchOpenVPNConfig(ctx context.Context, cc string) (result map[string]model.OOAPIOpenVPNConfig, err error) { + _, auth, err := c.GetCredsAndAuth() + if err != nil { + return nil, err + } + s := fmt.Sprintf("Bearer %s", auth.Token) + client := c.APIClientTemplate.BuildWithAuthorization(s) + query := url.Values{} + query.Add("country_code", cc) + err = client.GetJSONWithQuery( + ctx, "/api/v1/openvpn-config", query, &result) + return +} From 8bb671b079b6a8730255d04424af3163a39e009c Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 26 Mar 2024 13:34:34 +0100 Subject: [PATCH 13/53] use the newer endpoint, provider in url --- internal/engine/session.go | 4 +-- internal/experiment/openvpn/openvpn.go | 43 ++++++++++++++------------ internal/model/experiment.go | 2 +- internal/model/ooapi.go | 30 +++++++++++------- internal/probeservices/openvpn.go | 6 ++-- 5 files changed, 47 insertions(+), 38 deletions(-) diff --git a/internal/engine/session.go b/internal/engine/session.go index ffff4d6b3a..89dfaea8a6 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -373,10 +373,10 @@ func (s *Session) FetchTorTargets( // FetchOpenVPNConfig fetches openvpn config from the API. func (s *Session) FetchOpenVPNConfig( - ctx context.Context, cc string) (map[string]model.OOAPIOpenVPNConfig, error) { + ctx context.Context, cc string) (model.OOAPIVPNProviderConfig, error) { clnt, err := s.NewOrchestraClient(ctx) if err != nil { - return nil, err + return model.OOAPIVPNProviderConfig{}, err } return clnt.FetchOpenVPNConfig(ctx, cc) } diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index c796e444ac..b3bd084b72 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -224,36 +224,39 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI( // No options passed, let's hit OONI API for credential distribution. // TODO(ainghazal): cache credentials fetch? - configFromAPI, err := m.fetchProviderCredentials(ctx, sess) + + apiCreds, err := m.fetchProviderCredentials(ctx, sess) + // TODO(ainghazal): need to validate + if err != nil { sess.Logger().Warnf("Error fetching credentials from API: %s", err.Error()) return nil, err } - apiCreds, ok := configFromAPI[provider] - if ok { - sess.Logger().Infof("Got credentials from provider: %s", provider) + sess.Logger().Infof("Got credentials from provider: %s", provider) - ca, err := extractBase64Blob(apiCreds.CA) - if err == nil { - creds.CA = []byte(ca) - } - cert, err := extractBase64Blob(apiCreds.Cert) - if err == nil { - creds.Cert = []byte(cert) - } - key, err := extractBase64Blob(apiCreds.Key) - if err == nil { - creds.Key = []byte(key) - } + ca, err := extractBase64Blob(apiCreds.Config.CA) + if err != nil { + return nil, err + } + creds.CA = []byte(ca) + + cert, err := extractBase64Blob(apiCreds.Config.Cert) + if err != nil { + return nil, err + } + creds.Cert = []byte(cert) + + key, err := extractBase64Blob(apiCreds.Config.Key) + if err != nil { + return nil, err } + creds.Key = []byte(key) return creds, 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) { +func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTime time.Time, sess model.ExperimentSession, endpoint *endpoint) (*SingleConnection, error) { logger := sess.Logger() @@ -327,7 +330,7 @@ func (m *Measurer) connectAndHandshake( }, nil } -func (m *Measurer) fetchProviderCredentials(ctx context.Context, sess model.ExperimentSession) (map[string]model.OOAPIOpenVPNConfig, error) { +func (m *Measurer) fetchProviderCredentials(ctx context.Context, sess model.ExperimentSession) (model.OOAPIVPNProviderConfig, error) { // TODO do pass country code, can be useful to orchestrate campaigns specific to areas return sess.FetchOpenVPNConfig(ctx, "XX") } diff --git a/internal/model/experiment.go b/internal/model/experiment.go index 3b490b0221..bc99421e6e 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -22,7 +22,7 @@ type ExperimentSession interface { DefaultHTTPClient() HTTPClient // FetchOpenVPNConfig returns vpn config as a serialized JSON or an error. - FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]OOAPIOpenVPNConfig, error) + FetchOpenVPNConfig(ctx context.Context, cc string) (OOAPIVPNProviderConfig, error) // FetchPsiphonConfig returns psiphon's config as a serialized JSON or an error. FetchPsiphonConfig(ctx context.Context) ([]byte, error) diff --git a/internal/model/ooapi.go b/internal/model/ooapi.go index 2d1469064f..31402c0da5 100644 --- a/internal/model/ooapi.go +++ b/internal/model/ooapi.go @@ -99,26 +99,32 @@ type OOAPICheckReportIDResponse struct { V int64 `json:"v"` } -// OOAPIOpenVPNConfig is a minimal valid configuration subset for the openvpn experiment; at the moment it provides +// OOAPIVPNProviderConfig is a minimal valid configuration subset for the openvpn experiment; at the moment it provides // only credentials valid for endpoints in a provider. -type OOAPIOpenVPNConfig struct { +type OOAPIVPNProviderConfig struct { // Provider is the label for this provider. Provider string `json:"provider,omitempty"` - // CA is the Certificate Authority for the endpoints by this provider. - CA string `json:"ca"` + // Config is the provider-specific VPN Config. + Config struct { + // CA is the Certificate Authority for the endpoints by this provider. + CA string `json:"ca"` - // Cert is a valid certificate, for providers that use x509 certificate authentication. - Cert string `json:"cert,omitempty"` + // Cert is a valid certificate, for providers that use x509 certificate authentication. + Cert string `json:"cert,omitempty"` - // Key is a valid key, for providers that use x509 certificate authentication. - Key string `json:"key,omitempty"` + // Key is a valid key, for providers that use x509 certificate authentication. + Key string `json:"key,omitempty"` - // Username is a valid username, for providers that use password authentication. - Username string `json:"username,omitempty"` + // Username is a valid username, for providers that use password authentication. + Username string `json:"username,omitempty"` - // Password is a valid password, for providers that use password authentication. - Password string `json:"password,omitempty"` + // Password is a valid password, for providers that use password authentication. + Password string `json:"password,omitempty"` + } `json:"config"` + + // DateUpdated is when this credential was last updated in the server database. + DateUpdated time.Time `json:"date_updated"` } // OOAPIService describes a backend service. diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index d001fb74c4..abfddd7aeb 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -9,16 +9,16 @@ import ( ) // FetchOpenVPNConfig returns valid configuration for the openvpn experiment. -func (c Client) FetchOpenVPNConfig(ctx context.Context, cc string) (result map[string]model.OOAPIOpenVPNConfig, err error) { +func (c Client) FetchOpenVPNConfig(ctx context.Context, cc string) (result model.OOAPIVPNProviderConfig, err error) { _, auth, err := c.GetCredsAndAuth() if err != nil { - return nil, err + return model.OOAPIVPNProviderConfig{}, err } s := fmt.Sprintf("Bearer %s", auth.Token) client := c.APIClientTemplate.BuildWithAuthorization(s) query := url.Values{} query.Add("country_code", cc) err = client.GetJSONWithQuery( - ctx, "/api/v1/openvpn-config", query, &result) + ctx, "/api/v2/ooniprobe/vpn-config/riseup/", query, &result) return } From 1ea507f4d6cdf002d870c6102b71df0278887a4d Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 27 Mar 2024 13:18:10 +0100 Subject: [PATCH 14/53] wip: fetch config, process input as part of inputloader --- internal/engine/inputloader.go | 46 +++++++++++++++++++++++++ internal/engine/session.go | 4 +-- internal/experiment/openvpn/endpoint.go | 2 ++ internal/experiment/openvpn/openvpn.go | 7 +++- internal/model/experiment.go | 2 +- internal/model/ooapi.go | 5 ++- internal/probeservices/openvpn.go | 17 ++++++--- internal/registry/openvpn.go | 2 +- 8 files changed, 75 insertions(+), 10 deletions(-) diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index 4db0834dc0..4121ac0811 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -28,6 +28,7 @@ var ( // introduce this abstraction because it helps us with testing. type InputLoaderSession interface { CheckIn(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) + FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) } // InputLoaderLogger is the logger according to an InputLoader. @@ -299,6 +300,16 @@ func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]mode // loadRemote loads inputs from a remote source. func (il *InputLoader) loadRemote(ctx context.Context) ([]model.OOAPIURLInfo, error) { + switch registry.CanonicalizeExperimentName(il.ExperimentName) { + case "openvpn": + return il.loadRemoteOpenVPN(ctx) + default: + return il.loadRemoteWebConnectivity(ctx) + } +} + +// loadRemoteWebConnectivity loads webconnectivity inputs from a remote source. +func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.OOAPIURLInfo, error) { config := il.CheckInConfig if config == nil { // Note: Session.CheckIn documentation says it will fill in @@ -317,6 +328,32 @@ func (il *InputLoader) loadRemote(ctx context.Context) ([]model.OOAPIURLInfo, er return reply.WebConnectivity.URLs, nil } +// loadRemoteOpenVPN loads openvpn inputs from a remote source. +func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLInfo, error) { + reply, err := il.vpnConfig(ctx) + if err != nil { + return nil, err + } + + // VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo], + // since OOAPIURLInfo is oriented twards webconnectivity, + // but we force VPN targets in the URL and ignore all the other fields. + urls := make([]model.OOAPIURLInfo, 0) + + // 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. + for _, config := range reply { + for _, input := range config.Inputs { + urls = append(urls, model.OOAPIURLInfo{URL: input}) + } + } + if len(urls) == 0 { + return nil, ErrNoURLsReturned + } + // TODO(ainghazal): persist config for all providers + return urls, nil +} + // checkIn executes the check-in and filters the returned URLs to exclude // the URLs that are not part of the requested categories. This is done for // robustness, just in case we or the API do something wrong. @@ -335,6 +372,15 @@ func (il *InputLoader) checkIn( return &reply.Tests, nil } +// vpnConfig fetches vpn information for the configured providers +func (il *InputLoader) vpnConfig(ctx context.Context) (map[string]model.OOAPIVPNProviderConfig, error) { + reply, err := il.Session.FetchOpenVPNConfig(ctx, "XX") + if err != nil { + return nil, err + } + return reply, nil +} + // preventMistakes makes the code more robust with respect to any possible // integration issue where the backend returns to us URLs that don't // belong to the category codes we requested. diff --git a/internal/engine/session.go b/internal/engine/session.go index 89dfaea8a6..d10647b83e 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -373,10 +373,10 @@ func (s *Session) FetchTorTargets( // FetchOpenVPNConfig fetches openvpn config from the API. func (s *Session) FetchOpenVPNConfig( - ctx context.Context, cc string) (model.OOAPIVPNProviderConfig, error) { + ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) { clnt, err := s.NewOrchestraClient(ctx) if err != nil { - return model.OOAPIVPNProviderConfig{}, err + return nil, err } return clnt.FetchOpenVPNConfig(ctx, cc) } diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 4fafb6a86a..cc49f96e4b 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -115,6 +115,8 @@ func (e *endpoint) String() string { } // AsInputURI is a string representation of this endpoint. It contains more information than the endpoint itself. +// TODO: redo with latest format +// openvpn://provider.corp/?address=1.1.1.1:1194&transport=tcp func (e *endpoint) AsInputURI() string { provider := e.Provider if provider == "" { diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index b3bd084b72..dfbb043abe 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -330,7 +330,12 @@ 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) (model.OOAPIVPNProviderConfig, error) { // TODO do pass country code, can be useful to orchestrate campaigns specific to areas - return sess.FetchOpenVPNConfig(ctx, "XX") + config, err := sess.FetchOpenVPNConfig(ctx, "XX") + if err != nil { + return model.OOAPIVPNProviderConfig{}, err + } + return config["riseup"], nil } diff --git a/internal/model/experiment.go b/internal/model/experiment.go index bc99421e6e..c1aafeadd6 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -22,7 +22,7 @@ type ExperimentSession interface { DefaultHTTPClient() HTTPClient // FetchOpenVPNConfig returns vpn config as a serialized JSON or an error. - FetchOpenVPNConfig(ctx context.Context, cc string) (OOAPIVPNProviderConfig, error) + FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]OOAPIVPNProviderConfig, error) // FetchPsiphonConfig returns psiphon's config as a serialized JSON or an error. FetchPsiphonConfig(ctx context.Context) ([]byte, error) diff --git a/internal/model/ooapi.go b/internal/model/ooapi.go index 31402c0da5..9786f802ca 100644 --- a/internal/model/ooapi.go +++ b/internal/model/ooapi.go @@ -106,7 +106,7 @@ type OOAPIVPNProviderConfig struct { Provider string `json:"provider,omitempty"` // Config is the provider-specific VPN Config. - Config struct { + Config *struct { // CA is the Certificate Authority for the endpoints by this provider. CA string `json:"ca"` @@ -123,6 +123,9 @@ type OOAPIVPNProviderConfig struct { Password string `json:"password,omitempty"` } `json:"config"` + // Inputs is an array of valid endpoints for this provider. + Inputs []string + // DateUpdated is when this credential was last updated in the server database. DateUpdated time.Time `json:"date_updated"` } diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index abfddd7aeb..07a7f78018 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -9,16 +9,25 @@ import ( ) // FetchOpenVPNConfig returns valid configuration for the openvpn experiment. -func (c Client) FetchOpenVPNConfig(ctx context.Context, cc string) (result model.OOAPIVPNProviderConfig, err error) { +func (c Client) FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) { _, auth, err := c.GetCredsAndAuth() if err != nil { - return model.OOAPIVPNProviderConfig{}, err + return nil, err } s := fmt.Sprintf("Bearer %s", auth.Token) client := c.APIClientTemplate.BuildWithAuthorization(s) query := url.Values{} query.Add("country_code", cc) + + result := model.OOAPIVPNProviderConfig{} + err = client.GetJSONWithQuery( - ctx, "/api/v2/ooniprobe/vpn-config/riseup/", query, &result) - return + ctx, "/api/v2/ooniprobe/vpn-config/riseup/", query, &result, + ) + + allProviders := map[string]model.OOAPIVPNProviderConfig{ + "riseup": result, + } + + return allProviders, err } diff --git a/internal/registry/openvpn.go b/internal/registry/openvpn.go index 2a0f6b1e7f..8a4a5728e0 100644 --- a/internal/registry/openvpn.go +++ b/internal/registry/openvpn.go @@ -19,6 +19,6 @@ func init() { config: &openvpn.Config{}, enabledByDefault: false, interruptible: true, - inputPolicy: model.InputOptional, + inputPolicy: model.InputOrQueryBackend, } } From 46b40f19309a31d19387defe5adf3dd5257ec663 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 27 Mar 2024 14:48:49 +0100 Subject: [PATCH 15/53] parse input in new format --- internal/experiment/openvpn/endpoint.go | 39 +++++++++++-------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index cc49f96e4b..836f07bc28 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "math/rand" + "net" "net/url" "strings" @@ -60,42 +61,36 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { return nil, fmt.Errorf("%w: unknown scheme: %s", ErrInvalidInput, parsedURL.Scheme) } - host := parsedURL.Hostname() - if host == "" { - return nil, fmt.Errorf("%w: expected host: %s", ErrInvalidInput, parsedURL.Host) + provider := strings.TrimSuffix(parsedURL.Hostname(), ".corp") + if provider == "" { + return nil, fmt.Errorf("%w: expected provider as host: %s", ErrInvalidInput, parsedURL.Host) } - - port := parsedURL.Port() - if port == "" { - return nil, fmt.Errorf("%w: expected port: %s", ErrInvalidInput, parsedURL.Port()) + if provider != "riseup" { + // because we are hardcoding at the moment. figure out a way to pass info for + // arbitrary providers as options instead + return nil, fmt.Errorf("%w: unknown provider: %s", ErrInvalidInput, provider) } - pathParts := strings.Split(parsedURL.Path, "/") - if len(pathParts) != 3 { - return nil, fmt.Errorf("%w: invalid path: %s (%d)", ErrInvalidInput, pathParts, len(pathParts)) - } - transport := pathParts[1] + params := parsedURL.Query() + + transport := params.Get("transport") if transport != "tcp" && transport != "udp" { return nil, fmt.Errorf("%w: invalid transport: %s", ErrInvalidInput, transport) } - params := parsedURL.Query() - provider := params.Get("provider") - + address := params.Get("address") if provider == "" { return nil, fmt.Errorf("%w: please specify a provider as part of the input", ErrInvalidInput) } - - if provider != "riseup" { - // because we are hardcoding at the moment. figure out a way to pass info for - // arbitrary providers as options instead - return nil, fmt.Errorf("%w: unknown provider: %s", ErrInvalidInput, provider) + ip, port, err := net.SplitHostPort(address) + if err != nil { + return nil, fmt.Errorf("%w: cannot split ip:port", ErrInvalidInput) } endpoint := &endpoint{ - IPAddr: host, - Obfuscation: obfuscation, + IPAddr: ip, Port: port, + Obfuscation: obfuscation, Protocol: "openvpn", Provider: provider, Transport: transport, From 0c6a0626fb183bc880ed641441cb7cb61d67269b Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 27 Mar 2024 15:02:41 +0100 Subject: [PATCH 16/53] cache config from api --- internal/engine/session.go | 14 +++++++++++++- internal/experiment/openvpn/endpoint.go | 25 +++++++++++++++++-------- internal/probeservices/openvpn.go | 1 + 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/internal/engine/session.go b/internal/engine/session.go index d10647b83e..801a793ebe 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -66,6 +66,7 @@ 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 @@ -177,6 +178,7 @@ 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 { @@ -374,11 +376,21 @@ func (s *Session) FetchTorTargets( // FetchOpenVPNConfig fetches openvpn config from the API. func (s *Session) FetchOpenVPNConfig( ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) { + // TODO: need to cache only the requested provider, since it's different calls to the API. + if len(s.vpnConfig) > 0 { + return s.vpnConfig, nil + } + clnt, err := s.NewOrchestraClient(ctx) if err != nil { return nil, err } - return clnt.FetchOpenVPNConfig(ctx, cc) + config, err := clnt.FetchOpenVPNConfig(ctx, cc) + if err != nil { + return nil, err + } + s.vpnConfig = config + return config, nil } // KeyValueStore returns the configured key-value store. diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 836f07bc28..528ce05942 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -66,8 +66,8 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { return nil, fmt.Errorf("%w: expected provider as host: %s", ErrInvalidInput, parsedURL.Host) } if provider != "riseup" { - // because we are hardcoding at the moment. figure out a way to pass info for - // arbitrary providers as options instead + // I am hardcoding a single provider at the moment. + // I need to figure out a way to pass info for arbitrary providers as options instead. return nil, fmt.Errorf("%w: unknown provider: %s", ErrInvalidInput, provider) } @@ -98,7 +98,8 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { return endpoint, nil } -// String implements Stringer. This is a subset of the input URI scheme. +// String implements Stringer. This is a compact representation of the endpoint, +// which differs from the input URI scheme. func (e *endpoint) String() string { var proto string if e.Obfuscation == "obfs4" { @@ -109,16 +110,23 @@ func (e *endpoint) String() string { return fmt.Sprintf("%s://%s:%s/%s", proto, e.IPAddr, e.Port, e.Transport) } -// AsInputURI is a string representation of this endpoint. It contains more information than the endpoint itself. -// TODO: redo with latest format -// openvpn://provider.corp/?address=1.1.1.1:1194&transport=tcp +// AsInputURI is a string representation of this endpoint, as used in the experiment input URI format. func (e *endpoint) AsInputURI() string { + var proto string + if e.Obfuscation == "obfs4" { + proto = e.Protocol + "+obfs4" + } else { + proto = e.Protocol + } + provider := e.Provider if provider == "" { provider = "unknown" } - i := fmt.Sprintf("%s/?provider=%s", e.String(), provider) - return i + + return fmt.Sprintf( + "%s://%s.corp/?address=%s:%s&transport=%s", + proto, provider, e.IPAddr, e.Port, e.Transport) } // endpointList is a list of endpoints. @@ -128,6 +136,7 @@ type endpointList []*endpoint // This is a hardcoded list for now, but the idea is that we can receive this from the check-in api in the future. // In any case, having hardcoded endpoints is good as a fallback for the cases in which we cannot contact // OONI's backend. +// TODO: hardcoded, setup as backup if we cannot contact API. var allEndpoints = endpointList{ { Provider: "riseup", diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index 07a7f78018..0f5149bffc 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -10,6 +10,7 @@ import ( // FetchOpenVPNConfig returns valid configuration for the openvpn experiment. func (c Client) FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) { + fmt.Println("FETCHING OPENVPN CONFIG>>>>") _, auth, err := c.GetCredsAndAuth() if err != nil { return nil, err From 199f10524c0c3dcff6fac9431e761375c59e83b0 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 27 Mar 2024 15:23:50 +0100 Subject: [PATCH 17/53] cache per-provider config --- internal/engine/inputloader.go | 31 +++++++++++++++----------- internal/engine/session.go | 21 +++++++++-------- internal/experiment/openvpn/openvpn.go | 17 +++++++------- internal/model/experiment.go | 2 +- internal/probeservices/openvpn.go | 21 ++++++++--------- 5 files changed, 47 insertions(+), 45 deletions(-) diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index 4121ac0811..dca654a416 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -28,7 +28,8 @@ var ( // introduce this abstraction because it helps us with testing. type InputLoaderSession interface { CheckIn(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error) - FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) + FetchOpenVPNConfig(ctx context.Context, + provider, cc string) (*model.OOAPIVPNProviderConfig, error) } // InputLoaderLogger is the logger according to an InputLoader. @@ -328,29 +329,33 @@ func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.O return reply.WebConnectivity.URLs, nil } +// These are the providers that are enabled in the API. +var openvpnDefaultProviders = []string{ + "riseup", +} + // loadRemoteOpenVPN loads openvpn inputs from a remote source. func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLInfo, error) { - reply, err := il.vpnConfig(ctx) - if err != nil { - return nil, err - } - // VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo], // since OOAPIURLInfo is oriented twards webconnectivity, // but we force VPN targets in the URL and ignore all the other fields. urls := make([]model.OOAPIURLInfo, 0) - // 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. - for _, config := range reply { - for _, input := range config.Inputs { + for _, provider := range openvpnDefaultProviders { + reply, err := il.vpnConfig(ctx, provider) + if err != nil { + return nil, err + } + // 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. + for _, input := range reply.Inputs { urls = append(urls, model.OOAPIURLInfo{URL: input}) } } + if len(urls) == 0 { return nil, ErrNoURLsReturned } - // TODO(ainghazal): persist config for all providers return urls, nil } @@ -373,8 +378,8 @@ func (il *InputLoader) checkIn( } // vpnConfig fetches vpn information for the configured providers -func (il *InputLoader) vpnConfig(ctx context.Context) (map[string]model.OOAPIVPNProviderConfig, error) { - reply, err := il.Session.FetchOpenVPNConfig(ctx, "XX") +func (il *InputLoader) vpnConfig(ctx context.Context, provider string) (*model.OOAPIVPNProviderConfig, error) { + reply, err := il.Session.FetchOpenVPNConfig(ctx, provider, "XX") if err != nil { return nil, err } diff --git a/internal/engine/session.go b/internal/engine/session.go index 801a793ebe..fc7ac006e2 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -373,24 +373,23 @@ func (s *Session) FetchTorTargets( return clnt.FetchTorTargets(ctx, cc) } -// FetchOpenVPNConfig fetches openvpn config from the API. +// FetchOpenVPNConfig fetches openvpn config from the API if it's not found in the +// internal cache. We do this to avoid hitting the API for every input. func (s *Session) FetchOpenVPNConfig( - ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) { - // TODO: need to cache only the requested provider, since it's different calls to the API. - if len(s.vpnConfig) > 0 { - return s.vpnConfig, nil + 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 + return &model.OOAPIVPNProviderConfig{}, err } - config, err := clnt.FetchOpenVPNConfig(ctx, cc) + config, err := clnt.FetchOpenVPNConfig(ctx, provider, cc) if err != nil { - return nil, err + return &model.OOAPIVPNProviderConfig{}, err } - s.vpnConfig = config - return config, nil + s.vpnConfig[provider] = config + return &config, nil } // KeyValueStore returns the configured key-value store. diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index dfbb043abe..97bd514251 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -223,9 +223,7 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI( } // No options passed, let's hit OONI API for credential distribution. - // TODO(ainghazal): cache credentials fetch? - - apiCreds, err := m.fetchProviderCredentials(ctx, sess) + apiCreds, err := m.fetchProviderCredentials(ctx, sess, provider) // TODO(ainghazal): need to validate if err != nil { @@ -331,11 +329,14 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTim } // TODO: get cached from session instead of fetching every time -func (m *Measurer) fetchProviderCredentials(ctx context.Context, sess model.ExperimentSession) (model.OOAPIVPNProviderConfig, error) { - // TODO do pass country code, can be useful to orchestrate campaigns specific to areas - config, err := sess.FetchOpenVPNConfig(ctx, "XX") +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 + config, err := sess.FetchOpenVPNConfig(ctx, provider, "XX") if err != nil { - return model.OOAPIVPNProviderConfig{}, err + return &model.OOAPIVPNProviderConfig{}, err } - return config["riseup"], nil + return config, nil } diff --git a/internal/model/experiment.go b/internal/model/experiment.go index c1aafeadd6..b871613804 100644 --- a/internal/model/experiment.go +++ b/internal/model/experiment.go @@ -22,7 +22,7 @@ type ExperimentSession interface { DefaultHTTPClient() HTTPClient // FetchOpenVPNConfig returns vpn config as a serialized JSON or an error. - FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]OOAPIVPNProviderConfig, error) + FetchOpenVPNConfig(ctx context.Context, provider, cc string) (*OOAPIVPNProviderConfig, error) // FetchPsiphonConfig returns psiphon's config as a serialized JSON or an error. FetchPsiphonConfig(ctx context.Context) ([]byte, error) diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index 0f5149bffc..281d74ee87 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -9,26 +9,23 @@ import ( ) // FetchOpenVPNConfig returns valid configuration for the openvpn experiment. -func (c Client) FetchOpenVPNConfig(ctx context.Context, cc string) (map[string]model.OOAPIVPNProviderConfig, error) { - fmt.Println("FETCHING OPENVPN CONFIG>>>>") +// It accepts the provider label, and the country code for the probe, in case the API wants to +// return different targets to us depending on where we are located. +func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (result model.OOAPIVPNProviderConfig, err error) { _, auth, err := c.GetCredsAndAuth() if err != nil { - return nil, err + return model.OOAPIVPNProviderConfig{}, err } s := fmt.Sprintf("Bearer %s", auth.Token) client := c.APIClientTemplate.BuildWithAuthorization(s) query := url.Values{} query.Add("country_code", cc) - result := model.OOAPIVPNProviderConfig{} - err = client.GetJSONWithQuery( - ctx, "/api/v2/ooniprobe/vpn-config/riseup/", query, &result, + ctx, + fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%s/", provider), + query, + &result, ) - - allProviders := map[string]model.OOAPIVPNProviderConfig{ - "riseup": result, - } - - return allProviders, err + return } From 6ada20f1b19aab0cc91f08ba0389da261dfed2ab Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 27 Mar 2024 20:02:41 +0100 Subject: [PATCH 18/53] do not parse base64 in creds from api --- internal/experiment/openvpn/openvpn.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 97bd514251..cada7b34bc 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -190,6 +190,7 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI( ctx context.Context, sess model.ExperimentSession, provider string) (*vpnconfig.OpenVPNOptions, error) { + // TODO(ainghazal): Ideally, we need to know which authentication methods each provider uses, and this is // information that the experiment could hardcode. Sticking to Certificate-based auth for riseupvpn. @@ -200,6 +201,7 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI( if cfg.SafeCA != "" && cfg.SafeCert != "" && cfg.SafeKey != "" { // We override authentication info with what user provided in options. + // We expect the options to be encoded in base64 so that a single optin can be safely represented as command line options. ca, err := extractBase64Blob(cfg.SafeCA) if err != nil { return nil, err @@ -223,8 +225,9 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI( } // No options passed, let's hit OONI API for credential distribution. + // 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): need to validate + // TODO(ainghazal): validate if err != nil { sess.Logger().Warnf("Error fetching credentials from API: %s", err.Error()) @@ -232,23 +235,9 @@ func (m *Measurer) getCredentialsFromOptionsOrAPI( } sess.Logger().Infof("Got credentials from provider: %s", provider) - ca, err := extractBase64Blob(apiCreds.Config.CA) - if err != nil { - return nil, err - } - creds.CA = []byte(ca) - - cert, err := extractBase64Blob(apiCreds.Config.Cert) - if err != nil { - return nil, err - } - creds.Cert = []byte(cert) - - key, err := extractBase64Blob(apiCreds.Config.Key) - if err != nil { - return nil, err - } - creds.Key = []byte(key) + creds.CA = []byte(apiCreds.Config.CA) + creds.Cert = []byte(apiCreds.Config.Cert) + creds.Key = []byte(apiCreds.Config.Key) return creds, nil } From accaf9795b184b2ce17f253c396ec72c60fd8eee Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 27 Mar 2024 20:21:02 +0100 Subject: [PATCH 19/53] tests --- internal/engine/inputloader.go | 12 +- internal/engine/inputloader_test.go | 15 +- internal/engine/session.go | 4 +- internal/engine/session_internal_test.go | 13 + internal/experiment/openvpn/archival.go | 3 - internal/experiment/openvpn/endpoint.go | 58 ++- internal/experiment/openvpn/endpoint_test.go | 400 ++++++++++++++++++ internal/experiment/openvpn/openvpn.go | 299 ++++++++------ internal/experiment/openvpn/openvpn_test.go | 412 ++++++++++++++++++- internal/legacy/mockable/mockable.go | 8 + internal/mocks/session.go | 8 + internal/model/ooapi.go | 4 +- 12 files changed, 1060 insertions(+), 176 deletions(-) create mode 100644 internal/experiment/openvpn/endpoint_test.go diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index dca654a416..2ce8633e51 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -9,6 +9,7 @@ import ( "net/url" "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/experiment/openvpn" "github.com/ooni/probe-cli/v3/internal/fsx" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/registry" @@ -344,7 +345,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. @@ -354,7 +355,14 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI } if len(urls) == 0 { - return nil, ErrNoURLsReturned + // loadRemote returns ErrNoURLsReturned at this point for webconnectivity, + // but for OpenVPN we want to return a sensible default to be + // able to probe some endpoints even in very restrictive environments. + // Do note this means that you have to provide valid credentials + // by some other means. + for _, endpoint := range openvpn.DefaultEndpoints { + urls = append(urls, model.OOAPIURLInfo{URL: endpoint.AsInputURI()}) + } } return urls, nil } diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index 68e1ab5489..42a2abac94 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -448,6 +448,10 @@ type InputLoaderMockableSession struct { // be nil when Error is not-nil. Output *model.OOAPICheckInResult + // FetchOpenVPNConfigOutput contains the output of FetchOpenVPNConfig. + // It should be nil when Error is non-nil. + FetchOpenVPNConfigOutput *model.OOAPIVPNProviderConfig + // Error is the error to be returned by CheckIn. It // should be nil when Output is not-nil. Error error @@ -459,7 +463,16 @@ func (sess *InputLoaderMockableSession) CheckIn( if sess.Output == nil && sess.Error == nil { return nil, errors.New("both Output and Error are nil") } - return sess.Output, sess.Error + return sess.CheckinOutput, sess.Error +} + +// FetchOpenVPNConfig implements InputLoaderSession.FetchOpenVPNConfig. +func (sess *InputLoaderMockableSession) FetchOpenVPNConfig( + ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) { + if sess.FetchOpenVPNConfigOutput == nil && sess.Error == nil { + return nil, errors.New("both Output and Error are nil") + } + return sess.FetchOpenVPNConfigOutput, sess.Error } func TestInputLoaderCheckInFailure(t *testing.T) { diff --git a/internal/engine/session.go b/internal/engine/session.go index fc7ac006e2..d3c34fcd83 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -382,11 +382,11 @@ func (s *Session) FetchOpenVPNConfig( } clnt, err := s.NewOrchestraClient(ctx) if err != nil { - return &model.OOAPIVPNProviderConfig{}, err + return nil, err } config, err := clnt.FetchOpenVPNConfig(ctx, provider, cc) if err != nil { - return &model.OOAPIVPNProviderConfig{}, err + return nil, err } s.vpnConfig[provider] = config return &config, nil diff --git a/internal/engine/session_internal_test.go b/internal/engine/session_internal_test.go index b2971af49b..ea7ec005b1 100644 --- a/internal/engine/session_internal_test.go +++ b/internal/engine/session_internal_test.go @@ -253,6 +253,19 @@ func TestSessionMaybeLookupLocationContextLookupLocationContextFailure(t *testin } } +func TestSessionFetchOpenVPNConfigWithCancelledContext(t *testing.T) { + sess := &Session{} + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cause failure + resp, err := sess.FetchOpenVPNConfig(ctx, "riseup", "XX") + if !errors.Is(err, context.Canceled) { + t.Fatal("not the error we expected", err) + } + if resp != nil { + t.Fatal("expected nil response here") + } +} + func TestSessionFetchTorTargetsWithCancelledContext(t *testing.T) { sess := &Session{} ctx, cancel := context.WithCancel(context.Background()) diff --git a/internal/experiment/openvpn/archival.go b/internal/experiment/openvpn/archival.go index 54e261a46e..c10779217f 100644 --- a/internal/experiment/openvpn/archival.go +++ b/internal/experiment/openvpn/archival.go @@ -1,7 +1,5 @@ package openvpn -import "time" - // TODO(ainghazal): move to ooni internal archival package when consolidated. // OpenVPNOptions is a subset of [vpnconfig.OpenVPNOptions] that we want to include @@ -22,7 +20,6 @@ type ArchivalOpenVPNHandshakeResult struct { Provider string `json:"provider"` OpenVPNOptions OpenVPNOptions `json:"openvpn_options"` Status ArchivalOpenVPNConnectStatus `json:"status"` - StartTime time.Time `json:"handshake_start_time"` // T0 can differ from StartTime because for TCP we take T0 *after* dialing has successfully completed. // This might be counterintuitive, review. diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 528ce05942..7b8ad86b49 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 ( @@ -54,7 +55,7 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { var obfuscation string switch parsedURL.Scheme { case "openvpn": - obfuscation = "openvpn" + obfuscation = "none" case "openvpn+obfs4": obfuscation = "obfs4" default: @@ -79,13 +80,16 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { } address := params.Get("address") - if provider == "" { - return nil, fmt.Errorf("%w: please specify a provider as part of the input", ErrInvalidInput) + if address == "" { + return nil, fmt.Errorf("%w: please specify an address as part of the input", ErrInvalidInput) } ip, port, err := net.SplitHostPort(address) if err != nil { return nil, fmt.Errorf("%w: cannot split ip:port", ErrInvalidInput) } + if parsedIP := net.ParseIP(ip); parsedIP == nil { + return nil, fmt.Errorf("%w: bad ip", ErrInvalidInput) + } endpoint := &endpoint{ IPAddr: ip, @@ -99,7 +103,8 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { } // String implements Stringer. This is a compact representation of the endpoint, -// which differs from the input URI scheme. +// which differs from the input URI scheme. This is the canonical representation, that can be used +// to deterministically slice a list of endpoints, sort them lexicographically, etc. func (e *endpoint) String() string { var proto string if e.Obfuscation == "obfs4" { @@ -132,12 +137,11 @@ func (e *endpoint) AsInputURI() string { // endpointList is a list of endpoints. type endpointList []*endpoint -// allEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment. -// This is a hardcoded list for now, but the idea is that we can receive this from the check-in api in the future. -// In any case, having hardcoded endpoints is good as a fallback for the cases in which we cannot contact -// OONI's backend. -// TODO: hardcoded, setup as backup if we cannot contact API. -var allEndpoints = endpointList{ +// DefaultEndpoints contains a subset of known endpoints to be used if no input is passed to the experiment and +// 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{ { Provider: "riseup", IPAddr: "51.15.187.53", @@ -177,22 +181,24 @@ func isValidProvider(provider string) bool { return ok } -// getVPNConfig gets a properly configured [*vpnconfig.Config] object for the given endpoint. +// getOpenVPNConfig gets a properly configured [*vpnconfig.Config] object for the given endpoint. // To obtain that, we merge the endpoint specific configuration with base options. -// These base options are for the moment hardcoded. In the future we will want to be smarter -// about getting information for different providers. -func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, creds *vpnconfig.OpenVPNOptions) (*vpnconfig.Config, error) { - +// 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, + 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) { return nil, fmt.Errorf("%w: unknown provider: %s", ErrInvalidInput, provider) } - baseOptions := defaultOptionsByProvider[provider] cfg := vpnconfig.NewConfig( + vpnconfig.WithLogger(logger), vpnconfig.WithOpenVPNOptions( &vpnconfig.OpenVPNOptions{ // endpoint-specific options. @@ -210,12 +216,13 @@ func getVPNConfig(tracer *vpntracex.Tracer, endpoint *endpoint, creds *vpnconfig Key: creds.Key, }, ), - vpnconfig.WithHandshakeTracer(tracer)) + vpnconfig.WithHandshakeTracer(tracer), + ) - // TODO: validate options here and return an error. return cfg, nil } +// extractBase64Blob is used to pass credentials as command-line options. func extractBase64Blob(val string) (string, error) { s := strings.TrimPrefix(val, "base64:") if len(s) == len(val) { @@ -225,8 +232,15 @@ func extractBase64Blob(val string) (string, error) { if err != nil { return "", fmt.Errorf("%w: %s", ErrBadBase64Blob, err) } - if len(dec) == 0 { - return "", nil - } return string(dec), nil } + +func isValidProtocol(s string) bool { + if strings.HasPrefix(s, "openvpn://") { + return true + } + if strings.HasPrefix(s, "openvpn+obfs4://") { + return true + } + return false +} diff --git a/internal/experiment/openvpn/endpoint_test.go b/internal/experiment/openvpn/endpoint_test.go new file mode 100644 index 0000000000..502fd379c2 --- /dev/null +++ b/internal/experiment/openvpn/endpoint_test.go @@ -0,0 +1,400 @@ +package openvpn + +import ( + "errors" + "sort" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + vpnconfig "github.com/ooni/minivpn/pkg/config" + vpntracex "github.com/ooni/minivpn/pkg/tracex" +) + +func Test_newEndpointFromInputString(t *testing.T) { + type args struct { + uri string + } + tests := []struct { + name string + args args + want *endpoint + wantErr error + }{ + { + name: "valid endpoint returns good endpoint", + args: args{"openvpn://riseup.corp/?address=1.1.1.1:1194&transport=tcp"}, + want: &endpoint{ + IPAddr: "1.1.1.1", + Obfuscation: "none", + Port: "1194", + Protocol: "openvpn", + Provider: "riseup", + Transport: "tcp", + }, + wantErr: nil, + }, + { + name: "bad url fails", + args: args{"://address=1.1.1.1:1194&transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "openvpn+obfs4 does not fail", + args: args{"openvpn+obfs4://riseup.corp/?address=1.1.1.1:1194&transport=tcp"}, + want: &endpoint{ + IPAddr: "1.1.1.1", + Obfuscation: "obfs4", + Port: "1194", + Protocol: "openvpn", + Provider: "riseup", + Transport: "tcp", + }, + wantErr: nil, + }, + { + name: "unknown proto fails", + args: args{"unknown://riseup.corp/?address=1.1.1.1:1194&transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "any tld other than .corp fails", + args: args{"openvpn://riseup.org/?address=1.1.1.1:1194&transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "empty provider fails", + args: args{"openvpn://.corp/?address=1.1.1.1:1194&transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "non-registered provider fails", + args: args{"openvpn://nsavpn.corp/?address=1.1.1.1:1194&transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "endpoint with invalid ipv4 fails", + args: args{"openvpn://riseup.corp/?address=example.com:1194&transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "endpoint with no port fails", + args: args{"openvpn://riseup.corp/?address=1.1.1.1&transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "endpoint with empty transport fails", + args: args{"openvpn://riseup.corp/?address=1.1.1.1:1194&transport="}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "endpoint with no transport fails", + args: args{"openvpn://riseup.corp/?address=1.1.1.1:1194"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "endpoint with unknown transport fails", + args: args{"openvpn://riseup.corp/?address=1.1.1.1:1194&transport=uh"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "endpoint with no address fails", + args: args{"openvpn://riseup.corp/?transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + { + name: "endpoint with empty address fails", + args: args{"openvpn://riseup.corp/?address=&transport=tcp"}, + want: nil, + wantErr: ErrInvalidInput, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := newEndpointFromInputString(tt.args.uri) + if !errors.Is(err, tt.wantErr) { + t.Errorf("newEndpointFromInputString() error = %v, wantErr %v", err, tt.wantErr) + return + } + if diff := cmp.Diff(got, tt.want); diff != "" { + t.Errorf(diff) + } + }) + } +} + +func Test_EndpointToInputURI(t *testing.T) { + type args struct { + endpoint endpoint + } + tests := []struct { + name string + args args + want string + }{ + { + name: "good endpoint with plain openvpn", + args: args{ + endpoint{ + IPAddr: "1.1.1.1", + Obfuscation: "none", + Port: "443", + Protocol: "openvpn", + Provider: "shady", + Transport: "udp", + }, + }, + want: "openvpn://shady.corp/?address=1.1.1.1:443&transport=udp", + }, + { + name: "good endpoint with openvpn+obfs4", + args: args{ + endpoint{ + IPAddr: "1.1.1.1", + Obfuscation: "obfs4", + Port: "443", + Protocol: "openvpn", + Provider: "shady", + Transport: "udp", + }, + }, + want: "openvpn+obfs4://shady.corp/?address=1.1.1.1:443&transport=udp", + }, + { + name: "empty provider is marked as unknown", + args: args{ + endpoint{ + IPAddr: "1.1.1.1", + Obfuscation: "obfs4", + Port: "443", + Protocol: "openvpn", + Provider: "", + Transport: "udp", + }, + }, + want: "openvpn+obfs4://unknown.corp/?address=1.1.1.1:443&transport=udp", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.args.endpoint.AsInputURI(); cmp.Diff(got, tt.want) != "" { + t.Error(cmp.Diff(got, tt.want)) + } + }) + } +} + +func Test_endpoint_String(t *testing.T) { + type fields struct { + IPAddr string + Obfuscation string + Port string + Protocol string + Provider string + Transport string + } + tests := []struct { + name string + fields fields + want string + }{ + { + name: "well formed endpoint returns a well formed endpoint string", + fields: fields{ + IPAddr: "1.1.1.1", + Obfuscation: "none", + Port: "1194", + Protocol: "openvpn", + Provider: "unknown", + Transport: "tcp", + }, + want: "openvpn://1.1.1.1:1194/tcp", + }, + { + name: "well formed endpoint, openvpn+obfs4", + fields: fields{ + IPAddr: "1.1.1.1", + Obfuscation: "obfs4", + Port: "1194", + Protocol: "openvpn", + Provider: "unknown", + Transport: "tcp", + }, + want: "openvpn+obfs4://1.1.1.1:1194/tcp", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := &endpoint{ + IPAddr: tt.fields.IPAddr, + Obfuscation: tt.fields.Obfuscation, + Port: tt.fields.Port, + Protocol: tt.fields.Protocol, + Provider: tt.fields.Provider, + Transport: tt.fields.Transport, + } + if got := e.String(); got != tt.want { + t.Errorf("endpoint.String() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_endpointList_Shuffle(t *testing.T) { + shuffled := DefaultEndpoints.Shuffle() + sort.Slice(shuffled, func(i, j int) bool { + return shuffled[i].IPAddr < shuffled[j].IPAddr + }) + if diff := cmp.Diff(shuffled, DefaultEndpoints); diff != "" { + t.Error(diff) + } +} + +func Test_isValidProvider(t *testing.T) { + if valid := isValidProvider("riseup"); !valid { + t.Fatal("riseup is the only valid provider now") + } + if valid := isValidProvider("nsa"); valid { + t.Fatal("nsa will nevel be a provider") + } +} + +func Test_getVPNConfig(t *testing.T) { + tracer := vpntracex.NewTracer(time.Now()) + e := &endpoint{ + Provider: "riseup", + IPAddr: "1.1.1.1", + Port: "443", + Transport: "udp", + } + creds := &vpnconfig.OpenVPNOptions{ + CA: []byte("ca"), + Cert: []byte("cert"), + Key: []byte("key"), + } + + cfg, err := getOpenVPNConfig(tracer, nil, e, creds) + if err != nil { + t.Fatalf("did not expect error, got: %v", err) + } + if cfg.Tracer() != tracer { + t.Fatal("config tracer is not what passed") + } + if auth := cfg.OpenVPNOptions().Auth; auth != "SHA512" { + t.Errorf("expected auth %s, got %s", "SHA512", auth) + } + if cipher := cfg.OpenVPNOptions().Cipher; cipher != "AES-256-GCM" { + t.Errorf("expected cipher %s, got %s", "AES-256-GCM", cipher) + } + if remote := cfg.OpenVPNOptions().Remote; remote != e.IPAddr { + t.Errorf("expected remote %s, got %s", e.IPAddr, remote) + } + if port := cfg.OpenVPNOptions().Port; port != e.Port { + t.Errorf("expected port %s, got %s", e.Port, port) + } + if transport := cfg.OpenVPNOptions().Proto; string(transport) != e.Transport { + t.Errorf("expected transport %s, got %s", e.Transport, transport) + } + 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 != "" { + t.Error(diff) + } + if diff := cmp.Diff(cfg.OpenVPNOptions().Cert, creds.Cert); diff != "" { + t.Error(diff) + } + if diff := cmp.Diff(cfg.OpenVPNOptions().Key, creds.Key); diff != "" { + t.Error(diff) + } +} + +func Test_getVPNConfig_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"), + } + _, err := getOpenVPNConfig(tracer, nil, e, creds) + 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 := extractBase64Blob(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==" + _, err := extractBase64Blob(blob) + if !errors.Is(err, ErrBadBase64Blob) { + t.Fatal("should fail without prefix") + } + }) + t.Run("bad base64 blob should fail", func(t *testing.T) { + blob := "base64:dGhlIGJsdWUgb2N0b3B1cyBpcyB3YXRjaGluZw" + _, err := extractBase64Blob(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 := extractBase64Blob(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 := extractBase64Blob(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") { + t.Error("openvpn:// should be a valid protocol") + } + }) + t.Run("openvpn+obfs4 is valid", func(t *testing.T) { + if !isValidProtocol("openvpn+obfs4://foobar.bar") { + t.Error("openvpn+obfs4:// should be a valid protocol") + } + }) + t.Run("openvpn+other is not valid", func(t *testing.T) { + if isValidProtocol("openvpn+ss://foobar.bar") { + t.Error("openvpn+ss:// should not be a valid protocol") + } + }) +} diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index cada7b34bc..2dd0f99120 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "strconv" - "strings" "time" "github.com/ooni/probe-cli/v3/internal/measurexlite" @@ -18,16 +17,21 @@ import ( ) const ( - testVersion = "0.1.0" + testVersion = "0.1.1" openVPNProcol = "openvpn" ) +var ( + ErrBadAuth = errors.New("bad provider authentication") +) + // Config contains the experiment config. // // This contains all the settings that user can set to modify the behaviour // 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"` @@ -45,7 +49,7 @@ type TestKeys struct { // NewTestKeys creates new openvpn TestKeys. func NewTestKeys() *TestKeys { return &TestKeys{ - Success: true, + Success: false, NetworkEvents: []*vpntracex.Event{}, TCPConnect: []*model.ArchivalTCPConnectResult{}, OpenVPNHandshake: []*ArchivalOpenVPNHandshakeResult{}, @@ -72,8 +76,8 @@ func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { tk.NetworkEvents = append(tk.NetworkEvents, result.NetworkEvents...) } -// allConnectionsSuccessful returns true if all the registered handshakes have Status.Success equal to true. -func (tk *TestKeys) allConnectionsSuccessful() bool { +// AllConnectionsSuccessful returns true if all the registered handshakes have Status.Success equal to true. +func (tk *TestKeys) AllConnectionsSuccessful() bool { for _, c := range tk.OpenVPNHandshake { if !c.Status.Success { return false @@ -90,8 +94,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,170 +111,174 @@ 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 +func parseEndpoint(m *model.Measurement) (*endpoint, error) { + if m.Input != "" { + if ok := isValidProtocol(string(m.Input)); !ok { + return nil, ErrInvalidInput } - endpoints = append(endpoints, e) + return newEndpointFromInputString(string(m.Input)) } - return endpoints, nil + // The current InputPolicy should ensure we have a hardcoded input, + // so this error should only be raised if by mistake we change the InputPolicy. + return nil, fmt.Errorf("%w: %s", ErrInvalidInput, "input is mandatory") } -// ErrFailure is the error returned when you set the -// config.ReturnError field to true. -var ErrFailure = errors.New("mocked error") - -// Run implements model.ExperimentMeasurer.Run. -// A single run expects exactly ONE input (endpoint). -func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { - callbacks := args.Callbacks - measurement := args.Measurement - sess := args.Session +// AuthMethod is the authentication method used by a provider. +type AuthMethod string - var endpoint *endpoint - - if measurement.Input == "" { - // if input is null, we get one from the hardcoded list of inputs. - sess.Logger().Info("No input given, picking one endpoint at random") - endpoint = allEndpoints.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] - } +var ( + AuthCertificate = AuthMethod("cert") + AuthUserPass = AuthMethod("userpass") +) - tk := NewTestKeys() +var providerAuthentication = map[string]AuthMethod{ + "riseup": AuthCertificate, + "tunnelbear": AuthUserPass, + "surfshark": AuthUserPass, +} - sess.Logger().Infof("Probing endpoint %s", endpoint.String()) +func hasCredentialsInOptions(cfg Config, method AuthMethod) bool { + switch method { + case AuthCertificate: + ok := cfg.SafeCA != "" && cfg.SafeCert != "" && cfg.SafeKey != "" + return ok + default: + return false + } +} - // TODO: separate pre-connection checks - connResult, err := m.connectAndHandshake(ctx, int64(1), time.Now(), sess, endpoint) +// MaybeGetCredentialsFromOptions overrides authentication info with what user provided in options. +// Each certificate/key can be encoded in base64 so that a single option can be safely represented as command line options. +// This function returns no error if there are no credentials in the passed options, only if failing to parse them. +func MaybeGetCredentialsFromOptions(cfg Config, opts *vpnconfig.OpenVPNOptions, method AuthMethod) (bool, error) { + if ok := hasCredentialsInOptions(cfg, method); !ok { + return false, nil + } + ca, err := extractBase64Blob(cfg.SafeCA) if err != nil { - sess.Logger().Warn("Fatal error while attempting to connect to endpoint, aborting!") - return err + return false, err } - if connResult != nil { - tk.AddConnectionTestKeys(connResult) + opts.CA = []byte(ca) + + key, err := extractBase64Blob(cfg.SafeKey) + if err != nil { + return false, err } - tk.Success = tk.allConnectionsSuccessful() + opts.Key = []byte(key) - callbacks.OnProgress(1.0, "All endpoints probed") - measurement.TestKeys = tk + cert, err := extractBase64Blob(cfg.SafeCert) + if err != nil { + return false, err + } + opts.Cert = []byte(cert) + return true, nil +} - // 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?) +func (m *Measurer) getCredentialsFromAPI( + ctx context.Context, + sess model.ExperimentSession, + provider string, + opts *vpnconfig.OpenVPNOptions) error { + // 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 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()) + return err + } + sess.Logger().Infof("Got credentials from provider: %s", provider) - // 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! + opts.CA = []byte(apiCreds.Config.CA) + opts.Cert = []byte(apiCreds.Config.Cert) + opts.Key = []byte(apiCreds.Config.Key) return nil } -// getCredentialsFromOptionsOrAPI attempts to find valid credentials for the given provider, either +// GetCredentialsFromOptionsOrAPI attempts to find valid credentials for the given provider, either // from the passed Options (cli, oonirun), or from a remote call to the OONI API endpoint. -func (m *Measurer) getCredentialsFromOptionsOrAPI( +func (m *Measurer) GetCredentialsFromOptionsOrAPI( ctx context.Context, sess model.ExperimentSession, provider string) (*vpnconfig.OpenVPNOptions, error) { - // TODO(ainghazal): Ideally, we need to know which authentication methods each provider uses, and this is - // information that the experiment could hardcode. Sticking to Certificate-based auth for riseupvpn. + method, ok := providerAuthentication[provider] + if !ok { + return nil, fmt.Errorf("%w: provider auth unknown: %s", ErrInvalidInput, provider) + } - // get an empty options object to fill with credentials + // Empty options object to fill with credentials. creds := &vpnconfig.OpenVPNOptions{} - cfg := m.config - - if cfg.SafeCA != "" && cfg.SafeCert != "" && cfg.SafeKey != "" { - // We override authentication info with what user provided in options. - // We expect the options to be encoded in base64 so that a single optin can be safely represented as command line options. - ca, err := extractBase64Blob(cfg.SafeCA) + switch method { + case AuthCertificate: + ok, err := MaybeGetCredentialsFromOptions(m.config, creds, method) if err != nil { return nil, err } - creds.CA = []byte(ca) - - key, err := extractBase64Blob(cfg.SafeKey) - if err != nil { - return nil, err + if ok { + return creds, nil } - creds.Key = []byte(key) - - cert, err := extractBase64Blob(cfg.SafeCert) - if err != nil { + // No options passed, so let's get the credentials that inputbuilder should have cached + // for us after hitting the OONI API. + if err := m.getCredentialsFromAPI(ctx, sess, provider, creds); err != nil { return nil, err } - creds.Key = []byte(cert) - - // return options-based credentials return creds, nil - } - - // No options passed, let's hit OONI API for credential distribution. - // 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 - if err != nil { - sess.Logger().Warnf("Error fetching credentials from API: %s", err.Error()) - return nil, err + default: + return nil, fmt.Errorf("%w: method not implemented (%s)", ErrInvalidInput, method) } - sess.Logger().Infof("Got credentials from provider: %s", provider) - creds.CA = []byte(apiCreds.Config.CA) - creds.Cert = []byte(apiCreds.Config.Cert) - creds.Key = []byte(apiCreds.Config.Key) - - return creds, 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) { +// 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) { logger := sess.Logger() - // create a trace for the network dialer - trace := measurexlite.NewTrace(index, zeroTime) - - // TODO(ainghazal): can I pass tags to this tracer? - dialer := trace.NewDialerWithoutResolver(logger) - - // create a vpn tun Device that attempts to dial and performs the handshake - handshakeTracer := vpntracex.NewTracerWithTransactionID(zeroTime, index) - - credentials, err := m.getCredentialsFromOptionsOrAPI(ctx, sess, endpoint.Provider) + credentials, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, endpoint.Provider) if err != nil { return nil, err } - openvpnConfig, err := getVPNConfig(handshakeTracer, endpoint, credentials) + openvpnConfig, err := getOpenVPNConfig(tracer, logger, endpoint, credentials) if err != nil { return nil, err } + // TODO: sanity check (Remote, Port, Proto etc + missing certs) + return openvpnConfig, nil +} - tun, err := tunnel.Start(ctx, dialer, openvpnConfig) +// connectAndHandshake dials a connection and attempts an OpenVPN handshake using that dialer. +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 + tun, err := tunnel.Start(ctx, dialer, openvpnConfig) if err != nil { failure = err.Error() } - defer tun.Close() + if tun != nil { + defer tun.Close() + } handshakeEvents := handshakeTracer.Trace() port, _ := strconv.Atoi(endpoint.Port) @@ -286,7 +292,7 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTim if len(handshakeEvents) != 0 { tFirst = handshakeEvents[0].AtTime tLast = handshakeEvents[len(handshakeEvents)-1].AtTime - bootstrapTime = time.Since(zeroTime).Seconds() + bootstrapTime = tLast - tFirst } return &SingleConnection{ @@ -307,7 +313,6 @@ func (m *Measurer) connectAndHandshake(ctx context.Context, index int64, zeroTim Failure: &failure, Success: err == nil, }, - StartTime: zeroTime, T0: tFirst, T: tLast, Tags: []string{}, @@ -317,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( +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 67c0089bf2..2fff8bd9c5 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -3,55 +3,425 @@ package openvpn_test import ( "context" "errors" + "fmt" "testing" "time" - "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/experiment/example" - "github.com/ooni/probe-cli/v3/internal/legacy/mockable" + "github.com/google/go-cmp/cmp" + vpnconfig "github.com/ooni/minivpn/pkg/config" + vpntracex "github.com/ooni/minivpn/pkg/tracex" + "github.com/ooni/probe-cli/v3/internal/experiment/openvpn" + "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" ) -func TestSuccess(t *testing.T) { - m := example.NewExperimentMeasurer(example.Config{ - SleepTime: int64(2 * time.Millisecond), - }, "example") - if m.ExperimentName() != "example" { +func makeMockSession() *mocks.Session { + return &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 + }, + } +} + +func TestNewExperimentMeasurer(t *testing.T) { + m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn") + if m.ExperimentName() != "openvpn" { t.Fatal("invalid ExperimentName") } - if m.ExperimentVersion() != "0.1.0" { + if m.ExperimentVersion() != "0.1.1" { t.Fatal("invalid ExperimentVersion") } +} + +func TestNewTestKeys(t *testing.T) { + tk := openvpn.NewTestKeys() + if tk.Success != false { + t.Fatal("default success should be false") + } + if tk.NetworkEvents == nil { + t.Fatal("NetworkEvents not initialized") + } + if tk.TCPConnect == nil { + t.Fatal("TCPConnect not initialized") + } + if tk.OpenVPNHandshake == nil { + t.Fatal("OpenVPNHandshake not initialized") + } +} + +func TestMaybeGetCredentialsFromOptions(t *testing.T) { + t.Run("cert auth returns false if cert, key and ca are not all provided", func(t *testing.T) { + cfg := openvpn.Config{ + SafeCA: "base64:Zm9v", + SafeCert: "base64:Zm9v", + } + ok, err := openvpn.MaybeGetCredentialsFromOptions(cfg, &vpnconfig.OpenVPNOptions{}, openvpn.AuthCertificate) + if err != nil { + t.Fatal("should not raise error") + } + if ok { + t.Fatal("expected false") + } + }) + t.Run("cert auth returns ok if cert, key and ca are all provided", func(t *testing.T) { + cfg := openvpn.Config{ + SafeCA: "base64:Zm9v", + SafeCert: "base64:Zm9v", + SafeKey: "base64:Zm9v", + } + opts := &vpnconfig.OpenVPNOptions{} + ok, err := openvpn.MaybeGetCredentialsFromOptions(cfg, opts, openvpn.AuthCertificate) + if err != nil { + t.Fatalf("expected err=nil, got %v", err) + } + if !ok { + t.Fatal("expected true") + } + if diff := cmp.Diff(opts.CA, []byte("foo")); diff != "" { + t.Fatal(diff) + } + if diff := cmp.Diff(opts.Cert, []byte("foo")); diff != "" { + t.Fatal(diff) + } + if diff := cmp.Diff(opts.Key, []byte("foo")); diff != "" { + t.Fatal(diff) + } + }) + t.Run("cert auth returns false and error if CA base64 is bad blob", func(t *testing.T) { + cfg := openvpn.Config{ + SafeCA: "base64:Zm9vaaa", + SafeCert: "base64:Zm9v", + SafeKey: "base64:Zm9v", + } + opts := &vpnconfig.OpenVPNOptions{} + ok, err := openvpn.MaybeGetCredentialsFromOptions(cfg, opts, openvpn.AuthCertificate) + if ok { + t.Fatal("expected false") + } + if !errors.Is(err, openvpn.ErrBadBase64Blob) { + t.Fatalf("expected err=ErrBase64Blob, got %v", err) + } + }) + t.Run("cert auth returns false and error if key base64 is bad blob", func(t *testing.T) { + cfg := openvpn.Config{ + SafeCA: "base64:Zm9v", + SafeCert: "base64:Zm9v", + SafeKey: "base64:Zm9vaaa", + } + opts := &vpnconfig.OpenVPNOptions{} + ok, err := openvpn.MaybeGetCredentialsFromOptions(cfg, opts, openvpn.AuthCertificate) + if ok { + t.Fatal("expected false") + } + if !errors.Is(err, openvpn.ErrBadBase64Blob) { + t.Fatalf("expected err=ErrBase64Blob, got %v", err) + } + }) + t.Run("cert auth returns false and error if cert base64 is bad blob", func(t *testing.T) { + cfg := openvpn.Config{ + SafeCA: "base64:Zm9v", + SafeCert: "base64:Zm9vaaa", + SafeKey: "base64:Zm9v", + } + opts := &vpnconfig.OpenVPNOptions{} + ok, err := openvpn.MaybeGetCredentialsFromOptions(cfg, opts, openvpn.AuthCertificate) + if ok { + t.Fatal("expected false") + } + if !errors.Is(err, openvpn.ErrBadBase64Blob) { + t.Fatalf("expected err=ErrBase64Blob, got %v", err) + } + }) + t.Run("userpass auth returns error, not yet implemented", func(t *testing.T) { + cfg := openvpn.Config{} + ok, err := openvpn.MaybeGetCredentialsFromOptions(cfg, &vpnconfig.OpenVPNOptions{}, openvpn.AuthUserPass) + if ok { + t.Fatal("expected false") + } + if err != nil { + t.Fatalf("expected err=nil, got %v", err) + } + }) + +} + +func TestGetCredentialsFromOptionsOrAPI(t *testing.T) { + t.Run("non-registered provider raises error", func(t *testing.T) { + m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn").(openvpn.Measurer) + ctx := context.Background() + sess := makeMockSession() + opts, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, "nsa") + if !errors.Is(err, openvpn.ErrInvalidInput) { + t.Fatalf("expected err=ErrInvalidInput, got %v", err) + } + if opts != nil { + t.Fatal("expected opts=nil") + } + }) + t.Run("providers with userpass auth method raise error, not yet implemented", func(t *testing.T) { + m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn").(openvpn.Measurer) + ctx := context.Background() + sess := makeMockSession() + opts, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, "tunnelbear") + if !errors.Is(err, openvpn.ErrInvalidInput) { + t.Fatalf("expected err=ErrInvalidInput, got %v", err) + } + if opts != nil { + t.Fatal("expected opts=nil") + } + }) + t.Run("known cert auth provider and creds in options is ok", func(t *testing.T) { + config := openvpn.Config{ + SafeCA: "base64:Zm9v", + SafeCert: "base64:Zm9v", + SafeKey: "base64:Zm9v", + } + m := openvpn.NewExperimentMeasurer(config, "openvpn").(openvpn.Measurer) + ctx := context.Background() + sess := makeMockSession() + opts, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, "riseup") + if err != nil { + t.Fatalf("expected err=nil, got %v", err) + } + if opts == nil { + t.Fatal("expected non-nil options") + } + }) + t.Run("known cert auth provider and bad creds in options returns error", func(t *testing.T) { + config := openvpn.Config{ + SafeCA: "base64:Zm9v", + SafeCert: "base64:Zm9v", + SafeKey: "base64:Zm9vaaa", + } + m := openvpn.NewExperimentMeasurer(config, "openvpn").(openvpn.Measurer) + ctx := context.Background() + sess := makeMockSession() + opts, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, "riseup") + if !errors.Is(err, openvpn.ErrBadBase64Blob) { + t.Fatalf("expected err=ErrBadBase64, got %v", err) + } + if opts != nil { + t.Fatal("expected nil opts") + } + }) + t.Run("known cert auth provider with null options hits the api", func(t *testing.T) { + config := openvpn.Config{} + m := openvpn.NewExperimentMeasurer(config, "openvpn").(openvpn.Measurer) + ctx := context.Background() + sess := makeMockSession() + opts, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, "riseup") + if err != nil { + t.Fatalf("expected err=nil, got %v", err) + } + if opts == nil { + t.Fatalf("expected not-nil options, got %v", opts) + } + }) + t.Run("known cert auth provider with null options hits the api and raises error if api fails", func(t *testing.T) { + config := openvpn.Config{} + m := openvpn.NewExperimentMeasurer(config, "openvpn").(openvpn.Measurer) + ctx := context.Background() + + someError := errors.New("some error") + sess := makeMockSession() + sess.MockFetchOpenVPNConfig = func(context.Context, string, string) (*model.OOAPIVPNProviderConfig, error) { + return nil, someError + } + + opts, err := m.GetCredentialsFromOptionsOrAPI(ctx, sess, "riseup") + if !errors.Is(err, someError) { + t.Fatalf("expected err=someError, got %v", err) + } + if opts != nil { + t.Fatalf("expected nil options, got %v", opts) + } + }) +} + +func TestAddConnectionTestKeys(t *testing.T) { + t.Run("append connection result to empty keys", func(t *testing.T) { + tk := openvpn.NewTestKeys() + sc := &openvpn.SingleConnection{ + TCPConnect: &model.ArchivalTCPConnectResult{ + IP: "1.1.1.1", + Port: 1194, + Status: model.ArchivalTCPConnectStatus{ + Blocked: new(bool), + Failure: new(string), + Success: false, + }, + T0: 0.1, + T: 0.9, + Tags: []string{}, + TransactionID: 1, + }, + OpenVPNHandshake: &openvpn.ArchivalOpenVPNHandshakeResult{ + BootstrapTime: 1, + Endpoint: "aa", + IP: "1.1.1.1", + Port: 1194, + Transport: "tcp", + Provider: "unknown", + OpenVPNOptions: openvpn.OpenVPNOptions{}, + Status: openvpn.ArchivalOpenVPNConnectStatus{}, + T0: 0, + T: 0, + Tags: []string{}, + TransactionID: 1, + }, + NetworkEvents: []*vpntracex.Event{}, + } + tk.AddConnectionTestKeys(sc) + if diff := cmp.Diff(tk.TCPConnect[0], sc.TCPConnect); diff != "" { + t.Fatal(diff) + } + if diff := cmp.Diff(tk.OpenVPNHandshake[0], sc.OpenVPNHandshake); diff != "" { + t.Fatal(diff) + } + if diff := cmp.Diff(tk.NetworkEvents, sc.NetworkEvents); diff != "" { + t.Fatal(diff) + } + }) +} + +func TestAllConnectionsSuccessful(t *testing.T) { + t.Run("all success", func(t *testing.T) { + tk := openvpn.NewTestKeys() + tk.OpenVPNHandshake = []*openvpn.ArchivalOpenVPNHandshakeResult{ + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, + } + if tk.AllConnectionsSuccessful() != true { + t.Fatal("expected all connections successful") + } + }) + t.Run("one failure", func(t *testing.T) { + tk := openvpn.NewTestKeys() + tk.OpenVPNHandshake = []*openvpn.ArchivalOpenVPNHandshakeResult{ + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: false}}, + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, + } + if tk.AllConnectionsSuccessful() != false { + t.Fatal("expected false") + } + }) + t.Run("all failures", func(t *testing.T) { + tk := openvpn.NewTestKeys() + tk.OpenVPNHandshake = []*openvpn.ArchivalOpenVPNHandshakeResult{ + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: false}}, + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: false}}, + {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: false}}, + } + if tk.AllConnectionsSuccessful() != false { + t.Fatal("expected false") + } + }) +} + +func TestBadInputFailure(t *testing.T) { + m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn") ctx := context.Background() - sess := &mockable.Session{MockableLogger: log.Log} + sess := makeMockSession() callbacks := model.NewPrinterCallbacks(sess.Logger()) measurement := new(model.Measurement) + measurement.Input = "openvpn://badprovider/?address=aa" args := &model.ExperimentArgs{ Callbacks: callbacks, Measurement: measurement, Session: sess, } err := m.Run(ctx, args) - if err != nil { - t.Fatal(err) + if !errors.Is(err, openvpn.ErrInvalidInput) { + t.Fatalf("expected ErrInvalidInput, got %v", err) + } +} + +func TestVPNInput(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") } + // TODO -- do a real test, get credentials etc. +} + +func TestMeasurer_FetchProviderCredentials(t *testing.T) { + t.Run("Measurer.FetchProviderCredentials calls method in session", func(t *testing.T) { + m := openvpn.NewExperimentMeasurer( + openvpn.Config{}, + "openvpn").(openvpn.Measurer) + + sess := makeMockSession() + _, err := m.FetchProviderCredentials( + context.Background(), + sess, "riseup") + if err != nil { + t.Fatal("expected no error") + } + }) + t.Run("Measurer.FetchProviderCredentials raises error if API calls fail", func(t *testing.T) { + someError := errors.New("unexpected") + + m := openvpn.NewExperimentMeasurer( + openvpn.Config{}, + "openvpn").(openvpn.Measurer) + + sess := makeMockSession() + sess.MockFetchOpenVPNConfig = func(context.Context, string, string) (*model.OOAPIVPNProviderConfig, error) { + return nil, someError + } + _, err := m.FetchProviderCredentials( + context.Background(), + sess, "riseup") + if !errors.Is(err, someError) { + t.Fatalf("expected error %v, got %v", someError, err) + } + }) + } -func TestFailure(t *testing.T) { - m := example.NewExperimentMeasurer(example.Config{ - SleepTime: int64(2 * time.Millisecond), - ReturnError: true, - }, "example") +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 := &mockable.Session{MockableLogger: log.Log} + 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: new(model.Measurement), + 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 !errors.Is(err, example.ErrFailure) { - t.Fatal("expected an error here") + if err != nil { + t.Fatal(err) } } diff --git a/internal/legacy/mockable/mockable.go b/internal/legacy/mockable/mockable.go index 2f39a254c0..cac5718aa6 100644 --- a/internal/legacy/mockable/mockable.go +++ b/internal/legacy/mockable/mockable.go @@ -26,6 +26,7 @@ type Session struct { MockableFetchPsiphonConfigErr error MockableFetchTorTargetsResult map[string]model.OOAPITorTarget MockableFetchTorTargetsErr error + MockableFetchOpenVPNConfigErr error MockableCheckInInfo *model.OOAPICheckInResultNettests MockableCheckInErr error MockableResolverIP string @@ -34,6 +35,7 @@ type Session struct { MockableTempDir string MockableTorArgs []string MockableTorBinary string + MockableOpenVPNConfig *model.OOAPIVPNProviderConfig MockableTunnelDir string MockableUserAgent string } @@ -60,6 +62,12 @@ func (sess *Session) FetchTorTargets( return sess.MockableFetchTorTargetsResult, sess.MockableFetchTorTargetsErr } +// FetchOpenVPNConfig implements ExperimentSession.FetchOpenVPNConfig +func (sess *Session) FetchOpenVPNConfig( + ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) { + return sess.MockableOpenVPNConfig, sess.MockableFetchOpenVPNConfigErr +} + // KeyValueStore returns the configured key-value store. func (sess *Session) KeyValueStore() model.KeyValueStore { return &kvstore.Memory{} diff --git a/internal/mocks/session.go b/internal/mocks/session.go index c0750f7169..b26611268c 100644 --- a/internal/mocks/session.go +++ b/internal/mocks/session.go @@ -18,6 +18,9 @@ type Session struct { MockFetchTorTargets func( ctx context.Context, cc string) (map[string]model.OOAPITorTarget, error) + MockFetchOpenVPNConfig func( + ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) + MockKeyValueStore func() model.KeyValueStore MockLogger func() model.Logger @@ -70,6 +73,11 @@ func (sess *Session) FetchPsiphonConfig(ctx context.Context) ([]byte, error) { return sess.MockFetchPsiphonConfig(ctx) } +func (sess *Session) FetchOpenVPNConfig( + ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) { + return sess.MockFetchOpenVPNConfig(ctx, provider, cc) +} + func (sess *Session) FetchTorTargets( ctx context.Context, cc string) (map[string]model.OOAPITorTarget, error) { return sess.MockFetchTorTargets(ctx, cc) diff --git a/internal/model/ooapi.go b/internal/model/ooapi.go index 9786f802ca..da89835aff 100644 --- a/internal/model/ooapi.go +++ b/internal/model/ooapi.go @@ -100,7 +100,7 @@ type OOAPICheckReportIDResponse struct { } // OOAPIVPNProviderConfig is a minimal valid configuration subset for the openvpn experiment; at the moment it provides -// only credentials valid for endpoints in a provider. +// credentials valid for endpoints in a provider, and a list of inputs to be tested on this provider. type OOAPIVPNProviderConfig struct { // Provider is the label for this provider. Provider string `json:"provider,omitempty"` @@ -126,7 +126,7 @@ type OOAPIVPNProviderConfig struct { // Inputs is an array of valid endpoints for this provider. Inputs []string - // DateUpdated is when this credential was last updated in the server database. + // DateUpdated is when the credential set was last updated in the server database. DateUpdated time.Time `json:"date_updated"` } From bb25311ec1d190da4467e33a75b098177c37f503 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 3 Apr 2024 15:41:32 +0200 Subject: [PATCH 20/53] update docs --- internal/experiment/openvpn/endpoint.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 7b8ad86b49..0d3c9616e2 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -45,8 +45,8 @@ type endpoint struct { // newEndpointFromInputString constructs an endpoint after parsing an input string. // // The input URI is in the form: -// "openvpn://1.2.3.4:443/udp/&provider=tunnelbear" -// "openvpn+obfs4://1.2.3.4:443/tcp/&provider=riseup&cert=deadbeef" +// "openvpn://provider.corp/?address=1.2.3.4:1194&transport=udp +// "openvpn+obfs4://provider.corp/address=1.2.3.4:1194?&cert=deadbeef&iat=0" func newEndpointFromInputString(uri string) (*endpoint, error) { parsedURL, err := url.Parse(uri) if err != nil { @@ -66,9 +66,7 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { if provider == "" { return nil, fmt.Errorf("%w: expected provider as host: %s", ErrInvalidInput, parsedURL.Host) } - if provider != "riseup" { - // I am hardcoding a single provider at the moment. - // I need to figure out a way to pass info for arbitrary providers as options instead. + if !isValidProvider(provider) { return nil, fmt.Errorf("%w: unknown provider: %s", ErrInvalidInput, provider) } @@ -176,6 +174,8 @@ var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ }, } +// 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. func isValidProvider(provider string) bool { _, ok := defaultOptionsByProvider[provider] return ok From dfd8c834f1125e759526254a1c8f2517092a1d8e Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 3 Apr 2024 15:50:20 +0200 Subject: [PATCH 21/53] move archival structs to internal/model --- internal/experiment/openvpn/archival.go | 54 --------------------- internal/experiment/openvpn/openvpn.go | 22 ++++----- internal/experiment/openvpn/openvpn_test.go | 30 ++++++------ internal/model/archival.go | 35 +++++++++++++ 4 files changed, 61 insertions(+), 80 deletions(-) delete mode 100644 internal/experiment/openvpn/archival.go diff --git a/internal/experiment/openvpn/archival.go b/internal/experiment/openvpn/archival.go deleted file mode 100644 index c10779217f..0000000000 --- a/internal/experiment/openvpn/archival.go +++ /dev/null @@ -1,54 +0,0 @@ -package openvpn - -// TODO(ainghazal): move to ooni internal archival package when consolidated. - -// OpenVPNOptions is a subset of [vpnconfig.OpenVPNOptions] that we want to include -// in the archived result. -type OpenVPNOptions struct { - Auth string `json:"auth,omitempty"` - Cipher string `json:"cipher,omitempty"` - Compression string `json:"compression,omitempty"` -} - -// ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake. -type ArchivalOpenVPNHandshakeResult struct { - BootstrapTime float64 `json:"bootstrap_time,omitempty"` - Endpoint string `json:"endpoint"` - IP string `json:"ip"` - Port int `json:"port"` - Transport string `json:"transport"` - Provider string `json:"provider"` - OpenVPNOptions OpenVPNOptions `json:"openvpn_options"` - Status ArchivalOpenVPNConnectStatus `json:"status"` - - // T0 can differ from StartTime because for TCP we take T0 *after* dialing has successfully completed. - // This might be counterintuitive, review. - T0 float64 `json:"t0,omitempty"` - T float64 `json:"t"` - Tags []string `json:"tags"` - TransactionID int64 `json:"transaction_id,omitempty"` -} - -// ArchivalOpenVPNConnectStatus is the status of ArchivalOpenVPNConnectResult. -type ArchivalOpenVPNConnectStatus struct { - Blocked *bool `json:"blocked,omitempty"` - Failure *string `json:"failure"` - Success bool `json:"success"` -} - -type ArchivalNetworkEvent struct { - // TODO(ainghazal): could properly propagate I/O errors during the handshake. - // Right now the network events WILL NOT append any RW error, but I think following - // other experiments one would expect such errors to be appended as failures in the operation. - // However, getting the start boundary for the failed operation might be tricky, - // I need to think about it. - Address string `json:"address,omitempty"` - Failure *string `json:"failure"` - NumBytes int64 `json:"num_bytes,omitempty"` - Operation string `json:"operation"` - Proto string `json:"proto,omitempty"` - T0 float64 `json:"t0,omitempty"` - T float64 `json:"t"` - TransactionID int64 `json:"transaction_id,omitempty"` - Tags []string `json:"tags,omitempty"` -} diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 2dd0f99120..fe55f03806 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -40,10 +40,10 @@ type Config struct { // TestKeys contains the experiment's result. type TestKeys struct { - Success bool `json:"success"` - NetworkEvents []*vpntracex.Event `json:"network_events"` - TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` - OpenVPNHandshake []*ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` + Success bool `json:"success"` + NetworkEvents []*vpntracex.Event `json:"network_events"` + TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` + OpenVPNHandshake []*model.ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` } // NewTestKeys creates new openvpn TestKeys. @@ -52,15 +52,15 @@ func NewTestKeys() *TestKeys { Success: false, NetworkEvents: []*vpntracex.Event{}, TCPConnect: []*model.ArchivalTCPConnectResult{}, - OpenVPNHandshake: []*ArchivalOpenVPNHandshakeResult{}, + OpenVPNHandshake: []*model.ArchivalOpenVPNHandshakeResult{}, } } // SingleConnection contains the results of a single handshake. type SingleConnection struct { - TCPConnect *model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` - OpenVPNHandshake *ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` - NetworkEvents []*vpntracex.Event `json:"network_events"` + TCPConnect *model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` + OpenVPNHandshake *model.ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` + NetworkEvents []*vpntracex.Event `json:"network_events"` // TODO(ainghazal): make sure to document in the spec that these network events only cover the handshake. // TODO(ainghazal): in the future, we will want to store more operations under this struct for a single connection, // like pingResults or urlgetter calls. @@ -297,19 +297,19 @@ func (m *Measurer) connectAndHandshake( return &SingleConnection{ TCPConnect: trace.FirstTCPConnectOrNil(), - OpenVPNHandshake: &ArchivalOpenVPNHandshakeResult{ + OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ BootstrapTime: bootstrapTime, Endpoint: endpoint.String(), IP: endpoint.IPAddr, Port: port, Transport: endpoint.Transport, Provider: endpoint.Provider, - OpenVPNOptions: OpenVPNOptions{ + OpenVPNOptions: model.ArchivalOpenVPNOptions{ Cipher: openvpnConfig.OpenVPNOptions().Cipher, Auth: openvpnConfig.OpenVPNOptions().Auth, Compression: string(openvpnConfig.OpenVPNOptions().Compress), }, - Status: ArchivalOpenVPNConnectStatus{ + Status: model.ArchivalOpenVPNConnectStatus{ Failure: &failure, Success: err == nil, }, diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index 2fff8bd9c5..0952d285fb 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -273,15 +273,15 @@ func TestAddConnectionTestKeys(t *testing.T) { Tags: []string{}, TransactionID: 1, }, - OpenVPNHandshake: &openvpn.ArchivalOpenVPNHandshakeResult{ + OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ BootstrapTime: 1, Endpoint: "aa", IP: "1.1.1.1", Port: 1194, Transport: "tcp", Provider: "unknown", - OpenVPNOptions: openvpn.OpenVPNOptions{}, - Status: openvpn.ArchivalOpenVPNConnectStatus{}, + OpenVPNOptions: model.ArchivalOpenVPNOptions{}, + Status: model.ArchivalOpenVPNConnectStatus{}, T0: 0, T: 0, Tags: []string{}, @@ -305,10 +305,10 @@ func TestAddConnectionTestKeys(t *testing.T) { func TestAllConnectionsSuccessful(t *testing.T) { t.Run("all success", func(t *testing.T) { tk := openvpn.NewTestKeys() - tk.OpenVPNHandshake = []*openvpn.ArchivalOpenVPNHandshakeResult{ - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, + tk.OpenVPNHandshake = []*model.ArchivalOpenVPNHandshakeResult{ + {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, + {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, + {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, } if tk.AllConnectionsSuccessful() != true { t.Fatal("expected all connections successful") @@ -316,10 +316,10 @@ func TestAllConnectionsSuccessful(t *testing.T) { }) t.Run("one failure", func(t *testing.T) { tk := openvpn.NewTestKeys() - tk.OpenVPNHandshake = []*openvpn.ArchivalOpenVPNHandshakeResult{ - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: false}}, - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: true}}, + tk.OpenVPNHandshake = []*model.ArchivalOpenVPNHandshakeResult{ + {Status: model.ArchivalOpenVPNConnectStatus{Success: false}}, + {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, + {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, } if tk.AllConnectionsSuccessful() != false { t.Fatal("expected false") @@ -327,10 +327,10 @@ func TestAllConnectionsSuccessful(t *testing.T) { }) t.Run("all failures", func(t *testing.T) { tk := openvpn.NewTestKeys() - tk.OpenVPNHandshake = []*openvpn.ArchivalOpenVPNHandshakeResult{ - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: false}}, - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: false}}, - {Status: openvpn.ArchivalOpenVPNConnectStatus{Success: false}}, + tk.OpenVPNHandshake = []*model.ArchivalOpenVPNHandshakeResult{ + {Status: model.ArchivalOpenVPNConnectStatus{Success: false}}, + {Status: model.ArchivalOpenVPNConnectStatus{Success: false}}, + {Status: model.ArchivalOpenVPNConnectStatus{Success: false}}, } if tk.AllConnectionsSuccessful() != false { t.Fatal("expected false") diff --git a/internal/model/archival.go b/internal/model/archival.go index ecef3c2427..86c2b52aec 100644 --- a/internal/model/archival.go +++ b/internal/model/archival.go @@ -392,3 +392,38 @@ type ArchivalNetworkEvent struct { TransactionID int64 `json:"transaction_id,omitempty"` Tags []string `json:"tags,omitempty"` } + +// +// OpenVPN +// + +// ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake. +type ArchivalOpenVPNHandshakeResult struct { + BootstrapTime float64 `json:"bootstrap_time,omitempty"` + Endpoint string `json:"endpoint"` + IP string `json:"ip"` + Port int `json:"port"` + Transport string `json:"transport"` + Provider string `json:"provider"` + OpenVPNOptions ArchivalOpenVPNOptions `json:"openvpn_options"` + Status ArchivalOpenVPNConnectStatus `json:"status"` + T0 float64 `json:"t0,omitempty"` + T float64 `json:"t"` + Tags []string `json:"tags"` + TransactionID int64 `json:"transaction_id,omitempty"` +} + +// ArchivalOpenVPNOptions is a subset of [vpnconfig.OpenVPNOptions] that we want to include +// in the archived result. +type ArchivalOpenVPNOptions struct { + Auth string `json:"auth,omitempty"` + Cipher string `json:"cipher,omitempty"` + Compression string `json:"compression,omitempty"` +} + +// ArchivalOpenVPNConnectStatus is the status of ArchivalOpenVPNConnectResult. +type ArchivalOpenVPNConnectStatus struct { + Blocked *bool `json:"blocked,omitempty"` + Failure *string `json:"failure"` + Success bool `json:"success"` +} From d6d7ed8b6131de2c29da93bf981e6eff6fb68f60 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 3 Apr 2024 15:56:34 +0200 Subject: [PATCH 22/53] move list of enabled providers to expriment/openvpn --- internal/engine/inputloader.go | 10 +++------- internal/experiment/openvpn/endpoint.go | 11 +++++++++-- internal/experiment/openvpn/openvpn.go | 1 - 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index 2ce8633e51..988da0b50b 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -330,19 +330,15 @@ func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.O return reply.WebConnectivity.URLs, nil } -// These are the providers that are enabled in the API. -var openvpnDefaultProviders = []string{ - "riseup", -} - // loadRemoteOpenVPN loads openvpn inputs from a remote source. func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLInfo, error) { // VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo], - // since OOAPIURLInfo is oriented twards webconnectivity, + // since OOAPIURLInfo is oriented towards webconnectivity, // but we force VPN targets in the URL and ignore all the other fields. urls := make([]model.OOAPIURLInfo, 0) - for _, provider := range openvpnDefaultProviders { + // The openvpn experiment contains an array of the providers that the API knows about. + for _, provider := range openvpn.APIEnabledProviders { reply, err := il.vpnConfig(ctx, provider) if err != nil { break diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 0d3c9616e2..d8aacde16a 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -19,8 +19,8 @@ var ( ) // endpoint is a single endpoint to be probed. -// The information contained in here is not generally not sufficient to complete a connection: -// we need more info, as cipher selection or obfuscating proxy credentials. +// The information contained in here is not sufficient to complete a connection: +// we need to augment it with more info, as cipher selection or obfuscating proxy credentials. type endpoint struct { // IPAddr is the IP Address for this endpoint. IPAddr string @@ -174,6 +174,13 @@ var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ }, } +// 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. +var APIEnabledProviders = []string{ + "riseup", +} + // 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. func isValidProvider(provider string) bool { diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index fe55f03806..5d500e7383 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -31,7 +31,6 @@ var ( // 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"` From e291c0e5d893081c8e73c66085f7cd675c18ce3f Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 4 Apr 2024 11:30:01 +0200 Subject: [PATCH 23/53] do not use credentials --- internal/probeservices/openvpn.go | 7 +------ internal/registry/openvpn.go | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index 281d74ee87..802fbdfa50 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -12,12 +12,7 @@ import ( // It accepts the provider label, and the country code for the probe, in case the API wants to // return different targets to us depending on where we are located. func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (result model.OOAPIVPNProviderConfig, err error) { - _, auth, err := c.GetCredsAndAuth() - if err != nil { - return model.OOAPIVPNProviderConfig{}, err - } - s := fmt.Sprintf("Bearer %s", auth.Token) - client := c.APIClientTemplate.BuildWithAuthorization(s) + client := c.APIClientTemplate.Build() query := url.Values{} query.Add("country_code", cc) diff --git a/internal/registry/openvpn.go b/internal/registry/openvpn.go index 8a4a5728e0..7dba22e807 100644 --- a/internal/registry/openvpn.go +++ b/internal/registry/openvpn.go @@ -17,7 +17,7 @@ func init() { ) }, config: &openvpn.Config{}, - enabledByDefault: false, + enabledByDefault: true, interruptible: true, inputPolicy: model.InputOrQueryBackend, } From 8934898dbdc9fcd013b74fec08cbf81b26b5013d Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 4 Apr 2024 17:28:35 +0200 Subject: [PATCH 24/53] remove test file From 25c98a97b61c934629d8293d97d705ea1b35eddd Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Fri, 26 Apr 2024 11:43:27 +0200 Subject: [PATCH 25/53] go mod tidy --- go.mod | 1 + go.sum | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/go.mod b/go.mod index 7fecb15e37..506266d1ad 100644 --- a/go.mod +++ b/go.mod @@ -26,6 +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/netem v0.0.0-20240208095707-608dcbcd82b8 github.com/ooni/oocrypto v0.6.1 github.com/ooni/oohttp v0.7.2 diff --git a/go.sum b/go.sum index 726de7afcd..c54eab98f7 100644 --- a/go.sum +++ b/go.sum @@ -192,6 +192,8 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/gopacket v1.1.19 h1:ves8RnFZPGiFnTS0uPQStjwru6uO6h+nlr9j6fL7kF8= github.com/google/gopacket v1.1.19/go.mod h1:iJ8V8n6KS+z2U1A8pUwu8bW5SyEMkXJB8Yo/Vo+TKTo= +github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no= +github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/pprof v0.0.0-20230207041349-798e818bf904/go.mod h1:uglQLonpP8qtYCYyzA+8c/9qtqgA3qsXGYqCPKARAFg= github.com/google/pprof v0.0.0-20240509144519-723abb6459b7 h1:velgFPYr1X9TDwLIfkV7fWqsFlf7TeP11M/7kPd/dVI= github.com/google/pprof v0.0.0-20240509144519-723abb6459b7/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw= @@ -350,6 +352,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/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.1 h1:D0fGokmHoVKGBy39RxPxK77ov0Ob9Z5pdx4vKA6vpWk= From 4ef2b803b0e1c0b352d3b5781ada75436e3fd1ea Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Tue, 30 Apr 2024 18:04:07 +0200 Subject: [PATCH 26/53] add expectations for openvpn --- internal/registry/factory_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/registry/factory_test.go b/internal/registry/factory_test.go index b407f074ea..dd32ddafb3 100644 --- a/internal/registry/factory_test.go +++ b/internal/registry/factory_test.go @@ -455,6 +455,11 @@ func TestNewFactory(t *testing.T) { inputPolicy: model.InputNone, interruptible: true, }, + "openvpn": { + enabledByDefault: true, + inputPolicy: model.InputOrQueryBackend, + interruptible: true, + }, "portfiltering": { enabledByDefault: true, inputPolicy: model.InputNone, From 779ab03677d22a9fcc6ad04a9fcdf3f9dec8cb62 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Tue, 9 Apr 2024 00:38:04 +0200 Subject: [PATCH 27/53] add vpn suffix, there seems to be a mismatch in the provider name --- internal/probeservices/openvpn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index 802fbdfa50..fa8dca222c 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -18,7 +18,7 @@ func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (re err = client.GetJSONWithQuery( ctx, - fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%s/", provider), + fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%svpn/", provider), query, &result, ) From 47aa87d95cc086f0eea06e03d991534974ae4cf5 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 11 Apr 2024 21:49:07 +0200 Subject: [PATCH 28/53] add tests for inputloader --- internal/engine/inputloader_test.go | 61 ++++++++++++++++++++++++++++- internal/model/ooapi.go | 34 ++++++++-------- 2 files changed, 78 insertions(+), 17 deletions(-) diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index 42a2abac94..6561e37b20 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -9,6 +9,7 @@ import ( "strings" "syscall" "testing" + "time" "github.com/apex/log" "github.com/google/go-cmp/cmp" @@ -556,6 +557,64 @@ func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) { } } +func TestInputLoaderOpenVPNSuccessWithNoInput(t *testing.T) { + il := &InputLoader{ + ExperimentName: "openvpn", + InputPolicy: model.InputOrQueryBackend, + Session: &InputLoaderMockableSession{ + Error: nil, + }, + } + _, err := il.loadRemote(context.Background()) + if err != nil { + t.Fatal("we did not expect an error") + } +} + +func TestInputLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) { + il := &InputLoader{ + ExperimentName: "openvpn", + InputPolicy: model.InputOrQueryBackend, + Session: &InputLoaderMockableSession{ + Error: nil, + FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{ + Provider: "riseup", + Inputs: []string{ + "openvpn://foo.corp/?address=1.1.1.1:1194&transport=tcp", + }, + DateUpdated: time.Now(), + }, + }, + } + out, err := il.loadRemote(context.Background()) + if err != nil { + t.Fatal("we did not expect an error") + } + if len(out) != 1 { + t.Fatal("we expected output of len=1") + } +} + +func TestInputLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) { + expected := errors.New("mocked API error") + il := &InputLoader{ + ExperimentName: "openvpn", + InputPolicy: model.InputOrQueryBackend, + Session: &InputLoaderMockableSession{ + Error: expected, + }, + } + out, err := il.loadRemote(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(out) != 2 { + t.Fatal("we expected 2 fallback URLs") + } +} + +// TODO: WIP marker ------------------------------- + func TestPreventMistakesWithCategories(t *testing.T) { input := []model.OOAPIURLInfo{{ CategoryCode: "NEWS", @@ -692,7 +751,7 @@ func TestStringListToModelURLInfoWithError(t *testing.T) { expected := errors.New("mocked error") output, err := stringListToModelURLInfo(input, expected) if !errors.Is(err, expected) { - t.Fatal("no the error we expected", err) + t.Fatal("not the error we expected", err) } if output != nil { t.Fatal("unexpected nil output") diff --git a/internal/model/ooapi.go b/internal/model/ooapi.go index da89835aff..d0249e58aa 100644 --- a/internal/model/ooapi.go +++ b/internal/model/ooapi.go @@ -99,6 +99,23 @@ type OOAPICheckReportIDResponse struct { V int64 `json:"v"` } +type OOAPIVPNConfig struct { + // CA is the Certificate Authority for the endpoints by this provider. + CA string `json:"ca"` + + // Cert is a valid certificate, for providers that use x509 certificate authentication. + Cert string `json:"cert,omitempty"` + + // Key is a valid key, for providers that use x509 certificate authentication. + Key string `json:"key,omitempty"` + + // Username is a valid username, for providers that use password authentication. + Username string `json:"username,omitempty"` + + // Password is a valid password, for providers that use password authentication. + Password string `json:"password,omitempty"` +} + // OOAPIVPNProviderConfig is a minimal valid configuration subset for the openvpn experiment; at the moment it provides // credentials valid for endpoints in a provider, and a list of inputs to be tested on this provider. type OOAPIVPNProviderConfig struct { @@ -106,22 +123,7 @@ type OOAPIVPNProviderConfig struct { Provider string `json:"provider,omitempty"` // Config is the provider-specific VPN Config. - Config *struct { - // CA is the Certificate Authority for the endpoints by this provider. - CA string `json:"ca"` - - // Cert is a valid certificate, for providers that use x509 certificate authentication. - Cert string `json:"cert,omitempty"` - - // Key is a valid key, for providers that use x509 certificate authentication. - Key string `json:"key,omitempty"` - - // Username is a valid username, for providers that use password authentication. - Username string `json:"username,omitempty"` - - // Password is a valid password, for providers that use password authentication. - Password string `json:"password,omitempty"` - } `json:"config"` + Config *OOAPIVPNConfig `json:"config"` // Inputs is an array of valid endpoints for this provider. Inputs []string From 4a680322ca9cd9494eaa01a2d31c8ca82db27b05 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Thu, 2 May 2024 17:15:07 +0200 Subject: [PATCH 29/53] update after httpclientx api change --- internal/probeservices/openvpn.go | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index fa8dca222c..7488dd80a5 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -5,22 +5,33 @@ import ( "fmt" "net/url" + "github.com/ooni/probe-cli/v3/internal/httpclientx" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/urlx" ) // FetchOpenVPNConfig returns valid configuration for the openvpn experiment. // It accepts the provider label, and the country code for the probe, in case the API wants to // return different targets to us depending on where we are located. func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (result model.OOAPIVPNProviderConfig, err error) { - client := c.APIClientTemplate.Build() + // create query string query := url.Values{} query.Add("country_code", cc) - err = client.GetJSONWithQuery( - ctx, + URL, err := urlx.ResolveReference(c.BaseURL, fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%svpn/", provider), - query, - &result, - ) - return + query.Encode()) + if err != nil { + return + } + + // get response + // + // use a model.DiscardLogger to avoid logging bridges + return httpclientx.GetJSON[model.OOAPIVPNProviderConfig](ctx, URL, &httpclientx.Config{ + Client: c.HTTPClient, + Host: c.Host, + Logger: model.DiscardLogger, + UserAgent: c.UserAgent, + }) } From c11fbce58ba8380f3f38f9359b4fbd04dc5f3830 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Mon, 6 May 2024 15:32:47 +0200 Subject: [PATCH 30/53] fix after rebase --- internal/engine/inputloader_test.go | 2 +- internal/engine/session.go | 2 +- internal/experiment/openvpn/openvpn_test.go | 8 +------- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index 6561e37b20..cc9efd6dda 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -464,7 +464,7 @@ func (sess *InputLoaderMockableSession) CheckIn( if sess.Output == nil && sess.Error == nil { return nil, errors.New("both Output and Error are nil") } - return sess.CheckinOutput, sess.Error + return sess.Output, sess.Error } // FetchOpenVPNConfig implements InputLoaderSession.FetchOpenVPNConfig. diff --git a/internal/engine/session.go b/internal/engine/session.go index d3c34fcd83..0a31182182 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -380,7 +380,7 @@ func (s *Session) FetchOpenVPNConfig( if config, ok := s.vpnConfig[provider]; ok { return &config, nil } - clnt, err := s.NewOrchestraClient(ctx) + clnt, err := s.newOrchestraClient(ctx) if err != nil { return nil, err } diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index 0952d285fb..f76663b5b0 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -23,13 +23,7 @@ func makeMockSession() *mocks.Session { 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\"" - }{ + Config: &model.OOAPIVPNConfig{ CA: "ca", Cert: "cert", Key: "key", From f169fb6c9c1678de2c34b2986e2104daec894b0c Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Mon, 6 May 2024 15:56:39 +0200 Subject: [PATCH 31/53] test stub --- internal/probeservices/openvpn_test.go | 12 ++++++++++++ internal/probeservices/tor_test.go | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 internal/probeservices/openvpn_test.go diff --git a/internal/probeservices/openvpn_test.go b/internal/probeservices/openvpn_test.go new file mode 100644 index 0000000000..5b14664815 --- /dev/null +++ b/internal/probeservices/openvpn_test.go @@ -0,0 +1,12 @@ +package probeservices + +import ( + "testing" +) + +func TestFetchOpenVPNConfig(t *testing.T) { + + t.Run("is working as intended with a local test server", func(t *testing.T) { + // TODO(ain) + }) +} diff --git a/internal/probeservices/tor_test.go b/internal/probeservices/tor_test.go index bb91771b23..79dd2adc2c 100644 --- a/internal/probeservices/tor_test.go +++ b/internal/probeservices/tor_test.go @@ -242,7 +242,7 @@ func TestFetchTorTargets(t *testing.T) { t.Run("when we're not registered", func(t *testing.T) { clnt := newclient() - // With explicitly empty state so it's pretty obvioust there's no state + // With explicitly empty state so it's pretty obvious there's no state state := State{} // force the state to be empty From 337230c81fac541ab6432a5703e033e4c164b1e4 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Mon, 6 May 2024 16:14:52 +0200 Subject: [PATCH 32/53] adapt after merge/api change --- internal/probeservices/openvpn.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index 7488dd80a5..4a87ca70a5 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -27,11 +27,13 @@ func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (re // get response // - // use a model.DiscardLogger to avoid logging bridges - return httpclientx.GetJSON[model.OOAPIVPNProviderConfig](ctx, URL, &httpclientx.Config{ - Client: c.HTTPClient, - Host: c.Host, - Logger: model.DiscardLogger, - UserAgent: c.UserAgent, - }) + // use a model.DiscardLogger to avoid logging config + return httpclientx.GetJSON[model.OOAPIVPNProviderConfig]( + ctx, + httpclientx.NewEndpoint(URL).WithHostOverride(c.Host), + &httpclientx.Config{ + Client: c.HTTPClient, + Logger: model.DiscardLogger, + UserAgent: c.UserAgent, + }) } From 0a24b30146d1838fc093cf349b40ab47c0653b97 Mon Sep 17 00:00:00 2001 From: Ain Ghazal Date: Mon, 6 May 2024 18:17:12 +0200 Subject: [PATCH 33/53] add tests for probeservices/openvpn --- internal/model/ooapi.go | 2 +- internal/probeservices/openvpn.go | 2 +- internal/probeservices/openvpn_test.go | 97 +++++++++++++++++++++++ internal/testingx/oonibackendwithlogin.go | 28 +++++++ 4 files changed, 127 insertions(+), 2 deletions(-) diff --git a/internal/model/ooapi.go b/internal/model/ooapi.go index d0249e58aa..681de22cd9 100644 --- a/internal/model/ooapi.go +++ b/internal/model/ooapi.go @@ -126,7 +126,7 @@ type OOAPIVPNProviderConfig struct { Config *OOAPIVPNConfig `json:"config"` // Inputs is an array of valid endpoints for this provider. - Inputs []string + Inputs []string `json:"endpoints"` // DateUpdated is when the credential set was last updated in the server database. DateUpdated time.Time `json:"date_updated"` diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index 4a87ca70a5..18fbee8a3b 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -19,7 +19,7 @@ func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (re query.Add("country_code", cc) URL, err := urlx.ResolveReference(c.BaseURL, - fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%svpn/", provider), + fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%svpn", provider), query.Encode()) if err != nil { return diff --git a/internal/probeservices/openvpn_test.go b/internal/probeservices/openvpn_test.go index 5b14664815..f2e09e97a1 100644 --- a/internal/probeservices/openvpn_test.go +++ b/internal/probeservices/openvpn_test.go @@ -1,12 +1,109 @@ package probeservices import ( + "context" + "fmt" + "net/http" + "net/url" "testing" + + "github.com/apex/log" + "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" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingx" ) +func newclientWithStagingEnv() *Client { + client, err := NewClient( + &mockable.Session{ + MockableHTTPClient: http.DefaultClient, + MockableLogger: log.Log, + }, + model.OOAPIService{ + Address: "https://api.dev.ooni.io/", + Type: "https", + }, + ) + if err != nil { + panic(err) // so fail the test + } + return client +} + func TestFetchOpenVPNConfig(t *testing.T) { + // First, let's check whether we can get a response from the real OONI backend. + t.Run("is working as intended with the real backend", func(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + + // TODO(ain): switch to newclient() when backend in all environments + // deploys the vpn-config endpoint. + clnt := newclientWithStagingEnv() + + // run the tor flow + config, err := clnt.FetchOpenVPNConfig(context.Background(), "riseup", "ZZ") + + // we do not expect an error here + if err != nil { + t.Fatal(err) + } + + // we expect non-zero length targets + if len(config.Inputs) <= 0 { + fmt.Println(config) + t.Fatal("expected non-zero-length inputs") + } + }) t.Run("is working as intended with a local test server", func(t *testing.T) { + // create state for emulating the OONI backend + state := &testingx.OONIBackendWithLoginFlow{} + + // return something that matches thes expected data + state.SetOpenVPNConfig([]byte(`{ +"provider": "demovpn", +"protocol": "openvpn", +"config": { + "ca": "deadbeef", + "cert": "deadbeef", + "key": "deadbeef" + }, + "date_updated": "2024-05-06T15:22:13.152242Z", + "endpoints": [ + "openvpn://demovpn.corp/?address=1.1.1.1:53&transport=udp" + ] +} +`)) + + // expose the state via HTTP + srv := testingx.MustNewHTTPServer(state.NewMux()) + defer srv.Close() + // TODO(ain) + client := newclient() + + // override the HTTP client so we speak with our local server rather than the true backend + client.HTTPClient = &mocks.HTTPClient{ + MockDo: func(req *http.Request) (*http.Response, error) { + URL := runtimex.Try1(url.Parse(srv.URL)) + req.URL.Scheme = URL.Scheme + req.URL.Host = URL.Host + return http.DefaultClient.Do(req) + }, + MockCloseIdleConnections: func() { + http.DefaultClient.CloseIdleConnections() + }, + } + + // then we can try to fetch the config + _, err := client.FetchOpenVPNConfig(context.Background(), "demo", "ZZ") + + // we do not expect an error here + if err != nil { + t.Fatal(err) + } }) } diff --git a/internal/testingx/oonibackendwithlogin.go b/internal/testingx/oonibackendwithlogin.go index 9bcf3e0690..1cb3fede6e 100644 --- a/internal/testingx/oonibackendwithlogin.go +++ b/internal/testingx/oonibackendwithlogin.go @@ -38,6 +38,9 @@ type OONIBackendWithLoginFlow struct { // mu provides mutual exclusion. mu sync.Mutex + // openVPNConfig is the serialized openvpn config to send to clients. + openVPNConfig []byte + // psiphonConfig is the serialized psiphon config to send to authenticated clients. psiphonConfig []byte @@ -48,6 +51,15 @@ type OONIBackendWithLoginFlow struct { torTargets []byte } +// SetOpenVPNConfig sets openvpn configuration to use. +// +// This method is safe to call concurrently with incoming HTTP requests. +func (h *OONIBackendWithLoginFlow) SetOpenVPNConfig(config []byte) { + defer h.mu.Unlock() + h.mu.Lock() + h.openVPNConfig = config +} + // SetPsiphonConfig sets psiphon configuration to use. // // This method is safe to call concurrently with incoming HTTP requests. @@ -86,6 +98,7 @@ func (h *OONIBackendWithLoginFlow) NewMux() *http.ServeMux { mux.Handle("/api/v1/login", h.handleLogin()) mux.Handle("/api/v1/test-list/psiphon-config", h.withAuthentication(h.handlePsiphonConfig())) mux.Handle("/api/v1/test-list/tor-targets", h.withAuthentication(h.handleTorTargets())) + mux.Handle("/api/v2/ooniprobe/vpn-config/demovpn", h.handleOpenVPNConfig()) return mux } @@ -211,6 +224,21 @@ func (h *OONIBackendWithLoginFlow) handleLogin() http.Handler { }) } +func (h *OONIBackendWithLoginFlow) handleOpenVPNConfig() http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // make sure the method is OK + if r.Method != http.MethodGet { + w.WriteHeader(501) + return + } + + // we must lock because of SetOpenVPNConfig + h.mu.Lock() + w.Write(h.openVPNConfig) + h.mu.Unlock() + }) +} + func (h *OONIBackendWithLoginFlow) handlePsiphonConfig() http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // make sure the method is OK From 2f64b08663c33d9c91a82e8888f6e5cc0ab0ca5d Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 30 May 2024 13:11:00 +0200 Subject: [PATCH 34/53] minor style changes for clarity and correctness --- internal/engine/inputloader.go | 2 +- internal/engine/inputloader_test.go | 7 ++----- internal/engine/session.go | 6 ++++++ internal/experiment/openvpn/endpoint.go | 21 +++++++++++++++++---- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index 988da0b50b..c3f9a53f26 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -350,7 +350,7 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI } } - if len(urls) == 0 { + if len(urls) <= 0 { // loadRemote returns ErrNoURLsReturned at this point for webconnectivity, // but for OpenVPN we want to return a sensible default to be // able to probe some endpoints even in very restrictive environments. diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index cc9efd6dda..92096121f9 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -15,6 +15,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/runtimex" ) func TestInputLoaderInputNoneWithStaticInputs(t *testing.T) { @@ -470,9 +471,7 @@ func (sess *InputLoaderMockableSession) CheckIn( // FetchOpenVPNConfig implements InputLoaderSession.FetchOpenVPNConfig. func (sess *InputLoaderMockableSession) FetchOpenVPNConfig( ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) { - if sess.FetchOpenVPNConfigOutput == nil && sess.Error == nil { - return nil, errors.New("both Output and Error are nil") - } + runtimex.Assert(!(sess.FetchOpenVPNConfig == nil && sess.FetchOpenVPNConfigOutput == nil), "both FetchOpenVPNConfig") return sess.FetchOpenVPNConfigOutput, sess.Error } @@ -613,8 +612,6 @@ func TestInputLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) { } } -// TODO: WIP marker ------------------------------- - func TestPreventMistakesWithCategories(t *testing.T) { input := []model.OOAPIURLInfo{{ CategoryCode: "NEWS", diff --git a/internal/engine/session.go b/internal/engine/session.go index 0a31182182..6ac9cc77c9 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -366,6 +366,9 @@ func (s *Session) DefaultHTTPClient() model.HTTPClient { // FetchTorTargets fetches tor targets from the API. func (s *Session) FetchTorTargets( ctx context.Context, cc string) (map[string]model.OOAPITorTarget, error) { + s.mu.Lock() + defer s.mu.Unlock() + clnt, err := s.newOrchestraClient(ctx) if err != nil { return nil, err @@ -377,6 +380,9 @@ 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) { + s.mu.Lock() + defer s.mu.Unlock() + if config, ok := s.vpnConfig[provider]; ok { return &config, nil } diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index d8aacde16a..10bba5a9a6 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -110,7 +110,12 @@ func (e *endpoint) String() string { } else { proto = e.Protocol } - return fmt.Sprintf("%s://%s:%s/%s", proto, e.IPAddr, e.Port, e.Transport) + url := &url.URL{ + Scheme: proto, + Host: net.JoinHostPort(e.IPAddr, e.Port), + Path: e.Transport, + } + return url.String() } // AsInputURI is a string representation of this endpoint, as used in the experiment input URI format. @@ -127,9 +132,17 @@ func (e *endpoint) AsInputURI() string { provider = "unknown" } - return fmt.Sprintf( - "%s://%s.corp/?address=%s:%s&transport=%s", - proto, provider, e.IPAddr, e.Port, e.Transport) + values := map[string][]string{ + "address": {net.JoinHostPort(e.IPAddr, e.Port)}, + "transport": {e.Transport}, + } + + url := &url.URL{ + Scheme: proto, + Host: provider + ".corp", + RawQuery: url.Values(values).Encode(), + } + return url.String() } // endpointList is a list of endpoints. From 4bc13c850ea28a9476e2ad94ff97feded30e9174 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 30 May 2024 14:21:09 +0200 Subject: [PATCH 35/53] debug possible deadlock --- internal/engine/session.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/engine/session.go b/internal/engine/session.go index 6ac9cc77c9..b2276614e5 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -366,9 +366,6 @@ func (s *Session) DefaultHTTPClient() model.HTTPClient { // FetchTorTargets fetches tor targets from the API. func (s *Session) FetchTorTargets( ctx context.Context, cc string) (map[string]model.OOAPITorTarget, error) { - s.mu.Lock() - defer s.mu.Unlock() - clnt, err := s.newOrchestraClient(ctx) if err != nil { return nil, err @@ -380,8 +377,10 @@ 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) { - s.mu.Lock() - defer s.mu.Unlock() + + // TODO(ain): deadlock? + // defer s.mu.Unlock() + // s.mu.Lock() if config, ok := s.vpnConfig[provider]; ok { return &config, nil From 8fcfaace3baed10e2868d5fc9d3858b8d7cb216d Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 30 May 2024 14:21:47 +0200 Subject: [PATCH 36/53] prefix annoyance --- internal/experiment/openvpn/endpoint.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 10bba5a9a6..9c68dc57f7 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -191,7 +191,7 @@ var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ // 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. var APIEnabledProviders = []string{ - "riseup", + "riseupvpn", } // isValidProvider returns true if the provider is found as key in the registry of defaultOptionsByProvider. From 9d7a6bee54b38acadd4948e7098e91ac8532e8da Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 30 May 2024 16:14:10 +0200 Subject: [PATCH 37/53] debug --- internal/engine/session.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/engine/session.go b/internal/engine/session.go index b2276614e5..dc90f42966 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -664,8 +664,11 @@ func (s *Session) MaybeLookupBackendsContext(ctx context.Context) error { return nil } s.queryProbeServicesCount.Add(1) + fmt.Println(">>> get available probe services unlocked", s.getAvailableProbeServicesUnlocked()) candidates := probeservices.TryAll(ctx, s, s.getAvailableProbeServicesUnlocked()) + fmt.Println(">>> candidates", candidates) selected := probeservices.SelectBest(candidates) + fmt.Println(">>> selected", selected) if selected == nil { return ErrAllProbeServicesFailed } From 32f3f00e89c3763ff1fe57283bb72bed0ec9681b Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Thu, 30 May 2024 16:47:45 +0200 Subject: [PATCH 38/53] complete hacks to bypass vpn provider name in backend --- internal/engine/session.go | 3 --- internal/experiment/openvpn/endpoint.go | 15 ++++++++++++++- internal/experiment/openvpn/openvpn.go | 3 ++- internal/probeservices/openvpn.go | 8 +++++++- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/internal/engine/session.go b/internal/engine/session.go index dc90f42966..b2276614e5 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -664,11 +664,8 @@ func (s *Session) MaybeLookupBackendsContext(ctx context.Context) error { return nil } s.queryProbeServicesCount.Add(1) - fmt.Println(">>> get available probe services unlocked", s.getAvailableProbeServicesUnlocked()) candidates := probeservices.TryAll(ctx, s, s.getAvailableProbeServicesUnlocked()) - fmt.Println(">>> candidates", candidates) selected := probeservices.SelectBest(candidates) - fmt.Println(">>> selected", selected) if selected == nil { return ErrAllProbeServicesFailed } diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 9c68dc57f7..73bc251314 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -181,7 +181,7 @@ func (e endpointList) Shuffle() endpointList { // all the known providers. We extend this base config with credentials coming // from the OONI API. var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ - "riseup": { + "riseupvpn": { Auth: "SHA512", Cipher: "AES-256-GCM", }, @@ -191,7 +191,9 @@ var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ // 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. var APIEnabledProviders = []string{ + // TODO: fix the backend so that we can remove the spurious "vpn" suffix here. "riseupvpn", + "riseup", } // isValidProvider returns true if the provider is found as key in the registry of defaultOptionsByProvider. @@ -215,8 +217,19 @@ func getOpenVPNConfig( 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( diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 5d500e7383..67da8cfccd 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "strconv" + "strings" "time" "github.com/ooni/probe-cli/v3/internal/measurexlite" @@ -200,7 +201,7 @@ func (m *Measurer) GetCredentialsFromOptionsOrAPI( sess model.ExperimentSession, provider string) (*vpnconfig.OpenVPNOptions, error) { - method, ok := providerAuthentication[provider] + method, ok := providerAuthentication[strings.TrimSuffix(provider, "vpn")] if !ok { return nil, fmt.Errorf("%w: provider auth unknown: %s", ErrInvalidInput, provider) } diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index 18fbee8a3b..df2a0e9c46 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/url" + "strings" "github.com/ooni/probe-cli/v3/internal/httpclientx" "github.com/ooni/probe-cli/v3/internal/model" @@ -18,8 +19,13 @@ func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (re query := url.Values{} query.Add("country_code", cc) + // TODO(ain): remove temporary fix + if !strings.HasSuffix(provider, "vpn") { + provider = provider + "vpn" + } + URL, err := urlx.ResolveReference(c.BaseURL, - fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%svpn", provider), + fmt.Sprintf("/api/v2/ooniprobe/vpn-config/%s", provider), query.Encode()) if err != nil { return From a55816cbf621461ead07b61b558750a7a048f740 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Mon, 3 Jun 2024 17:12:14 +0200 Subject: [PATCH 39/53] fix tests for bad parsing --- internal/experiment/openvpn/endpoint_test.go | 38 ++++++++++---------- internal/experiment/openvpn/openvpn_test.go | 2 +- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/internal/experiment/openvpn/endpoint_test.go b/internal/experiment/openvpn/endpoint_test.go index 502fd379c2..60c289dc77 100644 --- a/internal/experiment/openvpn/endpoint_test.go +++ b/internal/experiment/openvpn/endpoint_test.go @@ -2,6 +2,7 @@ package openvpn import ( "errors" + "fmt" "sort" "testing" "time" @@ -23,13 +24,13 @@ func Test_newEndpointFromInputString(t *testing.T) { }{ { name: "valid endpoint returns good endpoint", - args: args{"openvpn://riseup.corp/?address=1.1.1.1:1194&transport=tcp"}, + args: args{"openvpn://riseupvpn.corp/?address=1.1.1.1:1194&transport=tcp"}, want: &endpoint{ IPAddr: "1.1.1.1", Obfuscation: "none", Port: "1194", Protocol: "openvpn", - Provider: "riseup", + Provider: "riseupvpn", Transport: "tcp", }, wantErr: nil, @@ -42,26 +43,26 @@ func Test_newEndpointFromInputString(t *testing.T) { }, { name: "openvpn+obfs4 does not fail", - args: args{"openvpn+obfs4://riseup.corp/?address=1.1.1.1:1194&transport=tcp"}, + args: args{"openvpn+obfs4://riseupvpn.corp/?address=1.1.1.1:1194&transport=tcp"}, want: &endpoint{ IPAddr: "1.1.1.1", Obfuscation: "obfs4", Port: "1194", Protocol: "openvpn", - Provider: "riseup", + Provider: "riseupvpn", Transport: "tcp", }, wantErr: nil, }, { name: "unknown proto fails", - args: args{"unknown://riseup.corp/?address=1.1.1.1:1194&transport=tcp"}, + args: args{"unknown://riseupvpn.corp/?address=1.1.1.1:1194&transport=tcp"}, want: nil, wantErr: ErrInvalidInput, }, { name: "any tld other than .corp fails", - args: args{"openvpn://riseup.org/?address=1.1.1.1:1194&transport=tcp"}, + args: args{"openvpn://riseupvpn.org/?address=1.1.1.1:1194&transport=tcp"}, want: nil, wantErr: ErrInvalidInput, }, @@ -79,43 +80,43 @@ func Test_newEndpointFromInputString(t *testing.T) { }, { name: "endpoint with invalid ipv4 fails", - args: args{"openvpn://riseup.corp/?address=example.com:1194&transport=tcp"}, + args: args{"openvpn://riseupvpn.corp/?address=example.com:1194&transport=tcp"}, want: nil, wantErr: ErrInvalidInput, }, { name: "endpoint with no port fails", - args: args{"openvpn://riseup.corp/?address=1.1.1.1&transport=tcp"}, + args: args{"openvpn://riseupvpn.corp/?address=1.1.1.1&transport=tcp"}, want: nil, wantErr: ErrInvalidInput, }, { name: "endpoint with empty transport fails", - args: args{"openvpn://riseup.corp/?address=1.1.1.1:1194&transport="}, + args: args{"openvpn://riseupvpn.corp/?address=1.1.1.1:1194&transport="}, want: nil, wantErr: ErrInvalidInput, }, { name: "endpoint with no transport fails", - args: args{"openvpn://riseup.corp/?address=1.1.1.1:1194"}, + args: args{"openvpn://riseupvpn.corp/?address=1.1.1.1:1194"}, want: nil, wantErr: ErrInvalidInput, }, { name: "endpoint with unknown transport fails", - args: args{"openvpn://riseup.corp/?address=1.1.1.1:1194&transport=uh"}, + args: args{"openvpn://riseupvpn.corp/?address=1.1.1.1:1194&transport=uh"}, want: nil, wantErr: ErrInvalidInput, }, { name: "endpoint with no address fails", - args: args{"openvpn://riseup.corp/?transport=tcp"}, + args: args{"openvpn://riseupvpn.corp/?transport=tcp"}, want: nil, wantErr: ErrInvalidInput, }, { name: "endpoint with empty address fails", - args: args{"openvpn://riseup.corp/?address=&transport=tcp"}, + args: args{"openvpn://riseupvpn.corp/?address=&transport=tcp"}, want: nil, wantErr: ErrInvalidInput, }, @@ -155,7 +156,7 @@ func Test_EndpointToInputURI(t *testing.T) { Transport: "udp", }, }, - want: "openvpn://shady.corp/?address=1.1.1.1:443&transport=udp", + want: "openvpn://shady.corp?address=1.1.1.1%3A443&transport=udp", }, { name: "good endpoint with openvpn+obfs4", @@ -169,7 +170,7 @@ func Test_EndpointToInputURI(t *testing.T) { Transport: "udp", }, }, - want: "openvpn+obfs4://shady.corp/?address=1.1.1.1:443&transport=udp", + want: "openvpn+obfs4://shady.corp?address=1.1.1.1%3A443&transport=udp", }, { name: "empty provider is marked as unknown", @@ -183,12 +184,13 @@ func Test_EndpointToInputURI(t *testing.T) { Transport: "udp", }, }, - want: "openvpn+obfs4://unknown.corp/?address=1.1.1.1:443&transport=udp", + want: "openvpn+obfs4://unknown.corp?address=1.1.1.1%3A443&transport=udp", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if got := tt.args.endpoint.AsInputURI(); cmp.Diff(got, tt.want) != "" { + fmt.Println("GOT", got) t.Error(cmp.Diff(got, tt.want)) } }) @@ -262,7 +264,7 @@ func Test_endpointList_Shuffle(t *testing.T) { } func Test_isValidProvider(t *testing.T) { - if valid := isValidProvider("riseup"); !valid { + if valid := isValidProvider("riseupvpn"); !valid { t.Fatal("riseup is the only valid provider now") } if valid := isValidProvider("nsa"); valid { @@ -273,7 +275,7 @@ func Test_isValidProvider(t *testing.T) { func Test_getVPNConfig(t *testing.T) { tracer := vpntracex.NewTracer(time.Now()) e := &endpoint{ - Provider: "riseup", + Provider: "riseupvpn", IPAddr: "1.1.1.1", Port: "443", Transport: "udp", diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index f76663b5b0..69ba0402a4 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -403,7 +403,7 @@ func TestSuccess(t *testing.T) { sess := makeMockSession() callbacks := model.NewPrinterCallbacks(sess.Logger()) measurement := new(model.Measurement) - measurement.Input = "openvpn://riseup.corp/?address=127.0.0.1:9989&transport=tcp" + measurement.Input = "openvpn://riseupvpn.corp/?address=127.0.0.1:9989&transport=tcp" args := &model.ExperimentArgs{ Callbacks: callbacks, Measurement: measurement, From 6ab511e34c5de0ad2dea9b9b947b018db8a7a864 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Mon, 3 Jun 2024 17:38:54 +0200 Subject: [PATCH 40/53] fix bad comparison --- internal/engine/inputloader_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index 92096121f9..620bb2822e 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -471,7 +471,7 @@ func (sess *InputLoaderMockableSession) CheckIn( // FetchOpenVPNConfig implements InputLoaderSession.FetchOpenVPNConfig. func (sess *InputLoaderMockableSession) FetchOpenVPNConfig( ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) { - runtimex.Assert(!(sess.FetchOpenVPNConfig == nil && sess.FetchOpenVPNConfigOutput == nil), "both FetchOpenVPNConfig") + runtimex.Assert(!(sess.Error == nil && sess.FetchOpenVPNConfigOutput == nil), "both FetchOpenVPNConfig and Error are nil") return sess.FetchOpenVPNConfigOutput, sess.Error } From 07daac4450eb7223e93e8c72a639733e105ca5ef Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Mon, 3 Jun 2024 17:39:15 +0200 Subject: [PATCH 41/53] revert removal of lock --- internal/engine/session.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/internal/engine/session.go b/internal/engine/session.go index b2276614e5..afb53fe1ed 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -377,10 +377,8 @@ 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) { - - // TODO(ain): deadlock? - // defer s.mu.Unlock() - // s.mu.Lock() + defer s.mu.Unlock() + s.mu.Lock() if config, ok := s.vpnConfig[provider]; ok { return &config, nil From 410126a6bdee1027043d36cbf727bf0a3c2767ea Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Mon, 3 Jun 2024 18:24:43 +0200 Subject: [PATCH 42/53] pass output to test to avoid panic --- internal/engine/inputloader_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index 620bb2822e..cb13aa3a5c 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -562,6 +562,13 @@ func TestInputLoaderOpenVPNSuccessWithNoInput(t *testing.T) { InputPolicy: model.InputOrQueryBackend, Session: &InputLoaderMockableSession{ Error: nil, + FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{ + Provider: "riseup", + Inputs: []string{ + "openvpn://foo.corp/?address=1.1.1.1:1194&transport=tcp", + }, + DateUpdated: time.Now(), + }, }, } _, err := il.loadRemote(context.Background()) From 2a6fc2536b0cfb1c496f589f49897b474c027582 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Mon, 3 Jun 2024 19:08:48 +0200 Subject: [PATCH 43/53] force single provider for now --- internal/engine/inputloader.go | 9 +++++++++ internal/engine/inputloader_test.go | 4 ++-- internal/experiment/openvpn/endpoint.go | 1 - 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index c3f9a53f26..d10d851205 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -304,6 +304,11 @@ func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]mode func (il *InputLoader) loadRemote(ctx context.Context) ([]model.OOAPIURLInfo, error) { switch registry.CanonicalizeExperimentName(il.ExperimentName) { case "openvpn": + // TODO(ainghazal): given the semantics of the current API call, in an ideal world we'd need to pass + // the desired provider here, if known. We only have one provider for now, so I'm acting like YAGNI. Another + // option is perhaps to coalesce all the known providers per proto into a single API call and let client + // pick whatever they want. + // This will likely improve after Richer Input is available. return il.loadRemoteOpenVPN(ctx) default: return il.loadRemoteWebConnectivity(ctx) @@ -335,9 +340,13 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI // VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo], // since OOAPIURLInfo is oriented towards webconnectivity, // but we force VPN targets in the URL and ignore all the other fields. + // There's still some level of impedance mismatch here, since it's also possible that + // the user of the library wants to use remotes by unknown providers passed via cli options, + // oonirun etc; in that case we'll need to extract the provider annotation from the URLs. urls := make([]model.OOAPIURLInfo, 0) // The openvpn experiment contains an array of the providers that the API knows about. + // We try to get all the remotes from the API for the list of enabled providers. for _, provider := range openvpn.APIEnabledProviders { reply, err := il.vpnConfig(ctx, provider) if err != nil { diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index cb13aa3a5c..0a7a889bb2 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -584,9 +584,9 @@ func TestInputLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) { Session: &InputLoaderMockableSession{ Error: nil, FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{ - Provider: "riseup", + Provider: "riseupvpn", Inputs: []string{ - "openvpn://foo.corp/?address=1.1.1.1:1194&transport=tcp", + "openvpn://foo.corp/?address=1.2.3.4:1194&transport=tcp", }, DateUpdated: time.Now(), }, diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 73bc251314..bd456dab6a 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -193,7 +193,6 @@ var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ var APIEnabledProviders = []string{ // TODO: fix the backend so that we can remove the spurious "vpn" suffix here. "riseupvpn", - "riseup", } // isValidProvider returns true if the provider is found as key in the registry of defaultOptionsByProvider. From ec8396a11465736f5282fb6e7750d0d651f387cc Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 4 Jun 2024 17:18:17 +0200 Subject: [PATCH 44/53] add changes after code review --- internal/engine/inputloader.go | 12 +++--- internal/experiment/openvpn/endpoint.go | 9 ++-- internal/experiment/openvpn/endpoint_test.go | 2 +- internal/experiment/openvpn/openvpn.go | 45 ++++++++++---------- internal/experiment/openvpn/openvpn_test.go | 44 ++++++++++++++----- internal/mocks/session_test.go | 16 +++++++ internal/model/ooapi.go | 2 + internal/probeservices/openvpn.go | 2 +- internal/probeservices/openvpn_test.go | 8 +--- 9 files changed, 89 insertions(+), 51 deletions(-) diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index d10d851205..29135156af 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -348,12 +348,12 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI // The openvpn experiment contains an array of the providers that the API knows about. // We try to get all the remotes from the API for the list of enabled providers. for _, provider := range openvpn.APIEnabledProviders { - reply, err := il.vpnConfig(ctx, provider) + // fetchOpenVPNConfig ultimately uses an internal cache in the session to avoid + // hitting the API too many times. + reply, err := il.fetchOpenVPNConfig(ctx, provider) if err != nil { - break + return urls, err } - // 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. for _, input := range reply.Inputs { urls = append(urls, model.OOAPIURLInfo{URL: input}) } @@ -390,8 +390,8 @@ func (il *InputLoader) checkIn( return &reply.Tests, nil } -// vpnConfig fetches vpn information for the configured providers -func (il *InputLoader) vpnConfig(ctx context.Context, provider string) (*model.OOAPIVPNProviderConfig, error) { +// fetchOpenVPNConfig fetches vpn information for the configured providers +func (il *InputLoader) fetchOpenVPNConfig(ctx context.Context, provider string) (*model.OOAPIVPNProviderConfig, error) { reply, err := il.Session.FetchOpenVPNConfig(ctx, provider, "XX") if err != nil { return nil, err diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index bd456dab6a..5c8c8a57c2 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -15,6 +15,7 @@ import ( ) var ( + // ErrBadBase64Blob is the error returned when we cannot decode an option passed as base64. ErrBadBase64Blob = errors.New("wrong base64 encoding") ) @@ -100,7 +101,7 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { return endpoint, nil } -// String implements Stringer. This is a compact representation of the endpoint, +// String implements [fmt.Stringer]. This is a compact representation of the endpoint, // which differs from the input URI scheme. This is the canonical representation, that can be used // to deterministically slice a list of endpoints, sort them lexicographically, etc. func (e *endpoint) String() string { @@ -169,7 +170,7 @@ var DefaultEndpoints = endpointList{ }, } -// Shuffle returns a shuffled copy of the endpointList. +// Shuffle randomizes the order of items in the endpoint list. func (e endpointList) Shuffle() endpointList { rand.Shuffle(len(e), func(i, j int) { e[i], e[j] = e[j], e[i] @@ -191,7 +192,7 @@ var defaultOptionsByProvider = map[string]*vpnconfig.OpenVPNOptions{ // 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. var APIEnabledProviders = []string{ - // TODO: fix the backend so that we can remove the spurious "vpn" suffix here. + // TODO(ainghazal): fix the backend so that we can remove the spurious "vpn" suffix here. "riseupvpn", } @@ -258,7 +259,7 @@ func getOpenVPNConfig( func extractBase64Blob(val string) (string, error) { s := strings.TrimPrefix(val, "base64:") if len(s) == len(val) { - return "", fmt.Errorf("%w: %s", ErrBadBase64Blob, "missing prefix") + return "", fmt.Errorf("%w: %s", ErrBadBase64Blob, "missing base64 prefix") } dec, err := base64.URLEncoding.DecodeString(strings.TrimSpace(s)) if err != nil { diff --git a/internal/experiment/openvpn/endpoint_test.go b/internal/experiment/openvpn/endpoint_test.go index 60c289dc77..bb141bf942 100644 --- a/internal/experiment/openvpn/endpoint_test.go +++ b/internal/experiment/openvpn/endpoint_test.go @@ -268,7 +268,7 @@ func Test_isValidProvider(t *testing.T) { t.Fatal("riseup is the only valid provider now") } if valid := isValidProvider("nsa"); valid { - t.Fatal("nsa will nevel be a provider") + t.Fatal("nsa will never be a provider") } } diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 67da8cfccd..314e0416e6 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -22,10 +22,6 @@ const ( openVPNProcol = "openvpn" ) -var ( - ErrBadAuth = errors.New("bad provider authentication") -) - // Config contains the experiment config. // // This contains all the settings that user can set to modify the behaviour @@ -69,6 +65,7 @@ type SingleConnection struct { // AddConnectionTestKeys adds the result of a single OpenVPN connection attempt to the // corresponding array in the [TestKeys] object. func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { + // Note that TCPConnect is nil when we're using UDP. if result.TCPConnect != nil { tk.TCPConnect = append(tk.TCPConnect, result.TCPConnect) } @@ -78,6 +75,9 @@ func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { // AllConnectionsSuccessful returns true if all the registered handshakes have Status.Success equal to true. func (tk *TestKeys) AllConnectionsSuccessful() bool { + if len(tk.OpenVPNHandshake) == 0 { + return false + } for _, c := range tk.OpenVPNHandshake { if !c.Status.Success { return false @@ -108,6 +108,7 @@ func (m Measurer) ExperimentVersion() string { } var ( + // ErrInvalidInput is returned if we failed to parse the input to obtain an endpoint we can measure. ErrInvalidInput = errors.New("invalid input") ) @@ -118,8 +119,6 @@ func parseEndpoint(m *model.Measurement) (*endpoint, error) { } return newEndpointFromInputString(string(m.Input)) } - // The current InputPolicy should ensure we have a hardcoded input, - // so this error should only be raised if by mistake we change the InputPolicy. return nil, fmt.Errorf("%w: %s", ErrInvalidInput, "input is mandatory") } @@ -127,8 +126,11 @@ func parseEndpoint(m *model.Measurement) (*endpoint, error) { type AuthMethod string var ( + // AuthCertificate is used for providers that authenticate clients via certificates. AuthCertificate = AuthMethod("cert") - AuthUserPass = AuthMethod("userpass") + + // AuthUserPass is used for providers that authenticate clients via username (or token) and password. + AuthUserPass = AuthMethod("userpass") ) var providerAuthentication = map[string]AuthMethod{ @@ -232,7 +234,7 @@ func (m *Measurer) GetCredentialsFromOptionsOrAPI( } // 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. +// 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, @@ -251,7 +253,7 @@ func (m *Measurer) mergeOpenVPNConfig( if err != nil { return nil, err } - // TODO: sanity check (Remote, Port, Proto etc + missing certs) + // TODO(ainghazal): sanity check (Remote, Port, Proto etc + missing certs) return openvpnConfig, nil } @@ -263,15 +265,16 @@ func (m *Measurer) connectAndHandshake( logger model.Logger, endpoint *endpoint, openvpnConfig *vpnconfig.Config, - handshakeTracer *vpntracex.Tracer) (*SingleConnection, error) { + handshakeTracer *vpntracex.Tracer) *SingleConnection { // 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 + + // Create a vpn tun Device that attempts to dial and performs the handshake. + // Any error will be returned as a failure in the SingleConnection result. tun, err := tunnel.Start(ctx, dialer, openvpnConfig) if err != nil { failure = err.Error() @@ -289,7 +292,7 @@ func (m *Measurer) connectAndHandshake( bootstrapTime float64 ) - if len(handshakeEvents) != 0 { + if len(handshakeEvents) > 0 { tFirst = handshakeEvents[0].AtTime tLast = handshakeEvents[len(handshakeEvents)-1].AtTime bootstrapTime = tLast - tFirst @@ -319,17 +322,19 @@ func (m *Measurer) connectAndHandshake( TransactionID: index, }, NetworkEvents: handshakeEvents, - }, nil + } } +// FetchProviderCredentials will extract credentials from the configuration we gathered for a given provider. func (m *Measurer) FetchProviderCredentials( ctx context.Context, sess model.ExperimentSession, provider string) (*model.OOAPIVPNProviderConfig, error) { // TODO(ainghazal): pass real country code, can be useful to orchestrate campaigns specific to areas. + // Since we have contacted the API previously, this call should use the cached info contained in the session. config, err := sess.FetchOpenVPNConfig(ctx, provider, "XX") if err != nil { - return &model.OOAPIVPNProviderConfig{}, err + return nil, err } return config, nil } @@ -359,14 +364,8 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { } 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) - } + connResult := m.connectAndHandshake(ctx, zeroTime, idx, sess.Logger(), endpoint, openvpnConfig, handshakeTracer) + tk.AddConnectionTestKeys(connResult) tk.Success = tk.AllConnectionsSuccessful() callbacks.OnProgress(1.0, "All endpoints probed") diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index 69ba0402a4..0e40d8fa38 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -3,7 +3,6 @@ package openvpn_test import ( "context" "errors" - "fmt" "testing" "time" @@ -154,7 +153,6 @@ func TestMaybeGetCredentialsFromOptions(t *testing.T) { t.Fatalf("expected err=nil, got %v", err) } }) - } func TestGetCredentialsFromOptionsOrAPI(t *testing.T) { @@ -251,7 +249,7 @@ func TestGetCredentialsFromOptionsOrAPI(t *testing.T) { } func TestAddConnectionTestKeys(t *testing.T) { - t.Run("append connection result to empty keys", func(t *testing.T) { + t.Run("append tcp connection result to empty keys", func(t *testing.T) { tk := openvpn.NewTestKeys() sc := &openvpn.SingleConnection{ TCPConnect: &model.ArchivalTCPConnectResult{ @@ -294,6 +292,38 @@ func TestAddConnectionTestKeys(t *testing.T) { t.Fatal(diff) } }) + + t.Run("append udp connection result to empty keys", func(t *testing.T) { + tk := openvpn.NewTestKeys() + sc := &openvpn.SingleConnection{ + TCPConnect: nil, + OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ + BootstrapTime: 1, + Endpoint: "aa", + IP: "1.1.1.1", + Port: 1194, + Transport: "udp", + Provider: "unknown", + OpenVPNOptions: model.ArchivalOpenVPNOptions{}, + Status: model.ArchivalOpenVPNConnectStatus{}, + T0: 0, + T: 0, + Tags: []string{}, + TransactionID: 1, + }, + NetworkEvents: []*vpntracex.Event{}, + } + tk.AddConnectionTestKeys(sc) + if len(tk.TCPConnect) != 0 { + t.Fatal("expected empty tcpconnect") + } + if diff := cmp.Diff(tk.OpenVPNHandshake[0], sc.OpenVPNHandshake); diff != "" { + t.Fatal(diff) + } + if diff := cmp.Diff(tk.NetworkEvents, sc.NetworkEvents); diff != "" { + t.Fatal(diff) + } + }) } func TestAllConnectionsSuccessful(t *testing.T) { @@ -354,7 +384,7 @@ func TestVPNInput(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") } - // TODO -- do a real test, get credentials etc. + // TODO(ainghazal): do a real test, get credentials etc. } func TestMeasurer_FetchProviderCredentials(t *testing.T) { @@ -389,7 +419,6 @@ func TestMeasurer_FetchProviderCredentials(t *testing.T) { t.Fatalf("expected error %v, got %v", someError, err) } }) - } func TestSuccess(t *testing.T) { @@ -409,11 +438,6 @@ func TestSuccess(t *testing.T) { 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) diff --git a/internal/mocks/session_test.go b/internal/mocks/session_test.go index b794941736..e26f67430d 100644 --- a/internal/mocks/session_test.go +++ b/internal/mocks/session_test.go @@ -80,6 +80,22 @@ func TestSession(t *testing.T) { } }) + t.Run("FetchOpenVPNConfig", func(t *testing.T) { + expected := errors.New("mocked err") + s := &Session{ + MockFetchOpenVPNConfig: func(ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) { + return nil, expected + }, + } + cfg, err := s.FetchOpenVPNConfig(context.Background(), "riseup", "XX") + if !errors.Is(err, expected) { + t.Fatal("unexpected err", err) + } + if cfg != nil { + t.Fatal("expected nil cfg") + } + }) + t.Run("KeyValueStore", func(t *testing.T) { expect := &KeyValueStore{} s := &Session{ diff --git a/internal/model/ooapi.go b/internal/model/ooapi.go index 681de22cd9..0442e8369a 100644 --- a/internal/model/ooapi.go +++ b/internal/model/ooapi.go @@ -99,6 +99,8 @@ type OOAPICheckReportIDResponse struct { V int64 `json:"v"` } +// OOAPIVPNConfig contains the configuration needed to start an OpenVPN connection, returned as part of +// [OOAPIVPNProviderConfig]. type OOAPIVPNConfig struct { // CA is the Certificate Authority for the endpoints by this provider. CA string `json:"ca"` diff --git a/internal/probeservices/openvpn.go b/internal/probeservices/openvpn.go index df2a0e9c46..326cb70cb7 100644 --- a/internal/probeservices/openvpn.go +++ b/internal/probeservices/openvpn.go @@ -19,7 +19,7 @@ func (c Client) FetchOpenVPNConfig(ctx context.Context, provider, cc string) (re query := url.Values{} query.Add("country_code", cc) - // TODO(ain): remove temporary fix + // TODO(ainghazal): remove temporary fix if !strings.HasSuffix(provider, "vpn") { provider = provider + "vpn" } diff --git a/internal/probeservices/openvpn_test.go b/internal/probeservices/openvpn_test.go index f2e09e97a1..a40b983f25 100644 --- a/internal/probeservices/openvpn_test.go +++ b/internal/probeservices/openvpn_test.go @@ -16,7 +16,7 @@ import ( ) func newclientWithStagingEnv() *Client { - client, err := NewClient( + client := runtimex.Try1(NewClient( &mockable.Session{ MockableHTTPClient: http.DefaultClient, MockableLogger: log.Log, @@ -25,10 +25,7 @@ func newclientWithStagingEnv() *Client { Address: "https://api.dev.ooni.io/", Type: "https", }, - ) - if err != nil { - panic(err) // so fail the test - } + )) return client } @@ -82,7 +79,6 @@ func TestFetchOpenVPNConfig(t *testing.T) { srv := testingx.MustNewHTTPServer(state.NewMux()) defer srv.Close() - // TODO(ain) client := newclient() // override the HTTP client so we speak with our local server rather than the true backend From b909756cd9600fdc78ba4a0b0763b30d447132d6 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 4 Jun 2024 17:50:26 +0200 Subject: [PATCH 45/53] move the lock after instantiating the client --- internal/engine/session.go | 10 ++++++--- internal/experiment/openvpn/openvpn_test.go | 24 +++++++++++---------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/internal/engine/session.go b/internal/engine/session.go index afb53fe1ed..bf9374dcf6 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -370,6 +370,8 @@ func (s *Session) FetchTorTargets( if err != nil { return nil, err } + + // here we could also lock the mutex. return clnt.FetchTorTargets(ctx, cc) } @@ -377,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) { - defer s.mu.Unlock() - s.mu.Lock() - if config, ok := s.vpnConfig[provider]; ok { return &config, nil } @@ -387,6 +386,11 @@ func (s *Session) FetchOpenVPNConfig( if err != nil { return nil, err } + + // we cannot lock earlier because newOrchestraClient locks the mutex. + defer s.mu.Unlock() + s.mu.Lock() + config, err := clnt.FetchOpenVPNConfig(ctx, provider, cc) if err != nil { return nil, err diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index 0e40d8fa38..b1e4b816d9 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -268,12 +268,12 @@ func TestAddConnectionTestKeys(t *testing.T) { OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ BootstrapTime: 1, Endpoint: "aa", + Failure: nil, IP: "1.1.1.1", Port: 1194, Transport: "tcp", Provider: "unknown", OpenVPNOptions: model.ArchivalOpenVPNOptions{}, - Status: model.ArchivalOpenVPNConnectStatus{}, T0: 0, T: 0, Tags: []string{}, @@ -300,12 +300,12 @@ func TestAddConnectionTestKeys(t *testing.T) { OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ BootstrapTime: 1, Endpoint: "aa", + Failure: nil, IP: "1.1.1.1", Port: 1194, Transport: "udp", Provider: "unknown", OpenVPNOptions: model.ArchivalOpenVPNOptions{}, - Status: model.ArchivalOpenVPNConnectStatus{}, T0: 0, T: 0, Tags: []string{}, @@ -330,31 +330,33 @@ func TestAllConnectionsSuccessful(t *testing.T) { t.Run("all success", func(t *testing.T) { tk := openvpn.NewTestKeys() tk.OpenVPNHandshake = []*model.ArchivalOpenVPNHandshakeResult{ - {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, - {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, - {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, + {Failure: nil}, + {Failure: nil}, + {Failure: nil}, } if tk.AllConnectionsSuccessful() != true { t.Fatal("expected all connections successful") } }) t.Run("one failure", func(t *testing.T) { + fail := "uh" tk := openvpn.NewTestKeys() tk.OpenVPNHandshake = []*model.ArchivalOpenVPNHandshakeResult{ - {Status: model.ArchivalOpenVPNConnectStatus{Success: false}}, - {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, - {Status: model.ArchivalOpenVPNConnectStatus{Success: true}}, + {Failure: &fail}, + {Failure: nil}, + {Failure: nil}, } if tk.AllConnectionsSuccessful() != false { t.Fatal("expected false") } }) t.Run("all failures", func(t *testing.T) { + fail := "uh" tk := openvpn.NewTestKeys() tk.OpenVPNHandshake = []*model.ArchivalOpenVPNHandshakeResult{ - {Status: model.ArchivalOpenVPNConnectStatus{Success: false}}, - {Status: model.ArchivalOpenVPNConnectStatus{Success: false}}, - {Status: model.ArchivalOpenVPNConnectStatus{Success: false}}, + {Failure: &fail}, + {Failure: &fail}, + {Failure: &fail}, } if tk.AllConnectionsSuccessful() != false { t.Fatal("expected false") From b8fbd823622631a965ff75b390d4fa732778e76c Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 4 Jun 2024 18:10:01 +0200 Subject: [PATCH 46/53] unflatten the failure --- internal/experiment/openvpn/openvpn.go | 9 +++----- internal/model/archival.go | 31 ++++++++++---------------- 2 files changed, 15 insertions(+), 25 deletions(-) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 314e0416e6..caca8424c5 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -73,13 +73,13 @@ func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { tk.NetworkEvents = append(tk.NetworkEvents, result.NetworkEvents...) } -// AllConnectionsSuccessful returns true if all the registered handshakes have Status.Success equal to true. +// AllConnectionsSuccessful returns true if all the registered handshakes have nil failures. func (tk *TestKeys) AllConnectionsSuccessful() bool { if len(tk.OpenVPNHandshake) == 0 { return false } for _, c := range tk.OpenVPNHandshake { - if !c.Status.Success { + if c.Failure != nil { return false } } @@ -303,6 +303,7 @@ func (m *Measurer) connectAndHandshake( OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ BootstrapTime: bootstrapTime, Endpoint: endpoint.String(), + Failure: &failure, IP: endpoint.IPAddr, Port: port, Transport: endpoint.Transport, @@ -312,10 +313,6 @@ func (m *Measurer) connectAndHandshake( Auth: openvpnConfig.OpenVPNOptions().Auth, Compression: string(openvpnConfig.OpenVPNOptions().Compress), }, - Status: model.ArchivalOpenVPNConnectStatus{ - Failure: &failure, - Success: err == nil, - }, T0: tFirst, T: tLast, Tags: []string{}, diff --git a/internal/model/archival.go b/internal/model/archival.go index 86c2b52aec..a25ee57105 100644 --- a/internal/model/archival.go +++ b/internal/model/archival.go @@ -399,18 +399,18 @@ type ArchivalNetworkEvent struct { // ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake. type ArchivalOpenVPNHandshakeResult struct { - BootstrapTime float64 `json:"bootstrap_time,omitempty"` - Endpoint string `json:"endpoint"` - IP string `json:"ip"` - Port int `json:"port"` - Transport string `json:"transport"` - Provider string `json:"provider"` - OpenVPNOptions ArchivalOpenVPNOptions `json:"openvpn_options"` - Status ArchivalOpenVPNConnectStatus `json:"status"` - T0 float64 `json:"t0,omitempty"` - T float64 `json:"t"` - Tags []string `json:"tags"` - TransactionID int64 `json:"transaction_id,omitempty"` + BootstrapTime float64 `json:"bootstrap_time,omitempty"` + Endpoint string `json:"endpoint"` + Failure *string `json:"failure"` + IP string `json:"ip"` + Port int `json:"port"` + Transport string `json:"transport"` + Provider string `json:"provider"` + OpenVPNOptions ArchivalOpenVPNOptions `json:"openvpn_options"` + T0 float64 `json:"t0,omitempty"` + T float64 `json:"t"` + Tags []string `json:"tags"` + TransactionID int64 `json:"transaction_id,omitempty"` } // ArchivalOpenVPNOptions is a subset of [vpnconfig.OpenVPNOptions] that we want to include @@ -420,10 +420,3 @@ type ArchivalOpenVPNOptions struct { Cipher string `json:"cipher,omitempty"` Compression string `json:"compression,omitempty"` } - -// ArchivalOpenVPNConnectStatus is the status of ArchivalOpenVPNConnectResult. -type ArchivalOpenVPNConnectStatus struct { - Blocked *bool `json:"blocked,omitempty"` - Failure *string `json:"failure"` - Success bool `json:"success"` -} From 8c5fa1181bca51d86dc45b836378c91b64d59be3 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 4 Jun 2024 18:40:09 +0200 Subject: [PATCH 47/53] abort experiment if no urls returned --- internal/engine/inputloader.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/engine/inputloader.go b/internal/engine/inputloader.go index 29135156af..d338703203 100644 --- a/internal/engine/inputloader.go +++ b/internal/engine/inputloader.go @@ -360,14 +360,10 @@ func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLI } if len(urls) <= 0 { - // loadRemote returns ErrNoURLsReturned at this point for webconnectivity, - // but for OpenVPN we want to return a sensible default to be - // able to probe some endpoints even in very restrictive environments. - // Do note this means that you have to provide valid credentials - // by some other means. - for _, endpoint := range openvpn.DefaultEndpoints { - urls = append(urls, model.OOAPIURLInfo{URL: endpoint.AsInputURI()}) - } + // At some point we might want to return [openvpn.DefaultEndpoints], + // but for now we're assuming that no targets means we've disabled + // the experiment on the backend. + return nil, ErrNoURLsReturned } return urls, nil } From 6377ba80745dd1828c08582429add9b400aec797 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 5 Jun 2024 17:41:02 +0200 Subject: [PATCH 48/53] add stub to receive options from oonirun descriptors --- internal/experiment/openvpn/endpoint.go | 7 ++++--- internal/experiment/openvpn/endpoint_test.go | 10 +++++----- internal/experiment/openvpn/openvpn.go | 21 ++++++++++++-------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index 5c8c8a57c2..6289d75cda 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -255,11 +255,12 @@ func getOpenVPNConfig( return cfg, nil } -// extractBase64Blob is used to pass credentials as command-line options. -func extractBase64Blob(val string) (string, error) { +// 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) { - return "", fmt.Errorf("%w: %s", ErrBadBase64Blob, "missing base64 prefix") + // 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 { diff --git a/internal/experiment/openvpn/endpoint_test.go b/internal/experiment/openvpn/endpoint_test.go index bb141bf942..4a954178c8 100644 --- a/internal/experiment/openvpn/endpoint_test.go +++ b/internal/experiment/openvpn/endpoint_test.go @@ -345,7 +345,7 @@ func Test_getVPNConfig_with_unknown_provider(t *testing.T) { func Test_extractBase64Blob(t *testing.T) { t.Run("decode good blob", func(t *testing.T) { blob := "base64:dGhlIGJsdWUgb2N0b3B1cyBpcyB3YXRjaGluZw==" - decoded, err := extractBase64Blob(blob) + decoded, err := maybeExtractBase64Blob(blob) if decoded != "the blue octopus is watching" { t.Fatal("could not decoded blob correctly") } @@ -355,28 +355,28 @@ func Test_extractBase64Blob(t *testing.T) { }) t.Run("try decode without prefix", func(t *testing.T) { blob := "dGhlIGJsdWUgb2N0b3B1cyBpcyB3YXRjaGluZw==" - _, err := extractBase64Blob(blob) + _, err := maybeExtractBase64Blob(blob) if !errors.Is(err, ErrBadBase64Blob) { t.Fatal("should fail without prefix") } }) t.Run("bad base64 blob should fail", func(t *testing.T) { blob := "base64:dGhlIGJsdWUgb2N0b3B1cyBpcyB3YXRjaGluZw" - _, err := extractBase64Blob(blob) + _, 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 := extractBase64Blob(blob) + _, 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 := extractBase64Blob(blob) + _, err := maybeExtractBase64Blob(blob) if !errors.Is(err, ErrBadBase64Blob) { t.Fatal("bad base64 data should fail") } diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index caca8424c5..cf4fc3b343 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -18,7 +18,7 @@ import ( ) const ( - testVersion = "0.1.1" + testVersion = "0.1.2" openVPNProcol = "openvpn" ) @@ -27,11 +27,16 @@ const ( // This contains all the settings that user can set to modify the behaviour // of this experiment. By tagging these variables with `ooni:"..."`, we allow // miniooni's -O flag to find them and set them. +// TODO(ainghazal): do pass Auth, Cipher and Compress to OpenVPN config options. type Config struct { - 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"` - SafeCA string `ooni:"ca to connect to the OpenVPN endpoint"` + Auth string `ooni:"OpenVPN authentication to use"` + Cipher string `ooni:"OpenVPN cipher to use"` + Compress string `ooni:"OpenVPN compression to use"` + Provider string `ooni:"VPN provider"` + Obfuscation string `ooni:"Obfuscation to use (obfs4, none)"` + SafeKey string `ooni:"key to connect to the OpenVPN endpoint"` + SafeCert string `ooni:"cert to connect to the OpenVPN endpoint"` + SafeCA string `ooni:"ca to connect to the OpenVPN endpoint"` } // TestKeys contains the experiment's result. @@ -156,19 +161,19 @@ func MaybeGetCredentialsFromOptions(cfg Config, opts *vpnconfig.OpenVPNOptions, if ok := hasCredentialsInOptions(cfg, method); !ok { return false, nil } - ca, err := extractBase64Blob(cfg.SafeCA) + ca, err := maybeExtractBase64Blob(cfg.SafeCA) if err != nil { return false, err } opts.CA = []byte(ca) - key, err := extractBase64Blob(cfg.SafeKey) + key, err := maybeExtractBase64Blob(cfg.SafeKey) if err != nil { return false, err } opts.Key = []byte(key) - cert, err := extractBase64Blob(cfg.SafeCert) + cert, err := maybeExtractBase64Blob(cfg.SafeCert) if err != nil { return false, err } From 9b27cc1ec180f11a79de33c89c3f25c343fa3872 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 5 Jun 2024 17:48:04 +0200 Subject: [PATCH 49/53] use measurexlite.NewFailure --- internal/experiment/openvpn/openvpn.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index cf4fc3b343..2d2af0e01b 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -276,14 +276,9 @@ func (m *Measurer) connectAndHandshake( 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. // Any error will be returned as a failure in the SingleConnection result. tun, err := tunnel.Start(ctx, dialer, openvpnConfig) - if err != nil { - failure = err.Error() - } if tun != nil { defer tun.Close() } @@ -308,7 +303,7 @@ func (m *Measurer) connectAndHandshake( OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ BootstrapTime: bootstrapTime, Endpoint: endpoint.String(), - Failure: &failure, + Failure: measurexlite.NewFailure(err), IP: endpoint.IPAddr, Port: port, Transport: endpoint.Transport, From 196d58496b110aeb9ed16655a8376f52c2051ebd Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 5 Jun 2024 18:05:28 +0200 Subject: [PATCH 50/53] update tests --- internal/experiment/openvpn/endpoint_test.go | 7 +++++-- internal/experiment/openvpn/openvpn_test.go | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/experiment/openvpn/endpoint_test.go b/internal/experiment/openvpn/endpoint_test.go index 4a954178c8..bc33301419 100644 --- a/internal/experiment/openvpn/endpoint_test.go +++ b/internal/experiment/openvpn/endpoint_test.go @@ -355,10 +355,13 @@ func Test_extractBase64Blob(t *testing.T) { }) t.Run("try decode without prefix", func(t *testing.T) { blob := "dGhlIGJsdWUgb2N0b3B1cyBpcyB3YXRjaGluZw==" - _, err := maybeExtractBase64Blob(blob) - if !errors.Is(err, ErrBadBase64Blob) { + 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" diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index b1e4b816d9..ed0f2b9adb 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -39,7 +39,7 @@ func TestNewExperimentMeasurer(t *testing.T) { if m.ExperimentName() != "openvpn" { t.Fatal("invalid ExperimentName") } - if m.ExperimentVersion() != "0.1.1" { + if m.ExperimentVersion() != "0.1.2" { t.Fatal("invalid ExperimentVersion") } } From 1a60013e6d51ecd1ee3e4a36da421c5a0e891828 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Wed, 5 Jun 2024 18:29:05 +0200 Subject: [PATCH 51/53] update test after change in default endpoints if api fails --- internal/engine/inputloader_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/engine/inputloader_test.go b/internal/engine/inputloader_test.go index 0a7a889bb2..bc8b9686d5 100644 --- a/internal/engine/inputloader_test.go +++ b/internal/engine/inputloader_test.go @@ -611,11 +611,11 @@ func TestInputLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) { }, } out, err := il.loadRemote(context.Background()) - if err != nil { - t.Fatal(err) + if err != expected { + t.Fatal("we expected an error") } - if len(out) != 2 { - t.Fatal("we expected 2 fallback URLs") + if len(out) != 0 { + t.Fatal("we expected no fallback URLs") } } From 9fac70c5acdafa616ac2b39a078719288227621b Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 6 Jun 2024 11:01:19 +0200 Subject: [PATCH 52/53] fix(registry): update openvpn registration --- internal/registry/openvpn.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/internal/registry/openvpn.go b/internal/registry/openvpn.go index 7dba22e807..cf9244786f 100644 --- a/internal/registry/openvpn.go +++ b/internal/registry/openvpn.go @@ -10,15 +10,17 @@ import ( ) func init() { - AllExperiments["openvpn"] = &Factory{ - build: func(config interface{}) model.ExperimentMeasurer { - return openvpn.NewExperimentMeasurer( - *config.(*openvpn.Config), "openvpn", - ) - }, - config: &openvpn.Config{}, - enabledByDefault: true, - interruptible: true, - inputPolicy: model.InputOrQueryBackend, + AllExperiments["openvpn"] = func() *Factory { + return &Factory{ + build: func(config interface{}) model.ExperimentMeasurer { + return openvpn.NewExperimentMeasurer( + *config.(*openvpn.Config), "openvpn", + ) + }, + config: &openvpn.Config{}, + enabledByDefault: true, + interruptible: true, + inputPolicy: model.InputOrQueryBackend, + } } } From 5afb8bd0a7acefb79abf35778e651c71c78ffc3a Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 6 Jun 2024 11:03:23 +0200 Subject: [PATCH 53/53] Update internal/engine/session.go --- internal/engine/session.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/engine/session.go b/internal/engine/session.go index bf9374dcf6..1a6bc5443b 100644 --- a/internal/engine/session.go +++ b/internal/engine/session.go @@ -371,7 +371,9 @@ func (s *Session) FetchTorTargets( return nil, err } - // here we could also lock the mutex. + // TODO(bassosimone,DecFox): here we could also lock the mutex + // or we should consider using the same strategy we used for the + // experiments, where we separated mutable state into dedicated types. return clnt.FetchTorTargets(ctx, cc) }