Project

General

Profile

Task #1370

Dropping privileges after start

Added by Alex Afanasyev over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
Core
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:

Description

This task is to provide ConfigFile option and machinery to drop privileges after start.

Configuration should go into "general" section and should include (commented out in the .sample file)

general 
{
    ; user "ndn-user"
    ; group "ndn-user"
}

History

#1 Updated by Alex Afanasyev over 5 years ago

  • Category set to Core

#2 Updated by Junxiao Shi over 5 years ago

Why command line instead of configuration file?

#3 Updated by Alex Afanasyev over 5 years ago

Oh.. I will update the spec. Managed to forget that we have the configuration file :)

#4 Updated by Alex Afanasyev over 5 years ago

  • Description updated (diff)

#5 Updated by Anonymous over 5 years ago

  • Status changed from New to In Progress
  • Assignee set to Anonymous

#6 Updated by Alex Afanasyev over 5 years ago

Just in case. This should not be the default settings (it will be default for packaged binaries).

#7 Updated by Anonymous over 5 years ago

What are default settings/behavior for building from source?

Also, is it correct for ps -eaf | grep nfd to show two nfd processes after the seteuid + setegid have been executed? (On OS X 10.9.2, started as sudo ./build/nfd)

  0 83900 81597   0 12:09PM ttys002    0:00.02 sudo ./build/nfd
  501 83901 83900   0 12:09PM ttys002    0:00.05 ./build/nfd

seteuid stuff is new to me, so this seems strange.

#8 Updated by Alex Afanasyev over 5 years ago

Unless somebody uncomments group and/or user in the config file, the privilege drop should not happen. The default is basically to give a commented out group and user, as in the task description.

I have no experience with seteuid/setegid, so somebody else should clarify that. I suppose that it is fine.

#9 Updated by Davide Pesavento over 5 years ago

Steve DiBenedetto wrote:

Also, is it correct for ps -eaf | grep nfd to show two nfd processes after the seteuid + setegid have been executed? (On OS X 10.9.2, started as sudo ./build/nfd)

  0 83900 81597   0 12:09PM ttys002    0:00.02 sudo ./build/nfd
  501 83901 83900   0 12:09PM ttys002    0:00.05 ./build/nfd

I don't see two nfd processes... I see one sudo and one nfd, which looks fine since you said you launched sudo ./build/nfd.

#10 Updated by Anonymous over 5 years ago

Sounds like I was misinterpreting the output. Thanks for the clarification.

The current WIP behavior is:

  1. no general section (or has general section, no user and no group) -> continue with current privileges
  2. user and group are specified -> seteuid and setegid
  3. user is specified, no group -> seteuid to user, setegid to group with same name as user
  4. no user, group is specified -> exception (do we want to allow this?)

Item 3 will fail on OS X because users don't belong to a group of the same name (looks like it's staff instead). Is this the desired behavior? Also, the usage of seteuid/setegid is based on Junxiao's last comment on #1332. Is this still what we want (as opposed to setuid/setgid)?

#11 Updated by Alex Afanasyev over 5 years ago

For 3 and 4 I would suggest different way. If not specified, just don't set it. We shouldn't infer anything about the group name if it is not specified. Also, if only group specified, why this should be an error?

#12 Updated by Anonymous over 5 years ago

Just being overzealous. I'll go with your way.

As for setuid vs seteuid: it seems like the difference is whether we intended to permanently (setuid) or temporarily drop privileges (seteuid). Is there any reason for us to only drop privileges temporarily?

#13 Updated by Alex Afanasyev over 5 years ago

Yes, there is a reason. Some operations may require root privileges in the future (e.g., rescanning interfaces and starting yo work with new ethernet device when it becomes available). If we drop privilege permanently, we will require restart whenever such action is required, which is very undesirable.

#14 Updated by Anonymous over 5 years ago

  • Status changed from In Progress to Code review

#15 Updated by Junxiao Shi over 5 years ago

Suggested API:

// daemon/core/privilege-helper.hpp
class PrivilegeHelper
{
public:
  void
  setConfig(...)

  void
  drop();

  void
  runElevated(boost::function<void()> f);
};

runElevated should have try-finally to ensure privilege is dropped even if there's an exception.

#16 Updated by Davide Pesavento over 5 years ago

Looks good. The class (or runElevated at least) could be made static so that it's easier to use throughout the codebase. The user/group -> uid/gid resolution should be done only once, while reading the config file.

#17 Updated by Anonymous over 5 years ago

Currently, user and group are specified in the general section of the configuration file. Should they be moved to their own section (runtime_privileges?) or should the privilege helper handle the general section (and renamed appropriately)?

#18 Updated by Junxiao Shi over 5 years ago

It doesn't look nice to have too many sections. Add mgmt/general-config-section.hpp (which contains a function, not a class) to process the general section, and calls a method on PrivilegeHelper static class to pass the user and group as string.

#19 Updated by Davide Pesavento over 5 years ago

Junxiao Shi wrote:

Add mgmt/general-config-section.hpp (which contains a function, not a class) to process the general section

Such a function could be put directly in mgmt/config-file.{cpp,hpp} in my opinion.

#20 Updated by Anonymous over 5 years ago

  • Status changed from Code review to Closed
  • % Done changed from 0 to 100

Also available in: Atom PDF