Skip to content

Commit

Permalink
fix(webconnectivitylte): include network events (#1503)
Browse files Browse the repository at this point in the history
While there, notice that we can increase the coverage in
webconnectivityqa.

Closes ooni/probe#2674
  • Loading branch information
bassosimone authored Feb 9, 2024
1 parent 912c3e6 commit 4439cbd
Show file tree
Hide file tree
Showing 29 changed files with 512 additions and 140 deletions.
9 changes: 8 additions & 1 deletion internal/experiment/webconnectivitylte/cleartextflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,14 @@ func (t *CleartextFlow) Run(parentCtx context.Context, index int64) error {
tcpDialer := trace.NewDialerWithoutResolver(t.Logger)
tcpConn, err := tcpDialer.DialContext(tcpCtx, "tcp", t.Address)
t.TestKeys.AppendTCPConnectResults(trace.TCPConnects()...)
defer t.TestKeys.AppendNetworkEvents(trace.NetworkEvents()...) // here to include "connect" events
defer func() {
// BUGFIX: we must call trace.NetworkEvents()... inside the defer block otherwise
// we miss the read/write network events. See https://github.com/ooni/probe/issues/2674.
//
// Additionally, we must register this defer here because we want to include
// the "connect" event in case connect has failed.
t.TestKeys.AppendNetworkEvents(trace.NetworkEvents()...)
}()
if err != nil {
ol.Stop(err)
return err
Expand Down
9 changes: 8 additions & 1 deletion internal/experiment/webconnectivitylte/secureflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,14 @@ func (t *SecureFlow) Run(parentCtx context.Context, index int64) error {
tcpDialer := trace.NewDialerWithoutResolver(t.Logger)
tcpConn, err := tcpDialer.DialContext(tcpCtx, "tcp", t.Address)
t.TestKeys.AppendTCPConnectResults(trace.TCPConnects()...)
defer t.TestKeys.AppendNetworkEvents(trace.NetworkEvents()...) // here to include "connect" events
defer func() {
// BUGFIX: we must call trace.NetworkEvents()... inside the defer block otherwise
// we miss the read/write network events. See https://github.com/ooni/probe/issues/2674.
//
// Additionally, we must register this defer here because we want to include
// the "connect" event in case connect has failed.
t.TestKeys.AppendNetworkEvents(trace.NetworkEvents()...)
}()
if err != nil {
ol.Stop(err)
return err
Expand Down
8 changes: 4 additions & 4 deletions internal/webconnectivityqa/badssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func badSSLWithExpiredCertificate() *TestCase {
// nothing
},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "consistent",
HTTPExperimentFailure: "ssl_invalid_certificate",
XStatus: 16, // StatusAnomalyControlFailure
Expand All @@ -38,7 +38,7 @@ func badSSLWithWrongServerName() *TestCase {
// nothing
},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "consistent",
HTTPExperimentFailure: "ssl_invalid_hostname",
XStatus: 16, // StatusAnomalyControlFailure
Expand All @@ -59,7 +59,7 @@ func badSSLWithUnknownAuthorityWithConsistentDNS() *TestCase {
// nothing
},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "consistent",
HTTPExperimentFailure: "ssl_unknown_authority",
XStatus: 16, // StatusAnomalyControlFailure
Expand Down Expand Up @@ -88,7 +88,7 @@ func badSSLWithUnknownAuthorityWithInconsistentDNS() *TestCase {

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "inconsistent",
HTTPExperimentFailure: "ssl_unknown_authority",
XStatus: 9248, // StatusExperimentHTTP | StatusAnomalyTLSHandshake | StatusAnomalyDNS
Expand Down
61 changes: 61 additions & 0 deletions internal/webconnectivityqa/checker.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package webconnectivityqa

import (
"errors"
"strings"

"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/must"
"github.com/ooni/probe-cli/v3/internal/netxlite"
"github.com/ooni/probe-cli/v3/internal/x/dslx"
)

// Checker checks whether a measurement is correct.
type Checker interface {
Check(mx *model.Measurement) error
}

// ReadWriteEventsExistentialChecker fails if there are zero network events.
type ReadWriteEventsExistentialChecker struct{}

var _ Checker = &ReadWriteEventsExistentialChecker{}

// ErrCheckerNoReadWriteEvents indicates that a checker did not find any read/write events.
var ErrCheckerNoReadWriteEvents = errors.New("no read or write events")

// ErrCheckerUnexpectedWebConnectivityVersion indicates that the version is unexpected
var ErrCheckerUnexpectedWebConnectivityVersion = errors.New("unexpected Web Connectivity version")

// Check implements Checker.
func (*ReadWriteEventsExistentialChecker) Check(mx *model.Measurement) error {
// we don't care about v0.4
if strings.HasPrefix(mx.TestVersion, "0.4.") {
return nil
}

// make sure it's v0.5
if !strings.HasPrefix(mx.TestVersion, "0.5.") {
return ErrCheckerUnexpectedWebConnectivityVersion
}

// serialize and reparse the test keys
var tk *dslx.Observations
must.UnmarshalJSON(must.MarshalJSON(mx.TestKeys), &tk)

// count the read/write events
var count int
for _, ev := range tk.NetworkEvents {
switch ev.Operation {
case netxlite.ReadOperation, netxlite.WriteOperation:
count++
default:
// nothing
}
}

// make sure there's at least one network event
if count <= 0 {
return ErrCheckerNoReadWriteEvents
}
return nil
}
112 changes: 112 additions & 0 deletions internal/webconnectivityqa/checker_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package webconnectivityqa_test

import (
"context"
"errors"
"testing"

"github.com/ooni/probe-cli/v3/internal/experiment/webconnectivitylte"
"github.com/ooni/probe-cli/v3/internal/mocks"
"github.com/ooni/probe-cli/v3/internal/model"
"github.com/ooni/probe-cli/v3/internal/must"
"github.com/ooni/probe-cli/v3/internal/optional"
"github.com/ooni/probe-cli/v3/internal/webconnectivityqa"
)

func TestConfigureCustomCheckers(t *testing.T) {
tc := &webconnectivityqa.TestCase{
Name: "",
Input: "",
ExpectErr: false,
ExpectTestKeys: &webconnectivityqa.TestKeys{
Accessible: true,
Blocking: nil,
},
Checkers: []webconnectivityqa.Checker{&webconnectivityqa.ReadWriteEventsExistentialChecker{}},
}
measurer := &mocks.ExperimentMeasurer{
MockExperimentName: func() string {
return "web_connectivity"
},
MockExperimentVersion: func() string {
return "0.5.28"
},
MockRun: func(ctx context.Context, args *model.ExperimentArgs) error {
args.Measurement.TestKeys = &webconnectivitylte.TestKeys{
Accessible: optional.Some(true),
Blocking: nil,
}
return nil
},
}
err := webconnectivityqa.RunTestCase(measurer, tc)
if !errors.Is(err, webconnectivityqa.ErrCheckerNoReadWriteEvents) {
t.Fatal("unexpected error", err)
}
}

func TestReadWriteEventsExistentialChecker(t *testing.T) {
type testcase struct {
name string
version string
tk string
expect error
}

cases := []testcase{{
name: "with Web Connectivity v0.4",
version: "0.4.3",
tk: `{}`,
expect: nil,
}, {
name: "with Web Connectivity v0.6",
version: "0.6.0",
tk: `{}`,
expect: webconnectivityqa.ErrCheckerUnexpectedWebConnectivityVersion,
}, {
name: "with read/write network events",
version: "0.5.28",
tk: `{"network_events":[{"operation":"read"},{"operation":"write"}]}`,
expect: nil,
}, {
name: "without network events",
version: "0.5.28",
tk: `{"network_events":[]}`,
expect: webconnectivityqa.ErrCheckerNoReadWriteEvents,
}, {
name: "with no read/write network events",
version: "0.5.28",
tk: `{"network_events":[{"operation":"connect"},{"operation":"close"}]}`,
expect: webconnectivityqa.ErrCheckerNoReadWriteEvents,
}}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
var tks map[string]any
must.UnmarshalJSON([]byte(tc.tk), &tks)

meas := &model.Measurement{
TestKeys: tks,
TestVersion: tc.version,
}

err := (&webconnectivityqa.ReadWriteEventsExistentialChecker{}).Check(meas)

switch {
case tc.expect == nil && err == nil:
return

case tc.expect == nil && err != nil:
t.Fatal("expected", tc.expect, "got", err)

case tc.expect != nil && err == nil:
t.Fatal("expected", tc.expect, "got", err)

case tc.expect != nil && err != nil:
if err.Error() != tc.expect.Error() {
t.Fatal("expected", tc.expect, "got", err)
}
}
})
}
}
24 changes: 8 additions & 16 deletions internal/webconnectivityqa/cloudflare.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,15 @@
package webconnectivityqa

import "github.com/ooni/probe-cli/v3/internal/netemx"

// cloudflareCAPTCHAWithHTTP obtains the cloudflare CAPTCHA using HTTP.
func cloudflareCAPTCHAWithHTTP() *TestCase {
// See https://github.com/ooni/probe/issues/2661 for an explanation of why
// here for now we're forced to declare "http-diff".
return &TestCase{
Name: "cloudflareCAPTCHAWithHTTP",
Flags: TestCaseFlagNoV04,
Input: "http://www.cloudflare-cache.com/",
Configure: func(env *netemx.QAEnv) {
// nothing
},
Name: "cloudflareCAPTCHAWithHTTP",
Flags: TestCaseFlagNoV04,
Input: "http://www.cloudflare-cache.com/",
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "consistent",
StatusCodeMatch: false,
BodyLengthMatch: false,
Expand All @@ -31,14 +26,11 @@ func cloudflareCAPTCHAWithHTTP() *TestCase {
// cloudflareCAPTCHAWithHTTPS obtains the cloudflare CAPTCHA using HTTPS.
func cloudflareCAPTCHAWithHTTPS() *TestCase {
return &TestCase{
Name: "cloudflareCAPTCHAWithHTTPS",
Flags: TestCaseFlagNoV04,
Input: "https://www.cloudflare-cache.com/",
Configure: func(env *netemx.QAEnv) {
// nothing
},
Name: "cloudflareCAPTCHAWithHTTPS",
Flags: TestCaseFlagNoV04,
Input: "https://www.cloudflare-cache.com/",
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "consistent",
StatusCodeMatch: false,
BodyLengthMatch: false,
Expand Down
4 changes: 2 additions & 2 deletions internal/webconnectivityqa/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func controlFailureWithSuccessfulHTTPWebsite() *TestCase {

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
ControlFailure: "unknown_failure: httpapi: all endpoints failed: [ connection_reset; connection_reset; connection_reset; connection_reset;]",
XStatus: 8, // StatusAnomalyControlUnreachable
Accessible: nil,
Expand Down Expand Up @@ -87,7 +87,7 @@ func controlFailureWithSuccessfulHTTPSWebsite() *TestCase {

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
ControlFailure: "unknown_failure: httpapi: all endpoints failed: [ connection_reset; connection_reset; connection_reset; connection_reset;]",
XStatus: 1, // StatusSuccessSecure
XBlockingFlags: 32, // AnalysisBlockingFlagSuccess
Expand Down
6 changes: 3 additions & 3 deletions internal/webconnectivityqa/dnsblocking.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func dnsBlockingAndroidDNSCacheNoData() *TestCase {
env.ISPResolverConfig().RemoveRecord("www.example.com")
},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSExperimentFailure: "android_dns_cache_no_data",
HTTPExperimentFailure: "android_dns_cache_no_data",
DNSConsistency: "inconsistent",
Expand Down Expand Up @@ -52,7 +52,7 @@ func dnsBlockingNXDOMAIN() *TestCase {
env.ISPResolverConfig().RemoveRecord("www.example.com")
},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSExperimentFailure: "dns_nxdomain_error",
HTTPExperimentFailure: "dns_nxdomain_error",
DNSConsistency: "inconsistent",
Expand All @@ -76,7 +76,7 @@ func dnsBlockingBOGON() *TestCase {
env.ISPResolverConfig().AddRecord("www.example.com", "", "10.10.34.35")
},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
HTTPExperimentFailure: "generic_timeout_error",
DNSExperimentFailure: nil,
DNSConsistency: "inconsistent",
Expand Down
8 changes: 4 additions & 4 deletions internal/webconnectivityqa/dnshijacking.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func dnsHijackingToProxyWithHTTPURL() *TestCase {

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "inconsistent",
BodyLengthMatch: true,
BodyProportion: 1,
Expand Down Expand Up @@ -63,7 +63,7 @@ func dnsHijackingToProxyWithHTTPSURL() *TestCase {

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "inconsistent",
BodyLengthMatch: true,
BodyProportion: 1,
Expand Down Expand Up @@ -97,7 +97,7 @@ func dnsHijackingToLocalhostWithHTTP() *TestCase {

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "inconsistent",
XDNSFlags: 5, // AnalysisFlagDNSBogon | AnalysisDNSFlagUnexpectedAddrs
XBlockingFlags: 33, // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
Expand Down Expand Up @@ -125,7 +125,7 @@ func dnsHijackingToLocalhostWithHTTPS() *TestCase {

},
ExpectErr: false,
ExpectTestKeys: &testKeys{
ExpectTestKeys: &TestKeys{
DNSConsistency: "inconsistent",
XDNSFlags: 5, // AnalysisFlagDNSBogon | AnalysisDNSFlagUnexpectedAddrs
XBlockingFlags: 33, // AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagSuccess
Expand Down
Loading

0 comments on commit 4439cbd

Please sign in to comment.