Re: [chrony-dev] [PATCH] Privilege Separation - Add helper process

[Bryan Christianson <bryan@xxxxxxxxxxxxx>]

> On 13/11/2015, at 10:44 PM, Miroslav Lichvar <mlichvar@xxxxxxxxxx> wrote:
> 
> On Sat, Nov 07, 2015 at 07:33:00AM +1300, Bryan Christianson wrote:
>> Privileged helper that will perform adjtime(), settimeofday(), bind() on
>> behalf of chronyd when running as non-root user.
> 
> 
> 
>> +static int
>> +getportfromaddress(const struct sockaddr *address, socklen_t address_len, int *ip_port)
> 
> There is a similar function for that, UTI_SockaddrToIPAndPort.

I looked at UTI_SockaddrToIPAndPort but it assumes that address_len is OK. Since we're replacing a direct call
to bind() it seemed I should also check the length. Although I guess there is very little risk that the supplied value of address_len would be incorrect so no problem to avoid duplicate code.

> 
>> 
>> if (port != 0 && port != CNF_GetNTPPort()) {
>>   return 0;
>> }
> 
> This should be checked in the helper. The main process could be
> compromised.

I wasn't sure if CNF_GetNTPPort() could be changed from chronyc. If it can be, then its value will have be added to the helper request.

> 
>> +int
>> +PRV_BindSocket(int sock, const struct sockaddr *address, socklen_t address_len)
>> +{
>> +  priv_request_t req;
>> +  priv_response_t res;
>> +  int port;
>> +
>> +  if (!getportfromaddress(address, address_len, &port)) {
>> +    LOG_FATAL(LOG_PrivOps, "PRV_BindSocket, invalid socket address");
>> +  }
>> +
>> +  if (!havehelper() || port == 0 || port >= 1024) {
>> +    /* dont use the helper if not required */
> 
> Is that 1024 a safe assumption? Considering it's not a time critical
> code, maybe always call the helper if it's running and later modify
> ntp_io to not call PRV_BindSocket for client-only sockets?

1-1023 used to be the range of reserved ports requiring privilege for binding. Makes sense to avoid calling the helper if we don't need to. However, no problem to drop the check.

-- 
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.