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

dnsdist: don't start as root within a systemd environment #7820

Merged
merged 7 commits into from Feb 7, 2020

Conversation

pieterlexis
Copy link
Contributor

@pieterlexis pieterlexis commented May 14, 2019

Short description

With this PR, dnsdist is never started as root on Linux systems with systemd. This PR adds a configure option to set the username and group in the service file. It also removes the capabilities for dnsdist to do setuid and setgid.

Discussion:

  • Should dnsdist log a warning when run inside systemd (NOTIFY_SOCKET defined) and -u or -g is set on the command line?

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)
  • checked that this code was merged to master

Copy link
Member

@rgacogne rgacogne left a comment

Looks good!

@pieterlexis
Copy link
Contributor Author

@pieterlexis pieterlexis commented May 20, 2019

@rgacogne Do you have an opinion on

Should dnsdist log a warning when run inside systemd (NOTIFY_SOCKET defined) and -u or -g is set on the command line?

We already refuse to start the recursor then chroot is enabled, so we could something similar here

@rgacogne
Copy link
Member

@rgacogne rgacogne commented May 20, 2019

Should dnsdist log a warning when run inside systemd (NOTIFY_SOCKET defined) and -u or -g is set on the command line?

I'm fine with refusing to start in that exact case, provided that we can figure out a clear enough error message :)

@pieterlexis
Copy link
Contributor Author

@pieterlexis pieterlexis commented May 24, 2019

I think we can target this for dnsdist 1.5.0?

@rgacogne rgacogne added this to the dnsdist-1.5.0 milestone May 27, 2019
@rgacogne
Copy link
Member

@rgacogne rgacogne commented May 27, 2019

I think we can target this for dnsdist 1.5.0?

Done!

@pieterlexis pieterlexis marked this pull request as ready for review May 29, 2019
@rgacogne
Copy link
Member

@rgacogne rgacogne commented Nov 22, 2019

circleci: test-auth-regress-odbc-mssql failure is unrelated to this PR, but unfortunately I can't restart that job.

@Habbie
Copy link
Member

@Habbie Habbie commented Nov 22, 2019

circleci: test-auth-regress-odbc-mssql failure is unrelated to this PR, but unfortunately I can't restart that job.

It would not help. I'll fix it on master soon.

@Habbie
Copy link
Member

@Habbie Habbie commented Nov 22, 2019

It would not help. I'll fix it on master soon.

I fixed it on master in June. A rebase would help. We can also ignore it for this PR :)

@rgacogne rgacogne merged commit ae067ab into PowerDNS:master Feb 7, 2020
20 of 21 checks passed
@pieterlexis pieterlexis deleted the systemd-no-setuid branch Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants