-
Notifications
You must be signed in to change notification settings - Fork 450
[Armis] Initial release of the armis #13429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
/test |
🚀 Benchmarks reportTo see the full report comment with |
💚 Build Succeeded
|
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
1. In the **alert data stream**, based on the documentation, we initially expected to use the `statusChangeTime` field for filtering updates. However, the `"after"` filter applies only to the primary `time` field from the alert endpoint and does not support filtering based on `statusChangeTime`. As a result, when an alert's status changes, the data collection process does not capture these updates. | ||
|
||
2. In the **vulnerability data stream**, our filtering mechanism for the **vulnerability search API** relies specifically on the `lastDetected` field. This means that when a user takes action on a vulnerability and `lastDetected` updates, only then will the event for that vulnerability be retrieved. Initially, we assumed this field would always have a value and could be used as a cursor timestamp for fetching data between intervals. However, due to inconsistencies in the API response, we observed cases where `lastDetected` is `null`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please share API docs via DM (if private) or the link (if public)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also if the docs are public, please add a link to README.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don’t have public API documentation at the moment. We can share a PDF version with you via DM.
: | ||
state.?cursor.last_timestamp, | ||
}, | ||
"want_more": has(body.?data.next) && body.data.next != null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both conditions has(body.?data.next) && body.data.next != null
are checking the same thing. Here body.data.next != null
may not be required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for other data streams if applicable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition body.data.next != null
is indeed necessary in this context. On the last page, body.data.next
can have a value of null
, and using only has(body.?data.next)
would still return true for a null
value. Therefore, the additional check ensures the logic handles this scenario correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any cases that it is not present? When does this happen? If it's always present, we don't need to use the has, or the optional types above, we can just compare against null.
"length": [string(state.page_size)], | ||
"orderBy": ["time"], | ||
?"from": has(state.offset) && state.offset != null ? optional.of([string(state.offset)]) : optional.none(), | ||
"aql":["in:alerts " + "after:" + state.start_time] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there documentation for this AQL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can share the AQL documentation as a PDF via DM.
"device-abc", | ||
"server-xyz" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In live data, are these IPs/domains? Can they be added to related*
or destination*
ECS fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don’t have live data for this field; the values are just examples provided.
"469" | ||
], | ||
"friendly_name": "Unusual Network Scanning Activity", | ||
"last_alert_update_time": "2025-03-29T02:50:06.124Z", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats the diff between last_alert_update_time
and status_change_time
and time
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
time
: Alert timestamp.status_change_time
: When the alert's status was last changed.last_alert_update_time
: We don't have live data for this field and are not sure what it represents.
"mac_address": "46-A7-4B-5D-B0-76", | ||
"manufacturer": "test", | ||
"model": "test", | ||
"name": "49f9ef97_1b8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In live data, do you see this field good to populate host.name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have mapped names to host.name
as it looks like super set of name in live data.
"host": { | ||
"id": "469", | ||
"ip": [ | ||
"175.16.199.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you populate host.geo.*
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ipAddress
field may contain comma-separated IPs, so it's not ideal to map it directly to host.geo.*
, which expects a single IP for accurate geolocation enrichment.
"tags": [ | ||
"Misconfigurations" | ||
], | ||
"type": "Servers", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this "type": "Servers",
makes more sense for host.type
. or just use type_enum
field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We’ve already mapped category to host.type
. Do we need to map the type
field instead of category
to host.type
?
@@ -0,0 +1,34 @@ | |||
# Use of "*" to use all namespaces defined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of https://docs.elastic.dev/security-solution/cloud-security/cdr/3p-dev-guide, we will be adding a transform later on for this data. So, we don't need a transform right now for vulnerability.
- host.id | ||
sort: "@timestamp" | ||
description: >- | ||
Latest Devices from Armis. As devices get updated, this transform stores only the latest state of each device inside the destination index. Thus the transform's destination index contains only the latest state of the device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious about the use case of this transform and what we are trying to solve.
I believe you can get total hosts number using unique counts aggregation without having to use a latest transform.
Are there properties of hosts that get modified and the latest values of such properties are important to the users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, whenever any property of a device is updated for a particular host ID, the lastSeen
field is also updated in the same log entry. The updated properties are reflected alongside it.
That’s why we’ve added the transform to retain the latest state of each host ID based on the most recent updates.
: | ||
state.?cursor.last_timestamp, | ||
}, | ||
"want_more": has(body.?data.next) && body.data.next != null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any cases that it is not present? When does this happen? If it's always present, we don't need to use the has, or the optional types above, we can just compare against null.
packages/armis/data_stream/alert/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/armis/data_stream/vulnerability/agent/stream/cel.yml.hbs
Outdated
Show resolved
Hide resolved
#13429 (comment) |
: | ||
resp.StatusCode == 401 ? | ||
{ | ||
"events": [{"message":"retry"}], | ||
"is_token_valid": false, | ||
"offset": has(state.offset) && state.offset != null ? state.offset : 0, | ||
"want_more": true, | ||
} | ||
// Armis doesn't support multiple sessions per token; generate a new token on 401 (expired) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
: | |
resp.StatusCode == 401 ? | |
{ | |
"events": [{"message":"retry"}], | |
"is_token_valid": false, | |
"offset": has(state.offset) && state.offset != null ? state.offset : 0, | |
"want_more": true, | |
} | |
// Armis doesn't support multiple sessions per token; generate a new token on 401 (expired) | |
: resp.StatusCode == 401 ? | |
// Armis doesn't support multiple sessions per token; generate a new token on 401 (expired). | |
{ | |
"events": [{"message":"retry"}], | |
"is_token_valid": false, | |
"offset": has(state.offset) && state.offset != null ? state.offset : 0, | |
"want_more": true, | |
} |
(similar below)
Hey @kcreddy, @efd6, @james-elastic - Just a quick update — there might be a delay in addressing the comments on this PR as we’ve been in touch with the Armis team and shared our observations. They are considering incorporating the changes as part of a future enhancement, which we can expect in a week or two., so we are currently not prioritising this. |
@piyush-elastic, can you make the PR as draft in the mean time? |
Proposed commit message
The initial release includes alert, device, and vulnerability data streams,
along with associated dashboards and visualizations.
Armis fields are mapped to their corresponding ECS fields where possible.
Test samples were derived from live data samples, which were subsequently
sanitized.
Checklist
changelog.yml
file.How to test this PR locally
Related Issues
Screenshots