From c5107f7ea0b40887d8a6287b34a2e2cabbe27a77 Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 21 Sep 2023 18:08:07 +0200 Subject: [PATCH] fix(enginenetx): store endpoint into the tactic (#1292) We cannot evaluate tactics on an IP address basis because different ports may cause tactics to fail. In principle we will probably only use port 443/tcp here, though it is better to avoid keeping lame code around. While there, start preparing for inserting the tactics into a map to keep statistics. Part of https://github.com/ooni/probe/issues/2531 --- .../enginenetx/httpsdialer_internal_test.go | 3 +- internal/enginenetx/httpsdialer_test.go | 48 ++++++++++++----- internal/enginenetx/httpsdialercore.go | 54 +++++++++++++------ internal/enginenetx/httpsdialernull.go | 5 +- 4 files changed, 80 insertions(+), 30 deletions(-) diff --git a/internal/enginenetx/httpsdialer_internal_test.go b/internal/enginenetx/httpsdialer_internal_test.go index 07162db4e0..3b240113d7 100644 --- a/internal/enginenetx/httpsdialer_internal_test.go +++ b/internal/enginenetx/httpsdialer_internal_test.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "errors" "fmt" + "net" "sync" "sync/atomic" "testing" @@ -29,7 +30,7 @@ func TestHTTPSDialerTacticsEmitter(t *testing.T) { var tactics []*HTTPSDialerTactic for idx := 0; idx < 255; idx++ { tactics = append(tactics, &HTTPSDialerTactic{ - IPAddr: fmt.Sprintf("10.0.0.%d", idx), + Endpoint: net.JoinHostPort(fmt.Sprintf("10.0.0.%d", idx), "443"), InitialDelay: 0, SNI: "www.example.com", VerifyHostname: "www.example.com", diff --git a/internal/enginenetx/httpsdialer_test.go b/internal/enginenetx/httpsdialer_test.go index 1ee5a05e55..78d5894201 100644 --- a/internal/enginenetx/httpsdialer_test.go +++ b/internal/enginenetx/httpsdialer_test.go @@ -476,27 +476,27 @@ func TestLoadHTTPSDialerPolicy(t *testing.T) { return runtimex.Try1(json.Marshal(&enginenetx.HTTPSDialerLoadablePolicy{ Domains: map[string][]*enginenetx.HTTPSDialerTactic{ "api.ooni.io": {{ - IPAddr: "162.55.247.208", + Endpoint: "162.55.247.208:443", InitialDelay: 0, SNI: "api.ooni.io", VerifyHostname: "api.ooni.io", }, { - IPAddr: "46.101.82.151", + Endpoint: "46.101.82.151:443", InitialDelay: 300 * time.Millisecond, SNI: "api.ooni.io", VerifyHostname: "api.ooni.io", }, { - IPAddr: "2a03:b0c0:1:d0::ec4:9001", + Endpoint: "[2a03:b0c0:1:d0::ec4:9001]:443", InitialDelay: 600 * time.Millisecond, SNI: "api.ooni.io", VerifyHostname: "api.ooni.io", }, { - IPAddr: "46.101.82.151", + Endpoint: "46.101.82.151:443", InitialDelay: 3000 * time.Millisecond, SNI: "www.example.com", VerifyHostname: "api.ooni.io", }, { - IPAddr: "2a03:b0c0:1:d0::ec4:9001", + Endpoint: "[2a03:b0c0:1:d0::ec4:9001]:443", InitialDelay: 3300 * time.Millisecond, SNI: "www.example.com", VerifyHostname: "api.ooni.io", @@ -508,27 +508,27 @@ func TestLoadHTTPSDialerPolicy(t *testing.T) { expectedPolicy: &enginenetx.HTTPSDialerLoadablePolicy{ Domains: map[string][]*enginenetx.HTTPSDialerTactic{ "api.ooni.io": {{ - IPAddr: "162.55.247.208", + Endpoint: "162.55.247.208:443", InitialDelay: 0, SNI: "api.ooni.io", VerifyHostname: "api.ooni.io", }, { - IPAddr: "46.101.82.151", + Endpoint: "46.101.82.151:443", InitialDelay: 300 * time.Millisecond, SNI: "api.ooni.io", VerifyHostname: "api.ooni.io", }, { - IPAddr: "2a03:b0c0:1:d0::ec4:9001", + Endpoint: "[2a03:b0c0:1:d0::ec4:9001]:443", InitialDelay: 600 * time.Millisecond, SNI: "api.ooni.io", VerifyHostname: "api.ooni.io", }, { - IPAddr: "46.101.82.151", + Endpoint: "46.101.82.151:443", InitialDelay: 3000 * time.Millisecond, SNI: "www.example.com", VerifyHostname: "api.ooni.io", }, { - IPAddr: "2a03:b0c0:1:d0::ec4:9001", + Endpoint: "[2a03:b0c0:1:d0::ec4:9001]:443", InitialDelay: 3300 * time.Millisecond, SNI: "www.example.com", VerifyHostname: "api.ooni.io", @@ -566,9 +566,9 @@ func TestLoadHTTPSDialerPolicy(t *testing.T) { func TestHTTPSDialerTactic(t *testing.T) { t.Run("String", func(t *testing.T) { - expected := `{"IPAddr":"162.55.247.208","InitialDelay":150000000,"SNI":"www.example.com","VerifyHostname":"api.ooni.io"}` + expected := `{"Endpoint":"162.55.247.208:443","InitialDelay":150000000,"SNI":"www.example.com","VerifyHostname":"api.ooni.io"}` ldt := &enginenetx.HTTPSDialerTactic{ - IPAddr: "162.55.247.208", + Endpoint: "162.55.247.208:443", InitialDelay: 150 * time.Millisecond, SNI: "www.example.com", VerifyHostname: "api.ooni.io", @@ -578,4 +578,28 @@ func TestHTTPSDialerTactic(t *testing.T) { t.Fatal(diff) } }) + + t.Run("Clone", func(t *testing.T) { + ff := &testingx.FakeFiller{} + var expect enginenetx.HTTPSDialerTactic + ff.Fill(&expect) + got := expect.Clone() + if diff := cmp.Diff(expect.String(), got.String()); diff != "" { + t.Fatal(diff) + } + }) + + t.Run("Summary", func(t *testing.T) { + expected := `162.55.247.208:443 sni=www.example.com verify=api.ooni.io` + ldt := &enginenetx.HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 150 * time.Millisecond, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + got := ldt.Summary() + if diff := cmp.Diff(expected, got); diff != "" { + t.Fatal(diff) + } + }) } diff --git a/internal/enginenetx/httpsdialercore.go b/internal/enginenetx/httpsdialercore.go index aca0b49780..e410d69b73 100644 --- a/internal/enginenetx/httpsdialercore.go +++ b/internal/enginenetx/httpsdialercore.go @@ -20,8 +20,8 @@ import ( // HTTPSDialerTactic is a tactic to establish a TLS connection. type HTTPSDialerTactic struct { - // IPAddr is the IP address to use for dialing. - IPAddr string + // Endpoint is the TCP endpoint to use for dialing. + Endpoint string // InitialDelay is the time in nanoseconds after which // you would like to start this policy. @@ -37,11 +37,37 @@ type HTTPSDialerTactic struct { var _ fmt.Stringer = &HTTPSDialerTactic{} +// Clone makes a deep copy of this [HTTPSDialerTactic]. +func (dt *HTTPSDialerTactic) Clone() *HTTPSDialerTactic { + return &HTTPSDialerTactic{ + Endpoint: dt.Endpoint, + InitialDelay: dt.InitialDelay, + SNI: dt.SNI, + VerifyHostname: dt.VerifyHostname, + } +} + // String implements fmt.Stringer. func (dt *HTTPSDialerTactic) String() string { return string(runtimex.Try1(json.Marshal(dt))) } +// Summary returns a string summarizing this [HTTPSDialerTactic] for the +// specific purpose of inserting the struct into a map. +// +// The fields used to compute the summary are: +// +// - IPAddr +// +// - SNI +// +// - VerifyHostname +// +// The returned string contains the above fields separated by space. +func (dt *HTTPSDialerTactic) Summary() string { + return fmt.Sprintf("%v sni=%v verify=%v", dt.Endpoint, dt.SNI, dt.VerifyHostname) +} + // HTTPSDialerPolicy describes the policy used by the [*HTTPSDialer]. type HTTPSDialerPolicy interface { // LookupTactics performs a DNS lookup for the given domain using the given resolver and @@ -49,7 +75,7 @@ type HTTPSDialerPolicy interface { // // This function MUST NOT return an empty list and a nil error. If this happens the // code inside [HTTPSDialer] will PANIC. - LookupTactics(ctx context.Context, domain string, reso model.Resolver) ([]*HTTPSDialerTactic, error) + LookupTactics(ctx context.Context, domain, port string, reso model.Resolver) ([]*HTTPSDialerTactic, error) // Parallelism returns the number of goroutines to create when TLS dialing. The // [HTTPSDialer] will PANIC if the returned number is less than 1. @@ -70,7 +96,7 @@ type HTTPSDialerStatsTracker interface { OnStarting(tactic *HTTPSDialerTactic) OnTCPConnectError(ctx context.Context, tactic *HTTPSDialerTactic, err error) OnTLSHandshakeError(ctx context.Context, tactic *HTTPSDialerTactic, err error) - OnTLSVerifyError(ctz context.Context, tactic *HTTPSDialerTactic, err error) + OnTLSVerifyError(ctx context.Context, tactic *HTTPSDialerTactic, err error) OnSuccess(tactic *HTTPSDialerTactic) } @@ -183,8 +209,8 @@ func (hd *HTTPSDialer) DialTLSContext(ctx context.Context, network string, endpo Prefix: fmt.Sprintf("[#%d] ", hd.idGenerator.Add(1)), Logger: hd.logger, } - ol := logx.NewOperationLogger(logger, "LookupTactics: %s", hostname) - tactics, err := hd.policy.LookupTactics(ctx, hostname, hd.resolver) + ol := logx.NewOperationLogger(logger, "LookupTactics: %s", net.JoinHostPort(hostname, port)) + tactics, err := hd.policy.LookupTactics(ctx, hostname, port, hd.resolver) if err != nil { ol.Stop(err) return nil, err @@ -201,7 +227,7 @@ func (hd *HTTPSDialer) DialTLSContext(ctx context.Context, network string, endpo hd.wg.Add(1) go func() { defer hd.wg.Done() - hd.worker(ctx, hostname, emitter, port, collector) + hd.worker(ctx, hostname, emitter, collector) }() } @@ -259,7 +285,6 @@ func (hd *HTTPSDialer) worker( ctx context.Context, hostname string, reader <-chan *HTTPSDialerTactic, - port string, writer chan<- *httpsDialerErrorOrConn, ) { // Note: no need to be concerned with the wait group here because @@ -277,7 +302,7 @@ func (hd *HTTPSDialer) worker( Prefix: fmt.Sprintf("[#%d] ", hd.idGenerator.Add(1)), Logger: hd.logger, } - conn, err := hd.dialTLS(ctx, logger, tactic, port) + conn, err := hd.dialTLS(ctx, logger, tactic) select { case <-ctx.Done(): @@ -297,8 +322,8 @@ func (hd *HTTPSDialer) worker( } // dialTLS performs the actual TLS dial. -func (hd *HTTPSDialer) dialTLS(ctx context.Context, - logger model.Logger, tactic *HTTPSDialerTactic, port string) (model.TLSConn, error) { +func (hd *HTTPSDialer) dialTLS( + ctx context.Context, logger model.Logger, tactic *HTTPSDialerTactic) (model.TLSConn, error) { // wait for the tactic to be ready to run if err := httpsDialerTacticWaitReady(ctx, tactic); err != nil { return nil, err @@ -311,10 +336,9 @@ func (hd *HTTPSDialer) dialTLS(ctx context.Context, netx := &netxlite.Netx{Underlying: hd.unet} // create dialer and establish TCP connection - endpoint := net.JoinHostPort(tactic.IPAddr, port) - ol := logx.NewOperationLogger(logger, "TCPConnect %s", endpoint) + ol := logx.NewOperationLogger(logger, "TCPConnect %s", tactic.Endpoint) dialer := netx.NewDialerWithoutResolver(logger) - tcpConn, err := dialer.DialContext(ctx, "tcp", endpoint) + tcpConn, err := dialer.DialContext(ctx, "tcp", tactic.Endpoint) ol.Stop(err) // handle a dialing error @@ -335,7 +359,7 @@ func (hd *HTTPSDialer) dialTLS(ctx context.Context, ol = logx.NewOperationLogger( logger, "TLSHandshake with %s SNI=%s ALPN=%v", - endpoint, + tactic.Endpoint, tlsConfig.ServerName, tlsConfig.NextProtos, ) diff --git a/internal/enginenetx/httpsdialernull.go b/internal/enginenetx/httpsdialernull.go index 5582c8a13b..1a26f53298 100644 --- a/internal/enginenetx/httpsdialernull.go +++ b/internal/enginenetx/httpsdialernull.go @@ -2,6 +2,7 @@ package enginenetx import ( "context" + "net" "time" "github.com/ooni/probe-cli/v3/internal/model" @@ -23,7 +24,7 @@ var _ HTTPSDialerPolicy = &HTTPSDialerNullPolicy{} // LookupTactics implements HTTPSDialerPolicy. func (*HTTPSDialerNullPolicy) LookupTactics( - ctx context.Context, domain string, reso model.Resolver) ([]*HTTPSDialerTactic, error) { + ctx context.Context, domain, port string, reso model.Resolver) ([]*HTTPSDialerTactic, error) { addrs, err := reso.LookupHost(ctx, domain) if err != nil { return nil, err @@ -33,7 +34,7 @@ func (*HTTPSDialerNullPolicy) LookupTactics( var tactics []*HTTPSDialerTactic for idx, addr := range addrs { tactics = append(tactics, &HTTPSDialerTactic{ - IPAddr: addr, + Endpoint: net.JoinHostPort(addr, port), InitialDelay: time.Duration(idx) * delay, // zero for the first dial SNI: domain, VerifyHostname: domain,