Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add the openvpn experiment #1585

Merged
merged 56 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from 55 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
5c0a415
stub for openvpn experiment
ainghazal Mar 6, 2024
06624b0
add hardcoded endpoint
ainghazal Mar 6, 2024
aaf6995
update archival format
ainghazal Mar 6, 2024
d4b65c3
wip
ainghazal Mar 6, 2024
c4bb706
hardcode credentials as step zero
ainghazal Mar 6, 2024
87bbe21
flatten test keys
ainghazal Mar 7, 2024
8c2e23e
wip
ainghazal Mar 7, 2024
54c9510
wip
ainghazal Mar 7, 2024
1bc0764
include openvpn options in handshake result
ainghazal Mar 7, 2024
567ed14
add fields to archival struct for handshake result
ainghazal Mar 7, 2024
5974d03
wip: input
ainghazal Mar 15, 2024
0852d1a
api call to fetch credentials
ainghazal Mar 19, 2024
8bb671b
use the newer endpoint, provider in url
ainghazal Mar 26, 2024
1ea507f
wip: fetch config, process input as part of inputloader
ainghazal Mar 27, 2024
46b40f1
parse input in new format
ainghazal Mar 27, 2024
0c6a062
cache config from api
ainghazal Mar 27, 2024
199f105
cache per-provider config
ainghazal Mar 27, 2024
6ada20f
do not parse base64 in creds from api
ainghazal Mar 27, 2024
accaf97
tests
ainghazal Mar 27, 2024
bb25311
update docs
ainghazal Apr 3, 2024
dfd8c83
move archival structs to internal/model
ainghazal Apr 3, 2024
d6d7ed8
move list of enabled providers to expriment/openvpn
ainghazal Apr 3, 2024
e291c0e
do not use credentials
ainghazal Apr 4, 2024
8934898
remove test file
ainghazal Apr 4, 2024
25c98a9
go mod tidy
ainghazal Apr 26, 2024
4ef2b80
add expectations for openvpn
ainghazal Apr 30, 2024
779ab03
add vpn suffix, there seems to be a mismatch in the provider name
ainghazal Apr 8, 2024
47aa87d
add tests for inputloader
ainghazal Apr 11, 2024
4a68032
update after httpclientx api change
ainghazal May 2, 2024
c11fbce
fix after rebase
ainghazal May 6, 2024
f169fb6
test stub
ainghazal May 6, 2024
337230c
adapt after merge/api change
ainghazal May 6, 2024
0a24b30
add tests for probeservices/openvpn
ainghazal May 6, 2024
2f64b08
minor style changes for clarity and correctness
ainghazal May 30, 2024
4bc13c8
debug possible deadlock
ainghazal May 30, 2024
8fcfaac
prefix annoyance
ainghazal May 30, 2024
9d7a6be
debug
ainghazal May 30, 2024
32f3f00
complete hacks to bypass vpn provider name in backend
ainghazal May 30, 2024
a55816c
fix tests for bad parsing
ainghazal Jun 3, 2024
6ab511e
fix bad comparison
ainghazal Jun 3, 2024
07daac4
revert removal of lock
ainghazal Jun 3, 2024
410126a
pass output to test to avoid panic
ainghazal Jun 3, 2024
2a6fc25
force single provider for now
ainghazal Jun 3, 2024
ec8396a
add changes after code review
ainghazal Jun 4, 2024
b909756
move the lock after instantiating the client
ainghazal Jun 4, 2024
b8fbd82
unflatten the failure
ainghazal Jun 4, 2024
8c5fa11
abort experiment if no urls returned
ainghazal Jun 4, 2024
330f1aa
Merge remote-tracking branch 'ooni/master' into feat/openvpn-2024-3
ainghazal Jun 4, 2024
1ce6835
Merge branch 'master' into feat/openvpn-2024-3
bassosimone Jun 5, 2024
6377ba8
add stub to receive options from oonirun descriptors
ainghazal Jun 5, 2024
9b27cc1
use measurexlite.NewFailure
ainghazal Jun 5, 2024
196d584
update tests
ainghazal Jun 5, 2024
1a60013
update test after change in default endpoints if api fails
ainghazal Jun 5, 2024
0fae492
Merge branch 'master' into feat/openvpn-2024-3
bassosimone Jun 5, 2024
9fac70c
fix(registry): update openvpn registration
bassosimone Jun 6, 2024
5afb8bd
Update internal/engine/session.go
bassosimone Jun 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
28 changes: 28 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,35 @@ func (s *Session) FetchTorTargets(
if err != nil {
return nil, err
}

// here we could also lock the mutex.
bassosimone marked this conversation as resolved.
Show resolved Hide resolved
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
Loading