Skip to content

Commit

Permalink
fix: avoid using 130.192.91.x for most netemx addresses (#1219)
Browse files Browse the repository at this point in the history
As we learned in #1216, using the
130.192.91.x namespace for every IP address in netemx breaks mapping
domain names to IP addresess in Web Connectivity. No IP address would
ever, in fact, be inconsistent, because they all belong to AS137.

Initially, I thought about overriding the code that maps IP addresses to
ASNs, to provide a custom implementation. But then I realized it was a
more thorough test to use the default implementation (relying on
maxminddb files) and using the correct IP addresses in the correct
address space.

My original thought for using 130.192.91.x addresses was that they were
not the right addresses for the domains we're testing, thus, in the
event in which netem was not WAI, all tests would have failed. However,
we have many tests checking that netem is WAI already, so probably I was
being excessively paranoid.

As a result, this patch modifies the code to use the correct addresses.
We're still using some 130.192.91.x addresses where it makes sense to do
so (user's IP address and default user's resolver).

Part of ooni/probe#1803.

## 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: see above
- [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: N/A
- [x] if you changed code inside an experiment, make sure you bump its
version number: N/A
  • Loading branch information
bassosimone committed Sep 4, 2023
1 parent d3d9782 commit 7109b14
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 111 deletions.
4 changes: 1 addition & 3 deletions internal/experiment/webconnectivitylte/qa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ func TestQA(t *testing.T) {
t.Skip("this nettest cannot run on Web Connectivity LTE")
}
measurer := NewExperimentMeasurer(&Config{
// We override the resolver used by default because the QA environment uses
// only IP addresses in the 130.192.91.x namespace for extra robustness in case
// netem is not working as intended and we're using the real network.
// We override the resolver to use the one we should be using with netem
DNSOverUDPResolver: net.JoinHostPort(netemx.QAEnvDefaultUncensoredResolverAddress, "53"),
})
if err := webconnectivityqa.RunTestCase(measurer, tc); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/experiment/webconnectivityqa/tcpblocking.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func tcpBlockingConnectTimeout() *TestCase {
Configure: func(env *netemx.QAEnv) {
env.DPIEngine().AddRule(&netem.DPIDropTrafficForServerEndpoint{
Logger: log.Log,
ServerIPAddress: "130.192.91.7", // www.example.com
ServerIPAddress: netemx.InternetScenarioAddressWwwExampleCom,
ServerPort: 443,
ServerProtocol: layers.IPProtocolTCP,
})
Expand Down
4 changes: 3 additions & 1 deletion internal/experiment/webconnectivityqa/tcpblocking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package webconnectivityqa

import (
"context"
"net"
"testing"

"github.com/apex/log"
Expand All @@ -16,7 +17,8 @@ func TestTCPBlockingConnectTimeout(t *testing.T) {

env.Do(func() {
dialer := netxlite.NewDialerWithoutResolver(log.Log)
conn, err := dialer.DialContext(context.Background(), "tcp", "130.192.91.7:443")
endpoint := net.JoinHostPort(netemx.InternetScenarioAddressWwwExampleCom, "443")
conn, err := dialer.DialContext(context.Background(), "tcp", endpoint)
if err == nil || err.Error() != netxlite.FailureGenericTimeoutError {
t.Fatal("unexpected error", err)
}
Expand Down
30 changes: 30 additions & 0 deletions internal/netemx/android_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package netemx

import (
"context"
"errors"
"testing"

"github.com/apex/log"
"github.com/ooni/probe-cli/v3/internal/netxlite"
)

// Make sure we can emulate Android's getaddrinfo behavior.
func TestEmulateAndroidGetaddrinfo(t *testing.T) {
env := MustNewScenario(InternetScenario)
defer env.Close()

env.EmulateAndroidGetaddrinfo(true)
defer env.EmulateAndroidGetaddrinfo(false)

env.Do(func() {
reso := netxlite.NewStdlibResolver(log.Log)
addrs, err := reso.LookupHost(context.Background(), "www.nonexistent.xyz")
if !errors.Is(err, netxlite.ErrAndroidDNSCacheNoData) {
t.Fatal("unexpected error")
}
if len(addrs) != 0 {
t.Fatal("expected zero-length addresses")
}
})
}
44 changes: 17 additions & 27 deletions internal/netemx/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"fmt"
"net"
"net/http"

"github.com/apex/log"
Expand All @@ -14,26 +15,15 @@ import (
"github.com/ooni/probe-cli/v3/internal/runtimex"
)

// exampleExampleComAddress is the address of example.com
const exampleExampleComAddress = "93.184.216.34"

// exampleClientAddress is the address used by the client
const exampleClientAddress = "130.192.91.211"

// exampleISPResolverAddress is the address used by the resolver.
const exampleISPResolverAddress = "130.192.3.24"

// exampleCensoredAddress is a censored IP address.
const exampleCensoredAddress = "10.10.34.35"

// exampleNewEnvironment creates a QA environment setting all possible options. We're going
// to use this QA environment in all the examples for this package.
func exampleNewEnvironment() *netemx.QAEnv {
return netemx.MustNewQAEnv(
netemx.QAEnvOptionDNSOverUDPResolvers("8.8.4.4", "9.9.9.9"),
netemx.QAEnvOptionClientAddress(exampleClientAddress),
netemx.QAEnvOptionISPResolverAddress(exampleISPResolverAddress),
netemx.QAEnvOptionHTTPServer(exampleExampleComAddress, netemx.ExampleWebPageHandlerFactory()),
netemx.QAEnvOptionClientAddress(netemx.QAEnvDefaultClientAddress),
netemx.QAEnvOptionISPResolverAddress(netemx.QAEnvDefaultISPResolverAddress),
netemx.QAEnvOptionHTTPServer(
netemx.InternetScenarioAddressWwwExampleCom, netemx.ExampleWebPageHandlerFactory()),
netemx.QAEnvOptionLogger(log.Log),
)
}
Expand All @@ -44,7 +34,7 @@ func exampleAddRecordToAllResolvers(env *netemx.QAEnv) {
env.AddRecordToAllResolvers(
"example.com",
"", // CNAME
exampleExampleComAddress,
netemx.InternetScenarioAddressWwwExampleCom,
)
}

Expand Down Expand Up @@ -103,14 +93,14 @@ func Example_resolverConfig() {
env.OtherResolversConfig().AddRecord(
"example.com",
"", // CNAME
exampleExampleComAddress,
netemx.InternetScenarioAddressWwwExampleCom,
)

// create a censored configuration for getaddrinfo
env.ISPResolverConfig().AddRecord(
"example.com",
"",
exampleCensoredAddress,
"10.10.34.35",
)

// collect the overall results
Expand Down Expand Up @@ -226,7 +216,7 @@ func Example_dohWithInternetScenario() {
})

// Output:
// [130.192.91.7]
// [93.184.216.34]
}

// This example shows how the [InternetScenario] defines DNS-over-UDP servers.
Expand All @@ -236,8 +226,8 @@ func Example_dnsOverUDPWithInternetScenario() {

env.Do(func() {
resolvers := []string{
"130.192.91.3:53", // possibly censored and used by the client's getaddrinfo
"130.192.91.4:53", // uncensored and used by the servers
net.JoinHostPort(netemx.QAEnvDefaultISPResolverAddress, "53"),
net.JoinHostPort(netemx.QAEnvDefaultUncensoredResolverAddress, "53"),
}

for _, endpoint := range resolvers {
Expand All @@ -255,8 +245,8 @@ func Example_dnsOverUDPWithInternetScenario() {
})

// Output:
// [130.192.91.7]
// [130.192.91.7]
// [93.184.216.34]
// [93.184.216.34]
}

// This example shows how the [InternetScenario] supports calling getaddrinfo.
Expand All @@ -277,7 +267,7 @@ func Example_getaddrinfoWithInternetScenario() {
})

// Output:
// [130.192.91.7]
// [93.184.216.34]
}

// This example shows how the [InternetScenario] defines an example.com-like webserver.
Expand Down Expand Up @@ -358,7 +348,7 @@ func Example_oohelperdWithInternetScenario() {

env.Do(func() {
client := netxlite.NewHTTPClientStdlib(log.Log)
thRequest := []byte(`{"http_request": "https://www.example.com/","http_request_headers":{},"tcp_connect":["130.192.91.7"]}`)
thRequest := []byte(`{"http_request": "https://www.example.com/","http_request_headers":{},"tcp_connect":["93.184.216.34:443"]}`)

req, err := http.NewRequest("POST", "https://0.th.ooni.org/", bytes.NewReader(thRequest))
if err != nil {
Expand All @@ -381,7 +371,7 @@ func Example_oohelperdWithInternetScenario() {
})

// Output:
// {"tcp_connect":{"130.192.91.7:443":{"status":true,"failure":null}},"tls_handshake":{"130.192.91.7:443":{"server_name":"www.example.com","status":true,"failure":null}},"quic_handshake":{},"http_request":{"body_length":194,"discovered_h3_endpoint":"www.example.com:443","failure":null,"title":"Default Web Page","headers":{"Alt-Svc":"h3=\":443\"","Content-Length":"194","Content-Type":"text/html; charset=utf-8","Date":"Thu, 24 Aug 2023 14:35:29 GMT"},"status_code":200},"http3_request":null,"dns":{"failure":null,"addrs":["130.192.91.7"]},"ip_info":{"130.192.91.7":{"asn":137,"flags":10}}}
// {"tcp_connect":{"93.184.216.34:443":{"status":true,"failure":null}},"tls_handshake":{"93.184.216.34:443":{"server_name":"www.example.com","status":true,"failure":null}},"quic_handshake":{},"http_request":{"body_length":194,"discovered_h3_endpoint":"www.example.com:443","failure":null,"title":"Default Web Page","headers":{"Alt-Svc":"h3=\":443\"","Content-Length":"194","Content-Type":"text/html; charset=utf-8","Date":"Thu, 24 Aug 2023 14:35:29 GMT"},"status_code":200},"http3_request":null,"dns":{"failure":null,"addrs":["93.184.216.34"]},"ip_info":{"93.184.216.34":{"asn":15133,"flags":11}}}
}

// This example shows how the [InternetScenario] defines a GeoIP service like Ubuntu's one.
Expand Down Expand Up @@ -411,5 +401,5 @@ func Example_ubuntuGeoIPWithInternetScenario() {
})

// Output:
// <?xml version="1.0" encoding="UTF-8"?><Response><Ip>130.192.91.2</Ip></Response>
// <?xml version="1.0" encoding="UTF-8"?><Response><Ip>130.192.91.211</Ip></Response>
}
38 changes: 12 additions & 26 deletions internal/netemx/oohelperd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,18 @@ import (
)

func TestOOHelperDHandler(t *testing.T) {
// we use completely unrelated IP addresses such that, in the unlikely event in
// which we're not using netem, the test is poised to fail.
//
// (These were two IP addresses assigned to me when I was at polito.it.)
const (
zeroThOONIOrgAddr = "130.192.91.211"
exampleComAddr = "130.192.91.231"
)

env := MustNewQAEnv(
QAEnvOptionHTTPServer(zeroThOONIOrgAddr, &OOHelperDFactory{}),
QAEnvOptionHTTPServer(exampleComAddr, ExampleWebPageHandlerFactory()),
)
env.AddRecordToAllResolvers("example.com", "web01.example.com", exampleComAddr)
env.AddRecordToAllResolvers("0.th.ooni.org", "0-th.ooni.org", zeroThOONIOrgAddr)
env := MustNewScenario(InternetScenario)
defer env.Close()

env.Do(func() {
thReq := &model.THRequest{
HTTPRequest: "https://example.com/",
HTTPRequest: "https://www.example.com/",
HTTPRequestHeaders: map[string][]string{
"accept": {model.HTTPHeaderAccept},
"accept-language": {model.HTTPHeaderAcceptLanguage},
"user-agent": {model.HTTPHeaderUserAgent},
},
TCPConnect: []string{exampleComAddr},
TCPConnect: []string{InternetScenarioAddressWwwExampleCom},
XQUICEnabled: true,
}
thReqRaw := runtimex.Try1(json.Marshal(thReq))
Expand Down Expand Up @@ -76,28 +62,28 @@ func TestOOHelperDHandler(t *testing.T) {

expectedTHResp := &model.THResponse{
TCPConnect: map[string]model.THTCPConnectResult{
"130.192.91.231:443": {
"93.184.216.34:443": {
Status: true,
Failure: nil,
},
},
TLSHandshake: map[string]model.THTLSHandshakeResult{
"130.192.91.231:443": {
ServerName: "example.com",
"93.184.216.34:443": {
ServerName: "www.example.com",
Status: true,
Failure: nil,
},
},
QUICHandshake: map[string]model.THTLSHandshakeResult{
"130.192.91.231:443": {
ServerName: "example.com",
"93.184.216.34:443": {
ServerName: "www.example.com",
Status: true,
Failure: nil,
},
},
HTTPRequest: model.THHTTPRequestResult{
BodyLength: 194,
DiscoveredH3Endpoint: "example.com:443",
DiscoveredH3Endpoint: "www.example.com:443",
Failure: nil,
Title: "Default Web Page",
Headers: map[string]string{
Expand All @@ -122,12 +108,12 @@ func TestOOHelperDHandler(t *testing.T) {
},
DNS: model.THDNSResult{
Failure: nil,
Addrs: []string{"130.192.91.231"},
Addrs: []string{"93.184.216.34"},
ASNs: nil,
},
IPInfo: map[string]*model.THIPInfo{
"130.192.91.231": {
ASN: 137,
"93.184.216.34": {
ASN: 15133,
Flags: 10,
},
},
Expand Down
18 changes: 3 additions & 15 deletions internal/netemx/qaenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,13 @@ import (
)

// QAEnvDefaultClientAddress is the default client IP address.
//
// The 130.192.91.x address space belongs to polito.it and is not used for hosting a DNS server, which
// gives us additional robustness in case netem is not working as intended. (We have several
// tests making sure of that, but additional robustness won't hurt.)
const QAEnvDefaultClientAddress = "130.192.91.2"
const QAEnvDefaultClientAddress = "130.192.91.211"

// QAEnvDefaultISPResolverAddress is the default IP address of the client ISP resolver.
//
// The 130.192.91.x address space belongs to polito.it and is not used for hosting a DNS server, which
// gives us additional robustness in case netem is not working as intended. (We have several
// tests making sure of that, but additional robustness won't hurt.)
const QAEnvDefaultISPResolverAddress = "130.192.91.3"
const QAEnvDefaultISPResolverAddress = "130.192.3.21"

// QAEnvDefaultUncensoredResolverAddress is the default uncensored resolver IP address.
//
// The 130.192.91.x address space belongs to polito.it and is not used for hosting a DNS server, which
// gives us additional robustness in case netem is not working as intended. (We have several
// tests making sure of that, but additional robustness won't hurt.)
const QAEnvDefaultUncensoredResolverAddress = "130.192.91.4"
const QAEnvDefaultUncensoredResolverAddress = "1.1.1.1"

type qaEnvConfig struct {
// clientAddress is the client IP address to use.
Expand Down
Loading

0 comments on commit 7109b14

Please sign in to comment.