Skip to content

Commit

Permalink
minipipeline: fix expected TCP & TLS failures (#1459)
Browse files Browse the repository at this point in the history 
This diff corrects an embarrassing bugs in the logic we were using for
expected TCP and TLS failures.

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

Additionally, we updated some tests because of the embarrassing bugs we
fixed.

Additionally, we fixed some comments that were outdated.

Part of ooni/probe#2652.

Closes ooni/probe#2299.
  • Loading branch information
@bassosimone
bassosimone authored Jan 22, 2024
1 parent 0a5604a commit a608f3a
Show file tree
Hide file tree
Showing 147 changed files with 6,642 additions and 3,623 deletions.
6 changes: 3 additions & 3 deletions internal/experiment/webconnectivityqa/badssl.go
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
78 changes: 39 additions & 39 deletions ...pipeline/testdata/webconnectivity/generated/badSSLWithExpiredCertificate/measurement.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"tunnel": 0
},
"input": "https://expired.badssl.com/",
"measurement_start_time": "2023-11-29 21:21:33",
"measurement_start_time": "2024-01-22 20:26:43",
"probe_asn": "AS137",
"probe_cc": "IT",
"probe_ip": "127.0.0.1",
Expand All @@ -19,7 +19,7 @@
"resolver_ip": "130.192.3.21",
"resolver_network_name": "Consortium GARR",
"software_name": "ooniprobe",
"software_version": "3.20.0-alpha",
"software_version": "3.21.0-alpha",
"test_helpers": {
"backend": {
"address": "https://0.th.ooni.org/",
Expand All @@ -37,8 +37,8 @@
"failure": null,
"operation": "connect",
"proto": "tcp",
"t0": 0.011208,
"t": 0.017397,
"t0": 0.011631,
"t": 0.017588,
"transaction_id": 3,
"tags": [
"depth=0",
Expand All @@ -48,11 +48,11 @@
{
"address": "104.154.89.105:443",
"failure": null,
"num_bytes": 2262,
"num_bytes": 2263,
"operation": "bytes_received_cumulative",
"proto": "tcp",
"t0": 0.028382,
"t": 0.028382,
"t0": 0.028537,
"t": 0.028537,
"transaction_id": 3,
"tags": [
"depth=0",
Expand All @@ -78,8 +78,8 @@
{
"failure": null,
"operation": "resolve_start",
"t0": 0.000096,
"t": 0.000096,
"t0": 0.000084,
"t": 0.000084,
"transaction_id": 2,
"tags": [
"depth=0"
Expand All @@ -91,8 +91,8 @@
"num_bytes": 36,
"operation": "write",
"proto": "udp",
"t0": 0.000132,
"t": 0.000154,
"t0": 0.000166,
"t": 0.000189,
"transaction_id": 2,
"tags": [
"depth=0"
Expand All @@ -104,8 +104,8 @@
"num_bytes": 36,
"operation": "write",
"proto": "udp",
"t0": 0.000132,
"t": 0.000197,
"t0": 0.000175,
"t": 0.000195,
"transaction_id": 2,
"tags": [
"depth=0"
Expand All @@ -117,8 +117,8 @@
"num_bytes": 70,
"operation": "read",
"proto": "udp",
"t0": 0.00016,
"t": 0.005865,
"t0": 0.000195,
"t": 0.005092,
"transaction_id": 2,
"tags": [
"depth=0"
Expand All @@ -130,8 +130,8 @@
"num_bytes": 36,
"operation": "read",
"proto": "udp",
"t0": 0.000202,
"t": 0.006335,
"t0": 0.0002,
"t": 0.006039,
"transaction_id": 2,
"tags": [
"depth=0"
Expand All @@ -140,8 +140,8 @@
{
"failure": null,
"operation": "resolve_done",
"t0": 0.006356,
"t": 0.006356,
"t0": 0.006058,
"t": 0.006058,
"transaction_id": 2,
"tags": [
"depth=0"
Expand Down Expand Up @@ -169,8 +169,8 @@
"resolver_hostname": null,
"resolver_port": null,
"resolver_address": "",
"t0": 0.000064,
"t": 0.00504,
"t0": 0.000081,
"t": 0.005809,
"tags": [
"depth=0"
],
Expand All @@ -190,12 +190,12 @@
"failure": null,
"hostname": "expired.badssl.com",
"query_type": "A",
"raw_response": "ki6BAAABAAEAAAAAB2V4cGlyZWQGYmFkc3NsA2NvbQAAAQABB2V4cGlyZWQGYmFkc3NsA2NvbQAAAQABAAAOEAAEaJpZaQ==",
"raw_response": "UImBAAABAAEAAAAAB2V4cGlyZWQGYmFkc3NsA2NvbQAAAQABB2V4cGlyZWQGYmFkc3NsA2NvbQAAAQABAAAOEAAEaJpZaQ==",
"resolver_hostname": null,
"resolver_port": null,
"resolver_address": "8.8.4.4:53",
"t0": 0.000111,
"t": 0.005873,
"t0": 0.00011,
"t": 0.005099,
"tags": [
"depth=0"
],
Expand All @@ -207,12 +207,12 @@
"failure": "dns_no_answer",
"hostname": "expired.badssl.com",
"query_type": "AAAA",
"raw_response": "T6iBAAABAAAAAAAAB2V4cGlyZWQGYmFkc3NsA2NvbQAAHAAB",
"raw_response": "YUGBAAABAAAAAAAAB2V4cGlyZWQGYmFkc3NsA2NvbQAAHAAB",
"resolver_hostname": null,
"resolver_port": null,
"resolver_address": "8.8.4.4:53",
"t0": 0.000104,
"t": 0.00634,
"t0": 0.000092,
"t": 0.006042,
"tags": [
"depth=0"
],
Expand All @@ -225,12 +225,11 @@
"ip": "104.154.89.105",
"port": 443,
"status": {
"blocked": false,
"failure": null,
"success": true
},
"t0": 0.011208,
"t": 0.017397,
"t0": 0.011631,
"t": 0.017588,
"tags": [
"depth=0",
"fetch_body=true"
Expand All @@ -248,13 +247,13 @@
"no_tls_verify": false,
"peer_certificates": [
{
"data": "MIIDeDCCAmCgAwIBAgIUZVi3z1rI9yOdGX4qAUVQJU7+CpYwDQYJKoZIhvcNAQELBQAwHzENMAsGA1UEChMET09OSTEOMAwGA1UEAxMFamFmYXIwHhcNMTcwNzE2MjMwMDAwWhcNMTcwNzE3MDEwMDAwWjA1MRYwFAYDVQQKEw1PT05JIE5ldGVtIENBMRswGQYDVQQDExJleHBpcmVkLmJhZHNzbC5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC12f6U3X/rzeLqwXgVVQvqoAL8v4uGS02ckjToWYfMXVVoqwatxuPzUx8yDwFg1P/xRbTDWiXnl729mj6Sikv53YI10R16db0IL+CT9zkSfgelowQKQ8f5YNEUg14WgqTglNE38RMMHFiQKN/G1pwCvtzooblvIUmXAGc9fkuAUgrml5GwUj6pvdYHPIQQAKfs/L6vG5TFTJw5JkGWN2Jtxc6gXY5P7R18axxfLDmqhwCIdn3klGQ3IlAY700TZLx1jNstq6M5GnVjB9e6nm/Cq4XN8h3Kg021V8BeLQDivjuyv/WEHv4Z3ZbN5vyDDDLr8ztAfbFMbnbIXJNDD7mXAgMBAAGjgZUwgZIwDgYDVR0PAQH/BAQDAgWgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFJcKAEVcONUhBKzvohJXx9daCOsvMB8GA1UdIwQYMBaAFLE2F+PqVf5Fk1Hx5iWbcYhEjWrCMB0GA1UdEQQWMBSCEmV4cGlyZWQuYmFkc3NsLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAt71sdaAU3GLtpgqu37GA9ZItLwW11dXj9qSVgBzmGKO8ArSoBtcIv0n40M/v8v4VBh1qRA5N8PKfIIP17nl1o8axN3WKY97O3CPZgDDYUaVD7D46pGNwAA0JcNMYKrnXArhsECKaDjrLMXuiFJJ2WIYQuDG5lhky8z10hMUpdYJ16PdFmE5Vzh79Z2fUFG7Aa0yJdDGSOO70L0ZQkWQt2CHmQBwAZheAwTyIlhOqgmzV2qcNbfQdEuxs6gvUQ26stPCEJchBkkm6Eiw5a5FHoPX6Lof3cHplO8+UiREr5IzL4sMDUF5NhoidkzYGO5JzrxPkX5J5dv5Rf38D3gAdog==",
"data": "MIIDeDCCAmCgAwIBAgIUHWb9oKKSy6FojD1Syq49UqJlRfswDQYJKoZIhvcNAQELBQAwHzENMAsGA1UEChMET09OSTEOMAwGA1UEAxMFamFmYXIwHhcNMTcwNzE2MjMwMDAwWhcNMTcwNzE3MDEwMDAwWjA1MRYwFAYDVQQKEw1PT05JIE5ldGVtIENBMRswGQYDVQQDExJleHBpcmVkLmJhZHNzbC5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCrP+7fVsLjfoGKWxIljkSWlngbbFt5rqhLOna8btf6+EKbOxwQv/oI2XFSNR6zqAcJYvm9VmLOjoNE5LEycTkhOXk6Br1Xk9kHw9D1XpzHk+aSvaJ331PKLfUkSI1u1OaZosVJqaXWMrPgpC9f6QRuCGbR+OnQlJtMK/25FBPiCEpJIcwu1xfVyAyRML1iFRr9K6k/Ub6Ey66r2HTHUlX0FpnpN3EaofUNOWerTrf2dGvA5vLdzFU3t+ACsTFzgRCjqy+Eyh5OfRKh4vy00A8aDPoIS7sIOTUL8a+Lgc21d61s8yTO7tOXTU1SkNp8ODA9Q0/m4M0o0kcN5CQGIgVvAgMBAAGjgZUwgZIwDgYDVR0PAQH/BAQDAgWgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFJmq4c0e8paD3EkSrP2YCS+QSR8/MB8GA1UdIwQYMBaAFIMBxNHu1kYXTul2IT/Y3DV99BEWMB0GA1UdEQQWMBSCEmV4cGlyZWQuYmFkc3NsLmNvbTANBgkqhkiG9w0BAQsFAAOCAQEAZZ+B7NwSFJhz/hz5SuoWaLpqEsCBKY8LBKB3QNzK61udsRA8+9oXBYmVyrw0oPPzy1QeGwScOCvygtfGDyshCD+ZPl2ZhZa6wLmEClhLA+TsF2VXRR9ixoenUFUQzvwaaOIG8vSUAwperppaoYuRiGp8cW2QBz5dilA7JCHFjd0E7PF7DxKOgDMdYwP1HZuB1WBBrGhovRz7Rb/u44ssIRNBdP1dk+FTQnWV+68dfMpBJCtHKW+yOzUtq5c64FqDMVWzaQcmb6fZsT8NQyoZQ/5hkia3fCLRU+6oiI2s3It22HRSOuw6YYOIIxerOwZnJDy3oI0ty61vYN19R1lVQw==",
"format": "base64"
}
],
"server_name": "expired.badssl.com",
"t0": 0.017414,
"t": 0.028356,
"t0": 0.017604,
"t": 0.028513,
"tags": [
"depth=0",
"fetch_body=true"
Expand All @@ -273,7 +272,7 @@
"en-US,en;q=0.9"
],
"User-Agent": [
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/116.0.0.0 Safari/537.36"
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/119.0.0.0 Safari/537.3"
]
},
"tcp_connect": [
Expand Down Expand Up @@ -322,16 +321,17 @@
"x_conn_priority_log": [
{
"msg": "create with [{Addr:104.154.89.105 Flags:3}]",
"t": 0.011168
"t": 0.01159
}
],
"control_failure": null,
"x_dns_flags": 0,
"dns_experiment_failure": null,
"dns_consistency": "consistent",
"http_experiment_failure": null,
"http_experiment_failure": "ssl_invalid_certificate",
"x_blocking_flags": 0,
"x_null_null_flags": 4,
"body_proportion": 0,
"body_length_match": null,
"headers_match": null,
"status_code_match": null,
Expand All @@ -340,7 +340,7 @@
"accessible": false
},
"test_name": "web_connectivity",
"test_runtime": 0.506483,
"test_start_time": "2023-11-29 21:21:33",
"test_version": "0.5.26"
"test_runtime": 0.506179,
"test_start_time": "2024-01-22 20:26:43",
"test_version": "0.5.28"
}
Loading

0 comments on commit a608f3a

Please sign in to comment.