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

Added --checks arg #34

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Added --checks arg #34

wants to merge 7 commits into from

Conversation

kz0ltan
Copy link

@kz0ltan kz0ltan commented Nov 20, 2021

Added --checks arg

Description

--checks can be used to select check methods to use for cdn detection

💭 Motivation and context

Certain checks can slow the detection process down unnecessarily (depending on the pipeline of course). This way, those can be disabled.

🧪 Testing

Just use the --checks argument to select the check methods needed:
--checks= Select detection types; possible values:
cname (c), HTTP headers (h), nameservers (n),
whois data (w). Default: "chnw"

✅ Checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • All relevant repo and/or project documentation has been updated
    to reflect the changes in this PR.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

…rs; collect all headers from previous responses, before redirections
Copy link
Collaborator

@Pascal-0x90 Pascal-0x90 left a comment

Choose a reason for hiding this comment

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

Along with my comments, this branch also needs to be updated to merge in the current state of cisagov:develop branch.

@@ -59,7 +95,7 @@ def __init__(self):

def ip(self, dom: Domain) -> List[int]:
"""Determine IP addresses the domain resolves to."""
dom_list: List[str] = [dom.url, "www." + dom.url]
Copy link
Collaborator

Choose a reason for hiding this comment

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

My main motivation was to add www. for domains which don't directly answer to their domain name but instead they will listen to www.domain.com. Is there a reason why you wanted to remove "www." ?

I have also thought about pairing in a light subdomain enumerator using like google but I think that's too big of a scope.

Copy link
Author

Choose a reason for hiding this comment

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

Adding www might add another host; I expect the user to provide all the hosts they want to test as an input, and avoid adding hostnames implicitly (without the user actually knowing about it).
I agree, subdomain enumeration is out of scope for this tool.

src/findcdn/cdnEngine/detectCDN/cdn_check.py Show resolved Hide resolved
src/findcdn/cdnEngine/detectCDN/cdn_check.py Show resolved Hide resolved
@kz0ltan
Copy link
Author

kz0ltan commented Dec 3, 2023

I'm not sure what the type-of-change labels mean in the checklist, could you point me to an explanation?

Should I wait for this instead of messing with this pull request?

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.

2 participants