| Re: [chrony-dev] [PATCH] Privilege Separation - Add helper process |
[ Thread Index |
Date Index
| More chrony.tuxfamily.org/chrony-dev Archives
]
On Sat, Nov 14, 2015 at 12:10:18AM +1300, Bryan Christianson wrote:
> > 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.
The sa_len value needs to be checked if it's not larger than
the sa field of the request struct, that's a good point. The call to
UTI_SockaddrToIPAndPort to extract the port number should be always
safe as it points to the struct allocated on the stack and its size is
constant.
> >> 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.
It can't be changed in runtime. If it was there would be no point in
checking the number in the helper process. The idea was to prevent an
attacker who is executing arbitrary code in the main process from
binding to a different privileged port.
> > 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.
I wasn't sure if it is always 1024 and it couldn't be configured on
some systems. Anyway, to me it looks like an unnecessary complication
in the code that's not worth saving the call to the helper.
--
Miroslav Lichvar
--
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.