From cea69f786e056f4b7b4a6d3193ab71c8c952dbd5 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Sat, 4 Mar 2023 02:17:41 +0100 Subject: [PATCH 01/16] RiseupVPN: Don't do TLS verification if fetching the ca cert failed or the ca was invalid. Instead of early returning the test, we can still gather the necessary configs to run the gateway tests. This change implies that there might occur multiple failures while fetching data from the API, which is why TestKeys.APIFailures becomes a string array. --- internal/experiment/riseupvpn/riseupvpn.go | 31 +++++++++------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index ff662cebb1..5b364399fb 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -61,7 +61,7 @@ type Config struct { // TestKeys contains riseupvpn test keys. type TestKeys struct { urlgetter.TestKeys - APIFailure *string `json:"api_failure"` + APIFailure []string `json:"api_failure"` APIStatus string `json:"api_status"` CACertStatus bool `json:"ca_cert_status"` FailingGateways []GatewayConnection `json:"failing_gateways"` @@ -86,12 +86,9 @@ func (tk *TestKeys) UpdateProviderAPITestKeys(v urlgetter.MultiOutput) { tk.Requests = append(tk.Requests, v.TestKeys.Requests...) tk.TCPConnect = append(tk.TCPConnect, v.TestKeys.TCPConnect...) tk.TLSHandshakes = append(tk.TLSHandshakes, v.TestKeys.TLSHandshakes...) - if tk.APIStatus != "ok" { - return // we already flipped the state - } if v.TestKeys.Failure != nil { tk.APIStatus = "blocked" - tk.APIFailure = v.TestKeys.Failure + tk.APIFailure = append(tk.APIFailure, *v.TestKeys.Failure) return } } @@ -147,11 +144,6 @@ func (tk *TestKeys) AddCACertFetchTestKeys(testKeys urlgetter.TestKeys) { tk.Requests = append(tk.Requests, testKeys.Requests...) tk.TCPConnect = append(tk.TCPConnect, testKeys.TCPConnect...) tk.TLSHandshakes = append(tk.TLSHandshakes, testKeys.TLSHandshakes...) - if testKeys.Failure != nil { - tk.APIStatus = "blocked" - tk.APIFailure = tk.Failure - tk.CACertStatus = false - } } // Measurer performs the measurement. @@ -208,17 +200,15 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { tk := entry.TestKeys testkeys.AddCACertFetchTestKeys(tk) if tk.Failure != nil { - // TODO(bassosimone,cyberta): should we update the testkeys - // in this case (e.g., APIFailure?) - // See https://github.com/ooni/probe/issues/1432. - return nil - } - if ok := certPool.AppendCertsFromPEM([]byte(tk.HTTPResponseBody)); !ok { testkeys.CACertStatus = false testkeys.APIStatus = "blocked" - errorValue := "invalid_ca" - testkeys.APIFailure = &errorValue - return nil + testkeys.APIFailure = append(testkeys.APIFailure, *tk.Failure) + certPool = nil + } else if ok := certPool.AppendCertsFromPEM([]byte(tk.HTTPResponseBody)); !ok { + testkeys.CACertStatus = false + testkeys.APIStatus = "blocked" + testkeys.APIFailure = append(testkeys.APIFailure, "invalid_ca") + certPool = nil } } @@ -230,16 +220,19 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { CertPool: certPool, Method: "GET", FailOnHTTPError: true, + NoTLSVerify: !testkeys.CACertStatus, }}, {Target: eipServiceURL, Config: urlgetter.Config{ CertPool: certPool, Method: "GET", FailOnHTTPError: true, + NoTLSVerify: !testkeys.CACertStatus, }}, {Target: geoServiceURL, Config: urlgetter.Config{ CertPool: certPool, Method: "GET", FailOnHTTPError: true, + NoTLSVerify: !testkeys.CACertStatus, }}, } for entry := range multi.CollectOverall(ctx, inputs, 1, 50, "riseupvpn", callbacks) { From 6b426ac2c06e6ebc6e4c70df7dbab2d99ae0b92c Mon Sep 17 00:00:00 2001 From: cyBerta Date: Mon, 6 Mar 2023 15:19:18 +0100 Subject: [PATCH 02/16] RiseupVPN: re-run fetches from the API using Tor+Snowflake, if the API was considered to be blocked --- internal/experiment/riseupvpn/riseupvpn.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 5b364399fb..6afedfccf0 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -235,10 +235,20 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { NoTLSVerify: !testkeys.CACertStatus, }}, } + for entry := range multi.CollectOverall(ctx, inputs, 1, 50, "riseupvpn", callbacks) { testkeys.UpdateProviderAPITestKeys(entry) } + if testkeys.APIStatus == "blocked" { + for _, input := range inputs { + input.Config.Tunnel = "torsf" + } + for entry := range multi.CollectOverall(ctx, inputs, 1, 50, "riseupvpn", callbacks) { + testkeys.UpdateProviderAPITestKeys(entry) + } + } + // test gateways now testkeys.TransportStatus = map[string]string{} gateways := parseGateways(testkeys) From c8f806744167ad1678985db4fa8aa6bc73300d08 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Mon, 6 Mar 2023 15:20:27 +0100 Subject: [PATCH 03/16] Fixing tests after chaning riseupvpn test logic. Gateway tests are no longer skipped, API config data is fetched even if CA cert is invalid or missing --- .../experiment/riseupvpn/riseupvpn_test.go | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn_test.go b/internal/experiment/riseupvpn/riseupvpn_test.go index 9af91c9699..d82b4d2eec 100644 --- a/internal/experiment/riseupvpn/riseupvpn_test.go +++ b/internal/experiment/riseupvpn/riseupvpn_test.go @@ -291,7 +291,7 @@ func TestUpdateWithMixedResults(t *testing.T) { if tk.APIStatus != "blocked" { t.Fatal("ApiStatus should be blocked") } - if *tk.APIFailure != netxlite.FailureEOFError { + if len(tk.APIFailure) > 0 && tk.APIFailure[0] != netxlite.FailureEOFError { t.Fatal("invalid ApiFailure") } if tk.FailingGateways != nil { @@ -344,11 +344,24 @@ func TestInvalidCaCert(t *testing.T) { if tk.APIStatus != "blocked" { t.Fatal("ApiStatus should be blocked") } - if tk.FailingGateways != nil { - t.Fatal("invalid FailingGateways") + + if tk.FailingGateways == nil || len(tk.FailingGateways) != 1 { + t.Fatal("invalid length of FailingGateways") } - if tk.TransportStatus != nil { - t.Fatal("invalid TransportStatus") + + gw := tk.FailingGateways[0] + if gw.IP != "234.345.234.345" { + t.Fatal("invalid failed gateway ip: " + fmt.Sprint(gw.IP)) + } + if gw.Port != 443 { + t.Fatal("invalid failed gateway port: " + fmt.Sprint(gw.Port)) + } + if gw.TransportType != "openvpn" { + t.Fatal("invalid failed transport type: " + fmt.Sprint(gw.TransportType)) + } + + if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "ok" { + t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) } } @@ -371,17 +384,17 @@ func TestFailureCaCertFetch(t *testing.T) { t.Fatal("invalid ApiStatus") } - if tk.APIFailure != nil { - t.Fatal("ApiFailure should be null") + if tk.APIFailure == nil || len(tk.APIFailure) != 1 || tk.APIFailure[0] != io.EOF.Error() { + t.Fatal("ApiFailure should not be null" + fmt.Sprint(tk.APIFailure)) } - if len(tk.Requests) > 1 { - t.Fatal("Unexpected requests") + if len(tk.Requests) == 1 { + t.Fatal("Too less requests, expected to run all API requests") } if tk.FailingGateways != nil { t.Fatal("invalid FailingGateways") } - if tk.TransportStatus != nil { - t.Fatal("invalid TransportStatus") + if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "ok" || tk.TransportStatus["obfs4"] != "ok" { + t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) } } From f71b99855d6050d3623fda37fa2ecd2b02109248 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Mon, 6 Mar 2023 23:19:45 +0100 Subject: [PATCH 04/16] Filter gateways: just expose a limited set of gateways for network testing, avoid those known to be overloaded. --- internal/experiment/riseupvpn/riseupvpn.go | 103 +++++++++++++++++++-- 1 file changed, 96 insertions(+), 7 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 6afedfccf0..8bd24a9d6e 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -24,6 +24,8 @@ const ( tcpConnect = "tcpconnect://" ) +var locations = []string{"Seattle", "Amsterdam"} + // EipService is the main JSON object of eip-service.json. type EipService struct { Gateways []GatewayV3 @@ -36,6 +38,7 @@ type GatewayV3 struct { } Host string IPAddress string `json:"ip_address"` + Location string `json:"location"` } // TransportV3 describes a transport. @@ -53,6 +56,24 @@ type GatewayConnection struct { TransportType string `json:"transport_type"` } +// GatewayLoad describes the load of a single Gateway. +type GatewayLoad struct { + Host string `json:"host"` + Fullness float64 `json:"fullness"` + Overload bool `json:"overload"` +} + +// GeoService represents the geoService API (also known as menshen) json response +type GeoService struct { + IPAddress string `json:"ip"` + Country string `json:"cc"` + City string `json:"city"` + Latitude float64 `json:"lat"` + Longitude float64 `json:"lon"` + Gateways []string `json:"gateways"` + SortedGateways []GatewayLoad `json:"sortedGateways"` +} + // Config contains the riseupvpn experiment config. type Config struct { urlgetter.Config @@ -302,18 +323,76 @@ func generateMultiInputs(gateways []GatewayV3, transportType string) []urlgetter } func parseGateways(testKeys *TestKeys) []GatewayV3 { + var eipService *EipService = nil + var geoService *GeoService = nil for _, requestEntry := range testKeys.Requests { if requestEntry.Request.URL == eipServiceURL && requestEntry.Failure == nil { - // TODO(bassosimone,cyberta): is it reasonable that we discard - // the error when the JSON we fetched cannot be parsed? - // See https://github.com/ooni/probe/issues/1432 - eipService, err := DecodeEIP3(requestEntry.Response.Body.Value) - if err == nil { - return eipService.Gateways + var err error = nil + eipService, err = DecodeEIP3(requestEntry.Response.Body.Value) + if err != nil { + testKeys.APIFailure = append(testKeys.APIFailure, "invalid_eipservice_response") + return nil + } + } else if requestEntry.Request.URL == geoServiceURL && requestEntry.Failure == nil { + var err error = nil + geoService, err = DecodeGeoService(requestEntry.Response.Body.Value) + if err != nil { + testKeys.APIFailure = append(testKeys.APIFailure, "invalid_geoservice_response") } } } - return nil + return filterGateways(eipService, geoService) +} + +func filterGateways(eipService *EipService, geoService *GeoService) []GatewayV3 { + var result []GatewayV3 = nil + for _, gateway := range eipService.Gateways { + if !gateway.hasTransport("obfs4") || + !gateway.isLocationUnderTest() || + geoService != nil && !geoService.isHealthyGateway(gateway) { + continue + } + result = append(result, gateway) + if len(result) == 3 { + return result + } + } + + return result +} + +func (gateway *GatewayV3) hasTransport(s string) bool { + for _, transport := range gateway.Capabilities.Transport { + if s == transport.Type { + return true + } + } + return false +} + +func (gateway *GatewayV3) isLocationUnderTest() bool { + for _, location := range locations { + if location == gateway.Location { + return true + } + } + return false +} + +func (geoService *GeoService) isHealthyGateway(gateway GatewayV3) bool { + if geoService.SortedGateways == nil { + // Earlier versions of the geoservice don't include the sorted gateway list containing the load info, + // so we can't say anything about the load of a gateway in that case. + // We assume it's an healthy location. Riseup will switch to the updated API soon *fingers crossed* + return true + } + for _, gatewayLoad := range geoService.SortedGateways { + if gatewayLoad.Host == gateway.Host { + return !gatewayLoad.Overload + } + } + + return false } // DecodeEIP3 decodes eip-service.json version 3 @@ -326,6 +405,16 @@ func DecodeEIP3(body string) (*EipService, error) { return &eip, nil } +// DecodeGeoService decodes geoService json +func DecodeGeoService(body string) (*GeoService, error) { + var gs GeoService + err := json.Unmarshal([]byte(body), &gs) + if err != nil { + return nil, err + } + return &gs, nil +} + // NewExperimentMeasurer creates a new ExperimentMeasurer. func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { return Measurer{Config: config} From e87a99b47319ba7596bf14151b89d1c64835c546 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Mon, 6 Mar 2023 23:48:29 +0100 Subject: [PATCH 05/16] RiseupVPN: reduce multi-collectors (url-getter) overallCount after reducing the amount of tested gateways --- internal/experiment/riseupvpn/riseupvpn.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 8bd24a9d6e..8be2be3c38 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -217,7 +217,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { FailOnHTTPError: true, }}, } - for entry := range multi.CollectOverall(ctx, inputs, 0, 50, "riseupvpn", callbacks) { + for entry := range multi.CollectOverall(ctx, inputs, 0, 20, "riseupvpn", callbacks) { tk := entry.TestKeys testkeys.AddCACertFetchTestKeys(tk) if tk.Failure != nil { @@ -257,7 +257,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { }}, } - for entry := range multi.CollectOverall(ctx, inputs, 1, 50, "riseupvpn", callbacks) { + for entry := range multi.CollectOverall(ctx, inputs, 1, 20, "riseupvpn", callbacks) { testkeys.UpdateProviderAPITestKeys(entry) } @@ -265,7 +265,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { for _, input := range inputs { input.Config.Tunnel = "torsf" } - for entry := range multi.CollectOverall(ctx, inputs, 1, 50, "riseupvpn", callbacks) { + for entry := range multi.CollectOverall(ctx, inputs, 1, 20, "riseupvpn", callbacks) { testkeys.UpdateProviderAPITestKeys(entry) } } From d39a04148d8835cd27b2403f81a63cc7c70fb537 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Fri, 17 Mar 2023 16:47:33 +0100 Subject: [PATCH 06/16] add null check in gateway filter function --- internal/experiment/riseupvpn/riseupvpn.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 8be2be3c38..c54e75f93b 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -346,15 +346,17 @@ func parseGateways(testKeys *TestKeys) []GatewayV3 { func filterGateways(eipService *EipService, geoService *GeoService) []GatewayV3 { var result []GatewayV3 = nil - for _, gateway := range eipService.Gateways { - if !gateway.hasTransport("obfs4") || - !gateway.isLocationUnderTest() || - geoService != nil && !geoService.isHealthyGateway(gateway) { - continue - } - result = append(result, gateway) - if len(result) == 3 { - return result + if eipService != nil { + for _, gateway := range eipService.Gateways { + if !gateway.hasTransport("obfs4") || + !gateway.isLocationUnderTest() || + geoService != nil && !geoService.isHealthyGateway(gateway) { + continue + } + result = append(result, gateway) + if len(result) == 3 { + return result + } } } From 745157153f46b49d4057ed4372b9eb7d8545b99b Mon Sep 17 00:00:00 2001 From: cyBerta Date: Sat, 18 Mar 2023 21:11:10 +0100 Subject: [PATCH 07/16] test only gateways from locations with high redundancy regarding obfs4 bridges, avoid hard-coding locations in the test --- internal/experiment/riseupvpn/riseupvpn.go | 44 ++++++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index c54e75f93b..eb72b1efdb 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -24,8 +24,6 @@ const ( tcpConnect = "tcpconnect://" ) -var locations = []string{"Seattle", "Amsterdam"} - // EipService is the main JSON object of eip-service.json. type EipService struct { Gateways []GatewayV3 @@ -344,12 +342,14 @@ func parseGateways(testKeys *TestKeys) []GatewayV3 { return filterGateways(eipService, geoService) } +// filterGateways selects a subset of available gateways supporting obfs4 func filterGateways(eipService *EipService, geoService *GeoService) []GatewayV3 { var result []GatewayV3 = nil if eipService != nil { + locations := getLocationsUnderTest(eipService, geoService) for _, gateway := range eipService.Gateways { if !gateway.hasTransport("obfs4") || - !gateway.isLocationUnderTest() || + !gateway.isLocationUnderTest(locations) || geoService != nil && !geoService.isHealthyGateway(gateway) { continue } @@ -359,6 +359,42 @@ func filterGateways(eipService *EipService, geoService *GeoService) []GatewayV3 } } } + return result +} + +// getLocationsUnderTest parses all gateways supporting obfs4 and returns the two locations having most obfs4 bridges +func getLocationsUnderTest(eipService *EipService, geoService *GeoService) []string { + var result []string = nil + if eipService != nil { + locationMap := map[string]int{} + locations := []string{} + for _, gateway := range eipService.Gateways { + if !gateway.hasTransport("obfs4") { + continue + } + if _, ok := locationMap[gateway.Location]; !ok { + locations = append(locations, gateway.Location) + } + locationMap[gateway.Location] += 1 + } + + location1 := "" + location2 := "" + for _, location := range locations { + if locationMap[location] > locationMap[location1] { + location2 = location1 + location1 = location + } else if locationMap[location] > locationMap[location2] { + location2 = location + } + } + if location1 != "" { + result = append(result, location1) + } + if location2 != "" { + result = append(result, location2) + } + } return result } @@ -372,7 +408,7 @@ func (gateway *GatewayV3) hasTransport(s string) bool { return false } -func (gateway *GatewayV3) isLocationUnderTest() bool { +func (gateway *GatewayV3) isLocationUnderTest(locations []string) bool { for _, location := range locations { if location == gateway.Location { return true From 015fd542001024c1ecc6f9f00b02157083e064a8 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Mon, 20 Mar 2023 15:27:40 +0100 Subject: [PATCH 08/16] update RiseupVPN tests --- internal/experiment/riseupvpn/riseupvpn.go | 16 +- .../experiment/riseupvpn/riseupvpn_test.go | 353 ++++++++++++++++-- 2 files changed, 338 insertions(+), 31 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index eb72b1efdb..f268693e55 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -29,14 +29,17 @@ type EipService struct { Gateways []GatewayV3 } +// Capabilities is a list of transports a gateway supports +type Capabilities struct { + Transport []TransportV3 +} + // GatewayV3 describes a gateway. type GatewayV3 struct { - Capabilities struct { - Transport []TransportV3 - } - Host string - IPAddress string `json:"ip_address"` - Location string `json:"location"` + Capabilities Capabilities + Host string + IPAddress string `json:"ip_address"` + Location string `json:"location"` } // TransportV3 describes a transport. @@ -430,6 +433,7 @@ func (geoService *GeoService) isHealthyGateway(gateway GatewayV3) bool { } } + // gateways that are not included in the geoservice should be considered unusable return false } diff --git a/internal/experiment/riseupvpn/riseupvpn_test.go b/internal/experiment/riseupvpn/riseupvpn_test.go index d82b4d2eec..f2591eaab0 100644 --- a/internal/experiment/riseupvpn/riseupvpn_test.go +++ b/internal/experiment/riseupvpn/riseupvpn_test.go @@ -16,6 +16,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/legacy/mockable" "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/model/mocks" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/tracex" ) @@ -142,8 +143,9 @@ const ( "serial": 3, "version": 3 }` - geoservice = `{"ip":"51.15.0.88","cc":"NL","city":"Haarlem","lat":52.381,"lon":4.6275,"gateways":["test1.riseup.net","test2.riseup.net"]}` - cacert = `-----BEGIN CERTIFICATE----- + geoservice = `{"ip":"51.15.0.88","cc":"NL","city":"Haarlem","lat":52.381,"lon":4.6275,"gateways":["test1.riseup.net","test2.riseup.net"]}` + geoService_update = `{"ip":"51.15.0.88","cc":"NL","city":"Haarlem","lat":52.381,"lon":4.6275,"gateways":["test1.riseup.net","test2.riseup.net"], "sortedGateways": [{ "host": "test1.riseup.net", "fullness": 0.2, "overload": false }, { "host": "test2.riseup.net", "fullness": 0.9, "overload": true }]}` + cacert = `-----BEGIN CERTIFICATE----- MIIFjTCCA3WgAwIBAgIBATANBgkqhkiG9w0BAQ0FADBZMRgwFgYDVQQKDA9SaXNl dXAgTmV0d29ya3MxGzAZBgNVBAsMEmh0dHBzOi8vcmlzZXVwLm5ldDEgMB4GA1UE AwwXUmlzZXVwIE5ldHdvcmtzIFJvb3QgQ0EwHhcNMTQwNDI4MDAwMDAwWhcNMjQw @@ -184,9 +186,10 @@ UN9SaWRlWKSdP4haujnzCoJbM7dU9bjvlGZNyXEekgeT0W2qFeGGp+yyUWw8tNsp providerurl = "https://riseup.net/provider.json" geoserviceurl = "https://api.black.riseup.net:9001/json" cacerturl = "https://black.riseup.net/ca.crt" - openvpnurl1 = "tcpconnect://234.345.234.345:443" - openvpnurl2 = "tcpconnect://123.456.123.456:443" + openvpnurl1 = "tcpconnect://234.345.234.345:443" // "Seattle" + openvpnurl2 = "tcpconnect://123.456.123.456:443" // "Paris" obfs4url1 = "tcpconnect://234.345.234.345:23042" + obfs4url2 = "tcpconnect://123.456.123.456:444" ) var RequestResponse = map[string]string{ @@ -197,6 +200,7 @@ var RequestResponse = map[string]string{ openvpnurl1: "", openvpnurl2: "", obfs4url1: "", + obfs4url2: "", } func TestNewExperimentMeasurer(t *testing.T) { @@ -204,20 +208,22 @@ func TestNewExperimentMeasurer(t *testing.T) { if measurer.ExperimentName() != "riseupvpn" { t.Fatal("unexpected name") } - if measurer.ExperimentVersion() != "0.2.0" { + if measurer.ExperimentVersion() != "0.3.0" { t.Fatal("unexpected version") } } func TestGood(t *testing.T) { + // the gateaway openvpnurl2 is filtered out, since it doesn't support additionally obfs4 measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{ cacerturl: true, eipserviceurl: true, providerurl: true, geoserviceurl: true, openvpnurl1: true, - openvpnurl2: true, + openvpnurl2: false, obfs4url1: true, + obfs4url2: false, })) tk := measurement.TestKeys.(*riseupvpn.TestKeys) @@ -311,6 +317,7 @@ func TestInvalidCaCert(t *testing.T) { openvpnurl1: "", openvpnurl2: "", obfs4url1: "", + obfs4url2: "", } measurer := riseupvpn.Measurer{ Config: riseupvpn.Config{}, @@ -319,13 +326,17 @@ func TestInvalidCaCert(t *testing.T) { eipserviceurl: true, providerurl: true, geoserviceurl: true, - openvpnurl1: false, - openvpnurl2: true, + openvpnurl1: true, + openvpnurl2: false, // filtered out, no obfs4 support obfs4url1: true, + obfs4url2: false, // filtered out }), } ctx := context.Background() - sess := &mockable.Session{MockableLogger: log.Log} + sess := &mocks.Session{MockLogger: func() model.Logger { + return model.DiscardLogger + }} + measurement := new(model.Measurement) callbacks := model.NewPrinterCallbacks(log.Log) args := &model.ExperimentArgs{ @@ -345,22 +356,11 @@ func TestInvalidCaCert(t *testing.T) { t.Fatal("ApiStatus should be blocked") } - if tk.FailingGateways == nil || len(tk.FailingGateways) != 1 { - t.Fatal("invalid length of FailingGateways") - } - - gw := tk.FailingGateways[0] - if gw.IP != "234.345.234.345" { - t.Fatal("invalid failed gateway ip: " + fmt.Sprint(gw.IP)) - } - if gw.Port != 443 { - t.Fatal("invalid failed gateway port: " + fmt.Sprint(gw.Port)) - } - if gw.TransportType != "openvpn" { - t.Fatal("invalid failed transport type: " + fmt.Sprint(gw.TransportType)) + if tk.FailingGateways != nil { + t.Fatal("invalid FailingGateways") } - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "ok" { + if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "ok" || tk.TransportStatus["obfs4"] != "ok" { t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) } } @@ -494,15 +494,89 @@ func TestFailureGeoIpServiceBlocked(t *testing.T) { } } -func TestFailureGateway1(t *testing.T) { +func TestFailureGateway1TransportNOK(t *testing.T) { measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{ cacerturl: true, eipserviceurl: true, providerurl: true, geoserviceurl: true, - openvpnurl1: false, + openvpnurl1: false, // failed gateway + openvpnurl2: false, // filtered out + obfs4url1: true, + obfs4url2: false, + })) + tk := measurement.TestKeys.(*riseupvpn.TestKeys) + if tk.CACertStatus != true { + t.Fatal("invalid CACertStatus ") + } + + if tk.FailingGateways == nil || len(tk.FailingGateways) != 1 { + t.Fatal("unexpected amount of failing gateways") + } + + gw := tk.FailingGateways[0] + if gw.IP != "234.345.234.345" { + t.Fatal("invalid failed gateway ip: " + fmt.Sprint(gw.IP)) + } + if gw.Port != 443 { + t.Fatal("invalid failed gateway port: " + fmt.Sprint(gw.Port)) + } + if gw.TransportType != "openvpn" { + t.Fatal("invalid failed transport type: " + fmt.Sprint(gw.TransportType)) + } + + if tk.APIStatus == "blocked" { + t.Fatal("invalid ApiStatus") + } + + if tk.APIFailure != nil { + t.Fatal("ApiFailure should be null") + } + + if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "blocked" { + t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) + } + + if tk.TransportStatus == nil || tk.TransportStatus["obfs4"] == "blocked" { + t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) + } +} + +func TestFailureGateway1TransportOK(t *testing.T) { + eipService, err := riseupvpn.DecodeEIP3(eipservice) + if err != nil { + t.Fatal("Preconditions for the test are not met.") + } + + //add obfs4 capability to 1. gateway + addObfs4Capability(&eipService.Gateways[0]) + + eipservicejson, err := json.Marshal(eipService) + if err != nil { + t.Fatal(err) + } + t.Log(string(eipservicejson)) + + requestResponseMap := map[string]string{ + eipserviceurl: string(eipservicejson), + providerurl: provider, + geoserviceurl: geoservice, + cacerturl: cacert, + openvpnurl1: "", + openvpnurl2: "", + obfs4url1: "", + obfs4url2: "", + } + + measurement := runDefaultMockTest(t, generateMockGetter(requestResponseMap, map[string]bool{ + cacerturl: true, + eipserviceurl: true, + providerurl: true, + geoserviceurl: true, + openvpnurl1: false, // failed gateway openvpnurl2: true, obfs4url1: true, + obfs4url2: true, })) tk := measurement.TestKeys.(*riseupvpn.TestKeys) if tk.CACertStatus != true { @@ -636,6 +710,208 @@ func TestMissingTransport(t *testing.T) { } } +func TestIgnoreOverloadedGateways(t *testing.T) { + eipService, err := riseupvpn.DecodeEIP3(eipservice) + if err != nil { + t.Fatal("Preconditions for the test are not met.") + } + + //add obfs4 capability for 1. gateway + addObfs4Capability(&eipService.Gateways[0]) + eipservicejson, err := json.Marshal(eipService) + if err != nil { + t.Fatal(err) + } + + requestResponseMap := map[string]string{ + eipserviceurl: string(eipservicejson), + providerurl: provider, + geoserviceurl: geoService_update, + cacerturl: cacert, + openvpnurl1: "", + openvpnurl2: "", + obfs4url1: "", + obfs4url2: "", + } + + measurement := runDefaultMockTest(t, generateMockGetter(requestResponseMap, map[string]bool{ + cacerturl: true, + eipserviceurl: true, + providerurl: true, + geoserviceurl: true, + openvpnurl1: false, // should be filtered out, since overloaded + openvpnurl2: true, + obfs4url1: false, // should be filtered out, since overloaded + obfs4url2: true, + })) + + tk := measurement.TestKeys.(*riseupvpn.TestKeys) + + if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] == "blocked" || tk.TransportStatus["obfs"] == "blocked" { + t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) + } + + if tk.FailingGateways != nil { + t.Fatal("unexpected amount of failing gateways. Overloaded gateways shouldn't be tested. " + fmt.Sprint(tk.FailingGateways)) + } +} + +func TestIgnoreLocationsWithFewObfs4Bridges(t *testing.T) { + eipService, err := riseupvpn.DecodeEIP3(eipservice) + if err != nil { + t.Fatal("Preconditions for the test are not met.") + } + + addObfs4Capability(&eipService.Gateways[0]) + addGateway(eipService, "vpn1.test", "123.12.123.11", "tokio") + addGateway(eipService, "vpn2.test", "123.12.123.12", "tokio") + addGateway(eipService, "vpn3.test", "123.12.123.13", "paris") + + eipservicejson, err := json.Marshal(eipService) + if err != nil { + t.Fatal(err) + } + + requestResponseMap := map[string]string{ + eipserviceurl: string(eipservicejson), + providerurl: provider, + geoserviceurl: geoservice, + cacerturl: cacert, + openvpnurl1: "", + openvpnurl2: "", + obfs4url1: "", + obfs4url2: "", + "tcpconnect://123.12.123.11:444": "", + "tcpconnect://123.12.123.12:444": "", + "tcpconnect://123.12.123.13:444": "", + } + + measurement := runDefaultMockTest(t, generateMockGetter(requestResponseMap, map[string]bool{ + cacerturl: true, + eipserviceurl: true, + providerurl: true, + geoserviceurl: false, + openvpnurl1: false, // should be filtered out, b/c its's location is not under test + openvpnurl2: true, + obfs4url1: false, // should be filtered out, b/c its's location is not under test + obfs4url2: true, + "tcpconnect://123.12.123.11:444": true, + "tcpconnect://123.12.123.12:444": true, + "tcpconnect://123.12.123.13:444": true, + })) + + tk := measurement.TestKeys.(*riseupvpn.TestKeys) + + if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] == "blocked" || tk.TransportStatus["obfs"] == "blocked" { + t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) + } + + if tk.FailingGateways != nil { + t.Fatal("unexpected amount of failing gateways. Only locations under test should be evaluated." + fmt.Sprint(tk.FailingGateways)) + } +} + +func TestIgnoreGatewaysNotIncludedInGeoAPIResponse(t *testing.T) { + eipService, err := riseupvpn.DecodeEIP3(eipservice) + if err != nil { + t.Fatal("Preconditions for the test are not met.") + } + + addGateway(eipService, "vpn1.test", "123.12.123.11", "tokio") + addGateway(eipService, "vpn2.test", "123.12.123.12", "tokio") + eipservicejson, err := json.Marshal(eipService) + if err != nil { + t.Fatal(err) + } + + requestResponseMap := map[string]string{ + eipserviceurl: string(eipservicejson), + providerurl: provider, + geoserviceurl: geoService_update, + cacerturl: cacert, + openvpnurl1: "", + openvpnurl2: "", + obfs4url1: "", + obfs4url2: "", + "tcpconnect://123.12.123.11:444": "", + "tcpconnect://123.12.123.12:444": "", + } + + measurement := runDefaultMockTest(t, generateMockGetter(requestResponseMap, map[string]bool{ + cacerturl: true, + eipserviceurl: true, + providerurl: true, + geoserviceurl: true, + openvpnurl1: true, + openvpnurl2: true, + obfs4url1: true, + obfs4url2: true, + "tcpconnect://123.12.123.11:444": false, // filtered out since they don't appear in the *valid* geoservice response + "tcpconnect://123.12.123.12:444": false, + })) + + tk := measurement.TestKeys.(*riseupvpn.TestKeys) + + if tk.FailingGateways != nil { + t.Fatal("unexpected amount of failing gateways. " + fmt.Sprint(tk.FailingGateways)) + } + +} + +func TestHandleInvalidGeoAPIResponse(t *testing.T) { + eipService, err := riseupvpn.DecodeEIP3(eipservice) + if err != nil { + t.Fatal("Preconditions for the test are not met.") + } + + //add obfs4 capability for 1. gateway + addObfs4Capability(&eipService.Gateways[0]) + eipservicejson, err := json.Marshal(eipService) + if err != nil { + t.Fatal(err) + } + + requestResponseMap := map[string]string{ + eipserviceurl: string(eipservicejson), + providerurl: provider, + geoserviceurl: "invalid", + cacerturl: cacert, + openvpnurl1: "", + openvpnurl2: "", + obfs4url1: "", + obfs4url2: "", + } + + measurement := runDefaultMockTest(t, generateMockGetter(requestResponseMap, map[string]bool{ + cacerturl: true, + eipserviceurl: true, + providerurl: true, + geoserviceurl: true, + openvpnurl1: false, // all gateways are assumed to be healthy + openvpnurl2: true, // and aren't filtered out + obfs4url1: false, // because the geoservice reply is misconfigured + obfs4url2: true, // and hence it's impossible to read the overload status + })) + + tk := measurement.TestKeys.(*riseupvpn.TestKeys) + + if tk.FailingGateways == nil || len(tk.FailingGateways) != 2 { + t.Fatal("unexpected amount of failing gateways. " + fmt.Sprint(tk.FailingGateways)) + } + + foundFailure := false + for _, failure := range tk.APIFailure { + if failure == "invalid_geoservice_response" { + foundFailure = true + break + } + } + + if !foundFailure { + t.Fatal("expected API Failure invalid_geoservice_response is missing: " + fmt.Sprint(tk.APIFailure)) + } +} + func TestSummaryKeysInvalidType(t *testing.T) { measurement := new(model.Measurement) m := &riseupvpn.Measurer{} @@ -827,3 +1103,30 @@ func runDefaultMockTest(t *testing.T, multiGetter urlgetter.MultiGetter) *model. } return measurement } + +func addObfs4Capability(gateway *riseupvpn.GatewayV3) { + transports := gateway.Capabilities.Transport + transport := riseupvpn.TransportV3{ + Type: "obfs4", + Protocols: []string{"tcp"}, + Ports: []string{"444"}, + Options: map[string]string{ + "cert": "XXXXXXXXXXXXXXXXXXXXXXXXX", + "iatMode": "0", + }, + } + + transports = append(transports, transport) + gateway.Capabilities.Transport = transports +} + +func addGateway(service *riseupvpn.EipService, host string, ipAddress string, location string) { + gateway := riseupvpn.GatewayV3{ + Capabilities: riseupvpn.Capabilities{}, + Host: host, + IPAddress: ipAddress, + Location: location, + } + addObfs4Capability(&gateway) + service.Gateways = append(service.Gateways, gateway) +} From a5a56853fa729f5f5335b6fb21336ad3af647291 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Mon, 20 Mar 2023 15:28:08 +0100 Subject: [PATCH 09/16] update RiseupVPN test version --- internal/experiment/riseupvpn/riseupvpn.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index f268693e55..eead30512b 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -17,7 +17,7 @@ import ( const ( testName = "riseupvpn" - testVersion = "0.2.0" + testVersion = "0.3.0" eipServiceURL = "https://api.black.riseup.net:443/3/config/eip-service.json" providerURL = "https://riseup.net/provider.json" geoServiceURL = "https://api.black.riseup.net:9001/json" From 55dc3ff3a7c8a34fb4b7199999624ab8a9318d51 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Mon, 20 Mar 2023 19:47:41 +0100 Subject: [PATCH 10/16] only take failures of RiseupVPNs main configuration endpoint into consideration for APIStatus blocked --- internal/experiment/riseupvpn/riseupvpn.go | 8 ++++--- .../experiment/riseupvpn/riseupvpn_test.go | 22 +++++++++++++------ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index eead30512b..7d9c3d1a00 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -109,7 +109,11 @@ func (tk *TestKeys) UpdateProviderAPITestKeys(v urlgetter.MultiOutput) { tk.TCPConnect = append(tk.TCPConnect, v.TestKeys.TCPConnect...) tk.TLSHandshakes = append(tk.TLSHandshakes, v.TestKeys.TLSHandshakes...) if v.TestKeys.Failure != nil { - tk.APIStatus = "blocked" + for _, request := range v.TestKeys.Requests { + if request.Request.URL == eipServiceURL && request.Failure != nil { + tk.APIStatus = "blocked" + } + } tk.APIFailure = append(tk.APIFailure, *v.TestKeys.Failure) return } @@ -223,12 +227,10 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { testkeys.AddCACertFetchTestKeys(tk) if tk.Failure != nil { testkeys.CACertStatus = false - testkeys.APIStatus = "blocked" testkeys.APIFailure = append(testkeys.APIFailure, *tk.Failure) certPool = nil } else if ok := certPool.AppendCertsFromPEM([]byte(tk.HTTPResponseBody)); !ok { testkeys.CACertStatus = false - testkeys.APIStatus = "blocked" testkeys.APIFailure = append(testkeys.APIFailure, "invalid_ca") certPool = nil } diff --git a/internal/experiment/riseupvpn/riseupvpn_test.go b/internal/experiment/riseupvpn/riseupvpn_test.go index f2591eaab0..023303bdf9 100644 --- a/internal/experiment/riseupvpn/riseupvpn_test.go +++ b/internal/experiment/riseupvpn/riseupvpn_test.go @@ -263,7 +263,7 @@ func TestUpdateWithMixedResults(t *testing.T) { tk.UpdateProviderAPITestKeys(urlgetter.MultiOutput{ Input: urlgetter.MultiInput{ Config: urlgetter.Config{Method: "GET"}, - Target: "https://api.black.riseup.net:443/3/config/eip-service.json", + Target: "https://riseup.net/provider.json", }, TestKeys: urlgetter.TestKeys{ HTTPResponseStatus: 200, @@ -272,9 +272,17 @@ func TestUpdateWithMixedResults(t *testing.T) { tk.UpdateProviderAPITestKeys(urlgetter.MultiOutput{ Input: urlgetter.MultiInput{ Config: urlgetter.Config{Method: "GET"}, - Target: "https://riseup.net/provider.json", + Target: "https://api.black.riseup.net:443/3/config/eip-service.json", }, TestKeys: urlgetter.TestKeys{ + Requests: []model.ArchivalHTTPRequestResult{ + { + Request: model.ArchivalHTTPRequest{URL: "https://api.black.riseup.net:443/3/config/eip-service.json"}, + Failure: (func() *string { + s := "eof" + return &s + })(), + }}, FailedOperation: (func() *string { s := netxlite.HTTPRoundTripOperation return &s @@ -352,8 +360,8 @@ func TestInvalidCaCert(t *testing.T) { if tk.CACertStatus == true { t.Fatal("unexpected CaCertStatus") } - if tk.APIStatus != "blocked" { - t.Fatal("ApiStatus should be blocked") + if tk.APIStatus != "ok" { + t.Fatal("ApiStatus should be ok") } if tk.FailingGateways != nil { @@ -380,7 +388,7 @@ func TestFailureCaCertFetch(t *testing.T) { if tk.CACertStatus != false { t.Fatal("invalid CACertStatus ") } - if tk.APIStatus != "blocked" { + if tk.APIStatus != "ok" { t.Fatal("invalid ApiStatus") } @@ -453,7 +461,7 @@ func TestFailureProviderUrlBlocked(t *testing.T) { if tk.CACertStatus != true { t.Fatal("invalid CACertStatus ") } - if tk.APIStatus != "blocked" { + if tk.APIStatus != "ok" { t.Fatal("invalid ApiStatus") } @@ -485,7 +493,7 @@ func TestFailureGeoIpServiceBlocked(t *testing.T) { } } - if tk.APIStatus != "blocked" { + if tk.APIStatus != "ok" { t.Fatal("invalid ApiStatus") } From afc4d641939ceeb193a7626d05f8fa3547e758df Mon Sep 17 00:00:00 2001 From: cyBerta Date: Tue, 21 Mar 2023 11:53:34 +0100 Subject: [PATCH 11/16] don't use invalid CA cert as indicator for an anomaly --- internal/experiment/riseupvpn/riseupvpn.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 7d9c3d1a00..2064320883 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -488,8 +488,8 @@ func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, e sk.FailingGateways = len(tk.FailingGateways) sk.TransportStatus = tk.TransportStatus // Note: the order in the following OR chains matter: TransportStatus - // is nil if APIBlocked or !CACertStatus - sk.IsAnomaly = (sk.APIBlocked || !tk.CACertStatus || + // is nil if APIBlocked + sk.IsAnomaly = (sk.APIBlocked || tk.TransportStatus["openvpn"] == "blocked" || tk.TransportStatus["obfs4"] == "blocked") return sk, nil From fd20e35615bd1fc3a8bde4ef5294213947dfd9a9 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Tue, 21 Mar 2023 11:54:52 +0100 Subject: [PATCH 12/16] use response code to check if an API request was successful --- internal/experiment/riseupvpn/riseupvpn.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 2064320883..ee694449f9 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -329,14 +329,14 @@ func parseGateways(testKeys *TestKeys) []GatewayV3 { var eipService *EipService = nil var geoService *GeoService = nil for _, requestEntry := range testKeys.Requests { - if requestEntry.Request.URL == eipServiceURL && requestEntry.Failure == nil { + if requestEntry.Request.URL == eipServiceURL && requestEntry.Response.Code == 200 { var err error = nil eipService, err = DecodeEIP3(requestEntry.Response.Body.Value) if err != nil { testKeys.APIFailure = append(testKeys.APIFailure, "invalid_eipservice_response") return nil } - } else if requestEntry.Request.URL == geoServiceURL && requestEntry.Failure == nil { + } else if requestEntry.Request.URL == geoServiceURL && requestEntry.Response.Code == 200 { var err error = nil geoService, err = DecodeGeoService(requestEntry.Response.Body.Value) if err != nil { From 7ebfd8d553149072213c4ef7c3b39f8c4b54c259 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Tue, 21 Mar 2023 12:15:57 +0100 Subject: [PATCH 13/16] fix setting tor+snowflake tunnel in case it's needed --- internal/experiment/riseupvpn/riseupvpn.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index ee694449f9..0098857960 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -265,8 +265,8 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { } if testkeys.APIStatus == "blocked" { - for _, input := range inputs { - input.Config.Tunnel = "torsf" + for i := range inputs { + inputs[i].Config.Tunnel = "torsf" } for entry := range multi.CollectOverall(ctx, inputs, 1, 20, "riseupvpn", callbacks) { testkeys.UpdateProviderAPITestKeys(entry) From 0160f0fcdf0fd7fa99e665a0b7f97b4b5255c119 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Tue, 21 Mar 2023 18:29:39 +0100 Subject: [PATCH 14/16] fix progress numbers for the case the API urlgetter tests re-run with Tor+Snowflake --- internal/experiment/riseupvpn/riseupvpn.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 0098857960..5707efcfcc 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -268,7 +268,8 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { for i := range inputs { inputs[i].Config.Tunnel = "torsf" } - for entry := range multi.CollectOverall(ctx, inputs, 1, 20, "riseupvpn", callbacks) { + + for entry := range multi.CollectOverall(ctx, inputs, 5, 20, "riseupvpn", callbacks) { testkeys.UpdateProviderAPITestKeys(entry) } } @@ -279,19 +280,25 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { openvpnEndpoints := generateMultiInputs(gateways, "openvpn") obfs4Endpoints := generateMultiInputs(gateways, "obfs4") overallCount := 1 + len(inputs) + len(openvpnEndpoints) + len(obfs4Endpoints) + startCount := 1 + len(inputs) + if testkeys.APIStatus == "blocked" { + startCount += len(inputs) + overallCount += len(inputs) + } // measure openvpn in parallel for entry := range multi.CollectOverall( - ctx, openvpnEndpoints, 1+len(inputs), overallCount, "riseupvpn", callbacks) { + ctx, openvpnEndpoints, startCount, overallCount, "riseupvpn", callbacks) { testkeys.AddGatewayConnectTestKeys(entry, "openvpn") } + startCount += len(openvpnEndpoints) // measure obfs4 in parallel // TODO(bassosimone): when urlgetter is able to do obfs4 handshakes, here // can possibly also test for the obfs4 handshake. // See https://github.com/ooni/probe/issues/1463. for entry := range multi.CollectOverall( - ctx, obfs4Endpoints, 1+len(inputs)+len(openvpnEndpoints), overallCount, "riseupvpn", callbacks) { + ctx, obfs4Endpoints, startCount, overallCount, "riseupvpn", callbacks) { testkeys.AddGatewayConnectTestKeys(entry, "obfs4") } From 2e868a06fdb89c8f328693a006a7e4791ab951fa Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 12 Jul 2023 15:04:27 +0200 Subject: [PATCH 15/16] fix(riseupvpn): make tests compile without warnings This change lands on top of @cyBerta changes to make sure the tests compile after recent renamings and to avoid warnings. --- internal/experiment/riseupvpn/riseupvpn_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn_test.go b/internal/experiment/riseupvpn/riseupvpn_test.go index 023303bdf9..7fe666594c 100644 --- a/internal/experiment/riseupvpn/riseupvpn_test.go +++ b/internal/experiment/riseupvpn/riseupvpn_test.go @@ -14,9 +14,8 @@ import ( "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/experiment/riseupvpn" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" - "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/model/mocks" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/tracex" ) @@ -696,7 +695,7 @@ func TestMissingTransport(t *testing.T) { } ctx := context.Background() - sess := &mockable.Session{MockableLogger: log.Log} + sess := &mocks.Session{MockLogger: func() model.Logger { return log.Log }} measurement := new(model.Measurement) callbacks := model.NewPrinterCallbacks(log.Log) args := &model.ExperimentArgs{ @@ -1100,9 +1099,7 @@ func runDefaultMockTest(t *testing.T, multiGetter urlgetter.MultiGetter) *model. args := &model.ExperimentArgs{ Callbacks: model.NewPrinterCallbacks(log.Log), Measurement: measurement, - Session: &mockable.Session{ - MockableLogger: log.Log, - }, + Session: &mocks.Session{MockLogger: func() model.Logger { return log.Log }}, } err := measurer.Run(context.Background(), args) From c3264d8d5df7634d14b2322ddac3809ff1aed390 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Sat, 5 Aug 2023 04:18:56 +0200 Subject: [PATCH 16/16] address requested changes after merge review: * never return IsAnomaly=true * early return in case the communication to the API fails * remove (most) top level TestKeys, including custom SummaryKeys and keep urlgetter keys * however keeps invalid CA cert handling, since unattended expired certs was a root cause for false positives in the past * removes snowflake + tor handling as part of the complexity reduction of the test * remove API response bodys from testkeys, addresses also removal of geolocation API reponse * adapts unit tests --- internal/experiment/riseupvpn/riseupvpn.go | 129 ++---- .../experiment/riseupvpn/riseupvpn_test.go | 417 +++--------------- 2 files changed, 89 insertions(+), 457 deletions(-) diff --git a/internal/experiment/riseupvpn/riseupvpn.go b/internal/experiment/riseupvpn/riseupvpn.go index 5707efcfcc..19b0d58b6d 100644 --- a/internal/experiment/riseupvpn/riseupvpn.go +++ b/internal/experiment/riseupvpn/riseupvpn.go @@ -6,13 +6,11 @@ package riseupvpn import ( "context" "encoding/json" - "errors" "time" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/netxlite" - "github.com/ooni/probe-cli/v3/internal/tracex" ) const ( @@ -24,19 +22,19 @@ const ( tcpConnect = "tcpconnect://" ) -// EipService is the main JSON object of eip-service.json. -type EipService struct { +// EipServiceV3 is the main JSON object of eip-service.json. +type EipServiceV3 struct { Gateways []GatewayV3 } -// Capabilities is a list of transports a gateway supports -type Capabilities struct { +// CapabilitiesV3 is a list of transports a gateway supports +type CapabilitiesV3 struct { Transport []TransportV3 } // GatewayV3 describes a gateway. type GatewayV3 struct { - Capabilities Capabilities + Capabilities CapabilitiesV3 Host string IPAddress string `json:"ip_address"` Location string `json:"location"` @@ -50,13 +48,6 @@ type TransportV3 struct { Options map[string]string } -// GatewayConnection describes the connection to a riseupvpn gateway. -type GatewayConnection struct { - IP string `json:"ip"` - Port int `json:"port"` - TransportType string `json:"transport_type"` -} - // GatewayLoad describes the load of a single Gateway. type GatewayLoad struct { Host string `json:"host"` @@ -83,21 +74,15 @@ type Config struct { // TestKeys contains riseupvpn test keys. type TestKeys struct { urlgetter.TestKeys - APIFailure []string `json:"api_failure"` - APIStatus string `json:"api_status"` - CACertStatus bool `json:"ca_cert_status"` - FailingGateways []GatewayConnection `json:"failing_gateways"` - TransportStatus map[string]string `json:"transport_status"` + APIFailure []string `json:"api_failure"` + CACertStatus bool `json:"ca_cert_status"` } // NewTestKeys creates new riseupvpn TestKeys. func NewTestKeys() *TestKeys { return &TestKeys{ - APIFailure: nil, - APIStatus: "ok", - CACertStatus: true, - FailingGateways: nil, - TransportStatus: nil, + APIFailure: nil, + CACertStatus: true, } } @@ -109,11 +94,6 @@ func (tk *TestKeys) UpdateProviderAPITestKeys(v urlgetter.MultiOutput) { tk.TCPConnect = append(tk.TCPConnect, v.TestKeys.TCPConnect...) tk.TLSHandshakes = append(tk.TLSHandshakes, v.TestKeys.TLSHandshakes...) if v.TestKeys.Failure != nil { - for _, request := range v.TestKeys.Requests { - if request.Request.URL == eipServiceURL && request.Failure != nil { - tk.APIStatus = "blocked" - } - } tk.APIFailure = append(tk.APIFailure, *v.TestKeys.Failure) return } @@ -125,42 +105,6 @@ func (tk *TestKeys) UpdateProviderAPITestKeys(v urlgetter.MultiOutput) { func (tk *TestKeys) AddGatewayConnectTestKeys(v urlgetter.MultiOutput, transportType string) { tk.NetworkEvents = append(tk.NetworkEvents, v.TestKeys.NetworkEvents...) tk.TCPConnect = append(tk.TCPConnect, v.TestKeys.TCPConnect...) - for _, tcpConnect := range v.TestKeys.TCPConnect { - if !tcpConnect.Status.Success { - gatewayConnection := newGatewayConnection(tcpConnect, transportType) - tk.FailingGateways = append(tk.FailingGateways, *gatewayConnection) - } - } -} - -func (tk *TestKeys) updateTransportStatus(openvpnGatewayCount, obfs4GatewayCount int) { - failingOpenvpnGateways, failingObfs4Gateways := 0, 0 - for _, gw := range tk.FailingGateways { - if gw.TransportType == "openvpn" { - failingOpenvpnGateways++ - } else if gw.TransportType == "obfs4" { - failingObfs4Gateways++ - } - } - if failingOpenvpnGateways < openvpnGatewayCount { - tk.TransportStatus["openvpn"] = "ok" - } else { - tk.TransportStatus["openvpn"] = "blocked" - } - if failingObfs4Gateways < obfs4GatewayCount { - tk.TransportStatus["obfs4"] = "ok" - } else { - tk.TransportStatus["obfs4"] = "blocked" - } -} - -func newGatewayConnection( - tcpConnect tracex.TCPConnectEntry, transportType string) *GatewayConnection { - return &GatewayConnection{ - IP: tcpConnect.IP, - Port: tcpConnect.Port, - TransportType: transportType, - } } // AddCACertFetchTestKeys adds generic urlgetter.Get() testKeys to riseupvpn specific test keys @@ -264,27 +208,23 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { testkeys.UpdateProviderAPITestKeys(entry) } - if testkeys.APIStatus == "blocked" { - for i := range inputs { - inputs[i].Config.Tunnel = "torsf" - } - - for entry := range multi.CollectOverall(ctx, inputs, 5, 20, "riseupvpn", callbacks) { - testkeys.UpdateProviderAPITestKeys(entry) + if testkeys.APIFailure != nil { + // scrub reponse bodys before early returning + var scrubbedRequests = []model.ArchivalHTTPRequestResult{} + for _, requestEntry := range testkeys.Requests { + requestEntry.Response.Body = model.ArchivalHTTPBody{Value: "[scrubbed]"} + scrubbedRequests = append(scrubbedRequests, requestEntry) } + testkeys.Requests = scrubbedRequests + return nil } // test gateways now - testkeys.TransportStatus = map[string]string{} gateways := parseGateways(testkeys) openvpnEndpoints := generateMultiInputs(gateways, "openvpn") obfs4Endpoints := generateMultiInputs(gateways, "obfs4") overallCount := 1 + len(inputs) + len(openvpnEndpoints) + len(obfs4Endpoints) startCount := 1 + len(inputs) - if testkeys.APIStatus == "blocked" { - startCount += len(inputs) - overallCount += len(inputs) - } // measure openvpn in parallel for entry := range multi.CollectOverall( @@ -303,7 +243,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error { } // set transport status based on gateway test results - testkeys.updateTransportStatus(len(openvpnEndpoints), len(obfs4Endpoints)) + //testkeys.updateTransportStatus(len(openvpnEndpoints), len(obfs4Endpoints)) return nil } @@ -333,8 +273,9 @@ func generateMultiInputs(gateways []GatewayV3, transportType string) []urlgetter } func parseGateways(testKeys *TestKeys) []GatewayV3 { - var eipService *EipService = nil + var eipService *EipServiceV3 = nil var geoService *GeoService = nil + var scrubbedRequests = []model.ArchivalHTTPRequestResult{} for _, requestEntry := range testKeys.Requests { if requestEntry.Request.URL == eipServiceURL && requestEntry.Response.Code == 200 { var err error = nil @@ -350,12 +291,15 @@ func parseGateways(testKeys *TestKeys) []GatewayV3 { testKeys.APIFailure = append(testKeys.APIFailure, "invalid_geoservice_response") } } + requestEntry.Response.Body = model.ArchivalHTTPBody{Value: "[scrubbed]"} + scrubbedRequests = append(scrubbedRequests, requestEntry) } + testKeys.Requests = scrubbedRequests return filterGateways(eipService, geoService) } // filterGateways selects a subset of available gateways supporting obfs4 -func filterGateways(eipService *EipService, geoService *GeoService) []GatewayV3 { +func filterGateways(eipService *EipServiceV3, geoService *GeoService) []GatewayV3 { var result []GatewayV3 = nil if eipService != nil { locations := getLocationsUnderTest(eipService, geoService) @@ -375,7 +319,7 @@ func filterGateways(eipService *EipService, geoService *GeoService) []GatewayV3 } // getLocationsUnderTest parses all gateways supporting obfs4 and returns the two locations having most obfs4 bridges -func getLocationsUnderTest(eipService *EipService, geoService *GeoService) []string { +func getLocationsUnderTest(eipService *EipServiceV3, geoService *GeoService) []string { var result []string = nil if eipService != nil { locationMap := map[string]int{} @@ -447,8 +391,8 @@ func (geoService *GeoService) isHealthyGateway(gateway GatewayV3) bool { } // DecodeEIP3 decodes eip-service.json version 3 -func DecodeEIP3(body string) (*EipService, error) { - var eip EipService +func DecodeEIP3(body string) (*EipServiceV3, error) { + var eip EipServiceV3 err := json.Unmarshal([]byte(body), &eip) if err != nil { return nil, err @@ -476,28 +420,11 @@ func NewExperimentMeasurer(config Config) model.ExperimentMeasurer { // Note that this structure is part of the ABI contract with ooniprobe // therefore we should be careful when changing it. type SummaryKeys struct { - APIBlocked bool `json:"api_blocked"` - ValidCACert bool `json:"valid_ca_cert"` - FailingGateways int `json:"failing_gateways"` - TransportStatus map[string]string `json:"transport_status"` - IsAnomaly bool `json:"-"` + IsAnomaly bool `json:"-"` } // GetSummaryKeys implements model.ExperimentMeasurer.GetSummaryKeys. func (m Measurer) GetSummaryKeys(measurement *model.Measurement) (interface{}, error) { sk := SummaryKeys{IsAnomaly: false} - tk, ok := measurement.TestKeys.(*TestKeys) - if !ok { - return sk, errors.New("invalid test keys type") - } - sk.APIBlocked = tk.APIStatus != "ok" - sk.ValidCACert = tk.CACertStatus - sk.FailingGateways = len(tk.FailingGateways) - sk.TransportStatus = tk.TransportStatus - // Note: the order in the following OR chains matter: TransportStatus - // is nil if APIBlocked - sk.IsAnomaly = (sk.APIBlocked || - tk.TransportStatus["openvpn"] == "blocked" || - tk.TransportStatus["obfs4"] == "blocked") return sk, nil } diff --git a/internal/experiment/riseupvpn/riseupvpn_test.go b/internal/experiment/riseupvpn/riseupvpn_test.go index 7fe666594c..d094dd9abe 100644 --- a/internal/experiment/riseupvpn/riseupvpn_test.go +++ b/internal/experiment/riseupvpn/riseupvpn_test.go @@ -11,7 +11,6 @@ import ( "testing" "github.com/apex/log" - "github.com/google/go-cmp/cmp" "github.com/ooni/probe-cli/v3/internal/experiment/riseupvpn" "github.com/ooni/probe-cli/v3/internal/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/mocks" @@ -238,20 +237,18 @@ func TestGood(t *testing.T) { if tk.APIFailure != nil { t.Fatal("unexpected ApiFailure") } - if tk.APIStatus != "ok" { - t.Fatal("unexpected ApiStatus") - } if tk.CACertStatus != true { t.Fatal("unexpected CaCertStatus") } - if tk.FailingGateways != nil { - t.Fatal("unexpected FailingGateways value") - } - if tk.TransportStatus == nil { - t.Fatal("unexpected nil TransportStatus struct ") + + hasOpenvpn1 := false + for _, tcpConnect := range tk.TCPConnect { + if tcpConnect.IP == "234.345.234.345" { + hasOpenvpn1 = true + } } - if tk.TransportStatus["openvpn"] != "ok" { - t.Fatal("unexpected openvpn transport status") + if !hasOpenvpn1 { + t.Fatalf("Gateway tests should run %t", hasOpenvpn1) } } @@ -301,18 +298,11 @@ func TestUpdateWithMixedResults(t *testing.T) { HTTPResponseStatus: 200, }, }) - if tk.APIStatus != "blocked" { - t.Fatal("ApiStatus should be blocked") - } + if len(tk.APIFailure) > 0 && tk.APIFailure[0] != netxlite.FailureEOFError { t.Fatal("invalid ApiFailure") } - if tk.FailingGateways != nil { - t.Fatal("invalid FailingGateways") - } - if tk.TransportStatus != nil { - t.Fatal("invalid TransportStatus") - } + } func TestInvalidCaCert(t *testing.T) { @@ -359,17 +349,6 @@ func TestInvalidCaCert(t *testing.T) { if tk.CACertStatus == true { t.Fatal("unexpected CaCertStatus") } - if tk.APIStatus != "ok" { - t.Fatal("ApiStatus should be ok") - } - - if tk.FailingGateways != nil { - t.Fatal("invalid FailingGateways") - } - - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "ok" || tk.TransportStatus["obfs4"] != "ok" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } } func TestFailureCaCertFetch(t *testing.T) { @@ -387,9 +366,6 @@ func TestFailureCaCertFetch(t *testing.T) { if tk.CACertStatus != false { t.Fatal("invalid CACertStatus ") } - if tk.APIStatus != "ok" { - t.Fatal("invalid ApiStatus") - } if tk.APIFailure == nil || len(tk.APIFailure) != 1 || tk.APIFailure[0] != io.EOF.Error() { t.Fatal("ApiFailure should not be null" + fmt.Sprint(tk.APIFailure)) @@ -397,11 +373,10 @@ func TestFailureCaCertFetch(t *testing.T) { if len(tk.Requests) == 1 { t.Fatal("Too less requests, expected to run all API requests") } - if tk.FailingGateways != nil { - t.Fatal("invalid FailingGateways") - } - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "ok" || tk.TransportStatus["obfs4"] != "ok" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) + for _, tcpConnect := range tk.TCPConnect { + if tcpConnect.IP == openvpnurl1 || tcpConnect.IP == openvpnurl2 || tcpConnect.IP == obfs4url1 || tcpConnect.IP == obfs4url2 { + t.Fatal("No gateaway tests should be run if API fails") + } } } @@ -428,10 +403,6 @@ func TestFailureEipServiceBlocked(t *testing.T) { } } - if tk.APIStatus != "blocked" { - t.Fatal("invalid ApiStatus") - } - if tk.APIFailure == nil { t.Fatal("ApiFailure should not be null") } @@ -460,9 +431,6 @@ func TestFailureProviderUrlBlocked(t *testing.T) { if tk.CACertStatus != true { t.Fatal("invalid CACertStatus ") } - if tk.APIStatus != "ok" { - t.Fatal("invalid ApiStatus") - } if tk.APIFailure == nil { t.Fatal("ApiFailure should not be null") @@ -492,10 +460,6 @@ func TestFailureGeoIpServiceBlocked(t *testing.T) { } } - if tk.APIStatus != "ok" { - t.Fatal("invalid ApiStatus") - } - if tk.APIFailure == nil { t.Fatal("ApiFailure should not be null") } @@ -517,204 +481,20 @@ func TestFailureGateway1TransportNOK(t *testing.T) { t.Fatal("invalid CACertStatus ") } - if tk.FailingGateways == nil || len(tk.FailingGateways) != 1 { - t.Fatal("unexpected amount of failing gateways") - } - - gw := tk.FailingGateways[0] - if gw.IP != "234.345.234.345" { - t.Fatal("invalid failed gateway ip: " + fmt.Sprint(gw.IP)) - } - if gw.Port != 443 { - t.Fatal("invalid failed gateway port: " + fmt.Sprint(gw.Port)) - } - if gw.TransportType != "openvpn" { - t.Fatal("invalid failed transport type: " + fmt.Sprint(gw.TransportType)) - } - - if tk.APIStatus == "blocked" { - t.Fatal("invalid ApiStatus") - } - - if tk.APIFailure != nil { - t.Fatal("ApiFailure should be null") - } - - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } - - if tk.TransportStatus == nil || tk.TransportStatus["obfs4"] == "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } -} - -func TestFailureGateway1TransportOK(t *testing.T) { - eipService, err := riseupvpn.DecodeEIP3(eipservice) - if err != nil { - t.Fatal("Preconditions for the test are not met.") - } - - //add obfs4 capability to 1. gateway - addObfs4Capability(&eipService.Gateways[0]) - - eipservicejson, err := json.Marshal(eipService) - if err != nil { - t.Fatal(err) - } - t.Log(string(eipservicejson)) - - requestResponseMap := map[string]string{ - eipserviceurl: string(eipservicejson), - providerurl: provider, - geoserviceurl: geoservice, - cacerturl: cacert, - openvpnurl1: "", - openvpnurl2: "", - obfs4url1: "", - obfs4url2: "", - } - - measurement := runDefaultMockTest(t, generateMockGetter(requestResponseMap, map[string]bool{ - cacerturl: true, - eipserviceurl: true, - providerurl: true, - geoserviceurl: true, - openvpnurl1: false, // failed gateway - openvpnurl2: true, - obfs4url1: true, - obfs4url2: true, - })) - tk := measurement.TestKeys.(*riseupvpn.TestKeys) - if tk.CACertStatus != true { - t.Fatal("invalid CACertStatus ") - } - - if tk.FailingGateways == nil || len(tk.FailingGateways) != 1 { - t.Fatal("unexpected amount of failing gateways") - } - - gw := tk.FailingGateways[0] - if gw.IP != "234.345.234.345" { - t.Fatal("invalid failed gateway ip: " + fmt.Sprint(gw.IP)) - } - if gw.Port != 443 { - t.Fatal("invalid failed gateway port: " + fmt.Sprint(gw.Port)) - } - if gw.TransportType != "openvpn" { - t.Fatal("invalid failed transport type: " + fmt.Sprint(gw.TransportType)) - } - - if tk.APIStatus == "blocked" { - t.Fatal("invalid ApiStatus") + for _, tcpConnect := range tk.TCPConnect { + if !tcpConnect.Status.Success { + if tcpConnect.IP != "234.345.234.345" { + t.Fatal("invalid failed gateway ip: " + fmt.Sprint(tcpConnect.IP)) + } + if tcpConnect.Port != 443 { + t.Fatal("invalid failed gateway port: " + fmt.Sprint(tcpConnect.Port)) + } + } } if tk.APIFailure != nil { t.Fatal("ApiFailure should be null") } - - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] == "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } - - if tk.TransportStatus == nil || tk.TransportStatus["obfs4"] == "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } -} - -func TestFailureTransport(t *testing.T) { - measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{ - cacerturl: true, - eipserviceurl: true, - providerurl: true, - geoserviceurl: true, - openvpnurl1: false, - openvpnurl2: false, - obfs4url1: false, - })) - tk := measurement.TestKeys.(*riseupvpn.TestKeys) - - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } - - if tk.TransportStatus == nil || tk.TransportStatus["obfs4"] != "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } -} - -func TestMissingTransport(t *testing.T) { - eipService, err := riseupvpn.DecodeEIP3(eipservice) - if err != nil { - t.Fatal("Preconditions for the test are not met.") - } - - //remove obfs4 capability from 2. gateway so that our - //mock provider supports only openvpn - index := -1 - transports := eipService.Gateways[1].Capabilities.Transport - for i, transport := range transports { - if transport.Type == "obfs4" { - index = i - break - } - } - if index == -1 { - t.Fatal("Preconditions for the test are not met. Default eipservice string should contain obfs4 transport.") - } - - transports[index] = transports[len(transports)-1] - transports = transports[:len(transports)-1] - eipService.Gateways[1].Capabilities.Transport = transports - eipservicejson, err := json.Marshal(eipservice) - if err != nil { - t.Fatal(err) - } - - requestResponseMap := map[string]string{ - eipserviceurl: string(eipservicejson), - providerurl: provider, - geoserviceurl: geoservice, - cacerturl: cacert, - openvpnurl1: "", - openvpnurl2: "", - obfs4url1: "", - } - - measurer := riseupvpn.Measurer{ - Config: riseupvpn.Config{}, - Getter: generateMockGetter(requestResponseMap, map[string]bool{ - cacerturl: true, - eipserviceurl: true, - providerurl: true, - geoserviceurl: true, - openvpnurl1: true, - openvpnurl2: true, - obfs4url1: false, - }), - } - - ctx := context.Background() - sess := &mocks.Session{MockLogger: func() model.Logger { return log.Log }} - measurement := new(model.Measurement) - callbacks := model.NewPrinterCallbacks(log.Log) - args := &model.ExperimentArgs{ - Callbacks: callbacks, - Measurement: measurement, - Session: sess, - } - err = measurer.Run(ctx, args) - if err != nil { - t.Fatal(err) - } - tk := measurement.TestKeys.(*riseupvpn.TestKeys) - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] != "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } - - if _, found := tk.TransportStatus["obfs"]; found { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } } func TestIgnoreOverloadedGateways(t *testing.T) { @@ -754,12 +534,10 @@ func TestIgnoreOverloadedGateways(t *testing.T) { tk := measurement.TestKeys.(*riseupvpn.TestKeys) - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] == "blocked" || tk.TransportStatus["obfs"] == "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) - } - - if tk.FailingGateways != nil { - t.Fatal("unexpected amount of failing gateways. Overloaded gateways shouldn't be tested. " + fmt.Sprint(tk.FailingGateways)) + for _, tcpConnect := range tk.TCPConnect { + if !tcpConnect.Status.Success { + t.Fatal("unexpected failing tcpconnect. Overloaded gateways shouldn't be tested. " + fmt.Sprint(tcpConnect)) + } } } @@ -809,13 +587,20 @@ func TestIgnoreLocationsWithFewObfs4Bridges(t *testing.T) { tk := measurement.TestKeys.(*riseupvpn.TestKeys) - if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] == "blocked" || tk.TransportStatus["obfs"] == "blocked" { - t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) + for _, tcpConnect := range tk.TCPConnect { + if tcpConnect.IP == "234.345.234.345" { // Seattle + t.Fatal("Locations with few gateways should be ignored") + } } + /* + if tk.TransportStatus == nil || tk.TransportStatus["openvpn"] == "blocked" || tk.TransportStatus["obfs"] == "blocked" { + t.Fatal("invalid TransportStatus: " + fmt.Sprint(tk.TransportStatus)) + } - if tk.FailingGateways != nil { - t.Fatal("unexpected amount of failing gateways. Only locations under test should be evaluated." + fmt.Sprint(tk.FailingGateways)) - } + if tk.FailingGateways != nil { + t.Fatal("unexpected amount of failing gateways. Only locations under test should be evaluated." + fmt.Sprint(tk.FailingGateways)) + } + */ } func TestIgnoreGatewaysNotIncludedInGeoAPIResponse(t *testing.T) { @@ -859,10 +644,16 @@ func TestIgnoreGatewaysNotIncludedInGeoAPIResponse(t *testing.T) { tk := measurement.TestKeys.(*riseupvpn.TestKeys) - if tk.FailingGateways != nil { - t.Fatal("unexpected amount of failing gateways. " + fmt.Sprint(tk.FailingGateways)) + for _, tcpConnect := range tk.TCPConnect { + if tcpConnect.IP == "123.12.123.11" { // Seattle + t.Fatal("Locations with that are not part of the geoip response should be ignored") + } } - + /* + if tk.FailingGateways != nil { + t.Fatal("unexpected amount of failing gateways. " + fmt.Sprint(tk.FailingGateways)) + } + */ } func TestHandleInvalidGeoAPIResponse(t *testing.T) { @@ -902,10 +693,10 @@ func TestHandleInvalidGeoAPIResponse(t *testing.T) { tk := measurement.TestKeys.(*riseupvpn.TestKeys) - if tk.FailingGateways == nil || len(tk.FailingGateways) != 2 { - t.Fatal("unexpected amount of failing gateways. " + fmt.Sprint(tk.FailingGateways)) - } - + /* if tk.FailingGateways == nil || len(tk.FailingGateways) != 2 { + t.Fatal("unexpected amount of failing gateways. " + fmt.Sprint(tk.FailingGateways)) + } + */ foundFailure := false for _, failure := range tk.APIFailure { if failure == "invalid_geoservice_response" { @@ -919,102 +710,15 @@ func TestHandleInvalidGeoAPIResponse(t *testing.T) { } } -func TestSummaryKeysInvalidType(t *testing.T) { +func TestSummaryKeysAlwaysReturnIsAnomalyFalse(t *testing.T) { measurement := new(model.Measurement) m := &riseupvpn.Measurer{} - _, err := m.GetSummaryKeys(measurement) - if err.Error() != "invalid test keys type" { - t.Fatal("not the error we expected") - } -} - -func TestSummaryKeysWorksAsIntended(t *testing.T) { - tests := []struct { - tk riseupvpn.TestKeys - sk riseupvpn.SummaryKeys - }{{ - tk: riseupvpn.TestKeys{ - APIStatus: "blocked", - CACertStatus: true, - FailingGateways: nil, - TransportStatus: nil, - }, - sk: riseupvpn.SummaryKeys{ - APIBlocked: true, - ValidCACert: true, - IsAnomaly: true, - TransportStatus: nil, - FailingGateways: 0, - }, - }, { - tk: riseupvpn.TestKeys{ - APIStatus: "ok", - CACertStatus: false, - FailingGateways: nil, - TransportStatus: nil, - }, - sk: riseupvpn.SummaryKeys{ - ValidCACert: false, - IsAnomaly: true, - FailingGateways: 0, - TransportStatus: nil, - }, - }, { - tk: riseupvpn.TestKeys{ - APIStatus: "ok", - CACertStatus: true, - FailingGateways: []riseupvpn.GatewayConnection{{ - IP: "1.1.1.1", - Port: 443, - TransportType: "obfs4", - }}, - TransportStatus: map[string]string{ - "obfs4": "blocked", - "openvpn": "ok", - }, - }, - sk: riseupvpn.SummaryKeys{ - FailingGateways: 1, - IsAnomaly: true, - ValidCACert: true, - TransportStatus: map[string]string{ - "obfs4": "blocked", - "openvpn": "ok", - }, - }, - }, { - tk: riseupvpn.TestKeys{ - APIStatus: "ok", - CACertStatus: true, - FailingGateways: nil, - TransportStatus: map[string]string{ - "openvpn": "ok", - }, - }, - sk: riseupvpn.SummaryKeys{ - ValidCACert: true, - IsAnomaly: false, - FailingGateways: 0, - TransportStatus: map[string]string{ - "openvpn": "ok", - }, - }, - }, + result, err := m.GetSummaryKeys(measurement) + if err != nil { + t.Fatal("GetSummaryKeys should never return an error") } - for idx, tt := range tests { - t.Run(fmt.Sprintf("%d", idx), func(t *testing.T) { - m := &riseupvpn.Measurer{} - measurement := &model.Measurement{TestKeys: &tt.tk} - got, err := m.GetSummaryKeys(measurement) - if err != nil { - t.Fatal(err) - return - } - sk := got.(riseupvpn.SummaryKeys) - if diff := cmp.Diff(tt.sk, sk); diff != "" { - t.Fatal(diff) - } - }) + if result.(riseupvpn.SummaryKeys).IsAnomaly { + t.Fatal("GetSummaryKeys should never return IsAnomaly true") } } @@ -1077,6 +781,7 @@ func generateMockGetter(requestResponse map[string]string, responseStatus map[st Value: responseBody, }, BodyIsTruncated: false, + Code: responseStatus, }}, }, TCPConnect: []tracex.TCPConnectEntry{tcpConnect}, @@ -1125,9 +830,9 @@ func addObfs4Capability(gateway *riseupvpn.GatewayV3) { gateway.Capabilities.Transport = transports } -func addGateway(service *riseupvpn.EipService, host string, ipAddress string, location string) { +func addGateway(service *riseupvpn.EipServiceV3, host string, ipAddress string, location string) { gateway := riseupvpn.GatewayV3{ - Capabilities: riseupvpn.Capabilities{}, + Capabilities: riseupvpn.CapabilitiesV3{}, Host: host, IPAddress: ipAddress, Location: location,