NM-294: SIEM Integration

[VishalDalwadi]

Describe your changes

Provide Issue ticket number if applicable/not in title

Provide testing steps

Checklist before requesting a review

• My changes affect only 10 files or less.
• I have performed a self-review of my code and tested it.
• If it is a new feature, I have added thorough tests, my code is <= 1450 lines.
• If it is a bugfix, my code is <= 200 lines.
• My functions are <= 80 lines.
• I have had my code reviewed by a peer.
• My unit tests pass locally.
• Netmaker is awesome.

[tenki-reviewer]

Review Complete

Files Reviewed: 9
Findings: 5

By Severity:

• 🔴 High: 3
• 🟠 Medium: 2

---

This PR introduces a new SIEM integration subsystem (Splunk, Datadog, Elastic, Sentinel) with REST CRUD endpoints, but contains several security issues that must be addressed before merging: stored credentials are returned in plaintext via the GET API, TLS verification is disabled by default for two providers, and user-controlled fields are interpolated unsanitised into outbound URLs enabling SSRF and path traversal.

Files Reviewed (9 files)

pro/controllers/integrations.go
pro/initialize.go
pro/integration/providers.go
pro/integration/siem_datadog.go
pro/integration/siem_elastic.go
pro/integration/siem_sentinel.go
pro/integration/siem_splunk.go
schema/integration.go
schema/models.go

[tenki-reviewer]

Risk: 🟠 High (72/100) — 3 high findings, 2 medium · 606 LOC across 9 files

---

Overview

PR #4021 adds a complete SIEM integration feature: a provider registry (pro/integration/providers.go), four provider implementations (Splunk, Datadog, Elastic, Sentinel), REST handlers for CRUD + test operations (pro/controllers/integrations.go), a new schema.Integration GORM model, and wires everything into pro/initialize.go. The design is clean and well-structured, but all four provider files and the controller have security issues that would expose credentials and enable server-side request forgery.

Security Findings

🔴 Credential Exposure via GET (High)

The getIntegration and upsertIntegration handlers in pro/controllers/integrations.go return the complete schema.Integration struct, including the raw Config JSON blob. This blob contains secrets — api_key, hec_token, password, shared_key — in plaintext. Any admin-level caller can read back credentials after they are stored.

🔴 TLS Disabled by Default — Elastic & Splunk (High)

Both siem_elastic.go and siem_splunk.go use InsecureSkipVerify: !cfg.TLSVerify. Because TLSVerify is a Go bool field (default false), TLS certificate validation is off by default. Every integration that doesn't explicitly set tls_verify: true sends credentials over an unverified TLS channel, enabling MITM attacks.

🔴 SSRF via Unvalidated site Field — Datadog (High)

In siem_datadog.go, cfg.Site is interpolated directly into the outbound URL (https://http-intake.logs.%s/api/v2/logs) without any validation. An admin who sets site to evil.com/path?x= causes the server to forward requests — including the Datadog API key — to an arbitrary external host.

🟡 SSRF via Unvalidated workspace_id — Sentinel (Medium)

In siem_sentinel.go, cfg.WorkspaceID is used in https://%s.ods.opinsights.azure.com/... without format validation. A non-GUID value with embedded path segments or a different hostname can redirect the HMAC-signed request off-domain, leaking the SharedKey.

🟡 Path Traversal via index Field — Elastic (Medium)

In siem_elastic.go, cfg.Index is inserted verbatim into the URL path (%s/%s/_doc). An index name containing / or .. can traverse to arbitrary Elasticsearch APIs (e.g. _cluster/settings, _cat/indices), potentially disclosing cluster metadata or modifying settings.

Other Notes

• pro/initialize.go and schema/models.go changes are straightforward registration of the new feature — no issues.
• The provider Lookup registry and extractAndValidateIntegration helper are well-designed; the whitelist approach prevents unknown provider IDs from reaching the database.
• Error handling is consistent across all handlers.
• The siem_splunk.go HECEndpoint is the full URL (caller-controlled), which is also a potential SSRF vector for the same reason as site in Datadog — consider validating it parses to an https:// scheme URL.

[tenki-reviewer]

🔴 Credentials returned in plaintext via GET integration endpoint (security)

The getIntegration handler (line 69) returns the full schema.Integration struct to the caller, including the Config field stored as raw JSON. This field contains provider secrets such as api_key (Datadog, Elastic), hec_token (Splunk), password (Elastic), and shared_key (Sentinel). Any admin-level API caller can retrieve these credentials in full after they have been saved, violating the principle of write-only secret storage. The upsertIntegration handler at line 117 has the same issue — it echoes back the full config including the submitted secret.

💡 Suggestion: Scrub sensitive fields before returning the integration object. Define a redacted view of the config: unmarshal intg.Config into a map[string]interface{}, replace known secret keys (api_key, hec_token, password, shared_key) with "***", re-marshal, and assign back before calling ReturnSuccessResponseWithJson. Apply the same redaction to the upsert response at line 117.

📋 Prompt for AI Agents

In pro/controllers/integrations.go, the getIntegration handler (line 69) and upsertIntegration handler (line 117) both return the full integration config including secrets (api_key, hec_token, password, shared_key). Before calling logic.ReturnSuccessResponseWithJson, add a helper function redactIntegration(intg *schema.Integration) *schema.Integration that: (1) unmarshals intg.Config into map[string]interface{}, (2) replaces the values of keys 'api_key', 'hec_token', 'password', 'shared_key' with '***' if present, (3) re-marshals and stores back into a copy of the integration struct, (4) returns the sanitised copy. Call this helper on both line 69 and line 117 before passing to ReturnSuccessResponseWithJson. This prevents credential exposure over the API.

[tenki-reviewer]

🔴 TLS certificate verification disabled by default for Elastic and Splunk providers (security)

Both siem_elastic.go (line 55) and siem_splunk.go (line 59) construct their HTTP client with InsecureSkipVerify: !cfg.TLSVerify. Because TLSVerify is a Go bool field, its JSON zero-value is false, so any config that omits tls_verify will have TLS verification turned off. An attacker with network access between Netmaker and the configured Elastic/Splunk endpoint can intercept the connection (MITM), obtaining the API key or HEC token in plaintext along with any log data that is sent. Administrators who expect TLS to be validated by default are silently exposed.

💡 Suggestion: Invert the field semantics to make TLS verification the safe default. Rename TLSVerify bool to TLSSkipVerify bool (JSON: tls_skip_verify) in both ElasticConfig and SplunkConfig, and change the TLS config line to InsecureSkipVerify: cfg.TLSSkipVerify. With this change, the zero value (false) means TLS is verified, and only explicit opt-in sets it to skip.

📋 Prompt for AI Agents

In pro/integration/siem_elastic.go line 55 and pro/integration/siem_splunk.go line 59, the TLS config uses InsecureSkipVerify: !cfg.TLSVerify. Because TLSVerify is a Go bool that defaults to false (JSON zero value), this disables TLS verification for all configs that omit the field. Fix: (1) In ElasticConfig (siem_elastic.go line 18) rename TLSVerify bool to TLSSkipVerify bool with json tag tls_skip_verify. (2) In SplunkConfig (siem_splunk.go line 18) make the same rename. (3) Change both TLS config lines from InsecureSkipVerify: !cfg.TLSVerify to InsecureSkipVerify: cfg.TLSSkipVerify. This makes TLS verification on by default (the safe default), and only disabled when the caller explicitly passes tls_skip_verify: true.

[tenki-reviewer]

🔴 Unvalidated site field in Datadog provider enables SSRF (security)

In siem_datadog.go line 54, cfg.Site is interpolated directly into https://http-intake.logs.%s/api/v2/logs via fmt.Sprintf. The Validate() method does not validate this field at all. An admin user can supply a crafted value such as evil.com/path?x= to redirect the outbound HTTP request — including the Datadog API key in the DD-API-KEY header — to an arbitrary attacker-controlled host. This makes the Netmaker server act as a proxy that exfiltrates the stored API key.

💡 Suggestion: In the Validate() method, add an allowlist check for cfg.Site. Datadog publishes a fixed set of valid site values: datadoghq.com, datadoghq.eu, us3.datadoghq.com, us5.datadoghq.com, ap1.datadoghq.com, ddog-gov.com. Reject any value not in this list. If extensibility is needed, at minimum verify the value contains no /, ?, #, :, or whitespace.

📋 Prompt for AI Agents

In pro/integration/siem_datadog.go, the Validate method (lines 21-31) does not validate the site field, which is later interpolated into a URL at line 54. Add validation in Validate(): define a set of allowed Datadog site values (datadoghq.com, datadoghq.eu, us3.datadoghq.com, us5.datadoghq.com, ap1.datadoghq.com, ddog-gov.com). If cfg.Site is non-empty and not in the allowlist, return fmt.Errorf("site %q is not a recognised Datadog site", cfg.Site). This prevents SSRF via a crafted site value redirecting outbound requests (including the API key) to an attacker-controlled host.

[tenki-reviewer]

🟠 Unvalidated workspace_id enables SSRF in Sentinel provider (security)

In siem_sentinel.go line 70, cfg.WorkspaceID is inserted directly into https://%s.ods.opinsights.azure.com/api/logs?api-version=2016-04-01. The Validate() function only checks that it is non-empty and that SharedKey is valid base64. It does not enforce that WorkspaceID is a UUID. A crafted value such as x.evil.com/path# would cause the server to send an HMAC-signed request to an attacker-controlled host, effectively leaking the SharedKey (because the attacker receives the signed string and can derive it via brute-force or reuse).

💡 Suggestion: In Validate(), enforce that cfg.WorkspaceID matches the UUID format using regexp.MustCompile("^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$"). Return an error for any non-conforming value.

📋 Prompt for AI Agents

In pro/integration/siem_sentinel.go, the Validate method (lines 22-39) does not validate the format of WorkspaceID, which is later interpolated into the Azure URL at line 70. Add a UUID format check after the non-empty check: compile a regex ^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$ (use a package-level var to avoid recompiling), and return fmt.Errorf("workspace_id must be a valid UUID") if the value does not match. Import the regexp package. This prevents SSRF via a crafted workspace ID that redirects the HMAC-signed request off-domain.

[tenki-reviewer]

🟠 Unvalidated index field allows path traversal in Elasticsearch URL (security)

In siem_elastic.go line 59, cfg.Index is inserted verbatim into fmt.Sprintf("%s/%s/_doc", cfg.Endpoint, cfg.Index). The Validate() function only checks it is non-empty. An index name such as ../../../_cluster/settings or myindex/_bulk?pretty=true&x= would cause the request to be sent to an unintended Elasticsearch API endpoint. This could expose cluster-level information or, depending on Elasticsearch permissions, modify cluster settings.

💡 Suggestion: In Validate(), after the non-empty check, validate cfg.Index against the character set allowed by Elasticsearch index names: ^[a-z0-9._-]+$ (Elasticsearch requires lowercase). Return an error if the value contains /, .., ?, #, or uppercase characters.

📋 Prompt for AI Agents

In pro/integration/siem_elastic.go, the Validate method (lines 23-39) checks that cfg.Index is non-empty but not its content. The index is used verbatim in the URL path at line 59. Add a character allowlist check after the non-empty check: use regexp.MustCompile("^[a-z0-9._-]+$") (Elasticsearch index names must be lowercase). Return fmt.Errorf("index contains invalid characters; use only lowercase letters, digits, dots, hyphens, underscores") if the check fails. Import the regexp package. This prevents path traversal into arbitrary Elasticsearch API endpoints.