From 4dc290747204c5815bfd52e1d86006b75256895b Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Wed, 30 Jun 2021 15:19:10 +0200 Subject: [PATCH] refactor: move base http3 transport into netxlite (#412) This diff is part of https://github.com/ooni/probe/issues/1505. You will notice that I have not adapted all the (great) tests we had previously. They should live at another layer, and namely the one that deals with performing measurements. When I'm refactoring such a layer I'll ensure those tests that I have not adapted here are reintroduced into the tree. --- .../netx/httptransport/http3transport.go | 37 +--- .../netx/httptransport/http3transport_test.go | 160 +----------------- .../netx/httptransport/httptransport.go | 2 +- internal/engine/netx/netx.go | 5 +- internal/netxlite/http3.go | 54 ++++++ internal/netxlite/http3_test.go | 25 +++ 6 files changed, 87 insertions(+), 196 deletions(-) create mode 100644 internal/netxlite/http3.go create mode 100644 internal/netxlite/http3_test.go diff --git a/internal/engine/netx/httptransport/http3transport.go b/internal/engine/netx/httptransport/http3transport.go index ae42740..b7b75e0 100644 --- a/internal/engine/netx/httptransport/http3transport.go +++ b/internal/engine/netx/httptransport/http3transport.go @@ -1,43 +1,10 @@ package httptransport import ( - "context" - "crypto/tls" - "net/http" - - "github.com/lucas-clemente/quic-go" - "github.com/lucas-clemente/quic-go/http3" - "github.com/ooni/probe-cli/v3/internal/engine/netx/quicdialer" + "github.com/ooni/probe-cli/v3/internal/netxlite" ) -// QUICWrapperDialer is a QUICDialer that wraps a ContextDialer -// This is necessary because the http3 RoundTripper does not support a DialContext method. -type QUICWrapperDialer struct { - Dialer quicdialer.ContextDialer -} - -// Dial implements QUICDialer.Dial -func (d QUICWrapperDialer) Dial(network, host string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) { - return d.Dialer.DialContext(context.Background(), network, host, tlsCfg, cfg) -} - -// HTTP3Transport is a httptransport.RoundTripper using the http3 protocol. -type HTTP3Transport struct { - http3.RoundTripper -} - -// CloseIdleConnections closes all the connections opened by this transport. -func (t *HTTP3Transport) CloseIdleConnections() { - t.RoundTripper.Close() -} - // NewHTTP3Transport creates a new HTTP3Transport instance. func NewHTTP3Transport(config Config) RoundTripper { - txp := &HTTP3Transport{} - txp.QuicConfig = &quic.Config{} - txp.TLSClientConfig = config.TLSConfig - txp.Dial = config.QUICDialer.Dial - return txp + return netxlite.NewHTTP3Transport(config.QUICDialer, config.TLSConfig) } - -var _ RoundTripper = &http.Transport{} diff --git a/internal/engine/netx/httptransport/http3transport_test.go b/internal/engine/netx/httptransport/http3transport_test.go index 3683c0b..a0e2418 100644 --- a/internal/engine/netx/httptransport/http3transport_test.go +++ b/internal/engine/netx/httptransport/http3transport_test.go @@ -1,166 +1,12 @@ package httptransport_test import ( - "context" - "crypto/tls" - "crypto/x509" - "errors" - "net" - "net/http" - "strings" "testing" - "github.com/lucas-clemente/quic-go" - "github.com/ooni/probe-cli/v3/internal/engine/netx/dialer" "github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport" ) -type MockQUICDialer struct{} - -func (d MockQUICDialer) Dial(network, host string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) { - return quic.DialAddrEarly(host, tlsCfg, cfg) -} - -type MockSNIQUICDialer struct { - namech chan string -} - -func (d MockSNIQUICDialer) Dial(network, host string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) { - d.namech <- tlsCfg.ServerName - return quic.DialAddrEarly(host, tlsCfg, cfg) -} - -type MockCertQUICDialer struct { - certch chan *x509.CertPool -} - -func (d MockCertQUICDialer) Dial(network, host string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) { - d.certch <- tlsCfg.RootCAs - return quic.DialAddrEarly(host, tlsCfg, cfg) -} - -func TestHTTP3TransportSNI(t *testing.T) { - namech := make(chan string, 1) - sni := "sni.org" - txp := httptransport.NewHTTP3Transport(httptransport.Config{ - Dialer: dialer.New(&dialer.Config{}, &net.Resolver{}), - QUICDialer: MockSNIQUICDialer{namech: namech}, - TLSConfig: &tls.Config{ServerName: sni}}) - req, err := http.NewRequest("GET", "https://www.google.com", nil) - if err != nil { - t.Fatal(err) - } - resp, err := txp.RoundTrip(req) - if err == nil { - t.Fatal("expected error here") - } - if resp != nil { - t.Fatal("expected nil resp here") - } - if !strings.Contains(err.Error(), "certificate is valid for www.google.com, not "+sni) { - t.Fatal("unexpected error type", err) - } - servername := <-namech - if servername != sni { - t.Fatal("unexpected server name", servername) - } -} - -func TestHTTP3TransportSNINoVerify(t *testing.T) { - namech := make(chan string, 1) - sni := "sni.org" - txp := httptransport.NewHTTP3Transport(httptransport.Config{ - Dialer: dialer.New(&dialer.Config{}, &net.Resolver{}), - QUICDialer: MockSNIQUICDialer{namech: namech}, - TLSConfig: &tls.Config{ServerName: sni, InsecureSkipVerify: true}}) - req, err := http.NewRequest("GET", "https://www.google.com", nil) - if err != nil { - t.Fatal(err) - } - resp, err := txp.RoundTrip(req) - if err != nil { - t.Fatalf("unexpected error: %+v", err) - } - if resp == nil { - t.Fatal("unexpected nil resp") - } - servername := <-namech - if servername != sni { - t.Fatal("unexpected server name", servername) - } -} - -func TestHTTP3TransportCABundle(t *testing.T) { - certch := make(chan *x509.CertPool, 1) - certpool := x509.NewCertPool() - txp := httptransport.NewHTTP3Transport(httptransport.Config{ - Dialer: dialer.New(&dialer.Config{}, &net.Resolver{}), - QUICDialer: MockCertQUICDialer{certch: certch}, - TLSConfig: &tls.Config{RootCAs: certpool}}) - req, err := http.NewRequest("GET", "https://www.google.com", nil) - if err != nil { - t.Fatal(err) - } - resp, err := txp.RoundTrip(req) - if err == nil { - t.Fatal("expected error here") - } - if resp != nil { - t.Fatal("expected nil resp here") - } - // since the certificate pool is empty, the unknown authority error should be thrown - if !strings.Contains(err.Error(), "certificate signed by unknown authority") { - t.Fatal("unexpected error type") - } - certs := <-certch - if certs != certpool { - t.Fatal("not the certpool we expected") - } - -} - -func TestUnitHTTP3TransportSuccess(t *testing.T) { - txp := httptransport.NewHTTP3Transport(httptransport.Config{ - Dialer: dialer.New(&dialer.Config{}, &net.Resolver{}), - QUICDialer: MockQUICDialer{}}) - - req, err := http.NewRequest("GET", "https://www.google.com", nil) - if err != nil { - t.Fatal(err) - } - resp, err := txp.RoundTrip(req) - if err != nil { - t.Fatal(err) - } - if resp == nil { - t.Fatal("unexpected nil response here") - } - if resp.StatusCode != 200 { - t.Fatal("HTTP statuscode should be 200 OK", resp.StatusCode) - } -} - -func TestUnitHTTP3TransportFailure(t *testing.T) { - txp := httptransport.NewHTTP3Transport(httptransport.Config{ - Dialer: dialer.New(&dialer.Config{}, &net.Resolver{}), - QUICDialer: MockQUICDialer{}}) - - ctx, cancel := context.WithCancel(context.Background()) - cancel() // so that the request immediately fails - req, err := http.NewRequestWithContext(ctx, "GET", "https://www.google.com", nil) - if err != nil { - t.Fatal(err) - } - resp, err := txp.RoundTrip(req) - if err == nil { - t.Fatal("expected error here") - } - // context.Canceled error occurs if the test host supports QUIC - // timeout error ("Handshake did not complete in time") occurs if the test host does not support QUIC - if !(errors.Is(err, context.Canceled) || strings.HasSuffix(err.Error(), "Handshake did not complete in time")) { - t.Fatal("not the error we expected", err) - } - if resp != nil { - t.Fatal("expected nil response here") - } +func TestNewHTTP3Transport(t *testing.T) { + // mainly to cover a line which otherwise won't be directly covered + httptransport.NewHTTP3Transport(httptransport.Config{}) } diff --git a/internal/engine/netx/httptransport/httptransport.go b/internal/engine/netx/httptransport/httptransport.go index 46b6864..6378444 100644 --- a/internal/engine/netx/httptransport/httptransport.go +++ b/internal/engine/netx/httptransport/httptransport.go @@ -30,7 +30,7 @@ type TLSDialer interface { // QUICDialer is the definition of dialer for QUIC assumed by this package. type QUICDialer interface { - Dial(network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) + DialContext(ctx context.Context, network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) } // RoundTripper is the definition of http.RoundTripper used by this package. diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index 3dda34e..321c531 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -54,7 +54,7 @@ type Dialer interface { // QUICDialer is the definition of a dialer for QUIC assumed by this package. type QUICDialer interface { - Dial(network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) + DialContext(ctx context.Context, network, addr string, tlsCfg *tls.Config, cfg *quic.Config) (quic.EarlySession, error) } // TLSDialer is the definition of a TLS dialer assumed by this package. @@ -175,8 +175,7 @@ func NewQUICDialer(config Config) QUICDialer { d = quicdialer.HandshakeSaver{Saver: config.TLSSaver, Dialer: d} } d = &netxlite.QUICDialerResolver{Resolver: config.FullResolver, Dialer: d} - var dialer QUICDialer = &httptransport.QUICWrapperDialer{Dialer: d} - return dialer + return d } // NewTLSDialer creates a new TLSDialer from the specified config diff --git a/internal/netxlite/http3.go b/internal/netxlite/http3.go new file mode 100644 index 0000000..6914f21 --- /dev/null +++ b/internal/netxlite/http3.go @@ -0,0 +1,54 @@ +package netxlite + +import ( + "context" + "crypto/tls" + "net/http" + + "github.com/lucas-clemente/quic-go" + "github.com/lucas-clemente/quic-go/http3" +) + +// http3Dialer adapts a QUICContextDialer to work with +// an http3.RoundTripper. This is necessary because the +// http3.RoundTripper does not support DialContext. +type http3Dialer struct { + Dialer QUICContextDialer +} + +// dial is like QUICContextDialer.DialContext but without context. +func (d *http3Dialer) dial(network, address string, tlsConfig *tls.Config, + quicConfig *quic.Config) (quic.EarlySession, error) { + return d.Dialer.DialContext( + context.Background(), network, address, tlsConfig, quicConfig) +} + +// http3Transport is an HTTPTransport using the http3 protocol. +type http3Transport struct { + child *http3.RoundTripper +} + +var _ HTTPTransport = &http3Transport{} + +// RoundTrip implements HTTPTransport.RoundTrip. +func (txp *http3Transport) RoundTrip(req *http.Request) (*http.Response, error) { + return txp.child.RoundTrip(req) +} + +// CloseIdleConnections implements HTTPTransport.CloseIdleConnections. +func (txp *http3Transport) CloseIdleConnections() { + txp.child.Close() +} + +// NewHTTP3Transport creates a new HTTPTransport using http3. The +// dialer argument MUST NOT be nil. If the tlsConfig argument is nil, +// then the code will use the default TLS configuration. +func NewHTTP3Transport( + dialer QUICContextDialer, tlsConfig *tls.Config) HTTPTransport { + return &http3Transport{ + child: &http3.RoundTripper{ + Dial: (&http3Dialer{dialer}).dial, + TLSClientConfig: tlsConfig, + }, + } +} diff --git a/internal/netxlite/http3_test.go b/internal/netxlite/http3_test.go new file mode 100644 index 0000000..e6a0d57 --- /dev/null +++ b/internal/netxlite/http3_test.go @@ -0,0 +1,25 @@ +package netxlite + +import ( + "crypto/tls" + "net" + "net/http" + "testing" +) + +func TestHTTP3TransportWorks(t *testing.T) { + d := &QUICDialerResolver{ + Dialer: &QUICDialerQUICGo{ + QUICListener: &QUICListenerStdlib{}, + }, + Resolver: &net.Resolver{}, + } + txp := NewHTTP3Transport(d, &tls.Config{}) + client := &http.Client{Transport: txp} + resp, err := client.Get("https://www.google.com/robots.txt") + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + txp.CloseIdleConnections() +}