Opened 10 years ago

Closed 10 years ago

Last modified 3 years ago

#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)

deluged-setuid.patch (3.3 KB) - added by cinderblock 10 years ago.
Updated version that sets gid correctly and before uid
deluge-webui-setuid.patch (2.9 KB) - added by cinderblock 10 years ago.
deluge-web-setuid.patch (2.9 KB) - added by cinderblock 10 years ago.
Updated version that sets gid correctly and before uid

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 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 10 years ago by damoxc

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()

comment:3 Changed 10 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 10 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 10 years ago by cinderblock

Updated version that sets gid correctly and before uid

Changed 10 years ago by cinderblock

Changed 10 years ago by cinderblock

Updated version that sets gid correctly and before uid

comment:5 Changed 10 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 10 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 10 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 10 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 5 years ago by Cas

  • Milestone changed from 2.0.x to 2.0

comment:10 Changed 3 years ago by Cas

  • Milestone changed from 2.0 to 2.0.0

Milestone renamed

Note: See TracTickets for help on using tickets.