tools: set ownership of syslog created log file to frr #4379
Conversation
Problem reported with errors if "log file /var/log/frr/frr.log" is entered and the file had been created by syslog with root ownership. This fix sets the ownership to frr/frrvty so that the file can be referenced and defined within frr if syslog created it. Signed-off-by: Don Slice <dslice@cumulusnetworks.com> Ticket: CM-18981
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7777/ This is a comment from an automated CI system. CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base12 Static Analyzer issues remaining.See details at |
|
This decreases security since an attacker that has gained remote code execution under the frr user can now tamper with the log files (e.g. to remove signs of the intrusion.) Writing the log file through syslog enforces append-only semantics as long as syslog / root is not compromised; this is lost with this change. |
| # cause issues. | ||
|
|
||
| $FileOwner frr | ||
| $FileGroup frrvty |
There was a problem hiding this comment.
In addition to the intrusion problem described in my conversation comment, I'm not convinced frrvty is the right group here. It's the group that controls vty access, which human operator users can be added to so they can use vtysh without being root. If we want to give them read access, then this makes sense, but then we also need to make sure the permissions are 0640. If the permissions include group write, this opens a hole that suddenly allows operators to tamper with the logs. If the permissions are 0644, the group doesn't matter.
So either this is accompanied by a permission setting to 0640, or the group setting is underdetermined in its effect and needs to go.
| # CLI. Without this, log file is created with root ownership which can | ||
| # cause issues. | ||
|
|
||
| $FileOwner frr |
There was a problem hiding this comment.
Ok so I just looked at the rsyslogd docs and these are global configuration parameters, which affect all following statements (the doc for $FileOwner and $FileGroup doesn't say specifically, but $FileCreateMode does and I think it's reasonable to assume these 3 work the same way.)
That would mean that the effect of these statements is not limited to 45-frr.conf (or the frr.log file created) but rather extends to any other logging config loaded after it, e.g. 50-foobar.conf.
I'm only 60% confident on this (I use syslog-ng personally), but this definitely needs to be confirmed or invalidated.
There was a problem hiding this comment.
Thanks for the feedback. I'll check into whether this is true or not and find alternatives.
|
@daveolson53 -> Thoughts here about how we can approach this issue better? |
|
After further discussion with Dave Olson, agree that we need to find a better answer. closing the PR. |
Problem reported with errors if "log file /var/log/frr/frr.log" is
entered and the file had been created by syslog with root ownership.
This fix sets the ownership to frr/frrvty so that the file can be
referenced and defined within frr if syslog created it.
Signed-off-by: Don Slice dslice@cumulusnetworks.com
Ticket: CM-18981