I reviewed chrony version 3.2-1build1 as checked into bionic. This isn't a
full security audit but rather a quick gauge of maintainability.
- There are ten CVEs in our database; the fixes mostly aren't enumerated
in our database, but many of the descriptions sound like they were
handed out 'conservatively' -- errors in administration-level command
channel or a malicious peer server operator in a position to interpose
traffic from another peer server.
I like the paranoia.
- chrony is a new, simpler, smaller, safer, ntp daemon. It's suitable for
client and server use, and supports some hardware drivers, NIC
timestamping, but perhaps not as many features as our old NTPD.
- libtomcrypt dependency is being worked on; apparently nss is an option
once we expose an "internal only" library.
- Does daemonize, nicely
- pre/post inst/rm scripts have autogenerated sections. Also:
- postinst script creates _chrony user and group, chowns /var/log/chrony
and /var/lib/chrony
- postinst cleans up after previous version "key" file (authentication has
been simplified in newer versions) in a complicated set of comparisons
- postrm removes /var/lib/chrony/, /etc/chrony/, _chrony user and group
- Initscript uses start-stop-daemon to start chrony
- systemd unit file is simple
- No dbus services
- No setuid files
- chronyc and chronyd executables in PATH
- No sudo fragments
- No udev rules
- test suite run at build; not comprehensive, but nice to have
- clean build logs
- sendmail is spawned to send mail via popen(). All variables are under
control of configuration file. No error handling in case the admin sets
the "mail to" variable to something silly long or dangerous, but this is
very low risk.
- Memory management looked careful
- file io looked careful
- logging looked careful
- TZ environment variable used to gather information on leap seconds,
looked careful
- Privileged operations looked careful
- I did not inspect cryptography
- Privileged portions of the code, privsep-style, looked careful; I did
not inspect privsep for safety
- Extensive networking, looked careful
- No temporary file handling
- No WebKit
- No JavaScript
- No PolicyKit
- Clean cppcheck
Errors are checked religiously, coding style is unique and awkward but not
a real impediment to maintenance. Obviously ntp is an involved protocol
and probably further flaws will be found -- and we will rely upon
upstream's help for all but the simplest of issues. It looks
professionally programmed.
The only issue I found has no security relevance but may be slightly
surprising:
- reference() uses snprintf() to build a string to call sendmail; the
username may not fit in the allocated space, and the code gets no
warning about this.
Any shell metacharacters in this setting would interfere with proper
operation of the program.
I'd like to see this addressed for reliability reasons but it's not a
pressing issue.
I reviewed chrony version 3.2-1build1 as checked into bionic. This isn't a
full security audit but rather a quick gauge of maintainability.
- There are ten CVEs in our database; the fixes mostly aren't enumerated level command
in our database, but many of the descriptions sound like they were
handed out 'conservatively' -- errors in administration-
channel or a malicious peer server operator in a position to interpose
traffic from another peer server.
I like the paranoia.
- chrony is a new, simpler, smaller, safer, ntp daemon. It's suitable for
client and server use, and supports some hardware drivers, NIC
timestamping, but perhaps not as many features as our old NTPD.
- Build-Depends: debhelper, bison, libedit-dev, libtomcrypt-dev,
libcap-dev, pps-tools, libseccomp-dev, pkg-config, asciidoctor
- libtomcrypt dependency is being worked on; apparently nss is an option
once we expose an "internal only" library.
- Does daemonize, nicely
- pre/post inst/rm scripts have autogenerated sections. Also:
- postinst script creates _chrony user and group, chowns /var/log/chrony
and /var/lib/chrony
- postinst cleans up after previous version "key" file (authentication has
been simplified in newer versions) in a complicated set of comparisons
- postrm removes /var/lib/chrony/, /etc/chrony/, _chrony user and group
- Initscript uses start-stop-daemon to start chrony
- systemd unit file is simple
- No dbus services
- No setuid files
- chronyc and chronyd executables in PATH
- No sudo fragments
- No udev rules
- test suite run at build; not comprehensive, but nice to have
- clean build logs
- sendmail is spawned to send mail via popen(). All variables are under
control of configuration file. No error handling in case the admin sets
the "mail to" variable to something silly long or dangerous, but this is
very low risk.
- Memory management looked careful
- file io looked careful
- logging looked careful
- TZ environment variable used to gather information on leap seconds,
looked careful
- Privileged operations looked careful
- I did not inspect cryptography
- Privileged portions of the code, privsep-style, looked careful; I did
not inspect privsep for safety
- Extensive networking, looked careful
- No temporary file handling
- No WebKit
- No JavaScript
- No PolicyKit
- Clean cppcheck
Errors are checked religiously, coding style is unique and awkward but not
a real impediment to maintenance. Obviously ntp is an involved protocol
and probably further flaws will be found -- and we will rely upon
upstream's help for all but the simplest of issues. It looks
professionally programmed.
The only issue I found has no security relevance but may be slightly
surprising:
- reference() uses snprintf() to build a string to call sendmail; the
username may not fit in the allocated space, and the code gets no
warning about this.
Any shell metacharacters in this setting would interfere with proper
operation of the program.
I'd like to see this addressed for reliability reasons but it's not a
pressing issue.
Security team ACK for promoting chrony to main.
Thanks