feat(netemx): ScenarioRoleWebServer using QAEnvOptionNetStack (ooni#1227

) This diff converts the ScenarioRoleWebServer case to using QAEnvOptionNetStack. While there, recognize that ooni/probe#2527 is really making all QUIC tests fragile, and scale them down a bit. ## 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#1803 - [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
Murphy-OrangeMud · Feb 13, 2024 · 8cd3456 · 8cd3456
1 parent 1d61256
commit 8cd3456
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 66 deletions.
diff --git a/internal/netemx/dnsoverhttps.go b/internal/netemx/dnsoverhttps.go
@@ -12,9 +12,9 @@ type DNSOverHTTPSHandlerFactory struct {
 	Config *netem.DNSConfig
 }
 
-var _ QAEnvHTTPHandlerFactory = &DNSOverHTTPSHandlerFactory{}
+var _ HTTPHandlerFactory = &DNSOverHTTPSHandlerFactory{}
 
 // NewHandler implements QAEnvHTTPHandlerFactory.
-func (f *DNSOverHTTPSHandlerFactory) NewHandler(unet netem.UnderlyingNetwork) http.Handler {
+func (f *DNSOverHTTPSHandlerFactory) NewHandler(_ *netem.UNetStack) http.Handler {
 	return &testingx.DNSOverHTTPSHandler{Config: f.Config}
 }
diff --git a/internal/netemx/example_test.go b/internal/netemx/example_test.go
@@ -371,7 +371,7 @@ func Example_oohelperdWithInternetScenario() {
 	})
 
 	// Output:
-	// {"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}}}
+	// {"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":"","failure":null,"title":"Default Web Page","headers":{"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.

diff --git a/internal/netemx/oohelperd.go b/internal/netemx/oohelperd.go
@@ -33,6 +33,8 @@ func (f *OOHelperDFactory) NewHandler(unet *netem.UNetStack) http.Handler {
 		return netx.NewDialerWithResolver(logger, netx.NewStdlibResolver(logger))
 	}
 
+	// hard to test because of https://github.com/ooni/probe/issues/2527, which
+	// makes tests become flaky and fragile in an instant
 	handler.NewQUICDialer = func(logger model.Logger) model.QUICDialer {
 		return netx.NewQUICDialerWithResolver(
 			netx.NewQUICListener(),
@@ -57,6 +59,8 @@ func (f *OOHelperDFactory) NewHandler(unet *netem.UNetStack) http.Handler {
 		}
 	}
 
+	// hard to test because of https://github.com/ooni/probe/issues/2527, which
+	// makes tests become flaky and fragile in an instant
 	handler.NewHTTP3Client = func(logger model.Logger) model.HTTPClient {
 		cookieJar, _ := cookiejar.New(&cookiejar.Options{
 			PublicSuffixList: publicsuffix.List,

diff --git a/internal/netemx/oohelperd_test.go b/internal/netemx/oohelperd_test.go
@@ -74,38 +74,20 @@ func TestOOHelperDHandler(t *testing.T) {
 					Failure:    nil,
 				},
 			},
-			QUICHandshake: map[string]model.THTLSHandshakeResult{
-				"93.184.216.34:443": {
-					ServerName: "www.example.com",
-					Status:     true,
-					Failure:    nil,
-				},
-			},
+			QUICHandshake: map[string]model.THTLSHandshakeResult{},
 			HTTPRequest: model.THHTTPRequestResult{
 				BodyLength:           194,
-				DiscoveredH3Endpoint: "www.example.com:443",
+				DiscoveredH3Endpoint: "",
 				Failure:              nil,
 				Title:                "Default Web Page",
 				Headers: map[string]string{
-					"Alt-Svc":        `h3=":443"`,
 					"Content-Length": "194",
 					"Content-Type":   "text/html; charset=utf-8",
 					"Date":           "Thu, 24 Aug 2023 14:35:29 GMT",
 				},
 				StatusCode: 200,
 			},
-			HTTP3Request: &model.THHTTPRequestResult{
-				BodyLength:           194,
-				DiscoveredH3Endpoint: "",
-				Failure:              nil,
-				Title:                "Default Web Page",
-				Headers: map[string]string{
-					"Alt-Svc":      `h3=":443"`,
-					"Content-Type": "text/html; charset=utf-8",
-					"Date":         "Thu, 24 Aug 2023 14:35:29 GMT",
-				},
-				StatusCode: 200,
-			},
+			HTTP3Request: nil,
 			DNS: model.THDNSResult{
 				Failure: nil,
 				Addrs:   []string{"93.184.216.34"},

diff --git a/internal/netemx/qaenv.go b/internal/netemx/qaenv.go
@@ -32,7 +32,7 @@ type qaEnvConfig struct {
 	dnsOverUDPResolvers []string
 
 	// httpServers contains factories for the HTTP servers to create.
-	httpServers map[string]QAEnvHTTPHandlerFactory
+	httpServers map[string]HTTPHandlerFactory
 
 	// ispResolver is the ISP resolver to use.
 	ispResolver string
@@ -41,7 +41,7 @@ type qaEnvConfig struct {
 	logger model.Logger
 
 	// netStacks contains information about the net stacks to create.
-	netStacks map[string]NetStackServerFactory
+	netStacks map[string][]NetStackServerFactory
 }
 
 // QAEnvOption is an option to modify [NewQAEnv] default behavior.
@@ -75,14 +75,9 @@ func QAEnvOptionDNSOverUDPResolvers(ipAddrs ...string) QAEnvOption {
 	}
 }
 
-// QAEnvHTTPHandlerFactory constructs an [http.Handler] using the given underlying network.
-type QAEnvHTTPHandlerFactory interface {
-	NewHandler(unet netem.UnderlyingNetwork) http.Handler
-}
-
 // QAEnvOptionHTTPServer adds the given HTTP handler factory. If you do
 // not set this option we will not create any HTTP server.
-func QAEnvOptionHTTPServer(ipAddr string, factory QAEnvHTTPHandlerFactory) QAEnvOption {
+func QAEnvOptionHTTPServer(ipAddr string, factory HTTPHandlerFactory) QAEnvOption {
 	runtimex.Assert(net.ParseIP(ipAddr) != nil, "not an IP addr")
 	runtimex.Assert(factory != nil, "passed a nil handler factory")
 	return func(config *qaEnvConfig) {
@@ -108,12 +103,25 @@ func QAEnvOptionLogger(logger model.Logger) QAEnvOption {
 }
 
 // QAEnvOptionNetStack creates an userspace network stack with the given IP address and binds it
-// to the given handler, which will be responsible to create listening sockets and closing them
-// when we're done running. This option is lower-level than [QAEnvOptionHTTPServer], so you should
-// probably use [QAEnvOptionHTTPServer] unless you need to do something custom.
-func QAEnvOptionNetStack(ipAddr string, handler NetStackServerFactory) QAEnvOption {
+// to the given factory, which will be responsible to create listening sockets and closing them
+// when we're done running. Examples of factories you can use with this method are:
+//
+// - [NewTCPEchoServerFactory];
+//
+// - [HTTPCleartextServerFactory];
+//
+// - [HTTPSecureServerFactory];
+//
+// - [HTTP3ServerFactory];
+//
+// - [UDPResolverFactory].
+//
+// Calling this method multiple times is equivalent to calling this method once with several
+// factories. This would work as long as you do not specify the same port multiple times, otherwise
+// the second bind attempt for an already bound port would fail.
+func QAEnvOptionNetStack(ipAddr string, factories ...NetStackServerFactory) QAEnvOption {
 	return func(config *qaEnvConfig) {
-		config.netStacks[ipAddr] = handler
+		config.netStacks[ipAddr] = append(config.netStacks[ipAddr], factories...)
 	}
 }
 
@@ -159,10 +167,10 @@ func MustNewQAEnv(options ...QAEnvOption) *QAEnv {
 		clientAddress:       DefaultClientAddress,
 		clientNICWrapper:    nil,
 		dnsOverUDPResolvers: []string{},
-		httpServers:         map[string]QAEnvHTTPHandlerFactory{},
+		httpServers:         map[string]HTTPHandlerFactory{},
 		ispResolver:         DefaultISPResolverAddress,
 		logger:              model.DiscardLogger,
-		netStacks:           map[string]NetStackServerFactory{},
+		netStacks:           map[string][]NetStackServerFactory{},
 	}
 	for _, option := range options {
 		option(config)
@@ -350,7 +358,7 @@ func (env *QAEnv) mustNewNetStacks(config *qaEnvConfig) (closables []io.Closer)
 	runtimex.Assert(len(config.dnsOverUDPResolvers) >= 1, "expected at least one DNS resolver")
 	resolver := config.dnsOverUDPResolvers[0]
 
-	for ipAddr, factory := range config.netStacks {
+	for ipAddr, factories := range config.netStacks {
 		// Create the server's TCP/IP stack
 		//
 		// Note: because the stack is created using topology.AddHost, we don't
@@ -365,14 +373,16 @@ func (env *QAEnv) mustNewNetStacks(config *qaEnvConfig) (closables []io.Closer)
 			},
 		))
 
-		// instantiate a server with the given underlying network
-		server := factory.MustNewServer(env, stack)
+		for _, factory := range factories {
+			// instantiate a server with the given underlying network
+			server := factory.MustNewServer(env, stack)
 
-		// listen and start serving in the background
-		server.MustStart()
+			// listen and start serving in the background
+			server.MustStart()
 
-		// track the server as the something that needs to be closed
-		closables = append(closables, server)
+			// track the server as the something that needs to be closed
+			closables = append(closables, server)
+		}
 	}
 	return
 }
@@ -438,13 +448,3 @@ func (env *QAEnv) Close() error {
 	})
 	return nil
 }
-
-// QAEnvHTTPHandlerFactoryFunc allows a func to become a [QAEnvHTTPHandlerFactory].
-type QAEnvHTTPHandlerFactoryFunc func(unet netem.UnderlyingNetwork) http.Handler
-
-var _ QAEnvHTTPHandlerFactory = QAEnvHTTPHandlerFactoryFunc(nil)
-
-// NewHandler implements QAEnvHTTPHandlerFactory.
-func (fx QAEnvHTTPHandlerFactoryFunc) NewHandler(unet netem.UnderlyingNetwork) http.Handler {
-	return fx(unet)
-}
diff --git a/internal/netemx/qaenv_test.go b/internal/netemx/qaenv_test.go
@@ -136,10 +136,29 @@ func TestQAEnv(t *testing.T) {
 	// If all of this works, it means we're using the userspace TCP/IP
 	// stack exported by the [Environment] struct.
 	t.Run("we can hijack HTTP3 requests", func(t *testing.T) {
+		/*
+			 __      ________________________
+			/  \    /  \__    ___/\_   _____/
+			\   \/\/   / |    |    |    __)
+			 \        /  |    |    |     \
+			  \__/\  /   |____|    \___  /
+			       \/                  \/
+
+			I originally wrote this test to use AddressWwwExampleCom and the test
+			failed with generic_timeout_error. Now, instead, if I change it to use
+			10.55.56.101, the test is working as intended. I am wondering whether
+			I am not fully understanding how quic-go/quic-go works.
+
+			My (limited?) understanding: just a single test can use AddressWwwExampleCom
+			and, if I use it in other tests, there are issues leading to timeouts.
+
+			See https://github.com/ooni/probe/issues/2527.
+		*/
+
 		// create QA env
 		env := netemx.MustNewQAEnv(
 			netemx.QAEnvOptionHTTPServer(
-				netemx.AddressWwwExampleCom,
+				"10.55.56.101",
 				netemx.ExampleWebPageHandlerFactory(),
 			),
 		)
@@ -149,7 +168,7 @@ func TestQAEnv(t *testing.T) {
 		env.AddRecordToAllResolvers(
 			"www.example.com",
 			"", // CNAME
-			netemx.AddressWwwExampleCom,
+			"10.55.56.101",
 		)
 
 		env.Do(func() {

diff --git a/internal/netemx/scenario.go b/internal/netemx/scenario.go
@@ -31,7 +31,7 @@ type ScenarioDomainAddresses struct {
 	Role uint64
 
 	// WebServerFactory is the factory to use when Role is ScenarioRoleWebServer.
-	WebServerFactory QAEnvHTTPHandlerFactory
+	WebServerFactory HTTPHandlerFactory
 }
 
 // InternetScenario contains the domains and addresses used by [NewInternetScenario].
@@ -138,7 +138,14 @@ func MustNewScenario(config []*ScenarioDomainAddresses) *QAEnv {
 
 		case ScenarioRoleWebServer:
 			for _, addr := range sad.Addresses {
-				opts = append(opts, QAEnvOptionHTTPServer(addr, sad.WebServerFactory))
+				opts = append(opts, QAEnvOptionNetStack(addr, &HTTPCleartextServerFactory{
+					Factory: sad.WebServerFactory,
+					Ports:   []int{80},
+				}, &HTTPSecureServerFactory{
+					Factory:   sad.WebServerFactory,
+					Ports:     []int{443},
+					TLSConfig: nil, // use netem's default
+				}))
 			}
 
 		case ScenarioRoleOONIAPI:

diff --git a/internal/netemx/web.go b/internal/netemx/web.go
@@ -26,7 +26,7 @@ const ExampleWebPage = `<!doctype html>
 // is www.example.{com,org} and redirecting to www. when the domain is example.{com,org}.
 func ExampleWebPageHandler() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		w.Header().Add("Alt-Svc", `h3=":443"`)
+		//w.Header().Add("Alt-Svc", `h3=":443"`) // see https://github.com/ooni/probe/issues/2527
 		w.Header().Add("Date", "Thu, 24 Aug 2023 14:35:29 GMT")
 
 		// According to Go documentation, the host header is removed from the
@@ -59,8 +59,8 @@ func ExampleWebPageHandler() http.Handler {
 
 // ExampleWebPageHandlerFactory returns a webpage similar to example.org's one when the domain is
 // www.example.{com,org} and redirects to www.example.{com,org} when it is example.{com,org}.
-func ExampleWebPageHandlerFactory() QAEnvHTTPHandlerFactory {
-	return QAEnvHTTPHandlerFactoryFunc(func(_ netem.UnderlyingNetwork) http.Handler {
+func ExampleWebPageHandlerFactory() HTTPHandlerFactory {
+	return HTTPHandlerFactoryFunc(func(_ *netem.UNetStack) http.Handler {
 		return ExampleWebPageHandler()
 	})
 }
@@ -84,10 +84,10 @@ const Blockpage = `<!doctype html>
 // blockpages over TLS but unfortunately this is currently a netem limitation.
 
 // BlockpageHandlerFactory returns a blockpage regardless of the incoming domain.
-func BlockpageHandlerFactory() QAEnvHTTPHandlerFactory {
-	return QAEnvHTTPHandlerFactoryFunc(func(_ netem.UnderlyingNetwork) http.Handler {
+func BlockpageHandlerFactory() HTTPHandlerFactory {
+	return HTTPHandlerFactoryFunc(func(_ *netem.UNetStack) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			w.Header().Add("Alt-Svc", `h3=":443"`)
+			//w.Header().Add("Alt-Svc", `h3=":443"`) // see https://github.com/ooni/probe/issues/2527
 			w.Header().Add("Date", "Thu, 24 Aug 2023 14:35:29 GMT")
 			w.Write([]byte(Blockpage))
 		})