diff --git a/.vscode/settings.json b/.vscode/settings.json index f4c2c50..412553b 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -5,5 +5,6 @@ "-GOCACHE", "-GOPATH" ] - } + }, + "C_Cpp.default.configurationProvider": "ms-vscode.makefile-tools" } diff --git a/internal/experiment/webconnectivity/analysiscore.go b/internal/experiment/webconnectivity/analysiscore.go index c41ffaa..100bfd5 100644 --- a/internal/experiment/webconnectivity/analysiscore.go +++ b/internal/experiment/webconnectivity/analysiscore.go @@ -3,6 +3,7 @@ package webconnectivity import ( "fmt" "net" + "net/url" "github.com/ooni/probe-cli/v3/internal/model" ) @@ -138,12 +139,20 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) { tk.Blocking = false tk.Accessible = true logger.Infof( - "SUCCESS: flags=%d accessible=%+v, blocking=%+v", + "ACCESSIBLE: flags=%d accessible=%+v, blocking=%+v", tk.BlockingFlags, tk.Accessible, tk.Blocking, ) default: - if tk.analysisWebsiteDownDetectNoAddrs(logger) { + // NullNull remediation + // + // If we arrive here, the measurement has failed. However, there are a + // bunch of cases where we can still explain what happened by applying specific + // algorithms to detect edge cases. + // + // The relative order of these algorithsm matters. + + if tk.analysisNullNullDetectNoAddrs(logger) { tk.Blocking = false tk.Accessible = false logger.Infof( @@ -152,7 +161,8 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) { ) return } - if tk.analysisWebsiteDownDetectAllConnectsFailed(logger) { + + if tk.analysisNullNullDetectAllConnectsFailed(logger) { tk.Blocking = false tk.Accessible = false logger.Infof( @@ -161,7 +171,8 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) { ) return } - if tk.analysisWebsiteDownDetectTLSMisconfigured(logger) { + + if tk.analysisNullNullDetectTLSMisconfigured(logger) { tk.Blocking = false tk.Accessible = false logger.Infof( @@ -170,6 +181,17 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) { ) return } + + if tk.analysisNullNullDetectSuccessfulHTTPS(logger) { + tk.Blocking = false + tk.Accessible = true + logger.Infof( + "ACCESSIBLE_HTTPS: flags=%d, accessible=%+v, blocking=%+v", + tk.BlockingFlags, tk.Accessible, tk.Blocking, + ) + return + } + tk.Blocking = nil tk.Accessible = nil logger.Warnf( @@ -180,24 +202,83 @@ func (tk *TestKeys) analysisToplevel(logger model.Logger) { } const ( - // analysisFlagWebsiteDownNoAddrs indicates neither the probe nor the TH were + // analysisFlagNullNullNoAddrs indicates neither the probe nor the TH were // able to get any IP addresses from any resolver. - analysisFlagWebsiteDownNoAddrs = 1 << iota + analysisFlagNullNullNoAddrs = 1 << iota - // analysisFlagWebsiteDownAllConnectsFailed indicates that all the connect + // analysisFlagNullNullAllConnectsFailed indicates that all the connect // attempts failed both in the probe and in the test helper. - analysisFlagWebsiteDownAllConnectsFailed + analysisFlagNullNullAllConnectsFailed - // analysisFlagWebsiteDownTLSMisconfigured indicates that all the TLS handshake + // analysisFlagNullNullTLSMisconfigured indicates that all the TLS handshake // attempts failed both in the probe and in the test helper. - analysisFlagWebsiteDownTLSMisconfigured + analysisFlagNullNullTLSMisconfigured + + // analysisFlagNullNullSuccessfulHTTPS indicates that we had no TH data + // but all the HTTP requests used always HTTPS and never failed. + analysisFlagNullNullSuccessfulHTTPS ) -// analysisWebsiteDownDetectTLSMisconfigured runs when .Blocking = nil and +// analysisNullNullDetectSuccessfulHTTPS runs when .Blocking = nil and +// .Accessible = nil to flag successul HTTPS measurements chains that +// occurred regardless of whatever else could have gone wrong. +// +// We need all requests to be HTTPS because an HTTP request in the +// chain breaks the ~reasonable assumption that our custom CA bundle +// is enough to protect against MITM. Of course, when we use this +// algorithm, we're not well positioned to flag server-side blocking. +// +// Version 0.4 of the probe implemented a similar algorithm, which +// however ran before other checks. Version, 0.5 on the contrary, runs +// this algorithm if any other heuristics failed. +// +// See https://github.com/ooni/probe/issues/2307 for more info. +func (tk *TestKeys) analysisNullNullDetectSuccessfulHTTPS(logger model.Logger) bool { + + // the chain is sorted from most recent to oldest but it does + // not matter much since we need to walk all of it. + // + // CAVEAT: this code assumes we have a single request chain + // inside the .Requests field, which seems fine because it's + // what Web Connectivity should be doing. + for _, req := range tk.Requests { + URL, err := url.Parse(req.Request.URL) + if err != nil { + // this looks like a bug + return false + } + if URL.Scheme != "https" { + // the whole chain must be HTTPS + return false + } + if req.Failure != nil { + // they must all succeed + return false + } + switch req.Response.Code { + case 200, 301, 302, 307, 308: + default: + // the response must be successful or redirect + return false + } + } + + // only if we have at least one request + if len(tk.Requests) > 0 { + logger.Info("website likely accessible: seen successful chain of HTTPS transactions") + tk.NullNullFlags |= analysisFlagNullNullSuccessfulHTTPS + return true + } + + // safety net otherwise + return false +} + +// analysisNullNullDetectTLSMisconfigured runs when .Blocking = nil and // .Accessible = nil to check whether by chance we had TLS issues both on the // probe side and on the TH side. This problem of detecting misconfiguration // of the server's TLS stack is discussed at https://github.com/ooni/probe/issues/2300. -func (tk *TestKeys) analysisWebsiteDownDetectTLSMisconfigured(logger model.Logger) bool { +func (tk *TestKeys) analysisNullNullDetectTLSMisconfigured(logger model.Logger) bool { if tk.Control == nil || tk.Control.TLSHandshake == nil { // we need TLS control data to say we are in this case return false @@ -233,7 +314,7 @@ func (tk *TestKeys) analysisWebsiteDownDetectTLSMisconfigured(logger model.Logge // only if we have had some TLS handshakes for both probe and TH if len(tk.TLSHandshakes) > 0 && len(tk.Control.TLSHandshake) > 0 { logger.Info("website likely down: all TLS handshake attempts failed for both probe and TH") - tk.WebsiteDownFlags |= analysisFlagWebsiteDownTLSMisconfigured + tk.NullNullFlags |= analysisFlagNullNullTLSMisconfigured return true } @@ -241,7 +322,7 @@ func (tk *TestKeys) analysisWebsiteDownDetectTLSMisconfigured(logger model.Logge return false } -// analysisWebsiteDownDetectAllConnectsFailed attempts to detect whether we are in +// analysisNullNullDetectAllConnectsFailed attempts to detect whether we are in // the .Blocking = nil, .Accessible = nil case because all the TCP connect // attempts by either the probe or the TH have failed. // @@ -249,7 +330,7 @@ func (tk *TestKeys) analysisWebsiteDownDetectTLSMisconfigured(logger model.Logge // for an example measurement with this behavior. // // See https://github.com/ooni/probe/issues/2299 for the reference issue. -func (tk *TestKeys) analysisWebsiteDownDetectAllConnectsFailed(logger model.Logger) bool { +func (tk *TestKeys) analysisNullNullDetectAllConnectsFailed(logger model.Logger) bool { if tk.Control == nil { // we need control data to say we're in this case return false @@ -275,7 +356,7 @@ func (tk *TestKeys) analysisWebsiteDownDetectAllConnectsFailed(logger model.Logg // only if we have had some addresses to connect if len(tk.TCPConnect) > 0 && len(tk.Control.TCPConnect) > 0 { logger.Info("website likely down: all TCP connect attempts failed for both probe and TH") - tk.WebsiteDownFlags |= analysisFlagWebsiteDownAllConnectsFailed + tk.NullNullFlags |= analysisFlagNullNullAllConnectsFailed return true } @@ -283,7 +364,7 @@ func (tk *TestKeys) analysisWebsiteDownDetectAllConnectsFailed(logger model.Logg return false } -// analysisWebsiteDownDetectNoAddrs attempts to see whether we +// analysisNullNullDetectNoAddrs attempts to see whether we // ended up into the .Blocking = nil, .Accessible = nil case because // the domain is expired and all queries returned no addresses. // @@ -297,7 +378,7 @@ func (tk *TestKeys) analysisWebsiteDownDetectAllConnectsFailed(logger model.Logg // // See https://github.com/ooni/probe/issues/2029 for more information // on Android's getaddrinfo behavior. -func (tk *TestKeys) analysisWebsiteDownDetectNoAddrs(logger model.Logger) bool { +func (tk *TestKeys) analysisNullNullDetectNoAddrs(logger model.Logger) bool { if tk.Control == nil { // we need control data to say we're in this case return false @@ -325,6 +406,6 @@ func (tk *TestKeys) analysisWebsiteDownDetectNoAddrs(logger model.Logger) bool { return false } logger.Infof("website likely down: all DNS lookups failed for both probe and TH") - tk.WebsiteDownFlags |= analysisFlagWebsiteDownNoAddrs + tk.NullNullFlags |= analysisFlagNullNullNoAddrs return true } diff --git a/internal/experiment/webconnectivity/iox.go b/internal/experiment/webconnectivity/iox.go index b494b30..09f425b 100644 --- a/internal/experiment/webconnectivity/iox.go +++ b/internal/experiment/webconnectivity/iox.go @@ -19,7 +19,7 @@ import ( // as [ctx] is done or when [reader] is closed, if applicable. // // This function transforms an errors.Is(err, io.EOF) to a nil error -// such as the standard library's io.ReadAll does. +// such as the standard library's ReadAll does. // // This function might return a non-zero-length buffer along with // an non-nil error in the case in which we could only read a portion diff --git a/internal/experiment/webconnectivity/measurer.go b/internal/experiment/webconnectivity/measurer.go index 0c3406b..ea3c325 100644 --- a/internal/experiment/webconnectivity/measurer.go +++ b/internal/experiment/webconnectivity/measurer.go @@ -36,7 +36,7 @@ func (m *Measurer) ExperimentName() string { // ExperimentVersion implements model.ExperimentMeasurer. func (m *Measurer) ExperimentVersion() string { - return "0.5.14" + return "0.5.15" } // Run implements model.ExperimentMeasurer. diff --git a/internal/experiment/webconnectivity/testkeys.go b/internal/experiment/webconnectivity/testkeys.go index e089e3e..839f625 100644 --- a/internal/experiment/webconnectivity/testkeys.go +++ b/internal/experiment/webconnectivity/testkeys.go @@ -90,8 +90,9 @@ type TestKeys struct { // BlockingFlags explains why we think that the website is blocked. BlockingFlags int64 `json:"x_blocking_flags"` - // WebsiteDownFlags explains why we determined that the website is down. - WebsiteDownFlags int64 `json:"x_website_down_flags"` + // NullNullFlags describes what the algorithm to avoid emitting + // blocking = null, accessible = null measurements did + NullNullFlags int64 `json:"x_null_null_flags"` // BodyLength match tells us whether the body length matches. BodyLengthMatch *bool `json:"body_length_match"` @@ -337,7 +338,7 @@ func NewTestKeys() *TestKeys { DNSConsistency: "", HTTPExperimentFailure: nil, BlockingFlags: 0, - WebsiteDownFlags: 0, + NullNullFlags: 0, BodyLengthMatch: nil, HeadersMatch: nil, StatusCodeMatch: nil,