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

Domain Scanner #1146

Merged
merged 46 commits into from
Oct 2, 2024
Merged

Domain Scanner #1146

merged 46 commits into from
Oct 2, 2024

Conversation

kshitijk4poor
Copy link
Collaborator

@kshitijk4poor kshitijk4poor commented Jul 13, 2024

🚀 Implement domain validation and task addition for efficient scanning

📝 Description

This PR introduces two complementary methods in the ArtemisBase class: check_domain_exists for domain validation and add_valid_domains_task for adding tasks for valid domains. The primary purpose of these additions is to ensure that only valid domains are sent for further scanning, improving efficiency and reducing unnecessary processing.

🔧 Changes

  • ✨ Added check_domain_exists and add_valid_domains_task method to ArtemisBase class
  • 🔍 Implemented domain validation logic using NS and A record lookups
  • 🔄 Created a method to filter and add tasks only for valid domains
  • Fix mock setup in ArtemisModuleTestCase to handle multiple arguments in lookup function calls, resolving TypeError in tests.
    • This change is important because:
      • It fixes a breaking error in the test suite, allowing tests to run successfully.
      • It more accurately mimics the real lookup function's signature, improving test fidelity.
      • It prevents false negatives in tests that could mask real issues in the code being tested.
      • It ensures that the mock can handle various lookup calls throughout the codebase, increasing test coverage and reliability.

🌟 Benefits

  • 🎯 Ensures only valid domains proceed to further scanning stages
  • 🚀 Improves overall system efficiency by reducing unnecessary scans
  • 🧩 Provides reusable domain validation logic across modules
  • 🔄 Centralizes the process of adding tasks for valid domains
  • 🛡️ Reduces potential false positives from invalid or non-existent domains

🤝 How the methods complement each other

  1. check_domain_exists verifies the validity of a domain by checking for NS or A records
  2. add_valid_domains_task uses check_domain_exists to filter out invalid domains
  3. Only domains that pass the validation are added as new tasks for further scanning

This combination ensures a streamlined and efficient domain processing pipeline.

📝Testing

  • Updated unit tests for the check_domain_exists method to ensure all scenarios are covered, including:
    • Domains with only NS records
    • Domains with only A records
    • Domains with both NS and A records
    • Non-existent domains
    • Scenarios where DNS resolution fails

TODO

  • remove NS records for subdomains maybe? since most of them will inherit it from the parent domains and for subdomains only A records will be checked.

📚 Related Issues

Closes #1094

@kshitijk4poor kshitijk4poor changed the title DNS Resolver Module DomainScanner Module Jul 22, 2024
@kshitijk4poor kshitijk4poor changed the title DomainScanner Module Domain Scanner Aug 2, 2024
.gitignore Outdated Show resolved Hide resolved
docker-compose.yaml Outdated Show resolved Hide resolved
@@ -214,7 +214,7 @@ def run(self, current_task: Task) -> None:
},
)

self.add_task(current_task, new_task)
self.add_task_if_domain_exists(current_task, new_task)
Copy link
Member

Choose a reason for hiding this comment

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

if we have open ports, we can be sure the domain exists ;) but checking in classifier is a good idea as well as it processes incoming tasks

michalkrzem
michalkrzem previously approved these changes Sep 12, 2024
michalkrzem
michalkrzem previously approved these changes Sep 12, 2024
@kazet
Copy link
Member

kazet commented Sep 16, 2024

waits for #1261 for the tests to pass

@kazet kazet merged commit 1380c7b into CERT-Polska:main Oct 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If a domain doesn't exist, don't scan it
3 participants