Skip to content

Fix #1378 - Switch to stub resolver, remove pyunbound - #1578

Merged
mxsasha merged 5 commits into
mainfrom
stub-resolver
Jan 29, 2025
Merged

Fix #1378 - Switch to stub resolver, remove pyunbound#1578
mxsasha merged 5 commits into
mainfrom
stub-resolver

Conversation

@mxsasha

@mxsasha mxsasha commented Dec 3, 2024

Copy link
Copy Markdown
Collaborator

Fun discoveries so far:

  • IPv6 NS connectivity check just checks if port 53 TCP is open. Not whether it's DNS, that's only checked when we fall back to UDP.
  • Precheck for mail says in the UI it looks for SOA, but it actually checks if any record exists at the label. It queries for SOA, but accepts NOANSWER.
  • It looks like we check MX dnssec by asking for the SOA? We don't check if the actual MX record is secure? See Check for A/AAAA/MX DNSSEC records #283 too.
  • The DNSSEC test says: "If a domain redirects to another domain via CNAME, then we also check if the CNAME domain is signed (which is conformant with the DNSSEC standard). If the CNAME domain is not signed, the result of this subtest will be negative.". I have been unable to locate any code that does this. No guarantees, it is possible I overlooked it.

TODO:

  • Inline todo's
  • Review
  • Remove permissive resolver
  • Remove or document remaining cb_data use (might clash with TLS branch and need to be left for later)
  • Remove compatibility shim after updating to dnspython 2.7 in Update all python deps, drop py3.7 pins #1610
  • Check do_resolve_aaaa rename to do_resolve_single_aaaa
  • Add to changelog
  • Fix up commits

@mxsasha
mxsasha force-pushed the stub-resolver branch 5 times, most recently from d9b3f7a to 6b8fc04 Compare January 9, 2025 15:43
@mxsasha mxsasha changed the title [WIP] Fix #1378 - Switch to stub resolver, remove pyunbound [hold for 1.9 fork] Fix #1378 - Switch to stub resolver, remove pyunbound Jan 10, 2025
@bwbroersma

Copy link
Copy Markdown
Collaborator

🎉 Great work!
Now IPV4_IP_RESOLVER_INTERNAL_PERMISSIVE is only 'used' on these two spots:

So I would say it's not really used (at least in production).
Not sure if the first message is even right, and the second can probably be rewritten. Then the whole resolver-permissive container (and config) can be removed, which would be nice.

@bwbroersma

Copy link
Copy Markdown
Collaborator

So this refactor the IPv6 connectivity:

  • leaves the TCP 53 port check in (as first method)
  • changes DNS resolving from unbound (with forward) to stub, can this somehow form an issue?

@bwbroersma

Copy link
Copy Markdown
Collaborator

BTW this refactor also centralizes DNS queries in dns_resolve.
So logging all DNS queries, including if they are valid DNSSEC, can be done more easily:

def dns_resolve(qname: str, rr_type: RdataType, allow_bogus=True, raise_on_no_answer=True):
answer = _get_resolver().resolve(dns.name.from_text(qname), rr_type, raise_on_no_answer=raise_on_no_answer)
dnssec_status = DNSSECStatus.from_message(answer.response)
if dnssec_status == DNSSECStatus.BOGUS and not allow_bogus:
raise ValidationFailure()
return answer.rrset, dnssec_status

Comment thread checks/http_client.py
Comment thread checks/resolver.py Outdated
Comment thread checks/tasks/routing.py Outdated
Comment thread checks/tasks/dnssec.py Outdated
Comment thread checks/tasks/dnssec.py Outdated
@mxsasha
mxsasha force-pushed the stub-resolver branch 3 times, most recently from c974736 to b970fcb Compare January 13, 2025 18:59
@mxsasha
mxsasha marked this pull request as ready for review January 13, 2025 19:09
@mxsasha mxsasha added this to the v1.10 milestone Jan 22, 2025
@mxsasha mxsasha changed the title [hold for 1.9 fork] Fix #1378 - Switch to stub resolver, remove pyunbound Fix #1378 - Switch to stub resolver, remove pyunbound Jan 28, 2025
@mxsasha
mxsasha force-pushed the stub-resolver branch 2 times, most recently from 4ab44f2 to 6073ff8 Compare January 28, 2025 17:28
@mxsasha
mxsasha deleted the stub-resolver branch January 29, 2025 13:17
@mxsasha
mxsasha restored the stub-resolver branch January 29, 2025 13:17
@mxsasha
mxsasha deleted the stub-resolver branch January 29, 2025 13:17
mxsasha pushed a commit that referenced this pull request Feb 6, 2025
… docs ref #1578 (#1656)

Signed-off-by: Joris Le Blansch <ping@apio.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants