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/rec: Drop remaining capabilities after startup #7138

Merged
merged 5 commits into from Jan 8, 2019

Conversation

Projects
None yet
2 participants
@rgacogne
Copy link
Member

rgacogne commented Nov 2, 2018

Short description

Drop any remaining capabilities after startup, for example if we have been started as root without --uid or --gid (please don't do that) or as an unprivileged user with ambient capabilities like CAP_NET_BIND_SERVICE.
This does change the existing behavior for setups not using --uid or --gid, because dnsdist used to keep CAP_NET_BIND_SERVICE around in that case, and won't after this PR is merged. The only case I can think of right now where it might matter is someone willing to start the webserver on a privileged port after startup, using the console.

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)

@rgacogne rgacogne added this to the dnsdist-1.4.0 milestone Nov 2, 2018

@rgacogne

This comment has been minimized.

Copy link
Member Author

rgacogne commented Nov 2, 2018

Added the same mechanism for the recursor.

@rgacogne rgacogne changed the title dnsdist: Drop remaining capabilities after startup dnsdist/rec: Drop remaining capabilities after startup Nov 2, 2018

@rgacogne rgacogne added the rec label Nov 2, 2018

@rgacogne rgacogne requested a review from pieterlexis Nov 5, 2018

@pieterlexis

This comment has been minimized.

Copy link
Member

pieterlexis commented Nov 5, 2018

This does change the existing behavior for setups not using --uid or --gid, because dnsdist used to keep CAP_NET_BIND_SERVICE around in that case, and won't after this PR is merged.

This should be documented somewhere indeed.

Another thing is that this capability should be added to the spec files, dockerfiles and debian/rules etc. so it is actually used in our packages (I can PR that if you want)

@rgacogne

This comment has been minimized.

Copy link
Member Author

rgacogne commented Nov 5, 2018

Another thing is that this capability should be added to the spec files, dockerfiles and debian/rules etc. so it is actually used in our packages (I can PR that if you want)

Do you know how dnsdist is currently started in our packages? With --uid dnsdist ? Do we create the dnsdist user using sysusers where available?
Same question for the rec :) I can check if you don't know, of course :)

@pieterlexis

This comment has been minimized.

Copy link
Member

pieterlexis commented Nov 5, 2018

Do you know how dnsdist is currently started in our packages? With --uid dnsdist ? Do we create the dnsdist user using sysusers where available?

  1. yes (debian, centos)
  2. no sysusers atm, they are created in the postinstall

Same question for the rec :) I can check if you don't know, of course :)

Similar answers as above :)

rgacogne and others added some commits Nov 2, 2018

Move the code using libcap to separate files
So we don't have to link every tool against libcap.

@rgacogne rgacogne force-pushed the rgacogne:dnsdist-libcap branch from 99450f6 to 88301a0 Dec 5, 2018

@rgacogne

This comment has been minimized.

Copy link
Member Author

rgacogne commented Dec 5, 2018

Rebased to fix a conflict.

@rgacogne rgacogne modified the milestones: dnsdist-1.4.0, rec-4.2.0 Dec 18, 2018

@rgacogne rgacogne modified the milestones: rec-4.2.0, rec-4.2.0-alpha1 Dec 26, 2018

@rgacogne rgacogne merged commit 374c109 into PowerDNS:master Jan 8, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rgacogne rgacogne deleted the rgacogne:dnsdist-libcap branch Jan 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.