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

enhancement: ability to set fs.suid_dumpable #7

Closed
andytson opened this issue Aug 26, 2021 · 10 comments
Closed

enhancement: ability to set fs.suid_dumpable #7

andytson opened this issue Aug 26, 2021 · 10 comments

Comments

@andytson
Copy link

Lots of applications start as root user, and setuid in order to run with lower privileges. This is usually to do with opening log files or binding to privileged ports.

Segfaults from these setuid'd processes do not create core dumps by default, and require the sysctl config fs.suid_dumpable set to 1 or 2.

Would it be possible to have an env var for the core-dump-agent to set this sysctl setting? 2 should be best, as it prevents the process being able to read it (if the host volume was even mounted in the container).

Processes that do this include:

  • nginx
  • apache
  • php-fpm
@No9
Copy link
Collaborator

No9 commented Aug 26, 2021

This is an interesting feature - Do you have a sample container definition to illustrate and test the requirement?

@andytson
Copy link
Author

andytson commented Sep 1, 2021

The following triggers a segfault after setuid, and the fs.suid_dumpable flag default of 0 provides no coredump, whereas 2 it does.

It's not otherwise as simple as su -c 'kill -11 $$' app as the segfault has to be in the setuid process rather than child process

https://gist.github.com/andytson-inviqa/e1c933ccaa1825a9c4449cbf823c3c13

@No9
Copy link
Collaborator

No9 commented Sep 7, 2021

Quick update I have an implementation of this here https://github.com/IBM/core-dump-handler/tree/suid-support
My quay.io access is broken due to the move to SSO so I need to setup another account to host the images and test.
Should get that done soon and I'll merge after that.

@No9
Copy link
Collaborator

No9 commented Sep 7, 2021

Actually setting up the new quay repo was easier than I thought
https://quay.io/repository/icdh/core-dump-handler?tab=builds

I have to wrap up for the day but you can point the chart to
quay.io/icdh/core-dump-handler:suid-support to test

@No9
Copy link
Collaborator

No9 commented Sep 8, 2021

Tested the https://github.com/IBM/core-dump-handler/tree/suid-support branch with the command

kubectl run -i -t segfaulter --image=quay.io/icdh/segfaulter --restart=Never

Where quay.io/icdh/segfaulter is based on the code initially supplied. -> https://github.com/No9/segfaulter

There was a nit in the quote escaping in the chart but now seems to work and core-dumps are generated as expected.
I'll let this sit for a few days and if there's no further feedback I'll merge

@andytson
Copy link
Author

andytson commented Sep 9, 2021

I've tested it myself and it appears to work as expected.

One comment though, but should SUID_LIMIT instead be called SUID_DUMPABLE?

As for quotes in helm, I tend to do value: {{ .VAR | quote }} as that ensures proper escaping

@andytson
Copy link
Author

andytson commented Sep 9, 2021

also thanks for the logs location fix, I was wondering why it was in /node without a container mount to view it

@No9
Copy link
Collaborator

No9 commented Sep 9, 2021

Appreciate the feedback - changed to SUID_DUMPABLE and used quote - thanks for the tip.
If there's nothing else I'll validate the file lock code I've also put in and then do a release.

@No9
Copy link
Collaborator

No9 commented Sep 9, 2021

lock code seems to be working well - Ran with 5ms intervals and the file uploads uncorrupted.

@No9
Copy link
Collaborator

No9 commented Sep 10, 2021

Merged #11 and built release v2.1.0
Thanks again @andytson for opening this issue and providing great feedback.
Please let me know if you have any other good ideas

@No9 No9 closed this as completed Sep 10, 2021
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

No branches or pull requests

2 participants