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.

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

Move the NXDomain catch to look at the results now that we dont raise… #3997

Merged
merged 4 commits into from
Jan 10, 2025

Conversation

underdarknl
Copy link
Contributor

… on no_result anymore due to dnssec error handling.

Changes

Looks a the resulting result list and decided an empty list means nxdomain, instead of waiting for the module to raise, which we told it not to.

Issue link

Closes #3996

Demo

Please add some proof in the form of screenshots or screen recordings to show (off) new functionality, if there are interesting new features for end-users.

QA notes

Please add some information for QA on how to test the newly created code.


Code Checklist

  • All the commits in this PR are properly PGP-signed and verified.
  • This PR only contains functionality relevant to the issue.
  • I have written unit tests for the changes or fixes I made.
  • I have checked the documentation and made changes where necessary.
  • I have performed a self-review of my code and refactored it to the best of my abilities.
  • Tickets have been created for newly discovered issues.
  • For any non-trivial functionality, I have added integration and/or end-to-end tests.
  • I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
  • I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.


Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

… on no_result anymore due to dnssec error handling.
@underdarknl underdarknl added the boefjes Issues related to boefjes label Dec 31, 2024
@underdarknl underdarknl requested a review from a team as a code owner December 31, 2024 11:00
ammar92
ammar92 previously approved these changes Jan 3, 2025
@ammar92
Copy link
Contributor

ammar92 commented Jan 3, 2025

While QA, I still get the following error:

Traceback (most recent call last):
  File "/app/boefjes/boefjes/local.py", line 58, in run
    return boefje_resource.module.run(boefje_meta)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/boefjes/boefjes/plugins/kat_dns/main.py", line 64, in run
    "dmarc_response": get_email_security_records(resolver, hostname, "_dmarc"),
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/boefjes/boefjes/plugins/kat_dns/main.py", line 87, in get_email_security_records
    answer = resolver.resolve(f"{record_subdomain}.{hostname}", "TXT", raise_on_no_answer=False)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/dns/resolver.py", line 1307, in resolve
    (request, answer) = resolution.next_request()
                        ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/dns/resolver.py", line 749, in next_request
    raise NXDOMAIN(qnames=self.qnames_to_try, responses=self.nxdomain_responses)
dns.resolver.NXDOMAIN: The DNS query name does not exist: _dmarc.ns1-125.akam.net.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/app/boefjes/boefjes/job_handler.py", line 144, in handle
    boefje_results = self.job_runner.run(boefje_meta, boefje_meta.environment)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/boefjes/boefjes/local.py", line 60, in run
    raise JobRuntimeError("Boefje failed") from e
boefjes.runtime_interfaces.JobRuntimeError: Boefje failed

This was a result with input OOI Hostname|internet|ns1-125.akam.net. I suppose a valid RRSet is returned while the exception isn't caught (because it's removed in the suggested implementation). According to the docs, passing a raise_on_no_answer as False means it won't raise a dns.resolver.NoAnswer exception if there’s no answer to the question, but it would still raise the other exceptions such as timeouts and dns.resolver.NXDOMAIN.

@ammar92
Copy link
Contributor

ammar92 commented Jan 8, 2025

Tested and seems to work again. No errors and no DKIMExist objects yielded

Copy link

sonarqubecloud bot commented Jan 9, 2025

@dekkers dekkers merged commit a09d6b1 into main Jan 10, 2025
21 checks passed
@dekkers dekkers deleted the fix/invalidnxdomainresults branch January 10, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
boefjes Issues related to boefjes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We do not catch NXDomain/NoAswer on invalid Domainkey / dmarc record queries.
3 participants