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

Disable rights change for pdns config dir #44

Merged
merged 4 commits into from
Feb 17, 2019
Merged

Disable rights change for pdns config dir #44

merged 4 commits into from
Feb 17, 2019

Conversation

KawaiDesu
Copy link
Contributor

Do not touch right for powerdns config and leave it as in package. Because of 750 (default is 755) I unable to place additional configuration files in config directory.

@KawaiDesu
Copy link
Contributor Author

Failed build is unrelated to my PR:
Failed to fetch http://repo.powerdns.com/ubuntu/pool/main/p/pdns/pdns-server_4.1.3-1pdns.artful_amd64.deb 404 Not Found"

@atosatto
Copy link
Contributor

atosatto commented Jan 29, 2019

Yes I can confirm that the issue with the CI is not related to your PR.
We will fix the CI and ask you to rebase your work on it.

@@ -24,9 +24,6 @@
file:
name: "{{ pdns_rec_config_dir }}"
state: directory
owner: root
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we explicitely set

owner: root
group: root
mode: 0755

instead of using the defaults?

@atosatto
Copy link
Contributor

Ping @KawaiDesu :)

@atosatto
Copy link
Contributor

Just rebased @KawaiDesu's work and implemented the code-review feedback.

@atosatto
Copy link
Contributor

Thanks @KawaiDesu for digging into the file permissions set by the role.
The new permissions set by the role now makes possible to hot-reload Lua configuration and scripts files without restarting the service.

@atosatto atosatto merged commit e67f9a5 into PowerDNS:master Feb 17, 2019
@KawaiDesu
Copy link
Contributor Author

@atosatto sorry for long reaction.
My pull request was about to not touch rights at all, because package must set them as intended and there is no reason to check or even change them after installation. Maybe, this can be fully parameterized (mode, owner, group) but totally not hardcoded as in different distribution (theoretically) can be different rights in package.
Mode 0755 for /etc/powerdns fixes our problem. but as concept still doubtful.

@atosatto
Copy link
Contributor

atosatto commented Feb 19, 2019

Hi @KawaiDesu sorry for taking over your PR but I thought this was abandoned.

Mode 0755 for /etc/powerdns fixes our problem. but as concept still doubtful.

The PowerDNS Recursor is started as root of any system and then the permissions are stripped to the one of the user and group configured by the setuid and setgid options (respectively pdns_rec_user and pdns_rec_group in our Ansible role.

Setting the permissions to 755 ensure that any user and group configured as runtime user for the PowerDNS Recursor service can access the folder.
The permissions of the configuration files are then restricted in order not to allow to be read / edit by not authorized users.

Note that the permissions set by the Ansible role won't be changed upgrading the package.
You can find, as an example, the .rpmspec file used to build the PowerDNS Recursor packages on CentOS/RHEL here: https://github.com/PowerDNS/pdns/blob/master/builder-support/specs/pdns-recursor.spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants