Skip to content
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

[Breaking change] Audit logging events have changed #82578

Closed
thomheymann opened this issue Nov 4, 2020 · 21 comments · Fixed by #116191
Closed

[Breaking change] Audit logging events have changed #82578

thomheymann opened this issue Nov 4, 2020 · 21 comments · Fixed by #116191
Assignees
Labels
Breaking Change Feature:Upgrade Assistant impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort NeededFor:Security Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@thomheymann
Copy link
Contributor

thomheymann commented Nov 4, 2020

Change description

Which release will ship the breaking change?

8.0

Describe the change. How will it manifest to users?

Dropped support for legacy audit event. These have been replaced with audit events in ECS format which simplify ingestion into the stack, analysis with SIEM solution and provide more context and meta data.

The following events will not longer be logged: https://www.elastic.co/guide/en/kibana/current/xpack-security-audit-logging.html#_audit_event_types

How many users will be affected?

Anyone who is using the previous Kibana audit logger.

What can users do to address the change manually?

The ECS audit logger is controlled by the same xpack.security.audit.enabled setting as the legacy audit logger so no configuration needs to be changed and new event will automatically get logged out in the new format, if audit logging was previously enabled.

However, if users have alerts setup in Kibana or external systems those will need to be updated to look for the new ECS audit events instead:

  • saved_objects_authorization_success - Instead filter events by event.category=database and event.outcome=success|unknown
  • saved_objects_authorization_failure - Instead filter events by event.category=database and event.outcome=failure - The failure reason will be captured in the error property

A full list of events can be found here: https://www.elastic.co/guide/en/kibana/master/xpack-security-audit-logging.html#xpack-security-ecs-audit-logging

How could we make migration easier with the Upgrade Assistant?

Upgrade Assistant could check if xpack.security.audit.enabled is set, and if so add a "warning" message with the migration guidance shared above. Users could dismiss the warning if it didn't apply to them or if they've addressed it.

Are there any edge cases?

n/a

Test Data

n/a

Cross links

n/a

@thomheymann thomheymann added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more Feature:Upgrade Assistant NeededFor:Security Breaking Change labels Nov 4, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@thomheymann thomheymann added Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more and removed Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more labels Nov 4, 2020
@cjcenizal
Copy link
Contributor

@thomheymann What if Upgrade Assistant checked if xpack.security.audit.enabled was set, and if so then it added an item to a "Warnings" section with the migration guidance you shared? Users could dismiss the warning if they decided it didn't apply to them or if they've addressed it. Would affected users find that helpful?

@thomheymann
Copy link
Contributor Author

@cjcenizal I didn't know that we can surface messaging within the migration assistant. It's a great idea - I think that would be really useful for affected users. Especially if we list out what events they can consume instead.

@cjcenizal
Copy link
Contributor

@thomheymann Sounds good! Thanks for the validation. We have the opportunity to redesign the Upgrade Assistant for an awesome 8.0 upgrade experience, so the sky's the limit. Anything you can think of that would help the user is fair game when considering new features we should add to UA.

@alisonelizabeth alisonelizabeth changed the title [Breaking change] [Breaking change] Audit logging events have changed Nov 16, 2020
@alisonelizabeth
Copy link
Contributor

alisonelizabeth commented Nov 16, 2020

FYI I updated the issue title and How could we make migration easier with the Upgrade Assistant? section with CJ's suggestion. @thomheymann feel free to add anything else you think might be helpful.

@alisonelizabeth
Copy link
Contributor

Hi @thomheymann! I'm going to remove the Elasticsearch UI team label and replace with security. The Upgrade Assistant will no longer be responsible for adding individual deprecation logic. Please consider using the deprecations service (#94845) provided by core to register this deprecation instead. All registered deprecations will be displayed in the Upgrade Assistant (to be implemented via #97159). Feel free to reach out to myself or the core team with any questions!

@alisonelizabeth alisonelizabeth removed the Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more label Apr 19, 2021
@botelastic botelastic bot added the needs-team Issues missing a team label label Apr 19, 2021
@alisonelizabeth alisonelizabeth added Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! and removed NeededFor:Security needs-team Issues missing a team label labels Apr 19, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@jportner
Copy link
Contributor

jportner commented Jul 7, 2021

PR to deprecate the legacy audit logger in 7.15: #104685

@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Aug 4, 2021
@jportner jportner linked a pull request Aug 30, 2021 that will close this issue
@jportner
Copy link
Contributor

jportner commented Sep 2, 2021

I'm auditing consumer usage of audit logging (both flavors, "legacy" and "ECS"). When we get rid of legacy audit logging, we want to make sure we have the same coverage with our ECS audit logging. Rather than create a new issue, I decided to put my findings here.

Investigation

Consumers access the audit loggers from the security plugin's setup contract:

  • security.audit.asScoped (ECS audit logger)
  • security.audit.getLogger (legacy audit logger)

Aside from the security plugin itself, consumers of the legacy audit logger:

security plugin

We record both legacy and ECS audit events in the SecureSavedObjectsClientWrapper and the SecureSpacesClientWrapper. This was added in the initial ECS Audit logging implementation.

Summary: ECS audit coverage is equivalent to legacy audit coverage.

encryptedSavedObjects plugin

We record legacy audit events when an object is encrypted or decrypted in the EncryptedSavedObjectsService, and we do not have ECS audit logger coverage here.

The usefulness of these events is questionable; an error is logged for any error (even if a user is not unauthorized), and we already record audit events at the SOC level. So it looks like these audit events are unnecessary and we don't need to attempt to record them with the ECS audit logger. Even if we wanted to, we couldn't do so easily, because we need a Request context to do that and it isn't available here.

Summary: ECS audit coverage is not equivalent to legacy audit coverage, but this is acceptable.

actions plugin

We record legacy audit events when authorization is checked via ActionsAuthorization.

We record ECS audit events in the ActionsClient when it consumes the ActionsAuthorization class.

It also exposes the ActionsAuthorization class directly to other consumers in its start contract via getActionsAuthorizationWithRequest. However, the only consumer of this is the alerting plugin.

Summary: ECS audit coverage is equivalent to legacy audit coverage.

alerting plugin

We record legacy audit events when authorization is checked via AlertingAuthorization.

We record ECS audit events in the RulesClient when it consumes the AlertingAuthorization and ActionsAuthorization classes.

It also exposes the AlertingAuthorization class directly to other consumers in its start contract via getAlertingAuthorizationWithRequest. There are two consumers of this: the rule_registry plugin and the timelines plugin.

Summary: ECS audit coverage is equivalent to legacy audit coverage.

rule_registry plugin

We record ECS audit events in the AlertsClient when it consumes the AlertingAuthorization class.

Summary: ECS audit coverage is equivalent to legacy audit coverage. However, these audit actions (alert_get, alert_update, alert_find) are not included in our docs: https://www.elastic.co/guide/en/kibana/master/xpack-security-audit-logging.html#_category_database

timelines plugin

We consume the AlertingAuthorization class in the timelineSearchStrategyProvider, but we do not record ECS audit events here.

Summary: ECS audit coverage is lacking. However, this authorization check was only added recently in 7.15 (via #105333). I will engage with the @elastic/security-threat-hunting team to remedy this.


Conclusion

  1. The encryptedSavedObjects legacy audit events are unnecessary
  2. We need some docs updates for the rule_registry audit actions
  3. We need to add two audit events (success/fail) to a single method in the timelines plugin, then document that action.

Edit: (2) and (3) will be done in 8.0 via #112040

Aside from that, our ECS audit logging coverage meets or exceeds our legacy audit logging coverage. Based on that, I'd say we are safe to remove the legacy audit logger in 8.0.

@jportner
Copy link
Contributor

jportner commented Sep 3, 2021

Re-reading this issue description:

The ECS audit logger is controlled by the same xpack.security.audit.enabled setting as the legacy audit logger so no configuration needs to be changed and new event will automatically get logged out in the new format, if audit logging was previously enabled.
...
Upgrade Assistant could check if xpack.security.audit.enabled is set, and if so add a "warning" message with the migration guidance shared above. Users could dismiss the warning if it didn't apply to them or if they've addressed it.

Currently, the platform logging service provides a "default" appender, but that will be removed in 8.0. So I don't think we can just allow xpack.security.audit.enabled itself to enable the ECS audit logger in 8.0+ without some additional work.

I see two options to proceed:

  1. Force users to configure xpack.security.audit.appender in 8.0+ (if a user does not change their config, Kibana will fail to start after the 8.0 upgrade)
  2. Starting in 8.0, add a default value for xpack.security.audit.appender (drop the audit logs to some file like ./audit.log)

I am leaning towards option 2, but I am curious to know what others think.
CC @legrego @thomheymann @watson

@thomheymann
Copy link
Contributor Author

Currently, the platform logging service provides a "default" appender, but that will be removed in 8.0. So I don't think we can just allow xpack.security.audit.enabled itself to enable the ECS audit logger in 8.0+ without some additional work.

What does this mean in practice? Do users need to manually configure whether they want to log to stdout or file or is this just an internal change for plugin developers?

  1. Force users to configure xpack.security.audit.appender in 8.0+ (if a user does not change their config, Kibana will fail to start after the 8.0 upgrade)
  2. Starting in 8.0, add a default value for xpack.security.audit.appender (drop the audit logs to some file like ./audit.log)

Yeh, definitely prefer option 2. as well here.

@watson
Copy link
Contributor

watson commented Sep 7, 2021

@jportner I'm voting for option 2 as well. I do not like it if Kibana gets too complicated to get up and running. Sensible defaults is the way to go IMO.

@jportner
Copy link
Contributor

jportner commented Sep 9, 2021

Sounds good, let's go with option 2.

@jportner
Copy link
Contributor

jportner commented Sep 9, 2021

What does this mean in practice? Do users need to manually configure whether they want to log to stdout or file or is this just an internal change for plugin developers?

According to this issue (#92082), the injected "default" legacy appender will be removed in 8.0. It hasn't been removed yet and I'm not sure what the user experience will be like as a result.
Either way, it sounds like we are in favor of adding our own custom default appender for the audit logger if users don't explicitly configure one. So it sounds like that won't impact us.

@legrego
Copy link
Member

legrego commented Sep 9, 2021

Either way, it sounds like we are in favor of adding our own custom default appender for the audit logger if users don't explicitly configure one. So it sounds like that won't impact us.

++. Based on #98213, this may need to be OS/package specific. @elastic/kibana-operations we are interested in writing Kibana's audit log to a dedicated file by default. Do you have any advice on where in the file system we should be writing these? I expect that the downloadable version is pretty simple (Kibana root?), but what about os packages, Docker, etc.?

@tylersmalley
Copy link
Contributor

@legrego we have to be careful with where we write to, and can't write to the root usually. We do have a configuration available for path.data which feels like it can be used here. @jbudz, do you foresee any issue with that?

@jbudz
Copy link
Member

jbudz commented Sep 9, 2021

I'd recommend creating a logs folder in the project root here. If this is needed for development too, we could commit a logs directory and cleanup the .gitempty at build time.

We can followup with a change here to write to /var/log/kibana for debian and rpm packages.

Consistency with elasticsearch would be ideal:

~/Downloads/elasticsearch-7.14.1 » ls logs                                                              130 ↵ jon@xps
elasticsearch_audit.json                   elasticsearch_index_indexing_slowlog.log  elasticsearch_server.json
elasticsearch_deprecation.json             elasticsearch_index_search_slowlog.json   gc.log
elasticsearch_deprecation.log              elasticsearch_index_search_slowlog.log    gc.log.00
elasticsearch_index_indexing_slowlog.json  elasticsearch.log

@lukeelmers
Copy link
Member

According to this issue (#92082), the injected "default" legacy appender will be removed in 8.0. It hasn't been removed yet and I'm not sure what the user experience will be like as a result.

JFYI, here is the issue tracking removal of the legacy logging infrastructure (which includes removing the injected default appender): #50660

@mshustov
Copy link
Contributor

mshustov commented Sep 15, 2021

Do I understand correctly that you are talking about this AuditService?

Isn't it using a new logging framework?

auditTrailAppender: config.appender ?? {

How the removal of the default appender might affect it?

we are interested in writing Kibana’s audit log to a dedicated file by default.

Is it the default behavior of the deprecated Audit logging service?
Cannot the Security service use the same approach as ESC audit logging does by declaring a custom file appender? It might be faster than implementing #92082

    appenders: {
      auditTrailFileAppender: config.auditTrailFileAppender ?? {
        type: 'file',
        fileName: ...,
        layout: {
          type: 'json',
        },
      },
    },
    loggers: [
      {
        name: '...',
        level: ...
        appenders: ['auditTrailFileAppender'],
      },
    ],

@jportner
Copy link
Contributor

Do I understand correctly that you are talking about this AuditService?

Yes!

Isn't it using a new logging framework?

Yes!

How the removal of the default appender might affect it?

we are interested in writing Kibana’s audit log to a dedicated file by default.

Is it the default behavior of the deprecated Audit logging service?
Cannot the Security service use the same approach as ESC audit logging does by declaring a custom file appender?

That's a great point, then ECS audit logging already doesn't rely on the "default" legacy appender at all.
So that sounds good, we just need to make the change there and point our default value for the auditTrailFileAppender to the correct directory based on the build.

@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Oct 12, 2021
@yctercero
Copy link
Contributor

@jportner this PR relates to use of audit logs in the events timelines search strategy and includes updates to the docs.

#112040

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Feature:Upgrade Assistant impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort NeededFor:Security Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet