Task 4: Network Topology Discovery

[remyluslosius]

Summary

• Implements comprehensive network topology discovery for hosts
• Discovers interfaces, routing, DNS, NTP, services, and connectivity
• Provides network security assessment and topology mapping capabilities

Features

• Network Interfaces: Discovers all interfaces with IP addresses, states, TX/RX statistics, link speed/duplex
• Routing Analysis: Extracts IPv4/IPv6 routing tables with gateways, metrics, and interface mappings
• DNS Configuration: Analyzes resolver configuration, systemd-resolved status, and resolution testing
• NTP Discovery: Supports chrony, ntp, systemd-timesyncd with server configuration and sync status
• Network Services: Identifies listening ports, protocols, and system service status
• Connectivity Testing: Performs ping tests and HTTPS connectivity to common destinations
• Security Assessment: Evaluates network hardening, firewall status, and risky port identification

Advanced Capabilities

• Network Topology Mapping: Generates visual network maps with connectivity matrices
• Security Scoring: Provides 0.0-1.0 security scores with vulnerability identification
• Multi-Tool Support: Uses modern (ip, ss) and legacy (ifconfig, netstat) tools for compatibility
• Comprehensive Parsing: Handles complex network command outputs across distributions

API Endpoints

• POST /api/host-network-discovery/hosts/{host_id}/network-discovery - Full network discovery
• POST /api/host-network-discovery/bulk-network-discovery - Bulk discovery (max 10 hosts)
• GET /api/host-network-discovery/hosts/{host_id}/network-security-assessment - Security evaluation
• POST /api/host-network-discovery/network-topology-map - Topology mapping
• GET /api/host-network-discovery/network-discovery-capabilities - Feature reference

Network Security Assessment

• Risk Evaluation: Identifies risky ports (FTP, Telnet, RDP, database ports)
• Firewall Detection: Checks iptables, ufw, firewalld status
• Hardening Analysis: Evaluates TCP SYN cookies, ICMP redirects, IP forwarding
• Vulnerability Reporting: Provides actionable security recommendations
• Scoring Algorithm: Comprehensive scoring based on security posture

Topology Mapping Features

• Multi-Host Analysis: Processes up to 20 hosts for topology generation
• Network Segmentation: Identifies network segments and VLAN configurations
• Connectivity Matrix: Maps inter-host connectivity and routing paths
• Visual Data: Provides structured data for network visualization tools
• Summary Statistics: Aggregates network health and connectivity metrics

Test Plan

• Test individual host network discovery across distributions
• Verify bulk discovery with rate limiting (10 host max)
• Test network security assessment accuracy
• Validate topology mapping with multiple hosts
• Confirm support for both modern and legacy network tools
• Test connectivity assessment and ping statistics
• Verify DNS and NTP configuration detection

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate failed

Failed conditions
2 Security Hotspots
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

To fix the issue, we need to remove capabilities from the import statement on line 22 in backend/app/main.py. The other imported modules in the list should remain unless also flagged as unused. This change is safe as long as capabilities is not required elsewhere in the codebase for side effects (e.g., automatic route registration), which has not been indicated in the snippet.

Specifically, edit the import statement so that capabilities is omitted.
Only change line 22 of backend/app/main.py. No other changes or additions are required.

To fix the problem, we must match the check_permission function signature by passing all required arguments. Specifically, we need to add the db argument to the function call at line 82. The db variable is available in the local context because it is a parameter of the endpoint function, so we can simply add it as the third argument. No additional imports or definitions are needed since everything required is already present within the provided code.

• Change file: backend/app/routes/host_network_discovery.py
• Locate the call to check_permission at line 82.
• Update it to include the db argument: check_permission(current_user, "hosts:read", db).

---

To fix this issue, we must sanitize the host_id value before including it in log messages. The recommended approach for plain text logs is to remove newline characters (\r\n, \n, and optionally \r) from the user input. We can do this by creating a sanitized version of host_id (e.g. sanitized_host_id = host_id.replace('\r', '').replace('\n', '')) and using this sanitized value in the logger call on line 116. This fix should be applied only at the point where the user-supplied value is included in the logger call; elsewhere, retain the original value for functional purposes. If no utility function exists, perform sanitization inline before logging.

Edit only lines around the logger call in backend/app/routes/host_network_discovery.py (specifically, the exception handling block in discover_host_network_topology). No external dependencies are needed.

---

To resolve the error, identify what the third required argument for check_permission should be. In most FastAPI + SQLAlchemy setups, permission checks typically need the current database session, which is db in this function. Therefore, the best fix is to add db as the third argument to the call check_permission(current_user, "hosts:read"), making it check_permission(current_user, "hosts:read", db). This should be changed directly at the highlighted line (139) in the bulk_discover_network_topology route.

No new methods or definitions are needed, assuming that this pattern matches other usages and that db is the expected argument.

---

To fix the log injection vulnerability, any untrusted user input written to logs should be sanitized to prevent special characters (especially newline and carriage return characters) from affecting the integrity of log entries. Specifically, before logging host_id on line 191, ensure that any carriage returns (\r) and line feeds (\n) are removed or replaced. This can be accomplished by adding a helper function to sanitize input, or inline using replace.

In this case, best practice is to define a small utility function (e.g., sanitize_for_log) within the file to sanitize any string going into logs. Then, before logging host_id, call this function.

Changes needed:

• Add a helper function such as sanitize_for_log(value) that removes/escapes problematic characters.
• Use this function on host_id before passing it to any log call (e.g., line 191).
• Do NOT change any logic or functionality of the route or its error handling.

---

To fix this problem, we must update the call to check_permission on line 223 to supply the required third parameter. We must determine, from context or convention, what the missing third argument represents. Based on typical RBAC function parameterization, it's likely to be something like a database session, request object, or similar contextual information—very commonly the database session (db), which is available in this function's parameters. Since db is provided by dependency injection and is present in nearby code, and because check_permission is often expected to have access to the database/session to check permissions, supplying db as the third parameter is the safest and cleanest solution. Only the call on line 223 needs editing; the rest of the code does not require modification.

To fix the log injection vulnerability, sanitize the user-provided host_id before interpolating it into log messages. Specifically, remove all newline (\n, \r) characters from host_id when logging. This can be done with host_id.replace('\n', '').replace('\r', ''). The fix should only apply to the value used in logging to avoid interfering with the actual logic where the original value may be needed. The affected region is line 252 in the error logging within the exception handler in the assess_host_network_security route function in backend/app/routes/host_network_discovery.py.

No import is needed; simply sanitize inline before logging.

---

To fix the problem, we need to supply the missing third argument to the check_permission function call at line 275. Since the function call is protecting bulk network topology mapping actions for multiple hosts, the most sensible resource argument to pass is either the list of host IDs or the request object containing them. The function’s likely usage pattern is: check_permission(user, permission, resource). The resource here would be the specific hosts being targeted. As the request object on this endpoint contains host_ids, it is appropriate to pass either request.host_ids or the whole request object, depending on convention elsewhere in the codebase. To minimize risk and maximize correctness, we will pass request.host_ids as the third argument, giving the access check direct visibility of the relevant resources.

No other significant code changes, imports, or external dependencies are required.

The fix is to ensure the call to check_permission on line 317 supplies the correct number of arguments—as per its definition (at least three). Since the other call on line 275 passes the arguments current_user, "hosts:read" (possibly missing the third argument too), but the context here is the /network-discovery-capabilities route and probably needs the database session as the third parameter. Given the context of FastAPI dependency injection, it is likely missing the db argument. To fix, modify the route function to accept db: Session = Depends(get_db) and then pass db as the third argument to check_permission. Edit lines 307–317 to introduce the db argument and update the call accordingly.

---

The best way to fix this problem is to remove the unused import statement for json from the file backend/app/services/host_network_discovery.py. Specifically, delete the line import json on line 6. No other changes are necessary, as this import is not used elsewhere in the visible code. Removing this will clean up the file and slightly reduce unnecessary dependencies, improving readability and avoiding confusion.

---