diff --git a/internal/enginenetx/httpsdialer_test.go b/internal/enginenetx/httpsdialer_test.go index 13b761b3e1..56f0fb3225 100644 --- a/internal/enginenetx/httpsdialer_test.go +++ b/internal/enginenetx/httpsdialer_test.go @@ -54,7 +54,7 @@ func (*httpsDialerCancelingContextStatsTracker) OnTLSHandshakeError(ctx context. } // OnTLSVerifyError implements enginenetx.HTTPSDialerStatsTracker. -func (*httpsDialerCancelingContextStatsTracker) OnTLSVerifyError(ctz context.Context, tactic *enginenetx.HTTPSDialerTactic, err error) { +func (*httpsDialerCancelingContextStatsTracker) OnTLSVerifyError(tactic *enginenetx.HTTPSDialerTactic, err error) { // nothing } diff --git a/internal/enginenetx/httpsdialercore.go b/internal/enginenetx/httpsdialercore.go index 189acc2005..84a9a0bd64 100644 --- a/internal/enginenetx/httpsdialercore.go +++ b/internal/enginenetx/httpsdialercore.go @@ -96,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(ctx context.Context, tactic *HTTPSDialerTactic, err error) + OnTLSVerifyError(tactic *HTTPSDialerTactic, err error) OnSuccess(tactic *HTTPSDialerTactic) } @@ -382,7 +382,7 @@ func (hd *HTTPSDialer) dialTLS( // handle verification error if err != nil { - hd.stats.OnTLSVerifyError(ctx, tactic, err) + hd.stats.OnTLSVerifyError(tactic, err) tlsConn.Close() return nil, err } diff --git a/internal/enginenetx/httpsdialernull.go b/internal/enginenetx/httpsdialernull.go index fccaba62ae..93022d83f1 100644 --- a/internal/enginenetx/httpsdialernull.go +++ b/internal/enginenetx/httpsdialernull.go @@ -80,6 +80,6 @@ func (*HTTPSDialerNullStatsTracker) OnTLSHandshakeError(ctx context.Context, tac } // OnTLSVerifyError implements HTTPSDialerStatsTracker. -func (*HTTPSDialerNullStatsTracker) OnTLSVerifyError(ctz context.Context, tactic *HTTPSDialerTactic, err error) { +func (*HTTPSDialerNullStatsTracker) OnTLSVerifyError(tactic *HTTPSDialerTactic, err error) { // nothing } diff --git a/internal/enginenetx/httpsdialerstats.go b/internal/enginenetx/httpsdialerstats.go new file mode 100644 index 0000000000..27a2142dee --- /dev/null +++ b/internal/enginenetx/httpsdialerstats.go @@ -0,0 +1,315 @@ +package enginenetx + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "sync" + "time" + + "github.com/ooni/probe-cli/v3/internal/model" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +// HTTPSDialerStatsTacticRecord keeps stats about an [HTTPSDialerTactic]. +type HTTPSDialerStatsTacticRecord struct { + // CountStarted counts the number of operations we started. + CountStarted int64 + + // CountTCPConnectError counts the number of TCP connect errors. + CountTCPConnectError int64 + + // CountTCPConnectInterrupt counts the number of interrupted TCP connect attempts. + CountTCPConnectInterrupt int64 + + // CountTLSHandshakeError counts the number of TLS handshake errors. + CountTLSHandshakeError int64 + + // CountTLSHandshakeInterrupt counts the number of interrupted TLS handshakes. + CountTLSHandshakeInterrupt int64 + + // CountTLSVerificationError counts the number of TLS verification errors. + CountTLSVerificationError int64 + + // CountSuccess counts the number of successes. + CountSuccess int64 + + // HistoTCPConnectError contains an histogram of TCP connect errors. + HistoTCPConnectError map[string]int64 + + // HistoTLSHandshakeError contains an histogram of TLS handshake errors. + HistoTLSHandshakeError map[string]int64 + + // HistoTLSVerificationError contains an histogram of TLS verification errors. + HistoTLSVerificationError map[string]int64 + + // LastUpdated is the last time we updated this record. + LastUpdated time.Time + + // Tactic is the underlying tactic. + Tactic *HTTPSDialerTactic +} + +// HTTPSDialerStatsTacticsContainer contains tactics. +type HTTPSDialerStatsTacticsContainer struct { + // Tactic maps the summary of a tactic to the tactic record. + Tactics map[string]*HTTPSDialerStatsTacticRecord +} + +// HTTPSDialerStatsContainerVersion is the current version of [HTTPSDialerStatsContainer]. +const HTTPSDialerStatsContainerVersion = 2 + +// HTTPSDialerStatsRootContainer is the root container for stats. +// +// The zero value is invalid; construct using [NewHTTPSDialerStatsRootContainer]. +type HTTPSDialerStatsRootContainer struct { + // Domains maps a domain name to its tactics + Domains map[string]*HTTPSDialerStatsTacticsContainer + + // Version is the version of the container data format. + Version int +} + +// Get returns the tactic record for the given [*HTTPSDialerTactic] instance. +// +// At the name implies, this function MUST be called while holding the [HTTPSDialerStatsManager] mutex. +func (c *HTTPSDialerStatsRootContainer) GetLocked(tactic *HTTPSDialerTactic) (*HTTPSDialerStatsTacticRecord, bool) { + domainRecord, found := c.Domains[tactic.VerifyHostname] + if !found { + return nil, false + } + tacticRecord, found := domainRecord.Tactics[tactic.Summary()] + return tacticRecord, found +} + +// Set sets the tactic record for the given the given [*HTTPSDialerTactic] instance. +// +// At the name implies, this function MUST be called while holding the [HTTPSDialerStatsManager] mutex. +func (c *HTTPSDialerStatsRootContainer) SetLocked(tactic *HTTPSDialerTactic, record *HTTPSDialerStatsTacticRecord) { + domainRecord, found := c.Domains[tactic.VerifyHostname] + if !found { + domainRecord = &HTTPSDialerStatsTacticsContainer{ + Tactics: map[string]*HTTPSDialerStatsTacticRecord{}, + } + + // make sure the map is initialized + if len(c.Domains) <= 0 { + c.Domains = make(map[string]*HTTPSDialerStatsTacticsContainer) + } + + c.Domains[tactic.VerifyHostname] = domainRecord + // fallthrough + } + domainRecord.Tactics[tactic.Summary()] = record +} + +// NewHTTPSDialerStatsRootContainer creates a new empty [*HTTPSDialerStatsRootContainer]. +func NewHTTPSDialerStatsRootContainer() *HTTPSDialerStatsRootContainer { + return &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + } +} + +// HTTPSDialerStatsManager implements [HTTPSDialerStatsTracker] by storing +// the relevant statistics in a [model.KeyValueStore]. +// +// The zero value of this structure is not ready to use; please, use the +// [NewHTTPSDialerStatsManager] factory to create a new instance. +type HTTPSDialerStatsManager struct { + // TimeNow is a field that allows you to override how we obtain the + // current time; modify this field BEFORE using this structure. + TimeNow func() time.Time + + // kvStore is the key-value store we're using + kvStore model.KeyValueStore + + // logger is the logger to use. + logger model.Logger + + // mu provides mutual exclusion when accessing the stats. + mu sync.Mutex + + // root is the root container for stats + root *HTTPSDialerStatsRootContainer +} + +// HTTPSDialerStatsKey is the key used in the key-value store to access the state. +const HTTPSDialerStatsKey = "httpsdialerstats.state" + +// errDialerStatsContainerWrongVersion means that the stats container document has the wrong version number. +var errDialerStatsContainerWrongVersion = errors.New("wrong stats container version") + +// loadHTTPSDialerStatsRootContainer loads a state container from the given key-value store. +func loadHTTPSDialerStatsRootContainer(kvStore model.KeyValueStore) (*HTTPSDialerStatsRootContainer, error) { + // load data from the kvstore + data, err := kvStore.Get(HTTPSDialerStatsKey) + if err != nil { + return nil, err + } + + // parse as JSON + var container HTTPSDialerStatsRootContainer + if err := json.Unmarshal(data, &container); err != nil { + return nil, err + } + + // make sure the version is OK + if container.Version != HTTPSDialerStatsContainerVersion { + err := fmt.Errorf( + "%s: %w: expected=%d got=%d", + HTTPSDialerStatsKey, + errDialerStatsContainerWrongVersion, + HTTPSDialerStatsContainerVersion, + container.Version, + ) + return nil, err + } + + return &container, nil +} + +// NewHTTPSDialerStatsManager constructs a new instance of [*HTTPSDialerStatsManager]. +func NewHTTPSDialerStatsManager(kvStore model.KeyValueStore, logger model.Logger) *HTTPSDialerStatsManager { + root, err := loadHTTPSDialerStatsRootContainer(kvStore) + if err != nil { + root = NewHTTPSDialerStatsRootContainer() + } + + return &HTTPSDialerStatsManager{ + TimeNow: time.Now, + root: root, + kvStore: kvStore, + logger: logger, + mu: sync.Mutex{}, + } +} + +var _ HTTPSDialerStatsTracker = &HTTPSDialerStatsManager{} + +// OnStarting implements HTTPSDialerStatsManager. +func (mt *HTTPSDialerStatsManager) OnStarting(tactic *HTTPSDialerTactic) { + // get exclusive access + defer mt.mu.Unlock() + mt.mu.Lock() + + // get the record + record, found := mt.root.GetLocked(tactic) + if !found { + record = &HTTPSDialerStatsTacticRecord{ + CountStarted: 0, + CountTCPConnectError: 0, + CountTCPConnectInterrupt: 0, + CountTLSHandshakeError: 0, + CountTLSHandshakeInterrupt: 0, + CountTLSVerificationError: 0, + CountSuccess: 0, + HistoTCPConnectError: map[string]int64{}, + HistoTLSHandshakeError: map[string]int64{}, + HistoTLSVerificationError: map[string]int64{}, + LastUpdated: time.Time{}, + Tactic: tactic.Clone(), // avoid storing the original + } + mt.root.SetLocked(tactic, record) + } + + // update stats + record.CountStarted++ + record.LastUpdated = mt.TimeNow() +} + +// OnTCPConnectError implements HTTPSDialerStatsManager. +func (mt *HTTPSDialerStatsManager) OnTCPConnectError(ctx context.Context, tactic *HTTPSDialerTactic, err error) { + // get exclusive access + defer mt.mu.Unlock() + mt.mu.Lock() + + // get the record + record, found := mt.root.GetLocked(tactic) + if !found { + mt.logger.Warnf("HTTPSDialerStatsManager.OnTCPConnectError: not found: %+v", tactic) + return + } + + // update stats + record.LastUpdated = mt.TimeNow() + if ctx.Err() != nil { + record.CountTCPConnectInterrupt++ + return + } + record.CountTCPConnectError++ + record.HistoTCPConnectError[err.Error()]++ +} + +// OnTLSHandshakeError implements HTTPSDialerStatsManager. +func (mt *HTTPSDialerStatsManager) OnTLSHandshakeError(ctx context.Context, tactic *HTTPSDialerTactic, err error) { + // get exclusive access + defer mt.mu.Unlock() + mt.mu.Lock() + + // get the record + record, found := mt.root.GetLocked(tactic) + if !found { + mt.logger.Warnf("HTTPSDialerStatsManager.OnTLSHandshakeError: not found: %+v", tactic) + return + } + + // update stats + record.LastUpdated = mt.TimeNow() + if ctx.Err() != nil { + record.CountTLSHandshakeInterrupt++ + return + } + record.CountTLSHandshakeError++ + record.HistoTLSHandshakeError[err.Error()]++ +} + +// OnTLSVerifyError implements HTTPSDialerStatsManager. +func (mt *HTTPSDialerStatsManager) OnTLSVerifyError(tactic *HTTPSDialerTactic, err error) { + // get exclusive access + defer mt.mu.Unlock() + mt.mu.Lock() + + // get the record + record, found := mt.root.GetLocked(tactic) + if !found { + mt.logger.Warnf("HTTPSDialerStatsManager.OnTLSVerificationError: not found: %+v", tactic) + return + } + + // update stats + record.CountTLSVerificationError++ + record.HistoTLSVerificationError[err.Error()]++ + record.LastUpdated = mt.TimeNow() +} + +// OnSuccess implements HTTPSDialerStatsManager. +func (mt *HTTPSDialerStatsManager) OnSuccess(tactic *HTTPSDialerTactic) { + // get exclusive access + defer mt.mu.Unlock() + mt.mu.Lock() + + // get the record + record, found := mt.root.GetLocked(tactic) + if !found { + mt.logger.Warnf("HTTPSDialerStatsManager.OnSuccess: not found: %+v", tactic) + return + } + + // update stats + record.CountSuccess++ + record.LastUpdated = mt.TimeNow() +} + +// Close implements io.Closer +func (mt *HTTPSDialerStatsManager) Close() error { + // TODO(bassosimone): do we need to apply a "once" semantics to this method? + + // get exclusive access + defer mt.mu.Unlock() + mt.mu.Lock() + + // write updated stats into the underlying key-value store + return mt.kvStore.Set(HTTPSDialerStatsKey, runtimex.Try1(json.Marshal(mt.root))) +} diff --git a/internal/enginenetx/httpsdialerstats_internal_test.go b/internal/enginenetx/httpsdialerstats_internal_test.go new file mode 100644 index 0000000000..c40b0e1f02 --- /dev/null +++ b/internal/enginenetx/httpsdialerstats_internal_test.go @@ -0,0 +1,416 @@ +package enginenetx + +import ( + "context" + "encoding/json" + "errors" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/mocks" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +func TestLoadHTTPSDialerStatsRootContainer(t *testing.T) { + type testcase struct { + // name is the test case name + name string + + // input returns the bytes we should Set into the key-value store + input func() []byte + + // expectedErr is the expected error string or an empty string + expectErr string + + // expectRoot is the expected root container content + expectRoot *HTTPSDialerStatsRootContainer + } + + cases := []testcase{{ + name: "when the key-value store does not contain any data", + input: func() []byte { + // Note that returning nil causes the code to NOT set anything into the kvstore + return nil + }, + expectErr: "no such key", + expectRoot: nil, + }, { + name: "when we cannot parse the serialized JSON", + input: func() []byte { + return []byte(`{`) + }, + expectErr: "unexpected end of JSON input", + expectRoot: nil, + }, { + name: "with invalid version", + input: func() []byte { + return []byte(`{"Version":1}`) + }, + expectErr: "httpsdialerstats.state: wrong stats container version: expected=2 got=1", + expectRoot: nil, + }, { + name: "on success", + input: func() []byte { + root := &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{ + "api.ooni.io": { + Tactics: map[string]*HTTPSDialerStatsTacticRecord{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 4, + CountTCPConnectError: 1, + CountTLSHandshakeError: 1, + CountTLSVerificationError: 1, + CountSuccess: 1, + HistoTCPConnectError: map[string]int64{ + "connection_refused": 1, + }, + HistoTLSHandshakeError: map[string]int64{ + "generic_timeout_error": 1, + }, + HistoTLSVerificationError: map[string]int64{ + "ssl_invalid_hostname": 1, + }, + LastUpdated: time.Date(2023, 9, 25, 0, 0, 0, 0, time.UTC), + Tactic: &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }, + }, + }, + }, + }, + Version: HTTPSDialerStatsContainerVersion, + } + return runtimex.Try1(json.Marshal(root)) + }, + expectErr: "", + expectRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{ + "api.ooni.io": { + Tactics: map[string]*HTTPSDialerStatsTacticRecord{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 4, + CountTCPConnectError: 1, + CountTLSHandshakeError: 1, + CountTLSVerificationError: 1, + CountSuccess: 1, + HistoTCPConnectError: map[string]int64{ + "connection_refused": 1, + }, + HistoTLSHandshakeError: map[string]int64{ + "generic_timeout_error": 1, + }, + HistoTLSVerificationError: map[string]int64{ + "ssl_invalid_hostname": 1, + }, + LastUpdated: time.Date(2023, 9, 25, 0, 0, 0, 0, time.UTC), + Tactic: &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }, + }, + }, + }, + }, + Version: HTTPSDialerStatsContainerVersion, + }, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + kvStore := &kvstore.Memory{} + if input := tc.input(); len(input) > 0 { + if err := kvStore.Set(HTTPSDialerStatsKey, input); err != nil { + t.Fatal(err) + } + } + + root, err := loadHTTPSDialerStatsRootContainer(kvStore) + + switch { + case err == nil && tc.expectErr == "": + // all good + + case err != nil && tc.expectErr == "": + t.Fatal("expected", tc.expectErr, "but got", err.Error()) + + case err == nil && tc.expectErr != "": + t.Fatal("expected", tc.expectErr, "but got", err) + + case err != nil && tc.expectErr != "": + if tc.expectErr != err.Error() { + t.Fatal("expected", tc.expectErr, "but got", err.Error()) + } + } + + if diff := cmp.Diff(tc.expectRoot, root); diff != "" { + t.Fatal(diff) + } + }) + } +} + +func TestHTTPSDialerStatsManagerCallbacks(t *testing.T) { + type testcase struct { + name string + initialRoot *HTTPSDialerStatsRootContainer + do func(stats *HTTPSDialerStatsManager) + expectWarnf int + expectRoot *HTTPSDialerStatsRootContainer + } + + cases := []testcase{ + + // When TCP connect fails and the reason is a canceled context + { + name: "OnTCPConnectError with ctx.Error() != nil", + initialRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{ + "api.ooni.io": { + Tactics: map[string]*HTTPSDialerStatsTacticRecord{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 1, + }, + }, + }, + }, + Version: HTTPSDialerStatsContainerVersion, + }, + do: func(stats *HTTPSDialerStatsManager) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // immediately! + + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTCPConnectError(ctx, tactic, err) + }, + expectWarnf: 0, + expectRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{ + "api.ooni.io": { + Tactics: map[string]*HTTPSDialerStatsTacticRecord{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 1, + CountTCPConnectInterrupt: 1, + }, + }, + }, + }, + Version: HTTPSDialerStatsContainerVersion, + }, + }, + + // When TCP connect fails and we don't already have a policy record + { + name: "OnTCPConnectError when we are missing the stats record for the domain", + initialRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + }, + do: func(stats *HTTPSDialerStatsManager) { + ctx := context.Background() + + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTCPConnectError(ctx, tactic, err) + }, + expectWarnf: 1, + expectRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + }, + }, + + // When TLS handshake fails and the reason is a canceled context + { + name: "OnTLSHandshakeError with ctx.Error() != nil", + initialRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{ + "api.ooni.io": { + Tactics: map[string]*HTTPSDialerStatsTacticRecord{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 1, + }, + }, + }, + }, + Version: HTTPSDialerStatsContainerVersion, + }, + do: func(stats *HTTPSDialerStatsManager) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() // immediately! + + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTLSHandshakeError(ctx, tactic, err) + }, + expectWarnf: 0, + expectRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{ + "api.ooni.io": { + Tactics: map[string]*HTTPSDialerStatsTacticRecord{ + "162.55.247.208:443 sni=www.example.com verify=api.ooni.io": { + CountStarted: 1, + CountTLSHandshakeInterrupt: 1, + }, + }, + }, + }, + Version: HTTPSDialerStatsContainerVersion, + }, + }, + + // When TLS handshake fails and we don't already have a policy record + { + name: "OnTLSHandshakeError when we are missing the stats record for the domain", + initialRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + }, + do: func(stats *HTTPSDialerStatsManager) { + ctx := context.Background() + + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTLSHandshakeError(ctx, tactic, err) + }, + expectWarnf: 1, + expectRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + }, + }, + + // When TLS verification fails and we don't already have a policy record + { + name: "OnTLSVerifyError when we are missing the stats record for the domain", + initialRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + }, + do: func(stats *HTTPSDialerStatsManager) { + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + err := errors.New("generic_timeout_error") + + stats.OnTLSVerifyError(tactic, err) + }, + expectWarnf: 1, + expectRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + }, + }, + + // With success when we don't already have a policy record + { + name: "OnSuccess when we are missing the stats record for the domain", + initialRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + }, + do: func(stats *HTTPSDialerStatsManager) { + tactic := &HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + } + + stats.OnSuccess(tactic) + }, + expectWarnf: 1, + expectRoot: &HTTPSDialerStatsRootContainer{ + Domains: map[string]*HTTPSDialerStatsTacticsContainer{}, + Version: HTTPSDialerStatsContainerVersion, + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // configure the initial value of the stats + kvStore := &kvstore.Memory{} + if err := kvStore.Set(HTTPSDialerStatsKey, runtimex.Try1(json.Marshal(tc.initialRoot))); err != nil { + t.Fatal(err) + } + + // create logger counting the number Warnf invocations + var warnfCount int + logger := &mocks.Logger{ + MockWarnf: func(format string, v ...any) { + warnfCount++ + }, + } + + // create the stats manager + stats := NewHTTPSDialerStatsManager(kvStore, logger) + + // invoke the proper stats callback + tc.do(stats) + + // close the stats to trigger a kvstore write + if err := stats.Close(); err != nil { + t.Fatal(err) + } + + // extract the possibly modified stats from the kvstore + var root *HTTPSDialerStatsRootContainer + rawRoot, err := kvStore.Get(HTTPSDialerStatsKey) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(rawRoot, &root); err != nil { + t.Fatal(err) + } + + // make sure the stats are the ones we expect + diffOptions := []cmp.Option{ + cmpopts.IgnoreFields(HTTPSDialerStatsTacticRecord{}, "LastUpdated"), + } + if diff := cmp.Diff(tc.expectRoot, root, diffOptions...); diff != "" { + t.Fatal(diff) + } + + // make sure we logged if necessary + if tc.expectWarnf != warnfCount { + t.Fatal("expected", tc.expectWarnf, "got", warnfCount) + } + }) + } +} diff --git a/internal/enginenetx/httpsdialerstats_test.go b/internal/enginenetx/httpsdialerstats_test.go new file mode 100644 index 0000000000..3ff5fdb5b2 --- /dev/null +++ b/internal/enginenetx/httpsdialerstats_test.go @@ -0,0 +1,279 @@ +package enginenetx_test + +import ( + "encoding/json" + "net" + "testing" + "time" + + "github.com/apex/log" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/ooni/netem" + "github.com/ooni/probe-cli/v3/internal/bytecounter" + "github.com/ooni/probe-cli/v3/internal/enginenetx" + "github.com/ooni/probe-cli/v3/internal/kvstore" + "github.com/ooni/probe-cli/v3/internal/netemx" + "github.com/ooni/probe-cli/v3/internal/netxlite" + "github.com/ooni/probe-cli/v3/internal/runtimex" +) + +func TestHTTPSDialerCollectStats(t *testing.T) { + // testcase is a test case run by this function + type testcase struct { + // name is the test case name + name string + + // URL is the URL to GET + URL string + + // initialPolicy is the initial policy to configure into the key-value store + initialPolicy func() []byte + + // configureDPI is the function to configure DPI + configureDPI func(dpi *netem.DPIEngine) + + // expectErr is the expected error string + expectErr string + + // statsDomain is the domain to lookup inside the stats + statsDomain string + + // statsTacticsSummary is the summary to lookup inside the stats + // once we have used the statsDomain to get a record + statsTacticsSummary string + + // expectStats contains the expected record containing tactics stats + expectStats *enginenetx.HTTPSDialerStatsTacticRecord + } + + cases := []testcase{ + + { + name: "with TCP connect failure", + URL: "https://api.ooni.io/", + initialPolicy: func() []byte { + p0 := &enginenetx.HTTPSDialerStaticPolicyRoot{ + Domains: map[string][]*enginenetx.HTTPSDialerTactic{ + // This policy has a different SNI and VerifyHostname, which gives + // us confidence that the stats are using the latter + "api.ooni.io": {{ + Endpoint: net.JoinHostPort(netemx.AddressApiOONIIo, "443"), + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }}, + }, + Version: enginenetx.HTTPSDialerStaticPolicyVersion, + } + return runtimex.Try1(json.Marshal(p0)) + }, + configureDPI: func(dpi *netem.DPIEngine) { + dpi.AddRule(&netem.DPICloseConnectionForServerEndpoint{ + Logger: log.Log, + ServerIPAddress: netemx.AddressApiOONIIo, + ServerPort: 443, + }) + }, + expectErr: `Get "https://api.ooni.io/": connection_refused`, + statsDomain: "api.ooni.io", + statsTacticsSummary: "162.55.247.208:443 sni=www.example.com verify=api.ooni.io", + expectStats: &enginenetx.HTTPSDialerStatsTacticRecord{ + CountStarted: 1, + CountTCPConnectError: 1, + CountTLSHandshakeError: 0, + CountTLSVerificationError: 0, + CountSuccess: 0, + HistoTCPConnectError: map[string]int64{ + "connection_refused": 1, + }, + HistoTLSHandshakeError: map[string]int64{}, + HistoTLSVerificationError: map[string]int64{}, + LastUpdated: time.Time{}, + Tactic: &enginenetx.HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }, + }, + }, + + { + name: "with TLS handshake failure", + URL: "https://api.ooni.io/", + initialPolicy: func() []byte { + p0 := &enginenetx.HTTPSDialerStaticPolicyRoot{ + Domains: map[string][]*enginenetx.HTTPSDialerTactic{ + // This policy has a different SNI and VerifyHostname, which gives + // us confidence that the stats are using the latter + "api.ooni.io": {{ + Endpoint: net.JoinHostPort(netemx.AddressApiOONIIo, "443"), + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }}, + }, + Version: enginenetx.HTTPSDialerStaticPolicyVersion, + } + return runtimex.Try1(json.Marshal(p0)) + }, + configureDPI: func(dpi *netem.DPIEngine) { + dpi.AddRule(&netem.DPIResetTrafficForTLSSNI{ + Logger: log.Log, + SNI: "www.example.com", + }) + }, + expectErr: `Get "https://api.ooni.io/": connection_reset`, + statsDomain: "api.ooni.io", + statsTacticsSummary: "162.55.247.208:443 sni=www.example.com verify=api.ooni.io", + expectStats: &enginenetx.HTTPSDialerStatsTacticRecord{ + CountStarted: 1, + CountTCPConnectError: 0, + CountTLSHandshakeError: 1, + CountTLSVerificationError: 0, + CountSuccess: 0, + HistoTCPConnectError: map[string]int64{}, + HistoTLSHandshakeError: map[string]int64{ + "connection_reset": 1, + }, + HistoTLSVerificationError: map[string]int64{}, + LastUpdated: time.Time{}, + Tactic: &enginenetx.HTTPSDialerTactic{ + Endpoint: "162.55.247.208:443", + InitialDelay: 0, + SNI: "www.example.com", + VerifyHostname: "api.ooni.io", + }, + }, + }, + + { + name: "with TLS verification failure", + URL: "https://api.ooni.io/", + initialPolicy: func() []byte { + p0 := &enginenetx.HTTPSDialerStaticPolicyRoot{ + Domains: map[string][]*enginenetx.HTTPSDialerTactic{ + // This policy has a different SNI and VerifyHostname, which gives + // us confidence that the stats are using the latter + "api.ooni.io": {{ + Endpoint: net.JoinHostPort(netemx.AddressBadSSLCom, "443"), + InitialDelay: 0, + SNI: "untrusted-root.badssl.com", + VerifyHostname: "api.ooni.io", + }}, + }, + Version: enginenetx.HTTPSDialerStaticPolicyVersion, + } + return runtimex.Try1(json.Marshal(p0)) + }, + configureDPI: func(dpi *netem.DPIEngine) { + // nothing + }, + expectErr: `Get "https://api.ooni.io/": ssl_invalid_hostname`, + statsDomain: "api.ooni.io", + statsTacticsSummary: "104.154.89.105:443 sni=untrusted-root.badssl.com verify=api.ooni.io", + expectStats: &enginenetx.HTTPSDialerStatsTacticRecord{ + CountStarted: 1, + CountTCPConnectError: 0, + CountTLSHandshakeError: 0, + CountTLSVerificationError: 1, + CountSuccess: 0, + HistoTCPConnectError: map[string]int64{}, + HistoTLSHandshakeError: map[string]int64{}, + HistoTLSVerificationError: map[string]int64{ + "ssl_invalid_hostname": 1, + }, + LastUpdated: time.Time{}, + Tactic: &enginenetx.HTTPSDialerTactic{ + Endpoint: "104.154.89.105:443", + InitialDelay: 0, + SNI: "untrusted-root.badssl.com", + VerifyHostname: "api.ooni.io", + }, + }, + }} + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + qa := netemx.MustNewScenario(netemx.InternetScenario) + defer qa.Close() + + // make sure we apply specific DPI rules + tc.configureDPI(qa.DPIEngine()) + + // create a memory key-value store where the engine will write stats that later we + // would be able to read to confirm we're collecting stats + kvStore := &kvstore.Memory{} + + initialPolicy := tc.initialPolicy() + t.Logf("initialPolicy: %s", string(initialPolicy)) + if err := kvStore.Set(enginenetx.HTTPSDialerStaticPolicyKey, initialPolicy); err != nil { + t.Fatal(err) + } + + qa.Do(func() { + byteCounter := bytecounter.New() + resolver := netxlite.NewStdlibResolver(log.Log) + + netx := enginenetx.NewNetwork(byteCounter, kvStore, log.Log, nil, resolver) + defer netx.Close() + + client := netx.NewHTTPClient() + + resp, err := client.Get(tc.URL) + + switch { + case err == nil && tc.expectErr == "": + // all good + + case err != nil && tc.expectErr == "": + t.Fatal("expected", tc.expectErr, "but got", err.Error()) + + case err == nil && tc.expectErr != "": + t.Fatal("expected", tc.expectErr, "but got", err) + + case err != nil && tc.expectErr != "": + if tc.expectErr != err.Error() { + t.Fatal("expected", tc.expectErr, "but got", err.Error()) + } + } + + if resp != nil { + defer resp.Body.Close() + } + }) + + // obtain the tactics container for the proper domain + rawStats, err := kvStore.Get(enginenetx.HTTPSDialerStatsKey) + if err != nil { + t.Fatal(err) + } + var rootStats enginenetx.HTTPSDialerStatsRootContainer + if err := json.Unmarshal(rawStats, &rootStats); err != nil { + t.Fatal(err) + } + tactics, good := rootStats.Domains[tc.statsDomain] + if !good { + t.Fatalf("no such record for `%s`", tc.statsDomain) + } + t.Logf("%+v", tactics) + + // we expect to see a single record + if len(tactics.Tactics) != 1 { + t.Fatal("expected a single tactic") + } + tactic, good := tactics.Tactics[tc.statsTacticsSummary] + if !good { + t.Fatalf("no such record for: %s", tc.statsTacticsSummary) + } + + diffOptions := []cmp.Option{ + cmpopts.IgnoreFields(enginenetx.HTTPSDialerStatsTacticRecord{}, "LastUpdated"), + } + if diff := cmp.Diff(tc.expectStats, tactic, diffOptions...); diff != "" { + t.Fatal(diff) + } + }) + } +} diff --git a/internal/enginenetx/network.go b/internal/enginenetx/network.go index 9e110ec100..e6b23a4290 100644 --- a/internal/enginenetx/network.go +++ b/internal/enginenetx/network.go @@ -14,7 +14,8 @@ import ( // Network is the network abstraction used by the OONI engine. type Network struct { - txp model.HTTPTransport + stats *HTTPSDialerStatsManager + txp model.HTTPTransport } // HTTPTransport returns the [model.HTTPTransport] that the engine should use. @@ -41,7 +42,8 @@ func (n *Network) Close() error { // make sure we close the transport's idle connections n.txp.CloseIdleConnections() - return nil + // make sure we sync stats to disk + return n.stats.Close() } // NewNetwork creates a new [*Network] for the engine. This network MUST NOT be @@ -77,19 +79,17 @@ func NewNetwork( // reasonably fine to use the legacy sequential dialer implemented in netxlite. dialer := netxlite.NewDialerWithResolver(logger, resolver) + // Create manager for keeping track of statistics + stats := NewHTTPSDialerStatsManager(kvStore, logger) + // Create a TLS dialer ONLY used for dialing TLS connections. This dialer will use // happy-eyeballs and possibly custom policies for dialing TLS connections. - // - // Additionally, please note the following limitations (to be overcome through - // future refactoring of this func): - // - // - for now, we're using a "null" stats tracker, meaning we don't track stats. httpsDialer := NewHTTPSDialer( logger, &netxlite.Netx{Underlying: nil}, // nil means using netxlite's singleton newHTTPSDialerPolicy(kvStore), resolver, - &HTTPSDialerNullStatsTracker{}, + stats, ) // Here we're creating a "new style" HTTPS transport, which has less @@ -122,7 +122,11 @@ func NewNetwork( // Make sure we count the bytes sent and received as part of the session txp = bytecounter.WrapHTTPTransport(txp, counter) - return &Network{txp} + netx := &Network{ + stats: stats, + txp: txp, + } + return netx } // newHTTPSDialerPolicy contains the logic to select the [HTTPSDialerPolicy] to use. diff --git a/internal/enginenetx/network_internal_test.go b/internal/enginenetx/network_internal_test.go index 511899cdd5..b21d8e75be 100644 --- a/internal/enginenetx/network_internal_test.go +++ b/internal/enginenetx/network_internal_test.go @@ -1,9 +1,13 @@ package enginenetx import ( + "sync" "testing" + "time" + "github.com/ooni/probe-cli/v3/internal/kvstore" "github.com/ooni/probe-cli/v3/internal/mocks" + "github.com/ooni/probe-cli/v3/internal/model" ) func TestNetworkUnit(t *testing.T) { @@ -22,7 +26,16 @@ func TestNetworkUnit(t *testing.T) { called = true }, } - netx := &Network{txp: expected} + netx := &Network{ + stats: &HTTPSDialerStatsManager{ + TimeNow: time.Now, + kvStore: &kvstore.Memory{}, + logger: model.DiscardLogger, + mu: sync.Mutex{}, + root: &HTTPSDialerStatsRootContainer{}, + }, + txp: expected, + } if err := netx.Close(); err != nil { t.Fatal(err) }