Skip to content

Commit

Permalink
fix(minipipeline): make expected TCP, TLS failures WAI
Browse files Browse the repository at this point in the history
This diff corrects an embarassing bug in the logic we were using
for expected TCP connect and TLS handshakes failures.

We spotted this issue with the new testcase we added called
websiteDownTCPConnect, which is also included.

Additionally, we needed to fix some tests because the logic bugs
were preventing us from seeing some events.

Part of ooni/probe#2652.

Closes ooni/probe#2299.
  • Loading branch information
bassosimone committed Jan 22, 2024
1 parent 0a5604a commit 7639bb1
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 23 deletions.
6 changes: 3 additions & 3 deletions internal/experiment/webconnectivityqa/badssl.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func badSSLWithExpiredCertificate() *TestCase {
DNSConsistency: "consistent",
HTTPExperimentFailure: "ssl_invalid_certificate",
XStatus: 16, // StatusAnomalyControlFailure
XNullNullFlags: 4, // analysisFlagNullNullTLSMisconfigured
XNullNullFlags: 4, // AnalysisFlagNullNullExpectedTLSHandshakeFailure
Accessible: false,
Blocking: false,
},
Expand All @@ -42,7 +42,7 @@ func badSSLWithWrongServerName() *TestCase {
DNSConsistency: "consistent",
HTTPExperimentFailure: "ssl_invalid_hostname",
XStatus: 16, // StatusAnomalyControlFailure
XNullNullFlags: 4, // analysisFlagNullNullTLSMisconfigured
XNullNullFlags: 4, // AnalysisFlagNullNullExpectedTLSHandshakeFailure
Accessible: false,
Blocking: false,
},
Expand All @@ -63,7 +63,7 @@ func badSSLWithUnknownAuthorityWithConsistentDNS() *TestCase {
DNSConsistency: "consistent",
HTTPExperimentFailure: "ssl_unknown_authority",
XStatus: 16, // StatusAnomalyControlFailure
XNullNullFlags: 4, // analysisFlagNullNullTLSMisconfigured
XNullNullFlags: 4, // AnalysisFlagNullNullExpectedTLSHandshakeFailure
Accessible: false,
Blocking: false,
},
Expand Down
2 changes: 1 addition & 1 deletion internal/experiment/webconnectivityqa/control.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func controlFailureWithSuccessfulHTTPSWebsite() *TestCase {
ControlFailure: "unknown_failure: httpapi: all endpoints failed: [ connection_reset; connection_reset; connection_reset; connection_reset;]",
XStatus: 1, // StatusSuccessSecure
XBlockingFlags: 32, // AnalysisBlockingFlagSuccess
XNullNullFlags: 8, // analysisFlagNullNullSuccessfulHTTPS
XNullNullFlags: 8, // AnalysisFlagNullNullSuccessfulHTTPS
Accessible: true,
Blocking: false,
},
Expand Down
4 changes: 2 additions & 2 deletions internal/experiment/webconnectivityqa/ghost.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func ghostDNSBlockingWithHTTP() *TestCase {
DNSExperimentFailure: nil,
DNSConsistency: "inconsistent",
XBlockingFlags: 16, // AnalysisBlockingFlagHTTPDiff
XNullNullFlags: 16, // AnalysisFlagNullNullUnexpectedDNSLookupSuccess
XNullNullFlags: 18, // AnalysisFlagNullNullExpectedTCPConnectFailure | AnalysisFlagNullNullUnexpectedDNSLookupSuccess
XStatus: 16, // StatusAnomalyControlFailure
Accessible: false,
Blocking: "dns",
Expand Down Expand Up @@ -72,7 +72,7 @@ func ghostDNSBlockingWithHTTPS() *TestCase {
DNSExperimentFailure: nil,
DNSConsistency: "inconsistent",
HTTPExperimentFailure: "connection_refused",
XNullNullFlags: 16, // AnalysisFlagNullNullUnexpectedDNSLookupSuccess
XNullNullFlags: 18, // AnalysisFlagNullNullExpectedTCPConnectFailure | AnalysisFlagNullNullUnexpectedDNSLookupSuccess
XStatus: 4256, // StatusExperimentConnect | StatusAnomalyDNS | StatusAnomalyConnect
Accessible: false,
Blocking: "dns",
Expand Down
1 change: 1 addition & 0 deletions internal/experiment/webconnectivityqa/tcpblocking.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func tcpBlockingConnectionRefusedWithInconsistentDNS() *TestCase {
HTTPExperimentFailure: "connection_refused",
XStatus: 4256, // StatusExperimentConnect | StatusAnomalyConnect | StatusAnomalyDNS
XDNSFlags: 4, // AnalysisDNSFlagUnexpectedAddrs
XNullNullFlags: 2, // AnalysisFlagNullNullExpectedTCPConnectFailure
XBlockingFlags: 35, // AnalysisBlockingFlagSuccess | AnalysisBlockingFlagDNSBlocking | AnalysisBlockingFlagTCPIPBlocking
Accessible: false,
Blocking: "dns",
Expand Down
1 change: 1 addition & 0 deletions internal/experiment/webconnectivityqa/testcase.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,6 @@ func AllTestCases() []*TestCase {
tlsBlockingConnectionResetWithInconsistentDNS(),

websiteDownNXDOMAIN(),
websiteDownTCPConnect(),
}
}
25 changes: 24 additions & 1 deletion internal/experiment/webconnectivityqa/websitedown.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,30 @@ func websiteDownNXDOMAIN() *TestCase {
DNSConsistency: "consistent",
XStatus: 2052, // StatusExperimentDNS | StatusSuccessNXDOMAIN
XBlockingFlags: 0,
XNullNullFlags: 1, // analysisFlagNullNullNoAddrs
XNullNullFlags: 1, // AnalysisFlagNullNullExpectedDNSLookupFailure
Accessible: false,
Blocking: false,
},
}
}

// websiteDownTCPConnect describes the test case where attempting to
// connect to the website doesn't work for both probe and TH.
//
// See https://github.com/ooni/probe/issues/2299.
func websiteDownTCPConnect() *TestCase {
return &TestCase{
Name: "websiteDownTCPConnect",
Flags: TestCaseFlagNoV04,
Input: "http://www.example.com:444/", // port where we're not listening.
Configure: nil,
ExpectErr: false,
ExpectTestKeys: &testKeys{
HTTPExperimentFailure: "connection_refused",
DNSConsistency: "consistent",
XStatus: 2052, // StatusExperimentDNS | StatusSuccessNXDOMAIN
XBlockingFlags: 0,
XNullNullFlags: 2, // AnalysisFlagNullNullExpectedTCPConnectFailure
Accessible: false,
Blocking: false,
},
Expand Down
26 changes: 10 additions & 16 deletions internal/minipipeline/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,21 +491,18 @@ func (wa *WebAnalysis) tcpComputeMetrics(c *WebObservationsContainer) {
}

// handle the case where both the probe and the control fail
//
// See https://explorer.ooni.org/measurement/20220911T105037Z_webconnectivity_IT_30722_n1_ruzuQ219SmIO9SrT?input=https://doh.centraleu.pi-dns.com/dns-query?dns=q80BAAABAAAAAAAAA3d3dwdleGFtcGxlA2NvbQAAAQAB
// for an example measurement with this behavior.
//
// See also https://github.com/ooni/probe/issues/2299.
if obs.TCPConnectFailure.Unwrap() != "" && obs.ControlTCPConnectFailure.Unwrap() != "" {
wa.TCPConnectExpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
continue
}

// handle the case where the control fails
if obs.ControlTCPConnectFailure.Unwrap() != "" {
// If also the probe failed mark this failure as expected.
//
// See https://explorer.ooni.org/measurement/20220911T105037Z_webconnectivity_IT_30722_n1_ruzuQ219SmIO9SrT?input=https://doh.centraleu.pi-dns.com/dns-query?dns=q80BAAABAAAAAAAAA3d3dwdleGFtcGxlA2NvbQAAAQAB
// for an example measurement with this behavior.
//
// See also https://github.com/ooni/probe/issues/2299.
if obs.TCPConnectFailure.Unwrap() != "" {
wa.TCPConnectExpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
}
continue
}

Expand Down Expand Up @@ -555,18 +552,15 @@ func (wa *WebAnalysis) tlsComputeMetrics(c *WebObservationsContainer) {
}

// handle the case where both the probe and the control fail
if obs.TLSHandshakeFailure.Unwrap() != "" && obs.ControlTCPConnectFailure.Unwrap() != "" {
//
// See https://github.com/ooni/probe/issues/2300.
if obs.TLSHandshakeFailure.Unwrap() != "" && obs.ControlTLSHandshakeFailure.Unwrap() != "" {
wa.TLSHandshakeExpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
continue
}

// handle the case where the control fails
if obs.ControlTLSHandshakeFailure.Unwrap() != "" {
// If also the probe failed mark this failure as expected.
//
// See https://github.com/ooni/probe/issues/2300.
if obs.TLSHandshakeFailure.Unwrap() != "" {
wa.TLSHandshakeExpectedFailure.Add(obs.EndpointTransactionID.Unwrap())
}
continue
}

Expand Down

0 comments on commit 7639bb1

Please sign in to comment.