c/ooni-probe-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(geolocate): always use netxlite functionality (#976)

Browse Source 
This change ensures that, in turn, we're able to "remote" all the traffic generated by the `geolocate` package, rather than missing some bits of it that were still using the standard library and caused _some_ geolocations to geolocate as the local host rather than as the remote host.

Extracted from https://github.com/ooni/probe-cli/pull/969, where we tested this functionality.

Closes https://github.com/ooni/probe/issues/1383 (which was long overdue).

Part of https://github.com/ooni/probe/issues/2340, because it allows us to make progress with that.
This commit is contained in:
Simone Basso 2022-10-12 18:07:42 +02:00 committed by GitHub
parent 86ffd6a0c4
commit 57a3919d2a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 93 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
internal/engine/geolocate/cloudflare.go
View File

@ -15,6 +15,7 @@ func cloudflareIPLookup(
httpClient *http.Client,
logger model.Logger,
userAgent string,
resolver model.Resolver,
) (string, error) {
data, err := (&httpx.APIClientTemplate{
BaseURL: "https://www.cloudflare.com",

2
internal/engine/geolocate/cloudflare_test.go
View File

@ -8,6 +8,7 @@ import (
"github.com/apex/log"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
)
func TestIPLookupWorksUsingcloudlflare(t *testing.T) {
@ -16,6 +17,7 @@ func TestIPLookupWorksUsingcloudlflare(t *testing.T) {
http.DefaultClient,
log.Log,
model.HTTPHeaderUserAgent,
netxlite.NewStdlibResolver(model.DiscardLogger),
)
if err != nil {
t.Fatal(err)

4
internal/engine/geolocate/geolocate.go
View File

@ -90,7 +90,9 @@ func NewTask(config Config) *Task {
probeIPLookupper: ipLookupClient(config),
probeASNLookupper: mmdbLookupper{},
resolverASNLookupper: mmdbLookupper{},
resolverIPLookupper: resolverLookupClient{},
resolverIPLookupper: resolverLookupClient{
Resolver: config.Resolver,
},
}
}

1
internal/engine/geolocate/invalid_test.go
View File

@ -12,6 +12,7 @@ func invalidIPLookup(
httpClient *http.Client,
logger model.Logger,
userAgent string,
resolver model.Resolver,
) (string, error) {
return "invalid IP", nil
}

3
internal/engine/geolocate/iplookup.go
View File

@ -27,6 +27,7 @@ var (
type lookupFunc func(
ctx context.Context, client *http.Client,
logger model.Logger, userAgent string,
resolver model.Resolver,
) (string, error)
type method struct {
@ -89,7 +90,7 @@ func (c ipLookupClient) doWithCustomFunc(
txp := netxlite.NewHTTPTransportWithResolver(c.Logger, c.Resolver)
clnt := &http.Client{Transport: txp}
defer clnt.CloseIdleConnections()
ip, err := fn(ctx, clnt, c.Logger, c.UserAgent)
ip, err := fn(ctx, clnt, c.Logger, c.UserAgent, c.Resolver)
if err != nil {
return model.DefaultProbeIP, err
}

9
internal/engine/geolocate/resolverlookup.go
View File

@ -3,7 +3,8 @@ package geolocate
import (
"context"
"errors"
"net"
"github.com/ooni/probe-cli/v3/internal/model"
)
var (
@ -16,7 +17,9 @@ type dnsResolver interface {
LookupHost(ctx context.Context, host string) (addrs []string, err error)
}
type resolverLookupClient struct{}
type resolverLookupClient struct {
Resolver model.Resolver
}
func (rlc resolverLookupClient) do(ctx context.Context, r dnsResolver) (string, error) {
var ips []string
@ -31,5 +34,5 @@ func (rlc resolverLookupClient) do(ctx context.Context, r dnsResolver) (string,
}
func (rlc resolverLookupClient) LookupResolverIP(ctx context.Context) (ip string, err error) {
return rlc.do(ctx, &net.Resolver{})
return rlc.do(ctx, rlc.Resolver)
}

16
internal/engine/geolocate/resolverlookup_test.go
View File

@ -4,10 +4,16 @@ import (
"context"
"errors"
"testing"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
)
func TestLookupResolverIP(t *testing.T) {
addr, err := (resolverLookupClient{}).LookupResolverIP(context.Background())
rlc := resolverLookupClient{
Resolver: netxlite.NewStdlibResolver(model.DiscardLogger),
}
addr, err := rlc.LookupResolverIP(context.Background())
if err != nil {
t.Fatal(err)
}
@ -26,7 +32,9 @@ func (bhl brokenHostLookupper) LookupHost(ctx context.Context, host string) ([]s
func TestLookupResolverIPFailure(t *testing.T) {
expected := errors.New("mocked error")
rlc := resolverLookupClient{}
rlc := resolverLookupClient{
Resolver: netxlite.NewStdlibResolver(model.DiscardLogger),
}
addr, err := rlc.do(context.Background(), brokenHostLookupper{
err: expected,
})
@ -39,7 +47,9 @@ func TestLookupResolverIPFailure(t *testing.T) {
}
func TestLookupResolverIPNoAddressReturned(t *testing.T) {
rlc := resolverLookupClient{}
rlc := resolverLookupClient{
Resolver: netxlite.NewStdlibResolver(model.DiscardLogger),
}
addr, err := rlc.do(context.Background(), brokenHostLookupper{})
if !errors.Is(err, ErrNoIPAddressReturned) {
t.Fatalf("not the error we expected: %+v", err)

40
internal/engine/geolocate/stun.go
View File

@ -2,43 +2,51 @@ package geolocate
import (
"context"
"net"
"net/http"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/pion/stun"
)
// TODO(bassosimone): we should modify the stun code to use
// the session resolver rather than using its own.
//
// See https://github.com/ooni/probe/issues/1383.
type stunClient interface {
Close() error
Start(m *stun.Message, h stun.Handler) error
}
type stunConfig struct {
Dial func(network string, address string) (stunClient, error)
Endpoint string
Logger model.Logger
Dialer model.Dialer // optional
Endpoint string
Logger model.Logger
NewClient func(conn net.Conn) (stunClient, error) // optional
Resolver model.Resolver
}
func stunDialer(network string, address string) (stunClient, error) {
return stun.Dial(network, address)
func stunNewClient(conn net.Conn) (stunClient, error) {
return stun.NewClient(conn)
}
func stunIPLookup(ctx context.Context, config stunConfig) (string, error) {
config.Logger.Debugf("STUNIPLookup: start using %s", config.Endpoint)
ip, err := func() (string, error) {
dial := config.Dial
if dial == nil {
dial = stunDialer
dialer := config.Dialer
if dialer == nil {
dialer = netxlite.NewDialerWithResolver(config.Logger, config.Resolver)
}
clnt, err := dial("udp", config.Endpoint)
conn, err := dialer.DialContext(ctx, "udp", config.Endpoint)
if err != nil {
return model.DefaultProbeIP, err
}
newClient := config.NewClient
if newClient == nil {
newClient = stunNewClient
}
clnt, err := newClient(conn)
if err != nil {
conn.Close()
return model.DefaultProbeIP, err
}
defer clnt.Close()
message := stun.MustBuild(stun.TransactionID, stun.BindingRequest)
errch, ipch := make(chan error, 1), make(chan string, 1)
@ -78,10 +86,12 @@ func stunEkigaIPLookup(
httpClient *http.Client,
logger model.Logger,
userAgent string,
resolver model.Resolver,
) (string, error) {
return stunIPLookup(ctx, stunConfig{
Endpoint: "stun.ekiga.net:3478",
Logger: logger,
Resolver: resolver,
})
}
@ -90,9 +100,11 @@ func stunGoogleIPLookup(
httpClient *http.Client,
logger model.Logger,
userAgent string,
resolver model.Resolver,
) (string, error) {
return stunIPLookup(ctx, stunConfig{
Endpoint: "stun.l.google.com:19302",
Logger: logger,
Resolver: resolver,
})
}

45
internal/engine/geolocate/stun_test.go
View File

@ -10,6 +10,8 @@ import (
"github.com/apex/log"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/model/mocks"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/pion/stun"
)
@ -19,6 +21,7 @@ func TestSTUNIPLookupCanceledContext(t *testing.T) {
ip, err := stunIPLookup(ctx, stunConfig{
Endpoint: "stun.ekiga.net:3478",
Logger: log.Log,
Resolver: netxlite.NewStdlibResolver(model.DiscardLogger),
})
if !errors.Is(err, context.Canceled) {
t.Fatalf("not the error we expected: %+v", err)
@ -32,8 +35,10 @@ func TestSTUNIPLookupDialFailure(t *testing.T) {
expected := errors.New("mocked error")
ctx := context.Background()
ip, err := stunIPLookup(ctx, stunConfig{
Dial: func(network, address string) (stunClient, error) {
return nil, expected
Dialer: &mocks.Dialer{
MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
return nil, expected
},
},
Endpoint: "stun.ekiga.net:3478",
Logger: log.Log,
@ -70,11 +75,17 @@ func TestSTUNIPLookupStartReturnsError(t *testing.T) {
expected := errors.New("mocked error")
ctx := context.Background()
ip, err := stunIPLookup(ctx, stunConfig{
Dial: func(network, address string) (stunClient, error) {
return MockableSTUNClient{StartErr: expected}, nil
Dialer: &mocks.Dialer{
MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
conn := &mocks.Conn{}
return conn, nil
},
},
Endpoint: "stun.ekiga.net:3478",
Logger: log.Log,
NewClient: func(conn net.Conn) (stunClient, error) {
return MockableSTUNClient{StartErr: expected}, nil
},
})
if !errors.Is(err, expected) {
t.Fatalf("not the error we expected: %+v", err)
@ -88,13 +99,19 @@ func TestSTUNIPLookupStunEventContainsError(t *testing.T) {
expected := errors.New("mocked error")
ctx := context.Background()
ip, err := stunIPLookup(ctx, stunConfig{
Dial: func(network, address string) (stunClient, error) {
Dialer: &mocks.Dialer{
MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
conn := &mocks.Conn{}
return conn, nil
},
},
Endpoint: "stun.ekiga.net:3478",
Logger: log.Log,
NewClient: func(conn net.Conn) (stunClient, error) {
return MockableSTUNClient{Event: stun.Event{
Error: expected,
}}, nil
},
Endpoint: "stun.ekiga.net:3478",
Logger: log.Log,
})
if !errors.Is(err, expected) {
t.Fatalf("not the error we expected: %+v", err)
@ -107,13 +124,19 @@ func TestSTUNIPLookupStunEventContainsError(t *testing.T) {
func TestSTUNIPLookupCannotDecodeMessage(t *testing.T) {
ctx := context.Background()
ip, err := stunIPLookup(ctx, stunConfig{
Dial: func(network, address string) (stunClient, error) {
Dialer: &mocks.Dialer{
MockDialContext: func(ctx context.Context, network, address string) (net.Conn, error) {
conn := &mocks.Conn{}
return conn, nil
},
},
Endpoint: "stun.ekiga.net:3478",
Logger: log.Log,
NewClient: func(conn net.Conn) (stunClient, error) {
return MockableSTUNClient{Event: stun.Event{
Message: &stun.Message{},
}}, nil
},
Endpoint: "stun.ekiga.net:3478",
Logger: log.Log,
})
if !errors.Is(err, stun.ErrAttributeNotFound) {
t.Fatalf("not the error we expected: %+v", err)
@ -129,6 +152,7 @@ func TestIPLookupWorksUsingSTUNEkiga(t *testing.T) {
http.DefaultClient,
log.Log,
model.HTTPHeaderUserAgent,
netxlite.NewStdlibResolver(model.DiscardLogger),
)
if err != nil {
t.Fatal(err)
@ -144,6 +168,7 @@ func TestIPLookupWorksUsingSTUNGoogle(t *testing.T) {
http.DefaultClient,
log.Log,
model.HTTPHeaderUserAgent,
netxlite.NewStdlibResolver(model.DiscardLogger),
)
if err != nil {
t.Fatal(err)

1
internal/engine/geolocate/ubuntu.go
View File

@ -19,6 +19,7 @@ func ubuntuIPLookup(
httpClient *http.Client,
logger model.Logger,
userAgent string,
resolver model.Resolver,
) (string, error) {
data, err := (&httpx.APIClientTemplate{
BaseURL: "https://geoip.ubuntu.com/",

3
internal/engine/geolocate/ubuntu_test.go
View File

@ -10,6 +10,7 @@ import (
"github.com/apex/log"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/netxlite"
)
func TestUbuntuParseError(t *testing.T) {
@ -23,6 +24,7 @@ func TestUbuntuParseError(t *testing.T) {
}},
log.Log,
model.HTTPHeaderUserAgent,
netxlite.NewStdlibResolver(model.DiscardLogger),
)
if err == nil || !strings.HasPrefix(err.Error(), "XML syntax error") {
t.Fatalf("not the error we expected: %+v", err)
@ -38,6 +40,7 @@ func TestIPLookupWorksUsingUbuntu(t *testing.T) {
http.DefaultClient,
log.Log,
model.HTTPHeaderUserAgent,
netxlite.NewStdlibResolver(model.DiscardLogger),
)
if err != nil {
t.Fatal(err)