-
Notifications
You must be signed in to change notification settings - Fork 474
Integration of Microsoft SQL Server - Audit Events #2009
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
Integration of Microsoft SQL Server - Audit Events #2009
Conversation
Define split ingest processor to transform raw data into new line separated entries
Fix event_time from date to text due to agent side parsing errors
- Add all fields - Add conversions where necessasry
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
There was an initial discussion on this on slack, awaiting further changes to be pushed before doing another review. |
- Change field types to numbers for those which can be queried by range - Set kv target directly to sqlserver.audit and remove _temp - Update README
- Add event_id as configurable input - Handle tags and preserve original event - Add processors
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
...ages/microsoft_sqlserver/data_stream/audit/_dev/test/pipeline/test-events.json-expected.json
Outdated
Show resolved
Hide resolved
packages/microsoft_sqlserver/data_stream/audit/fields/fields.yml
Outdated
Show resolved
Hide resolved
packages/microsoft_sqlserver/data_stream/audit/fields/fields.yml
Outdated
Show resolved
Hide resolved
packages/microsoft_sqlserver/data_stream/audit/fields/fields.yml
Outdated
Show resolved
Hide resolved
packages/microsoft_sqlserver/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/microsoft_sqlserver/data_stream/audit/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
target_field: process | ||
ignore_missing: true | ||
- rename: | ||
field: winlog.user |
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.
It looks like this might be the process that wrote the event log rather than the SQL user that performed the action. So I would leave it unmodified.
I would expect user.*
to be based on the user that's logged into the DB. I think that means server_principal_*
is used to populate user.*
and target_server_principal_*
gets copied into user.target.*
.
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 have addressed these too. From what I understand the server_principal_sid
for a domain account is stored in the encoded binary form in SQL Server so it probably isn't sending target_server_principal_sid
(I haven't seen it in my tests) as they are the same. I've set it anyway in case we receive it, we'll set it.
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.
So these are not the standard Windows SID strings (S-R-I-S-S format), but are a SQS server SID in varbinary format? Perhaps it we should retain the original "principal" keys to have some traceability on where the values of user.* came from.
If we never retain the original principal fields then they should be removed from the mappings.
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 value in these depend on the type of login / account. SQL Server can be configured to allow logins in Mixed Mode
or Windows Authentication Mode
. Microsoft recommends to use Windows authentication as it is more secure. In mixed mode the user could be a SQL server user in which case the SID is a 16 byte varbinary specific to SQL Server version. But if the login is tied to Windows authentication then SQL server hands off the authentication to Windows domain controller. For this case the SID is windows SID but converted into binary format. There seems to be functions available to convert them back to string representation (S-1-x-x-x
) in SQL server.
From what I understand, the value could be the original principal SID but in a different format. It could be converted between varbinary and text formats in SQL Server.
Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
…ngest_pipeline/default.yml Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
* Update fields from text to keyword where applicable * Change host.mac format to ECS recommendation
…leet integrations
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.
LGTM! Great work on the package, sorry it tok so long. Did a lot of the reviewing outside of Github as well
* Set SPN and Target SPN to user.* and user.target.* ECS fields * Cleanup
@andrewkroh Thank you for your comments. I've addressed them all. I'll wait for you to take another look. |
Thank you @P1llus for your inputs and help with this integration! No worries about the timelines. |
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 the organization of the pipeline needs changed if events other than 33205 are expected to be processed. Everything else LGTM.
Once the SQL Server has been configured to log audit events to event log and has no issues w.r.t permissions etc all events are logged with id There are some audit related events reaching other IDs like |
What does this PR do?
Build a new integration package for Microsoft SQL Server Audit Events. (#1635).
Checklist
changelog.yml
file.manifest.yml
file to point to the latest Elastic stack release (e.g.^7.13.0
).Author's Checklist
Still working on adding visualizations and saved searches etc.
How to test this PR locally
Related issues
Screenshots