Skip to content

Commit

Permalink
feat: add the openvpn experiment (#1585)
Browse files Browse the repository at this point in the history
# Description

First iteration of a new openvpn experiment. This takes a set of
endpoints and uses minivpn to attempt a handshake with each of the
configured endpoints.

A new API endpoint has been added to the backend that is able to
distribute valid configuration parameters for an OpenVPN connection,
including credentials.

# Checklist

- [x] I have read the [contribution
guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request:
ooni/probe#2688
- [x] if you changed anything related to how experiments work and you
need to reflect these changes in the ooni/spec repository, please link
to the related ooni/spec pull request:
ooni/spec#293
- [x] if you changed code inside an experiment, make sure you bump its
version number

---------

Co-authored-by: Simone Basso <[email protected]>
  • Loading branch information
ainghazal and bassosimone authored Jun 6, 2024
1 parent 15dac36 commit 1e4f104
Show file tree
Hide file tree
Showing 22 changed files with 2,005 additions and 2 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ require (
github.com/miekg/dns v1.1.59
github.com/mitchellh/go-wordwrap v1.0.1
github.com/montanaflynn/stats v0.7.1
github.com/ooni/minivpn v0.0.6
github.com/ooni/netem v0.0.0-20240208095707-608dcbcd82b8
github.com/ooni/oocrypto v0.6.1
github.com/ooni/oohttp v0.7.2
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/gopacket v1.1.19 h1:ves8RnFZPGiFnTS0uPQStjwru6uO6h+nlr9j6fL7kF8=
github.com/google/gopacket v1.1.19/go.mod h1:iJ8V8n6KS+z2U1A8pUwu8bW5SyEMkXJB8Yo/Vo+TKTo=
github.com/google/martian v2.1.0+incompatible h1:/CP5g8u/VJHijgedC/Legn3BAbAaWPgecwXBIDzw5no=
github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
github.com/google/pprof v0.0.0-20230207041349-798e818bf904/go.mod h1:uglQLonpP8qtYCYyzA+8c/9qtqgA3qsXGYqCPKARAFg=
github.com/google/pprof v0.0.0-20240509144519-723abb6459b7 h1:velgFPYr1X9TDwLIfkV7fWqsFlf7TeP11M/7kPd/dVI=
github.com/google/pprof v0.0.0-20240509144519-723abb6459b7/go.mod h1:kf6iHlnVGwgKolg33glAes7Yg/8iWP8ukqeldJSO7jw=
Expand Down Expand Up @@ -350,6 +352,8 @@ github.com/onsi/ginkgo/v2 v2.17.3/go.mod h1:nP2DPOQoNsQmsVyv5rDA8JkXQoCs6goXIvr/
github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
github.com/onsi/gomega v1.33.0 h1:snPCflnZrpMsy94p4lXVEkHo12lmPnc3vY5XBbreexE=
github.com/onsi/gomega v1.33.0/go.mod h1:+925n5YtiFsLzzafLUHzVMBpvvRAzrydIBiSIxjX3wY=
github.com/ooni/minivpn v0.0.6 h1:pGTsYRtofEupMrJL28f1IXO1LJslSI7dEHxSadNgGik=
github.com/ooni/minivpn v0.0.6/go.mod h1:0KNwmK2Wg9lDbk936XjtxvCq4tPNbK4C3IJvyLwIMrE=
github.com/ooni/netem v0.0.0-20240208095707-608dcbcd82b8 h1:kJ2wn19lIP/y9ng85BbFRdWKHK6Er116Bbt5uhqHVD4=
github.com/ooni/netem v0.0.0-20240208095707-608dcbcd82b8/go.mod h1:b/wAvTR5n92Vk2b0SBmuMU0xO4ZGVrsXtU7zjTby7vw=
github.com/ooni/oocrypto v0.6.1 h1:D0fGokmHoVKGBy39RxPxK77ov0Ob9Z5pdx4vKA6vpWk=
Expand Down
60 changes: 60 additions & 0 deletions internal/engine/inputloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/url"

"github.com/apex/log"
"github.com/ooni/probe-cli/v3/internal/experiment/openvpn"
"github.com/ooni/probe-cli/v3/internal/fsx"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/registry"
Expand All @@ -28,6 +29,8 @@ var (
// introduce this abstraction because it helps us with testing.
type InputLoaderSession interface {
CheckIn(ctx context.Context, config *model.OOAPICheckInConfig) (*model.OOAPICheckInResult, error)
FetchOpenVPNConfig(ctx context.Context,
provider, cc string) (*model.OOAPIVPNProviderConfig, error)
}

// InputLoaderLogger is the logger according to an InputLoader.
Expand Down Expand Up @@ -299,6 +302,21 @@ func (il *InputLoader) readfile(filepath string, open inputLoaderOpenFn) ([]mode

// loadRemote loads inputs from a remote source.
func (il *InputLoader) loadRemote(ctx context.Context) ([]model.OOAPIURLInfo, error) {
switch registry.CanonicalizeExperimentName(il.ExperimentName) {
case "openvpn":
// TODO(ainghazal): given the semantics of the current API call, in an ideal world we'd need to pass
// the desired provider here, if known. We only have one provider for now, so I'm acting like YAGNI. Another
// option is perhaps to coalesce all the known providers per proto into a single API call and let client
// pick whatever they want.
// This will likely improve after Richer Input is available.
return il.loadRemoteOpenVPN(ctx)
default:
return il.loadRemoteWebConnectivity(ctx)
}
}

// loadRemoteWebConnectivity loads webconnectivity inputs from a remote source.
func (il *InputLoader) loadRemoteWebConnectivity(ctx context.Context) ([]model.OOAPIURLInfo, error) {
config := il.CheckInConfig
if config == nil {
// Note: Session.CheckIn documentation says it will fill in
Expand All @@ -317,6 +335,39 @@ func (il *InputLoader) loadRemote(ctx context.Context) ([]model.OOAPIURLInfo, er
return reply.WebConnectivity.URLs, nil
}

// loadRemoteOpenVPN loads openvpn inputs from a remote source.
func (il *InputLoader) loadRemoteOpenVPN(ctx context.Context) ([]model.OOAPIURLInfo, error) {
// VPN Inputs do not match exactly the semantics expected from [model.OOAPIURLInfo],
// since OOAPIURLInfo is oriented towards webconnectivity,
// but we force VPN targets in the URL and ignore all the other fields.
// There's still some level of impedance mismatch here, since it's also possible that
// the user of the library wants to use remotes by unknown providers passed via cli options,
// oonirun etc; in that case we'll need to extract the provider annotation from the URLs.
urls := make([]model.OOAPIURLInfo, 0)

// The openvpn experiment contains an array of the providers that the API knows about.
// We try to get all the remotes from the API for the list of enabled providers.
for _, provider := range openvpn.APIEnabledProviders {
// fetchOpenVPNConfig ultimately uses an internal cache in the session to avoid
// hitting the API too many times.
reply, err := il.fetchOpenVPNConfig(ctx, provider)
if err != nil {
return urls, err
}
for _, input := range reply.Inputs {
urls = append(urls, model.OOAPIURLInfo{URL: input})
}
}

if len(urls) <= 0 {
// At some point we might want to return [openvpn.DefaultEndpoints],
// but for now we're assuming that no targets means we've disabled
// the experiment on the backend.
return nil, ErrNoURLsReturned
}
return urls, nil
}

// checkIn executes the check-in and filters the returned URLs to exclude
// the URLs that are not part of the requested categories. This is done for
// robustness, just in case we or the API do something wrong.
Expand All @@ -335,6 +386,15 @@ func (il *InputLoader) checkIn(
return &reply.Tests, nil
}

// fetchOpenVPNConfig fetches vpn information for the configured providers
func (il *InputLoader) fetchOpenVPNConfig(ctx context.Context, provider string) (*model.OOAPIVPNProviderConfig, error) {
reply, err := il.Session.FetchOpenVPNConfig(ctx, provider, "XX")
if err != nil {
return nil, err
}
return reply, nil
}

// preventMistakes makes the code more robust with respect to any possible
// integration issue where the backend returns to us URLs that don't
// belong to the category codes we requested.
Expand Down
78 changes: 77 additions & 1 deletion internal/engine/inputloader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import (
"strings"
"syscall"
"testing"
"time"

"github.com/apex/log"
"github.com/google/go-cmp/cmp"
"github.com/ooni/probe-cli/v3/internal/kvstore"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/runtimex"
)

func TestInputLoaderInputNoneWithStaticInputs(t *testing.T) {
Expand Down Expand Up @@ -448,6 +450,10 @@ type InputLoaderMockableSession struct {
// be nil when Error is not-nil.
Output *model.OOAPICheckInResult

// FetchOpenVPNConfigOutput contains the output of FetchOpenVPNConfig.
// It should be nil when Error is non-nil.
FetchOpenVPNConfigOutput *model.OOAPIVPNProviderConfig

// Error is the error to be returned by CheckIn. It
// should be nil when Output is not-nil.
Error error
Expand All @@ -462,6 +468,13 @@ func (sess *InputLoaderMockableSession) CheckIn(
return sess.Output, sess.Error
}

// FetchOpenVPNConfig implements InputLoaderSession.FetchOpenVPNConfig.
func (sess *InputLoaderMockableSession) FetchOpenVPNConfig(
ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) {
runtimex.Assert(!(sess.Error == nil && sess.FetchOpenVPNConfigOutput == nil), "both FetchOpenVPNConfig and Error are nil")
return sess.FetchOpenVPNConfigOutput, sess.Error
}

func TestInputLoaderCheckInFailure(t *testing.T) {
il := &InputLoader{
Session: &InputLoaderMockableSession{
Expand Down Expand Up @@ -543,6 +556,69 @@ func TestInputLoaderCheckInSuccessWithSomeURLs(t *testing.T) {
}
}

func TestInputLoaderOpenVPNSuccessWithNoInput(t *testing.T) {
il := &InputLoader{
ExperimentName: "openvpn",
InputPolicy: model.InputOrQueryBackend,
Session: &InputLoaderMockableSession{
Error: nil,
FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{
Provider: "riseup",
Inputs: []string{
"openvpn://foo.corp/?address=1.1.1.1:1194&transport=tcp",
},
DateUpdated: time.Now(),
},
},
}
_, err := il.loadRemote(context.Background())
if err != nil {
t.Fatal("we did not expect an error")
}
}

func TestInputLoaderOpenVPNSuccessWithNoInputAndAPICall(t *testing.T) {
il := &InputLoader{
ExperimentName: "openvpn",
InputPolicy: model.InputOrQueryBackend,
Session: &InputLoaderMockableSession{
Error: nil,
FetchOpenVPNConfigOutput: &model.OOAPIVPNProviderConfig{
Provider: "riseupvpn",
Inputs: []string{
"openvpn://foo.corp/?address=1.2.3.4:1194&transport=tcp",
},
DateUpdated: time.Now(),
},
},
}
out, err := il.loadRemote(context.Background())
if err != nil {
t.Fatal("we did not expect an error")
}
if len(out) != 1 {
t.Fatal("we expected output of len=1")
}
}

func TestInputLoaderOpenVPNWithAPIFailureAndFallback(t *testing.T) {
expected := errors.New("mocked API error")
il := &InputLoader{
ExperimentName: "openvpn",
InputPolicy: model.InputOrQueryBackend,
Session: &InputLoaderMockableSession{
Error: expected,
},
}
out, err := il.loadRemote(context.Background())
if err != expected {
t.Fatal("we expected an error")
}
if len(out) != 0 {
t.Fatal("we expected no fallback URLs")
}
}

func TestPreventMistakesWithCategories(t *testing.T) {
input := []model.OOAPIURLInfo{{
CategoryCode: "NEWS",
Expand Down Expand Up @@ -679,7 +755,7 @@ func TestStringListToModelURLInfoWithError(t *testing.T) {
expected := errors.New("mocked error")
output, err := stringListToModelURLInfo(input, expected)
if !errors.Is(err, expected) {
t.Fatal("no the error we expected", err)
t.Fatal("not the error we expected", err)
}
if output != nil {
t.Fatal("unexpected nil output")
Expand Down
30 changes: 30 additions & 0 deletions internal/engine/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ type Session struct {
softwareName string
softwareVersion string
tempDir string
vpnConfig map[string]model.OOAPIVPNProviderConfig

// closeOnce allows us to call Close just once.
closeOnce sync.Once
Expand Down Expand Up @@ -177,6 +178,7 @@ func NewSession(ctx context.Context, config SessionConfig) (*Session, error) {
torArgs: config.TorArgs,
torBinary: config.TorBinary,
tunnelDir: config.TunnelDir,
vpnConfig: make(map[string]model.OOAPIVPNProviderConfig),
}
proxyURL := config.ProxyURL
if proxyURL != nil {
Expand Down Expand Up @@ -368,9 +370,37 @@ func (s *Session) FetchTorTargets(
if err != nil {
return nil, err
}

// TODO(bassosimone,DecFox): here we could also lock the mutex
// or we should consider using the same strategy we used for the
// experiments, where we separated mutable state into dedicated types.
return clnt.FetchTorTargets(ctx, cc)
}

// FetchOpenVPNConfig fetches openvpn config from the API if it's not found in the
// internal cache. We do this to avoid hitting the API for every input.
func (s *Session) FetchOpenVPNConfig(
ctx context.Context, provider, cc string) (*model.OOAPIVPNProviderConfig, error) {
if config, ok := s.vpnConfig[provider]; ok {
return &config, nil
}
clnt, err := s.newOrchestraClient(ctx)
if err != nil {
return nil, err
}

// we cannot lock earlier because newOrchestraClient locks the mutex.
defer s.mu.Unlock()
s.mu.Lock()

config, err := clnt.FetchOpenVPNConfig(ctx, provider, cc)
if err != nil {
return nil, err
}
s.vpnConfig[provider] = config
return &config, nil
}

// KeyValueStore returns the configured key-value store.
func (s *Session) KeyValueStore() model.KeyValueStore {
return s.kvStore
Expand Down
13 changes: 13 additions & 0 deletions internal/engine/session_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,19 @@ func TestSessionMaybeLookupLocationContextLookupLocationContextFailure(t *testin
}
}

func TestSessionFetchOpenVPNConfigWithCancelledContext(t *testing.T) {
sess := &Session{}
ctx, cancel := context.WithCancel(context.Background())
cancel() // cause failure
resp, err := sess.FetchOpenVPNConfig(ctx, "riseup", "XX")
if !errors.Is(err, context.Canceled) {
t.Fatal("not the error we expected", err)
}
if resp != nil {
t.Fatal("expected nil response here")
}
}

func TestSessionFetchTorTargetsWithCancelledContext(t *testing.T) {
sess := &Session{}
ctx, cancel := context.WithCancel(context.Background())
Expand Down
Loading

0 comments on commit 1e4f104

Please sign in to comment.