[Threat Intel 4/11] SSRF-guarded HTTP client for feed fetching - #287200
Merged
stephmilovic merged 9 commits intoSep 2, 2026
Conversation
stephmilovic
added
release_note:skip
kibanamachine
added
the
reviewer:libra
Contributor
This was referenced Aug 25, 2026
[Threat Intel 1/11] Gate enrichment completion on extract_iocs, drop detection_actionability
#287197
Merged
stephmilovic
force-pushed
the
threat-intel-4a-ssrf-client
branch
from
August 26, 2026 16:22
94510d7 to
ed1e710
Compare
stephmilovic
marked this pull request as ready for review
August 26, 2026 16:22
|
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
kibanamachine
left a comment
kibanamachine
left a comment
Contributor
There was a problem hiding this comment.
Libra found 3 issues.
Generated by Libra
Contributor
Author
|
All three addressed, details in the threads. The DNS one was the most valuable: the bound looked like it was there and was not, because the signal was created after validation. Since the hostname is operator-supplied that was a real way to pin a task worker indefinitely. Also confirming the observation that |
kibanamachine
left a comment
kibanamachine
left a comment
Contributor
There was a problem hiding this comment.
Libra found 3 issues.
Generated by Libra
Contributor
Author
|
All three addressed, details in the threads. The Azure one is the finding I would most want a second opinion on, because it is the only address here that is not in an RFC special-use range: it looks like ordinary public space to every rule in the classifier, and only Azure knows it is special. If there are other cloud platform endpoints with that shape, they would have the same problem, and I would rather add them now than discover them one review at a time. I also probed the guard against the obfuscated host forms while I was in here, to check the fix composed rather than assuming: integer, hex, and octal IPv4, IPv4-mapped and IPv4-compatible IPv6, 6to4, NAT64, and a trailing-dot FQDN all normalize into something the classifier rejects, and a genuinely public IPv4-mapped address stays routable. The Azure /32 is pinned through both the literal and mapped paths for the same reason. |
kibanamachine
left a comment
kibanamachine
left a comment
Contributor
There was a problem hiding this comment.
Libra found 3 issues.
Generated by Libra
jonwalstedt
approved these changes
Aug 28, 2026
jonwalstedt
force-pushed
the
threat-intel-4a-ssrf-client
branch
from
August 28, 2026 19:59
1ffbdfc to
58324e6
Compare
Contributor
|
@stephmilovic — the TI MVP scope-narrowing handoff is complete. Summary for the whole stack (posting here on the lead PR so it's in one place): Rewrites force-pushed to all 8 fork branches (base stays
Commit preservation: I force-pushed with #287199 is closed (no surviving downstream dependency; the in-scope helpers were transplanted onto their owning PRs as above). Validation: integration branch merges all 8 clean; Docs: Project AlertZero docs updated in a draft PR — elastic/project-alertzero#163 (new decision D25, resolved approved-source questions, buildout/mvp-slice/mvp-spec/risk-register/changelog, regenerated docs-data.js + megadoc; the I left all PR descriptions untouched per your note — those are yours to update. |
Contributor
Author
|
@elasticmachine merge upstream |
stephmilovic
force-pushed
the
threat-intel-4a-ssrf-client
branch
from
September 1, 2026 20:06
4209fb4 to
db9cc7d
Compare
Fourth slice of the threat intel supply pipeline, and the security-critical one.
Every outbound feed fetch goes through this. Nothing calls it yet and
`threatIntelSupplyEnabled` defaults off, so it is inert.
Feed URLs are operator-supplied, so a source is a request for Kibana to fetch a URL
of someone else's choosing. The guard:
- Rejects any scheme other than http and https.
- Classifies literal hosts against the shared special-use ranges, which covers the
obfuscated IPv4 forms (integer, hex, octal) and the IPv4-mapped and
IPv4-compatible IPv6 forms that smuggle a restricted address through an IPv6
literal. The WHATWG URL parser normalizes most of these before we see them, and
the tests assert that.
- Rejects hostnames that always resolve somewhere local: single-label names, and
the `.localhost`, `.local`, `.internal`, and `.localdomain` suffixes. A trailing
dot is stripped first, since `metadata.google.internal.` is a valid absolute FQDN
that resolves identically and does not end with `.internal`.
- Resolves the hostname and re-checks every answer, so a public name pointing at a
private address is rejected before any connection.
- Pins the connection to the address it validated. Validating and then letting Node
resolve the name again at connect time leaves a DNS-rebinding window where the
second answer differs from the checked one, so the request goes through a
dispatcher whose lookup returns the pinned address. TLS SNI and the Host header
keep the original hostname, so certificate verification is unaffected.
- Re-runs all of that on every redirect hop, with its own pin per hop, and caps the
chain. An open redirect on a legitimate feed host is the usual way in.
- Strips `Authorization`, `x-api-key`, and `Cookie` on cross-origin hops.
- Caps the response body incrementally on the decoded stream, because
`Content-Length` is absent on chunked and gzipped responses.
Credentials embedded in a feed URL are moved into an `Authorization: Basic` header
and the request goes to the credential-free URL. Node's fetch rejects a URL
containing credentials outright, before it issues anything, so without this an
authenticated feed could be saved, validated, and scheduled, and then fail on every
run. The derived header goes through the same cross-origin stripping.
`assertSafeUrl` is deliberately not exported. On its own it cannot see where a DNS
name points, so a caller who reached for it directly would miss the
public-name-private-record case. `assertSafeUrlResolved` is the complete check.
The `fetchFn` and `lookupFn` test seams are reachable only through
`createFetchUrl`, so production callers cannot bypass the guard by accident.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…andling Three review findings on the SSRF client. The DNS pre-flight sat outside every bound this function has. `dns.lookup` takes no AbortSignal, and the combined timeout/cancellation signal was created after validation, so a resolver that never answers kept a feed run pending indefinitely past both the 30s timeout and a step abort. Hostnames are operator-supplied, so that is a task-worker exhaustion vector rather than a hypothetical. The signal is now established first and every pre-flight, initial and per-redirect, races it. Every 3xx was treated as a redirect, but fetch only follows 301, 302, 303, 307, and 308. A 304 answering a caller's conditional headers therefore threw "missing Location" instead of being returned to the adapter. The redirect body was released only after URL parsing, the hop-limit check, and the destination SSRF check, all of which can throw. `finally` then calls `Agent.close()`, which waits on in-flight requests, and an unread stream keeps the request in flight, so a server streaming an endless redirect body could hang the run instead of failing it. The body is now released immediately after the status is classified, before anything throwable. This is a real gap in the earlier close-ordering fix, which only moved the cancel ahead of `close()` on the success path. Tests cover all three: a resolver that never answers, a caller abort mid-lookup, a pre-aborted signal, a 304 round-trip, a followed 308, and body release on each of the three throwable redirect paths. The existing redirect tests used `body: null`, which is why none of this was exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…als, test the pin Three more review findings on the SSRF client. 168.63.129.16 was treated as ordinary public space. It is not in any RFC special-use range, so every rule in the classifier passed it, but on Azure it is the VM-scoped platform endpoint (WireServer / host plugin). Since feed URLs are operator-supplied and `FetchUrlOptions.headers` lets a caller add the `x-ms-*` headers it wants, that was a way through the SSRF boundary on any Azure deployment. The other cloud metadata endpoints sit in link-local 169.254.0.0/16 and were already covered. Added as a /32, with neighbours asserted routable so it cannot silently widen. Proxy-Authorization was missing from the cross-origin strip set. It is a standard credential header and the public `headers` option can carry it, so a feed host redirecting cross-origin could collect proxy credentials. The DNS-pinning test only asserted that a dispatcher object existed. It would have passed if the Agent re-resolved the hostname or pinned the wrong address, which is the entire guarantee, so the central anti-rebinding property was effectively untested. The connect lookup is now a named function exported as `pinnedLookupForTest`, so the pin is asserted directly instead of by reflecting into undici's private option storage: both call shapes, an IPv6 address, indifference to the hostname asked for, and the pre-flight-to-pin handoff under a resolver that returns a private address on its second answer. Plus per-hop dispatcher separation. Verified it bites: making the pin answer with a different address fails 4 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The /32 is matched in the IPv4 classifier, and IPv4-mapped IPv6 is normalized back through it, so both spellings are blocked. Pinning it so a future change to either path cannot open the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ed, fix header case Three review findings on the SSRF client, one of them a real bypass. `http://[::ffff:0:169.254.169.254]/` reached the metadata endpoint. The URL parser canonicalizes it to `::ffff:0:a9fe:a9fe`, and the classifier only matched `::ffff:<hi>:<lo>`, so the extra zero group made it look routable. Because a literal host is treated as already-validated, DNS validation is skipped, which makes a miss here a direct route to the embedded target rather than a defence-in-depth gap. Rather than add the one spelling, the embedded-IPv4 forms are now matched by shape: all leading groups zero, any intervening groups only the `ffff` marker or zeros, and the last two groups carrying the address. That covers mapped, translatable/SIIT, and IPv4-compatible in one rule. Enumerating spellings has now missed one, and the same generalization applies to the dotted matcher. `redactUrl`'s fallback runs precisely because the URL did not parse, so it must not assume valid userinfo syntax. `[^/@\s]*@` stopped at the first `/` and the first `@`, so `//user:sec/ret@host` was returned untouched and `//us@er:s3cr3t@host` kept its password, both reachable from `assertSafeUrl`'s error message. It over-redacts through the last `@` now: losing part of a malformed URL beats logging a credential. Caller precedence over the URL-derived credential only worked for the exact spelling `Authorization`. Header names are case-insensitive, so a caller passing `authorization` left both keys in the object, and since fetch normalizes names the stale URL credential could be sent alongside the intended one. Detection is case-insensitive now, with tests across four spellings asserting exactly one authorization header survives and the URL credential is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
stephmilovic
force-pushed
the
threat-intel-4a-ssrf-client
branch
from
September 1, 2026 22:10
db9cc7d to
5c215dc
Compare
stephmilovic
removed
the
Team: SecuritySolution
Estimate decoded size from the base64 payload length so fixture data URLs cannot allocate multi-megabyte buffers before the feed cap check runs. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
Test Failures
History
|
This was referenced Sep 4, 2026
stephmilovic
added a commit
that referenced
this pull request
Sep 4, 2026
…og API (#287204) ## Summary Adds LLM enrichment services and Threat Intelligence HTTP routes, including the fixed-catalog source list and enable/disable API. Manual report creation and provenance URLs go through the shared HTTP/HTTPS normalizer. Also wires the routes, inference features, and one-time bootstrap into `plugin.ts`, gated by `threatIntelSupplyEnabled`. This wiring is interim: it exists so the enrichment eval suite (see Evaluation below) has routes to call ahead of the full pipeline wiring. `#287207` replaces it with a proper `wiring.ts` module; that PR's description has a rebase note covering the `plugin.ts` conflict resolution. ## Where this sits | # | PR | Depends on | Status | |---|---|---|---| | 1 | #287197 workflow gate correction + fixtures | nothing | ✅ merged | | 2 | #287198 contracts, constants, shared libs | nothing | ✅ merged | | 3 | #287199 content parsing | #287198 | closed | | 4 | #287200 SSRF-guarded HTTP client | #287198 | ✅ merged | | 5 | #287201 index templates, seeding, inference features | #287198, #287200 | ✅ merged | | 6 | #287202 indicator alias | #287198, #287201 | ✅ merged | | 7 | #287203 IOC extraction | #287198, #287200 | ✅ merged | | 8 | #287204 LLM services, routes, source catalog API **← this PR** | #287198, #287200, #287202, #287203 | 👀 ready | | 9 | #287205 RSS adapter | #287198, #287200, #287203, #287204 | draft | | 10 | #287206 remaining adapters, dispatcher, fetch_source step | #287198, #287200, #287203, #287205 | draft | | 11 | #287207 promote/scrub tasks and plugin wiring | #287198–#287206 | draft | | 12 | #287479 generator fixture article URLs | nothing | ✅ merged | | 13 | #289343 Scout API tests for the deterministic routes | #287204 | draft | | 14 | #289345 enrichment eval suite | #287204 | draft | #287479 came first in the merge train and is already on `main`. The numbered series (#287197–#287207) stacks on top in dependency order. #287199 (content parsing) was closed; IOC extraction ships its own section-header classifier. ## Scope review follow-up - Shared `normalizeProvenanceUrl` helper (HTTP/HTTPS only, strips credentials, bounded length). - Source create/delete routes removed. Updates may change only `enabled`. - Lists and mutations reject catalog IDs outside the approved set. - Ensures the per-space indicator alias on `list_sources` (idempotent). ## Evaluation A `@kbn/evals` suite exercises all four enrichment stages against the BlackHat demo pack article text, posting each input directly to the internal route and scoring the structured response. It runs as a tracked baseline (scores recorded, no build-failing thresholds yet). Latest full run, EIS Claude Sonnet 4.6 as model and judge, 1 repetition, 20 examples: | Stage | Evaluator | Score | |---|---|---| | assess_relevance | IsIntelligenceMatch | 1.00 | | assess_relevance | RelevanceShapeValid | 1.00 | | classify_severity | SeverityExactMatch | 0.83 | | classify_severity | SeverityWithinOneLevel | 1.00 | | enrich_taxonomy | CategoryRecall | 0.80 | | enrich_taxonomy | RegionRecall | 1.00 | | extract_diamond | DiamondNoIocLeak | 1.00 | | extract_diamond | DiamondSignalCount | 1.00 | | extract_diamond | criteria (LLM judge, majority vote) | 1.00 | The suite ships in #289345, a separate eval-only PR that stacks on this one (it needs the routes and the flag-gated wiring added here). The four deterministic routes are covered separately by Scout API tests in #289343. Any prompt or calibration fix the evals surface lands by amending this PR, not the eval PR. ## To test ``` node scripts/jest --config x-pack/solutions/security/plugins/security_solution/server/threat_intel/jest.config.js routes/list_sources.test.ts services/provenance_url.test.ts ``` _PR developed with Cursor + Auto_ --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Jon Wålstedt <jon.walstedt@elastic.co> Co-authored-by: Cursor <cursoragent@cursor.com>
jonwalstedt
added a commit
that referenced
this pull request
Sep 11, 2026
…d the flag (#287207) ## Summary Wires Threat Intelligence behind `threatIntelSupplyEnabled`: bootstrap, managed workflow installation, promote/scrub tasks, and route registration. Promote mirrors extracted IOCs into `.threat-intel-indicators` for Indicator Match rules. ## Where this sits | # | PR | Depends on | Status | |---|---|---|---| | 1 | #287197 workflow gate correction + fixtures | nothing | ✅ merged | | 2 | #287198 contracts, constants, shared libs | nothing | ✅ merged | | 3 | #287199 content parsing | #287198 | closed | | 4 | #287200 SSRF-guarded HTTP client | #287198 | ✅ merged | | 5 | #287201 index templates, seeding, inference features | #287198, #287200 | ✅ merged | | 6 | #287202 indicator alias | #287198, #287201 | ✅ merged | | 7 | #287203 IOC extraction | #287198, #287200 | ✅ merged | | 8 | #287204 LLM services, routes, source catalog API | #287198, #287200, #287202, #287203 | ✅ merged | | 9 | #287205 RSS adapter | #287198, #287200, #287203, #287204 | ✅ merged | | 10 | #287206 remaining adapters, dispatcher, fetch_source step | #287198, #287200, #287203, #287205 | ✅ merged | | 11 | #287207 promote/scrub tasks and plugin wiring **← this PR** | #287198–#287206 | ready for review | | 12 | #287479 generator fixture article URLs | nothing | ✅ merged | | 13 | #289343 Scout API tests for the deterministic routes | #287204 | ready for review | | 14 | #289345 enrichment eval suite | #287204 | ready for review | | 15 | #290144 read APIs, readiness, space-keyed attribution | #287207 | draft | #287479 came first in the merge train and is already on `main`. The numbered series (#287197–#287205) is merged; #287206 and #287207 now sit directly on `main`. #287199 (content parsing) was closed; IOC extraction ships its own section-header classifier. ## Scope review follow-up - Installs the managed workflows with the right space topology: `attribute_alerts_to_reports` installs per space (it queries `.alerts-security.alerts-*` and writes per-space hit totals, so a global install would clobber every space's totals onto one shared doc), while `ingest_threat_feeds` and `enrich_threat_report` install once globally. Alert analysis and threat intel share a single `ready()` call so neither install set is dropped, and spaces created after boot are reconciled on the promote task. All three ship `enabled: false`, and enrich routes its HTTP calls through a fixed space (`default`) instead of the install-time `workflow.spaceId` (which is `'*'` globally). - Ensures the default-space indicator alias on start. - Chunks promotion bulk writes and treats HTTP 408/500 as retryable bulk failures. - Sanitizes provenance and extracted IOC reference URLs during promotion. - Documents direct-index cross-space isolation as the blocker to enabling the feature. ## Bootstrap fix folded in from #289343 `seed_default_sources.ts` sorted the legacy-source disable scan on `_id`, which Elasticsearch rejects with `illegal_argument_exception: Fielddata access on the _id field is disallowed` unless `indices.id_field_data.enabled` is set, so it failed bootstrap on a stock cluster. Found this while getting #289343's Scout suite green and moved the fix here since this PR is the one wiring up bootstrap and already needs `security-threat-hunting` review, so it isn't adding a reviewer #289343 wouldn't otherwise need. ## Product boundary `threatIntelSupplyEnabled` defaults to false. Do not enable until direct-index cross-space isolation is hardened or the administrator trust model is explicitly accepted. ## To test ``` node scripts/jest --config x-pack/solutions/security/plugins/security_solution/server/threat_intel/jest.config.js wiring.test.ts tasks/promote_threat_indicators.test.ts node scripts/jest x-pack/solutions/security/plugins/security_solution/server/workflows/ node scripts/jest src/platform/packages/shared/kbn-workflows/managed/definitions/threat_intel/ ``` _PR developed with Cursor + Auto + Sonnet 5 + Opus 4.8_ --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Jon Wålstedt <jon.walstedt@elastic.co> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the SSRF-guarded HTTP client used by Threat Intelligence feed adapters. Network fetches go through pinned DNS, blocked private ranges, and bounded redirects. Inline
data:URLs are supported only for local generator fixtures, with encoded and decoded size limits.Where this sits
#287479 came first in the merge train and is already on
main. The numbered series (#287197–#287207) stacks on top in dependency order. #287199 (content parsing) was closed; IOC extraction ships its own section-header classifier.Scope review follow-up
data:URLs in the inline fixture path.Product boundary
threatIntelSupplyEnabledstays off until direct-index cross-space isolation is hardened. This PR only adds the client.To test
PR developed with Cursor + Auto