Project

General

Profile

Bug #5213

It should be possible to use "-f" even if tvheadend is started as "non-root" user

Added by M. Reimer about 6 years ago. Updated about 6 years ago.

Status:
Fixed
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Start date:
2018-09-19
Due date:
% Done:

0%

Estimated time:
Found in version:
git
Affected Versions:

Description

I want to start tvheadend directly with reduced privileges. Tvheadend doesn't need "root" privileges from the start, so it is a bit more secure to let the init system (systemd in my case) do this job.

This works great if "-f" is not used but fails if I want tvheadend to fork as soon as it is ready.

For some reason "-f" triggers tvheadend to "guess" some user and group and still try to "setuid/setgid", which will fail if tvheadend was started as "non-root".

Is this really required? And if so, why?

I can create a pull request to fix this, but decided to create the issue first to see if it makes sense to create the pull request.

Maybe also to get some feedback on how the "core team" would want me to fix this.

History

#1

Updated by Jaroslav Kysela about 6 years ago

What about to use 'Type = simple' in the systemd's config and do not use '-f'?

#2

Updated by M. Reimer about 6 years ago

I'm doing exactly this right now and it seems to work OK, so far.

I just thought it may be nice to keep the forking as systemd can use this as some kind of "ready signal" to know when the service can be expected to be "started and ready".

#3

Updated by M. Reimer about 6 years ago

I've just found the "systemd watchdog" feature, which is on GIT.

This also solves the problem of telling systemd when the service is ready, so this new feature resolves my initial problem.

As I still think that there should be no linkage between "user switch" and "forking", I would still want to leave this issue open for a developer to decide. IMHO forking should work also if the "executing user" is already a user with "reduced privileges". In this case there is no need to setuid/setgit. Just fork.

#4

Updated by Jaroslav Kysela about 6 years ago

Yes, the forking might be separated from the daemonize + priviledge handling which is combined now. If you do a patch, I'll merge it, but '-f' parameter function must be kept. Use a different parameter like '-F'.

#5

Updated by M. Reimer about 6 years ago

How about just making the "-f" parameter working in the case tvheadend was started as non-root?

Currently -f works like this:

1. Guess some user (UID 1, whatever this would be on my system) and switch to it. Then switch to group "video". Both steps will switch to a defined user/group if at least "-u" is given.
2. Next do the forking

The "problem" is this block: https://github.com/tvheadend/tvheadend/blob/master/src/main.c#L1049-L1131
which is my "step 1".

My guess is, that everything, we need, is a change in this one line:
https://github.com/tvheadend/tvheadend/blob/master/src/main.c#L1049

- if(opt_fork || opt_group || opt_user) {
+ if((opt_fork && getuid() == 0) || opt_group || opt_user) {

Or in other words: If "-f" is set, then run this block if we are root. If we are not root, then this block is not executed.

I haven't tested this, but I can do this weekend. If I'm right, then "-f" keeps its functionality if tvheadend is started as root (privileges will be dropped even if only "-f" is given) but after this change "-f" will also work if started as "non-root" user (will just fork and not try to switch user, which will fail if we are "unprivileged" already).

#6

Updated by Jaroslav Kysela about 6 years ago

Note that if you specify the user through '-u' and '-g' parameters, the priviledges should be (re)set correctly for the non-root start, too. Perhaps, we can just add a detection before the fork code for the non-root run and get uid/gid from the current task state.

#7

Updated by M. Reimer about 6 years ago

Jaroslav Kysela wrote:

Note that if you specify the user through '-u' and '-g' parameters, the priviledges should be (re)set correctly for the non-root start, too.

Yes, I've also found that. It's ugly, but it may work and I'll add it to my service file, as this provides backwards compatibility to "older" Tvheadend versions.

Perhaps, we can just add a detection before the fork code for the non-root run and get uid/gid from the current task state.

This would just add to the already existing ugliness of the code :P

My small change, proposed in #5, will just cause this whole block to be skipped in the "non-root fork" case:
https://github.com/tvheadend/tvheadend/blob/master/src/main.c#L1049-L1082

Which causes uid and pid to be kept on "-1" and so the code in this area doesn't cause errors anymore:
https://github.com/tvheadend/tvheadend/blob/master/src/main.c#L1098-L1107

#8

Updated by Jaroslav Kysela about 6 years ago

I applied your suggested code change with the better default root fork user selection to:

https://github.com/tvheadend/tvheadend/commit/d4b569a92513b0e11b709fb88121761c74262e52

#9

Updated by Jaroslav Kysela about 6 years ago

  • Status changed from New to Fixed

Also available in: Atom PDF