From 642154d8f6d2274e28fccd11ace87bbce6a9d6e9 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 2 Jul 2024 12:35:09 +0200 Subject: [PATCH 1/5] add bootstrap time --- internal/experiment/openvpn/openvpn.go | 28 ++++++++++++++++++--- internal/experiment/openvpn/openvpn_test.go | 4 +-- internal/model/archival.go | 2 +- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 23e904a9e..7b20463ff 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -17,7 +17,7 @@ import ( const ( testName = "openvpn" - testVersion = "0.1.3" + testVersion = "0.1.4" openVPNProtocol = "openvpn" ) @@ -48,6 +48,9 @@ type TestKeys struct { NetworkEvents []*vpntracex.Event `json:"network_events"` TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` OpenVPNHandshake []*model.ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` + BootstrapTime float64 `json:"bootstrap_time,omitempty"` + Tunnel string `json:"tunnel"` + Failure *string `json:"failure"` } // NewTestKeys creates new openvpn TestKeys. @@ -57,11 +60,15 @@ func NewTestKeys() *TestKeys { NetworkEvents: []*vpntracex.Event{}, TCPConnect: []*model.ArchivalTCPConnectResult{}, OpenVPNHandshake: []*model.ArchivalOpenVPNHandshakeResult{}, + BootstrapTime: 0, + Tunnel: "openvpn", + Failure: nil, } } // SingleConnection contains the results of a single handshake. type SingleConnection struct { + BootstrapTime float64 TCPConnect *model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` OpenVPNHandshake *model.ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` NetworkEvents []*vpntracex.Event `json:"network_events"` @@ -79,6 +86,13 @@ func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { } tk.OpenVPNHandshake = append(tk.OpenVPNHandshake, result.OpenVPNHandshake) tk.NetworkEvents = append(tk.NetworkEvents, result.NetworkEvents...) + + // we assume one measurement has exactly one effective connection + tk.BootstrapTime = result.BootstrapTime + + if result.OpenVPNHandshake.Failure != nil { + tk.Failure = result.OpenVPNHandshake.Failure + } } // AllConnectionsSuccessful returns true if all the registered handshakes have nil failures. @@ -147,12 +161,18 @@ func (m *Measurer) connectAndHandshake( handshakeEvents := handshakeTracer.Trace() port, _ := strconv.Atoi(endpoint.Port) - t0, t, bootstrapTime := TimestampsFromHandshake(handshakeEvents) + t0, t, handshakeTime := TimestampsFromHandshake(handshakeEvents) + + var bootstrapTime float64 + if err == nil { + bootstrapTime = time.Since(zeroTime).Seconds() + } return &SingleConnection{ - TCPConnect: trace.FirstTCPConnectOrNil(), + BootstrapTime: bootstrapTime, + TCPConnect: trace.FirstTCPConnectOrNil(), OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ - BootstrapTime: bootstrapTime, + HandshakeTime: handshakeTime, Endpoint: endpoint.String(), Failure: measurexlite.NewFailure(err), IP: endpoint.IPAddr, diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index f050fab3a..521d1636a 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -79,7 +79,7 @@ func TestAddConnectionTestKeys(t *testing.T) { TransactionID: 1, }, OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ - BootstrapTime: 1, + HandshakeTime: 1, Endpoint: "aa", Failure: nil, IP: "1.1.1.1", @@ -111,7 +111,7 @@ func TestAddConnectionTestKeys(t *testing.T) { sc := &openvpn.SingleConnection{ TCPConnect: nil, OpenVPNHandshake: &model.ArchivalOpenVPNHandshakeResult{ - BootstrapTime: 1, + HandshakeTime: 1, Endpoint: "aa", Failure: nil, IP: "1.1.1.1", diff --git a/internal/model/archival.go b/internal/model/archival.go index a25ee5710..8a17e745e 100644 --- a/internal/model/archival.go +++ b/internal/model/archival.go @@ -399,9 +399,9 @@ type ArchivalNetworkEvent struct { // ArchivalOpenVPNHandshakeResult contains the result of a OpenVPN handshake. type ArchivalOpenVPNHandshakeResult struct { - BootstrapTime float64 `json:"bootstrap_time,omitempty"` Endpoint string `json:"endpoint"` Failure *string `json:"failure"` + HandshakeTime float64 `json:"handshake_time,omitempty"` IP string `json:"ip"` Port int `json:"port"` Transport string `json:"transport"` From 94374dfaeb2de24e095c6a5d3e7fc8046fe47d82 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 2 Jul 2024 12:57:28 +0200 Subject: [PATCH 2/5] add tests for bootstrap time --- internal/experiment/openvpn/openvpn.go | 8 +- internal/experiment/openvpn/openvpn_test.go | 103 +++++++++++++++++++- 2 files changed, 108 insertions(+), 3 deletions(-) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 7b20463ff..0a42f79bb 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -72,9 +72,8 @@ type SingleConnection struct { 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. + // like pingResults or urlgetter calls. Be sure to modify the spec when that happens. } // AddConnectionTestKeys adds the result of a single OpenVPN connection attempt to the @@ -92,6 +91,7 @@ func (tk *TestKeys) AddConnectionTestKeys(result *SingleConnection) { if result.OpenVPNHandshake.Failure != nil { tk.Failure = result.OpenVPNHandshake.Failure + tk.BootstrapTime = 0 } } @@ -163,6 +163,7 @@ func (m *Measurer) connectAndHandshake( t0, t, handshakeTime := TimestampsFromHandshake(handshakeEvents) + // the bootstrap time is defined to be zero if there's a handshake failure. var bootstrapTime float64 if err == nil { bootstrapTime = time.Since(zeroTime).Seconds() @@ -209,7 +210,9 @@ func TimestampsFromHandshake(events []*vpntracex.Event) (float64, float64, float return t0, t, duration } +// TODO: delete-me // FetchProviderCredentials will extract credentials from the configuration we gathered for a given provider. +/* func (m *Measurer) FetchProviderCredentials( ctx context.Context, sess model.ExperimentSession, @@ -222,6 +225,7 @@ func (m *Measurer) FetchProviderCredentials( } return config, nil } +*/ // Run implements model.ExperimentMeasurer.Run. // A single run expects exactly ONE input (endpoint), but we can modify whether diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index 521d1636a..b99fcfb69 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -10,6 +10,7 @@ import ( "github.com/google/go-cmp/cmp" vpntracex "github.com/ooni/minivpn/pkg/tracex" "github.com/ooni/probe-cli/v3/internal/experiment/openvpn" + "github.com/ooni/probe-cli/v3/internal/measurexlite" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/targetloading" @@ -40,7 +41,7 @@ func TestNewExperimentMeasurer(t *testing.T) { if m.ExperimentName() != "openvpn" { t.Fatal("invalid ExperimentName") } - if m.ExperimentVersion() != "0.1.3" { + if m.ExperimentVersion() != "0.1.4" { t.Fatal("invalid ExperimentVersion") } } @@ -219,6 +220,7 @@ func TestVPNInput(t *testing.T) { // TODO(ainghazal): 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.Measurer) @@ -248,6 +250,7 @@ func TestMeasurer_FetchProviderCredentials(t *testing.T) { } }) } +*/ func TestSuccess(t *testing.T) { m := openvpn.NewExperimentMeasurer() @@ -313,3 +316,101 @@ func TestTimestampsFromHandshake(t *testing.T) { } }) } + +func TestBootstrapTimeWithNoFailure(t *testing.T) { + bootstrapTime := 1.2305 + tk := openvpn.NewTestKeys() + sc := &openvpn.SingleConnection{ + BootstrapTime: bootstrapTime, + 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: &model.ArchivalOpenVPNHandshakeResult{ + HandshakeTime: 1.20, + Endpoint: "aa", + Failure: nil, + IP: "1.1.1.1", + Port: 1194, + Transport: "tcp", + Provider: "unknown", + OpenVPNOptions: model.ArchivalOpenVPNOptions{}, + T0: 0.03, + T: 1.23, + Tags: []string{}, + TransactionID: 1, + }, + NetworkEvents: []*vpntracex.Event{}, + } + tk.AddConnectionTestKeys(sc) + + if tk.Failure != nil { + t.Fatal("expected nil failure") + } + if tk.BootstrapTime != bootstrapTime { + t.Fatal("wrong bootstrap time") + } + if tk.Tunnel != "openvpn" { + t.Fatal("tunnel should be openvpn") + } +} + +func TestBootstrapTimeWithFailure(t *testing.T) { + bootstrapTime := 6.1 + + handshakeError := errors.New("mocked error") + handshakeFailure := measurexlite.NewFailure(handshakeError) + + tk := openvpn.NewTestKeys() + sc := &openvpn.SingleConnection{ + BootstrapTime: bootstrapTime, + 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: &model.ArchivalOpenVPNHandshakeResult{ + HandshakeTime: 1.20, + Endpoint: "aa", + Failure: handshakeFailure, + IP: "1.1.1.1", + Port: 1194, + Transport: "tcp", + Provider: "unknown", + OpenVPNOptions: model.ArchivalOpenVPNOptions{}, + T0: 0.03, + T: 1.23, + Tags: []string{}, + TransactionID: 1, + }, + NetworkEvents: []*vpntracex.Event{}, + } + tk.AddConnectionTestKeys(sc) + + if tk.Failure != handshakeFailure { + t.Fatalf("expected handshake failure, got %v", tk.Failure) + } + if tk.BootstrapTime != 0 { + t.Fatalf("wrong bootstrap time: expected 0, got %v", tk.BootstrapTime) + } + if tk.Tunnel != "openvpn" { + t.Fatal("tunnel should be openvpn") + } +} From 6779a48f14d96e46e408c4ea1a360e82f00f3243 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 2 Jul 2024 12:58:26 +0200 Subject: [PATCH 3/5] remove unused code --- internal/experiment/openvpn/openvpn.go | 17 -------- internal/experiment/openvpn/openvpn_test.go | 46 ++++----------------- 2 files changed, 7 insertions(+), 56 deletions(-) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index 0a42f79bb..d69dc3988 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -210,23 +210,6 @@ func TimestampsFromHandshake(events []*vpntracex.Event) (float64, float64, float return t0, t, duration } -// TODO: delete-me -// 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 nil, 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. diff --git a/internal/experiment/openvpn/openvpn_test.go b/internal/experiment/openvpn/openvpn_test.go index b99fcfb69..58bb90339 100644 --- a/internal/experiment/openvpn/openvpn_test.go +++ b/internal/experiment/openvpn/openvpn_test.go @@ -213,45 +213,6 @@ func TestBadTargetURLFailure(t *testing.T) { } } -func TestVPNInput(t *testing.T) { - if testing.Short() { - t.Skip("skip test in short mode") - } - // TODO(ainghazal): 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.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.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 TestSuccess(t *testing.T) { m := openvpn.NewExperimentMeasurer() ctx := context.Background() @@ -414,3 +375,10 @@ func TestBootstrapTimeWithFailure(t *testing.T) { t.Fatal("tunnel should be openvpn") } } + +func TestVPNInput(t *testing.T) { + if testing.Short() { + t.Skip("skip test in short mode") + } + // TODO(ainghazal): do a real test, get credentials etc. +} From 3063d6e125303dea8a6e6b04c32f688a1a0a49ea Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 2 Jul 2024 13:01:34 +0200 Subject: [PATCH 4/5] x --- internal/experiment/openvpn/richerinput_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/experiment/openvpn/richerinput_test.go b/internal/experiment/openvpn/richerinput_test.go index 6c8097f1a..0e829f3d9 100644 --- a/internal/experiment/openvpn/richerinput_test.go +++ b/internal/experiment/openvpn/richerinput_test.go @@ -225,5 +225,4 @@ func TestTargetLoaderLoadFromBackend(t *testing.T) { if targets[1].String() != "openvpn://target1" { t.Fatal("expected openvpn://target1") } - } From 03e533fd784dbb508a1ff142cbac2ebd3b30ad05 Mon Sep 17 00:00:00 2001 From: ain ghazal Date: Tue, 2 Jul 2024 13:08:06 +0200 Subject: [PATCH 5/5] bootstrap time should not be omitted --- internal/experiment/openvpn/openvpn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/experiment/openvpn/openvpn.go b/internal/experiment/openvpn/openvpn.go index d69dc3988..bc81da16b 100644 --- a/internal/experiment/openvpn/openvpn.go +++ b/internal/experiment/openvpn/openvpn.go @@ -48,7 +48,7 @@ type TestKeys struct { NetworkEvents []*vpntracex.Event `json:"network_events"` TCPConnect []*model.ArchivalTCPConnectResult `json:"tcp_connect,omitempty"` OpenVPNHandshake []*model.ArchivalOpenVPNHandshakeResult `json:"openvpn_handshake"` - BootstrapTime float64 `json:"bootstrap_time,omitempty"` + BootstrapTime float64 `json:"bootstrap_time"` Tunnel string `json:"tunnel"` Failure *string `json:"failure"` }