Task 2: Security Infrastructure Discovery

[remyluslosius]

Summary

• Implements SSH-based security infrastructure discovery for hosts
• Detects package managers, service managers, SELinux, AppArmor, and firewall services
• Provides both individual and bulk discovery capabilities with comprehensive error handling

Features

• Package Manager Detection: Discovers dnf, yum, apt, zypper, pacman, pkg, apk with version info
• Service Manager Identification: Detects systemd vs traditional init systems
• SELinux Status: Checks enforcement mode, policy type, and configuration
• AppArmor Profiles: Monitors profile counts and enforcement status
• Firewall Services: Identifies active firewall services and rule counts
• Security Tools Summary: Compiles comprehensive list of detected security tools

API Endpoints

• POST /api/host-security-discovery/hosts/{host_id}/security-discovery - Individual discovery
• POST /api/host-security-discovery/bulk-security-discovery - Bulk discovery (max 50 hosts)
• GET /api/host-security-discovery/hosts/{host_id}/security-summary - Quick security summary

Implementation Details

• Uses SSH connectivity service for secure remote command execution
• Implements robust error handling and timeout management
• Provides detailed logging for troubleshooting and audit trails
• Returns structured JSON responses with discovery metadata

Test Plan

• Test individual host security discovery
• Verify bulk discovery operations
• Test error handling for unreachable hosts
• Validate security tool detection accuracy
• Confirm API authentication and authorization

🤖 Generated with Claude Code

[sonarqubecloud]

Quality Gate failed

Failed conditions
4 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

The check_permission function is being called on line 63 with only two arguments, but the function signature requires at least three (most likely: the user, the permission string, and a resource/context object). To fix this, the appropriate third argument that matches the permission check context should be provided. Based on standard RBAC patterns and the context of this route (acting on a particular host), the host object (which represents the resource to access) should be passed in as the third argument. The local variable host is already defined before line 63, after retrieving it from the database, so the fix is to pass host as the third argument.

Change line 63 in backend/app/routes/host_security_discovery.py from:

check_permission(current_user, "hosts:read")

to:

check_permission(current_user, "hosts:read", host)

This fix requires no further changes, as all arguments are now correctly supplied and the function's behaviour remains intact.

---

To fix this issue, all user-supplied data logged in this file (specifically host_id) must be sanitized before being written to the logs. The best approach here is to strip or remove any newline characters (\r, \n) and carriage returns, which can be used for log entry injection. This can be done using .replace('\n', '').replace('\r', '') on the relevant string(s).

Specifically, in backend/app/routes/host_security_discovery.py, line 97 is:

logger.error(f"Security discovery failed for host {host_id}: {str(e)}")

You should update it so that host_id is sanitized prior to use in the log entry:

• Just before line 97, assign a sanitized version:
  • sanitized_host_id = host_id.replace('\n', '').replace('\r', '')
  • Use this value in the log entry.

No additional methods or imports are required.

---

To fix the problem, we need to supply the correct number of arguments to the check_permission function on line 120 of backend/app/routes/host_security_discovery.py. Specifically, we must determine what the third required parameter to check_permission should be. Often, RBAC functions take as parameters: the user, the permission string, and some subject (such as an object, resource, or context) being checked.

Since this is an endpoint authorizing bulk discovery across multiple hosts (as given by the request object), the third argument could plausibly be the db session (for context), or even the Host object, or a list of resources. However, since we are at the bulk level, and both current_user and db are available, it is most likely that db is expected as the third parameter. We'll add db as the third argument to the function call unless the signature is known to require something else.

Edit the code at line 120 so the function call is: check_permission(current_user, "hosts:read", db). No import or method changes are required.

---

To fix this log injection vulnerability, any potentially user-controlled string (here, host_id) should be sanitized before being interpolated into a log entry. Specifically, newline characters should be removed or replaced, to prevent log forging attacks. The best way is to replace occurrences of both \r and \n in host_id with empty strings (effectively removing any newlines). This should be done immediately before logging. Only lines where host_id is logged as part of an error should be modified; regular UUID processing does not require further changes unless it too logs unsanitized values. No new dependencies are required; Python str.replace is sufficient.

Edit the logging statement on line 172 to use a sanitized version of host_id. Optionally, sanitize the logging on line 169 as well if desired (because it too includes user input in errors, though not as a log line but in the errors dict; it's not strictly a log write, but for completeness same technique can be applied).

---

To fix the problem, supply the required third argument to check_permission in the get_host_security_summary function. Since there is a pre-existing database session object named db, and RBAC permission checks very often require this parameter for database queries or context, it is logical to pass db as the third argument.

Edit the call at line 204 in backend/app/routes/host_security_discovery.py within the get_host_security_summary endpoint:

• Change check_permission(current_user, "hosts:read") to check_permission(current_user, "hosts:read", db).

No further changes, imports, or method definitions are necessary, as db is already being provided to the function as a dependency.

---

To remediate this, we should sanitize the host_id variable before including it in log entries. Specifically, we should strip out all line breaks (\n, \r) to prevent users from injecting new log entries. The recommended approach is to define a helper function inside the file (since we don’t see any external sanitizer imported) that removes problematic characters from user input. Whenever logging host_id, pass it through this sanitizer first.

Changes to make in backend/app/routes/host_security_discovery.py:

• Add a simple sanitizer function (e.g., sanitize_for_log) above the route definition.
• Update the logger.error line on 259 to use the sanitized version of host_id (do not log the raw user input).