Skip to content

Commit

Permalink
x
Browse files Browse the repository at this point in the history
  • Loading branch information
@bassosimone
bassosimone committed Oct 11, 2023
1 parent f7114c0 commit 8230542
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 36 deletions.
17 changes: 6 additions & 11 deletions internal/experiment/riseupvpn/riseupvpn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ package riseupvpn
import (
"context"
"encoding/json"
"errors"
"fmt"
"time"

"github.com/ooni/probe-cli/v3/internal/experiment/urlgetter"
Expand Down Expand Up @@ -126,9 +124,6 @@ func (m Measurer) ExperimentVersion() string {
return testVersion
}

// ErrBootstrap indicates that the riseupvpn experiment failed to bootstrap correctly.
var ErrBootstrap = errors.New("riseupvn: bootstrap failure")

// Run implements ExperimentMeasurer.Run.
func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
callbacks := args.Callbacks
Expand Down Expand Up @@ -160,15 +155,15 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
}},
}

// Q: why fail the experiment if we cannot fetch the CA or the config? Cannot we just
// Q: why returning early if we cannot fetch the CA or the config? Cannot we just
// disable certificate verification and fetch the config?
//
// A: I do not feel comfortable with fetching without verying the certificates since
// this means the experiment could be person-in-the-middled and forced to perform TCP
// connect to arbitrary hosts, which maybe is harmless but still a bummer.
//
// TODO(https://github.com/ooni/probe/issues/2559): solve this problem by serving
// the correct CA and the testing targets to probes using check-in v2 (aka richer input).
// TODO(https://github.com/ooni/probe/issues/2559): solve this problem by serving the
// correct CA and the endpoints to probes using check-in v2 (aka richer input).

nullCallbacks := model.NewPrinterCallbacks(model.DiscardLogger)
for entry := range multi.CollectOverall(ctx, inputs, 0, 20, "riseupvpn", nullCallbacks) {
Expand All @@ -177,12 +172,12 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
if tk.Failure != nil {
testkeys.CACertStatus = false
testkeys.APIFailures = append(testkeys.APIFailures, *tk.Failure)
return fmt.Errorf("%w: %s", ErrBootstrap, *tk.Failure)
return nil
}
if ok := certPool.AppendCertsFromPEM([]byte(tk.HTTPResponseBody)); !ok {
testkeys.CACertStatus = false
testkeys.APIFailures = append(testkeys.APIFailures, "invalid_ca")
return fmt.Errorf("%w: invalid_ca", ErrBootstrap)
return nil
}
}

Expand Down Expand Up @@ -210,7 +205,7 @@ func (m Measurer) Run(ctx context.Context, args *model.ExperimentArgs) error {
testkeys.UpdateProviderAPITestKeys(entry)
tk := entry.TestKeys
if tk.Failure != nil {
return fmt.Errorf("%w: %s", ErrBootstrap, *tk.Failure)
return nil
}
}

Expand Down
105 changes: 80 additions & 25 deletions internal/experiment/riseupvpn/riseupvpn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func TestNewExperimentMeasurer(t *testing.T) {

func TestGood(t *testing.T) {
// the gateaway openvpnurl2 is filtered out, since it doesn't support additionally obfs4
measurement, err := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
cacerturl: true,
eipserviceurl: true,
providerurl: true,
Expand All @@ -222,9 +222,6 @@ func TestGood(t *testing.T) {
obfs4url1: true,
obfs4url2: false,
}))
if err != nil {
t.Fatal(err)
}

tk := measurement.TestKeys.(*riseupvpn.TestKeys)
if tk.Agent != "" {
Expand Down Expand Up @@ -343,13 +340,17 @@ func TestInvalidCaCert(t *testing.T) {
Session: sess,
}
err := measurer.Run(ctx, args)
if !errors.Is(err, riseupvpn.ErrBootstrap) {
t.Fatal("unexpected error", err)
if err != nil {
t.Fatal(err)
}
tk := measurement.TestKeys.(*riseupvpn.TestKeys)
if tk.CACertStatus == true {
t.Fatal("unexpected CaCertStatus")
}
}

func TestFailureCaCertFetch(t *testing.T) {
_, err := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
cacerturl: false,
eipserviceurl: true,
providerurl: true,
Expand All @@ -358,13 +359,27 @@ func TestFailureCaCertFetch(t *testing.T) {
openvpnurl2: true,
obfs4url1: true,
}))
if !errors.Is(err, riseupvpn.ErrBootstrap) {
t.Fatal("unexpected error", err)

tk := measurement.TestKeys.(*riseupvpn.TestKeys)
if tk.CACertStatus != false {
t.Fatal("invalid CACertStatus ")
}

if len(tk.APIFailures) != 1 || tk.APIFailures[0] != io.EOF.Error() {
t.Fatal("APIFailures should not be empty", tk.APIFailures)
}
if len(tk.Requests) != 1 {
t.Fatal("Expected a single request in this case")
}
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")
}
}
}

func TestFailureEipServiceBlocked(t *testing.T) {
_, err := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
cacerturl: true,
eipserviceurl: false,
providerurl: true,
Expand All @@ -373,13 +388,26 @@ func TestFailureEipServiceBlocked(t *testing.T) {
openvpnurl2: true,
obfs4url1: true,
}))
if !errors.Is(err, riseupvpn.ErrBootstrap) {
t.Fatal("unexpected error", err)
tk := measurement.TestKeys.(*riseupvpn.TestKeys)
if tk.CACertStatus != true {
t.Fatal("invalid CACertStatus")
}

for _, entry := range tk.Requests {
if entry.Request.URL == "https://api.black.riseup.net:443/3/config/eip-service.json" {
if entry.Failure == nil {
t.Fatal("Failure for " + entry.Request.URL + " should not be null")
}
}
}

if len(tk.APIFailures) <= 0 {
t.Fatal("APIFailures should not be empty")
}
}

func TestFailureProviderUrlBlocked(t *testing.T) {
_, err := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
cacerturl: true,
eipserviceurl: true,
providerurl: false,
Expand All @@ -388,13 +416,27 @@ func TestFailureProviderUrlBlocked(t *testing.T) {
openvpnurl2: true,
obfs4url1: true,
}))
if !errors.Is(err, riseupvpn.ErrBootstrap) {
t.Fatal("unexpected error", err)
tk := measurement.TestKeys.(*riseupvpn.TestKeys)

for _, entry := range tk.Requests {
if entry.Request.URL == "https://riseup.net/provider.json" {
if entry.Failure == nil {
t.Fatal("Failure for " + entry.Request.URL + " should not be null")
}
}
}

if tk.CACertStatus != true {
t.Fatal("invalid CACertStatus ")
}

if len(tk.APIFailures) <= 0 {
t.Fatal("APIFailures should not be empty")
}
}

func TestFailureGeoIpServiceBlocked(t *testing.T) {
_, err := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
cacerturl: true,
eipserviceurl: true,
providerurl: true,
Expand All @@ -403,13 +445,26 @@ func TestFailureGeoIpServiceBlocked(t *testing.T) {
openvpnurl2: true,
obfs4url1: true,
}))
if !errors.Is(err, riseupvpn.ErrBootstrap) {
t.Fatal("unexpected error", err)
tk := measurement.TestKeys.(*riseupvpn.TestKeys)
if tk.CACertStatus != true {
t.Fatal("invalid CACertStatus ")
}

for _, entry := range tk.Requests {
if entry.Request.URL == "https://api.black.riseup.net:9001/json" {
if entry.Failure == nil {
t.Fatal("Failure for " + entry.Request.URL + " should not be null")
}
}
}

if len(tk.APIFailures) <= 0 {
t.Fatal("APIFailures should not be empty")
}
}

func TestFailureGateway1TransportNOK(t *testing.T) {
measurement, err := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
measurement := runDefaultMockTest(t, generateDefaultMockGetter(map[string]bool{
cacerturl: true,
eipserviceurl: true,
providerurl: true,
Expand All @@ -419,10 +474,6 @@ func TestFailureGateway1TransportNOK(t *testing.T) {
obfs4url1: true,
obfs4url2: false,
}))
if err != nil {
t.Fatal(err)
}

tk := measurement.TestKeys.(*riseupvpn.TestKeys)
if tk.CACertStatus != true {
t.Fatal("invalid CACertStatus ")
Expand Down Expand Up @@ -528,7 +579,7 @@ func generateDefaultMockGetter(responseStatuses map[string]bool) urlgetter.Multi
return generateMockGetter(RequestResponse, responseStatuses)
}

func runDefaultMockTest(t *testing.T, multiGetter urlgetter.MultiGetter) (*model.Measurement, error) {
func runDefaultMockTest(t *testing.T, multiGetter urlgetter.MultiGetter) *model.Measurement {
measurer := riseupvpn.Measurer{
Config: riseupvpn.Config{},
Getter: multiGetter,
Expand All @@ -541,5 +592,9 @@ func runDefaultMockTest(t *testing.T, multiGetter urlgetter.MultiGetter) (*model
Session: &mocks.Session{MockLogger: func() model.Logger { return log.Log }},
}
err := measurer.Run(context.Background(), args)
return measurement, err

if err != nil {
t.Fatal(err)
}
return measurement
}

0 comments on commit 8230542

Please sign in to comment.