Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PowerDNS running as privileged user if config file is wrong #92

Open
saz opened this issue Jan 31, 2021 · 5 comments
Open

PowerDNS running as privileged user if config file is wrong #92

saz opened this issue Jan 31, 2021 · 5 comments
Labels

Comments

@saz
Copy link
Contributor

saz commented Jan 31, 2021

As this module isn't setting setuid and setgid to an unprivileged user, PowerDNS is running as root if those options are not set (tested on Ubuntu).

Either add a message to the README of this behavior or set setuid and setgid by default.

@ju5t
Copy link
Collaborator

ju5t commented Jan 31, 2021

You're on a roll. Thanks for bringing this up.

Is this with the official packages released by PowerDNS? I'm not familiar with the build system of PowerDNS, but they do change it on CentOS and Debian Buster.

I think we should 100% warn users if this is the default behaviour for some operating systems. At the same time, if it is an official package from PowerDNS, I would probably go for a PR or bug report on their end. I wouldn't change the default because it might interfere with the package PowerDNS provides.

@saz
Copy link
Contributor Author

saz commented Jan 31, 2021

It all depends on the configuration you got on your system.

This module isn't changing existing values, but if you're - for example - migrate from something else, you might start with a non-package default config version. If this version is lacking the proper values, it's running as root.

My original finding is based on that. Starting with a clean version from a package is working as expected, but some users might still "break" the config and then it's running as root.

I'd def. go with platform specific defaults here to avoid this behavior.
Yes, this will be a breaking change, but one, which should be done.

The way how this module is managing the powerdns configuration is quite nice, but has the implication, that there are unmanaged lines within the file, which might also break during upgrades: if a valid setting gets removed, PowerDNS might complain about it and won't start, although the user is unaware of such a setting.
It even gets more complicated, as package upgrades won't replace the configuration, as there are changes in the config file.
For now, I don't have a solution for this, sadly.

@saz saz changed the title PowerDNS running as privileged user by default (tested on Ubuntu) PowerDNS running as privileged user if config file is wrong (tested on Ubuntu) Jan 31, 2021
@saz
Copy link
Contributor Author

saz commented Jan 31, 2021

As my assumption was wrong, I've changed the title.

@ju5t
Copy link
Collaborator

ju5t commented Jan 31, 2021

but has the implication, that there are unmanaged lines within the file, which might also break during upgrades

It does. It even means if a setting is invalid, it will still be added as there is no way to verify it right now (I think). If there is a way to verify it automatically, I would love to include it.

Yes, this will be a breaking change, but one, which should be done.

I thought about it and think you're right. As long as we make sure we don't break the installation of PowerDNS and stick to the defaults they use themselves, I'm up for changing it. This way it shouldn't interfere if you didn't change it. I'd also like to make this an optional feature though as there might be reasons I really can't think of right now to run it as root.

@ju5t ju5t changed the title PowerDNS running as privileged user if config file is wrong (tested on Ubuntu) PowerDNS running as privileged user if config file is wrong Jan 31, 2021
@ju5t
Copy link
Collaborator

ju5t commented Jan 31, 2021

I've removed the tested on Ubuntu part, because this could happen on any system.

@ju5t ju5t added the security label Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants