Deluge
#1473 closed patch (Fixed)
Adding support for switching uid after startup for daemons
| Reported by: | cinderblock | Owned by: | |
|---|---|---|---|
| Priority: | minor | Milestone: | 2.0.0 |
| Component: | Core | Version: | 1.3.1 |
| Keywords: | Cc: | cameron@… |
Description
I've written a patch to let deluged and deluge-web be started by root and then switch UIDs (and GIDs). The main reason for this (for me) is so they can write a pidfile (specified at the command line) as root and then switch to regular/lower privileges.
I also cleaned up the forking in deluged (before ever looking at the deluge-web source) and then saw that it was done in this much cleaner way in the deluge-web daemon.
Attachments (3)
Change History (13)
comment:1 Changed 13 years ago by cinderblock
- Cc cameron@… added
- Component changed from other to core
- Milestone changed from Future to 1.3.2
- Priority changed from major to minor
- Type changed from defect to patch
- Version changed from other (please specify) to 1.3.1
comment:2 Changed 13 years ago by damoxc
comment:3 Changed 13 years ago by cinderblock
Does just moving the imports to inside the 'if not deluge.common.windows_check():' fix that? While I write plenty of software, my python level is still at "hacker"
comment:4 Changed 13 years ago by damoxc
Yup that'll be absolutely fine. Python won't execute the code inside the if when it's running on windows, and it will be as if that if wasn't there on other platforms.
One mistake in the deluged-setuid.patch, true should be True, true will throw a name error in Python.
Also it would be quite nice if the options were only added when not running on windows too, since otherwise there are pointless options added to the output, so wrapping those in an if deluge.common.windows_check() would be excellent.
Changed 13 years ago by cinderblock
comment:5 Changed 13 years ago by cinderblock
I've moved the appropriate imports around add taken out the options on non-supported systems. This enabled some trimming of extra tests in many places.
I also applied that change to the forking command line options. They don't need to be displayed if unsupported. The one problem with this is for the daemon, if the option isn't displayed, it would default to effectively false, and then try to daemonize on non supported systems, so I had to check if running windows/osx twice.
One obvious (and probably preferred) solution would be to change the --do-not-daemonize option to --fork to match the webui. An obvious problem with that is the chance of breaking current installations.
PS: While I think this should be obvious (especially since so little was written) if there are any copyright/legal concerns with my work, the standard OSS non-liability claims apply. If there are any rights that might be mine, I release my code to the Public Domain.
comment:6 Changed 13 years ago by damoxc
Thanks a lot, I will try and get around to applying your patch to master at some point today!
comment:7 Changed 13 years ago by damoxc
- Resolution set to fixed
- Status changed from new to closed
Applied in master r9a54beef78, had to make a slight change to the deluge-web patch as deluge.common wasn't imported when the options are added, but sorted that.
comment:8 Changed 13 years ago by damoxc
- Milestone changed from 1.3.2 to 1.4.0
Sorry should probably add since the 1.3.x series is supposed to be only bug fixes I'm only going to apply this to master.
comment:9 Changed 8 years ago by Cas
- Milestone changed from 2.0.x to 2.0
We can't apply the patch in it's current state.
It can't use pwd/grp as they don't exist on Windows so will have to add some checks using deluge.common.is_windows()