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

log4j2.properties should compress audit logs by default. maybe all logs. #63843

Closed
wasserman opened this issue Oct 16, 2020 · 10 comments · Fixed by #64472
Closed

log4j2.properties should compress audit logs by default. maybe all logs. #63843

wasserman opened this issue Oct 16, 2020 · 10 comments · Fixed by #64472
Assignees
Labels
:Core/Infra/Logging Log management and logging utilities >enhancement :Security/Audit X-Pack Audit logging Team:Core/Infra Meta label for core/infra team Team:Security Meta label for security team

Comments

@wasserman
Copy link
Contributor

When audit logs are enabled they can generate a lot of data. We have been fighting with disk space issues regularly. It seems like all logs should gzip in the rotation by default since it will create fires that people will respond to in various ways depending on their understanding of log4j or even Elastic. Of course people need to adjust the logging to suite their needs, but a sane default would be nice.

https://github.com/elastic/elasticsearch/blob/77661af2c5905b16884d1b0d5c7b7c9e86b7bee7/x-pack/plugin/core/src/main/config/log4j2.properties

Our solution was to use a block similar to this. Feel free to adopt it or share a recommendation.

appender.audit_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_audit-%d{yyyy-MM-dd}-%i.json.gz
appender.audit_rolling.policies.type = Policies
appender.audit_rolling.policies.time.type = TimeBasedTriggeringPolicy
appender.audit_rolling.policies.time.interval = 1
appender.audit_rolling.policies.time.modulate = true
appender.audit_rolling.policies.size.type = SizeBasedTriggeringPolicy
appender.audit_rolling.policies.size.size = 128MB
appender.audit_rolling.strategy.type = DefaultRolloverStrategy
appender.audit_rolling.strategy.fileIndex = nomax
appender.audit_rolling.strategy.action.type = Delete
appender.audit_rolling.strategy.action.basepath = ${sys:es.logs.base_path}
appender.audit_rolling.strategy.action.condition.type = IfFileName
appender.audit_rolling.strategy.action.condition.glob = ${sys:es.logs.cluster_name}_audit*
appender.audit_rolling.strategy.action.condition.nested_condition.type = IfLastModified
appender.audit_rolling.strategy.action.condition.nested_condition.age = 7D

Thanks!

@wasserman wasserman added >enhancement needs:triage Requires assignment of a team area label labels Oct 16, 2020
@somayaj
Copy link

somayaj commented Oct 17, 2020

@wasserman can I work this?

@wasserman
Copy link
Contributor Author

@somayaj I don't work for Elastic. What are you asking?

@somayaj
Copy link

somayaj commented Oct 18, 2020 via email

@gwbrown gwbrown added :Core/Infra/Logging Log management and logging utilities :Security/Audit X-Pack Audit logging and removed needs:triage Requires assignment of a team area label labels Oct 22, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Logging)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Audit)

@elasticmachine elasticmachine added Team:Security Meta label for security team Team:Core/Infra Meta label for core/infra team labels Oct 22, 2020
@pgomulka pgomulka self-assigned this Oct 29, 2020
@pgomulka
Copy link
Contributor

@bytebilly confirmed with Security team that this was just an overlooked and audit logs should be compressed as well.

@pgomulka
Copy link
Contributor

pgomulka commented Nov 2, 2020

@bytebilly security logs are at the moment rolled over by date. Do we still want this?

appender.audit_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_audit-%d{yyyy-MM-dd}.json
appender.audit_rolling.policies.type = Policies
appender.audit_rolling.policies.time.type = TimeBasedTriggeringPolicy
appender.audit_rolling.policies.time.interval = 1
appender.audit_rolling.policies.time.modulate = true

Zipping logs is most useful when combining this with Size based policy. I.e. new zip file every for 1GB of logs.
Do we want to keep both time and size based policies?
Also do we want to limit number of zipped files? We do this for other logs and limit to 5 files (4zips and 1 active file)

cc @elastic/es-security

pgomulka added a commit to pgomulka/elasticsearch that referenced this issue Nov 2, 2020
audit logs should be compressed when rolling over due to size based
triggering policy breaching 1GB.
Total number of zipped files should be the same as for other log = 4

closes elastic#63843
@bytebilly
Copy link
Contributor

Good questions @pgomulka.

Zipping logs is most useful when combining this with Size based policy. I.e. new zip file every for 1GB of logs.
Do we want to keep both time and size based policies?

In general I am not aware of any reason why we want to avoid size limits for these logs.
However, this introduces a breaking change since it affects appender.audit_rolling.filePattern.
We are also introducing a trailing .gz that has a similar impact to potential customer's flows (e.g. ingest patterns).

I want to double check that we are ok doing that and we don't see it as a blocker.
What do you think?

@bytebilly
Copy link
Contributor

Also do we want to limit number of zipped files? We do this for other logs and limit to 5 files (4zips and 1 active file)

I would argue that since this is the audit trail, we should guarantee its best availability even if something fails (e.g. ingest into a monitoring cluster). Deleting old files can reduce the ability to investigate on security incidents.

I had no feedback that it is a recurring problem, so I'd prefer to keep them. Does it make sense to you?

@pgomulka
Copy link
Contributor

pgomulka commented Nov 4, 2020

agree -this is a breaking change so we can introduce it in v8 and deprecate in 7.x

also agree, since this is security audit we can keep them all.

pgomulka added a commit that referenced this issue Dec 2, 2020
audit logs should be compressed when rolling over due to size based
triggering policy breaching 1GB.
Files are not being deleted.

closes #63843
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Logging Log management and logging utilities >enhancement :Security/Audit X-Pack Audit logging Team:Core/Infra Meta label for core/infra team Team:Security Meta label for security team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants