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

Fixed security issue with config file #33

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

jkellerer
Copy link
Contributor

This PR fixes a security issue with supervisord(is logged when used in latest Ubuntu releases).

Resolved problems:

  • Prevent supervisord from searching for config files at standard paths by specifying the config to use.
  • Create a complete config without unnecessary remote-control interfaces that are initialized when keeping the config from the distribution.
  • Finally, tell supervisord that running as root is intended and no warning must be logged (after fixing the issues).

@Fmstrat
Copy link
Owner

Fmstrat commented May 12, 2023

@jkellerer I know it's been a while, but could you rebase against develop and resolve conflicts? Everything's been updated now to the latest ubuntu and samba.

@jkellerer jkellerer force-pushed the fixed-supervisord-security-warning branch from f0bda18 to a16decf Compare May 13, 2023 12:06
@jkellerer
Copy link
Contributor Author

@Fmstrat , rebase is done

@jkellerer jkellerer force-pushed the fixed-supervisord-security-warning branch from a16decf to 4adf67a Compare May 13, 2023 12:12
@Fmstrat
Copy link
Owner

Fmstrat commented May 15, 2023

I'm not sure what security implication this has from reviewing it, but I do see you've replaced logs with stdout. If this is the case, which I'm good with, shouldn't line 182 and any other line like with tail change?

@jkellerer
Copy link
Contributor Author

jkellerer commented May 15, 2023

The security implications are listed in the description of the PR. It was about not specifying the config file in the call which means supervisor will search at default locations and use some files provided by the distribution. E.g. they enable remote control which is something not needed for this case.

Regarding log redirection, yes I agree, it should not be sent to a file at all. I can update this part.

@jkellerer
Copy link
Contributor Author

@Fmstrat, log output should now only be stdout or err (no files). I had added env var LOGLEVEL to override defaults (or smb.conf).

@jkellerer jkellerer force-pushed the fixed-supervisord-security-warning branch from ae3815d to 612b472 Compare May 20, 2023 16:02
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