add deprecation warning for allow_superuser: true
#16555
Conversation
| deprecation_logger.deprecated("WARNING: You are currently running Logstash with superuser privileges. " + | ||
| "Starting from version 9, this will be disabled by default. " + | ||
| "To avoid disruption during the upgrade, set 'allow_superuser' to true now if you wish to continue running as superuser temporarily after the upgrade. " + | ||
| "Note that this is not recommended for security reasons.") |
There was a problem hiding this comment.
I would a bit simplify the statement.
| deprecation_logger.deprecated("WARNING: You are currently running Logstash with superuser privileges. " + | |
| "Starting from version 9, this will be disabled by default. " + | |
| "To avoid disruption during the upgrade, set 'allow_superuser' to true now if you wish to continue running as superuser temporarily after the upgrade. " + | |
| "Note that this is not recommended for security reasons.") | |
| deprecation_logger.deprecated("WARNING: You are currently running Logstash with superuser privileges which is not recommended for security reasons. " + | |
| "Starting from version 9, this will not be allowed by default unless 'allow_superuser' is intentionally set to true.") |
There was a problem hiding this comment.
@karenzone could you suggest how to do the deprecation communication? We want to suggest user to set 'allow_superuser' to true now for preparing v9 upgrade. The setting is optional at this moment, I am not sure how to deliver the message.
There was a problem hiding this comment.
@karenzone We are changing the behaviour when someone attempts to run Logstash as a superuser.
Running Logstash as a superuser is not recommended for security reasons, as it breaks the "Principle of Least Privilege", and increases any potential attack surface.
The current behaviour is to allow users to run as as a superuser by default, adding a setting allow_superuser:false to lock that down that has to be done specifically, potentially leading to a situation where Logstash could be run as root inadvertently.
In 9.0, we are going to switch the behaviour such that it has to be a conscious decision to allow Logstash to be run as a "superuser" that the user has to allow explicitly by setting allow_superuser to true
There was a problem hiding this comment.
Thanks for the ping and clarification, @kaisecheng and @robbavey. I'll take a look.
There was a problem hiding this comment.
Thanks for the question and the additional clarification. It seems like @mashhurs is on the right track with his suggestion. I started with his suggestion and iterated on it.
Does this achieve your goals:
"Starting from version 9, running with superuser privileges is not permitted unless you explicitly set 'allow_superuser' to true, thereby acknowledging the possible security risks."
If not, please let me know and we'll get it right.
logstash-core/lib/logstash/runner.rb
Outdated
| if setting("allow_superuser") | ||
| deprecation_logger.deprecated("NOTICE: Running Logstash as superuser is not recommended and won't be allowed in the future. Set 'allow_superuser' to 'false' to avoid startup errors in future releases.") | ||
| deprecation_logger.deprecated("NOTICE: Running Logstash as superuser will be completely disallowed in future versions. " + | ||
| "To prepare for this and avoid startup errors in future releases, it is strongly recommended to set 'allow_superuser' to false now " + |
There was a problem hiding this comment.
@karenzone please also consider this message. Both deprecation logs can print together.
|
💚 Build Succeeded
History
|
| deprecation_logger.deprecated("WARNING: You are currently running Logstash with superuser privileges. " + | ||
| "Starting from version 9, this will be disabled by default. " + | ||
| "To avoid disruption during the upgrade, set 'allow_superuser' to true now if you wish to continue running as superuser temporarily after the upgrade. " + | ||
| "Note that this is not recommended for security reasons.") |
There was a problem hiding this comment.
Thanks for the question and the additional clarification. It seems like @mashhurs is on the right track with his suggestion. I started with his suggestion and iterated on it.
Does this achieve your goals:
"Starting from version 9, running with superuser privileges is not permitted unless you explicitly set 'allow_superuser' to true, thereby acknowledging the possible security risks."
If not, please let me know and we'll get it right.
Release notes
Add deprecation warning to
allow_superuser: true. Starting from version 9, this will be disabled by default. To avoid disruption during the upgrade, set 'allow_superuser' to true now if you wish to continue running as superuser temporarily after the upgrade.What does this PR do?
allow_superuseris unsetWhy is it important/What is the impact to the user?
Checklist
Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs