Bug 2158659

Summary: rsyslog libcap-ng integration doesn't play well with -N option (config file validation)
Product: Red Hat Enterprise Linux 9 Reporter: Robert Muir <rcmuir>
Component: rsyslogAssignee: Attila Lakatos <alakatos>
Status: CLOSED ERRATA QA Contact: Dalibor Pospíšil <dapospis>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 9.0CC: alakatos, dapospis, jik, jpazdziora, rsroka
Target Milestone: rcKeywords: AutoVerified, Triaged
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: rsyslog-8.2102.0-111.el9 Doc Type: No Doc Update
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-05-09 07:44:54 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Robert Muir 2023-01-06 01:47:14 UTC
Description of problem:

libcap-ng integration introduced in bug 2127404 always drops capabilities, even when running with -N (validate config file only).

This breaks the use-case of trying to simply validate a config file before deploying it, e.g. with ansible:

  template:
    src: rsyslog.conf
    dest: /etc/rsyslog.conf
    validate: /sbin/rsyslogd -f %s -N 1
  become: true

You'll get a strange Permission Denied error, because ansible will upload the new config file to a tmp directory owned by ordinaryuser, then attempt to validate as root, before deploying to /etc/rsyslog.conf. Because CAP_DAC_OVERRIDE is dropped, rsyslog -N 1 can't access the file owned by the ordinary user.


Version-Release number of selected component (if applicable): v8.2102.0-107.el9


How reproducible: always


Steps to Reproduce:
1. make a rsyslog.conf owned by an ordinary user with 700 permissions
2. run rsyslog -f /path/to/rsyslog.conf -N 1 as root

Actual results: Permission denied (and its a sneaky one to debug, strace is needed)


Expected results: Validate the config file only. The manual page for -N states:

"Do a config check.  Do NOT run in regular mode, just check configuration file correctness. This option is meant to verify a config file."

Additional info:

Maybe we can just avoid dropping capabilities when the -N flag is supplied?
As a workaround, we removed the "validate: " option in our ansible, but that's not great as it makes things more difficult to debug.

Comment 1 Robert Muir 2023-01-06 02:13:30 UTC
I'm also curious if a simpler alternative to patching the source code to drop capabilities would be to just specify CapabilityBoundingSet=... in the rsyslog.service unit, see systemd.exec(5)
Then I wouldn't have issues e.g. validating the config file, but the service would still be protected.

Comment 2 Attila Lakatos 2023-01-06 10:05:21 UTC
Hello,

(In reply to Robert Muir from comment #0)
> Description of problem:
> 
> libcap-ng integration introduced in bug 2127404 always drops capabilities,
> even when running with -N (validate config file only).
> 
> This breaks the use-case of trying to simply validate a config file before
> deploying it, e.g. with ansible:
> 
>   template:
>     src: rsyslog.conf
>     dest: /etc/rsyslog.conf
>     validate: /sbin/rsyslogd -f %s -N 1
>   become: true
> 
> You'll get a strange Permission Denied error, because ansible will upload
> the new config file to a tmp directory owned by ordinaryuser, then attempt
> to validate as root, before deploying to /etc/rsyslog.conf. Because
> CAP_DAC_OVERRIDE is dropped, rsyslog -N 1 can't access the file owned by the
> ordinary user.
This was the whole point of introducing libcapng inside rsyslog. We do not want to give any extra capabilities to the rsyslog daemon. Isn't it possible to validate the config file before uploading it to a temporary directory? Or to validate the config as an ordinary user?

> 
> 
> Version-Release number of selected component (if applicable):
> v8.2102.0-107.el9
> 
> 
> How reproducible: always
> 
> 
> Steps to Reproduce:
> 1. make a rsyslog.conf owned by an ordinary user with 700 permissions
> 2. run rsyslog -f /path/to/rsyslog.conf -N 1 as root
> 
> Actual results: Permission denied (and its a sneaky one to debug, strace is
> needed)
> 
> 
> Expected results: Validate the config file only. The manual page for -N
> states:
> 
> "Do a config check.  Do NOT run in regular mode, just check configuration
> file correctness. This option is meant to verify a config file."
> 
> Additional info:
> 
> Maybe we can just avoid dropping capabilities when the -N flag is supplied?
I can bring this topic upstream, but I don't think it's a good idea.

> As a workaround, we removed the "validate: " option in our ansible, but
> that's not great as it makes things more difficult to debug.

Comment 3 Robert Muir 2023-01-06 12:56:56 UTC
> This was the whole point of introducing libcapng inside rsyslog. We do not want to give any extra capabilities to the rsyslog daemon. Isn't it possible to validate the config file before uploading it to a temporary directory? Or to validate the config as an ordinary user?

This isn't running a daemon though: it is just running a foreground process to validate the config file. As far as validating differently/with different user, that isn't how ansible works. It is one reason why I opened the bug, as I expect anyone else doing the same thing will run into it.

Again, to me it is very strange that C code is patched to drop extra capabilities: there's no control over this and no way to workaround problems with it. After debugging the issue, I was digging through rsyslog codebase trying to find where this was happening and coming up empty.

I would again recommend to instead use the systemd feature to drop capabilities (CapabilityBoundingSet), it will do the same thing, and drop capabilities for the actual running background service. Additionally it is fully transparent, and if users need to tweak the behavior it is possible for them to do so.

Comment 4 Attila Lakatos 2023-01-13 09:17:28 UTC
This was a security enhancement, so that's why there is no way to turn it off.

I gave a second thought to the list of enabled capabilities and modified it. Could you try out the latest scratch-build and let me know if that helps your use case?
Scratch-build: https://kojihub.stream.rdu2.redhat.com/koji/taskinfo?taskID=1782475

Comment 5 Robert Muir 2023-01-13 12:28:13 UTC
Sorry, the argument doesn't make sense to me.

There's plenty of systemd security controls (not just CapabilityBoundingSet, but many others around filesystem, networking, seccomp integration, etc), but redhat doesn't use them to sandbox services, instead prefers hacking C-code? Its an extremely risky approach with a high maintenance cost, upgrading packages is gonna be more painful!

Run 'systemd-analyze security' on a centos box to see what I mean. Its literally a one-liner to add CapabilityBoundingSet= to the rsyslog.service! It does the exact same thing as the C-code patch!

As far as the scratch-build, i get NXDOMAIN for kojihub.stream.rdu2.redhat.com. Am I supposed to be able to reach it?

Comment 16 Jan Pazdziora 2023-02-27 13:29:16 UTC
I'm rather unhappy about the reintroduction of CAP_DAC_OVERRIDE -- that's serious weakening of the security posture.

That

> ansible will upload the new config file to a tmp directory owned by ordinaryuser, then attempt to validate as root

approach -- is that a general Ansible pattern? Why not upload the new config file with ownership by root? If this is about validating the file in /tmp, why not just chgrp root /tmp/that-file && chmod g+r /tmp/that-file, or something similar?

Comment 17 Robert Muir 2023-02-27 13:42:07 UTC
> I'm rather unhappy about the reintroduction of CAP_DAC_OVERRIDE -- that's serious weakening of the security posture.

So please, please, please, follow my advice, and stop hacking C libraries like this, it is wrong, just use the systemd functionality to drop capabilities :)

This is a textbook case.

When I look at https://www.freedesktop.org/wiki/Software/systemd/ under "#12: Securing Your Services" it leads to this post: http://0pointer.de/blog/projects/security.html

Quoting from the post:
> Making use of systemd's CapabilityBoundingSet= option is often a simple, discoverable and cheap replacement for patching all system daemons individually to control the capability bounding set on their own.

As far as ansible...

> approach -- is that a general Ansible pattern? Why not upload the new config file with ownership by root? If this is about validating the file in /tmp, why not just chgrp root /tmp/that-file && chmod g+r /tmp/that-file, or something similar?

Thats how ansible works. I have no control over this as a user. Isn't Ansible your product? :)

Comment 19 errata-xmlrpc 2023-05-09 07:44:54 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (rsyslog bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2023:2303

Comment 20 Jonathan Kamens 2023-08-01 12:56:07 UTC
There is nothing in the release notes indicating that this issue was fixed in any way.

Either the release notes are wrong or closing this ticket was wrong.

Please either reopen this issue or explain how it was fixed.

Comment 21 Attila Lakatos 2023-08-07 09:04:26 UTC
(In reply to Jonathan Kamens from comment #20)
> There is nothing in the release notes indicating that this issue was fixed
> in any way.
> 
> Either the release notes are wrong or closing this ticket was wrong.
> 
> Please either reopen this issue or explain how it was fixed.

Check out https://bugzilla.redhat.com/show_bug.cgi?id=2216919 and https://bugzilla.redhat.com/show_bug.cgi?id=2225088