From b834af83ac62d5d47d0b188a5a13bfac63ec21ad Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Sun, 5 Sep 2021 21:23:47 +0200 Subject: [PATCH] feat: upgrade oohttp and propagate changes (#461) Part of https://github.com/ooni/probe/issues/1506 --- go.mod | 5 ++-- go.sum | 11 ++++---- internal/netxlite/mocks/tlsconn.go | 15 ++++++---- internal/netxlite/mocks/tlsconn_test.go | 7 +++-- internal/netxlite/tls.go | 24 ++++++---------- internal/netxlite/tls_test.go | 2 +- internal/netxlite/utls.go | 29 +++++++++++++++++-- internal/netxlite/utls_test.go | 37 +++++++++++++++++++++++++ 8 files changed, 95 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index 0835a38..8b922a2 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,7 @@ require ( github.com/miekg/dns v1.1.42 github.com/mitchellh/go-wordwrap v1.0.1 github.com/montanaflynn/stats v0.6.6 - github.com/ooni/oohttp v0.0.0-20210818104219-f8ceac6f2622 + github.com/ooni/oohttp v0.0.0-20210901122724-bbe70ef3c22a github.com/ooni/probe-assets v0.3.1 github.com/ooni/psiphon v0.8.0 github.com/oschwald/geoip2-golang v1.5.0 @@ -43,8 +43,9 @@ require ( gitlab.com/yawning/utls.git v0.0.12-1 golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 // indirect golang.org/x/mod v0.5.0 // indirect - golang.org/x/net v0.0.0-20210813160813-60bc85c4be6d + golang.org/x/net v0.0.0-20210903162142-ad29c8ab022f golang.org/x/sys v0.0.0-20210817190340-bfb29a6856f2 + golang.org/x/text v0.3.7 // indirect golang.org/x/tools v0.1.5 // indirect gopkg.in/AlecAivazis/survey.v1 v1.8.8 gopkg.in/mgo.v2 v2.0.0-20190816093944-a6b53ec6cb22 // indirect diff --git a/go.sum b/go.sum index 568975a..337ffe5 100644 --- a/go.sum +++ b/go.sum @@ -393,8 +393,8 @@ github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1y github.com/onsi/gomega v1.10.3/go.mod h1:V9xEwhxec5O8UDM77eCW8vLymOMltsqPVYWrpDsH8xc= github.com/onsi/gomega v1.13.0 h1:7lLHu94wT9Ij0o6EWWclhu0aOh32VxhkwEJvzuWPeak= github.com/onsi/gomega v1.13.0/go.mod h1:lRk9szgn8TxENtWd0Tp4c3wjlRfMTMH27I+3Je41yGY= -github.com/ooni/oohttp v0.0.0-20210818104219-f8ceac6f2622 h1:Wpu4o3J3fLD4BPqA3CmrnbXVAAWKEjramvfhDUFZp+E= -github.com/ooni/oohttp v0.0.0-20210818104219-f8ceac6f2622/go.mod h1:kgtoj+Dn4bmx09hEUgbPI7YX0gkWlu+fz2I0S5auyX4= +github.com/ooni/oohttp v0.0.0-20210901122724-bbe70ef3c22a h1:Vhcn8oTWHy/3SIKcUI2eb3XKWHy74779iCTrMGNCuao= +github.com/ooni/oohttp v0.0.0-20210901122724-bbe70ef3c22a/go.mod h1:kgtoj+Dn4bmx09hEUgbPI7YX0gkWlu+fz2I0S5auyX4= github.com/ooni/probe-assets v0.3.1 h1:6PDcoJTICJxL8PdeM0+a3ZfkTWrFfCn90fUqTWR0LDA= github.com/ooni/probe-assets v0.3.1/go.mod h1:N0PyNM3aadlYDDCFXAPzs54HC54+MZA/4/xnCtd9EAo= github.com/ooni/psiphon v0.8.0 h1:digldztBlINi3HWuxdK4gFhkiaheAoDVjZN/ApZHWBM= @@ -712,8 +712,8 @@ golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLd golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= -golang.org/x/net v0.0.0-20210813160813-60bc85c4be6d h1:LO7XpTYMwTqxjLcGWPijK3vRXg1aWdlNOVOHRq45d7c= -golang.org/x/net v0.0.0-20210813160813-60bc85c4be6d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= +golang.org/x/net v0.0.0-20210903162142-ad29c8ab022f h1:w6wWR0H+nyVpbSAQbzVEIACVyr/h8l/BEkY6Sokc7Eg= +golang.org/x/net v0.0.0-20210903162142-ad29c8ab022f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181203162652-d668ce993890/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -784,8 +784,9 @@ golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/time v0.0.0-20180412165947-fbb02b2291d2/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/internal/netxlite/mocks/tlsconn.go b/internal/netxlite/mocks/tlsconn.go index 2b5f221..8307b3a 100644 --- a/internal/netxlite/mocks/tlsconn.go +++ b/internal/netxlite/mocks/tlsconn.go @@ -1,6 +1,9 @@ package mocks -import "crypto/tls" +import ( + "context" + "crypto/tls" +) // TLSConn allows to mock netxlite.TLSConn. type TLSConn struct { @@ -10,8 +13,8 @@ type TLSConn struct { // MockConnectionState allows to mock the ConnectionState method. MockConnectionState func() tls.ConnectionState - // MockHandshake allows to mock the Handshake method. - MockHandshake func() error + // MockHandshakeContext allows to mock the HandshakeContext method. + MockHandshakeContext func(ctx context.Context) error } // ConnectionState calls MockConnectionState. @@ -19,7 +22,7 @@ func (c *TLSConn) ConnectionState() tls.ConnectionState { return c.MockConnectionState() } -// Handshake calls MockHandshake. -func (c *TLSConn) Handshake() error { - return c.MockHandshake() +// HandshakeContext calls MockHandshakeContext. +func (c *TLSConn) HandshakeContext(ctx context.Context) error { + return c.MockHandshakeContext(ctx) } diff --git a/internal/netxlite/mocks/tlsconn_test.go b/internal/netxlite/mocks/tlsconn_test.go index 3031fce..7b55a81 100644 --- a/internal/netxlite/mocks/tlsconn_test.go +++ b/internal/netxlite/mocks/tlsconn_test.go @@ -1,6 +1,7 @@ package mocks import ( + "context" "crypto/tls" "errors" "reflect" @@ -20,14 +21,14 @@ func TestTLSConnConnectionState(t *testing.T) { } } -func TestTLSConnHandshake(t *testing.T) { +func TestTLSConnHandshakeContext(t *testing.T) { expected := errors.New("mocked error") c := &TLSConn{ - MockHandshake: func() error { + MockHandshakeContext: func(ctx context.Context) error { return expected }, } - err := c.Handshake() + err := c.HandshakeContext(context.Background()) if !errors.Is(err, expected) { t.Fatal("not the error we expected", err) } diff --git a/internal/netxlite/tls.go b/internal/netxlite/tls.go index 5136503..73e14cf 100644 --- a/internal/netxlite/tls.go +++ b/internal/netxlite/tls.go @@ -8,6 +8,8 @@ import ( "fmt" "net" "time" + + oohttp "github.com/ooni/oohttp" ) var ( @@ -103,17 +105,12 @@ func ConfigureTLSVersion(config *tls.Config, version string) error { return nil } -// TLSConn is any tls.Conn-like structure. -type TLSConn interface { - // net.Conn is the embedded conn. - net.Conn +// TLSConn is the type of connection that oohttp expects from +// any library that implements TLS functionality. +type TLSConn = oohttp.TLSConn - // ConnectionState returns the TLS connection state. - ConnectionState() tls.ConnectionState - - // Handshake performs the handshake. - Handshake() error -} +// Ensures that a tls.Conn implements the TLSConn interface. +var _ TLSConn = &tls.Conn{} // TLSHandshaker is the generic TLS handshaker. type TLSHandshaker interface { @@ -154,11 +151,6 @@ var defaultCertPool = NewDefaultCertPool() // Handshake implements Handshaker.Handshake. This function will // configure the code to use the built-in Mozilla CA if the config // field contains a nil RootCAs field. -// -// Bug -// -// Until Go 1.17 is released, this function will not honour -// the context. We'll however always enforce an overall timeout. func (h *tlsHandshakerConfigurable) Handshake( ctx context.Context, conn net.Conn, config *tls.Config, ) (net.Conn, tls.ConnectionState, error) { @@ -173,7 +165,7 @@ func (h *tlsHandshakerConfigurable) Handshake( config.RootCAs = defaultCertPool } tlsconn := h.newConn(conn, config) - if err := tlsconn.Handshake(); err != nil { + if err := tlsconn.HandshakeContext(ctx); err != nil { return nil, tls.ConnectionState{}, err } return tlsconn, tlsconn.ConnectionState(), nil diff --git a/internal/netxlite/tls_test.go b/internal/netxlite/tls_test.go index 0ba5c59..50b5276 100644 --- a/internal/netxlite/tls_test.go +++ b/internal/netxlite/tls_test.go @@ -190,7 +190,7 @@ func TestTLSHandshakerConfigurableSetsDefaultRootCAs(t *testing.T) { NewConn: func(conn net.Conn, config *tls.Config) TLSConn { gotTLSConfig = config return &mocks.TLSConn{ - MockHandshake: func() error { + MockHandshakeContext: func(ctx context.Context) error { return expected }, } diff --git a/internal/netxlite/utls.go b/internal/netxlite/utls.go index 6b1070c..aedaacd 100644 --- a/internal/netxlite/utls.go +++ b/internal/netxlite/utls.go @@ -1,6 +1,7 @@ package netxlite import ( + "context" "crypto/tls" "net" @@ -21,9 +22,13 @@ func NewTLSHandshakerUTLS(logger Logger, id *utls.ClientHelloID) TLSHandshaker { // utlsConn implements TLSConn and uses a utls UConn as its underlying connection type utlsConn struct { *utls.UConn + testableHandshake func() error } -// newConnUTLS creates a NewConn function creating a utls connection with a specified ClientHelloID +// Ensures that a utlsConn implements the TLSConn interface. +var _ TLSConn = &utlsConn{} + +// newConnUTLS returns a NewConn function for creating utlsConn instances. func newConnUTLS(clientHello *utls.ClientHelloID) func(conn net.Conn, config *tls.Config) TLSConn { return func(conn net.Conn, config *tls.Config) TLSConn { uConfig := &utls.Config{ @@ -34,10 +39,30 @@ func newConnUTLS(clientHello *utls.ClientHelloID) func(conn net.Conn, config *tl DynamicRecordSizingDisabled: config.DynamicRecordSizingDisabled, } tlsConn := utls.UClient(conn, uConfig, *clientHello) - return &utlsConn{tlsConn} + return &utlsConn{UConn: tlsConn} } } +func (c *utlsConn) HandshakeContext(ctx context.Context) error { + errch := make(chan error, 1) + go func() { + errch <- c.handshakefn()() + }() + select { + case err := <-errch: + return err + case <-ctx.Done(): + return ctx.Err() + } +} + +func (c *utlsConn) handshakefn() func() error { + if c.testableHandshake != nil { + return c.testableHandshake + } + return c.UConn.Handshake +} + func (c *utlsConn) ConnectionState() tls.ConnectionState { uState := c.Conn.ConnectionState() return tls.ConnectionState{ diff --git a/internal/netxlite/utls_test.go b/internal/netxlite/utls_test.go index 3fe01ce..d62c24c 100644 --- a/internal/netxlite/utls_test.go +++ b/internal/netxlite/utls_test.go @@ -3,8 +3,11 @@ package netxlite import ( "context" "crypto/tls" + "errors" "net" + "sync" "testing" + "time" "github.com/apex/log" utls "gitlab.com/yawning/utls.git" @@ -45,3 +48,37 @@ func TestNewTLSHandshakerUTLSTypes(t *testing.T) { t.Fatal("expected non-nil NewConn") } } + +func TestUTLSConnHandshakeNotInterrupted(t *testing.T) { + ctx := context.Background() + conn := &utlsConn{ + testableHandshake: func() error { + return nil + }, + } + err := conn.HandshakeContext(ctx) + if err != nil { + t.Fatal(err) + } +} + +func TestUTLSConnHandshakeInterrupted(t *testing.T) { + wg := sync.WaitGroup{} + wg.Add(1) + sigch := make(chan interface{}) + ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond) + defer cancel() + conn := &utlsConn{ + testableHandshake: func() error { + defer wg.Done() + <-sigch + return nil + }, + } + err := conn.HandshakeContext(ctx) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatal("not the error we expected", err) + } + close(sigch) + wg.Wait() +}