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>
Switch from memcmp() to the new constant-time function to compare the
received and expected authentication data generated with a symmetric key
(NTP MAC or AES CMAC).
While this doesn't seem to be strictly necessary with the current
code, it is a recommended practice to prevent timing attacks. If
memcmp() compared the MACs one byte at a time (a typical memcmp()
implementation works with wider integers for better performance) and
chronyd as an NTP client/server/peer was leaking the timing of the
comparison (e.g. in the monitoring protocol), an attacker might be able
for a given NTP request or response find in a sequence the individual
bytes of the MAC by observing differences in the timing over a large
number of attempts. However, this process would likely be so slow the
authenticated request or response would not be useful in a MITM attack
as the expected origin timestamp is changing with each poll.
Extend the keys unit test to compare the time the function takes to
compare two identical MACs and MACs differing in the first byte
(maximizing the timing difference). It should fail if the compiler's
optimizations figure out the function can return early. The test is not
included in the util unit test to avoid compile-time optimizations with
the function and its caller together. The test can be disabled by
setting NO_TIMING_TESTS environment variable if it turns out to be
unreliable.
Log a warning message if the file specified by the keyfile or
ntsserverkey directive is world-readable or writable, which is likely
an insecure misconfiguration. There is no check of directories
containing the file.
Log important changes from chronyc for auditing purposes.
Add log messages for:
- loaded symmetric keys and server NTS keys (logged also on start)
- modified maxupdateskew and makestep
- enabled/disabled local reference mode (logged also on start)
- reset time smoothing (logged also on clock steps)
- reset sources
For consistency and safety, change the CMC and HSH functions to accept
signed lengths and handle negative values as errors. Also, change the
input data type to void * to not require casting in the caller.
The daemon transmit timestamps are precompensated for the time it takes
to generate a MAC using a symmetric key (as measured on chronyd start)
and also an average round-trip time of the Samba signing of MS-SNTP
responses. This improves accuracy of the transmit timestamp, but it
has some issues.
The correction has a random error which is changing over time due to
variable CPU frequency, system load, migration to a different machine,
etc. If the measured delay is too large, the correction may cause the
transmit timestamp to be later than the actual transmission. Also, the
delay is measured for a packet of a minimal length with no extension
fields, and there is no support for NTS.
Drop the precompensation in favor of the interleaved mode, which now
avoids the authentication delay even when no kernel/hardware timestamps
are available.
Allow a cipher (AES128 or AES256) to be specified as the type of a key
in the key file to authenticate NTP packets with a CMAC instead of the
NTPv4 (RFC 5905) MAC using a hash function. This follows RFC 8573.
Remove the magic constant compensating for copying, conversions, etc.
It cannot possibly be accurate on all hardware. The delay is supposed to
be a minimum delay.
Fix mismatches between the format and sign of variables passed to
printf() or scanf(), which were found in a Frama-C analysis and gcc
using the -Wformat-signedness option.
It was never used for anything and messages in debug output already
include filenames, which can be easily grepped if there is a need
to see log messages only from a particular file.
If the MAC in NTPv4 requests would be truncated, use version 3 by
default to avoid the truncation. This is necessary for compatibility
with older chronyd servers, which do not respond to messages with
truncated MACs.
Replace struct timeval with struct timespec as the main data type for
timestamps. This will allow the NTP code to work with timestamps in
nanosecond resolution.
After restricting authentication of servers and peers to the specified
key, a short key in the key file is a security problem from the client's
point of view only if it's specified for a source.
Consider 80 bits as the absolute minimum for a secure symmetric key. If
a loaded key is shorter, send a warning to the system log to encourage
the admin to replace it with a longer key.
This should reduce the number of possible memory leaks reported by
valgrind. The remaining reported leaks are sched tqe allocation, async
DNS instance allocation, cmdmon response/timestamp cell allocation, and
clientlog subnet allocation.
Allow different hash functions to be used in the NTP and cmdmon
protocols. This breaks the cmdmon protocol compatibility. Extended key
file format is used to specify the hash functions for chronyd and new
authhash command is added to chronyc. MD5 is the default and the only
function included in the chrony source code, other functions will be
available from libraries.
