Use lchown(), the safer variant of chown() that does not follow
symlinks, when changing the ownership of a created directory (logdir,
dumpdir, ntsdumpdir, and the directory of bindcmdaddress) to the chrony
user.
Fix one of the sizeofs in open_unix_socket() to correctly specify
sock_dir2 instead of sock_dir1. They have the same size, but don't rely
on that.
Fixes: 90d808ed28 ("client: mitigate unsafe permissions change on chronyc socket")
The recent rework of refclock reachability to better work with
driver-specific filtering (PHC driver dropping samples with unexpected
delay) introduced an issue that a PPS refclock is indicated as reachable
even when its "lock" refclock is permanently unreachable, or its samples
constistently fail in other sample checks, and no actual samples can be
accumulated. This breaks the new maxunreach option.
Rework the refclock code to provide samples from drivers together with
their quality level (all drivers except PHC provide samples with
constant quality of 1) and drop samples with quality 0 after passing
all checks, right before the actual accumulation in the median sample
filter. Increment the reachability counter only for samples that would
be accumulated.
This fixes the problem with refclocks indicated as reachable when their
samples would be dropped for other reasons than the PHC-specific delay
filter, and the maxunreach option can work as expected.
Fixes: b9b338a8df ("refclock: rework update of reachability")
Modify the HCL_ProcessReadings() function to try to always provide
a valid sample. Instead of dropping a sample outside of the expected
delay, provide its assumed quality level as a small integer (relative to
already accumulated samples), and let the caller decide what quality is
acceptable.
In version 6.15 the Linux kernel started checking write access on the
PHC file descriptor in the PTP_PIN_SETFUNC and PTP_EXTTS_REQUEST ioctls.
chronyd opened the PHC device as readonly, which caused the PHC refclock
driver configured with the extpps option to fail with the
"Could not enable external PHC timestamping" error message.
To ensure compatibility with new kernel versions, add flags to the
SYS_Linux_OpenPHC() function and open the device with the O_RDWR flag
when the extpps option is enabled.
The TLS_CreateInstance() function handles a NULL alpn_name, but the
other session functions would crash if it was NULL. Change the function
to not handle the NULL for consistency and avoid potential confusion.
Fixes: 3e32e7e694 ("tls: move gnutls code into tls_gnutls.c")
Add an additional parameter to TLS_CreateInstance() to save the label of
the connection (server name on the client side and client IP
address:port on the server side) instead of the server name (which is
NULL on the server side) to fix the log messages.
Fixes: 3e32e7e694 ("tls: move gnutls code into tls_gnutls.c")
Don't assume gnutls_init() leaves the session pointer at NULL when it
returns with an error status. It might be a session that was already
allocated and then freed without resetting it to NULL after an error.
Fixes: 3e32e7e694 ("tls: move gnutls code into tls_gnutls.c")
Add maxunreach option to NTP sources and refclocks to specify the
maximum number of polls that the source can stay selected for
synchronization when it is unreachable (i.e. no valid sample was
received in the last 8 polls).
It is an additional requirement to having at least one sample more
recent than the oldest sample of reachable sources.
The default value is 100000. Setting the option to 0 disables selection
of unreachable sources, which matches RFC 5905.
To minimize the impact of potential attacks targeting chronyc started
under root (e.g. performed by a local chronyd process running without
root privileges, a remote chronyd process, or a MITM attacker on the
network), add support for changing the effective UID/GID in chronyc
after start.
The user can be specified by the -u option, similarly to chronyd. The
default chronyc user can be changed by the --with-chronyc-user
configure option. The default value of the default chronyc user is
"root", i.e. chronyc doesn't try to change the identity by default.
The default chronyc user does not follow the default chronyd user
set by the configure --with-user option to avoid errors on systems where
chronyc is not allowed to change its UID/GID (e.g. by a SELinux policy).
Drop the SCK_FLAG_ALL_PERMISSIONS support from the socket code.
chronyc is now calling chmod() on its socket itself in a hidden
directory to mitigate the unsafe operation.
When chronyc running under root binds its Unix domain socket, it needs
to change the socket permissions in order for chronyd running without
root privileges to be able to send a response to the socket.
There is a race condition between the bind() and chmod() calls. If an
attacker was able to execute arbitrary code in the chronyd process, it
might be able to wait for chronyc to be executed under root, replace the
socket with a symlink between the two calls, and cause the privileged
chronyc process to change permissions of something else, possibly
leading to a privilege escalation.
There doesn't seem to be a safe and portable way to change the socket
permissions directly. Changing the process umask could be problematic in
future with threads.
Hide the socket in two levels of subdirectories (the lower one having
a randomly generated name and not visible to the chronyd process) to
make the socket path unpredictable, and force the bind() or chmod() call
to fail if the visible upper directory is replaced.
Reported-by: Matthias Gerstner <mgerstner@suse.de>
When UTI_OpenFile() is removing an existing file to be replaced by a new
file, it could potentially get stuck in an infinite loop if something
was able to consistently win the race and create a new file before
chronyd.
Log a warning message after 100 failed attempts and repeat on each 10x
increase to make it more obvious to the admin, if it ever happens.
Reported-by: Eric Sesterhenn <eric.sesterhenn@x41-dsec.de>
After (re)loading symmetric NTP keys from the key file, there is an
attempt to erase the strings from the stack by calling memset() on the
buffer. However, compilers are free (and have been shown to do) optimize
this call out.
Remove the memset() call to not pretend the stack cannot not contain any
sensitive information. There is no such attempt made for the server and
client NTS keys.
Reported-by: Eric Sesterhenn <eric.sesterhenn@x41-dsec.de>
fgets returns either a valid pointer with the same value as its first
argument or NULL on error or EOF.
GCC 12.2.0 -Wextra warns against relational comparison of the return
value:
leapdb.c:127:38: warning: ordered comparison of pointer with integer zero [-Wextra]
For clarity, and because the C standard doesn't mandate that valid pointers
have to compare greater than the null pointer constant, replace the
relational expression with an equality expression
Currently nts_ke_session.c directly calls into gnutls.
This patch moves the calls to gnutls into tls_gnutls.c with an API
defined in tls.h. This way it becomes possible to use different TLS
implementations in future patches.
Signed-off-by: Anthony Brandon <anthony@amarulasolutions.com>
Linux 2.6.39 was released in 2011.
Refuse to start if a kernel version before 2.6.39 is detected. Assume
the ADJ_SETOFFSET adjtimex mode is always supported. Its verification
briefly reset the timex maxerror value to 0, which possibly confused
applications checking the value at that moment.
Drop the unneeded workaround for slow frequency updates in versions
2.6.27-2.6.32.
Add a note that three servers is the generally recommended minimum for
an NTP client to be able to detect a falseticker. Mention that the pool
directive uses four servers. Update the links to the pool join page and
list of public servers.
If the RTC file descriptor was closed and removed after a read error,
don't try to close and remove it again in the driver finalization to
avoid an assertion failure on the negative descriptor.
Fixes: 4f22883f4e ("refclock: add new refclock for RTCs")
When logging to stderr, don't close it in finalization in case something
else still wanted to write to it. Leave it as it is together with stdin
and stdout.
The sourcedir reload triggered by the chronyc "reload sources"
command incorrectly assumed that NSR_AddSourceByName() can return
only the NSR_Success status when a source is added. It ignored the
NSR_UnresolvedName status returned for a source whose name needs to
be resolved after the call (i.e. not specified with an IP address)
and added the source again, effectively multiplying it if the name
can be resolved to a different IP address.
Fix the code to check for the NSR_UnresolvedName status to correctly
determine whether the source was already added before and should not be
added again.
Reported-by: MichaelR <MichaelR42@runbox.com>
Fixes: 916ed70c4a ("conf: save source status in sourcedir reload")
Mention the NOTIFY_SOCKET variable to make it more obvious what is
preventing chronyd from starting in case it's unexpectedly inherited in
a chroot etc.
Modify the code to avoid making the following calls incorrectly reported
as important findings by the coverity static analyzer:
- memset() of size 0 at the end of an array
- mktime() on a struct tm that has uninitialized tm_yday
On some systems (e.g. FreeBSD) the source Unix domain socket path
provided by recvmsg() as msg_name is not always null-terminated even if
more space than required for sockaddr_un is provided due to the padding
in the sockaddr_all union, and the returned msg_namelen value does not
indicate it is missing the termination. If a cmdmon client bound its
socket to a maximum-length path (chronyc doesn't allow that), the path
would be overread when printing a debug message and trying to send a
response.
Drop messages from paths not shorter than sun_path to avoid working with
un-printf()able and/or unreachable addresses. The clients are expected
to not use the maximum-length paths.
If a clock step enabled by the makestep directive or requested by the
makestep command fails, accumulate the missing step back to keep the
tracking offset valid.
This fixes time served by an instance configured with the makestep
directive and the -x option (the null driver cannot perform steps) at
the same time. It will still generate error log messages.
