From a647cf4988a0ff5bbeb8430d1734b62076f07c5c Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Tue, 8 Jun 2021 21:14:45 +0200 Subject: [PATCH] refactor(netx): remove forwardes for tlsx (#365) Part of https://github.com/ooni/probe/issues/1591 --- .../engine/experiment/riseupvpn/riseupvpn.go | 4 ++-- internal/engine/experiment/signal/signal.go | 6 +++--- .../engine/experiment/urlgetter/configurer.go | 3 ++- .../experiment/urlgetter/configurer_test.go | 4 ++-- internal/engine/netx/netx.go | 16 ++-------------- internal/engine/netx/netx_test.go | 3 ++- 6 files changed, 13 insertions(+), 23 deletions(-) diff --git a/internal/engine/experiment/riseupvpn/riseupvpn.go b/internal/engine/experiment/riseupvpn/riseupvpn.go index 34c1e75..12ed75c 100644 --- a/internal/engine/experiment/riseupvpn/riseupvpn.go +++ b/internal/engine/experiment/riseupvpn/riseupvpn.go @@ -11,8 +11,8 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/archival" + "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsx" ) const ( @@ -183,7 +183,7 @@ func (m Measurer) Run(ctx context.Context, sess model.ExperimentSession, measurement.TestKeys = testkeys urlgetter.RegisterExtensions(measurement) - certPool := netx.NewDefaultCertPool() + certPool := tlsx.NewDefaultCertPool() // used multiple times below multi := urlgetter.Multi{ diff --git a/internal/engine/experiment/signal/signal.go b/internal/engine/experiment/signal/signal.go index 6189632..88cfe3f 100644 --- a/internal/engine/experiment/signal/signal.go +++ b/internal/engine/experiment/signal/signal.go @@ -10,7 +10,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" "github.com/ooni/probe-cli/v3/internal/engine/model" - "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsx" ) const ( @@ -111,12 +111,12 @@ func (m Measurer) Run(ctx context.Context, sess model.ExperimentSession, defer cancel() urlgetter.RegisterExtensions(measurement) - certPool := netx.NewDefaultCertPool() + certPool := tlsx.NewDefaultCertPool() signalCABytes := []byte(signalCA) if m.Config.SignalCA != "" { signalCABytes = []byte(m.Config.SignalCA) } - if certPool.AppendCertsFromPEM(signalCABytes) == false { + if !certPool.AppendCertsFromPEM(signalCABytes) { return errors.New("AppendCertsFromPEM failed") } diff --git a/internal/engine/experiment/urlgetter/configurer.go b/internal/engine/experiment/urlgetter/configurer.go index c92d90b..b992284 100644 --- a/internal/engine/experiment/urlgetter/configurer.go +++ b/internal/engine/experiment/urlgetter/configurer.go @@ -10,6 +10,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/model" "github.com/ooni/probe-cli/v3/internal/engine/netx" + "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" ) @@ -89,7 +90,7 @@ func (c Configurer) NewConfiguration() (Configuration, error) { if c.Config.TLSServerName != "" { configuration.HTTPConfig.TLSConfig.ServerName = c.Config.TLSServerName } - err = netx.ConfigureTLSVersion( + err = tlsx.ConfigureTLSVersion( configuration.HTTPConfig.TLSConfig, c.Config.TLSVersion, ) if err != nil { diff --git a/internal/engine/experiment/urlgetter/configurer_test.go b/internal/engine/experiment/urlgetter/configurer_test.go index 28490f8..6009261 100644 --- a/internal/engine/experiment/urlgetter/configurer_test.go +++ b/internal/engine/experiment/urlgetter/configurer_test.go @@ -9,8 +9,8 @@ import ( "github.com/apex/log" "github.com/ooni/probe-cli/v3/internal/engine/experiment/urlgetter" - "github.com/ooni/probe-cli/v3/internal/engine/netx" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" + "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" ) @@ -711,7 +711,7 @@ func TestConfigurerNewConfigurationTLSvInvalid(t *testing.T) { Saver: saver, } _, err := configurer.NewConfiguration() - if !errors.Is(err, netx.ErrInvalidTLSVersion) { + if !errors.Is(err, tlsx.ErrInvalidTLSVersion) { t.Fatalf("not the error we expected: %+v", err) } } diff --git a/internal/engine/netx/netx.go b/internal/engine/netx/netx.go index 2d38ce0..40fc4a6 100644 --- a/internal/engine/netx/netx.go +++ b/internal/engine/netx/netx.go @@ -109,11 +109,7 @@ type tlsHandshaker interface { net.Conn, tls.ConnectionState, error) } -// NewDefaultCertPool returns a copy of the default x509 -// certificate pool that we bundle from Mozilla. -var NewDefaultCertPool = tlsx.NewDefaultCertPool - -var defaultCertPool *x509.CertPool = NewDefaultCertPool() +var defaultCertPool *x509.CertPool = tlsx.NewDefaultCertPool() // NewResolver creates a new resolver from the specified config func NewResolver(config Config) Resolver { @@ -308,14 +304,6 @@ func NewDNSClient(config Config, URL string) (DNSClient, error) { return NewDNSClientWithOverrides(config, URL, "", "", "") } -// ErrInvalidTLSVersion indicates that you passed us a string -// that does not represent a valid TLS version. -var ErrInvalidTLSVersion = tlsx.ErrInvalidTLSVersion - -// ConfigureTLSVersion configures the correct TLS version into -// the specified *tls.Config or returns an error. -var ConfigureTLSVersion = tlsx.ConfigureTLSVersion - // NewDNSClientWithOverrides creates a new DNS client, similar to NewDNSClient, // with the option to override the default Hostname and SNI. func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride, @@ -336,7 +324,7 @@ func NewDNSClientWithOverrides(config Config, URL, hostOverride, SNIOverride, return c, err } config.TLSConfig = &tls.Config{ServerName: SNIOverride} - if err := ConfigureTLSVersion(config.TLSConfig, TLSVersion); err != nil { + if err := tlsx.ConfigureTLSVersion(config.TLSConfig, TLSVersion); err != nil { return c, err } switch resolverURL.Scheme { diff --git a/internal/engine/netx/netx_test.go b/internal/engine/netx/netx_test.go index 82105d9..84fda72 100644 --- a/internal/engine/netx/netx_test.go +++ b/internal/engine/netx/netx_test.go @@ -14,6 +14,7 @@ import ( "github.com/ooni/probe-cli/v3/internal/engine/netx/httptransport" "github.com/ooni/probe-cli/v3/internal/engine/netx/resolver" "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsdialer" + "github.com/ooni/probe-cli/v3/internal/engine/netx/tlsx" "github.com/ooni/probe-cli/v3/internal/engine/netx/trace" ) @@ -1188,7 +1189,7 @@ func TestNewDNSClientBadUDPEndpoint(t *testing.T) { func TestNewDNSCLientWithInvalidTLSVersion(t *testing.T) { _, err := netx.NewDNSClientWithOverrides( netx.Config{}, "dot://8.8.8.8", "", "", "TLSv999") - if !errors.Is(err, netx.ErrInvalidTLSVersion) { + if !errors.Is(err, tlsx.ErrInvalidTLSVersion) { t.Fatalf("not the error we expected: %+v", err) } }