-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add masscan support #92
base: master
Are you sure you want to change the base?
Conversation
Thanks! Great job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, the previous note was out-of-scope of this PR. I removed this message and will fix it later, separately from these changes. Thanks for attention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes are required. At the moment, the module does not save data and crashes at the end of the scan. Please, pay attention to the comments and tracebacks that I attached.
Python environment:
Python 3.7.7 (default, Mar 10 2020, 15:43:33)
[Clang 11.0.0 (clang-1100.0.33.17)] on darwin
System:
MacOS Catalina 10.15.4
2d712be
to
ecce702
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes are required. We need more accurate testing.
- What happens if the network connection disappears during scanning because of overload?
- What happens if the network connection disappears between ranges? For example, we have 5 ranges for scanning, and internet connection disappears after second one? What happens with the last 3 ranges?
- What happens in case if we have a VPN connector enabled?
- What happens if masscan returns 0 results?
Also, please update the |
??? |
Any news? @svkirillov |
6f871b8
to
9bac373
Compare
@manmolecular need review |
Sorry, that is not all review! Github just finishes my review too early for some reason, idk why. I will continue looking and testing it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little fixes required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some bugs are still present.
Is it okay that we use the key port
for multiple ports? It's better to turn:
{
...
"port": "80"
...
}
Into the:
{
...
"ports": ["80"]
...
}
Cause with the masscan module and based on all the features that you implement this transformation looks logical. And moreover, different logical parts like counters and so on now works strange.
BUT, this is it, and obviously we don't have time for this ⏰, so it seems that I need to fix it later somehow.
So, thanks for the job, but some changes are still required (just to support at least the basic logic).
:param host: ip of the host to scan | ||
:param rate: packet rate argument for Masscan | ||
:param arguments: arguments for Masscan | ||
:param ports: ports to scan with Masscan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you check the unique entities' counter? Run scan with the flag -cu
to count unique entities, and you will see:
{
"80": 18,
"443": 10,
"3389": 9,
"22": 4,
"111": 4,
"25": 3,
"21": 3,
"139": 3,
"53": 3,
"443,22": 2,
"80,443": 2,
"80,111": 2,
"110": 2,
"1947,2040": 1,
"23": 1,
"443,1034,21,1088": 1,
"443,53,139": 1,
"25,443": 1,
"636": 1,
"3389,80": 1,
"111,80": 1,
"110,80": 1,
"21,443": 1,
"80,1935": 1,
"3370,1947": 1,
"3306,22": 1,
"3306,873": 1,
"465": 1,
"111,443": 1,
"80,139": 1,
"80,143,995": 1,
"2222,53,22": 1,
"22,110,995,25,143": 1,
}
Something like "443,1034,21,1088"
is not okay, we need to separate ports.
else: | ||
self.masscan.scan(hosts=host, arguments="", sudo=sudo) | ||
|
||
self.results = {host: self.masscan[host] for host in self.masscan.all_hosts} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you check the country.json
file? Run scan with the flag -cu
and you will see:
{
"": 103
}
Because masscan scan results don't have any country-related data or latitude and longitude. We need to skip empty data from counting in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same for the organization.json
, proto.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I don't even talk about this case. Did you check it?
By default, we put something like (0, 0) coordinates in case if the host doesn't have any coordinates, but, these cases are really rare (because Shodan/Censys usually provide us information about location). So, for 5-10 hosts in 1.000 it is okay to put (0, 0) coordinates, but if you have 1.000 hosts and all of them don't have coordinates it's not the good idea to put them on the map (imho).
How r u doin'? What about fixes? Will it take too long to fix it or maybe you need any help? Let me know. |
Add initial masscan support