diff mbox series

[ovs-dev,v3] rhel: Use correct user in the logrotate configuration file

Message ID 20180808142725.1732-1-mchandras@suse.de
State Accepted
Headers show
Series [ovs-dev,v3] rhel: Use correct user in the logrotate configuration file | expand

Commit Message

Markos Chandras Aug. 8, 2018, 2:27 p.m. UTC
The /var/log/openvswitch directory is owned by the openvswitch user but
logrotate could be running as root or as another user. As a result of
which, rpmlint prints the following warning when building the spec file
on SUSE Linux Enterprise:

openvswitch.x86_64: W: suse-logrotate-user-writable-log-dir /var/log/openvswitch openvswitch:openvswitch 0750
The log directory is writable by unprivileged users. Please fix the
permissions so only root can write there or add the 'su' option
to your logrotate config

In order to fix that, we should run the logrotate script as the same
user which runs the various Open vSwitch daemons. If this is a new
installation, then this user is the 'openvswitch' one, but if we are
upgrading from an older release, then the user is normally 'root'.
As such, we set the initial user to 'root' and we fix this up in the
%post scriptlet.

Cc: Aaron Conole <aconole@redhat.com>
Cc: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Markos Chandras <mchandras@suse.de>
---
 rhel/etc_logrotate.d_openvswitch                 | 1 +
 rhel/openvswitch-fedora.spec.in                  | 4 +++-
 rhel/usr_lib_systemd_system_ovsdb-server.service | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

Comments

0-day Robot Aug. 8, 2018, 2:56 p.m. UTC | #1
Bleep bloop.  Greetings Markos Chandras, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 84 characters long (recommended limit is 79)
#82 FILE: rhel/usr_lib_systemd_system_ovsdb-server.service:13:
ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch

Lines checked: 89, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
Timothy Redaelli Aug. 8, 2018, 3:52 p.m. UTC | #2
On Wed,  8 Aug 2018 17:27:25 +0300
Markos Chandras <mchandras@suse.de> wrote:

> The /var/log/openvswitch directory is owned by the openvswitch user
> but logrotate could be running as root or as another user. As a
> result of which, rpmlint prints the following warning when building
> the spec file on SUSE Linux Enterprise:
> 
> openvswitch.x86_64: W:
> suse-logrotate-user-writable-log-dir /var/log/openvswitch
> openvswitch:openvswitch 0750 The log directory is writable by
> unprivileged users. Please fix the permissions so only root can write
> there or add the 'su' option to your logrotate config
> 
> In order to fix that, we should run the logrotate script as the same
> user which runs the various Open vSwitch daemons. If this is a new
> installation, then this user is the 'openvswitch' one, but if we are
> upgrading from an older release, then the user is normally 'root'.
> As such, we set the initial user to 'root' and we fix this up in the
> %post scriptlet.
> 
> Cc: Aaron Conole <aconole@redhat.com>
> Cc: Timothy Redaelli <tredaelli@redhat.com>
> Signed-off-by: Markos Chandras <mchandras@suse.de>

Acked-by: Timothy Redaelli <tredaelli@redhat.com>
Ben Pfaff Aug. 8, 2018, 5:58 p.m. UTC | #3
On Wed, Aug 08, 2018 at 05:52:55PM +0200, Timothy Redaelli wrote:
> On Wed,  8 Aug 2018 17:27:25 +0300
> Markos Chandras <mchandras@suse.de> wrote:
> 
> > The /var/log/openvswitch directory is owned by the openvswitch user
> > but logrotate could be running as root or as another user. As a
> > result of which, rpmlint prints the following warning when building
> > the spec file on SUSE Linux Enterprise:
> > 
> > openvswitch.x86_64: W:
> > suse-logrotate-user-writable-log-dir /var/log/openvswitch
> > openvswitch:openvswitch 0750 The log directory is writable by
> > unprivileged users. Please fix the permissions so only root can write
> > there or add the 'su' option to your logrotate config
> > 
> > In order to fix that, we should run the logrotate script as the same
> > user which runs the various Open vSwitch daemons. If this is a new
> > installation, then this user is the 'openvswitch' one, but if we are
> > upgrading from an older release, then the user is normally 'root'.
> > As such, we set the initial user to 'root' and we fix this up in the
> > %post scriptlet.
> > 
> > Cc: Aaron Conole <aconole@redhat.com>
> > Cc: Timothy Redaelli <tredaelli@redhat.com>
> > Signed-off-by: Markos Chandras <mchandras@suse.de>
> 
> Acked-by: Timothy Redaelli <tredaelli@redhat.com>

Thanks Markos and Timothy.  I applied this to master and branch-2.10.
If should be backported further, please let me know.
diff mbox series

Patch

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index ed7d733c9..f4302ffbc 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -6,6 +6,7 @@ 
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
+    su root root
     daily
     compress
     sharedscripts
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 9f8664e95..c2d3200e1 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -405,6 +405,7 @@  exit 0
 %post
 if [ $1 -eq 1 ]; then
     sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
+    sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' %{_sysconfdir}/logrotate.d/openvswitch
 
 %if %{with dpdk}
     sed -i \
@@ -414,6 +415,7 @@  if [ $1 -eq 1 ]; then
 
     # In the case of upgrade, this is not needed.
     chown -R openvswitch:openvswitch /etc/openvswitch
+    chown -R openvswitch:openvswitch /var/log/openvswitch
 fi
 
 %if 0%{?systemd_post:1}
@@ -601,7 +603,7 @@  fi
 %endif
 %doc NOTICE README.rst NEWS rhel/README.RHEL.rst
 /var/lib/openvswitch
-%attr(750,openvswitch,openvswitch) /var/log/openvswitch
+%attr(750,root,root) /var/log/openvswitch
 %ghost %attr(755,root,root) %{_rundir}/openvswitch
 
 %files ovn-docker
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 0fa57a925..70da1ec95 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -10,7 +10,7 @@  Type=forking
 Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
-ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
+ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch /var/log/openvswitch
 ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo "OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
 EnvironmentFile=-/run/openvswitch/useropts
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \