Opened 8 years ago

Closed 8 years ago

#1508 closed task (complete)

Move dropping root into sockcreator startup

Reported by: vorner Owned by: vorner
Priority: very high Milestone: Sprint-20120110
Component: ~Boss of BIND (obsolete) Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: Core Feature Depending on Ticket: Socket creator
Estimated Difficulty: 4 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

Once the #805 is finished, both auth and resorver will start using socket creator as a side effect of the change and therefore won't need to start as root. So the dropping of root privileges in boss can happen immediately after the socket creator is started, probably as part of the isc.bind10.special_component.SockCreator? class. The isc.bind10.special_component.SetUID will no longer be needed.

The effect (unknown to me) to the dhcp side should be checked first.

Subtickets

Change History (12)

comment:1 Changed 8 years ago by jinmei

I'm afraid DHCPv4 will need more power than what socket creator
(currently) provides. We'll need to be able to get access to
something like BPF.

comment:2 Changed 8 years ago by jelte

  • Estimated Difficulty changed from 0 to 4

comment:3 Changed 8 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20120110

comment:4 Changed 8 years ago by jelte

  • Priority changed from major to blocker

comment:5 Changed 8 years ago by vorner

  • Owner set to vorner
  • Status changed from new to accepted

comment:6 Changed 8 years ago by vorner

  • Owner changed from vorner to UnAssigned
  • Status changed from accepted to reviewing

Hello

The branch is ready for review. I did run systest, but due to a problem with dhcp tests on my machine, I was unable to run complete tests (which I hope will solve itself before merging to master).

I'd wait for the merge so it happens together with #1509 and #1510 (and maybe even #805).

The proposed changelog is:

[func]*		vorner
The workaround with setuid component in configuration is no longer needed
and the component was removed. If you have your own configuration, make
sure to remove it, or bind10 will refuse to start.

comment:7 Changed 8 years ago by jinmei

The code seems to do what it seems to intend to do. But I have some
higher level comment.

BIND10_SETUID is now used far from it's defined. It's also not very
obvious that it changes the UID of boss itself now it's done in
something named "SockCreator?". Can't all of these be done in the
main boss code?...and, thinking about it from this point, I now
personally think the idea of pseudo "setuid" component wasn't that
bad. Whether or when to change uid is in itself independent from
the sock creator component; it's the business of the main boss
logic. So, rather than pushing this logic to SockCreator?, it seems
to me to make more sense if we separate it from SockCreator? and let
the (main) boss control whether and how to do it. (Keep) using th
SetUID component is one way to do that; on looking at the code again,
I also wonder whether we could do it more explicitly (and separately
from the component framework) around start_all_components. That is,
Rather than starting all core components at once:

        # Start the real core (sockcreator, msgq, cfgmgr)
        self._component_configurator.startup(self.__core_components)

we might make it one step more gradual: first start the socket
creator; then the boss explicitly change uid (if specified so by -u);
then start the rest of the core components.

comment:8 Changed 8 years ago by jinmei

  • Owner changed from UnAssigned to vorner

comment:9 follow-up: Changed 8 years ago by vorner

  • Owner changed from vorner to jinmei

Hello

Considering the code will disappear soon (as we talked about today, when we make the socket creator setuid-root, it won't be needed), should I write a version that moves the pseudocomponent, or should I merge it like it is? It still should be merged (or changed), otherwise the other two tickets will make everything run as root for now. I don't like the idea with yet more granular startup, the boss should know as little about the components as possible.

Thank you

comment:10 in reply to: ↑ 9 Changed 8 years ago by jinmei

Replying to vorner:

Considering the code will disappear soon (as we talked about today, when we make the socket creator setuid-root, it won't be needed), should I write a version that moves the pseudocomponent, or should I merge it like it is? It still should be merged (or changed), otherwise the other two tickets will make everything run as root for now.

I thought we'd still keep the -u option for the boss (so we can still
run it as a non root even if the OS's startup framework doesn't
support the ability to specify the user explicitly, without requiring
the administrator to prepare a special script or something). Would we
do this in the very initialization part of the boss then (which is
fine to me btw)?

Assuming we provide the capability of -u somehow, and the "workaround"
of this branch will be removed as a result of that, I'm okay with that.

I don't like the idea with yet more granular startup, the boss should know as little about the components as possible.

I disagree: IMO where to change the user (when necessary) should be a
decision at a higher level than a particular component (in this case,
which is specifically the socket creator component). That would be
the main boss itself if we hardcode these special components as we do
now; if we even fully generalize the special things, that would be a
matter of user configuration.

But, depending on how we do -u in the post setuid era, the
disagreement on this point won't be an issue any more. And, if so,
I'd simply drop this part of this discussion.

comment:11 Changed 8 years ago by jinmei

  • Owner changed from jinmei to vorner

comment:12 Changed 8 years ago by vorner

  • Resolution set to complete
  • Status changed from reviewing to closed

Thanks, merged, closing.

Note: See TracTickets for help on using tickets.