diff --git a/internal/experiment/openvpn/endpoint.go b/internal/experiment/openvpn/endpoint.go index d18b50680..914eba8c7 100644 --- a/internal/experiment/openvpn/endpoint.go +++ b/internal/experiment/openvpn/endpoint.go @@ -54,7 +54,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: @@ -86,6 +86,9 @@ func newEndpointFromInputString(uri string) (*endpoint, error) { 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, diff --git a/internal/experiment/openvpn/endpoint_test.go b/internal/experiment/openvpn/endpoint_test.go new file mode 100644 index 000000000..5b14a0ad1 --- /dev/null +++ b/internal/experiment/openvpn/endpoint_test.go @@ -0,0 +1,143 @@ +package openvpn + +import ( + "errors" + "testing" + + "github.com/google/go-cmp/cmp" +) + +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: "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: "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, + }, + } + 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", + }, + } + 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)) + } + }) + } +} + +// TODO: test the endpoint uri string too. diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index f4948edfb..4174e50c9 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -15,6 +15,42 @@ import ( vpntracex "github.com/ooni/minivpn/pkg/tracex" ) +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.1" { + t.Fatal("invalid ExperimentVersion") + } +} + func TestNewTestKeys(t *testing.T) { tk := openvpn.NewTestKeys() if tk.Success != false { @@ -31,6 +67,8 @@ func TestNewTestKeys(t *testing.T) { } } +// TODO refactoring tests ----------------------------------------------- + func TestAddConnectionTestKeys(t *testing.T) { t.Run("append connection result to empty keys", func(t *testing.T) { tk := openvpn.NewTestKeys() @@ -116,36 +154,8 @@ func TestAllConnectionsSuccessful(t *testing.T) { func TestSuccess(t *testing.T) { m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn") - if m.ExperimentName() != "openvpn" { - t.Fatal("invalid ExperimentName") - } - if m.ExperimentVersion() != "0.1.1" { - t.Fatal("invalid ExperimentVersion") - } ctx := context.Background() - sess := &mocks.Session{ - MockLogger: func() model.Logger { - return model.DiscardLogger - }, - MockFetchOpenVPNConfig: func(context.Context, string, string) (*model.OOAPIVPNProviderConfig, error) { - return &model.OOAPIVPNProviderConfig{ - Provider: "provider", - Config: &struct { - CA string "json:\"ca\"" - Cert string "json:\"cert,omitempty\"" - Key string "json:\"key,omitempty\"" - Username string "json:\"username,omitempty\"" - Password string "json:\"password,omitempty\"" - }{ - CA: "ca", - Cert: "cert", - Key: "key", - }, - Inputs: []string{}, - DateUpdated: time.Now(), - }, nil - }, - } + sess := makeMockSession() callbacks := model.NewPrinterCallbacks(sess.Logger()) measurement := new(model.Measurement) args := &model.ExperimentArgs{ @@ -160,7 +170,8 @@ func TestSuccess(t *testing.T) { } } -func TestFailure(t *testing.T) { +// TODO -- test incorrect certs failure. +func TestBadInputFailure(t *testing.T) { m := openvpn.NewExperimentMeasurer(openvpn.Config{}, "openvpn") ctx := context.Background() sess := &mocks.Session{ @@ -179,3 +190,10 @@ func TestFailure(t *testing.T) { t.Fatal("expected an error here") } } + +func TestVPNInput(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + // TODO -- do a real test +}