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

fix(netemx): address issues with quic-go/quic-go #1228

Merged
merged 2 commits into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion internal/netemx/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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":"","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}}}
// {"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
8 changes: 6 additions & 2 deletions internal/netemx/http3.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ func (srv *http3Server) mustListenPortLocked(handler http.Handler, ipAddr net.IP
}
go srvr.Serve(listener)

// make sure we track the server (the .Serve method will close the
// listener once we close the server itself)
// make sure we track and close the listener: assuming the server was closing the
// listener seems to be the root cause of https://github.com/ooni/probe/issues/2527
// and closing the listener completely fixes the issue.
srv.closers = append(srv.closers, listener)

// make sure we track the server
srv.closers = append(srv.closers, srvr)
}
46 changes: 4 additions & 42 deletions internal/netemx/http3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,8 @@ import (

func TestHTTP3ServerFactory(t *testing.T) {
t.Run("when using the TLSConfig provided by netem", 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.57, 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.
*/

env := MustNewQAEnv(
QAEnvOptionNetStack("10.55.56.57", &HTTP3ServerFactory{
QAEnvOptionNetStack(AddressWwwExampleCom, &HTTP3ServerFactory{
Factory: HTTPHandlerFactoryFunc(func(_ *netem.UNetStack) http.Handler {
return ExampleWebPageHandler()
}),
Expand All @@ -43,7 +24,7 @@ func TestHTTP3ServerFactory(t *testing.T) {
)
defer env.Close()

env.AddRecordToAllResolvers("www.example.com", "", "10.55.56.57")
env.AddRecordToAllResolvers("www.example.com", "", AddressWwwExampleCom)

env.Do(func() {
client := netxlite.NewHTTP3ClientWithResolver(log.Log, netxlite.NewStdlibResolver(log.Log))
Expand All @@ -67,31 +48,12 @@ func TestHTTP3ServerFactory(t *testing.T) {
})

t.Run("when using an incompatible TLS config", 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.100, 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.
*/

// we're creating a distinct MITM TLS config and we're using it, so we expect
// that we're not able to verify certificates in client code
mitmConfig := runtimex.Try1(netem.NewTLSMITMConfig())

env := MustNewQAEnv(
QAEnvOptionNetStack("10.55.56.100", &HTTP3ServerFactory{
QAEnvOptionNetStack(AddressWwwExampleCom, &HTTP3ServerFactory{
Factory: HTTPHandlerFactoryFunc(func(_ *netem.UNetStack) http.Handler {
return ExampleWebPageHandler()
}),
Expand All @@ -101,7 +63,7 @@ func TestHTTP3ServerFactory(t *testing.T) {
)
defer env.Close()

env.AddRecordToAllResolvers("www.example.com", "", "10.55.56.100")
env.AddRecordToAllResolvers("www.example.com", "", AddressWwwExampleCom)

env.Do(func() {
client := netxlite.NewHTTP3ClientWithResolver(log.Log, netxlite.NewStdlibResolver(log.Log))
Expand Down
4 changes: 0 additions & 4 deletions internal/netemx/oohelperd.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ 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(),
Expand All @@ -59,8 +57,6 @@ 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,
Expand Down
24 changes: 21 additions & 3 deletions internal/netemx/oohelperd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,38 @@ func TestOOHelperDHandler(t *testing.T) {
Failure: nil,
},
},
QUICHandshake: map[string]model.THTLSHandshakeResult{},
QUICHandshake: map[string]model.THTLSHandshakeResult{
"93.184.216.34:443": {
ServerName: "www.example.com",
Status: true,
Failure: nil,
},
},
HTTPRequest: model.THHTTPRequestResult{
BodyLength: 194,
DiscoveredH3Endpoint: "",
DiscoveredH3Endpoint: "www.example.com:443",
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: nil,
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,
},
DNS: model.THDNSResult{
Failure: nil,
Addrs: []string{"93.184.216.34"},
Expand Down
23 changes: 2 additions & 21 deletions internal/netemx/qaenv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,29 +136,10 @@ 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(
"10.55.56.101",
netemx.AddressWwwExampleCom,
netemx.ExampleWebPageHandlerFactory(),
),
)
Expand All @@ -168,7 +149,7 @@ func TestQAEnv(t *testing.T) {
env.AddRecordToAllResolvers(
"www.example.com",
"", // CNAME
"10.55.56.101",
netemx.AddressWwwExampleCom,
)

env.Do(func() {
Expand Down
4 changes: 4 additions & 0 deletions internal/netemx/scenario.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@ func MustNewScenario(config []*ScenarioDomainAddresses) *QAEnv {
Factory: sad.WebServerFactory,
Ports: []int{443},
TLSConfig: nil, // use netem's default
}, &HTTP3ServerFactory{
Factory: sad.WebServerFactory,
Ports: []int{443},
TLSConfig: nil, // use netem's default
}))
}

Expand Down
4 changes: 2 additions & 2 deletions internal/netemx/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`) // see https://github.com/ooni/probe/issues/2527
w.Header().Add("Alt-Svc", `h3=":443"`)
w.Header().Add("Date", "Thu, 24 Aug 2023 14:35:29 GMT")

// According to Go documentation, the host header is removed from the
Expand Down Expand Up @@ -87,7 +87,7 @@ const Blockpage = `<!doctype html>
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"`) // see https://github.com/ooni/probe/issues/2527
w.Header().Add("Alt-Svc", `h3=":443"`)
w.Header().Add("Date", "Thu, 24 Aug 2023 14:35:29 GMT")
w.Write([]byte(Blockpage))
})
Expand Down