elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
1.2k stars 24.85k forks source link

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

Closed wasserman closed 3 years ago

wasserman commented 4 years ago

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!

somayaj commented 4 years ago

@wasserman can I work this?

wasserman commented 4 years ago

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

somayaj commented 4 years ago

Can i make the change as documented on the issue and create the PR.

On Sun, Oct 18, 2020 at 9:11 AM wasserman notifications@github.com wrote:

@somayaj https://github.com/somayaj I don't work for Elastic. What are you asking?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/elastic/elasticsearch/issues/63843#issuecomment-711173467, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIRAZGPGOEFJOC3DCIKTPJTSLLZRLANCNFSM4STU7G5Q .

elasticmachine commented 4 years ago

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

elasticmachine commented 4 years ago

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

pgomulka commented 4 years ago

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

pgomulka commented 4 years ago

@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

bytebilly commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.