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.

Sign up for GitHub

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

Jump to bottom

CRIT Can't drop privilege as nonroot user #695

Closed
ngton opened this issue Nov 18, 2015 · 8 comments
Closed

CRIT Can't drop privilege as nonroot user #695

ngton opened this issue Nov 18, 2015 · 8 comments
Labels
docs logging question

Comments

@ngton
Copy link

ngton commented Nov 18, 2015

If I understand correctly, this is being generated because supervisord is started as a non-root user and the user parameter is specified in the config file.

As per the documentation:

user

If supervisord is run as the root user, switch users to this UNIX user account before doing any meaningful processing. This value has no effect if supervisord is not run as root.

If run as a non-root user, this parameter should not then cause a CRIT message in the logs when supervisord is working as designed.
@mnaberez
Copy link
Member

If run as a non-root user, this parameter should not then cause a CRIT message in the logs when supervisord is working as designed.

You have run supervisord as non-root. At the same time you have asked it to drop privileges by specifying user=, which it can't do because it is non-root. This is not a valid configuration.

You can comment out user= if you want to run it as the current user, and it will not log this message.

@ngton
Copy link
Author

ngton commented Nov 18, 2015

Hang on, the docs do not say this is an invalid configuration, quite the opposite. "This value has no effect if supervisord is not run as root"

Therefore it should not attempt to switch user and not generate a CRIT message. I believe this issue has been closed incorrectly.

@mnaberez mnaberez reopened this Nov 18, 2015
@mnaberez mnaberez added the docs label Nov 18, 2015
@mnaberez
Copy link
Member

I think the existing behavior is much more sane than just ignoring it would be. You asked to switch users by specifying user=, but supervisord can't switch users because it was not started as root, so you got a message logged saying it can't. If you don't want that error message, don't set user=.

It would be fine to clarify the docs to mention the log message, so I'll leave this open as a docs issue.

@ngton
Copy link
Author

ngton commented Nov 19, 2015

This isn't a docs issue.

The logical extension of what you're saying is that supervisord cannot be configured to be able to run as root and non-root. I don't think this is either intended or desirable. Why cripple functionality like that?

The real problem here is that supervisord is running wrong code path. It should not attempt to change user and should simply ignore user= when running as non-root, as per the docs.

@mnaberez
Copy link
Member

The logical extension of what you're saying is that supervisord cannot be configured to be able to run as root and non-root. I don't think this is either intended or desirable. Why cripple functionality like that?

I can't see where you are coming from with this statement. supervisord works fine when run as root or as non-root, and many people run it both ways.

If you started supervisord as non-root, you can't switch users with user= in the config file. It can't switch users because the operating system won't allow it to do that unless it is root. Remove user= and you won't get the message.

The real problem here is that supervisord is running wrong code path. It should not attempt to change user and should simply ignore user= when running as non-root, as per the docs.

Running the wrong code path would be trying to setuid when non-root and failing. It doesn't do that. If you set user= in the config file, then supervisord checks if it is running as root. If it's not, then it logs a message telling you it can't switch the user like you asked it to do.

@ngton
Copy link
Author

ngton commented Nov 19, 2015

What I mean is that what if you want to be able to run supervisord as root and non-root without configuration changes. The docs are clear that this is possible. "This value has no effect if supervisord is not run as root"

There is no way this should be CRIT message.

@mnaberez
Copy link
Member

Sorry but I'm not going to keep going around and around on this. I'm fine admitting the docs aren't perfect and can make an update to clarify that it logs this message. Your configuration file asked it to switch users, it can't because it's not root, so it logged the message saying it can't do what you asked. I'm not going to change that.

@ngton
Copy link
Author

ngton commented Nov 19, 2015

Well at least don't make it a CRIT. This doesn't require "immediate user attention". The product is working as designed

@Supervisor Supervisor locked and limited conversation to collaborators Nov 19, 2015
elgalu pushed a commit to elgalu/supervisor that referenced this issue Oct 25, 2018
Merge remote-tracking branch 'upstream/master'
4f391b3 
* upstream/master: (61 commits)
  Fix a typo
  Merge pull request Supervisor#703 from gudata/patch-1
  Add pypy3 to tox.ini and .travis.yml
  Use `make html` to build the docs under tox
  Fix package name for Sphinx
  Fix import for Mock
  Add docs to travis build
  Test that doc building + readme are correct
  Add Supervisor 3.2.0 release date to Supervisor 4 changelog
  Fix system.multicall() broken by faster start/stop patch In past versions, startProcess() and stopProcess() would always return a callback.  50d1857 changed this so they may return either a callback or a bool, but the code that handles system.multicall() was not updated.  If multicall was used with stopProcess() followed by startProcess(), it would try to start the process before it had finished stopping.  This broke the restart process link on the web interface.
  Added supervisor_checks to the docs.
  Fix typo in docs
  Clarify behavior of user= option. Closes Supervisor#695
  Fix start/stop buttons on web broken by faster start/stop patch In past versions, startProcess() and stopProcess() would always return a callback.  50d1857 changed this so they may return either a callback or a bool, but the web interface was not updated.
  Show error messages when clearing a log on the web interface
  Move code for start and stop actions near each other
  Show errors when stopping a process on the web interface
  Show string description for unexpected faults
  Show all error messages in TailView. Fixes Supervisor#627
  Implement __str__ so code and text of RPCError are logged This is helpful for troubleshooting issues like Supervisor#627 where the traceback doesn't show the contents of the RPCError.
  ...
alexsilva pushed a commit to alexsilva/supervisor that referenced this issue Jun 22, 2019
@mnaberez @alexsilva
Clarify behavior of user= option. Closes Supervisor#695
01f0f8a 
(cherry picked from commit 5bfb269)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
docs logging question
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mnaberez @ngton