Skip to content

Conversation

bukka
Copy link
Member

@bukka bukka commented Dec 1, 2019

Re-introduced CapabilityBoundingSet that got removed in 7.4: 67cd427 as it was breaking apps. It looks that chroot has been also omitted and it needs to be verified again what is actually needed by FPM. As such this targets master only.

Any feedback for additional capabilities is welcome.

@SjonHortensius
Copy link
Contributor

This looks fine and I wouldn't hesitate too long before putting it in 7.4(.1) again like this

@@ -32,6 +32,9 @@ NoNewPrivileges=true
# but no physical devices such as /dev/sda.
PrivateDevices=true

# Required for dropping privileges for running as a different user and changin owner and root.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Required for dropping privileges for running as a different user and changin owner and root.
# Required for dropping privileges for running as a different user and changing owner and root.

@SjonHortensius
Copy link
Contributor

Actually - CAP_KILL is also required for workers that run under a different user for fpm to reload properly. As this is hard to debug it might be useful to add that by default

@nikic
Copy link
Member

nikic commented Dec 9, 2019

I think at this point it's better to just not add this.

@SjonHortensius
Copy link
Contributor

@nikic I disagree, are you afraid there will be many more bug-reports caused by this more secure-by-default setup? Maybe documenting this in the Migrating-to-7.4 docs with possibly a link to How to find out what linux capabilities a process requires to work? is a better method of preventing that?

@bukka
Copy link
Member Author

bukka commented Dec 10, 2019

This won't go to 7.4. It's just for master.

@iluuu1994
Copy link
Member

@bukka What's the state here?

@bukka
Copy link
Member Author

bukka commented Dec 20, 2022

I have got this on my list and will eventually pick it up.

@krakjoe
Copy link
Member

krakjoe commented Aug 31, 2025

@bukka I can't find any explanation of why this broke apps in the first place, and wouldn't if it were added again. If this is still applicable can you ensure this is committed with a reasonably complete explanation of what went wrong in the first place and how we are doing things differently.

If this is not applicable any longer or you otherwise don't intend to merge this ever, please close the PR.

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

Successfully merging this pull request may close these issues.

5 participants