From 174a41192862fd013bff9b20bff68c65d4cf4f92 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 5 Sep 2023 20:24:59 +0200 Subject: [PATCH 1/2] feat(testingx): import and adapt jafar TLS SNI proxy This is the last bit of jafar we needed to import and adapt. What remains to be done now is finishing converting QA tests. Then, we can remove jafar and QA. Reference issue: https://github.com/ooni/probe/issues/1803. --- internal/logx/handler_test.go | 9 +- internal/logx/prefix_test.go | 15 +-- internal/testingx/tlssniproxy.go | 132 +++++++++++++++++++++++ internal/testingx/tlssniproxy_test.go | 121 +++++++++++++++++++++ internal/testingx/tlsx.go | 13 +-- internal/testingx/tlsx_test.go | 149 +++++++++++++++++++++++++- script/nocopyreadall.bash | 6 ++ 7 files changed, 427 insertions(+), 18 deletions(-) create mode 100644 internal/testingx/tlssniproxy.go create mode 100644 internal/testingx/tlssniproxy_test.go diff --git a/internal/logx/handler_test.go b/internal/logx/handler_test.go index c83c1c85a5..65eaccacd3 100644 --- a/internal/logx/handler_test.go +++ b/internal/logx/handler_test.go @@ -1,4 +1,4 @@ -package logx +package logx_test import ( "fmt" @@ -8,12 +8,13 @@ import ( "github.com/apex/log" "github.com/google/go-cmp/cmp" + "github.com/ooni/probe-cli/v3/internal/logx" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/testingx" ) func TestNewHandlerWithDefaultSettings(t *testing.T) { - lh := NewHandlerWithDefaultSettings() + lh := logx.NewHandlerWithDefaultSettings() if lh.Emoji { t.Fatal("expected false") } @@ -27,8 +28,8 @@ func TestNewHandlerWithDefaultSettings(t *testing.T) { } // creates a new handler with deterministic time to help with testing -func newHandlerForTesting() *Handler { - lh := NewHandlerWithDefaultSettings() +func newHandlerForTesting() *logx.Handler { + lh := logx.NewHandlerWithDefaultSettings() dtime := testingx.NewTimeDeterministic(time.Now()) lh.Now = dtime.Now lh.StartTime = dtime.Now() diff --git a/internal/logx/prefix_test.go b/internal/logx/prefix_test.go index 6b60e7a0c1..8c22ff9ee0 100644 --- a/internal/logx/prefix_test.go +++ b/internal/logx/prefix_test.go @@ -1,8 +1,9 @@ -package logx +package logx_test import ( "testing" + "github.com/ooni/probe-cli/v3/internal/logx" "github.com/ooni/probe-cli/v3/internal/mocks" ) @@ -16,7 +17,7 @@ func TestPrefixLogger(t *testing.T) { } }, } - logger := &PrefixLogger{ + logger := &logx.PrefixLogger{ Prefix: "<0>", Logger: base, } @@ -32,7 +33,7 @@ func TestPrefixLogger(t *testing.T) { } }, } - logger := &PrefixLogger{ + logger := &logx.PrefixLogger{ Prefix: "<0>", Logger: base, } @@ -48,7 +49,7 @@ func TestPrefixLogger(t *testing.T) { } }, } - logger := &PrefixLogger{ + logger := &logx.PrefixLogger{ Prefix: "<0>", Logger: base, } @@ -64,7 +65,7 @@ func TestPrefixLogger(t *testing.T) { } }, } - logger := &PrefixLogger{ + logger := &logx.PrefixLogger{ Prefix: "<0>", Logger: base, } @@ -80,7 +81,7 @@ func TestPrefixLogger(t *testing.T) { } }, } - logger := &PrefixLogger{ + logger := &logx.PrefixLogger{ Prefix: "<0>", Logger: base, } @@ -96,7 +97,7 @@ func TestPrefixLogger(t *testing.T) { } }, } - logger := &PrefixLogger{ + logger := &logx.PrefixLogger{ Prefix: "<0>", Logger: base, } diff --git a/internal/testingx/tlssniproxy.go b/internal/testingx/tlssniproxy.go new file mode 100644 index 0000000000..4c1f9bdf78 --- /dev/null +++ b/internal/testingx/tlssniproxy.go @@ -0,0 +1,132 @@ +package testingx + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "sync" + + "github.com/ooni/netem" + "github.com/ooni/probe-cli/v3/internal/logx" + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +// TLSSNIProxyNetx is how [TLSSNIProxy] views [*netxlite.Netx]. +type TLSSNIProxyNetx interface { + NewDialerWithResolver(dl model.DebugLogger, r model.Resolver, w ...model.DialerWrapper) model.Dialer + NewStdlibResolver(logger model.DebugLogger, wrappers ...model.DNSTransportWrapper) model.Resolver +} + +// TLSSNIProxy is a proxy using the SNI to figure out where to connect to. +type TLSSNIProxy struct { + // closeOnce provides "once" semantics for Close. + closeOnce sync.Once + + // listener is the TCP listener we're using. + listener net.Listener + + // logger is the logger we should use. + logger model.Logger + + // netx is the underlying network. + netx TLSSNIProxyNetx + + // wg is the wait group for the background listener + wg *sync.WaitGroup +} + +// TODO(bassosimone): MustNewTLSSNIProxyEx prototype would be simpler if +// netx.SNI was also able to create listening TCP connections + +// MustNewTLSSNIProxyEx creates a new [*TLSSNIProxy]. +func MustNewTLSSNIProxyEx( + logger model.Logger, netx TLSSNIProxyNetx, tcpAddr *net.TCPAddr, tcpListener TCPListener) *TLSSNIProxy { + listener := runtimex.Try1(tcpListener.ListenTCP("tcp", tcpAddr)) + proxy := &TLSSNIProxy{ + closeOnce: sync.Once{}, + listener: listener, + logger: &logx.PrefixLogger{ + Prefix: fmt.Sprintf("%-16s", "TLSPROXY"), + Logger: logger, + }, + netx: netx, + wg: &sync.WaitGroup{}, + } + proxy.wg.Add(1) + go proxy.mainloop() + return proxy +} + +// Close implements io.Closer +func (tp *TLSSNIProxy) Close() (err error) { + tp.closeOnce.Do(func() { + err = tp.listener.Close() + tp.wg.Wait() + }) + return +} + +// Endpoint returns the listening endpoint or nil after Close has been called. +func (tp *TLSSNIProxy) Endpoint() string { + return tp.listener.Addr().String() +} + +func (tp *TLSSNIProxy) mainloop() { + // make sure panics don't crash the process + defer runtimex.CatchLogAndIgnorePanic(tp.logger, "TLSSNIProxy.mainloop") + + defer tp.wg.Done() + for { + conn, err := tp.listener.Accept() + if errors.Is(err, net.ErrClosed) { + return + } + + // use panics to reduce the testing surface, which is ~okay given + // that this code is meant to support testing + runtimex.PanicOnError(err, "tp.listener.Accept() failed") + + // we're creating a goroutine per connection, which is ~okay because + // this code is designed for helping with testing + go tp.handle(conn) + } +} + +func (tp *TLSSNIProxy) handle(clientConn net.Conn) { + // make sure panics don't crash the process + defer runtimex.CatchLogAndIgnorePanic(tp.logger, "TLSSNIProxy.handle") + + // make sure we close the client connection + defer clientConn.Close() + + // read initial records + buffer := make([]byte, 1<<17) + count := runtimex.Try1(clientConn.Read(buffer)) + rawRecords := buffer[:count] + + // inspecty the raw records to find the SNI + sni := runtimex.Try1(netem.ExtractTLSServerName(rawRecords)) + + // connect to the remote host + tcpDialer := tp.netx.NewDialerWithResolver(tp.logger, tp.netx.NewStdlibResolver(tp.logger)) + serverConn := runtimex.Try1(tcpDialer.DialContext(context.Background(), "tcp", net.JoinHostPort(sni, "443"))) + defer serverConn.Close() + + // forward the initial records to the server + _ = runtimex.Try1(serverConn.Write(rawRecords)) + + // route traffic between the conns + wg := &sync.WaitGroup{} + wg.Add(2) + go tp.forward(wg, clientConn, serverConn) + go tp.forward(wg, serverConn, clientConn) + wg.Wait() +} + +func (tp *TLSSNIProxy) forward(wg *sync.WaitGroup, left, right net.Conn) { + defer wg.Done() + io.Copy(right, left) +} diff --git a/internal/testingx/tlssniproxy_test.go b/internal/testingx/tlssniproxy_test.go new file mode 100644 index 0000000000..0ee9c5c11f --- /dev/null +++ b/internal/testingx/tlssniproxy_test.go @@ -0,0 +1,121 @@ +package testingx_test + +import ( + "context" + "crypto/tls" + "io" + "net" + "net/http" + "testing" + + "github.com/apex/log" + "github.com/ooni/netem" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" + "github.com/ooni/probe-cli/v3/internal/testingx" +) + +func TestTLSSNIProxy(t *testing.T) { + // testcase is a test case run by this function + type testcase struct { + name string + construct func() (*testingx.TLSSNIProxy, *netxlite.Netx, []io.Closer) + short bool + } + + testcases := []testcase{{ + name: "when using the real network", + construct: func() (*testingx.TLSSNIProxy, *netxlite.Netx, []io.Closer) { + var closers []io.Closer + + netxProxy := &netxlite.Netx{ + Underlying: nil, // use the network + } + tcpAddr := &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1)} + tcpListener := &testingx.TCPListenerStdlib{} + + proxy := testingx.MustNewTLSSNIProxyEx(log.Log, netxProxy, tcpAddr, tcpListener) + closers = append(closers, proxy) + + netxClient := &netxlite.Netx{ + Underlying: nil, // use the network + } + + return proxy, netxClient, closers + }, + short: false, + }, { + name: "when using netem", + construct: func() (*testingx.TLSSNIProxy, *netxlite.Netx, []io.Closer) { + var closers []io.Closer + + topology := runtimex.Try1(netem.NewStarTopology(log.Log)) + closers = append(closers, topology) + + wwwStack := runtimex.Try1(topology.AddHost("142.251.209.14", "142.251.209.14", &netem.LinkConfig{})) + proxyStack := runtimex.Try1(topology.AddHost("10.0.0.1", "142.251.209.14", &netem.LinkConfig{})) + clientStack := runtimex.Try1(topology.AddHost("10.0.0.2", "142.251.209.14", &netem.LinkConfig{})) + + dnsConfig := netem.NewDNSConfig() + dnsConfig.AddRecord("www.google.com", "", "142.251.209.14") + dnsServer := runtimex.Try1(netem.NewDNSServer(log.Log, wwwStack, "142.251.209.14", dnsConfig)) + closers = append(closers, dnsServer) + + wwwServer := testingx.MustNewHTTPServerTLSEx( + &net.TCPAddr{IP: net.IPv4(142, 251, 209, 14), Port: 443}, + wwwStack, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("Bonsoir, Elliot!")) + }), + wwwStack, + ) + closers = append(closers, wwwServer) + + proxy := testingx.MustNewTLSSNIProxyEx( + log.Log, + &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: proxyStack}}, + &net.TCPAddr{IP: net.IPv4(10, 0, 0, 1), Port: 443}, + proxyStack, + ) + closers = append(closers, proxy) + + clientNet := &netxlite.Netx{Underlying: &netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}} + return proxy, clientNet, closers + }, + short: true, + }} + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if !tc.short && testing.Short() { + t.Skip("skip test in short mode") + } + + proxy, clientNet, closers := tc.construct() + defer func() { + for _, closer := range closers { + closer.Close() + } + }() + + //log.SetLevel(log.DebugLevel) + + tlsConfig := &tls.Config{ + ServerName: "www.google.com", + } + tcpDialer := clientNet.NewDialerWithResolver(log.Log, clientNet.NewStdlibResolver(log.Log)) + tlsHandshaker := clientNet.NewTLSHandshakerStdlib(log.Log) + tlsDialer := netxlite.NewTLSDialerWithConfig(tcpDialer, tlsHandshaker, tlsConfig) + + conn, err := tlsDialer.DialTLSContext(context.Background(), "tcp", proxy.Endpoint()) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + + tconn := conn.(netxlite.TLSConn) + connstate := tconn.ConnectionState() + t.Logf("%+v", connstate) + }) + } +} diff --git a/internal/testingx/tlsx.go b/internal/testingx/tlsx.go index e841275c6c..efcdc676ba 100644 --- a/internal/testingx/tlsx.go +++ b/internal/testingx/tlsx.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "github.com/apex/log" "github.com/ooni/netem" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -142,16 +143,16 @@ func (p *TLSServer) Close() (err error) { } func (p *TLSServer) mainloop(ctx context.Context) { + defer runtimex.CatchLogAndIgnorePanic(log.Log, "TLSServer.mainloop") defer p.wg.Done() for { conn, err := p.listener.Accept() - if errors.Is(err, net.ErrClosed) { - return - } - if err != nil { - continue - } + + // because this is a testing server and because golang returns net.ErrClosed while + // gvisor returns "invalid argument", here we're using panic to handle the error + // such that we can quickly exit and we don't need to test these implementation details + runtimex.PanicOnError(err, "p.listener.Accept") // create a goroutine for connection, which is overkill in general // but reasonable for a server designed for testing diff --git a/internal/testingx/tlsx_test.go b/internal/testingx/tlsx_test.go index 942956e798..5784e892b0 100644 --- a/internal/testingx/tlsx_test.go +++ b/internal/testingx/tlsx_test.go @@ -8,11 +8,13 @@ import ( "crypto/tls" "errors" "io" + "net" "testing" "time" "github.com/apex/log" "github.com/google/go-cmp/cmp" + "github.com/ooni/netem" "github.com/ooni/probe-cli/v3/internal/netxlite" "github.com/ooni/probe-cli/v3/internal/runtimex" "github.com/ooni/probe-cli/v3/internal/testingx" @@ -124,6 +126,9 @@ func TestTLSHandlerWithStdlib(t *testing.T) { // fallthrough } + // make sure we close the connection + defer tlsConn.Close() + // read bytes from the connection data, err := io.ReadAll(io.LimitReader(tlsConn, 1<<14)) if err != nil { @@ -137,5 +142,147 @@ func TestTLSHandlerWithStdlib(t *testing.T) { } func TestTLSHandlerWithNetem(t *testing.T) { - t.Skip("test not implemented") + // testcase is a test case implemented by this func + type testcase struct { + // name is the name of the test case + name string + + // reasonToSkip indicates the reason why we should skip this test + reasonToSkip string + + // newHandler is the factory for creating a new handler + newHandler func() testingx.TLSHandler + + // timeout is the TLS handshake timeout + timeout time.Duration + + // expectErr is the expected TLS handshake error + expectErr error + + // expectBody is the text we expect to receive otherwise + expectBody []byte + } + + // create MITM config + mitm := testingx.MustNewTLSMITMProviderNetem() + + testcases := []testcase{{ + name: "with TLSHandlerTimeout", + newHandler: func() testingx.TLSHandler { + return testingx.TLSHandlerTimeout() + }, + timeout: 1 * time.Second, + expectErr: errors.New(netxlite.FailureGenericTimeoutError), + expectBody: []byte{}, + }, { + name: "with TLSHandlerSendAlert", + newHandler: func() testingx.TLSHandler { + return testingx.TLSHandlerSendAlert(testingx.TLSAlertUnrecognizedName) + }, + timeout: 10 * time.Second, + expectErr: errors.New(netxlite.FailureSSLInvalidHostname), + expectBody: []byte{}, + }, { + name: "with TLSHandlerEOF", + newHandler: func() testingx.TLSHandler { + return testingx.TLSHandlerEOF() + }, + timeout: 10 * time.Second, + expectErr: errors.New(netxlite.FailureEOFError), + expectBody: []byte{}, + }, { + name: "with TLSHandlerReset", + reasonToSkip: "GVisor implements SO_LINGER but there is no gonet.TCPConn.SetLinger", + newHandler: func() testingx.TLSHandler { + return testingx.TLSHandlerReset() + }, + timeout: 10 * time.Second, + expectErr: errors.New(netxlite.FailureConnectionReset), + expectBody: []byte{}, + }, { + name: "with TLSHandlerHandshakeAndWriteText", + newHandler: func() testingx.TLSHandler { + return testingx.TLSHandlerHandshakeAndWriteText(mitm, testingx.HTTPBlockpage451) + }, + timeout: 10 * time.Second, + expectErr: nil, + expectBody: testingx.HTTPBlockpage451, + }} + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if tc.reasonToSkip != "" { + t.Skip(tc.reasonToSkip) + } + + //log.SetLevel(log.DebugLevel) + + // create a star topology for this test case + topology := runtimex.Try1(netem.NewStarTopology(log.Log)) + defer topology.Close() + + // create the server + serverStack := runtimex.Try1(topology.AddHost("142.251.209.14", "0.0.0.0", &netem.LinkConfig{})) + server := testingx.MustNewTLSServerEx( + &net.TCPAddr{IP: net.IPv4(142, 251, 209, 14), Port: 443}, + serverStack, + tc.newHandler(), + ) + defer server.Close() + + // create the client stack + clientStack := runtimex.Try1(topology.AddHost("10.0.0.2", "142.251.209.14", &netem.LinkConfig{})) + + // use the client stack + netxlite.WithCustomTProxy(&netxlite.NetemUnderlyingNetworkAdapter{UNet: clientStack}, func() { + // create TLS config with a specific SNI + tlsConfig := &tls.Config{ + RootCAs: runtimex.Try1(mitm.DefaultCertPool()), + ServerName: "www.example.com", + } + + // create a TLS dialer + tcpDialer := netxlite.NewDialerWithoutResolver(log.Log) + tlsHandshaker := netxlite.NewTLSHandshakerStdlib(log.Log) + tlsDialer := netxlite.NewTLSDialerWithConfig(tcpDialer, tlsHandshaker, tlsConfig) + + // create a context with a timeout + ctx, cancel := context.WithTimeout(context.Background(), tc.timeout) + defer cancel() + + // establish a TLS connection + tlsConn, err := tlsDialer.DialTLSContext(ctx, "tcp", server.Endpoint()) + + // check the result of the handshake + switch { + case tc.expectErr == nil && err != nil: + t.Fatal("expected", tc.expectErr, "but got", err) + + case tc.expectErr != nil && err == nil: + t.Fatal("expected", tc.expectErr, "but got", err) + + case tc.expectErr != nil && err != nil: + if err.Error() != tc.expectErr.Error() { + t.Fatal("expected", tc.expectErr, "but got", err) + } + return + + default: + // fallthrough + } + + // make sure we close the connection + defer tlsConn.Close() + + // read bytes from the connection + data, err := io.ReadAll(io.LimitReader(tlsConn, 1<<14)) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(tc.expectBody, data); diff != "" { + t.Fatal(diff) + } + }) + }) + } } diff --git a/script/nocopyreadall.bash b/script/nocopyreadall.bash index aa5ea1c7df..8180292d87 100755 --- a/script/nocopyreadall.bash +++ b/script/nocopyreadall.bash @@ -45,6 +45,12 @@ for file in $(find . -type f -name \*.go); do continue fi + if [ "$file" = "./internal/testingx/tlssniproxy.go" ]; then + # We're allowed to use ReadAll and Copy in this file because + # it's code that we only use for testing purposes. + continue + fi + if [ "$file" = "./internal/testingx/tlsx_test.go" ]; then # We're allowed to use ReadAll and Copy in this file because # it's code that we only use for testing purposes. From b308ff94a292401f2774e81c61d630ecfaab3362 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 5 Sep 2023 20:32:00 +0200 Subject: [PATCH 2/2] Apply suggestions from code review --- internal/testingx/tlssniproxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testingx/tlssniproxy.go b/internal/testingx/tlssniproxy.go index 4c1f9bdf78..ffd8a1e602 100644 --- a/internal/testingx/tlssniproxy.go +++ b/internal/testingx/tlssniproxy.go @@ -39,7 +39,7 @@ type TLSSNIProxy struct { } // TODO(bassosimone): MustNewTLSSNIProxyEx prototype would be simpler if -// netx.SNI was also able to create listening TCP connections +// netxlite.Netx was also able to create listening TCP connections // MustNewTLSSNIProxyEx creates a new [*TLSSNIProxy]. func MustNewTLSSNIProxyEx(