Skip to content

Commit

Permalink
address requested changes after merge review:
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@cyBerta
cyBerta committed Aug 5, 2023
1 parent 2e868a0 commit c3264d8
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 457 deletions.
129 changes: 28 additions & 101 deletions internal/experiment/riseupvpn/riseupvpn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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"`
Expand All @@ -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"`
Expand All @@ -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,
}
}

Expand All @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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
}

Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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{}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Loading

0 comments on commit c3264d8

Please sign in to comment.