[santa] update ingest pipeline to match new log format #3347
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
jamiehynds
added
Team:Security-External Integrations
enhancement
DamiaPoquet
changed the title
|
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
|
/test |
Is there any use case for preserving those old log samples? If I leave the original ones, the tests will fail. |
|
Do we know that these are no users still on the old version? If we are going to drop support for the older log format then this should really be a breaking change and marked as such. The preferred option though would be to retain support for the old format while adding the new. |
|
In my opinion, it doesn't make sense to support a pre-release output format for Santa. All the GA versions are outputting logs in the new format. I'll mark this release as a breaking change. |
|
/test |
🌐 Coverage report
|
| "pid": 72012, | ||
| "pidversion": 1097765, |
There was a problem hiding this comment.
process.pidversion is not an allowed ECS field, so it should be placed in santa, but we could construct a process.entity_id with it and process.pid, something like "{{{process.pid}}}-{{[process.pidversion}}}". @andrewkroh WDYT? We don't have any examples of it being used anywhere else AFAICS.
There was a problem hiding this comment.
That sounds like a good idea. I am looking for some documentation on what pidversion in the Santa logs means just to confirm that it makes sense to use it as part of process.entity_id.
There was a problem hiding this comment.
From what I could find it's just another pid, but from the kernel to prevent pid collision on pid rollover, e.g. https://www.exploit-db.com/exploits/46648. The Apple documentation is also not wildly helpful https://developer.apple.com/documentation/kernel/proc_persona_info/1560796-pidversion, the source helps https://github.com/apple/darwin-xnu/blob/2ff845c2e033bd0ff64b5b6aa6063a1f8f65aa32/bsd/kern/kern_exec.c#L6215-L6216.
This in the context of this gives the best answer I could find.
There was a problem hiding this comment.
I have changed the field name to santa.pidversion while the ECS is not ready yet to support it.
|
/test |
There was a problem hiding this comment.
LGTM. We could add a process.entity_id independently.
I was curious how Elastic Endpoint generates its process.entity_id to see if we could replicate that here. The code is
(void)StringLib::FormatDuplicate(str, "%s-%llu-%llu.%llu", Cache::GetEndpointID().c_str(),
(unsigned long long)pid, (unsigned long long)ts.tv_sec, (unsigned long long)ts.tv_nsec);
(void)Titan_Encoding_toBase64(str.c_str(), str.size(), (void **)&(pBase64), &(base64Size));
But given that the start time we have is coming from the logger timestamp and lacks nanosecond precision, I don't think we can. So the only thing I would say is that if we do add an entity_id we should consider incorporating the agent ID.
SGTM |
What does this PR do?
The log output for Santa changed on versions 0.9.19, v0.9.20, and 0.9.21, making this integration fundamentally broken since then. This PR addresses the new log format parsing.
Checklist
changelog.ymlfile.Author's Checklist
pidversionHow to test this PR locally
Related issues
I didn't find any related issue, although it seems to be broken since 2017.