noursaidi commented 2 years ago

This PR is still WIP but close to completion - submitting PR for any comments. Outstanding is just implementing CI tests and and documentation. I'm also invesitgating if the HTTP test can be merged into this the services detection to save running two 65k port NMAP scans.

This PR maintains the current NMAP functionality and extends it by also check for disallowed services on all ports (test document here). in summary:

runs a nmap scan against all ports with service detection option (-sV) for TCP ports
runs an nmap scan against ports 69,161,162 for UDP and any other UDP port in module_config.json
For each TCP services(pop, telnet, ftp, imap, smtp), check if the service has been found in the NMAP scan or it's default port was open
For UDP services (tftp, SNMP), check if the default port was open
Output results for each test item (security.service.X)

The existing nmap scan has been retained for now (expected to be used within ATA) and is selected with a "services_scan": false

codecov[bot] commented 2 years ago

Codecov Report

Merging #935 (b282a08) into master (e9e851a) will increase coverage by 0.17%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #935      +/-   ##
==========================================
+ Coverage   82.56%   82.73%   +0.17%     
==========================================
  Files          46       46              
  Lines        5862     5862              
==========================================
+ Hits         4840     4850      +10     
+ Misses       1022     1012      -10

Flag	Coverage Δ
ata	`63.90% <ø> (+1.26%)`	:arrow_up:
aux	`68.24% <ø> (ø)`
base	`66.51% <ø> (-0.02%)`	:arrow_down:
dhcp	`67.50% <ø> (ø)`
many	`67.20% <ø> (-0.35%)`	:arrow_down:
mud	`72.12% <ø> (-0.07%)`	:arrow_down:
switch	`67.77% <ø> (-0.02%)`	:arrow_down:
topo	`66.45% <ø> (-0.06%)`	:arrow_down:
unit	`32.48% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
daq/acl_state_collector.py	`82.19% <0.00%> (-1.37%)`	:arrow_down:
daq/host.py	`91.35% <0.00%> (+0.28%)`	:arrow_up:
daq/runner.py	`85.86% <0.00%> (+0.31%)`	:arrow_up:
daq/tcpdump_helper.py	`80.95% <0.00%> (+2.38%)`	:arrow_up:
daq/stream_monitor.py	`90.08% <0.00%> (+2.47%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e9e851a...b282a08. Read the comment docs.

noursaidi commented 2 years ago

@grafnu @pisuke do you have any comments or questions on this PR? Except for CI fixes which I'm working my way though (test_mud may cause me some issues with unexpected long scan duration but I'm diagnosing that one - I haven't encountered very long NMAP module durations, usually <10 mins on my devices and much shorter on faux devices, but what I propose to deal with long scans if it is an issue:

splitting out the nmap port scan which is usually quick; and nmap service scan which takes longer depending on the number of ports found open;
setting a timeout for the services detection which is a function of the configured module expiry (say half of it)
if the service detection nmap scan times out, then set the security.services.X tests to skip or gone,
Still give a result for the security.nmap.ports scan) so the module results are still useful

noursaidi commented 2 years ago

Comments addressed - CI tests should be functional

faucetsdn / daq

NMAP test upgrade - service detection #935

Codecov Report