Skip to content

Commit

Permalink
fix(enginenetx): store endpoint into the tactic (ooni#1292)
Browse files Browse the repository at this point in the history 
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 ooni/probe#2531
  • Loading branch information
@bassosimone @Murphy-OrangeMud
bassosimone authored and Murphy-OrangeMud committed Feb 13, 2024
1 parent 6ce0847 commit c5107f7
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 30 deletions.
3 changes: 2 additions & 1 deletion internal/enginenetx/httpsdialer_internal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/tls"
"errors"
"fmt"
"net"
"sync"
"sync/atomic"
"testing"
Expand All @@ -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",
Expand Down
48 changes: 36 additions & 12 deletions internal/enginenetx/httpsdialer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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)
}
})
}
54 changes: 39 additions & 15 deletions internal/enginenetx/httpsdialercore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -37,19 +37,45 @@ 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
// returns either a list of tactics for dialing or an error.
//
// 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.
Expand All @@ -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)
}

Expand Down Expand Up @@ -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
Expand All @@ -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)
}()
}

Expand Down Expand Up @@ -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
Expand All @@ -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():
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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,
)
Expand Down
5 changes: 3 additions & 2 deletions internal/enginenetx/httpsdialernull.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package enginenetx

import (
"context"
"net"
"time"

"github.com/ooni/probe-cli/v3/internal/model"
Expand All @@ -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
Expand All @@ -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,
Expand Down

0 comments on commit c5107f7

Please sign in to comment.