From 39cb5959c94d81edcc371a9b4b7fbb90c59a8cfd Mon Sep 17 00:00:00 2001 From: Simone Basso Date: Thu, 8 Sep 2022 10:02:47 +0200 Subject: [PATCH] fix(datafmt): sync measurexlite and v0.5 with previous code (#942) * fix(model/archival.go): more optional keys Basically, `t0` and `transaction_id` should be optional. Version 0.4.x of web_connectivity should not include them, version 0.5.x should. There is a technical reason why v0.4.x should not include them. The code it is based on, tracex, does not record these two fields. Whereas, v0.5.x, uses measurexlite, which records these two fields. Part of https://github.com/ooni/probe/issues/2238 * fix(webconnectivity@v0.5): add more fields This diff adds the following fields to webconnectivity@v0.5: 1. agent, always set to "redirect" (legacy field); 2. client_resolver, properly initialized w/ the resolver's IPv4 address; 3. retries, legacy field always set to null; 4. socksproxy, legacy field always set to null. Part of https://github.com/ooni/probe/issues/2238 * fix(webconnectivity@v0.5): register extensions The general idea behind this field is that we would be able in the future to tweak the data model for some fields, by declaring we're using a later version, so it seems useful to add it. See https://github.com/ooni/probe/issues/2238 * fix(measurexlite): use tcp or quic for tls handshake network This diff fixes a bug where measurexlite was using "tls" as the protocol for the TLS handshake when using TCP. While this choice _could_ make sense, the rest of the code we have written so far uses "tcp" instead. Using "tcp" makes more sense because it allows you to search for the same endpoint across different events by checking for the same network and for the same endpoint rather than special casing TLS handshakes for using "tls" when the endpoint is "tcp". See https://github.com/ooni/probe/issues/2238 * chore: run alltests.yml for "alltestsbuild" branches Part of https://github.com/ooni/probe/issues/2238 --- .github/workflows/alltests.yml | 1 + .../experiment/webconnectivity/measurer.go | 13 ++++ .../experiment/webconnectivity/testkeys.go | 65 +++++++++++++------ internal/measurexlite/tls.go | 2 +- internal/measurexlite/tls_test.go | 8 +-- internal/model/archival.go | 20 +++--- 6 files changed, 74 insertions(+), 35 deletions(-) diff --git a/.github/workflows/alltests.yml b/.github/workflows/alltests.yml index 87d9996..7e66467 100644 --- a/.github/workflows/alltests.yml +++ b/.github/workflows/alltests.yml @@ -5,6 +5,7 @@ on: branches: - "release/**" - "fullbuild" + - "alltestsbuild" jobs: test: diff --git a/internal/experiment/webconnectivity/measurer.go b/internal/experiment/webconnectivity/measurer.go index 41eef70..a2cf8cd 100644 --- a/internal/experiment/webconnectivity/measurer.go +++ b/internal/experiment/webconnectivity/measurer.go @@ -99,6 +99,8 @@ func (m *Measurer) Run(ctx context.Context, sess model.ExperimentSession, tk.SetControlFailure(webconnectivity.ErrNoAvailableTestHelpers) } + registerExtensions(measurement) + // start background tasks resos := &DNSResolvers{ DNSCache: NewDNSCache(), @@ -133,3 +135,14 @@ func (m *Measurer) Run(ctx context.Context, sess model.ExperimentSession, // the measurement from being submitted to the OONI collector. return tk.fundamentalFailure } + +// registerExtensions registers the extensions used by this +// experiment into the given measurement. +func registerExtensions(m *model.Measurement) { + model.ArchivalExtHTTP.AddTo(m) + model.ArchivalExtDNS.AddTo(m) + model.ArchivalExtNetevents.AddTo(m) + model.ArchivalExtTCPConnect.AddTo(m) + model.ArchivalExtTLSHandshake.AddTo(m) + model.ArchivalExtTunnel.AddTo(m) +} diff --git a/internal/experiment/webconnectivity/testkeys.go b/internal/experiment/webconnectivity/testkeys.go index 813cee3..9518943 100644 --- a/internal/experiment/webconnectivity/testkeys.go +++ b/internal/experiment/webconnectivity/testkeys.go @@ -17,6 +17,18 @@ import ( // TestKeys contains the results produced by web_connectivity. type TestKeys struct { + // Agent is the HTTP agent we use. + Agent string `json:"agent"` + + // ClientResolver is the IPv4 of the resolver used by getaddrinfo. + ClientResolver string `json:"client_resolver"` + + // Retries is a legacy field always set to nil by web_connectivity@v0.4.x + Retries *int64 `json:"retries"` + + // SOCKSProxy is a legacy field always set to nil by web_connectivity@v0.4.x + SOCKSProxy *string `json:"socksproxy"` + // NetworkEvents contains network events. NetworkEvents []*model.ArchivalNetworkEvent `json:"network_events"` @@ -258,10 +270,21 @@ func (tk *TestKeys) WithDNSWhoami(fun func(*DNSWhoamiInfo)) { tk.mu.Unlock() } +// SetClientResolver sets the ClientResolver field. +func (tk *TestKeys) SetClientResolver(value string) { + tk.mu.Lock() + tk.ClientResolver = value + tk.mu.Unlock() +} + // NewTestKeys creates a new instance of TestKeys. func NewTestKeys() *TestKeys { return &TestKeys{ - NetworkEvents: []*model.ArchivalNetworkEvent{}, + Agent: "redirect", + ClientResolver: "", + Retries: nil, + SOCKSProxy: nil, + NetworkEvents: []*model.ArchivalNetworkEvent{}, DNSWoami: &DNSWhoamiInfo{ SystemV4: []DNSWhoamiInfoEntry{}, UDPv4: map[string][]DNSWhoamiInfoEntry{}, @@ -277,25 +300,27 @@ func NewTestKeys() *TestKeys { NetworkEvents: []*model.ArchivalNetworkEvent{}, Queries: []*model.ArchivalDNSLookupResult{}, }, - Queries: []*model.ArchivalDNSLookupResult{}, - Requests: []*model.ArchivalHTTPRequestResult{}, - TCPConnect: []*model.ArchivalTCPConnectResult{}, - TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{}, - Control: nil, - ControlFailure: nil, - DNSFlags: 0, - DNSExperimentFailure: nil, - DNSConsistency: "", - BlockingFlags: 0, - BodyLengthMatch: nil, - HeadersMatch: nil, - StatusCodeMatch: nil, - TitleMatch: nil, - Blocking: nil, - Accessible: nil, - ControlRequest: nil, - fundamentalFailure: nil, - mu: &sync.Mutex{}, + DNSLateReplies: []*model.ArchivalDNSLookupResult{}, + Queries: []*model.ArchivalDNSLookupResult{}, + Requests: []*model.ArchivalHTTPRequestResult{}, + TCPConnect: []*model.ArchivalTCPConnectResult{}, + TLSHandshakes: []*model.ArchivalTLSOrQUICHandshakeResult{}, + Control: nil, + ControlFailure: nil, + DNSFlags: 0, + DNSExperimentFailure: nil, + DNSConsistency: "", + HTTPExperimentFailure: nil, + BlockingFlags: 0, + BodyLengthMatch: nil, + HeadersMatch: nil, + StatusCodeMatch: nil, + TitleMatch: nil, + Blocking: nil, + Accessible: nil, + ControlRequest: nil, + fundamentalFailure: nil, + mu: &sync.Mutex{}, } } diff --git a/internal/measurexlite/tls.go b/internal/measurexlite/tls.go index 816e69c..02eeced 100644 --- a/internal/measurexlite/tls.go +++ b/internal/measurexlite/tls.go @@ -57,7 +57,7 @@ func (tx *Trace) OnTLSHandshakeDone(started time.Time, remoteAddr string, config case tx.tlsHandshake <- NewArchivalTLSOrQUICHandshakeResult( tx.Index, started.Sub(tx.ZeroTime), - "tls", + "tcp", remoteAddr, config, state, diff --git a/internal/measurexlite/tls_test.go b/internal/measurexlite/tls_test.go index 46c8c50..1a91188 100644 --- a/internal/measurexlite/tls_test.go +++ b/internal/measurexlite/tls_test.go @@ -120,7 +120,7 @@ func TestNewTLSHandshakerStdlib(t *testing.T) { } expectedFailure := "unknown_failure: mocked" expect := &model.ArchivalTLSOrQUICHandshakeResult{ - Network: "tls", + Network: "tcp", Address: "1.1.1.1:443", CipherSuite: "", Failure: &expectedFailure, @@ -275,7 +275,7 @@ func TestNewTLSHandshakerStdlib(t *testing.T) { t.Fatal("expected to see a single TLSHandshake event") } expected := &model.ArchivalTLSOrQUICHandshakeResult{ - Network: "tls", + Network: "tcp", Address: conn.RemoteAddr().String(), CipherSuite: netxlite.TLSCipherSuiteString(connState.CipherSuite), Failure: nil, @@ -362,7 +362,7 @@ func TestFirstTLSHandshake(t *testing.T) { zeroTime := time.Now() trace := NewTrace(0, zeroTime) expect := []*model.ArchivalTLSOrQUICHandshakeResult{{ - Network: "tls", + Network: "tcp", Address: "1.1.1.1:443", CipherSuite: "", Failure: nil, @@ -374,7 +374,7 @@ func TestFirstTLSHandshake(t *testing.T) { Tags: []string{}, TLSVersion: "", }, { - Network: "tls", + Network: "tcp", Address: "8.8.8.8:443", CipherSuite: "", Failure: nil, diff --git a/internal/model/archival.go b/internal/model/archival.go index 23a333f..887dfa3 100644 --- a/internal/model/archival.go +++ b/internal/model/archival.go @@ -123,9 +123,9 @@ type ArchivalDNSLookupResult struct { ResolverHostname *string `json:"resolver_hostname"` ResolverPort *string `json:"resolver_port"` ResolverAddress string `json:"resolver_address"` - T0 float64 `json:"t0"` + T0 float64 `json:"t0,omitempty"` T float64 `json:"t"` - TransactionID int64 `json:"transaction_id"` + TransactionID int64 `json:"transaction_id,omitempty"` } // ArchivalDNSAnswer is a DNS answer. @@ -150,9 +150,9 @@ type ArchivalTCPConnectResult struct { IP string `json:"ip"` Port int `json:"port"` Status ArchivalTCPConnectStatus `json:"status"` - T0 float64 `json:"t0"` + T0 float64 `json:"t0,omitempty"` T float64 `json:"t"` - TransactionID int64 `json:"transaction_id"` + TransactionID int64 `json:"transaction_id,omitempty"` } // ArchivalTCPConnectStatus is the status of ArchivalTCPConnectResult. @@ -178,11 +178,11 @@ type ArchivalTLSOrQUICHandshakeResult struct { NoTLSVerify bool `json:"no_tls_verify"` PeerCertificates []ArchivalMaybeBinaryData `json:"peer_certificates"` ServerName string `json:"server_name"` - T0 float64 `json:"t0"` + T0 float64 `json:"t0,omitempty"` T float64 `json:"t"` Tags []string `json:"tags"` TLSVersion string `json:"tls_version"` - TransactionID int64 `json:"transaction_id"` + TransactionID int64 `json:"transaction_id,omitempty"` } // @@ -199,9 +199,9 @@ type ArchivalHTTPRequestResult struct { Failure *string `json:"failure"` Request ArchivalHTTPRequest `json:"request"` Response ArchivalHTTPResponse `json:"response"` - T0 float64 `json:"t0"` + T0 float64 `json:"t0,omitempty"` T float64 `json:"t"` - TransactionID int64 `json:"transaction_id"` + TransactionID int64 `json:"transaction_id,omitempty"` } // ArchivalHTTPRequest contains an HTTP request. @@ -322,8 +322,8 @@ type ArchivalNetworkEvent struct { NumBytes int64 `json:"num_bytes,omitempty"` Operation string `json:"operation"` Proto string `json:"proto,omitempty"` - T0 float64 `json:"t0"` + T0 float64 `json:"t0,omitempty"` T float64 `json:"t"` - TransactionID int64 `json:"transaction_id"` + TransactionID int64 `json:"transaction_id,omitempty"` Tags []string `json:"tags,omitempty"` }