Skip to content

Fix: Handle CNAME in Quad9 resolver and prevent DNS visualizer errors (#2959) #3005

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
Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Fix: Handle CNAME in Quad9 resolver and prevent DNS visualizer errors (#2959) #3005

wants to merge 6 commits into from

Conversation

PranavShukla7
Copy link

@PranavShukla7 PranavShukla7 commented Sep 18, 2025 •
edited
Loading

Description

This PR fixes the errors reported in issue #2959 during DNS analysis.

Normalized Quad9 DNS records (A, AAAA, CNAME) into a consistent dict format.

Ensured the resolutions key is always present in analyzer reports.

Prevented runtime errors (AttributeError, TypeError, KeyError) when CNAME records are returned.

This improves stability of DNS analysis in both Quad9 resolver and DNS visualizer.

Type of change

  • [✓ ] Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

Checklist

  • [ ✓] I have read and understood the rules about how to Contribute to this project
  • [ ✓] The pull request is for the branch develop
  • A new plugin (analyzer, connector, visualizer, playbook, pivot or ingestor) was added or changed, in which case:
    • I strictly followed the documentation "How to create a Plugin"
    • Usage file was updated. A link to the PR to the docs repo has been added as a comment here.
    • Advanced-Usage was updated (in case the plugin provides additional optional configuration). A link to the PR to the docs repo has been added as a comment here.
    • I have dumped the configuration from Django Admin using the dumpplugin command and added it in the project as a data migration. ("How to share a plugin with the community")
    • If a File analyzer was added and it supports a mimetype which is not already supported, you added a sample of that type inside the archive test_files.zip and you added the default tests for that mimetype in test_classes.py.
    • If you created a new analyzer and it is free (does not require any API key), please add it in the FREE_TO_USE_ANALYZERS playbook by following this guide.
    • Check if it could make sense to add that analyzer/connector to other freely available playbooks.
    • I have provided the resulting raw JSON of a finished analysis and a screenshot of the results.
    • If the plugin interacts with an external service, I have created an attribute called precisely url that contains this information. This is required for Health Checks (HEAD HTTP requests).
    • If the plugin requires mocked testing, _monkeypatch() was used in its class to apply the necessary decorators.
    • I have added that raw JSON sample to the MockUpResponse of the _monkeypatch() method. This serves us to provide a valid sample for testing.
    • I have created the corresponding DataModel for the new analyzer following the documentation
  • [ ✓] I have inserted the copyright banner at the start of the file: # This file is a part of IntelOwl https://github.com/intelowlproject/IntelOwl # See the file 'LICENSE' for copying permission.
  • [✓ ] Please avoid adding new libraries as requirements whenever it is possible. Use new libraries only if strictly needed to solve the issue you are working for. In case of doubt, ask a maintainer permission to use a specific library.
  • If external libraries/packages with restrictive licenses were added, they were added in the Legal Notice section.
  • [ ✓] Linters (Black, Flake, Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.
  • I have added tests for the feature/bug I solved (see tests folder). All the tests (new and old ones) gave 0 errors.
  • If the GUI has been modified:
    • I have a provided a screenshot of the result in the PR.
    • I have created new frontend tests for the new component or updated existing ones.
  • [ ✓] After you had submitted the PR, if DeepSource, Django Doctors or other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.

@mlodic
Copy link
Member

mlodic commented Sep 19, 2025

hey thanks for the contribution. However it is really important that you don't delete the PR template because there are steps that you need to follow to have this PR to be approved. So please, re-add it, read through it and adjust the PR accordingly.

Also, this PR does not solve the 2nd part of the reported issues, the one related to the visualizer.

Thank you

@mlodic mlodic mentioned this pull request Sep 19, 2025
Open
@PranavShukla7
Copy link
Author

@mlodic Thanks for pointing that out. I’ll re-add the PR template and update it accordingly.

@PranavShukla7
Copy link
Author

Hey @mlodic I have updated my PR could you please review it?

@mlodic mlodic linked an issue Sep 26, 2025 that may be closed by this pull request
DNS analysis errors in visualizers and Quad9 #2959
Open
mlodic
mlodic reviewed Sep 26, 2025
View reviewed changes
@mlodic
Copy link
Member

mlodic commented Sep 26, 2025

the PR is not for the develop branch....and again, the PR template is there to be there completely, not just parts. I am waiting for these changes, otherwise I'll have to close the PR.

@PranavShukla7 PranavShukla7 changed the base branch from master to develop September 26, 2025 13:01
Fix: DNS visualizer handle missing/empty 'resolutions' (intelowlproje…
c8521c6 
…ct#2959)

Ensure visualizer handles analyzer reports without 'resolutions' and works with normalized
records from Quad9 resolver.
@PranavShukla7 PranavShukla7 force-pushed the fix/quad9-dns-issue-2959 branch from 8995af1 to c8521c6 Compare September 27, 2025 19:43
@PranavShukla7
Copy link
Author

Hey @mlodic , I’ve updated the PR to target develop and filled out the full template. Could you please verify when you get a chance?

mlodic
mlodic reviewed Sep 29, 2025
View reviewed changes
Copy link
Member

@mlodic mlodic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please execute an analysis of a random domain like google.com with quad9 and screen shot the result?

@PranavShukla7
Implement Quad9DNSResolver with JSON response parsing
0f8bb85 
Switched the Quad9DNSResolver to use the Quad9 DoH JSON API instead of 
raw DNS wire parsing. This makes the analyzer simpler and aligned with 
maintainer’s request. 

The response is now parsed from the "Answer" section of the JSON to extract A record resolutions, while preserving retry and error handling logic.
Also restored the missing comment for clarity.
@PranavShukla7
Copy link
Author

@mlodic , I first tried running the Quad9 analysis inside Docker, but ran into permission issues with the container.
To work around this, I tested the same DNS-over-HTTPS flow using Cloudflare, just to confirm the analyzer logic is working as intended.
Here’s the result for google.com:
Screenshot From 2025-10-03 13-01-17

@mlodic
Copy link
Member

mlodic commented Oct 3, 2025

@PranavShukla7 hey, thanks for testing this but still this does not prove that it works as intended in IntelOwl. It's important to have a local installation of IntelOwl and try every change done in the plugins manually at least once to confirm that there are no regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mlodic mlodic mlodic left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DNS analysis errors in visualizers and Quad9
3 participants
@PranavShukla7 @mlodic @drosetti