Repository navigation
[stormshield] Add ECS event categorization and network mappings - #20031
Conversation
|
💚 CLA has been signed |
✅ Elastic Docs Style Checker (Vale)No issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
🚀 Benchmarks reportTo see the full report comment with |
andrewkroh
added
documentation
a0275dc to
59232a6
Compare
andrewkroh
removed
the
needs CLA
11084e6 to
d5a60c4
Compare
arenard
changed the title
d5a60c4 to
069f288
Compare
069f288 to
a1623ea
Compare
andrewkroh
added
the
Team:Integration-Experience
|
Pinging @elastic/integration-experience (Team:Integration-Experience) |
![vera-review-bot[bot]](https://avatars.githubusercontent.com/in/3297022?s=60&v=4){kind=small}
a1623ea to
4ce4236
Compare
4ce4236 to
ccd7280
Compare
| def logtype = ctx.stormshield?.logtype; | ||
| def entry = params.logtypes[logtype]; | ||
| if (entry == null) { | ||
| entry = ['kind': 'event', 'category': ['network'], 'type': ['info']]; |
There was a problem hiding this comment.
Severity: 🔵 Low confidence: low path: packages/stormshield/data_stream/log/elasticsearch/ingest_pipeline/default.yml:703
The categorization fallback forces event.category: [network] for any unlisted log family, so a non-network family (e.g. the futuretype fixture) is labelled a network event; consider a neutral fallback that omits category instead.
Details
When logtype is absent from the logtypes map, the script assigns kind: event, category: [network], type: [info]. The test-log-families.log futuretype sample confirms this happens today: an unknown family is emitted with event.category: [network]. Because event.category drives detection-rule scoping and the network dashboards, any future/unmapped family (which could be host-, configuration-, or authentication-oriented) is silently pulled into the network bucket. event.kind: event is a safe default; the questionable part is asserting a concrete category the event may not belong to.
Recommendation:
Drop the forced category (and type) from the unknown-family fallback so unmapped families are left uncategorized rather than mislabelled as network:
def logtype = ctx.stormshield?.logtype;
def entry = params.logtypes[logtype];
if (ctx.event == null) {
ctx.event = [:];
}
if (entry == null) {
// Unknown family: mark as a generic event but do not assert a category.
ctx.event.kind = 'event';
} else {
ctx.event.kind = entry.kind;
ctx.event.category = new ArrayList(entry.category);
ctx.event.type = new ArrayList(entry.type);
if (params.action_logtypes.contains(logtype) && ctx.event.action instanceof String) {
def mapped = params.action_types[ctx.event.action.toLowerCase()];
if (mapped != null && !ctx.event.type.contains(mapped)) {
ctx.event.type.add(mapped);
}
}
}
If the network default is intentional for this firewall, no change is needed — the documentation already states it.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
The network default is intentional and documented in the README's categorization notes. All 26 families the vendor currently documents are explicitly categorized, so the fallback only guards families introduced by future firmware.
On a network appliance, network/info is the least-wrong assumption for those, with stormshield.logtype remaining the precise discriminator for scoping.
Happy to switch to a category-less fallback (event.kind: event only) if it is prefered.
There was a problem hiding this comment.
I suppose the argument for making the fallback category-less is that if there is a new family added in the future, it should be:
- Hopefully obvious that something is amiss since we don't have categorizations on the event
- "Easier" to make a change to set the correct category, versus having to make a "breaking" change when we change the categorization from network to something else. The concern here is a user starts relying on "network" being present before we catch the incorrect labeling, then we break their workflow.
I think for the second reason, I'm leaning towards leaving unknown events uncategorized.
There was a problem hiding this comment.
Agreed, your forward-compatibility argument settles it. Switched to a category-less fallback: unknown families now get event.kind: event only, with no event.category/event.type asserted. The futuretype test sample and the README note are updated accordingly.
ccd7280 to
6a2d7fa
Compare
|
No issues across the latest commits 7109bf4, 6a2d7fa. Review summaryIssues found across earlier commits 3ccf9e6…ccd7280 (27 commits) — 1 low
Issues found across earlier commits 4ce4236 — 2 mediumIssues found across earlier commits a1623ea — 3 medium
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
|
taylor-swanson
left a comment
taylor-swanson
left a comment
There was a problem hiding this comment.
Overall I think the changes look great. I put my two cents about the fallback for category-less down below. Otherwise, just a couple of minor changes to some if statements.
| def logtype = ctx.stormshield?.logtype; | ||
| def entry = params.logtypes[logtype]; | ||
| if (entry == null) { | ||
| entry = ['kind': 'event', 'category': ['network'], 'type': ['info']]; |
There was a problem hiding this comment.
I suppose the argument for making the fallback category-less is that if there is a new family added in the future, it should be:
- Hopefully obvious that something is amiss since we don't have categorizations on the event
- "Easier" to make a change to set the correct category, versus having to make a "breaking" change when we change the categorization from network to something else. The concern here is a user starts relying on "network" being present before we catch the incorrect labeling, then we break their workflow.
I think for the second reason, I'm leaning towards leaving unknown events uncategorized.
6a2d7fa to
f2dbfd6
Compare
Set event.kind, event.category and event.type for every SNS log family through a table-driven Painless script keyed on logtype, with allowed/denied event.type modifiers derived case-insensitively from the rule action. The periodic statistics families (monitor, filterstat, count, routerstat, authstat, ipsecstat) are tagged event.kind: metric so health telemetry can be excluded from detection scopes with a single filter. Unknown families fall back to event.kind: event without asserting a category or type, covered by a dedicated test sample. The script only runs on documents without an error, so the pipeline_error kind set by sub-pipeline failure handlers is preserved. Add the ECS network mappings tracked in elastic#10114: rcvd to destination.bytes (mirroring the existing sent mapping), network.bytes as the sum of both, srcif/srcifname/dstif/dstifname copied to observer.ingress and observer.egress interface fields, and network.direction computed with private ranges as internal networks. Promote alarm fields (alarmid to event.code, risk to event.risk_score, pri to event.severity on the alarm scale where 1 is major and 4 is minor), HTTP proxy fields (arg to url.original, url.domain from destination.domain), and populate related.hosts. All mappings are copies; no stormshield.* field moves or changes location. Guard the ASN geoip processors with null checks, matching the geo processors. Fix the count sub-pipeline, which failed on every count log since creation: the script called splitByToken, which does not exist in Painless, and collapsed all rules into a single map. Rule counters are now a sorted array under stormshield.metadata.rule_stats. Extend the metadata move list with the documented alarm, proxy, vpn and auth fields so they land under stormshield.metadata instead of being left unmapped. Add pipeline test samples for the alarm, auth, web, vpn, xvpn, count and filter families, derived from the log format examples in the official Stormshield documentation with sanitized addresses, including uppercase and empty action values. Add an Alarms (IPS) dashboard keyed on the new categorization fields. Document the categorization table in the README.
f2dbfd6 to
5098fd8
Compare
|
✅ All changelog entries have the correct PR link. |
💚 Build Succeeded
History
|
|
Package stormshield - 1.6.0 containing this change is available at https://epr.elastic.co/package/stormshield/1.6.0/ |
…tic#20031) Set event.kind, event.category and event.type for every SNS log family through a table-driven Painless script keyed on logtype, with allowed/denied event.type modifiers derived case-insensitively from the rule action. The periodic statistics families (monitor, filterstat, count, routerstat, authstat, ipsecstat) are tagged event.kind: metric so health telemetry can be excluded from detection scopes with a single filter. Unknown families fall back to event.kind: event without asserting a category or type, covered by a dedicated test sample. The script only runs on documents without an error, so the pipeline_error kind set by sub-pipeline failure handlers is preserved. Add the ECS network mappings tracked in elastic#10114: rcvd to destination.bytes (mirroring the existing sent mapping), network.bytes as the sum of both, srcif/srcifname/dstif/dstifname copied to observer.ingress and observer.egress interface fields, and network.direction computed with private ranges as internal networks. Promote alarm fields (alarmid to event.code, risk to event.risk_score, pri to event.severity on the alarm scale where 1 is major and 4 is minor), HTTP proxy fields (arg to url.original, url.domain from destination.domain), and populate related.hosts. All mappings are copies; no stormshield.* field moves or changes location. Guard the ASN geoip processors with null checks, matching the geo processors. Fix the count sub-pipeline, which failed on every count log since creation: the script called splitByToken, which does not exist in Painless, and collapsed all rules into a single map. Rule counters are now a sorted array under stormshield.metadata.rule_stats. Extend the metadata move list with the documented alarm, proxy, vpn and auth fields so they land under stormshield.metadata instead of being left unmapped. Add pipeline test samples for the alarm, auth, web, vpn, xvpn, count and filter families, derived from the log format examples in the official Stormshield documentation with sanitized addresses, including uppercase and empty action values. Add an Alarms (IPS) dashboard keyed on the new categorization fields. Document the categorization table in the README.
Proposed commit message
Set
event.kind,event.categoryandevent.typefor every SNS log family through a table-driven Painless script keyed onlogtype, withallowed/deniedevent.typemodifiers derived case-insensitively from the rule action (pass/block; an empty action corresponds to a rule set to Log and adds no modifier). The periodic statistics families (monitor,filterstat,count,routerstat,authstat,ipsecstat) are taggedevent.kind: metricso health telemetry can be excluded from detection scopes with a single filter. Unknown families fall back toevent.kind: eventwithout asserting a category or type, covered by a dedicated test sample. The script only runs on documents without an error, so thepipeline_errorkind set by sub-pipeline failure handlers is preserved.Add the ECS network mappings tracked in #10114:
rcvdtodestination.bytes(mirroring the existingsentmapping),network.bytesas the sum of both,srcif/srcifname/dstif/dstifnamecopied toobserver.ingress/observer.egressinterface fields, andnetwork.directioncomputed with private ranges as internal networks. Promote alarm fields (alarmidtoevent.code,risktoevent.risk_score,pritoevent.severityon the alarm scale where 1 is major and 4 is minor), HTTP proxy fields (argtourl.original,url.domainfromdestination.domain), and populaterelated.hosts. Guard the ASN geoip processors with null checks, matching the geo processors.Fix the count sub-pipeline, which failed on every count log since the package was created: the script called
splitByToken, which does not exist in Painless (runtime error:dynamic method [java.lang.String, splitByToken/2] not found), and collapsed all rules into a single map. Rule counters now form a sorted array understormshield.metadata.rule_stats, and a counter that does not parse (non-numeric value, out-of-range category or unexpected key shape) is skipped instead of sending the whole document topipeline_error.Add pipeline test samples for the alarm, auth, web, vpn, xvpn, count and filter families plus an unknown-family sample, an Alarms (IPS) dashboard keyed on the new categorization fields, and the categorization table in the README.
Design notes
stormshield.*field is renamed, moved or removed, andevent.actionkeeps the verbatim vendor value, so existing dashboards and saved queries are unaffected. This keeps the change a minor version bump.logdata stream is kept, following the fortinet_fortigate / cisco_asa / sonicwall_firewall precedent for multi-family syslog firewall packages; the categorization script follows the sonicwall_firewall table-driven pattern.event.outcomederivation and deeper per-family enrichment (vpn IKE fields, sandboxing verdicts, the v5 restapi family) are planned as follow-ups.Block), lowercase (pass) and empty action values, and the unknown-family fallback.Checklist
changelog.ymlfile.Author's Checklist
elastic-package check,test pipeline,test staticandtest system(udp, tcp, tls) all pass against stack 9.4.3data_stream.datasetandstormshield.logtypefilters, and every aggregated field is aggregatable and declared in the package field filessample_event.jsonregenerated throughelastic-package test system --generateevent.action,log.syslog.severity.name,stormshield.fw,stormshield.logtype,stormshield.in_bytes,stormshield.out_bytes) are untouchedHow to test this PR locally
Related issues