From e8369e54ac9774c981c97d21288c5a34a21fed6e Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Dec 2023 10:43:10 +0100 Subject: [PATCH 01/12] feat: avoid referencing oohttp.TLSConn directly --- internal/feature/oocryptofeat/tlsconn.go | 9 +++++++ internal/model/netx.go | 30 +++++++++++++++++------- 2 files changed, 30 insertions(+), 9 deletions(-) create mode 100644 internal/feature/oocryptofeat/tlsconn.go diff --git a/internal/feature/oocryptofeat/tlsconn.go b/internal/feature/oocryptofeat/tlsconn.go new file mode 100644 index 0000000000..19ec72d179 --- /dev/null +++ b/internal/feature/oocryptofeat/tlsconn.go @@ -0,0 +1,9 @@ +package oocryptofeat + +import ( + oohttp "github.com/ooni/oohttp" + "github.com/ooni/probe-cli/v3/internal/model" +) + +// Make sure the two models are compatible with each other. +var _ model.TLSConn = oohttp.TLSConn(nil) diff --git a/internal/model/netx.go b/internal/model/netx.go index f6f4bc78ff..04b6253f41 100644 --- a/internal/model/netx.go +++ b/internal/model/netx.go @@ -13,7 +13,6 @@ import ( "syscall" "time" - oohttp "github.com/ooni/oohttp" "github.com/quic-go/quic-go" utls "gitlab.com/yawning/utls.git" ) @@ -309,12 +308,25 @@ type Resolver interface { LookupNS(ctx context.Context, domain string) ([]*net.NS, error) } -// TLSConn is the type of connection that oohttp expects from -// any library that implements TLS functionality. By using this -// kind of TLSConn we're able to use both the standard library -// and gitlab.com/yawning/utls.git to perform TLS operations. Note -// that the stdlib's tls.Conn implements this interface. -type TLSConn = oohttp.TLSConn +// TLSConn is the interface representing a *tls.Conn compatible +// connection, which could possibly be different from a *tls.Conn +// as long as it implements the interface. You can use, for +// example, refraction-networking/utls instead of the stdlib. +type TLSConn interface { + // net.Conn is the underlying interface + net.Conn + + // ConnectionState returns the ConnectionState according + // to the standard library. + ConnectionState() tls.ConnectionState + + // HandshakeContext performs an TLS handshake bounded + // in time by the given context. + HandshakeContext(ctx context.Context) error + + // NetConn returns the underlying net.Conn + NetConn() net.Conn +} // Ensures that a [*tls.Conn] implements the [TLSConn] interface. var _ TLSConn = &tls.Conn{} @@ -325,7 +337,7 @@ type TLSDialer interface { CloseIdleConnections() // DialTLSContext dials a TLS connection. This method will always return - // to you a oohttp.TLSConn, so you can always safely cast to it. + // to you a TLSConn, so you can always safely cast to it. // // The endpoint is an endpoint like the ones accepted by [net.DialContext]. For example, // x.org:443, 130.192.91.211:443 and [::1]:443. Note that IPv6 addrs are quoted. @@ -357,7 +369,7 @@ type TLSHandshaker interface { // Trace allows to collect measurement traces. A trace is injected into // netx operations using context.WithValue. Netx code retrieves the trace -// using context.Value. See docs/design/dd-003-step-by-step.md for the +// using context.Value. See the docs/design/dd-003-step-by-step.md for the // design document explaining why we implemented context-based tracing. type Trace interface { // TimeNow returns the current time. Normally, this should be the same From 2007b75ee5f9b7d3d1e0fb9a02e06dfaa717dc00 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Dec 2023 11:36:42 +0100 Subject: [PATCH 02/12] feat: make oohttp optional --- internal/feature/oocryptofeat/tlsconn.go | 9 -- internal/feature/oohttpfeat/oohttp_enabled.go | 110 ++++++++++++++++++ .../feature/oohttpfeat/oohttp_otherwise.go | 3 + internal/netxlite/httpfactory.go | 46 +++++--- internal/netxlite/httpfactory_test.go | 51 +++----- internal/netxlite/httpquirks.go | 29 +++-- internal/netxlite/httpquirks_test.go | 12 +- internal/netxlite/httpstdlib.go | 4 +- 8 files changed, 180 insertions(+), 84 deletions(-) delete mode 100644 internal/feature/oocryptofeat/tlsconn.go create mode 100644 internal/feature/oohttpfeat/oohttp_enabled.go create mode 100644 internal/feature/oohttpfeat/oohttp_otherwise.go diff --git a/internal/feature/oocryptofeat/tlsconn.go b/internal/feature/oocryptofeat/tlsconn.go deleted file mode 100644 index 19ec72d179..0000000000 --- a/internal/feature/oocryptofeat/tlsconn.go +++ /dev/null @@ -1,9 +0,0 @@ -package oocryptofeat - -import ( - oohttp "github.com/ooni/oohttp" - "github.com/ooni/probe-cli/v3/internal/model" -) - -// Make sure the two models are compatible with each other. -var _ model.TLSConn = oohttp.TLSConn(nil) diff --git a/internal/feature/oohttpfeat/oohttp_enabled.go b/internal/feature/oohttpfeat/oohttp_enabled.go new file mode 100644 index 0000000000..075460f646 --- /dev/null +++ b/internal/feature/oohttpfeat/oohttp_enabled.go @@ -0,0 +1,110 @@ +//go:build go1.20 + +package oohttpfeat + +import ( + "context" + "crypto/tls" + "net" + "net/http" + "net/url" + + oohttp "github.com/ooni/oohttp" + "github.com/ooni/probe-cli/v3/internal/model" +) + +// Make sure the two models are compatible with each other. +var _ model.TLSConn = oohttp.TLSConn(nil) + +// HTTPTransport is a wrapper for either net/http or oohttp's Transport. +type HTTPTransport struct { + txp *oohttp.StdlibTransport +} + +// HTTPRequest is the type of the underlying *http.Request we're using in this library, which +// in turns depends on how we're being compiled. +type HTTPRequest = oohttp.Request + +// ExpectedForceAttemptHTTP2 is the expected value returned by GetForceAttemptHTTP2. +const ExpectedForceAttemptHTTP2 = true + +// NewHTTPTransport creates a new [*HTTPTransport] instance. +func NewHTTPTransport() *HTTPTransport { + txp := &HTTPTransport{&oohttp.StdlibTransport{Transport: oohttp.DefaultTransport.(*oohttp.Transport).Clone()}} + + // When we're using our net/http fork, we know we're able to + // actually force HTTP/2 without any issue + txp.txp.ForceAttemptHTTP2 = ExpectedForceAttemptHTTP2 + + return txp +} + +// SetDialContext sets the DialContext field. +func (txp *HTTPTransport) SetDialContext(fx func(ctx context.Context, network, address string) (net.Conn, error)) { + txp.txp.DialContext = fx +} + +// GetDialContext returns the value of the DialContext field. +func (txp *HTTPTransport) GetDialContext() func(ctx context.Context, network, address string) (net.Conn, error) { + return txp.txp.DialContext +} + +// SetDialTLSContext sets the DialTLSContext field. +func (txp *HTTPTransport) SetDialTLSContext(fx func(ctx context.Context, network, address string) (net.Conn, error)) { + txp.txp.DialTLSContext = fx +} + +// GetDialTLSContext returns the value of the DialTLSContext field. +func (txp *HTTPTransport) GetDialTLSContext() func(ctx context.Context, network, address string) (net.Conn, error) { + return txp.txp.DialTLSContext +} + +// SetProxy sets the Proxy field. +func (txp *HTTPTransport) SetProxy(fx func(*HTTPRequest) (*url.URL, error)) { + txp.txp.Proxy = fx +} + +// GetProxy returns the value of the Proxy field. +func (txp *HTTPTransport) GetProxy() func(*HTTPRequest) (*url.URL, error) { + return txp.txp.Proxy +} + +// SetMaxConnsPerHost sets the MaxConnsPerHost field. +func (txp *HTTPTransport) SetMaxConnsPerHost(value int) { + txp.txp.MaxConnsPerHost = value +} + +// GetMaxConnsPerHost returns the value of the MaxConnsPerHost field. +func (txp *HTTPTransport) GetMaxConnsPerHost() int { + return txp.txp.MaxConnsPerHost +} + +// SetDisableCompression sets the DisableCompression field. +func (txp *HTTPTransport) SetDisableCompression(value bool) { + txp.txp.DisableCompression = value +} + +// GetDisableCompression returns the value of the DisableCompression field. +func (txp *HTTPTransport) GetDisableCompression() bool { + return txp.txp.DisableCompression +} + +// SetTLSClientConfig sets the TLSClientConfig field. +func (txp *HTTPTransport) SetTLSClientConfig(value *tls.Config) { + txp.txp.TLSClientConfig = value +} + +// GetForceAttemptHTTP2 returns the value of the ForceAttemptHTTP2 field. +func (txp *HTTPTransport) GetForceAttemptHTTP2() bool { + return txp.txp.ForceAttemptHTTP2 +} + +// CloseIdleConnections closes the idle connections. +func (txp *HTTPTransport) CloseIdleConnections() { + txp.txp.CloseIdleConnections() +} + +// RoundTrip performs an HTTP round trip. +func (txp *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { + return txp.txp.RoundTrip(req) +} diff --git a/internal/feature/oohttpfeat/oohttp_otherwise.go b/internal/feature/oohttpfeat/oohttp_otherwise.go new file mode 100644 index 0000000000..0d6a00007f --- /dev/null +++ b/internal/feature/oohttpfeat/oohttp_otherwise.go @@ -0,0 +1,3 @@ +//go:build go1.21 || ooni_feature_disable_ooohttp + +package oohttpfeat diff --git a/internal/netxlite/httpfactory.go b/internal/netxlite/httpfactory.go index 0e57f249fe..e95db5822a 100644 --- a/internal/netxlite/httpfactory.go +++ b/internal/netxlite/httpfactory.go @@ -4,12 +4,12 @@ import ( "crypto/tls" "net/url" - oohttp "github.com/ooni/oohttp" + "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" "github.com/ooni/probe-cli/v3/internal/model" ) // HTTPTransportOption is an initialization option for [NewHTTPTransport]. -type HTTPTransportOption func(txp *oohttp.Transport) +type HTTPTransportOption func(txp *oohttpfeat.HTTPTransport) // NewHTTPTransport is the high-level factory to create a [model.HTTPTransport] using // github.com/ooni/oohttp as the HTTP library with HTTP/1.1 and HTTP2 support. @@ -41,20 +41,28 @@ type HTTPTransportOption func(txp *oohttp.Transport) // This factory is the RECOMMENDED way of creating a [model.HTTPTransport]. func NewHTTPTransportWithOptions(logger model.Logger, dialer model.Dialer, tlsDialer model.TLSDialer, options ...HTTPTransportOption) model.HTTPTransport { - // Using oohttp to support any TLS library. - txp := oohttp.DefaultTransport.(*oohttp.Transport).Clone() + // Using oohttp to support any TLS library iff it's possible to do so, otherwise we + // are going to use the standard library w/o using HTTP/2 support. + txp := oohttpfeat.NewHTTPTransport() // This wrapping ensures that we always have a timeout when we // are using HTTP; see https://github.com/ooni/probe/issues/1609. dialer = &httpDialerWithReadTimeout{dialer} - txp.DialContext = dialer.DialContext + txp.SetDialContext(dialer.DialContext) tlsDialer = &httpTLSDialerWithReadTimeout{tlsDialer} - txp.DialTLSContext = tlsDialer.DialTLSContext + txp.SetDialTLSContext(tlsDialer.DialTLSContext) // As documented, disable proxies and force HTTP/2 - txp.DisableCompression = true - txp.Proxy = nil - txp.ForceAttemptHTTP2 = true + txp.SetDisableCompression(true) + txp.SetProxy(nil) + + // We now rely on feature/oohttpfeat to decide whether it's + // possible for us to actually enable HTTP/2. + // + // txp.ForceAttemptHTTP2 = true + // + // Please, keep this comment until at least 3.21 because it provides + // an historical documentation of how we changed the codebase. // Apply all the required options for _, option := range options { @@ -63,7 +71,7 @@ func NewHTTPTransportWithOptions(logger model.Logger, // Return a fully wrapped HTTP transport return WrapHTTPTransport(logger, &httpTransportConnectionsCloser{ - HTTPTransport: &httpTransportStdlib{&oohttp.StdlibTransport{Transport: txp}}, + HTTPTransport: &httpTransportStdlib{txp}, Dialer: dialer, TLSDialer: tlsDialer, }) @@ -72,27 +80,27 @@ func NewHTTPTransportWithOptions(logger model.Logger, // HTTPTransportOptionProxyURL configures the transport to use the given proxyURL // or disables proxying (already the default) if the proxyURL is nil. func HTTPTransportOptionProxyURL(proxyURL *url.URL) HTTPTransportOption { - return func(txp *oohttp.Transport) { - txp.Proxy = func(r *oohttp.Request) (*url.URL, error) { + return func(txp *oohttpfeat.HTTPTransport) { + txp.SetProxy(func(r *oohttpfeat.HTTPRequest) (*url.URL, error) { // "If Proxy is nil or returns a nil *URL, no proxy is used." return proxyURL, nil - } + }) } } // HTTPTransportOptionMaxConnsPerHost configures the .MaxConnPerHosts field, which // otherwise uses the default set in github.com/ooni/oohttp. func HTTPTransportOptionMaxConnsPerHost(value int) HTTPTransportOption { - return func(txp *oohttp.Transport) { - txp.MaxConnsPerHost = value + return func(txp *oohttpfeat.HTTPTransport) { + txp.SetMaxConnsPerHost(value) } } // HTTPTransportOptionDisableCompression configures the .DisableCompression field, which // otherwise is set to true, so that this code is ready for measuring out of the box. func HTTPTransportOptionDisableCompression(value bool) HTTPTransportOption { - return func(txp *oohttp.Transport) { - txp.DisableCompression = value + return func(txp *oohttpfeat.HTTPTransport) { + txp.SetDisableCompression(value) } } @@ -104,7 +112,7 @@ func HTTPTransportOptionDisableCompression(value bool) HTTPTransportOption { // this limitation. Future releases MIGHT use a different technique and, as such, // we MAY remove this option when we don't need it anymore. func HTTPTransportOptionTLSClientConfig(config *tls.Config) HTTPTransportOption { - return func(txp *oohttp.Transport) { - txp.TLSClientConfig = config + return func(txp *oohttpfeat.HTTPTransport) { + txp.SetTLSClientConfig(config) } } diff --git a/internal/netxlite/httpfactory_test.go b/internal/netxlite/httpfactory_test.go index c02955c11c..88027f6056 100644 --- a/internal/netxlite/httpfactory_test.go +++ b/internal/netxlite/httpfactory_test.go @@ -1,13 +1,10 @@ package netxlite import ( - "crypto/tls" "net/url" "testing" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" - oohttp "github.com/ooni/oohttp" + "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -42,54 +39,33 @@ func TestNewHTTPTransportWithOptions(t *testing.T) { // make sure there's the stdlib adapter stdlibAdapter := txpCloser.HTTPTransport.(*httpTransportStdlib) - oohttpStdlibAdapter := stdlibAdapter.StdlibTransport - underlying := oohttpStdlibAdapter.Transport - - // now let's check that everything is configured as intended - expectedTxp := oohttp.DefaultTransport.(*oohttp.Transport).Clone() - diff := cmp.Diff( - expectedTxp, - underlying, - cmpopts.IgnoreUnexported(oohttp.Transport{}), - cmpopts.IgnoreUnexported(tls.Config{}), - cmpopts.IgnoreFields( - oohttp.Transport{}, - "DialContext", - "DialTLSContext", - "DisableCompression", - "Proxy", - "ForceAttemptHTTP2", - ), - ) - if diff != "" { - t.Fatal(diff) - } + underlying := stdlibAdapter.StdlibTransport // finish checking by explicitly inspecting the fields we modify - if underlying.DialContext == nil { + if underlying.GetDialContext() == nil { t.Fatal("expected non-nil .DialContext") } - if underlying.DialTLSContext == nil { + if underlying.GetDialTLSContext() == nil { t.Fatal("expected non-nil .DialTLSContext") } - if underlying.Proxy != nil { + if underlying.GetProxy() != nil { t.Fatal("expected nil .Proxy") } - if !underlying.ForceAttemptHTTP2 { + if underlying.GetForceAttemptHTTP2() != oohttpfeat.ExpectedForceAttemptHTTP2 { t.Fatal("expected true .ForceAttemptHTTP2") } - if !underlying.DisableCompression { + if !underlying.GetDisableCompression() { t.Fatal("expected true .DisableCompression") } }) - unwrap := func(txp model.HTTPTransport) *oohttp.Transport { + unwrap := func(txp model.HTTPTransport) *oohttpfeat.HTTPTransport { txpLogger := txp.(*httpTransportLogger) txpErrWrapper := txpLogger.HTTPTransport.(*httpTransportErrWrapper) txpCloser := txpErrWrapper.HTTPTransport.(*httpTransportConnectionsCloser) stdlibAdapter := txpCloser.HTTPTransport.(*httpTransportStdlib) oohttpStdlibAdapter := stdlibAdapter.StdlibTransport - return oohttpStdlibAdapter.Transport + return oohttpStdlibAdapter } t.Run("make sure HTTPTransportOptionProxyURL is WAI", func(t *testing.T) { @@ -104,10 +80,11 @@ func TestNewHTTPTransportWithOptions(t *testing.T) { HTTPTransportOptionProxyURL(expectedURL), ) underlying := unwrap(txp) - if underlying.Proxy == nil { + proxy := underlying.GetProxy() + if proxy == nil { t.Fatal("expected non-nil .Proxy") } - got, err := underlying.Proxy(&oohttp.Request{}) + got, err := proxy(&oohttpfeat.HTTPRequest{}) if err != nil { t.Fatal(err) } @@ -133,7 +110,7 @@ func TestNewHTTPTransportWithOptions(t *testing.T) { HTTPTransportOptionMaxConnsPerHost(expectedValue), ) underlying := unwrap(txp) - got := underlying.MaxConnsPerHost + got := underlying.GetMaxConnsPerHost() if got != expectedValue { t.Fatal("not the expected value") } @@ -156,7 +133,7 @@ func TestNewHTTPTransportWithOptions(t *testing.T) { HTTPTransportOptionDisableCompression(expectedValue), ) underlying := unwrap(txp) - got := underlying.DisableCompression + got := underlying.GetDisableCompression() if got != expectedValue { t.Fatal("not the expected value") } diff --git a/internal/netxlite/httpquirks.go b/internal/netxlite/httpquirks.go index d4c26945e0..a596d6c77f 100644 --- a/internal/netxlite/httpquirks.go +++ b/internal/netxlite/httpquirks.go @@ -11,7 +11,7 @@ package netxlite import ( "net/http" - oohttp "github.com/ooni/oohttp" + "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -60,37 +60,42 @@ func NewHTTPTransport(logger model.DebugLogger, dialer model.Dialer, tlsDialer m // // This function behavior is QUIRKY as documented in [NewHTTPTransport]. func newOOHTTPBaseTransport(dialer model.Dialer, tlsDialer model.TLSDialer) model.HTTPTransport { - // Using oohttp to support any TLS library. - txp := oohttp.DefaultTransport.(*oohttp.Transport).Clone() + // Using oohttp to support any TLS library iff it's possible to do so, otherwise we + // are going to use the standard library w/o using HTTP/2 support. + txp := oohttpfeat.NewHTTPTransport() // This wrapping ensures that we always have a timeout when we // are using HTTP; see https://github.com/ooni/probe/issues/1609. dialer = &httpDialerWithReadTimeout{dialer} - txp.DialContext = dialer.DialContext + txp.SetDialContext(dialer.DialContext) tlsDialer = &httpTLSDialerWithReadTimeout{tlsDialer} - txp.DialTLSContext = tlsDialer.DialTLSContext + txp.SetDialTLSContext(tlsDialer.DialTLSContext) // We are using a different strategy to implement proxy: we // use a specific dialer that knows about proxying. - txp.Proxy = nil + txp.SetProxy(nil) // Better for Cloudflare DNS and also better because we have less // noisy events and we can better understand what happened. - txp.MaxConnsPerHost = 1 + txp.SetMaxConnsPerHost(1) // The following (1) reduces the number of headers that Go will // automatically send for us and (2) ensures that we always receive // back the true headers, such as Content-Length. This change is // functional to OONI's goal of observing the network. - txp.DisableCompression = true + txp.SetDisableCompression(true) - // Required to enable using HTTP/2 (which will be anyway forced - // upon us when we are using TLS parroting). - txp.ForceAttemptHTTP2 = true + // We now rely on feature/oohttpfeat to decide whether it's + // possible for us to actually enable HTTP/2. + // + // txp.ForceAttemptHTTP2 = true + // + // Please, keep this comment until at least 3.21 because it provides + // an historical documentation of how we changed the codebase. // Ensure we correctly forward CloseIdleConnections. return &httpTransportConnectionsCloser{ - HTTPTransport: &httpTransportStdlib{&oohttp.StdlibTransport{Transport: txp}}, + HTTPTransport: &httpTransportStdlib{txp}, Dialer: dialer, TLSDialer: tlsDialer, } diff --git a/internal/netxlite/httpquirks_test.go b/internal/netxlite/httpquirks_test.go index 3043293e4e..2695a99b06 100644 --- a/internal/netxlite/httpquirks_test.go +++ b/internal/netxlite/httpquirks_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/apex/log" + "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -85,19 +86,20 @@ func TestNewHTTPTransport(t *testing.T) { t.Fatal("invalid tls dialer") } stdlib := connectionsCloser.HTTPTransport.(*httpTransportStdlib) - if !stdlib.StdlibTransport.ForceAttemptHTTP2 { + underlying := stdlib.StdlibTransport + if underlying.GetForceAttemptHTTP2() != oohttpfeat.ExpectedForceAttemptHTTP2 { t.Fatal("invalid ForceAttemptHTTP2") } - if !stdlib.StdlibTransport.DisableCompression { + if !underlying.GetDisableCompression() { t.Fatal("invalid DisableCompression") } - if stdlib.StdlibTransport.MaxConnsPerHost != 1 { + if underlying.GetMaxConnsPerHost() != 1 { t.Fatal("invalid MaxConnPerHost") } - if stdlib.StdlibTransport.DialTLSContext == nil { + if underlying.GetDialTLSContext() == nil { t.Fatal("invalid DialTLSContext") } - if stdlib.StdlibTransport.DialContext == nil { + if underlying.GetDialContext() == nil { t.Fatal("invalid DialContext") } }) diff --git a/internal/netxlite/httpstdlib.go b/internal/netxlite/httpstdlib.go index 2f3063dbf9..5e235cf43e 100644 --- a/internal/netxlite/httpstdlib.go +++ b/internal/netxlite/httpstdlib.go @@ -7,13 +7,13 @@ package netxlite import ( "net/http" - oohttp "github.com/ooni/oohttp" + "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" "github.com/ooni/probe-cli/v3/internal/model" ) // stdlibTransport wraps oohttp.StdlibTransport to add .Network() type httpTransportStdlib struct { - StdlibTransport *oohttp.StdlibTransport + StdlibTransport *oohttpfeat.HTTPTransport } var _ model.HTTPTransport = &httpTransportStdlib{} From 878aa5d2900bfa2f04ad00f698c51488b3027817 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Dec 2023 11:42:35 +0100 Subject: [PATCH 03/12] make sure we can disable ootls --- internal/feature/oohttpfeat/doc.go | 4 ++++ internal/feature/ootlsfeat/doc.go | 6 ++++++ internal/feature/ootlsfeat/ootls_enabled.go | 16 ++++++++++++++++ internal/netxlite/tls.go | 4 ++-- 4 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 internal/feature/oohttpfeat/doc.go create mode 100644 internal/feature/ootlsfeat/doc.go create mode 100644 internal/feature/ootlsfeat/ootls_enabled.go diff --git a/internal/feature/oohttpfeat/doc.go b/internal/feature/oohttpfeat/doc.go new file mode 100644 index 0000000000..b3aa891a5e --- /dev/null +++ b/internal/feature/oohttpfeat/doc.go @@ -0,0 +1,4 @@ +// Package oohttpfeat implements the oohttp feature. When it is possible +// to enable this feature, we will be able to use HTTP/2 with every TLS +// library. Oherwise, HTTP/2 would only work with crypto/tls. +package oohttpfeat diff --git a/internal/feature/ootlsfeat/doc.go b/internal/feature/ootlsfeat/doc.go new file mode 100644 index 0000000000..8bb310f2aa --- /dev/null +++ b/internal/feature/ootlsfeat/doc.go @@ -0,0 +1,6 @@ +// Package oohttpfeat implements the ootls feature. When it is possible +// to enable this feature, we include mitigations that make TLS more robust +// for Android, as documented by the following blog post +// https://ooni.org/post/making-ooni-probe-android-more-resilient/. Otherwise, +// we will just use the standard library. +package ootlsfeat diff --git a/internal/feature/ootlsfeat/ootls_enabled.go b/internal/feature/ootlsfeat/ootls_enabled.go new file mode 100644 index 0000000000..657f3da274 --- /dev/null +++ b/internal/feature/ootlsfeat/ootls_enabled.go @@ -0,0 +1,16 @@ +//go:build go1.20 + +package ootlsfeat + +import ( + "crypto/tls" + "net" + + ootls "github.com/ooni/oocrypto/tls" + "github.com/ooni/probe-cli/v3/internal/model" +) + +// NewClientConnStdlib returns a new client connection using the standard library's TLS stack. +func NewClientConnStdlib(conn net.Conn, config *tls.Config) (model.TLSConn, error) { + return ootls.NewClientConnStdlib(conn, config) +} diff --git a/internal/netxlite/tls.go b/internal/netxlite/tls.go index 14df796eca..80b989b557 100644 --- a/internal/netxlite/tls.go +++ b/internal/netxlite/tls.go @@ -13,7 +13,7 @@ import ( "net" "time" - ootls "github.com/ooni/oocrypto/tls" + "github.com/ooni/probe-cli/v3/internal/feature/ootlsfeat" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -248,7 +248,7 @@ func (h *tlsHandshakerConfigurable) newConn(conn net.Conn, config *tls.Config) ( if h.NewConn != nil { return h.NewConn(conn, config) } - return ootls.NewClientConnStdlib(conn, config) + return ootlsfeat.NewClientConnStdlib(conn, config) } // tlsHandshakerLogger is a TLSHandshaker with logging. From b2e656b55101bc8d98c0b058a21095560aef6033 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Dec 2023 11:52:26 +0100 Subject: [PATCH 04/12] x --- internal/feature/oohttpfeat/oohttp_enabled.go | 2 +- .../feature/oohttpfeat/oohttp_otherwise.go | 103 +++++++++++++++++- internal/feature/ootlsfeat/ootls_enabled.go | 2 +- internal/feature/ootlsfeat/ootls_otherwise.go | 15 +++ 4 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 internal/feature/ootlsfeat/ootls_otherwise.go diff --git a/internal/feature/oohttpfeat/oohttp_enabled.go b/internal/feature/oohttpfeat/oohttp_enabled.go index 075460f646..1d5c2d9da3 100644 --- a/internal/feature/oohttpfeat/oohttp_enabled.go +++ b/internal/feature/oohttpfeat/oohttp_enabled.go @@ -1,4 +1,4 @@ -//go:build go1.20 +//go:build !go1.21 && !ooni_feature_disable_oohttp package oohttpfeat diff --git a/internal/feature/oohttpfeat/oohttp_otherwise.go b/internal/feature/oohttpfeat/oohttp_otherwise.go index 0d6a00007f..f3c1aa4687 100644 --- a/internal/feature/oohttpfeat/oohttp_otherwise.go +++ b/internal/feature/oohttpfeat/oohttp_otherwise.go @@ -1,3 +1,104 @@ -//go:build go1.21 || ooni_feature_disable_ooohttp +//go:build go1.21 || ooni_feature_disable_oohttp package oohttpfeat + +import ( + "context" + "crypto/tls" + "net" + "net/http" + "net/url" +) + +// HTTPTransport is a wrapper for either net/http or oohttp's Transport. +type HTTPTransport struct { + txp *http.Transport +} + +// HTTPRequest is the type of the underlying *http.Request we're using in this library, which +// in turns depends on how we're being compiled. +type HTTPRequest = http.Request + +// ExpectedForceAttemptHTTP2 is the expected value returned by GetForceAttemptHTTP2. +const ExpectedForceAttemptHTTP2 = false + +// NewHTTPTransport creates a new [*HTTPTransport] instance. +func NewHTTPTransport() *HTTPTransport { + txp := &HTTPTransport{http.DefaultTransport.(*http.Transport).Clone()} + + // When we're using our net/http fork, we know we're able to + // actually force HTTP/2 without any issue + txp.txp.ForceAttemptHTTP2 = ExpectedForceAttemptHTTP2 + + return txp +} + +// SetDialContext sets the DialContext field. +func (txp *HTTPTransport) SetDialContext(fx func(ctx context.Context, network, address string) (net.Conn, error)) { + txp.txp.DialContext = fx +} + +// GetDialContext returns the value of the DialContext field. +func (txp *HTTPTransport) GetDialContext() func(ctx context.Context, network, address string) (net.Conn, error) { + return txp.txp.DialContext +} + +// SetDialTLSContext sets the DialTLSContext field. +func (txp *HTTPTransport) SetDialTLSContext(fx func(ctx context.Context, network, address string) (net.Conn, error)) { + txp.txp.DialTLSContext = fx +} + +// GetDialTLSContext returns the value of the DialTLSContext field. +func (txp *HTTPTransport) GetDialTLSContext() func(ctx context.Context, network, address string) (net.Conn, error) { + return txp.txp.DialTLSContext +} + +// SetProxy sets the Proxy field. +func (txp *HTTPTransport) SetProxy(fx func(*HTTPRequest) (*url.URL, error)) { + txp.txp.Proxy = fx +} + +// GetProxy returns the value of the Proxy field. +func (txp *HTTPTransport) GetProxy() func(*HTTPRequest) (*url.URL, error) { + return txp.txp.Proxy +} + +// SetMaxConnsPerHost sets the MaxConnsPerHost field. +func (txp *HTTPTransport) SetMaxConnsPerHost(value int) { + txp.txp.MaxConnsPerHost = value +} + +// GetMaxConnsPerHost returns the value of the MaxConnsPerHost field. +func (txp *HTTPTransport) GetMaxConnsPerHost() int { + return txp.txp.MaxConnsPerHost +} + +// SetDisableCompression sets the DisableCompression field. +func (txp *HTTPTransport) SetDisableCompression(value bool) { + txp.txp.DisableCompression = value +} + +// GetDisableCompression returns the value of the DisableCompression field. +func (txp *HTTPTransport) GetDisableCompression() bool { + return txp.txp.DisableCompression +} + +// SetTLSClientConfig sets the TLSClientConfig field. +func (txp *HTTPTransport) SetTLSClientConfig(value *tls.Config) { + txp.txp.TLSClientConfig = value +} + +// GetForceAttemptHTTP2 returns the value of the ForceAttemptHTTP2 field. +func (txp *HTTPTransport) GetForceAttemptHTTP2() bool { + return txp.txp.ForceAttemptHTTP2 +} + +// CloseIdleConnections closes the idle connections. +func (txp *HTTPTransport) CloseIdleConnections() { + txp.txp.CloseIdleConnections() +} + +// RoundTrip performs an HTTP round trip. +func (txp *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { + return txp.txp.RoundTrip(req) +} diff --git a/internal/feature/ootlsfeat/ootls_enabled.go b/internal/feature/ootlsfeat/ootls_enabled.go index 657f3da274..1289db84aa 100644 --- a/internal/feature/ootlsfeat/ootls_enabled.go +++ b/internal/feature/ootlsfeat/ootls_enabled.go @@ -1,4 +1,4 @@ -//go:build go1.20 +//go:build !go1.21 && !ooni_feature_disable_oohttp package ootlsfeat diff --git a/internal/feature/ootlsfeat/ootls_otherwise.go b/internal/feature/ootlsfeat/ootls_otherwise.go new file mode 100644 index 0000000000..6e2d818217 --- /dev/null +++ b/internal/feature/ootlsfeat/ootls_otherwise.go @@ -0,0 +1,15 @@ +//go:build go1.21 || ooni_feature_disable_oohttp + +package ootlsfeat + +import ( + "crypto/tls" + "net" + + "github.com/ooni/probe-cli/v3/internal/model" +) + +// NewClientConnStdlib returns a new client connection using the standard library's TLS stack. +func NewClientConnStdlib(conn net.Conn, config *tls.Config) (model.TLSConn, error) { + return tls.Client(conn, config), nil +} From f464d2ec96aa4a479657f772116f17f2495d0f8f Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Dec 2023 12:05:32 +0100 Subject: [PATCH 05/12] x --- internal/feature/ootlsfeat/doc.go | 2 +- internal/feature/psiphonfeat/doc.go | 4 ++++ internal/feature/psiphonfeat/psiphon_enabled.go | 15 +++++++++++++++ internal/feature/psiphonfeat/psiphon_otherwise.go | 10 ++++++++++ internal/feature/psiphonfeat/tunnel.go | 15 +++++++++++++++ internal/tunnel/psiphon.go | 11 +++++------ internal/tunnel/psiphon_test.go | 4 ++-- 7 files changed, 52 insertions(+), 9 deletions(-) create mode 100644 internal/feature/psiphonfeat/doc.go create mode 100644 internal/feature/psiphonfeat/psiphon_enabled.go create mode 100644 internal/feature/psiphonfeat/psiphon_otherwise.go create mode 100644 internal/feature/psiphonfeat/tunnel.go diff --git a/internal/feature/ootlsfeat/doc.go b/internal/feature/ootlsfeat/doc.go index 8bb310f2aa..509ea63bc0 100644 --- a/internal/feature/ootlsfeat/doc.go +++ b/internal/feature/ootlsfeat/doc.go @@ -1,4 +1,4 @@ -// Package oohttpfeat implements the ootls feature. When it is possible +// Package ootlsfeat implements the ootls feature. When it is possible // to enable this feature, we include mitigations that make TLS more robust // for Android, as documented by the following blog post // https://ooni.org/post/making-ooni-probe-android-more-resilient/. Otherwise, diff --git a/internal/feature/psiphonfeat/doc.go b/internal/feature/psiphonfeat/doc.go new file mode 100644 index 0000000000..2c63f0f937 --- /dev/null +++ b/internal/feature/psiphonfeat/doc.go @@ -0,0 +1,4 @@ +// Package psiphonfeat implements the psiphon feature. When it is possible +// to enable this feature, we are able to start psiphon tunnels. Otherwise, it's +// not possible to start psiphon tunnels. +package psiphonfeat diff --git a/internal/feature/psiphonfeat/psiphon_enabled.go b/internal/feature/psiphonfeat/psiphon_enabled.go new file mode 100644 index 0000000000..7cd1579312 --- /dev/null +++ b/internal/feature/psiphonfeat/psiphon_enabled.go @@ -0,0 +1,15 @@ +//go:build !go1.21 && !ooni_feature_disable_psiphon + +package psiphonfeat + +import ( + "context" + + "github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib" +) + +// Start attempts to start the Psiphon tunnel and returns either a Tunnel or an error. +func Start(ctx context.Context, config []byte, workdir string) (Tunnel, error) { + return clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{ + DataRootDirectory: &workdir}, nil, nil) +} diff --git a/internal/feature/psiphonfeat/psiphon_otherwise.go b/internal/feature/psiphonfeat/psiphon_otherwise.go new file mode 100644 index 0000000000..4791ac8b4e --- /dev/null +++ b/internal/feature/psiphonfeat/psiphon_otherwise.go @@ -0,0 +1,10 @@ +//go:build go1.21 || ooni_feature_disable_psiphon + +package psiphonfeat + +import "context" + +// Start attempts to start the Psiphon tunnel and returns either a Tunnel or an error. +func Start(ctx context.Context, config []byte, workdir string) (Tunnel, error) { + return nil, ErrFeatureNotEnabled +} diff --git a/internal/feature/psiphonfeat/tunnel.go b/internal/feature/psiphonfeat/tunnel.go new file mode 100644 index 0000000000..f7a339d115 --- /dev/null +++ b/internal/feature/psiphonfeat/tunnel.go @@ -0,0 +1,15 @@ +package psiphonfeat + +import "errors" + +// Tunnel is the interface implementing the Psiphon tunnel. +type Tunnel interface { + // Stop stops a running Psiphon tunnel. + Stop() + + // GetSOCKSProxyPort returns the SOCKS5 port used by the tunnel. + GetSOCKSProxyPort() int +} + +// ErrFeatureNotEnabled indicates that the Psiphon feature is not enabled in this build. +var ErrFeatureNotEnabled = errors.New("psiphonfeat: not enabled") diff --git a/internal/tunnel/psiphon.go b/internal/tunnel/psiphon.go index 322668df74..62c58d4470 100644 --- a/internal/tunnel/psiphon.go +++ b/internal/tunnel/psiphon.go @@ -8,14 +8,13 @@ import ( "path/filepath" "time" - "github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib" + "github.com/ooni/probe-cli/v3/internal/feature/psiphonfeat" ) // mockableStartPsiphon allows us to test for psiphon startup failures. var mockableStartPsiphon = func( - ctx context.Context, config []byte, workdir string) (*clientlib.PsiphonTunnel, error) { - return clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{ - DataRootDirectory: &workdir}, nil, nil) + ctx context.Context, config []byte, workdir string) (psiphonfeat.Tunnel, error) { + return psiphonfeat.Start(ctx, config, workdir) } // psiphonTunnel is a psiphon tunnel @@ -24,7 +23,7 @@ type psiphonTunnel struct { bootstrapTime time.Duration // tunnel is the underlying psiphon tunnel - tunnel *clientlib.PsiphonTunnel + tunnel psiphonfeat.Tunnel } // psiphonMakeWorkingDir creates the working directory @@ -81,7 +80,7 @@ func (t *psiphonTunnel) SOCKS5ProxyURL() *url.URL { return &url.URL{ Scheme: "socks5", Host: net.JoinHostPort( - "127.0.0.1", fmt.Sprintf("%d", t.tunnel.SOCKSProxyPort)), + "127.0.0.1", fmt.Sprintf("%d", t.tunnel.GetSOCKSProxyPort())), } } diff --git a/internal/tunnel/psiphon_test.go b/internal/tunnel/psiphon_test.go index 1573a837bd..b39210d210 100644 --- a/internal/tunnel/psiphon_test.go +++ b/internal/tunnel/psiphon_test.go @@ -6,7 +6,7 @@ import ( "os" "testing" - "github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib" + "github.com/ooni/probe-cli/v3/internal/feature/psiphonfeat" ) func TestPsiphonWithCancelledContext(t *testing.T) { @@ -87,7 +87,7 @@ func TestPsiphonStartFailure(t *testing.T) { mockableStartPsiphon = oldStartPsiphon }() mockableStartPsiphon = func(ctx context.Context, config []byte, - workdir string) (*clientlib.PsiphonTunnel, error) { + workdir string) (psiphonfeat.Tunnel, error) { return nil, expected } tunnel, _, err := psiphonStart(context.Background(), &Config{ From f962d7f7c559f6b8dcc8614c02282d542528cb76 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Dec 2023 12:08:45 +0100 Subject: [PATCH 06/12] run tests using go1.21 --- .github/workflows/go1.21.yml | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 .github/workflows/go1.21.yml diff --git a/.github/workflows/go1.21.yml b/.github/workflows/go1.21.yml new file mode 100644 index 0000000000..40fcc7192f --- /dev/null +++ b/.github/workflows/go1.21.yml @@ -0,0 +1,22 @@ +# Runs the whole test suite using go1.21 +name: alltests +on: + pull_request: + push: + branches: + - "release/**" + - "fullbuild" + - "alltestsbuild" + +jobs: + test: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v3 + + - uses: magnetikonline/action-golang-cache@v4 + with: + go-version: ~1.21 + cache-key-suffix: "-alltests-go1.21" + + - run: go test -race -tags shaping ./... From e3447a3dcc791d071441765d9ab063bae1ff388d Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Dec 2023 12:11:59 +0100 Subject: [PATCH 07/12] x --- .github/workflows/go1.21.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go1.21.yml b/.github/workflows/go1.21.yml index 40fcc7192f..6e2a6caae3 100644 --- a/.github/workflows/go1.21.yml +++ b/.github/workflows/go1.21.yml @@ -1,5 +1,5 @@ # Runs the whole test suite using go1.21 -name: alltests +name: alltests-go1.21 on: pull_request: push: From bd0129622d9dae7d01346cf9e6900f02970e7251 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 12 Dec 2023 22:08:18 +0100 Subject: [PATCH 08/12] fixxx --- internal/feature/psiphonfeat/psiphon_enabled.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/internal/feature/psiphonfeat/psiphon_enabled.go b/internal/feature/psiphonfeat/psiphon_enabled.go index 7cd1579312..6791deb59f 100644 --- a/internal/feature/psiphonfeat/psiphon_enabled.go +++ b/internal/feature/psiphonfeat/psiphon_enabled.go @@ -10,6 +10,19 @@ import ( // Start attempts to start the Psiphon tunnel and returns either a Tunnel or an error. func Start(ctx context.Context, config []byte, workdir string) (Tunnel, error) { - return clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{ + tun, err := clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{ DataRootDirectory: &workdir}, nil, nil) + if err != nil { + return nil, err + } + return &tunnel{tun}, nil +} + +type tunnel struct { + *clientlib.PsiphonTunnel +} + +// GetSOCKSProxyPort implements Tunnel. +func (t *tunnel) GetSOCKSProxyPort() int { + return t.SOCKSProxyPort } From e39e6ef13834ab3f0dd74e89dc4900de2ba09d03 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 13 Dec 2023 10:23:43 +0100 Subject: [PATCH 09/12] x --- internal/feature/oohttpfeat/doc.go | 4 - internal/feature/oohttpfeat/oohttp_enabled.go | 110 ------------------ .../feature/oohttpfeat/oohttp_otherwise.go | 104 ----------------- internal/feature/ootlsfeat/doc.go | 6 - internal/feature/ootlsfeat/ootls_enabled.go | 16 --- internal/feature/ootlsfeat/ootls_otherwise.go | 15 --- internal/model/netx.go | 30 ++--- internal/netxlite/httpfactory.go | 46 +++----- internal/netxlite/httpfactory_test.go | 51 +++++--- internal/netxlite/httpquirks.go | 29 ++--- internal/netxlite/httpquirks_test.go | 12 +- internal/netxlite/httpstdlib.go | 4 +- internal/netxlite/tls.go | 4 +- 13 files changed, 86 insertions(+), 345 deletions(-) delete mode 100644 internal/feature/oohttpfeat/doc.go delete mode 100644 internal/feature/oohttpfeat/oohttp_enabled.go delete mode 100644 internal/feature/oohttpfeat/oohttp_otherwise.go delete mode 100644 internal/feature/ootlsfeat/doc.go delete mode 100644 internal/feature/ootlsfeat/ootls_enabled.go delete mode 100644 internal/feature/ootlsfeat/ootls_otherwise.go diff --git a/internal/feature/oohttpfeat/doc.go b/internal/feature/oohttpfeat/doc.go deleted file mode 100644 index b3aa891a5e..0000000000 --- a/internal/feature/oohttpfeat/doc.go +++ /dev/null @@ -1,4 +0,0 @@ -// Package oohttpfeat implements the oohttp feature. When it is possible -// to enable this feature, we will be able to use HTTP/2 with every TLS -// library. Oherwise, HTTP/2 would only work with crypto/tls. -package oohttpfeat diff --git a/internal/feature/oohttpfeat/oohttp_enabled.go b/internal/feature/oohttpfeat/oohttp_enabled.go deleted file mode 100644 index 1d5c2d9da3..0000000000 --- a/internal/feature/oohttpfeat/oohttp_enabled.go +++ /dev/null @@ -1,110 +0,0 @@ -//go:build !go1.21 && !ooni_feature_disable_oohttp - -package oohttpfeat - -import ( - "context" - "crypto/tls" - "net" - "net/http" - "net/url" - - oohttp "github.com/ooni/oohttp" - "github.com/ooni/probe-cli/v3/internal/model" -) - -// Make sure the two models are compatible with each other. -var _ model.TLSConn = oohttp.TLSConn(nil) - -// HTTPTransport is a wrapper for either net/http or oohttp's Transport. -type HTTPTransport struct { - txp *oohttp.StdlibTransport -} - -// HTTPRequest is the type of the underlying *http.Request we're using in this library, which -// in turns depends on how we're being compiled. -type HTTPRequest = oohttp.Request - -// ExpectedForceAttemptHTTP2 is the expected value returned by GetForceAttemptHTTP2. -const ExpectedForceAttemptHTTP2 = true - -// NewHTTPTransport creates a new [*HTTPTransport] instance. -func NewHTTPTransport() *HTTPTransport { - txp := &HTTPTransport{&oohttp.StdlibTransport{Transport: oohttp.DefaultTransport.(*oohttp.Transport).Clone()}} - - // When we're using our net/http fork, we know we're able to - // actually force HTTP/2 without any issue - txp.txp.ForceAttemptHTTP2 = ExpectedForceAttemptHTTP2 - - return txp -} - -// SetDialContext sets the DialContext field. -func (txp *HTTPTransport) SetDialContext(fx func(ctx context.Context, network, address string) (net.Conn, error)) { - txp.txp.DialContext = fx -} - -// GetDialContext returns the value of the DialContext field. -func (txp *HTTPTransport) GetDialContext() func(ctx context.Context, network, address string) (net.Conn, error) { - return txp.txp.DialContext -} - -// SetDialTLSContext sets the DialTLSContext field. -func (txp *HTTPTransport) SetDialTLSContext(fx func(ctx context.Context, network, address string) (net.Conn, error)) { - txp.txp.DialTLSContext = fx -} - -// GetDialTLSContext returns the value of the DialTLSContext field. -func (txp *HTTPTransport) GetDialTLSContext() func(ctx context.Context, network, address string) (net.Conn, error) { - return txp.txp.DialTLSContext -} - -// SetProxy sets the Proxy field. -func (txp *HTTPTransport) SetProxy(fx func(*HTTPRequest) (*url.URL, error)) { - txp.txp.Proxy = fx -} - -// GetProxy returns the value of the Proxy field. -func (txp *HTTPTransport) GetProxy() func(*HTTPRequest) (*url.URL, error) { - return txp.txp.Proxy -} - -// SetMaxConnsPerHost sets the MaxConnsPerHost field. -func (txp *HTTPTransport) SetMaxConnsPerHost(value int) { - txp.txp.MaxConnsPerHost = value -} - -// GetMaxConnsPerHost returns the value of the MaxConnsPerHost field. -func (txp *HTTPTransport) GetMaxConnsPerHost() int { - return txp.txp.MaxConnsPerHost -} - -// SetDisableCompression sets the DisableCompression field. -func (txp *HTTPTransport) SetDisableCompression(value bool) { - txp.txp.DisableCompression = value -} - -// GetDisableCompression returns the value of the DisableCompression field. -func (txp *HTTPTransport) GetDisableCompression() bool { - return txp.txp.DisableCompression -} - -// SetTLSClientConfig sets the TLSClientConfig field. -func (txp *HTTPTransport) SetTLSClientConfig(value *tls.Config) { - txp.txp.TLSClientConfig = value -} - -// GetForceAttemptHTTP2 returns the value of the ForceAttemptHTTP2 field. -func (txp *HTTPTransport) GetForceAttemptHTTP2() bool { - return txp.txp.ForceAttemptHTTP2 -} - -// CloseIdleConnections closes the idle connections. -func (txp *HTTPTransport) CloseIdleConnections() { - txp.txp.CloseIdleConnections() -} - -// RoundTrip performs an HTTP round trip. -func (txp *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { - return txp.txp.RoundTrip(req) -} diff --git a/internal/feature/oohttpfeat/oohttp_otherwise.go b/internal/feature/oohttpfeat/oohttp_otherwise.go deleted file mode 100644 index f3c1aa4687..0000000000 --- a/internal/feature/oohttpfeat/oohttp_otherwise.go +++ /dev/null @@ -1,104 +0,0 @@ -//go:build go1.21 || ooni_feature_disable_oohttp - -package oohttpfeat - -import ( - "context" - "crypto/tls" - "net" - "net/http" - "net/url" -) - -// HTTPTransport is a wrapper for either net/http or oohttp's Transport. -type HTTPTransport struct { - txp *http.Transport -} - -// HTTPRequest is the type of the underlying *http.Request we're using in this library, which -// in turns depends on how we're being compiled. -type HTTPRequest = http.Request - -// ExpectedForceAttemptHTTP2 is the expected value returned by GetForceAttemptHTTP2. -const ExpectedForceAttemptHTTP2 = false - -// NewHTTPTransport creates a new [*HTTPTransport] instance. -func NewHTTPTransport() *HTTPTransport { - txp := &HTTPTransport{http.DefaultTransport.(*http.Transport).Clone()} - - // When we're using our net/http fork, we know we're able to - // actually force HTTP/2 without any issue - txp.txp.ForceAttemptHTTP2 = ExpectedForceAttemptHTTP2 - - return txp -} - -// SetDialContext sets the DialContext field. -func (txp *HTTPTransport) SetDialContext(fx func(ctx context.Context, network, address string) (net.Conn, error)) { - txp.txp.DialContext = fx -} - -// GetDialContext returns the value of the DialContext field. -func (txp *HTTPTransport) GetDialContext() func(ctx context.Context, network, address string) (net.Conn, error) { - return txp.txp.DialContext -} - -// SetDialTLSContext sets the DialTLSContext field. -func (txp *HTTPTransport) SetDialTLSContext(fx func(ctx context.Context, network, address string) (net.Conn, error)) { - txp.txp.DialTLSContext = fx -} - -// GetDialTLSContext returns the value of the DialTLSContext field. -func (txp *HTTPTransport) GetDialTLSContext() func(ctx context.Context, network, address string) (net.Conn, error) { - return txp.txp.DialTLSContext -} - -// SetProxy sets the Proxy field. -func (txp *HTTPTransport) SetProxy(fx func(*HTTPRequest) (*url.URL, error)) { - txp.txp.Proxy = fx -} - -// GetProxy returns the value of the Proxy field. -func (txp *HTTPTransport) GetProxy() func(*HTTPRequest) (*url.URL, error) { - return txp.txp.Proxy -} - -// SetMaxConnsPerHost sets the MaxConnsPerHost field. -func (txp *HTTPTransport) SetMaxConnsPerHost(value int) { - txp.txp.MaxConnsPerHost = value -} - -// GetMaxConnsPerHost returns the value of the MaxConnsPerHost field. -func (txp *HTTPTransport) GetMaxConnsPerHost() int { - return txp.txp.MaxConnsPerHost -} - -// SetDisableCompression sets the DisableCompression field. -func (txp *HTTPTransport) SetDisableCompression(value bool) { - txp.txp.DisableCompression = value -} - -// GetDisableCompression returns the value of the DisableCompression field. -func (txp *HTTPTransport) GetDisableCompression() bool { - return txp.txp.DisableCompression -} - -// SetTLSClientConfig sets the TLSClientConfig field. -func (txp *HTTPTransport) SetTLSClientConfig(value *tls.Config) { - txp.txp.TLSClientConfig = value -} - -// GetForceAttemptHTTP2 returns the value of the ForceAttemptHTTP2 field. -func (txp *HTTPTransport) GetForceAttemptHTTP2() bool { - return txp.txp.ForceAttemptHTTP2 -} - -// CloseIdleConnections closes the idle connections. -func (txp *HTTPTransport) CloseIdleConnections() { - txp.txp.CloseIdleConnections() -} - -// RoundTrip performs an HTTP round trip. -func (txp *HTTPTransport) RoundTrip(req *http.Request) (*http.Response, error) { - return txp.txp.RoundTrip(req) -} diff --git a/internal/feature/ootlsfeat/doc.go b/internal/feature/ootlsfeat/doc.go deleted file mode 100644 index 509ea63bc0..0000000000 --- a/internal/feature/ootlsfeat/doc.go +++ /dev/null @@ -1,6 +0,0 @@ -// Package ootlsfeat implements the ootls feature. When it is possible -// to enable this feature, we include mitigations that make TLS more robust -// for Android, as documented by the following blog post -// https://ooni.org/post/making-ooni-probe-android-more-resilient/. Otherwise, -// we will just use the standard library. -package ootlsfeat diff --git a/internal/feature/ootlsfeat/ootls_enabled.go b/internal/feature/ootlsfeat/ootls_enabled.go deleted file mode 100644 index 1289db84aa..0000000000 --- a/internal/feature/ootlsfeat/ootls_enabled.go +++ /dev/null @@ -1,16 +0,0 @@ -//go:build !go1.21 && !ooni_feature_disable_oohttp - -package ootlsfeat - -import ( - "crypto/tls" - "net" - - ootls "github.com/ooni/oocrypto/tls" - "github.com/ooni/probe-cli/v3/internal/model" -) - -// NewClientConnStdlib returns a new client connection using the standard library's TLS stack. -func NewClientConnStdlib(conn net.Conn, config *tls.Config) (model.TLSConn, error) { - return ootls.NewClientConnStdlib(conn, config) -} diff --git a/internal/feature/ootlsfeat/ootls_otherwise.go b/internal/feature/ootlsfeat/ootls_otherwise.go deleted file mode 100644 index 6e2d818217..0000000000 --- a/internal/feature/ootlsfeat/ootls_otherwise.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build go1.21 || ooni_feature_disable_oohttp - -package ootlsfeat - -import ( - "crypto/tls" - "net" - - "github.com/ooni/probe-cli/v3/internal/model" -) - -// NewClientConnStdlib returns a new client connection using the standard library's TLS stack. -func NewClientConnStdlib(conn net.Conn, config *tls.Config) (model.TLSConn, error) { - return tls.Client(conn, config), nil -} diff --git a/internal/model/netx.go b/internal/model/netx.go index 04b6253f41..f6f4bc78ff 100644 --- a/internal/model/netx.go +++ b/internal/model/netx.go @@ -13,6 +13,7 @@ import ( "syscall" "time" + oohttp "github.com/ooni/oohttp" "github.com/quic-go/quic-go" utls "gitlab.com/yawning/utls.git" ) @@ -308,25 +309,12 @@ type Resolver interface { LookupNS(ctx context.Context, domain string) ([]*net.NS, error) } -// TLSConn is the interface representing a *tls.Conn compatible -// connection, which could possibly be different from a *tls.Conn -// as long as it implements the interface. You can use, for -// example, refraction-networking/utls instead of the stdlib. -type TLSConn interface { - // net.Conn is the underlying interface - net.Conn - - // ConnectionState returns the ConnectionState according - // to the standard library. - ConnectionState() tls.ConnectionState - - // HandshakeContext performs an TLS handshake bounded - // in time by the given context. - HandshakeContext(ctx context.Context) error - - // NetConn returns the underlying net.Conn - NetConn() net.Conn -} +// TLSConn is the type of connection that oohttp expects from +// any library that implements TLS functionality. By using this +// kind of TLSConn we're able to use both the standard library +// and gitlab.com/yawning/utls.git to perform TLS operations. Note +// that the stdlib's tls.Conn implements this interface. +type TLSConn = oohttp.TLSConn // Ensures that a [*tls.Conn] implements the [TLSConn] interface. var _ TLSConn = &tls.Conn{} @@ -337,7 +325,7 @@ type TLSDialer interface { CloseIdleConnections() // DialTLSContext dials a TLS connection. This method will always return - // to you a TLSConn, so you can always safely cast to it. + // to you a oohttp.TLSConn, so you can always safely cast to it. // // The endpoint is an endpoint like the ones accepted by [net.DialContext]. For example, // x.org:443, 130.192.91.211:443 and [::1]:443. Note that IPv6 addrs are quoted. @@ -369,7 +357,7 @@ type TLSHandshaker interface { // Trace allows to collect measurement traces. A trace is injected into // netx operations using context.WithValue. Netx code retrieves the trace -// using context.Value. See the docs/design/dd-003-step-by-step.md for the +// using context.Value. See docs/design/dd-003-step-by-step.md for the // design document explaining why we implemented context-based tracing. type Trace interface { // TimeNow returns the current time. Normally, this should be the same diff --git a/internal/netxlite/httpfactory.go b/internal/netxlite/httpfactory.go index e95db5822a..0e57f249fe 100644 --- a/internal/netxlite/httpfactory.go +++ b/internal/netxlite/httpfactory.go @@ -4,12 +4,12 @@ import ( "crypto/tls" "net/url" - "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" + oohttp "github.com/ooni/oohttp" "github.com/ooni/probe-cli/v3/internal/model" ) // HTTPTransportOption is an initialization option for [NewHTTPTransport]. -type HTTPTransportOption func(txp *oohttpfeat.HTTPTransport) +type HTTPTransportOption func(txp *oohttp.Transport) // NewHTTPTransport is the high-level factory to create a [model.HTTPTransport] using // github.com/ooni/oohttp as the HTTP library with HTTP/1.1 and HTTP2 support. @@ -41,28 +41,20 @@ type HTTPTransportOption func(txp *oohttpfeat.HTTPTransport) // This factory is the RECOMMENDED way of creating a [model.HTTPTransport]. func NewHTTPTransportWithOptions(logger model.Logger, dialer model.Dialer, tlsDialer model.TLSDialer, options ...HTTPTransportOption) model.HTTPTransport { - // Using oohttp to support any TLS library iff it's possible to do so, otherwise we - // are going to use the standard library w/o using HTTP/2 support. - txp := oohttpfeat.NewHTTPTransport() + // Using oohttp to support any TLS library. + txp := oohttp.DefaultTransport.(*oohttp.Transport).Clone() // This wrapping ensures that we always have a timeout when we // are using HTTP; see https://github.com/ooni/probe/issues/1609. dialer = &httpDialerWithReadTimeout{dialer} - txp.SetDialContext(dialer.DialContext) + txp.DialContext = dialer.DialContext tlsDialer = &httpTLSDialerWithReadTimeout{tlsDialer} - txp.SetDialTLSContext(tlsDialer.DialTLSContext) + txp.DialTLSContext = tlsDialer.DialTLSContext // As documented, disable proxies and force HTTP/2 - txp.SetDisableCompression(true) - txp.SetProxy(nil) - - // We now rely on feature/oohttpfeat to decide whether it's - // possible for us to actually enable HTTP/2. - // - // txp.ForceAttemptHTTP2 = true - // - // Please, keep this comment until at least 3.21 because it provides - // an historical documentation of how we changed the codebase. + txp.DisableCompression = true + txp.Proxy = nil + txp.ForceAttemptHTTP2 = true // Apply all the required options for _, option := range options { @@ -71,7 +63,7 @@ func NewHTTPTransportWithOptions(logger model.Logger, // Return a fully wrapped HTTP transport return WrapHTTPTransport(logger, &httpTransportConnectionsCloser{ - HTTPTransport: &httpTransportStdlib{txp}, + HTTPTransport: &httpTransportStdlib{&oohttp.StdlibTransport{Transport: txp}}, Dialer: dialer, TLSDialer: tlsDialer, }) @@ -80,27 +72,27 @@ func NewHTTPTransportWithOptions(logger model.Logger, // HTTPTransportOptionProxyURL configures the transport to use the given proxyURL // or disables proxying (already the default) if the proxyURL is nil. func HTTPTransportOptionProxyURL(proxyURL *url.URL) HTTPTransportOption { - return func(txp *oohttpfeat.HTTPTransport) { - txp.SetProxy(func(r *oohttpfeat.HTTPRequest) (*url.URL, error) { + return func(txp *oohttp.Transport) { + txp.Proxy = func(r *oohttp.Request) (*url.URL, error) { // "If Proxy is nil or returns a nil *URL, no proxy is used." return proxyURL, nil - }) + } } } // HTTPTransportOptionMaxConnsPerHost configures the .MaxConnPerHosts field, which // otherwise uses the default set in github.com/ooni/oohttp. func HTTPTransportOptionMaxConnsPerHost(value int) HTTPTransportOption { - return func(txp *oohttpfeat.HTTPTransport) { - txp.SetMaxConnsPerHost(value) + return func(txp *oohttp.Transport) { + txp.MaxConnsPerHost = value } } // HTTPTransportOptionDisableCompression configures the .DisableCompression field, which // otherwise is set to true, so that this code is ready for measuring out of the box. func HTTPTransportOptionDisableCompression(value bool) HTTPTransportOption { - return func(txp *oohttpfeat.HTTPTransport) { - txp.SetDisableCompression(value) + return func(txp *oohttp.Transport) { + txp.DisableCompression = value } } @@ -112,7 +104,7 @@ func HTTPTransportOptionDisableCompression(value bool) HTTPTransportOption { // this limitation. Future releases MIGHT use a different technique and, as such, // we MAY remove this option when we don't need it anymore. func HTTPTransportOptionTLSClientConfig(config *tls.Config) HTTPTransportOption { - return func(txp *oohttpfeat.HTTPTransport) { - txp.SetTLSClientConfig(config) + return func(txp *oohttp.Transport) { + txp.TLSClientConfig = config } } diff --git a/internal/netxlite/httpfactory_test.go b/internal/netxlite/httpfactory_test.go index 88027f6056..c02955c11c 100644 --- a/internal/netxlite/httpfactory_test.go +++ b/internal/netxlite/httpfactory_test.go @@ -1,10 +1,13 @@ package netxlite import ( + "crypto/tls" "net/url" "testing" - "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + oohttp "github.com/ooni/oohttp" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -39,33 +42,54 @@ func TestNewHTTPTransportWithOptions(t *testing.T) { // make sure there's the stdlib adapter stdlibAdapter := txpCloser.HTTPTransport.(*httpTransportStdlib) - underlying := stdlibAdapter.StdlibTransport + oohttpStdlibAdapter := stdlibAdapter.StdlibTransport + underlying := oohttpStdlibAdapter.Transport + + // now let's check that everything is configured as intended + expectedTxp := oohttp.DefaultTransport.(*oohttp.Transport).Clone() + diff := cmp.Diff( + expectedTxp, + underlying, + cmpopts.IgnoreUnexported(oohttp.Transport{}), + cmpopts.IgnoreUnexported(tls.Config{}), + cmpopts.IgnoreFields( + oohttp.Transport{}, + "DialContext", + "DialTLSContext", + "DisableCompression", + "Proxy", + "ForceAttemptHTTP2", + ), + ) + if diff != "" { + t.Fatal(diff) + } // finish checking by explicitly inspecting the fields we modify - if underlying.GetDialContext() == nil { + if underlying.DialContext == nil { t.Fatal("expected non-nil .DialContext") } - if underlying.GetDialTLSContext() == nil { + if underlying.DialTLSContext == nil { t.Fatal("expected non-nil .DialTLSContext") } - if underlying.GetProxy() != nil { + if underlying.Proxy != nil { t.Fatal("expected nil .Proxy") } - if underlying.GetForceAttemptHTTP2() != oohttpfeat.ExpectedForceAttemptHTTP2 { + if !underlying.ForceAttemptHTTP2 { t.Fatal("expected true .ForceAttemptHTTP2") } - if !underlying.GetDisableCompression() { + if !underlying.DisableCompression { t.Fatal("expected true .DisableCompression") } }) - unwrap := func(txp model.HTTPTransport) *oohttpfeat.HTTPTransport { + unwrap := func(txp model.HTTPTransport) *oohttp.Transport { txpLogger := txp.(*httpTransportLogger) txpErrWrapper := txpLogger.HTTPTransport.(*httpTransportErrWrapper) txpCloser := txpErrWrapper.HTTPTransport.(*httpTransportConnectionsCloser) stdlibAdapter := txpCloser.HTTPTransport.(*httpTransportStdlib) oohttpStdlibAdapter := stdlibAdapter.StdlibTransport - return oohttpStdlibAdapter + return oohttpStdlibAdapter.Transport } t.Run("make sure HTTPTransportOptionProxyURL is WAI", func(t *testing.T) { @@ -80,11 +104,10 @@ func TestNewHTTPTransportWithOptions(t *testing.T) { HTTPTransportOptionProxyURL(expectedURL), ) underlying := unwrap(txp) - proxy := underlying.GetProxy() - if proxy == nil { + if underlying.Proxy == nil { t.Fatal("expected non-nil .Proxy") } - got, err := proxy(&oohttpfeat.HTTPRequest{}) + got, err := underlying.Proxy(&oohttp.Request{}) if err != nil { t.Fatal(err) } @@ -110,7 +133,7 @@ func TestNewHTTPTransportWithOptions(t *testing.T) { HTTPTransportOptionMaxConnsPerHost(expectedValue), ) underlying := unwrap(txp) - got := underlying.GetMaxConnsPerHost() + got := underlying.MaxConnsPerHost if got != expectedValue { t.Fatal("not the expected value") } @@ -133,7 +156,7 @@ func TestNewHTTPTransportWithOptions(t *testing.T) { HTTPTransportOptionDisableCompression(expectedValue), ) underlying := unwrap(txp) - got := underlying.GetDisableCompression() + got := underlying.DisableCompression if got != expectedValue { t.Fatal("not the expected value") } diff --git a/internal/netxlite/httpquirks.go b/internal/netxlite/httpquirks.go index a596d6c77f..d4c26945e0 100644 --- a/internal/netxlite/httpquirks.go +++ b/internal/netxlite/httpquirks.go @@ -11,7 +11,7 @@ package netxlite import ( "net/http" - "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" + oohttp "github.com/ooni/oohttp" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -60,42 +60,37 @@ func NewHTTPTransport(logger model.DebugLogger, dialer model.Dialer, tlsDialer m // // This function behavior is QUIRKY as documented in [NewHTTPTransport]. func newOOHTTPBaseTransport(dialer model.Dialer, tlsDialer model.TLSDialer) model.HTTPTransport { - // Using oohttp to support any TLS library iff it's possible to do so, otherwise we - // are going to use the standard library w/o using HTTP/2 support. - txp := oohttpfeat.NewHTTPTransport() + // Using oohttp to support any TLS library. + txp := oohttp.DefaultTransport.(*oohttp.Transport).Clone() // This wrapping ensures that we always have a timeout when we // are using HTTP; see https://github.com/ooni/probe/issues/1609. dialer = &httpDialerWithReadTimeout{dialer} - txp.SetDialContext(dialer.DialContext) + txp.DialContext = dialer.DialContext tlsDialer = &httpTLSDialerWithReadTimeout{tlsDialer} - txp.SetDialTLSContext(tlsDialer.DialTLSContext) + txp.DialTLSContext = tlsDialer.DialTLSContext // We are using a different strategy to implement proxy: we // use a specific dialer that knows about proxying. - txp.SetProxy(nil) + txp.Proxy = nil // Better for Cloudflare DNS and also better because we have less // noisy events and we can better understand what happened. - txp.SetMaxConnsPerHost(1) + txp.MaxConnsPerHost = 1 // The following (1) reduces the number of headers that Go will // automatically send for us and (2) ensures that we always receive // back the true headers, such as Content-Length. This change is // functional to OONI's goal of observing the network. - txp.SetDisableCompression(true) + txp.DisableCompression = true - // We now rely on feature/oohttpfeat to decide whether it's - // possible for us to actually enable HTTP/2. - // - // txp.ForceAttemptHTTP2 = true - // - // Please, keep this comment until at least 3.21 because it provides - // an historical documentation of how we changed the codebase. + // Required to enable using HTTP/2 (which will be anyway forced + // upon us when we are using TLS parroting). + txp.ForceAttemptHTTP2 = true // Ensure we correctly forward CloseIdleConnections. return &httpTransportConnectionsCloser{ - HTTPTransport: &httpTransportStdlib{txp}, + HTTPTransport: &httpTransportStdlib{&oohttp.StdlibTransport{Transport: txp}}, Dialer: dialer, TLSDialer: tlsDialer, } diff --git a/internal/netxlite/httpquirks_test.go b/internal/netxlite/httpquirks_test.go index 2695a99b06..3043293e4e 100644 --- a/internal/netxlite/httpquirks_test.go +++ b/internal/netxlite/httpquirks_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/apex/log" - "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" "github.com/ooni/probe-cli/v3/internal/mocks" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -86,20 +85,19 @@ func TestNewHTTPTransport(t *testing.T) { t.Fatal("invalid tls dialer") } stdlib := connectionsCloser.HTTPTransport.(*httpTransportStdlib) - underlying := stdlib.StdlibTransport - if underlying.GetForceAttemptHTTP2() != oohttpfeat.ExpectedForceAttemptHTTP2 { + if !stdlib.StdlibTransport.ForceAttemptHTTP2 { t.Fatal("invalid ForceAttemptHTTP2") } - if !underlying.GetDisableCompression() { + if !stdlib.StdlibTransport.DisableCompression { t.Fatal("invalid DisableCompression") } - if underlying.GetMaxConnsPerHost() != 1 { + if stdlib.StdlibTransport.MaxConnsPerHost != 1 { t.Fatal("invalid MaxConnPerHost") } - if underlying.GetDialTLSContext() == nil { + if stdlib.StdlibTransport.DialTLSContext == nil { t.Fatal("invalid DialTLSContext") } - if underlying.GetDialContext() == nil { + if stdlib.StdlibTransport.DialContext == nil { t.Fatal("invalid DialContext") } }) diff --git a/internal/netxlite/httpstdlib.go b/internal/netxlite/httpstdlib.go index 5e235cf43e..2f3063dbf9 100644 --- a/internal/netxlite/httpstdlib.go +++ b/internal/netxlite/httpstdlib.go @@ -7,13 +7,13 @@ package netxlite import ( "net/http" - "github.com/ooni/probe-cli/v3/internal/feature/oohttpfeat" + oohttp "github.com/ooni/oohttp" "github.com/ooni/probe-cli/v3/internal/model" ) // stdlibTransport wraps oohttp.StdlibTransport to add .Network() type httpTransportStdlib struct { - StdlibTransport *oohttpfeat.HTTPTransport + StdlibTransport *oohttp.StdlibTransport } var _ model.HTTPTransport = &httpTransportStdlib{} diff --git a/internal/netxlite/tls.go b/internal/netxlite/tls.go index 80b989b557..14df796eca 100644 --- a/internal/netxlite/tls.go +++ b/internal/netxlite/tls.go @@ -13,7 +13,7 @@ import ( "net" "time" - "github.com/ooni/probe-cli/v3/internal/feature/ootlsfeat" + ootls "github.com/ooni/oocrypto/tls" "github.com/ooni/probe-cli/v3/internal/model" "github.com/ooni/probe-cli/v3/internal/runtimex" ) @@ -248,7 +248,7 @@ func (h *tlsHandshakerConfigurable) newConn(conn net.Conn, config *tls.Config) ( if h.NewConn != nil { return h.NewConn(conn, config) } - return ootlsfeat.NewClientConnStdlib(conn, config) + return ootls.NewClientConnStdlib(conn, config) } // tlsHandshakerLogger is a TLSHandshaker with logging. From 500f3df53f42ed2908f2d7b8bd33a1482b27882f Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 13 Dec 2023 10:37:52 +0100 Subject: [PATCH 10/12] x --- internal/feature/psiphonfeat/psiphon_enabled.go | 3 +++ internal/feature/psiphonfeat/psiphon_otherwise.go | 3 +++ internal/tunnel/psiphon_integration_test.go | 4 ++++ 3 files changed, 10 insertions(+) diff --git a/internal/feature/psiphonfeat/psiphon_enabled.go b/internal/feature/psiphonfeat/psiphon_enabled.go index 6791deb59f..6adc0222a4 100644 --- a/internal/feature/psiphonfeat/psiphon_enabled.go +++ b/internal/feature/psiphonfeat/psiphon_enabled.go @@ -8,6 +8,9 @@ import ( "github.com/Psiphon-Labs/psiphon-tunnel-core/ClientLibrary/clientlib" ) +// Enabled indicates whether this feature is enabled. +const Enabled = true + // Start attempts to start the Psiphon tunnel and returns either a Tunnel or an error. func Start(ctx context.Context, config []byte, workdir string) (Tunnel, error) { tun, err := clientlib.StartTunnel(ctx, config, "", clientlib.Parameters{ diff --git a/internal/feature/psiphonfeat/psiphon_otherwise.go b/internal/feature/psiphonfeat/psiphon_otherwise.go index 4791ac8b4e..9793121712 100644 --- a/internal/feature/psiphonfeat/psiphon_otherwise.go +++ b/internal/feature/psiphonfeat/psiphon_otherwise.go @@ -4,6 +4,9 @@ package psiphonfeat import "context" +// Enabled indicates whether this feature is enabled. +const Enabled = false + // Start attempts to start the Psiphon tunnel and returns either a Tunnel or an error. func Start(ctx context.Context, config []byte, workdir string) (Tunnel, error) { return nil, ErrFeatureNotEnabled diff --git a/internal/tunnel/psiphon_integration_test.go b/internal/tunnel/psiphon_integration_test.go index 40e680cd27..4151fe5dd0 100644 --- a/internal/tunnel/psiphon_integration_test.go +++ b/internal/tunnel/psiphon_integration_test.go @@ -7,6 +7,7 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine" + "github.com/ooni/probe-cli/v3/internal/feature/psiphonfeat" "github.com/ooni/probe-cli/v3/internal/tunnel" ) @@ -14,6 +15,9 @@ func TestPsiphonStartStop(t *testing.T) { if testing.Short() { t.Skip("skip test in short mode") } + if !psiphonfeat.Enabled { + t.Skip("psiphon feature not enabled") + } tunnelDir, err := ioutil.TempDir("testdata", "psiphon") if err != nil { t.Fatal(err) From 595477d83e233b5c6dc6a4b95e262daa1392dd62 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 13 Dec 2023 10:42:01 +0100 Subject: [PATCH 11/12] doc: relax restrictions on go version --- Readme.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Readme.md b/Readme.md index 8bbed67b80..228bb09a96 100644 --- a/Readme.md +++ b/Readme.md @@ -60,9 +60,10 @@ Debian/Ubuntu. Once `ooniprobe` is installed, refer to the ## Developer instructions -This repository requires _exactly_ the Go version mentioned by the -[GOVERSION](GOVERSION) file (i.e., go1.20.12). Using a different version of -Go _may_ work as intended but is not recommended: we depend +This repository _should really_ use the Go version mentioned by the +[GOVERSION](GOVERSION) file (i.e., go1.20.12). Using a later version of +Go _should_ work as intended. Using an older version of Go is +_definitely not recommended_ and _may not even compile_. Here's why: we rely on packages forked from the standard library; so, it is more robust to use the same version of Go from which we forked those packages from. From f2a5682e583322cfccac579fca50b07e3afe36ac Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 13 Dec 2023 10:53:27 +0100 Subject: [PATCH 12/12] try to make go1.21 tests green --- .github/workflows/go1.21.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go1.21.yml b/.github/workflows/go1.21.yml index 6e2a6caae3..38312250c8 100644 --- a/.github/workflows/go1.21.yml +++ b/.github/workflows/go1.21.yml @@ -19,4 +19,6 @@ jobs: go-version: ~1.21 cache-key-suffix: "-alltests-go1.21" - - run: go test -race -tags shaping ./... + # We cannot run buildtool tests using an unexpected version of Go because the + # tests check whether we're using the expected version of Go 😂😂😂😂. + - run: go test -race -tags shaping $(go list ./...|grep -v 'internal/cmd/buildtool')