From c3264d8d5df7634d14b2322ddac3809ff1aed390 Mon Sep 17 00:00:00 2001 From: cyBerta Date: Sat, 5 Aug 2023 04:18:56 +0200 Subject: [PATCH] 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,