Re: [chrony-dev] [PATCH] MacOSX - Drop root privileges

[Bryan Christianson <bryan@xxxxxxxxxxxxx>]

> On 5/11/2015, at 8:31 PM, Miroslav Lichvar <mlichvar@xxxxxxxxxx> wrote:
> 
> On Thu, Nov 05, 2015 at 06:02:14PM +1300, Bryan Christianson wrote:
>> PRV_BindSocket() has to create the socket, unless we create the socket in the daemon, pass it in, bind() and then send it back. I'm not sure sending the socket in 2 directions is a good idea. That's why I call nio_bind_socket() from the privileged helper rather just factoring out the call to bind().
> 
> Hm, why would the descriptor need to be sent back from the helper
> after binding? If possible I'd rather keep the creation of the socket
> in the main process and send it to the helper just for the binding
> operation.

OK - I'll try this and let you know how it goes.

>>> I'd rather see the selection between making a call to the privileged
>>> process or calling the function directly in PRV_BindSocket() and
>>> ntp_io.c always calling that function.
>> 
>> Should the code I factored out in ntp_io.c as nio_bind_socket() be moved to privops.c ? I had thought it was better to leave it where it was and just expose it through PRV_bind_socket.
> 
> If it's really not possible to have a privileged call that binds only
> and does nothing else, then I guess yes, the code would be easier to
> read I think. But the function name would need to be different, it
> does much more than just binding.

Lets see if I can get bind(socket_from_parent, ...) to work first

> 
>> We do have the case where some of the function arguments are optionally NULL (adjtime() and settimeofday()). Currently I set up a single operator for each function call, but maybe I should use a separate operator for each combination of args. The union for adjtime would then have either one or two struct timeval members.
>> 
>> e.g. 
>> 	op_ADJTIME_first ==> adjtime(tv, NULL);
>> 	op_ADJTIME_second ==> adjtime(NULL, res); // not on MacOS - it segfaults
>> 	op_ADJTIME_both ==> adjtime(tv, res);
> 
> Would the second form need to be run in the helper? I think at least
> on Linux it doesn't require root privileges as it's a read-only
> operation. The first one is a special case of the third, so I'd just
> add one operator for the third case and ignore the result if the
> caller passed NULL.

Second case is not relevant to MacOS so I'll leave it for now and implement it if it is actually required,

> 
>> and similarly for settimeofday();
> 
> I think it's perfectly find to support only the case when tv is not
> NULL and tz is NULL.
> 
> You can add assert() calls for the unsupported cases to make it clear
> they are not supported and abort cleanly if some code tries to use
> them.

Cool - that makes life easier :)

Thanks for your help

--
Bryan


--
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.