Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

webconnectivity: properly flag NXDOMAIN caused by nonexisting website #1536

Closed
andresazp opened this issue May 4, 2020 · 10 comments
Closed

webconnectivity: properly flag NXDOMAIN caused by nonexisting website #1536

andresazp opened this issue May 4, 2020 · 10 comments
Assignees
@bassosimone
Labels
2024-01-data-quality-cleanup Data quality issues addressed on 2024-01 bug Something isn't working data quality ooni/probe-engine priority/low

Comments

@andresazp
Copy link

Expected Behavior

If a measurement has a control failure, it shouldn't be label as accessible; especially if the probe couldn't load it

Actual Behavior

I'm still checking, but in some cases were the experiment DNs results in no answers (typical VE censorship) and there is a DNS_lookup_error in the control.

In this example a site is possibly DNS-blocked, but DNS control failure leads the current logic: https://explorer.ooni.org/measurement/20200504T045941Z_AS8048_jtDBVHr1LbzO92dFWQ2rgUTadiWfSgLmNIQ55T4a8CpciJWiek?input=https://saludvzla.com/

Perhaps a new category, similar to how it fails in other conditions, should be introduced; Or like on the explorer it says "Ok", instead of "Accesible"

Update: after reviewing form different points, indeed the domain is not giving any answers, so there wan't a problem with the control per se, this example doesn't reflect an error.

But it might still be reasonable to label such as response as "non inconsistent". If there are problems with the dns control or answers from control are manipulated, then it shouldn't necessarily be assumed that the site is accesible, even if the fail is the same.

Furthermore if there is an active HTTP block in place that couldn't be verified beacuse of a temporary DNS outage, perhaps the best is not to have it labeled as "accesible", again, no contact to the site was performed.

Specifications

  • Version:
    ooni probe 2.3.0
    measurement kit 0.10.11
  • Platform: iOs, presumably all platforms as it's part of the measurement (?)
@andresazp
Copy link
Author

Update, after reviewing form different points, indeed the domain is not giving any answers.
So take it as a grain of salt, as this example doesn't reflect an error, but it might still be reasonable to label such as response as "non inconsistenten"

If there are problems with the dns control or answers from control are manipulated, then it shouldn't necessarily be assumed that the site is accesible, even if the fail si the same

@hellais
Copy link
Member

hellais commented May 4, 2020

Thanks for reporting this.

In the measurements you link the control in fact did return a response which was consistent with the response of the probe.

When I lookup the domain now from this network I also get a DNS resolution error.

Since we did get a control response which was consistent with the probe we did not mark it as an inconsistency.

I think this is expected behaviour for the probe as we want to rule out false positives due to a website being down.

@bassosimone what do you think of this?

@hellais hellais transferred this issue from ooni/probe May 4, 2020
@bassosimone bassosimone changed the title Don't label site "accessible" if there is a control failure, specially not if really inaccesoble Don't label site "accessible" if there is a control failure, specially not if really inaccessible May 4, 2020
@bassosimone
Copy link
Contributor

@bassosimone what do you think of this?

Web Connectivity is working as intended in this example. As @hellais says, the domain does not exist, therefore the control and the probe are currently consistent.

At the same time, the Web Connectivity model needs to be improved, in my view. I can say that I sympathise with @andresazp concerns here. It seems to me that a good high level objective to pursue is to make the measurement result more obvious to understand.

On a more strategic standpoint, I am not able to say how. I have not spent enough time thinking on a comprehensive strategy to make this happen. (I am thinking about this, but I'd rather not write down my incomplete thoughts, and I'd rather spend more time thinking on that.)

On a more practical standpoint, I feel like this case where both the control and the probe agree that a website is down should not be framed as "accessible". Perhaps we should flag the website as being "down". I put, however, a previous paragraph regarding strategy because I feel like this problem is best solved by thinking at Web Connectivity in a more systemic way rather than going into the spec and adding a patch for this specific use case.

At the same time, this use case has been noted and will be kept into account. I will keep this issue open and will definitely tend to it when the time finally comes to rewrite Web Connectivity in Go.

This sprint I did lots of progress and have most building blocks in place.

While it is true that the rewrite in Go should primarily aim to be backwards compatible, it is also a very good time to think again at the model and design extensions and improvements.

Thanks for letting us know about this issue!

@bassosimone
Copy link
Contributor

(Moved the issue to Sprint 14, which AFAICT is when we'll rewrite Web Connectivity in Go.)

@bassosimone
Copy link
Contributor

We need to re-evaluate this, now that Web Connectivity is written in Go (ooni/probe-engine#806).

@bassosimone
Copy link
Contributor

See also measurement-kit/measurement-kit#1651

@bassosimone
Copy link
Contributor

See also measurement-kit/measurement-kit#1653

bassosimone referenced this issue in ooni/probe-engine Aug 13, 2020
@bassosimone
QA/webconnectivity.py: add test case for nonexistent domain
9eac20e 
The behavior of probe-engine is better than the one of MK. We have
disabled this check for MK and we should open an issue.

The following diff shows the difference in the toplevel keys:

```diff
--- MK.jsonl	2020-08-13 13:12:05.741791884 +0200
+++ OPE.jsonl	2020-08-13 13:12:14.092840549 +0200
@@ -1,9 +1,9 @@
 {
   "input": "http://antani.xyz",
   "test_keys": {
-    "accessible": true,
+    "accessible": null,
     "agent": "redirect",
-    "blocking": false,
+    "blocking": null,
     "body_length_match": null,
     "body_proportion": 0,
     "client_resolver": "91.80.36.88",
@@ -16,15 +16,16 @@
         "body_length": -1,
         "failure": "dns_lookup_error",
         "headers": {},
-        "status_code": -1
+        "status_code": -1,
+        "title": ""
       },
       "tcp_connect": {}
     },
     "control_failure": null,
     "dns_consistency": "consistent",
-    "dns_experiment_failure": null,
+    "dns_experiment_failure": "dns_nxdomain_error",
     "headers_match": null,
-    "http_experiment_failure": "dns_lookup_error",
+    "http_experiment_failure": null,
     "retries": null,
     "socksproxy": null,
     "status_code_match": null,
```

Because `antani.xyz` does not exist, OPE is clearly right here. I am also
wondering whether this issue with MK is related to the following issue:

https://github.com/ooni/probe-engine/issues/579

Maybe?

What I find rather interesting, also, is that there is no indication of
"dns_experiment_failure" in MK even though it clearly failed. Yet, we see
consistency in the "dns_consistency" key. But, still, I would argue it's
not so smart to elide the DNS failure in this case.

Also, note that the HTTP experiment is not even performed by OPE because
we really don't know what to do without IP addresses for the domain.
bassosimone referenced this issue in ooni/probe-engine Aug 13, 2020
@bassosimone
QA/webconnectivity.py: add test case for nonexistent domain (#842)
049283a 
The behavior of probe-engine is better than the one of MK. We have
disabled this check for MK and we should open an issue.

The following diff shows the difference in the toplevel keys:

```diff
--- MK.jsonl	2020-08-13 13:12:05.741791884 +0200
+++ OPE.jsonl	2020-08-13 13:12:14.092840549 +0200
@@ -1,9 +1,9 @@
 {
   "input": "http://antani.xyz",
   "test_keys": {
-    "accessible": true,
+    "accessible": null,
     "agent": "redirect",
-    "blocking": false,
+    "blocking": null,
     "body_length_match": null,
     "body_proportion": 0,
     "client_resolver": "91.80.36.88",
@@ -16,15 +16,16 @@
         "body_length": -1,
         "failure": "dns_lookup_error",
         "headers": {},
-        "status_code": -1
+        "status_code": -1,
+        "title": ""
       },
       "tcp_connect": {}
     },
     "control_failure": null,
     "dns_consistency": "consistent",
-    "dns_experiment_failure": null,
+    "dns_experiment_failure": "dns_nxdomain_error",
     "headers_match": null,
-    "http_experiment_failure": "dns_lookup_error",
+    "http_experiment_failure": null,
     "retries": null,
     "socksproxy": null,
     "status_code_match": null,
```

Because `antani.xyz` does not exist, OPE is clearly right here. I am also
wondering whether this issue with MK is related to the following issue:

https://github.com/ooni/probe-engine/issues/579

Maybe?^H^H^H Most likely

What I find rather interesting, also, is that there is no indication of
"dns_experiment_failure" in MK even though it clearly failed. Yet, we see
consistency in the "dns_consistency" key. But, still, I would argue it's
not so smart to elide the DNS failure in this case.

Also, note that the HTTP experiment is not even performed by OPE because
we really don't know what to do without IP addresses for the domain.
@bassosimone bassosimone changed the title Don't label site "accessible" if there is a control failure, specially not if really inaccessible webconnectivity: properly flag NXDOMAIN caused by nonexisting website Aug 13, 2020
@bassosimone
Copy link
Contributor

Changed the title of the issue to reflect the considerations made in ooni/probe-engine#842.

bassosimone referenced this issue in ooni/probe-engine Aug 14, 2020
@bassosimone
QA/webconnectivity: do like MK when domain is NXDOMAIN
b597576 
This impacts on https://github.com/ooni/probe-engine/issues/579.

Part of #810.
@bassosimone
Copy link
Contributor

In ooni/probe-engine#846 I've changed the implementation of probe-engine to do exactly what MK does, because it seems more proper to mimic MK first and fix the underlying issue later.

bassosimone referenced this issue in ooni/probe-engine Aug 14, 2020
@bassosimone
QA/webconnectivity: do like MK when domain is NXDOMAIN (#846)
0e4aebb 
This impacts on https://github.com/ooni/probe-engine/issues/579.

Part of #810.
@bassosimone bassosimone transferred this issue from ooni/probe-engine Jun 7, 2021
@bassosimone bassosimone added the bug Something isn't working label Jul 6, 2021
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 25, 2023
@bassosimone
chore(webconnectivityqa): import webconnectivity_nonexistent_domain
462f50c 
This diff imports the QA/webconnectivity.py integration test into
the webconnectivityqa framework and removes it from Python.

Part of ooni/probe#1803.

See also ooni/probe#1536.
@bassosimone bassosimone mentioned this issue Aug 25, 2023
Merged
4 tasks
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 25, 2023
@bassosimone
chore(webconnectivityqa): import webconnectivity_nonexistent_domain (#…
1e3a47d 
…1213)

This diff imports the QA/webconnectivity.py integration test into the
webconnectivityqa framework and removes it from Python. While there,
remove other QA/webconnectivity.py tests that I have already converted.

Part of ooni/probe#1803.

See also ooni/probe#1536.

## Checklist

- [x] I have read the [contribution
guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request: see above
- [x] if you changed anything related to how experiments work and you
need to reflect these changes in the ooni/spec repository, please link
to the related ooni/spec pull request: N/A
- [x] if you changed code inside an experiment, make sure you bump its
version number: N/A
@bassosimone
Copy link
Contributor

I fixed this in ooni/probe-cli#1462.

@bassosimone bassosimone added the 2024-01-data-quality-cleanup Data quality issues addressed on 2024-01 label Jan 26, 2024
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
@bassosimone @Murphy-OrangeMud
chore(webconnectivityqa): import webconnectivity_nonexistent_domain (o…
582f095 
…oni#1213)

This diff imports the QA/webconnectivity.py integration test into the
webconnectivityqa framework and removes it from Python. While there,
remove other QA/webconnectivity.py tests that I have already converted.

Part of ooni/probe#1803.

See also ooni/probe#1536.

## Checklist

- [x] I have read the [contribution
guidelines](https://github.com/ooni/probe-cli/blob/master/CONTRIBUTING.md)
- [x] reference issue for this pull request: see above
- [x] if you changed anything related to how experiments work and you
need to reflect these changes in the ooni/spec repository, please link
to the related ooni/spec pull request: N/A
- [x] if you changed code inside an experiment, make sure you bump its
version number: N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
2024-01-data-quality-cleanup Data quality issues addressed on 2024-01 bug Something isn't working data quality ooni/probe-engine priority/low
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bassosimone @hellais @andresazp