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

netxlite: land improvements from websteps winter 2022 repo #2096

Closed
bassosimone opened this issue May 13, 2022 · 2 comments
Closed

netxlite: land improvements from websteps winter 2022 repo #2096

bassosimone opened this issue May 13, 2022 · 2 comments
Assignees
@bassosimone
Labels
chore routine tasks that must be done, but require little active brain power data quality enhancement improving existing code or new feature ooni/probe-engine priority/low

Comments

@bassosimone
Copy link
Contributor

This issue is about landing netxlite patches developed in the websteps winter 2022 repository. See https://github.com/bassosimone/websteps-illustrated/tree/a65f3e8579b59565789a1f38c468b0a9646783cd/internal/netxlite.
@bassosimone bassosimone added enhancement improving existing code or new feature priority/medium chore routine tasks that must be done, but require little active brain power ooni/probe-engine labels May 13, 2022
@bassosimone bassosimone self-assigned this May 13, 2022
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
feat(netxlite): implement parallel resolver
b321bea 
This diff imports the parallel resolver from websteps winter 2022
edition, which was originally implemented here:

bassosimone/websteps-illustrated@55231d7

See ooni/probe#2096
@bassosimone bassosimone mentioned this issue May 13, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
feat(netxlite): implement parallel resolver (#724)
ec0561e 
This diff imports the parallel resolver from websteps winter 2022
edition, which was originally implemented here:

bassosimone/websteps-illustrated@55231d7

See ooni/probe#2096
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
fix(netxlite): LookupHTTPS: short circuit IP addr
b0a8237 
This diff fixes the short-circuit-IP-addr resolver to
correctly handle IP addrs during LookupHTTPS.

The original diff was: bassosimone/websteps-illustrated@2b51d14

See ooni/probe#2096

While there, add unit tests for IPv6.
@bassosimone bassosimone mentioned this issue May 13, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
fix(netxlite): LookupHTTPS: short circuit IP addr (#725)
e126e73 
This diff fixes the short-circuit-IP-addr resolver to
correctly handle IP addrs during LookupHTTPS.

The original diff was: bassosimone/websteps-illustrated@2b51d14

See ooni/probe#2096

While there, add unit tests for IPv6.
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
fix(netxlite): consolidate IPv4/IPv6 checking code
c62b5f1 
Originally bassosimone/websteps-illustrated@966e7f7

See ooni/probe#2096
@bassosimone bassosimone mentioned this issue May 13, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
fix(netxlite): consolidate IPv4/IPv6 checking code (#726)
192dfd4 
Originally bassosimone/websteps-illustrated@966e7f7

See ooni/probe#2096
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
fix(netxlite): HTTPSSvc: better no_answer checks
c3e19d3 
I've seen some measurements returning some IP addresses for HTTPSSvc
queries but not returning any ALPN value.

For example:

```JSON
% d4
decoding DNS round trip 0:

;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 57768
;; flags: rd; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 0
;; QUESTION SECTION:
;psiphon.ca.                    IN      HTTPS

;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 57768
;; flags: qr rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0
;; QUESTION SECTION:
;psiphon.ca.                    IN      HTTPS
;; ANSWER SECTION:
psiphon.ca.             121     IN      A       31.13.85.53
```

Now, the response is clearly bogus. At the time of this writing that
IP address belongs to Facebook. This measurement has been collected in
China, so it's expected for the GFW to behave like this.

Yet, I don't feel like it's accurate to report this measurement as a
"no answer" response. Rather, this response is a valid one containing
a clearly invalid IP address and should be flagged as such.

Originally: bassosimone/websteps-illustrated@57a023b

See ooni/probe#2096
@bassosimone bassosimone mentioned this issue May 13, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
fix(netxlite): HTTPSSvc: better no_answer checks (#727)
b872dd0 
I've seen some measurements returning some IP addresses for HTTPSSvc
queries but not returning any ALPN value.

For example:

```
% d4
decoding DNS round trip 0:

;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 57768
;; flags: rd; QUERY: 1, ANSWER: 0, AUTHORITY: 0, ADDITIONAL: 0
;; QUESTION SECTION:
;psiphon.ca.                    IN      HTTPS

;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 57768
;; flags: qr rd ra; QUERY: 1, ANSWER: 1, AUTHORITY: 0, ADDITIONAL: 0
;; QUESTION SECTION:
;psiphon.ca.                    IN      HTTPS
;; ANSWER SECTION:
psiphon.ca.             121     IN      A       31.13.85.53
```

Now, the response is clearly bogus. At the time of this writing that
IP address belongs to Facebook. This measurement has been collected in
China, so it's expected for the GFW to behave like this.

Yet, I don't feel like it's accurate to report this measurement as a
"no answer" response. Rather, this response is a valid one containing
a clearly invalid IP address and should be flagged as such.

Originally: bassosimone/websteps-illustrated@57a023b

See ooni/probe#2096
bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
fix(netxlite): map servfail error
57f851d 
This error occurred for example when querying kazemjalali.com
in websteps measurements run from Iran.

This error is relatively uncommon, but it still makes sense to
create a specific mapping rule for it.

Originally: bassosimone/websteps-illustrated@4269e82

See ooni/probe#2096
@bassosimone bassosimone mentioned this issue May 13, 2022
Merged
@bassosimone
Copy link
Contributor Author

The next diff to apply is bassosimone/websteps-illustrated@c2f7cca (see https://github.com/bassosimone/websteps-illustrated/commits/main/internal/netxlite)

bassosimone added a commit to ooni/probe-cli that referenced this issue May 13, 2022
@bassosimone
fix(netxlite): map servfail error (#728)
5904e69 
This error occurred for example when querying kazemjalali.com
in websteps measurements run from Iran.

This error is relatively uncommon, but it still makes sense to
create a specific mapping rule for it.

Originally: bassosimone/websteps-illustrated@4269e82

See ooni/probe#2096
bassosimone added a commit to ooni/probe-cli that referenced this issue May 14, 2022
@bassosimone
netxlite: factor common code for parsing UDPAddr
3ec8b6f 
This diff has been extracted from bassosimone/websteps-illustrated@c2f7cca

See ooni/probe#2096
@bassosimone bassosimone mentioned this issue May 14, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 14, 2022
@bassosimone
netxlite: factor common code for parsing UDPAddr (#730)
6c388d2 
This diff has been extracted from bassosimone/websteps-illustrated@c2f7cca

See ooni/probe#2096
bassosimone added a commit to ooni/probe-cli that referenced this issue May 14, 2022
@bassosimone
refactor(netxlite): add Transport suffix to DNS transports
69161be 
This diff has been extracted from bassosimone/websteps-illustrated@c2f7cca

See ooni/probe#2096
@bassosimone bassosimone mentioned this issue May 14, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 14, 2022
@bassosimone
refactor(netxlite): add Transport suffix to DNS transports (#731)
f5b801a 
This diff has been extracted from bassosimone/websteps-illustrated@c2f7cca

See ooni/probe#2096
bassosimone added a commit to ooni/probe-cli that referenced this issue May 14, 2022
@bassosimone
fix(netxlite): reject replies with wrong queryID
65b745f 
This diff has been extracted from bassosimone/websteps-illustrated@c2f7cca

See ooni/probe#2096

While there, export DecodeReply to decode a raw reply without
interpreting the Rcode or parsing the results, which seems a
nice extra feature to have to more flexibly parse DNS replies
in other parts of the codebase.
@bassosimone bassosimone mentioned this issue May 14, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 14, 2022
@bassosimone
fix(netxlite): reject replies with wrong queryID (#732)
9d2301c 
This diff has been extracted from bassosimone/websteps-illustrated@c2f7cca

See ooni/probe#2096

While there, export DecodeReply to decode a raw reply without
interpreting the Rcode or parsing the results, which seems a
nice extra feature to have to more flexibly parse DNS replies
in other parts of the codebase.
bassosimone added a commit to ooni/probe-cli that referenced this issue May 15, 2022
@bassosimone
fix(netxlite): prefer composition over embedding
c0e37a9 
This diff has been extracted from bassosimone/websteps-illustrated@8848c8c

See ooni/probe#2096

While there, perform comprehensive netxlite code review
and apply minor changes and improve the docs.
@bassosimone bassosimone mentioned this issue May 15, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 15, 2022
@bassosimone
fix(netxlite): prefer composition over embedding (#733)
c1b06a2 
This diff has been extracted and adapted from bassosimone/websteps-illustrated@8848c8c

The reason to prefer composition over embedding is that we want the
build to break if we add new methods to interfaces we define. If the build
does not break, we may forget about wrapping methods we should
actually be wrapping. I noticed this issue inside netxlite when I was working
on websteps-illustrated and I added support for NS and PTR queries.

See ooni/probe#2096

While there, perform comprehensive netxlite code review
and apply minor changes and improve the docs.
bassosimone added a commit to ooni/probe-cli that referenced this issue May 16, 2022
@bassosimone
feat(netxlite): implements NS queries
85892bf 
This diff has been extracted from bassosimone/websteps-illustrated@eb0bf38.

See ooni/probe#2096.

While there, skip the broken tests caused by issue
ooni/probe#2098.
@bassosimone bassosimone mentioned this issue May 16, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 16, 2022
@bassosimone
feat(netxlite): implements NS queries (#734)
ce052b6 
This diff has been extracted from bassosimone/websteps-illustrated@eb0bf38.

See ooni/probe#2096.

While there, skip the broken tests caused by issue
ooni/probe#2098.
bassosimone added a commit to ooni/probe-cli that referenced this issue May 16, 2022
@bassosimone
feat(netxlite): ensure we only accept DNS responses
a449050 
Previously, the DNS decoded did not check whether it was parsing
a DNS query or a DNS response, which was wrong.

As a side note, it seems I am using "reply" in the codebase instead
of "response", which seems bettern DNS terminology.

This diff has been extracted from bassosimone/websteps-illustrated@9249d14

See ooni/probe#2096.
@bassosimone bassosimone mentioned this issue May 16, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue May 16, 2022
@bassosimone
fix(netxlite): ensure we only accept DNS responses (#735)
7c45f7b 
Previously, the DNS decoder did not check whether it was parsing
a DNS query or a DNS response, which was wrong.

As a side note, it seems I am using "reply" in the codebase instead
of "response". The latter seems correct DNS terminology.

This diff has been extracted from bassosimone/websteps-illustrated@9249d14

See ooni/probe#2096.
@bassosimone bassosimone mentioned this issue May 17, 2022
Closed
bassosimone added a commit to ooni/probe-cli that referenced this issue Jun 1, 2022
@bassosimone
refactor(netxlite): introduce the getaddrinfo transport
6209071 
This diff modifies the system resolver to use a getaddrinf transport.

Obviously the transport is a fake, but its existence will allow us
to observe DNS events more naturally.

A lookup using the system resolver would be a ANY lookup that will
contain all the resolved IP addresses into the same response.

This change was also part of websteps-illustrated, albeit the way in
which I did it there was less clean than what we have here.

Ref issue: ooni/probe#2096
@bassosimone bassosimone mentioned this issue Jun 1, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jun 1, 2022
@bassosimone
refactor(netxlite): introduce the getaddrinfo transport (#775)
923d81c 
This diff modifies the system resolver to use a getaddrinf transport.

Obviously the transport is a fake, but its existence will allow us
to observe DNS events more naturally.

A lookup using the system resolver would be a ANY lookup that will
contain all the resolved IP addresses into the same response.

This change was also part of websteps-illustrated, albeit the way in
which I did it there was less clean than what we have here.

Ref issue: ooni/probe#2096
bassosimone added a commit to ooni/probe-cli that referenced this issue Jun 1, 2022
@bassosimone
feat(netxlite): implement DNSTransport wrapping
e66a945 
Acknowledge that transports MAY be used in isolation (i.e., outside
of a Resolver) and add support for wrapping.

Ensure that every factory that creates an unwrapped type is named
accordingly to hopefully ensure there are no surprises.

Implement DNSTransport wrapping and use a technique similar to the
one used by Dialer to customize the DNSTransport while constructing
more complex data types (e.g., a specific resolver).

Ensure that the stdlib resolver's own "getaddrinfo" transport (1)
is wrapped and (2) could be extended during construction.

This work is part of my ongoing effort to bring to this repository
websteps-illustrated changes relative to netxlite.

Ref issue: ooni/probe#2096
@bassosimone bassosimone mentioned this issue Jun 1, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Jun 1, 2022
@bassosimone
feat(netxlite): implement DNSTransport wrapping (#776)
8f7e380 
Acknowledge that transports MAY be used in isolation (i.e., outside
of a Resolver) and add support for wrapping.

Ensure that every factory that creates an unwrapped type is named
accordingly to hopefully ensure there are no surprises.

Implement DNSTransport wrapping and use a technique similar to the
one used by Dialer to customize the DNSTransport while constructing
more complex data types (e.g., a specific resolver).

Ensure that the stdlib resolver's own "getaddrinfo" transport (1)
is wrapped and (2) could be extended during construction.

This work is part of my ongoing effort to bring to this repository
websteps-illustrated changes relative to netxlite.

Ref issue: ooni/probe#2096
@bassosimone
Copy link
Contributor Author

bassosimone commented Jun 13, 2022 •
edited
Loading

These are the diffs we still need to import into netxlite:

@bassosimone bassosimone mentioned this issue Aug 23, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
chore routine tasks that must be done, but require little active brain power data quality enhancement improving existing code or new feature ooni/probe-engine priority/low
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bassosimone @jbonisteel