From 5a539a9d5c93ebec67d9d87e3d1580e09ca7d9f9 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 28 May 2024 19:49:15 +0200 Subject: [PATCH 01/46] Add NTP-server/client implementation Signed-off-by: DL6ER --- src/CMakeLists.txt | 2 + src/api/docs/content/specs/config.yaml | 26 ++ src/args.c | 12 + src/config/config.c | 32 ++ src/config/config.h | 11 + src/config/dnsmasq_config.c | 9 + src/dnsmasq_interface.c | 7 +- src/ntp/CMakeLists.txt | 19 ++ src/ntp/client.c | 226 +++++++++++++++ src/ntp/ntp.h | 29 ++ src/ntp/server.c | 387 +++++++++++++++++++++++++ test/pihole.toml | 20 ++ test/test_suite.bats | 20 +- 13 files changed, 793 insertions(+), 7 deletions(-) create mode 100644 src/ntp/CMakeLists.txt create mode 100644 src/ntp/client.c create mode 100644 src/ntp/ntp.h create mode 100644 src/ntp/server.c diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c040c688..cdfe9493 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -274,6 +274,7 @@ add_executable(pihole-FTL $ $ $ + $ ) if(STATIC) set_target_properties(pihole-FTL PROPERTIES LINK_SEARCH_START_STATIC ON) @@ -314,6 +315,7 @@ add_subdirectory(tre-regex) add_subdirectory(syscalls) add_subdirectory(config) add_subdirectory(tools) +add_subdirectory(ntp) find_library(LIBREADLINE NAMES libreadline${CMAKE_STATIC_LIBRARY_SUFFIX} readline) find_library(LIBHISTORY NAMES libhistory${CMAKE_STATIC_LIBRARY_SUFFIX} history) diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index 0b785754..853c6450 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -324,6 +324,25 @@ components: type: array items: type: string + ntp: + type: object + properties: + ipv4: + type: object + properties: + active: + type: boolean + address: + type: string + x-format: ipv4 + ipv6: + type: object + properties: + active: + type: boolean + address: + type: string + x-format: ipv6 resolver: type: object properties: @@ -656,6 +675,13 @@ components: hosts: - "11:22:33:44:55:66,192.168.1.123" - "11:22:33:44:55:67,192.168.1.124,hostname" + ntp: + ipv4: + active: true + address: "" + ipv6: + active: true + address: "" resolver: resolveIPv4: true resolveIPv6: true diff --git a/src/args.c b/src/args.c index e4bbd0cd..1e7431ae 100644 --- a/src/args.c +++ b/src/args.c @@ -66,6 +66,8 @@ #include "files.h" // resolveHostname() #include "resolve.h" +// ntp_client() +#include "ntp/ntp.h" // defined in dnsmasq.c extern void print_dnsmasq_version(const char *yellow, const char *green, const char *bold, const char *normal); @@ -305,6 +307,16 @@ void parse_args(int argc, char* argv[]) exit(write_teleporter_zip_to_disk() ? EXIT_SUCCESS : EXIT_FAILURE); } + // Create test NTP client + if((argc == 2 || argc == 3) && strcmp(argv[1], "ntp-client") == 0) + { + // Enable stdout printing + cli_mode = true; + log_ctrl(false, true); + const char *server = argc == 3 ? argv[2] : "127.0.0.1"; + exit(ntp_client(server) ? EXIT_SUCCESS : EXIT_FAILURE); + } + // Import teleporter archive through CLI if(argc == 3 && strcmp(argv[1], "--teleporter") == 0) { diff --git a/src/config/config.c b/src/config/config.c index f5bb3fa1..444f2b38 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -794,6 +794,38 @@ void initConfig(struct config *conf) conf->dhcp.hosts.c = validate_stub; // Type-based checking + dnsmasq syntax checking + // struct ntp + conf->ntp.ipv4.active.k = "ntp.ipv4.active"; + conf->ntp.ipv4.active.h = "Should FTL act as an NTP server (IPv4)?"; + conf->ntp.ipv4.active.t = CONF_BOOL; + conf->ntp.ipv4.active.f = FLAG_RESTART_FTL; + conf->ntp.ipv4.active.d.b = true; + conf->ntp.ipv4.active.c = validate_stub; // Only type-based checking + + conf->ntp.ipv4.address.k = "ntp.ipv4.address"; + conf->ntp.ipv4.address.h = "IPv4 address to listen on for NTP requests"; + conf->ntp.ipv4.address.a = cJSON_CreateStringReference(" or empty string (\"\") for wildcard"); + conf->ntp.ipv4.address.t = CONF_STRUCT_IN_ADDR; + conf->ntp.ipv4.address.f = FLAG_RESTART_FTL; + memset(&conf->ntp.ipv4.address.d.in_addr, 0, sizeof(struct in_addr)); + conf->ntp.ipv4.address.c = validate_stub; // Only type-based checking + + conf->ntp.ipv6.active.k = "ntp.ipv6.active"; + conf->ntp.ipv6.active.h = "Should FTL act as an NTP server (IPv6)?"; + conf->ntp.ipv6.active.t = CONF_BOOL; + conf->ntp.ipv6.active.f = FLAG_RESTART_FTL; + conf->ntp.ipv6.active.d.b = true; + conf->ntp.ipv6.active.c = validate_stub; // Only type-based checking + + conf->ntp.ipv6.address.k = "ntp.ipv6.address"; + conf->ntp.ipv6.address.h = "IPv6 address to listen on for NTP requests"; + conf->ntp.ipv6.address.a = cJSON_CreateStringReference(" or empty string (\"\") for wildcard"); + conf->ntp.ipv6.address.t = CONF_STRUCT_IN6_ADDR; + conf->ntp.ipv6.address.f = FLAG_RESTART_FTL; + memset(&conf->ntp.ipv6.address.d.in6_addr, 0, sizeof(struct in6_addr)); + conf->ntp.ipv6.address.c = validate_stub; // Only type-based checking + + // struct resolver conf->resolver.resolveIPv6.k = "resolver.resolveIPv6"; conf->resolver.resolveIPv6.h = "Should FTL try to resolve IPv6 addresses to hostnames?"; diff --git a/src/config/config.h b/src/config/config.h index 6cd4f05e..b22cddf0 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -190,6 +190,17 @@ struct config { struct conf_item hosts; } dhcp; + struct { + struct { + struct conf_item active; + struct conf_item address; + } ipv4; + struct { + struct conf_item active; + struct conf_item address; + } ipv6; + } ntp; + struct { struct conf_item resolveIPv4; struct conf_item resolveIPv6; diff --git a/src/config/dnsmasq_config.c b/src/config/dnsmasq_config.c index 8943c23f..a9da85ea 100644 --- a/src/config/dnsmasq_config.c +++ b/src/config/dnsmasq_config.c @@ -582,6 +582,15 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_ fputs("log-dhcp\n\n", pihole_conf); } + // Check if IPv4 NTP server is active and broadcast it as DHCP option + if(conf->ntp.ipv4.active.v.b) + { + fputs("# Add NTP server to DHCP\n", pihole_conf); + // The special address 0.0.0.0 is taken to mean "the + // address of the machine running the DHCP server" + fputs("dhcp-option=option:ntp-server,0.0.0.0", pihole_conf); + } + // Add per-host parameters if(cJSON_GetArraySize(conf->dhcp.hosts.v.json) > 0) { diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 4bef6109..6dc59c7a 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -58,6 +58,8 @@ #include "config/config.h" // FTL_fork_and_bind_sockets() #include "main.h" +// ntp_server_start() +#include "ntp/ntp.h" // Private prototypes static void print_flags(const unsigned int flags); @@ -2888,6 +2890,9 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw, bool dnsmasq_start) // so they will not listen to real-time signals handle_realtime_signals(); + // Initialize NTP server + ntp_server_start(); + // We will use the attributes object later to start all threads in // detached mode pthread_attr_t attr; @@ -3601,4 +3606,4 @@ void FTL_connection_error(const char *reason, const union mysockaddr *addr) if(server != NULL) free(server); } -} \ No newline at end of file +} diff --git a/src/ntp/CMakeLists.txt b/src/ntp/CMakeLists.txt new file mode 100644 index 00000000..7eca589a --- /dev/null +++ b/src/ntp/CMakeLists.txt @@ -0,0 +1,19 @@ +# Pi-hole: A black hole for Internet advertisements +# (c) 2024 Pi-hole, LLC (https://pi-hole.net) +# Network-wide ad blocking via your own hardware. +# +# FTL Engine +# /src/ntp/CMakeList.txt +# +# This file is copyright under the latest version of the EUPL. +# Please see LICENSE file for your rights under this license. + +set(ntp_sources + server.c + client.c + ntp.h + ) + +add_library(ntp OBJECT ${ntp_sources}) +target_compile_options(ntp PRIVATE "${EXTRAWARN}") +target_include_directories(ntp PRIVATE ${PROJECT_SOURCE_DIR}/src) diff --git a/src/ntp/client.c b/src/ntp/client.c new file mode 100644 index 00000000..9919daf1 --- /dev/null +++ b/src/ntp/client.c @@ -0,0 +1,226 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2024 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* NTP client routines +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +// close() +#include +// clock_gettime() +#include +// socket(), connect(), send(), recv(), AF_INET, SOCK_DGRAM, IPPROTO_UDP +#include +// getaddrinfo(), freeaddrinfo(), struct addrinfo +#include +// memcpy() +#include +// pow() +#include +// ctime() +#include +// errno +#include + +#include "ntp.h" +#include "log.h" + +// Create minimal NTP request, see server implementation for details about the +// packet structure +static bool request(int fd, uint32_t org[2]) +{ + // NTP Packet buffer + unsigned char buf[48] = {0}; + + // LI = 0, VN = 4 (current version), Mode = 3 (Client) + buf[0] = 0x23; + + // Set Origin Timestamp + gettime32(org, true); + memcpy(&buf[40], &org[0], 2 * sizeof(uint32_t)); + + // Send request + if(send(fd, buf, 48, 0) != 48) + { + log_warn("Failed to send data to NTP server: %s", strerror(errno)); + return false; + } + + return true; +} + +static bool get_reply(int fd, uint32_t org_[2]) +{ + // NTP Packet buffer + unsigned char buf[48]; + // NTP Packet buffer as uint32_t + uint32_t *pt = (uint32_t *)((void*)&buf[24]);; + + // Receive reply + if(recv(fd, buf, 48, 0) < 48) + { + log_warn("Failed to receive data from NTP server: %s", strerror(errno)); + return false; + } + + // Extract precision of server clock + signed char rho = (signed char)buf[3]; + if(rho < -32 || rho > 0) + { + // Accepted limits are 2^-32 (~ 0.2 nanoseconds) + // to 2^0 (= 1 second) + log_warn("Received NTP reply has invalid precision: 2^(%i), assuming microsecond accuracy", rho); + rho = -19; + } + // Compute precision of server clock in seconds 2^rho + const double s_rho = pow(2, rho); + + // Extract Transmit Timestamp + // org = Origin Timestamp (Transmit Timestamp @ Client) + uint32_t org[2]; + org[0] = ntohl(*pt++); + org[1] = ntohl(*pt++); + // rec = Receive Timestamp (Receive Timestamp @ Server) + uint32_t rec[2]; + rec[0] = ntohl(*pt++); + rec[1] = ntohl(*pt++); + // xmt = Transmit Timestamp (Transmit Timestamp @ Server) + uint32_t xmt[2]; + xmt[0] = ntohl(*pt++); + xmt[1] = ntohl(*pt++); + + // dst = Destination Timestamp (Receive Timestamp @ Client) + uint32_t dst[2]; + gettime32(dst, false); + + // Check org_ and org are identical (otherwise, the reply corresponds to + // a different request and should be ignored), note that the byte order + // of the received packet is already converted while org_ is still in + // network byte order + if(ntohl(org_[0]) != org[0] || ntohl(org_[1]) != org[1]) + { + log_warn("Received NTP reply does not match request"); + return false; + } + + // Check stratum, mode, version, etc. + if((buf[0] & 0x07) != 4) + { + log_warn("Received NTP reply has invalid version"); + return false; + } + + // Calculate delay and offset + const double tfrac = 4294967296.0; // 2^32 as double + const double T1 = org[0] + org[1] / tfrac; + const double T2 = rec[0] + rec[1] / tfrac; + const double T3 = xmt[0] + xmt[1] / tfrac; + const double T4 = dst[0] + dst[1] / tfrac; + + // RFC 5905, Section 8: On-wire protocol + // It is recommended to use double precision floating point arithmetic + // for the calculations to allow unambiguous interpretation of the + // results within the maximum adjustment range of 68 years. + + // Compute offset of client clock relative to server clock + const double theta = ( ( T2 - T1 ) + ( T3 - T4 ) ) / 2; + // Compute round-trip delay + double delta = ( T4 - T1 ) - ( T3 - T2 ); + + // In some scenarios where the initial frequency offset of the client is + // relatively large and the actual propagation time small, it is + // possible for the delay computation to become negative. For instance, + // if the frequency difference is 100 ppm and the interval T4-T1 is 64 + // s, the apparent delay is -6.4 ms. Since negative values are + // misleading in subsequent computations, the value of delta should be + // clamped not less than s.rho, where s.rho is the system precision + // described in Section 11.1, expressed in seconds. + if(delta < s_rho) + { + log_warn("Negative delay detected, clamping to 0"); + delta = 0; + } + + // Print current time at client + char client_time_str[26]; + const time_t client_time = dst[0]; + ctime_r(&client_time, client_time_str); + // Remove trailing newline + client_time_str[24] = '\0'; + log_info("Current time at client: %s", client_time_str); + + // Print current time at server + char server_time_str[26]; + const time_t server_time = xmt[0]; + // Remove trailing newline + server_time_str[24] = '\0'; + ctime_r(&server_time, server_time_str); + log_info("Current time at server: %s", server_time_str); + + // Print offset and delay + log_info("Time offset: %e s", theta); + log_info("Round-trip delay: %e s", delta); + + // Offset and delay larger than 0.1 seconds are considered as invalid + // during local testing + return theta < 0.1 && delta < 0.1; +} + +bool ntp_client(const char *server) +{ + const int protocol = strchr(server, ':') != NULL ? AF_INET6 : AF_INET; + + // Create UDP socket + const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); + if(s == -1) + { + log_err("Cannot create UDP socket"); + return false; + } + + // Set socket timeout to 2 seconds + struct timeval tv; + tv.tv_sec = 2; + tv.tv_usec = 0; + if(setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) != 0) + { + log_err("Cannot set socket timeout"); + close(s); + return false; + } + + // Resolve server address + struct addrinfo *saddr; + if(getaddrinfo(server, "123", NULL, &saddr) != 0) + { + log_err("Cannot resolve NTP server address"); + close(s); + return false; + } + + // Set address to send to/receive from + if(connect(s, saddr->ai_addr, saddr->ai_addrlen) != 0) + { + log_err("Cannot connect to NTP server"); + close(s); + return false; + } + freeaddrinfo(saddr); + + // Send request + uint32_t org[2]; + if(!request(s, org)) + { + close(s); + return false; + } + + // Get reply + const bool status = get_reply(s, org); + close(s); + + return status; +} diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h new file mode 100644 index 00000000..c6e159c5 --- /dev/null +++ b/src/ntp/ntp.h @@ -0,0 +1,29 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2024 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* NTP prototypes +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +#ifndef NTP_H +#define NTP_H + +// uint64_t +#include +// bool +#include + +//uint64_t gettime32(void); +void gettime32(uint32_t ts[], const bool netorder); +//uint64_t gettime64(void); + +bool ntp_server_start(void); +bool ntp_client(const char *server); + +#endif // NTP_H + + + diff --git a/src/ntp/server.c b/src/ntp/server.c new file mode 100644 index 00000000..86c54667 --- /dev/null +++ b/src/ntp/server.c @@ -0,0 +1,387 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2024 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* NTP server routines +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +// exit(0) +#include +// memcpy() +#include +// close() +#include +// fork(), wait() +#include +// clock_gettime() +#include +//#include +#include +// wait() +#include +// htonl(), etc. +#include +// errno +#include +// ctime() +#include +// log2() +#include +// pthread_create +#include +// PR_SET_NAME +#include + +#include "ntp.h" +#include "log.h" +#include "config/config.h" + +// Retrieves the current system time, adjusts it to a 1900 epoch, converts it to +// a 32-bit fraction of a second, and optionally converts it to network byte +// order. +void gettime32(uint32_t tv[], const bool netorder) +{ + struct timespec ts; + // CLOCK_REALTIME is the system-wide realtime clock. + // It is both affected by discontinuous jumps in the system time (e.g., + // if the system administrator manually changes the clock), and by the + // incremental adjustments performed by adjtime(3) and NTP. + clock_gettime(CLOCK_REALTIME, &ts); + + // Set the epoch to 1900 (add seconds from 1900 to 1970) + tv[0] = ts.tv_sec + 2208988800ULL; + // Convert microseconds to 32 bit fraction of a second + tv[1] = (ts.tv_nsec * 0x100000000ULL) / 1000000000ULL; + + if (netorder) + { + tv[0] = htonl(tv[0]); + tv[1] = htonl(tv[1]); + } +} + +// Create and send an NTP reply to the client +static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const socklen_t saddrlen, + const unsigned char recv_buf[], const uint32_t recv_time[2]) +{ + // Buffer for the response + unsigned char send_buf[48]; + memset(send_buf, 0, sizeof(send_buf)); + + // DWORD-aligned pointer to the send buffer + uint32_t *u32p = (uint32_t*)((void*)&send_buf[0]); + // DWORD-aligned read-only pointer to the receive buffer + const uint32_t *u32r = (uint32_t*)((void*)&recv_buf[0]); + +// NTP Packet Header Format (RFC 5905), page 18 +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// |LI | VN |Mode | Stratum | Poll | Precision | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + + // Check if the first byte is valid: mode is expected to be 3 ("client") + if ((recv_buf[0] & 0x07) != 0x3) { + log_warn("Received invalid NTP request: not from an NTP client, ignoring"); + return 1; + } + + // set LI = 0 (no warning about leap seconds), set version-number to + // 4 and set mode = 4 ("server") + send_buf[0] = (0x04 << 3) + 0x04; + + // Set stratum to "secondary server" as we have derived time via + // external NTP as well. May be set to 1 if we want to be a primary + // server (synchronized by a hardware clock with GPS, etc.) + send_buf[1] = 0x02; + + // Copy Poll value from client + send_buf[2] = recv_buf[2]; + + // Precision in Nanoseconds from CLOCK_REALTIME + struct timespec ts; + clock_getres(CLOCK_REALTIME, &ts); + // Precision in log2 seconds + signed char precision = (signed char)(1.0*log2(1e-9*ts.tv_nsec)); + // Precision in log2 seconds + send_buf[3] = precision; + + // Advance 32 bit pointer to the next field + u32p++; + +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | Root Delay | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | Root Dispersion | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + + /* zur Vereinfachung , Root Delay = 0, Root Dispersion = 0 */ + *u32p++ = 0; + *u32p++ = 0; + +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | Reference ID | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + + // Reference ID = 'LOCL" (LOCAL CLOCK) + // A four-octet, left-justified, zero-padded ASCII string assigned to + // the reference clock + memcpy(u32p++, "LOCL", 4); + +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | | +// + Reference Timestamp (64) + +// | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Time when the system clock was last set or corrected, in NTP + // timestamp format. As this is not a stratum 1 server, we don't have + // a hardware clock to set this value. +#ifdef MOCK_REFTIME + // Mock this timestamp with the current time of the server minus 1 + // minute. + uint32_t ref_time[2]; + gettime32(ref_time, true); + ref_time[0] = ref_time[0] - htonl(60); // subtract 60 seconds + memcpy(u32p, ref_time, 2 * sizeof(uint32_t)); + u32p += 2; +#else + // A stateless server copies T3 and T4 from the client packet to T1 and + // T2 of the server packet and tacks on the transmit timestamp T3 before + // sending it to the client. + *u32p++ = u32r[8]; + *u32p++ = u32r[9]; +#endif +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | | +// + Origin Timestamp (64) + +// | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Time at the client when the request departed for the server, in NTP + // timestamp format. (this is the client's transmit time) + *u32p++ = u32r[10]; + *u32p++ = u32r[11]; + +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | | +// + Receive Timestamp (64) + +// | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Time at the server when the request arrived from the client, in NTP + // timestamp format. (this is the server's receive time) + memcpy(u32p, recv_time, 2 * sizeof(uint32_t)); + u32p += 2; + +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | | +// + Transmit Timestamp (64) + +// | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Time at the server when the response left for the client, in NTP + // timestamp format. (this is the server's transmit time) + uint32_t transmit_time[2]; + gettime32(transmit_time, true); + memcpy(u32p, transmit_time, 2 * sizeof(uint32_t)); + u32p += 2; + +// 0 1 2 3 +// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | | +// . . +// . Extension Field 1 (variable) . +// . . +// | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | | +// . . +// . Extension Field 2 (variable) . +// . . +// | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | Key Identifier | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// | | +// | dgst (128) | +// | | +// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ +// +// Figure 8: Packet Header Format + + // Send the response + errno = 0; + if(sendto(socket_fd, send_buf, sizeof(send_buf), 0, saddr_p, saddrlen) < 48) + { + log_err("NTP send error: %s", strerror(errno)); + return 1; + } + + return 0; +} + +// Process incoming NTP requests +static void request_process_loop(int fd, const char *ipstr, const int protocol) +{ + log_info("NTP server listening on %s:123 (%s)", ipstr, protocol == AF_INET ? "IPv4" : "IPv6"); + while (true) + { + unsigned char buf[48]; + struct sockaddr src_addr; + socklen_t src_addrlen = sizeof(src_addr); + while(recvfrom(fd, buf, sizeof(buf), 0, &src_addr, &src_addrlen) < 48); // ignore invalid requests + + // Get the current time in NTP format + uint32_t recv_time[2]; + gettime32(recv_time, true); + + struct sockaddr_in sin; + memcpy(&sin, &src_addr, sizeof(sin)); + // printf("Request from %s\n", inet_ntoa(sin.sin_addr)); + + const pid_t pid = fork(); + if (pid == 0) { + /* Child */ + ntp_reply(fd, &src_addr , src_addrlen, buf, recv_time); + exit(0); + } else if (pid == -1) { + log_err("fork() error"); + return; + } + // return to parent + } +} +/* +// Wait for a child process to exit +static void wait_wrapper(int _a) +{ + int s; + wait(&s); +}*/ + +// Start the NTP server +static void *ntp_bind_and_listen(void *param) +{ +// signal(SIGCHLD, wait_wrapper); + const int protocol = param == 0 ? AF_INET : AF_INET6; + + // Create a socket + errno = 0; + const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); + if(s == -1) + { + log_warn("Cannot create NTP socket (%s), IPv%i NTP server not available", + strerror(errno), protocol == AF_INET ? 4 : 6); + return NULL; + } + + // Bind the socket to the NTP port + char ipstr[INET6_ADDRSTRLEN + 1]; + memset(ipstr, 0, sizeof(ipstr)); + if(protocol == AF_INET) + { + // IPv4 - set thread name + prctl(PR_SET_NAME, "NTP (IPv4)", 0, 0, 0); + + // Prepare the bind address + struct sockaddr_in bind_addr; + memset(&bind_addr, 0, sizeof(bind_addr)); + bind_addr.sin_family = AF_INET; // IPv4 + bind_addr.sin_port = htons(123); // NTP port + memcpy(&bind_addr.sin_addr, &config.ntp.ipv4.address.v.in_addr, sizeof(bind_addr.sin_addr)); + inet_ntop(AF_INET, &bind_addr.sin_addr, ipstr, sizeof(ipstr) - 1); + + // Bind the socket + errno = 0; + if(bind(s, (struct sockaddr *)&bind_addr, sizeof(bind_addr)) != 0) + { + log_warn("Cannot bind to IPv4 address %s:123 (%s), IPv4 NTP server not available", + ipstr, strerror(errno)); + return NULL; + } + } + else + { + // IPv6 - set thread name + prctl(PR_SET_NAME, "NTP (IPv6)", 0, 0, 0); + + // Set socket options to allow IPv6 only, otherwise it will bind + // to both IPv4 and IPv6 and show IPv4 addresses as + // v4-mapped-on-v6 addresses + int opt = 1; + if(setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, &opt, sizeof(opt)) != 0) + { + log_warn("Cannot set socket option IPV6_V6ONLY (%s), IPv6 NTP server not available", strerror(errno)); + return NULL; + } + + // Prepare the bind address + struct sockaddr_in6 bind_addr; + memset(&bind_addr, 0, sizeof(bind_addr)); + bind_addr.sin6_family = AF_INET6; // IPv6 + bind_addr.sin6_port = htons(123); // NTP port + memcpy(&bind_addr.sin6_addr, &config.ntp.ipv6.address.v.in6_addr, sizeof(bind_addr.sin6_addr)); + inet_ntop(AF_INET6, &bind_addr.sin6_addr, ipstr, sizeof(ipstr) - 1); + + // Bind the socket + errno = 0; + if(bind(s, (struct sockaddr *)&bind_addr, sizeof(bind_addr)) != 0) + { + log_warn("Cannot bind to IPv6 address %s:123 (%s), IPv6 NTP server not available", + ipstr, strerror(errno)); + return NULL; + } + } + + request_process_loop(s, ipstr, protocol); + close(s); + + return NULL; +} + +// Start the NTP server +bool ntp_server_start(void) +{ + // Spawn two pthreads, one for IPv4 and one for IPv6 + + // IPv4 + if(config.ntp.ipv4.active.v.b) + { + // Create a thread for the IPv4 NTP server + pthread_t thread; + if (pthread_create(&thread, NULL, ntp_bind_and_listen, (void *)0) != 0) + { + log_err("Can not create NTP server thread for IPv4"); + return false; + } + } + + // IPv6 + if(config.ntp.ipv6.active.v.b) + { + // Create a thread for the IPv6 NTP server + pthread_t thread; + if (pthread_create(&thread, NULL, ntp_bind_and_listen, (void *)1) != 0) + { + log_err("Can not create NTP server thread for IPv6"); + return false; + } + } + + sleep(10); + + return true; +} diff --git a/test/pihole.toml b/test/pihole.toml index 75f8244c..364d542d 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -442,6 +442,26 @@ # "[][,id:|*][,set:][,tag:][,][,][,][,ignore]" hosts = [] + [ntp.ipv4] + # Should FTL act as an NTP server (IPv4)? + active = true + + # IPv4 address to listen on for NTP requests + # + # Possible values are: + # or empty string ("") for wildcard (0.0.0.0) + address = "" + + [ntp.ipv6] + # Should FTL act as an NTP server (IPv6)? + active = true + + # IPv6 address to listen on for NTP requests + # + # Possible values are: + # or empty string ("") for wildcard (::) + address = "" + [resolver] # Should FTL try to resolve IPv4 addresses to hostnames? resolveIPv4 = false ### CHANGED, default = true diff --git a/test/test_suite.bats b/test/test_suite.bats index 94c04019..1e0b99d1 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -26,7 +26,7 @@ #} # @test "Running a second instance is detected and prevented" { - run bash -c 'su pihole -s /bin/sh -c "/home/pihole/pihole-FTL -f"' + run bash -c 'su pihole -s /bin/sh -c "./pihole-FTL -f"' printf "%s\n" "${lines[@]}" [[ "${lines[@]}" == *"CRIT: Initialization of shared memory failed."* ]] [[ "${lines[@]}" == *"INFO: pihole-FTL is already running"* ]] @@ -54,7 +54,7 @@ @test "Number of compiled regex filters as expected" { run bash -c 'grep "Compiled [0-9]* allow" /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" - [[ ${lines[0]} == *"Compiled 2 allow and 11 deny regex for 1 client in "* ]] + [[ ${lines[0]} == *"Compiled 2 allow and 11 deny regex"* ]] } @test "denied domain is blocked" { @@ -490,15 +490,15 @@ } @test "Test fail on invalid CLI argument" { - run bash -c '/home/pihole/pihole-FTL abc' + run bash -c './pihole-FTL abc' printf "%s\n" "${lines[@]}" [[ ${lines[0]} == "pihole-FTL: invalid option -- 'abc'" ]] - [[ ${lines[1]} == "Command: '/home/pihole/pihole-FTL abc'" ]] - [[ ${lines[2]} == "Try '/home/pihole/pihole-FTL --help' for more information" ]] + [[ ${lines[1]} == "Command: './pihole-FTL abc'" ]] + [[ ${lines[2]} == "Try './pihole-FTL --help' for more information" ]] } @test "Help CLI argument return help text" { - run bash -c '/home/pihole/pihole-FTL help' + run bash -c './pihole-FTL help' printf "%s\n" "${lines[@]}" [[ ${lines[0]} == "The Pi-hole FTL engine - "* ]] } @@ -1390,6 +1390,14 @@ [[ ${lines[0]} == '{"error":{"key":"bad_request","message":"Config items set via environment variables cannot be changed via the API","hint":"misc.nice"},"took":'*'}' ]] } +@test "Check NTP server is broadcasting correct time" { + run bash -c './pihole-FTL ntp-client 127.0.0.1' + printf "%s\n" "${lines[@]}" + [[ $status == 0 ]] +} + +# We cannot easily test IPv6 as it may not be available in docker (CI) + @test "API domain search: Non-existing domain returns expected JSON" { run bash -c 'curl -s 127.0.0.1/api/search/non.existent' printf "%s\n" "${lines[@]}" From cdc7d00f8e26249cc3f3388cc388cef7badf29ac Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 28 May 2024 21:37:29 +0200 Subject: [PATCH 02/46] Add missing help text for new ntp-client option Signed-off-by: DL6ER --- src/args.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/args.c b/src/args.c index 1e7431ae..6427ac9b 100644 --- a/src/args.c +++ b/src/args.c @@ -1016,6 +1016,8 @@ void parse_args(int argc, char* argv[]) printf("%sOther:%s\n", yellow, normal); printf("\t%sptr %sIP%s Resolve IP address to hostname\n", green, cyan, normal); printf("\t%ssha256sum %sfile%s Calculate SHA256 checksum of a file\n", green, cyan, normal); + printf("\t%sntp-client %s[server]%s Request network time from %sserver%s\n", green, cyan, normal, cyan, normal); + printf("\t defaults to 127.0.0.1 if omitted\n"); printf("\t%sdhcp-discover%s Discover DHCP servers in the local\n", green, normal); printf("\t network\n"); printf("\t%sarp-scan %s[-a/-x]%s Use ARP to scan local network for\n", green, cyan, normal); From 51de04c18c4f66c8d2b6241e8790841212abfc89 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 29 May 2024 20:24:09 +0200 Subject: [PATCH 03/46] Fix include paths Signed-off-by: DL6ER --- src/ntp/client.c | 2 +- src/ntp/server.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 0f586d13..092df60f 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -26,7 +26,7 @@ // errno #include -#include "ntp.h" +#include "ntp/ntp.h" #include "log.h" // Create minimal NTP request, see server implementation for details about the diff --git a/src/ntp/server.c b/src/ntp/server.c index eb9b8a33..6814f7d4 100644 --- a/src/ntp/server.c +++ b/src/ntp/server.c @@ -36,7 +36,7 @@ // PR_SET_NAME #include -#include "ntp.h" +#include "ntp/ntp.h" #include "log.h" #include "config/config.h" From d406327ae41ceec180e3561991e0294efe7b377f Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sat, 1 Jun 2024 10:48:05 +0200 Subject: [PATCH 04/46] Synchronize pihole.toml and config.c Signed-off-by: DL6ER --- src/config/config.c | 4 ++-- test/pihole.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config/config.c b/src/config/config.c index 4437be01..72aac826 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -811,7 +811,7 @@ void initConfig(struct config *conf) conf->ntp.ipv4.address.k = "ntp.ipv4.address"; conf->ntp.ipv4.address.h = "IPv4 address to listen on for NTP requests"; - conf->ntp.ipv4.address.a = cJSON_CreateStringReference(" or empty string (\"\") for wildcard"); + conf->ntp.ipv4.address.a = cJSON_CreateStringReference(" or empty string (\"\") for wildcard (0.0.0.0)"); conf->ntp.ipv4.address.t = CONF_STRUCT_IN_ADDR; conf->ntp.ipv4.address.f = FLAG_RESTART_FTL; memset(&conf->ntp.ipv4.address.d.in_addr, 0, sizeof(struct in_addr)); @@ -826,7 +826,7 @@ void initConfig(struct config *conf) conf->ntp.ipv6.address.k = "ntp.ipv6.address"; conf->ntp.ipv6.address.h = "IPv6 address to listen on for NTP requests"; - conf->ntp.ipv6.address.a = cJSON_CreateStringReference(" or empty string (\"\") for wildcard"); + conf->ntp.ipv6.address.a = cJSON_CreateStringReference(" or empty string (\"\") for wildcard (::)"); conf->ntp.ipv6.address.t = CONF_STRUCT_IN6_ADDR; conf->ntp.ipv6.address.f = FLAG_RESTART_FTL; memset(&conf->ntp.ipv6.address.d.in6_addr, 0, sizeof(struct in6_addr)); diff --git a/test/pihole.toml b/test/pihole.toml index eef8df8b..3c4b651a 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -1059,7 +1059,7 @@ all = true ### CHANGED, default = false # Configuration statistics: -# 136 total entries out of which 82 entries are default +# 140 total entries out of which 86 entries are default # --> 54 entries are modified # 2 entries are forced through environment: # - misc.nice From daa26ae9cba27718dc0609aa83e49956aeea0717 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 2 Jun 2024 05:56:58 +0200 Subject: [PATCH 05/46] Implement time updating via optional --update flag and switch to unsigned 64 bit and double computations as mandated by RFC 5905 (page 28) Signed-off-by: DL6ER --- src/args.c | 20 +++++-- src/ntp/client.c | 126 +++++++++++++++++++++++++++++-------------- src/ntp/ntp.h | 30 +++++++++-- src/ntp/server.c | 88 +++++++++++------------------- test/test_suite.bats | 2 +- 5 files changed, 157 insertions(+), 109 deletions(-) diff --git a/src/args.c b/src/args.c index a563192f..ab7d6c44 100644 --- a/src/args.c +++ b/src/args.c @@ -308,13 +308,17 @@ void parse_args(int argc, char* argv[]) } // Create test NTP client - if((argc == 2 || argc == 3) && strcmp(argv[1], "ntp-client") == 0) + if((argc > 1 && argc < 5) && strcmp(argv[1], "ntp") == 0) { // Enable stdout printing cli_mode = true; log_ctrl(false, true); - const char *server = argc == 3 ? argv[2] : "127.0.0.1"; - exit(ntp_client(server) ? EXIT_SUCCESS : EXIT_FAILURE); + const bool update = (argc > 2 && strcmp(argv[2], "--update") == 0) || + (argc > 3 && strcmp(argv[3], "--update") == 0); + const char *server = "127.0.0.1"; + if(argc > 2 && strcmp(argv[2], "--update") != 0) + server = argv[2]; + exit(ntp_client(server, update) ? EXIT_SUCCESS : EXIT_FAILURE); } // Import teleporter archive through CLI @@ -1029,12 +1033,18 @@ void parse_args(int argc, char* argv[]) printf(" Encoding: %spihole-FTL idn2 %sdomain%s\n", green, cyan, normal); printf(" Decoding: %spihole-FTL idn2 -d %spunycode%s\n\n", green, cyan, normal); + printf("%sNTP client:%s\n", yellow, normal); + printf(" Query an NTP server for the current time and print the\n"); + printf(" result in human-readable format. An optional %sserver%s may be\n", cyan, normal); + printf(" as argument. If the server is omitted, 127.0.0.1 is used.\n\n"); + printf(" The system time is updated on the system when the optional\n"); + printf(" %s--update%s flag is given.\n\n", purple, normal); + printf(" Usage: %spihole-FTL ntp %s[server]%s %s[--update]%s\n\n", green, cyan, normal, purple, normal); + printf("%sOther:%s\n", yellow, normal); printf("\t%sptr %sIP%s %s[tcp]%s Resolve IP address to hostname\n", green, cyan, normal, purple, normal); printf("\t Append %stcp%s to use TCP instead of UDP\n", purple, normal); printf("\t%ssha256sum %sfile%s Calculate SHA256 checksum of a file\n", green, cyan, normal); - printf("\t%sntp-client %s[server]%s Request network time from %sserver%s\n", green, cyan, normal, cyan, normal); - printf("\t defaults to 127.0.0.1 if omitted\n"); printf("\t%sdhcp-discover%s Discover DHCP servers in the local\n", green, normal); printf("\t network\n"); printf("\t%sarp-scan %s[-a/-x]%s Use ARP to scan local network for\n", green, cyan, normal); diff --git a/src/ntp/client.c b/src/ntp/client.c index 092df60f..f5a960d8 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -25,13 +25,15 @@ #include // errno #include +// PRIi64 +#include #include "ntp/ntp.h" #include "log.h" // Create minimal NTP request, see server implementation for details about the // packet structure -static bool request(int fd, uint32_t org[2]) +static bool request(int fd, uint64_t *org) { // NTP Packet buffer unsigned char buf[48] = {0}; @@ -39,9 +41,14 @@ static bool request(int fd, uint32_t org[2]) // LI = 0, VN = 4 (current version), Mode = 3 (Client) buf[0] = 0x23; + // Minimum poll interval (2^6 = 64 seconds) + buf[2] = 0x06; + // Set Origin Timestamp - gettime32(org, true); - memcpy(&buf[40], &org[0], 2 * sizeof(uint32_t)); + *org = gettime64(); + //memcpy(&buf[40], &org[0], 2 * sizeof(uint32_t)); + const uint64_t norg = hton64(*org); + memcpy(&buf[40], &norg, sizeof(norg)); // Send request if(send(fd, buf, 48, 0) != 48) @@ -53,12 +60,10 @@ static bool request(int fd, uint32_t org[2]) return true; } -static bool get_reply(int fd, uint32_t org_[2]) +static bool reply(int fd, uint64_t *org_, const bool settime) { // NTP Packet buffer unsigned char buf[48]; - // NTP Packet buffer as uint32_t - uint32_t *pt = (uint32_t *)((void*)&buf[24]);; // Receive reply if(recv(fd, buf, 48, 0) < 48) @@ -81,27 +86,24 @@ static bool get_reply(int fd, uint32_t org_[2]) // Extract Transmit Timestamp // org = Origin Timestamp (Transmit Timestamp @ Client) - uint32_t org[2]; - org[0] = ntohl(*pt++); - org[1] = ntohl(*pt++); + uint64_t netbuffer; + memcpy(&netbuffer, &buf[24], sizeof(netbuffer)); + const uint64_t org = ntoh64(netbuffer); // rec = Receive Timestamp (Receive Timestamp @ Server) - uint32_t rec[2]; - rec[0] = ntohl(*pt++); - rec[1] = ntohl(*pt++); + memcpy(&netbuffer, &buf[32], sizeof(netbuffer)); + const uint64_t rec = ntoh64(netbuffer); // xmt = Transmit Timestamp (Transmit Timestamp @ Server) - uint32_t xmt[2]; - xmt[0] = ntohl(*pt++); - xmt[1] = ntohl(*pt++); + memcpy(&netbuffer, &buf[40], sizeof(netbuffer)); + const uint64_t xmt = ntoh64(netbuffer); // dst = Destination Timestamp (Receive Timestamp @ Client) - uint32_t dst[2]; - gettime32(dst, false); + uint64_t dst = gettime64(); // Check org_ and org are identical (otherwise, the reply corresponds to // a different request and should be ignored), note that the byte order // of the received packet is already converted while org_ is still in // network byte order - if(ntohl(org_[0]) != org[0] || ntohl(org_[1]) != org[1]) + if(*org_ != org) { log_warn("Received NTP reply does not match request"); return false; @@ -115,11 +117,10 @@ static bool get_reply(int fd, uint32_t org_[2]) } // Calculate delay and offset - const double tfrac = 4294967296.0; // 2^32 as double - const double T1 = org[0] + org[1] / tfrac; - const double T2 = rec[0] + rec[1] / tfrac; - const double T3 = xmt[0] + xmt[1] / tfrac; - const double T4 = dst[0] + dst[1] / tfrac; + const double T1 = org / FRAC; + const double T2 = rec / FRAC; + const double T3 = xmt / FRAC; + const double T4 = dst / FRAC; // RFC 5905, Section 8: On-wire protocol // It is recommended to use double precision floating point arithmetic @@ -128,7 +129,9 @@ static bool get_reply(int fd, uint32_t org_[2]) // Compute offset of client clock relative to server clock const double theta = ( ( T2 - T1 ) + ( T3 - T4 ) ) / 2; - // Compute round-trip delay + // Compute round-trip delay, which represents the delay of the packet + // passing through the network, which can be due switches and network + // technologies are highly variable double delta = ( T4 - T1 ) - ( T3 - T2 ); // In some scenarios where the initial frequency offset of the client is @@ -140,37 +143,78 @@ static bool get_reply(int fd, uint32_t org_[2]) // clamped not less than s.rho, where s.rho is the system precision // described in Section 11.1, expressed in seconds. if(delta < s_rho) - { - log_warn("Negative delay detected, clamping to 0"); delta = 0; - } // Print current time at client - char client_time_str[26]; - const time_t client_time = dst[0]; - strncpy(client_time_str, ctime(&client_time), sizeof(client_time_str) -1); - // Remove trailing newline - client_time_str[24] = '\0'; + char client_time_str[128]; + struct timeval client_time; + client_time.tv_sec = NTPtoSEC(dst); + client_time.tv_usec = NTPtoUSEC(dst); + struct tm *client_tm = localtime(&client_time.tv_sec); + snprintf(client_time_str, sizeof(client_time_str), "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", + client_tm->tm_year + 1900, client_tm->tm_mon + 1, client_tm->tm_mday, + client_tm->tm_hour, client_tm->tm_min, client_tm->tm_sec, client_time.tv_usec, + client_tm->tm_zone); + client_time_str[sizeof(client_time_str) - 1] = '\0'; log_info("Current time at client: %s", client_time_str); // Print current time at server - char server_time_str[26]; - const time_t server_time = xmt[0]; - strncpy(server_time_str, ctime(&server_time), sizeof(server_time_str) -1); - // Remove trailing newline - server_time_str[24] = '\0'; + char server_time_str[128]; + struct timeval server_time; + server_time.tv_sec = NTPtoSEC(xmt); + server_time.tv_usec = NTPtoUSEC(xmt); + struct tm *server_tm = localtime(&server_time.tv_sec); + snprintf(server_time_str, sizeof(server_time_str), "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", + server_tm->tm_year + 1900, server_tm->tm_mon + 1, server_tm->tm_mday, + server_tm->tm_hour, server_tm->tm_min, server_tm->tm_sec, server_time.tv_usec, + server_tm->tm_zone); + server_time_str[sizeof(server_time_str) - 1] = '\0'; log_info("Current time at server: %s", server_time_str); // Print offset and delay log_info("Time offset: %e s", theta); log_info("Round-trip delay: %e s", delta); + // Set time if requested + if(settime) + { + // Get current time + struct timeval unix_time; + gettimeofday(&unix_time, NULL); + + // Convert from double to native format (signed) and add to the + // current time. Note the addition is done in native format to + // avoid overflow or loss of precision. + const uint64_t ntp_time = D2LFP(theta) + U2LFP(unix_time); + + // Convert NTP to native format + unix_time.tv_sec = NTPtoSEC(ntp_time); + unix_time.tv_usec = NTPtoUSEC(ntp_time); + + // Print new time + char new_time_str[128]; + struct tm *new_time_tm = localtime(&unix_time.tv_sec); + snprintf(new_time_str, sizeof(new_time_str), "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", + new_time_tm->tm_year + 1900, new_time_tm->tm_mon + 1, new_time_tm->tm_mday, + new_time_tm->tm_hour, new_time_tm->tm_min, new_time_tm->tm_sec, unix_time.tv_usec, + new_time_tm->tm_zone); + new_time_str[sizeof(new_time_str) - 1] = '\0'; + + // Set time + if(settimeofday(&unix_time, NULL) != 0) + { + log_warn("Failed to set time to %s: %s", new_time_str, strerror(errno)); + return false; + } + log_info("Updated time at client: %s", new_time_str); + } + // Offset and delay larger than 0.1 seconds are considered as invalid // during local testing return theta < 0.1 && delta < 0.1; } -bool ntp_client(const char *server) +bool ntp_client(const char *server, const bool settime) { const int protocol = strchr(server, ':') != NULL ? AF_INET6 : AF_INET; @@ -212,15 +256,15 @@ bool ntp_client(const char *server) freeaddrinfo(saddr); // Send request - uint32_t org[2]; - if(!request(s, org)) + uint64_t org; + if(!request(s, &org)) { close(s); return false; } // Get reply - const bool status = get_reply(s, org); + const bool status = reply(s, &org, settime); close(s); return status; diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index c6e159c5..c71d3740 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -16,12 +16,34 @@ // bool #include -//uint64_t gettime32(void); -void gettime32(uint32_t ts[], const bool netorder); -//uint64_t gettime64(void); +// Get current time in NTP (64bit) format +uint64_t gettime64(void); +// Start NTP server bool ntp_server_start(void); -bool ntp_client(const char *server); + +// Start NTP client +bool ntp_client(const char *server, const bool settime); + +// number of seconds between 1900 and 1970 (MSB=1) +#define DIFF_SEC_1900_1970 (2208988800UL) +// number of seconds between 1970 and Feb 7, 2036 (6:28:16 UTC) (MSB=0) +#define DIFF_SEC_1970_2036 (2085978496UL) + +// Timestamp conversion macroni (RFC 5905, Appendix A) +#define FRAC 4294967296. // 2^32 as double +#define D2LFP(a) ((uint64_t)((a) * FRAC)) // NTP timestamp +#define LFP2D(a) ((double)(a) / FRAC) +#define U2LFP(a) (((uint64_t)((a).tv_sec + DIFF_SEC_1900_1970) << 32) + (uint64_t) ((a).tv_usec / 1e6 * FRAC)) + +// Convert NTP timestamp to seconds and microseconds +//#define NTPtoSEC(x) (((x & 0x80000000) != 0) ? ((x >> 32) - DIFF_SEC_1900_1970) : ((x >> 32) + DIFF_SEC_1970_2036)) +#define NTPtoSEC(x) ((x >> 32) - DIFF_SEC_1900_1970) +#define NTPtoUSEC(x) (suseconds_t)((LFP2D(x & 0xFFFFFFFF) * 1e6)) + +// Convert uint64_t to network byte order and vice versa +#define hton64(x) ((((uint64_t)htonl(x)) << 32) + htonl((x) >> 32)) +#define ntoh64(x) ((((uint64_t)ntohl(x)) << 32) + ntohl((x) >> 32)) #endif // NTP_H diff --git a/src/ntp/server.c b/src/ntp/server.c index 6814f7d4..fd924076 100644 --- a/src/ntp/server.c +++ b/src/ntp/server.c @@ -29,8 +29,6 @@ #include // ctime() #include -// log2() -#include // pthread_create #include // PR_SET_NAME @@ -40,33 +38,17 @@ #include "log.h" #include "config/config.h" -// Retrieves the current system time, adjusts it to a 1900 epoch, converts it to -// a 32-bit fraction of a second, and optionally converts it to network byte -// order. -void gettime32(uint32_t tv[], const bool netorder) +// RFC 5905 Appendix A.4: Kernel System Clock Interface +uint64_t gettime64(void) { - struct timespec ts; - // CLOCK_REALTIME is the system-wide realtime clock. - // It is both affected by discontinuous jumps in the system time (e.g., - // if the system administrator manually changes the clock), and by the - // incremental adjustments performed by adjtime(3) and NTP. - clock_gettime(CLOCK_REALTIME, &ts); - - // Set the epoch to 1900 (add seconds from 1900 to 1970) - tv[0] = ts.tv_sec + 2208988800ULL; - // Convert microseconds to 32 bit fraction of a second - tv[1] = (ts.tv_nsec * 0x100000000ULL) / 1000000000ULL; - - if (netorder) - { - tv[0] = htonl(tv[0]); - tv[1] = htonl(tv[1]); - } + struct timeval unix_time; + gettimeofday(&unix_time, NULL); + return (U2LFP(unix_time)); } // Create and send an NTP reply to the client static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const socklen_t saddrlen, - const unsigned char recv_buf[], const uint32_t recv_time[2]) + const unsigned char recv_buf[], const uint64_t *recv_time) { // Buffer for the response unsigned char send_buf[48]; @@ -102,13 +84,8 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // Copy Poll value from client send_buf[2] = recv_buf[2]; - // Precision in Nanoseconds from CLOCK_REALTIME - struct timespec ts; - clock_getres(CLOCK_REALTIME, &ts); - // Precision in log2 seconds - signed char precision = (signed char)(1.0*log2(1e-9*ts.tv_nsec)); - // Precision in log2 seconds - send_buf[3] = precision; + // Precision (log2(1e-6) = -19.931568569324174) + send_buf[3] = (signed char)(-19); // Advance 32 bit pointer to the next field u32p++; @@ -143,23 +120,23 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // + Reference Timestamp (64) + // | | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Time when the system clock was last set or corrected, in NTP // timestamp format. As this is not a stratum 1 server, we don't have // a hardware clock to set this value. #ifdef MOCK_REFTIME // Mock this timestamp with the current time of the server minus 1 // minute. - uint32_t ref_time[2]; - gettime32(ref_time, true); - ref_time[0] = ref_time[0] - htonl(60); // subtract 60 seconds - memcpy(u32p, ref_time, 2 * sizeof(uint32_t)); + const uint64_t ref_time = gettime64() - 60 * 1000000; + const uint64_t net_ref_time = hton64(ref_time); + memcpy(u32p, &net_ref_time, sizeof(uint64_t)); u32p += 2; #else // A stateless server copies T3 and T4 from the client packet to T1 and // T2 of the server packet and tacks on the transmit timestamp T3 before // sending it to the client. - *u32p++ = u32r[8]; - *u32p++ = u32r[9]; + memcpy(u32p, &u32r[8], sizeof(uint64_t)); + u32p += 2; #endif // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -168,10 +145,11 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // + Origin Timestamp (64) + // | | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Time at the client when the request departed for the server, in NTP // timestamp format. (this is the client's transmit time) - *u32p++ = u32r[10]; - *u32p++ = u32r[11]; + memcpy(u32p, &u32r[10], sizeof(uint64_t)); + u32p += 2; // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -180,9 +158,11 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // + Receive Timestamp (64) + // | | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Time at the server when the request arrived from the client, in NTP // timestamp format. (this is the server's receive time) - memcpy(u32p, recv_time, 2 * sizeof(uint32_t)); + const uint64_t net_recv_time = hton64(*recv_time); + memcpy(u32p, &net_recv_time, sizeof(uint64_t)); u32p += 2; // 0 1 2 3 @@ -192,12 +172,13 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // + Transmit Timestamp (64) + // | | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ + // Time at the server when the response left for the client, in NTP // timestamp format. (this is the server's transmit time) - uint32_t transmit_time[2]; - gettime32(transmit_time, true); - memcpy(u32p, transmit_time, 2 * sizeof(uint32_t)); - u32p += 2; + const uint64_t transmit_time = gettime64(); + const uint64_t net_transmit_time = hton64(transmit_time); + memcpy(u32p, &net_transmit_time, sizeof(uint64_t)); + // u32p += 2; // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -245,18 +226,17 @@ static void request_process_loop(int fd, const char *ipstr, const int protocol) socklen_t src_addrlen = sizeof(src_addr); while(recvfrom(fd, buf, sizeof(buf), 0, &src_addr, &src_addrlen) < 48); // ignore invalid requests - // Get the current time in NTP format - uint32_t recv_time[2]; - gettime32(recv_time, true); + // Get the current time in NTP format directly after receiving + // the request + const uint64_t recv_time = gettime64(); struct sockaddr_in sin; memcpy(&sin, &src_addr, sizeof(sin)); - // printf("Request from %s\n", inet_ntoa(sin.sin_addr)); const pid_t pid = fork(); if (pid == 0) { - /* Child */ - ntp_reply(fd, &src_addr , src_addrlen, buf, recv_time); + // Child + ntp_reply(fd, &src_addr , src_addrlen, buf, &recv_time); exit(0); } else if (pid == -1) { log_err("fork() error"); @@ -265,18 +245,10 @@ static void request_process_loop(int fd, const char *ipstr, const int protocol) // return to parent } } -/* -// Wait for a child process to exit -static void wait_wrapper(int _a) -{ - int s; - wait(&s); -}*/ // Start the NTP server static void *ntp_bind_and_listen(void *param) { -// signal(SIGCHLD, wait_wrapper); const int protocol = param == 0 ? AF_INET : AF_INET6; // Create a socket diff --git a/test/test_suite.bats b/test/test_suite.bats index 92cd34e1..f16e4d7f 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -1360,7 +1360,7 @@ } @test "Check NTP server is broadcasting correct time" { - run bash -c './pihole-FTL ntp-client 127.0.0.1' + run bash -c './pihole-FTL ntp 127.0.0.1' printf "%s\n" "${lines[@]}" [[ $status == 0 ]] } From 79c966e2e66bff2c530b6b4e279283dfd0937d22 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 2 Jun 2024 07:24:05 +0200 Subject: [PATCH 06/46] Reduce code duplication Signed-off-by: DL6ER --- src/ntp/client.c | 51 +++++++++++++++++++----------------------------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index f5a960d8..632a067c 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -60,6 +60,22 @@ static bool request(int fd, uint64_t *org) return true; } +// Display NTP time in human-readable format +static void display_time(const char *description, const uint64_t ntp_time) +{ + char client_time_str[128]; + struct timeval client_time; + client_time.tv_sec = NTPtoSEC(ntp_time); + client_time.tv_usec = NTPtoUSEC(ntp_time); + struct tm *client_tm = localtime(&client_time.tv_sec); + snprintf(client_time_str, sizeof(client_time_str), "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", + client_tm->tm_year + 1900, client_tm->tm_mon + 1, client_tm->tm_mday, + client_tm->tm_hour, client_tm->tm_min, client_tm->tm_sec, client_time.tv_usec, + client_tm->tm_zone); + client_time_str[sizeof(client_time_str) - 1] = '\0'; + log_info("%s: %s", description, client_time_str); +} + static bool reply(int fd, uint64_t *org_, const bool settime) { // NTP Packet buffer @@ -146,30 +162,10 @@ static bool reply(int fd, uint64_t *org_, const bool settime) delta = 0; // Print current time at client - char client_time_str[128]; - struct timeval client_time; - client_time.tv_sec = NTPtoSEC(dst); - client_time.tv_usec = NTPtoUSEC(dst); - struct tm *client_tm = localtime(&client_time.tv_sec); - snprintf(client_time_str, sizeof(client_time_str), "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", - client_tm->tm_year + 1900, client_tm->tm_mon + 1, client_tm->tm_mday, - client_tm->tm_hour, client_tm->tm_min, client_tm->tm_sec, client_time.tv_usec, - client_tm->tm_zone); - client_time_str[sizeof(client_time_str) - 1] = '\0'; - log_info("Current time at client: %s", client_time_str); + display_time("Current time at client", dst); // Print current time at server - char server_time_str[128]; - struct timeval server_time; - server_time.tv_sec = NTPtoSEC(xmt); - server_time.tv_usec = NTPtoUSEC(xmt); - struct tm *server_tm = localtime(&server_time.tv_sec); - snprintf(server_time_str, sizeof(server_time_str), "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", - server_tm->tm_year + 1900, server_tm->tm_mon + 1, server_tm->tm_mday, - server_tm->tm_hour, server_tm->tm_min, server_tm->tm_sec, server_time.tv_usec, - server_tm->tm_zone); - server_time_str[sizeof(server_time_str) - 1] = '\0'; - log_info("Current time at server: %s", server_time_str); + display_time("Current time at server", xmt); // Print offset and delay log_info("Time offset: %e s", theta); @@ -192,21 +188,14 @@ static bool reply(int fd, uint64_t *org_, const bool settime) unix_time.tv_usec = NTPtoUSEC(ntp_time); // Print new time - char new_time_str[128]; - struct tm *new_time_tm = localtime(&unix_time.tv_sec); - snprintf(new_time_str, sizeof(new_time_str), "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", - new_time_tm->tm_year + 1900, new_time_tm->tm_mon + 1, new_time_tm->tm_mday, - new_time_tm->tm_hour, new_time_tm->tm_min, new_time_tm->tm_sec, unix_time.tv_usec, - new_time_tm->tm_zone); - new_time_str[sizeof(new_time_str) - 1] = '\0'; + display_time("Setting time to", ntp_time); // Set time if(settimeofday(&unix_time, NULL) != 0) { - log_warn("Failed to set time to %s: %s", new_time_str, strerror(errno)); + log_warn("Failed to set time: %s", strerror(errno)); return false; } - log_info("Updated time at client: %s", new_time_str); } // Offset and delay larger than 0.1 seconds are considered as invalid From f8990e769ae519fc65955b77d69f9cbfcc7252be Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 2 Jun 2024 07:31:55 +0200 Subject: [PATCH 07/46] Average over up to eight successive NTP queries to reduce total time error during synchronization Signed-off-by: DL6ER --- src/ntp/client.c | 242 ++++++++++++++++++++++++++++++----------------- src/ntp/ntp.h | 4 + src/ntp/server.c | 24 ++--- 3 files changed, 171 insertions(+), 99 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 632a067c..41089c0f 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -31,9 +31,18 @@ #include "ntp/ntp.h" #include "log.h" +struct ntp_sync +{ + uint64_t org; + uint64_t xmt; + double theta; + double delta; + double precision; +}; + // Create minimal NTP request, see server implementation for details about the // packet structure -static bool request(int fd, uint64_t *org) +static bool request(int fd, struct ntp_sync *ntp) { // NTP Packet buffer unsigned char buf[48] = {0}; @@ -44,16 +53,19 @@ static bool request(int fd, uint64_t *org) // Minimum poll interval (2^6 = 64 seconds) buf[2] = 0x06; - // Set Origin Timestamp - *org = gettime64(); - //memcpy(&buf[40], &org[0], 2 * sizeof(uint32_t)); - const uint64_t norg = hton64(*org); + // Set Reference Timestamp (ref) to 0 + // This is the time at which the local clock was last set or corrected. + memset(&buf[8], 0, sizeof(uint64_t)); + + // Set Origin Timestamp (org) in NTP format + ntp->org = gettime64(); + const uint64_t norg = hton64(ntp->org); memcpy(&buf[40], &norg, sizeof(norg)); // Send request if(send(fd, buf, 48, 0) != 48) { - log_warn("Failed to send data to NTP server: %s", strerror(errno)); + printf("Failed to send data to NTP server: %s\n", strerror(errno)); return false; } @@ -61,6 +73,8 @@ static bool request(int fd, uint64_t *org) } // Display NTP time in human-readable format +// This function is similar to get_timestr() in src/log.c but differs in that it +// includes microseconds whereas get_timestr() only includes milliseconds static void display_time(const char *description, const uint64_t ntp_time) { char client_time_str[128]; @@ -73,10 +87,10 @@ static void display_time(const char *description, const uint64_t ntp_time) client_tm->tm_hour, client_tm->tm_min, client_tm->tm_sec, client_time.tv_usec, client_tm->tm_zone); client_time_str[sizeof(client_time_str) - 1] = '\0'; - log_info("%s: %s", description, client_time_str); + printf("%s: %s\n", description, client_time_str); } -static bool reply(int fd, uint64_t *org_, const bool settime) +static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) { // NTP Packet buffer unsigned char buf[48]; @@ -84,7 +98,7 @@ static bool reply(int fd, uint64_t *org_, const bool settime) // Receive reply if(recv(fd, buf, 48, 0) < 48) { - log_warn("Failed to receive data from NTP server: %s", strerror(errno)); + printf("Failed to receive data from NTP server: %s\n", strerror(errno)); return false; } @@ -94,11 +108,11 @@ static bool reply(int fd, uint64_t *org_, const bool settime) { // Accepted limits are 2^-32 (~ 0.2 nanoseconds) // to 2^0 (= 1 second) - log_warn("Received NTP reply has invalid precision: 2^(%i), assuming microsecond accuracy", rho); + printf("Received NTP reply has invalid precision: 2^(%i), assuming microsecond accuracy\n", rho); rho = -19; } // Compute precision of server clock in seconds 2^rho - const double s_rho = pow(2, rho); + ntp->precision = pow(2, rho); // Extract Transmit Timestamp // org = Origin Timestamp (Transmit Timestamp @ Client) @@ -110,7 +124,7 @@ static bool reply(int fd, uint64_t *org_, const bool settime) const uint64_t rec = ntoh64(netbuffer); // xmt = Transmit Timestamp (Transmit Timestamp @ Server) memcpy(&netbuffer, &buf[40], sizeof(netbuffer)); - const uint64_t xmt = ntoh64(netbuffer); + ntp->xmt = ntoh64(netbuffer); // dst = Destination Timestamp (Receive Timestamp @ Client) uint64_t dst = gettime64(); @@ -119,23 +133,23 @@ static bool reply(int fd, uint64_t *org_, const bool settime) // a different request and should be ignored), note that the byte order // of the received packet is already converted while org_ is still in // network byte order - if(*org_ != org) + if(ntp->org != org) { - log_warn("Received NTP reply does not match request"); + printf("Received NTP reply does not match request (request %"PRIx64", reply %"PRIx64")\n", ntp->org, org); return false; } // Check stratum, mode, version, etc. if((buf[0] & 0x07) != 4) { - log_warn("Received NTP reply has invalid version"); + printf("Received NTP reply has invalid version\n"); return false; } // Calculate delay and offset - const double T1 = org / FRAC; + const double T1 = ntp->org / FRAC; const double T2 = rec / FRAC; - const double T3 = xmt / FRAC; + const double T3 = ntp->xmt / FRAC; const double T4 = dst / FRAC; // RFC 5905, Section 8: On-wire protocol @@ -144,11 +158,11 @@ static bool reply(int fd, uint64_t *org_, const bool settime) // results within the maximum adjustment range of 68 years. // Compute offset of client clock relative to server clock - const double theta = ( ( T2 - T1 ) + ( T3 - T4 ) ) / 2; + ntp->theta = ( ( T2 - T1 ) + ( T3 - T4 ) ) / 2; // Compute round-trip delay, which represents the delay of the packet // passing through the network, which can be due switches and network // technologies are highly variable - double delta = ( T4 - T1 ) - ( T3 - T2 ); + ntp->delta = ( T4 - T1 ) - ( T3 - T2 ); // In some scenarios where the initial frequency offset of the client is // relatively large and the actual propagation time small, it is @@ -158,18 +172,131 @@ static bool reply(int fd, uint64_t *org_, const bool settime) // misleading in subsequent computations, the value of delta should be // clamped not less than s.rho, where s.rho is the system precision // described in Section 11.1, expressed in seconds. - if(delta < s_rho) - delta = 0; + if(ntp->delta < ntp->precision) + ntp->delta = 0; + +# // Return early if not verbose + if(!verbose) + return true; // Print current time at client display_time("Current time at client", dst); // Print current time at server - display_time("Current time at server", xmt); + display_time("Current time at server", ntp->xmt); // Print offset and delay - log_info("Time offset: %e s", theta); - log_info("Round-trip delay: %e s", delta); + printf("Time offset: %e s\n", ntp->theta); + printf("Round-trip delay: %e s\n", ntp->delta); + + return true; +} + +bool ntp_client(const char *server, const bool settime) +{ + const int protocol = strchr(server, ':') != NULL ? AF_INET6 : AF_INET; + + // Create UDP socket + const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); + if(s == -1) + { + printf("ERROR: Cannot create UDP socket\n"); + return false; + } + + // Set socket timeout to 2 seconds + struct timeval tv; + tv.tv_sec = 2; + tv.tv_usec = 0; + if(setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) != 0) + { + printf("ERROR: Cannot set socket timeout\n"); + close(s); + return false; + } + + // Resolve server address + struct addrinfo *saddr; + if(getaddrinfo(server, "123", NULL, &saddr) != 0) + { + printf("ERROR: Cannot resolve NTP server address\n"); + close(s); + return false; + } + + // Set address to send to/receive from + if(connect(s, saddr->ai_addr, saddr->ai_addrlen) != 0) + { + printf("ERROR: Cannot connect to NTP server\n"); + close(s); + return false; + } + freeaddrinfo(saddr); + + struct ntp_sync ntp[NTP_AVERGAGE_COUNT]; + memset(&ntp, 0, sizeof(ntp)); + for(unsigned int i = 0; i < NTP_AVERGAGE_COUNT; i++) + { + // Send request + if(!request(s, &ntp[i])) + { + close(s); + return false; + } + // Get reply + if(!reply(s, &ntp[i], false)) + continue; + + // Sleep for 100 ms to avoid flooding the server + printf("."); + fflush(stdout); + usleep(100000); + } + printf("\n"); + + // Close socket + close(s); + + // Compute average and standard deviation + unsigned int valid = 0; + double theta_avg = 0.0, theta_stdev = 0.0; + double delta_avg = 0.0, delta_stdev = 0.0; + for(unsigned int i = 0; i < NTP_AVERGAGE_COUNT; i++) + { + // Skip invalid values + if(fabs(ntp[i].theta) < ntp[i].precision || + fabs(ntp[i].delta) < ntp[i].precision) + continue; + + theta_avg += ntp[i].theta; + delta_avg += ntp[i].delta; + valid++; + } + + if(valid == 0) + { + printf("No valid NTP replies received, check server and network connectivity\n\n"); + return false; + } + printf("Received %u/%d valid NTP replies\n\n", valid, NTP_AVERGAGE_COUNT); + + theta_avg /= valid; + delta_avg /= valid; + for(unsigned int i = 0; i < NTP_AVERGAGE_COUNT; i++) + { + // Skip invalid values + if(fabs(ntp[i].theta) < ntp[i].precision || + fabs(ntp[i].delta) < ntp[i].precision) + continue; + + theta_stdev += pow(ntp[i].theta - theta_avg, 2); + delta_stdev += pow(ntp[i].delta - delta_avg, 2); + } + theta_stdev = sqrt(theta_stdev / valid); + delta_stdev = sqrt(delta_stdev / valid); + + printf("Average time offset: (%e +/- %e s)\n", theta_avg, theta_stdev); + printf("Average round-trip delay: (%e +/- %e s)\n", delta_avg, delta_stdev); // Set time if requested if(settime) @@ -181,80 +308,25 @@ static bool reply(int fd, uint64_t *org_, const bool settime) // Convert from double to native format (signed) and add to the // current time. Note the addition is done in native format to // avoid overflow or loss of precision. - const uint64_t ntp_time = D2LFP(theta) + U2LFP(unix_time); + const uint64_t ntp_time = U2LFP(unix_time) + D2LFP(theta_avg); // Convert NTP to native format unix_time.tv_sec = NTPtoSEC(ntp_time); unix_time.tv_usec = NTPtoUSEC(ntp_time); // Print new time - display_time("Setting time to", ntp_time); + display_time("Setting local time to", ntp_time); // Set time if(settimeofday(&unix_time, NULL) != 0) { - log_warn("Failed to set time: %s", strerror(errno)); + printf("Failed to set time: %s\n", + errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); return false; } } // Offset and delay larger than 0.1 seconds are considered as invalid - // during local testing - return theta < 0.1 && delta < 0.1; -} - -bool ntp_client(const char *server, const bool settime) -{ - const int protocol = strchr(server, ':') != NULL ? AF_INET6 : AF_INET; - - // Create UDP socket - const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); - if(s == -1) - { - log_err("Cannot create UDP socket"); - return false; - } - - // Set socket timeout to 2 seconds - struct timeval tv; - tv.tv_sec = 2; - tv.tv_usec = 0; - if(setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) != 0) - { - log_err("Cannot set socket timeout"); - close(s); - return false; - } - - // Resolve server address - struct addrinfo *saddr; - if(getaddrinfo(server, "123", NULL, &saddr) != 0) - { - log_err("Cannot resolve NTP server address"); - close(s); - return false; - } - - // Set address to send to/receive from - if(connect(s, saddr->ai_addr, saddr->ai_addrlen) != 0) - { - log_err("Cannot connect to NTP server"); - close(s); - return false; - } - freeaddrinfo(saddr); - - // Send request - uint64_t org; - if(!request(s, &org)) - { - close(s); - return false; - } - - // Get reply - const bool status = reply(s, &org, settime); - close(s); - - return status; + // during local testing (e.g., when the server is on the same machine) + return theta_avg < 0.1 && delta_avg < 0.1; } diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index c71d3740..cc82c9b9 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -25,6 +25,10 @@ bool ntp_server_start(void); // Start NTP client bool ntp_client(const char *server, const bool settime); +// Number of NTP queries to average. The more queries, the more accurate the +// time, but the longer it takes to synchronize. The minimum is 1. +#define NTP_AVERGAGE_COUNT 8 + // number of seconds between 1900 and 1970 (MSB=1) #define DIFF_SEC_1900_1970 (2208988800UL) // number of seconds between 1970 and Feb 7, 2036 (6:28:16 UTC) (MSB=0) diff --git a/src/ntp/server.c b/src/ntp/server.c index fd924076..e1d593a5 100644 --- a/src/ntp/server.c +++ b/src/ntp/server.c @@ -84,8 +84,10 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // Copy Poll value from client send_buf[2] = recv_buf[2]; - // Precision (log2(1e-6) = -19.931568569324174) - send_buf[3] = (signed char)(-19); + // Precision: the precision of the local clock, in seconds to the + // nearest power of two. + // log2(1 usec = 1e-6 s) = -19.931568569324174 + send_buf[3] = (signed char)(-20); // Advance 32 bit pointer to the next field u32p++; @@ -98,9 +100,11 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // | Root Dispersion | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - /* zur Vereinfachung , Root Delay = 0, Root Dispersion = 0 */ - *u32p++ = 0; - *u32p++ = 0; + // Assume Root Delay (total roundtrip delay to the primary reference + // source) = 0, Root Dispersion (the nominal error relative to the + // primary reference source) = 0 as we don't have these numbers + *u32p++ = 0.0; + *u32p++ = 0.0; // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -124,20 +128,12 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // Time when the system clock was last set or corrected, in NTP // timestamp format. As this is not a stratum 1 server, we don't have // a hardware clock to set this value. -#ifdef MOCK_REFTIME - // Mock this timestamp with the current time of the server minus 1 - // minute. - const uint64_t ref_time = gettime64() - 60 * 1000000; - const uint64_t net_ref_time = hton64(ref_time); - memcpy(u32p, &net_ref_time, sizeof(uint64_t)); - u32p += 2; -#else // A stateless server copies T3 and T4 from the client packet to T1 and // T2 of the server packet and tacks on the transmit timestamp T3 before // sending it to the client. memcpy(u32p, &u32r[8], sizeof(uint64_t)); u32p += 2; -#endif + // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ From 5b9df0237248dc5cd891ef724787e8e2a220a85f Mon Sep 17 00:00:00 2001 From: DL6ER Date: Sun, 2 Jun 2024 20:07:41 +0200 Subject: [PATCH 08/46] Add missing newlines in dnsmasq config Signed-off-by: DL6ER --- src/config/dnsmasq_config.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/config/dnsmasq_config.c b/src/config/dnsmasq_config.c index 86b9952f..5f3244bf 100644 --- a/src/config/dnsmasq_config.c +++ b/src/config/dnsmasq_config.c @@ -588,14 +588,14 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_ fputs("# Add NTP server to DHCP\n", pihole_conf); // The special address 0.0.0.0 is taken to mean "the // address of the machine running the DHCP server" - fputs("dhcp-option=option:ntp-server,0.0.0.0", pihole_conf); + fputs("dhcp-option=option:ntp-server,0.0.0.0\n\n", pihole_conf); } - + // Add option to ignore unknown clients if enabled if(conf->dhcp.ignoreUnknownClients.v.b) { fputs("# Ignore clients not configured below\n", pihole_conf); - fputs("dhcp-ignore=tag:!known\n", pihole_conf); + fputs("dhcp-ignore=tag:!known\n\n", pihole_conf); } // Add per-host parameters From 7c0d7e87e844089c8ae105c55899450e39aaeec9 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 4 Jun 2024 07:55:06 +0200 Subject: [PATCH 09/46] Add debug.ntp flag Signed-off-by: DL6ER --- src/api/docs/content/specs/config.yaml | 3 + src/args.c | 1 + src/config/config.c | 7 ++ src/config/config.h | 1 + src/enums.h | 1 + src/log.c | 2 + src/ntp/client.c | 90 +++++++++++++++++--------- src/ntp/ntp.h | 7 ++ src/ntp/server.c | 55 ++++++++++++---- test/pihole.toml | 7 +- 10 files changed, 128 insertions(+), 46 deletions(-) diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index babf5f4c..c1b4edfa 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -566,6 +566,8 @@ components: type: boolean reserved: type: boolean + ntp: + type: boolean all: type: boolean topics: @@ -784,6 +786,7 @@ components: webserver: false extra: false reserved: false + ntp: false all: false config_one: summary: One option diff --git a/src/args.c b/src/args.c index 66ebcd37..622c894b 100644 --- a/src/args.c +++ b/src/args.c @@ -313,6 +313,7 @@ void parse_args(int argc, char* argv[]) // Enable stdout printing cli_mode = true; log_ctrl(false, true); + readFTLconf(&config, false); const bool update = (argc > 2 && strcmp(argv[2], "--update") == 0) || (argc > 3 && strcmp(argv[3], "--update") == 0); const char *server = "127.0.0.1"; diff --git a/src/config/config.c b/src/config/config.c index 72aac826..a7f29c02 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -1480,6 +1480,13 @@ void initConfig(struct config *conf) conf->debug.reserved.d.b = false; conf->debug.reserved.c = validate_stub; // Only type-based checking + conf->debug.ntp.k = "debug.ntp"; + conf->debug.ntp.h = "Print information about NTP synchronization"; + conf->debug.ntp.t = CONF_BOOL; + conf->debug.ntp.f = FLAG_ADVANCED_SETTING; + conf->debug.ntp.d.b = false; + conf->debug.ntp.c = validate_stub; // Only type-based checking + conf->debug.all.k = "debug.all"; conf->debug.all.h = "Set all debug flags at once. This is a convenience option to enable all debug flags at once. Note that this option is not persistent, setting it to true will enable all *remaining* debug flags but unsetting it will disable *all* debug flags."; conf->debug.all.t = CONF_ALL_DEBUG_BOOL; diff --git a/src/config/config.h b/src/config/config.h index 7ce87d1e..6c000146 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -323,6 +323,7 @@ struct config { struct conf_item webserver; struct conf_item extra; struct conf_item reserved; + struct conf_item ntp; // all must be the last item in this struct struct conf_item all; } debug; diff --git a/src/enums.h b/src/enums.h index 09769a9c..67dd1fbc 100644 --- a/src/enums.h +++ b/src/enums.h @@ -162,6 +162,7 @@ enum debug_flag { DEBUG_WEBSERVER, DEBUG_EXTRA, DEBUG_RESERVED, + DEBUG_NTP, DEBUG_MAX } __attribute__ ((packed)); diff --git a/src/log.c b/src/log.c index a10fe836..0fbbe447 100644 --- a/src/log.c +++ b/src/log.c @@ -219,6 +219,8 @@ const char *debugstr(const enum debug_flag flag) return "DEBUG_WEBSERVER"; case DEBUG_RESERVED: return "DEBUG_RESERVED"; + case DEBUG_NTP: + return "DEBUG_NTP"; case DEBUG_MAX: return "DEBUG_MAX"; case DEBUG_NONE: // fall through diff --git a/src/ntp/client.c b/src/ntp/client.c index 41089c0f..0a1fd4d5 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -8,7 +8,7 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -#include "FTL.h" +#include "ntp/ntp.h" // close() #include // clock_gettime() @@ -27,9 +27,8 @@ #include // PRIi64 #include - -#include "ntp/ntp.h" -#include "log.h" +// config struct +#include "config/config.h" struct ntp_sync { @@ -65,7 +64,7 @@ static bool request(int fd, struct ntp_sync *ntp) // Send request if(send(fd, buf, 48, 0) != 48) { - printf("Failed to send data to NTP server: %s\n", strerror(errno)); + log_err("Failed to send data to NTP server: %s", strerror(errno)); return false; } @@ -75,19 +74,44 @@ static bool request(int fd, struct ntp_sync *ntp) // Display NTP time in human-readable format // This function is similar to get_timestr() in src/log.c but differs in that it // includes microseconds whereas get_timestr() only includes milliseconds -static void display_time(const char *description, const uint64_t ntp_time) +static void format_NTP_time(char time_str[TIMESTR_SIZE], const uint64_t ntp_time) { - char client_time_str[128]; struct timeval client_time; client_time.tv_sec = NTPtoSEC(ntp_time); client_time.tv_usec = NTPtoUSEC(ntp_time); struct tm *client_tm = localtime(&client_time.tv_sec); - snprintf(client_time_str, sizeof(client_time_str), "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", - client_tm->tm_year + 1900, client_tm->tm_mon + 1, client_tm->tm_mday, - client_tm->tm_hour, client_tm->tm_min, client_tm->tm_sec, client_time.tv_usec, - client_tm->tm_zone); - client_time_str[sizeof(client_time_str) - 1] = '\0'; - printf("%s: %s\n", description, client_time_str); + snprintf(time_str, TIMESTR_SIZE, "%04i-%02i-%02i %02i:%02i:%02i.%06"PRIi64" %s", + client_tm->tm_year + 1900, client_tm->tm_mon + 1, client_tm->tm_mday, + client_tm->tm_hour, client_tm->tm_min, client_tm->tm_sec, client_time.tv_usec, + client_tm->tm_zone); + time_str[TIMESTR_SIZE - 1] = '\0'; +} + +// Print NTP timestamp in human-readable form for debugging +void print_debug_time(const char *label, const uint32_t *u32p, const uint64_t ntp_time) +{ + // Get the time from the appropriate buffer + uint64_t timevar; + if(u32p != NULL) + { + memcpy(&timevar, u32p, sizeof(uint64_t)); + // Convert to host byte order + timevar = ntoh64(timevar); + } + else + { + // Use the provided time (already in host byte order) + timevar = ntp_time; + } + + + // Format the time + char time_str[TIMESTR_SIZE]; + format_NTP_time(time_str, timevar); + + // Print the time + log_debug(DEBUG_NTP, "%s: %08"PRIx64".%08"PRIx64" = %s", label, + (timevar >> 32) & 0xFFFFFFFF, timevar & 0xFFFFFFFF, time_str); } static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) @@ -98,7 +122,7 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // Receive reply if(recv(fd, buf, 48, 0) < 48) { - printf("Failed to receive data from NTP server: %s\n", strerror(errno)); + log_err("Failed to receive data from NTP server: %s", strerror(errno)); return false; } @@ -108,7 +132,7 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) { // Accepted limits are 2^-32 (~ 0.2 nanoseconds) // to 2^0 (= 1 second) - printf("Received NTP reply has invalid precision: 2^(%i), assuming microsecond accuracy\n", rho); + log_warn("Received NTP reply has invalid precision: 2^(%i), assuming microsecond accuracy", rho); rho = -19; } // Compute precision of server clock in seconds 2^rho @@ -135,14 +159,14 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // network byte order if(ntp->org != org) { - printf("Received NTP reply does not match request (request %"PRIx64", reply %"PRIx64")\n", ntp->org, org); + log_warn("Received NTP reply does not match request (request %"PRIx64", reply %"PRIx64")", ntp->org, org); return false; } // Check stratum, mode, version, etc. if((buf[0] & 0x07) != 4) { - printf("Received NTP reply has invalid version\n"); + log_warn("Received NTP reply has invalid version"); return false; } @@ -176,18 +200,18 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) ntp->delta = 0; # // Return early if not verbose - if(!verbose) + if(!config.debug.ntp.v.b) return true; // Print current time at client - display_time("Current time at client", dst); + print_debug_time("Current time at client", NULL, dst); // Print current time at server - display_time("Current time at server", ntp->xmt); + print_debug_time("Current time at server", NULL, ntp->xmt); // Print offset and delay - printf("Time offset: %e s\n", ntp->theta); - printf("Round-trip delay: %e s\n", ntp->delta); + log_debug(DEBUG_NTP, "Time offset: %e s", ntp->theta); + log_debug(DEBUG_NTP, "Round-trip delay: %e s", ntp->delta); return true; } @@ -200,7 +224,7 @@ bool ntp_client(const char *server, const bool settime) const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); if(s == -1) { - printf("ERROR: Cannot create UDP socket\n"); + log_err("Cannot create UDP socket\n"); return false; } @@ -210,7 +234,7 @@ bool ntp_client(const char *server, const bool settime) tv.tv_usec = 0; if(setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) != 0) { - printf("ERROR: Cannot set socket timeout\n"); + log_err("Cannot set socket timeout\n"); close(s); return false; } @@ -219,7 +243,7 @@ bool ntp_client(const char *server, const bool settime) struct addrinfo *saddr; if(getaddrinfo(server, "123", NULL, &saddr) != 0) { - printf("ERROR: Cannot resolve NTP server address\n"); + log_err("Cannot resolve NTP server address\n"); close(s); return false; } @@ -227,7 +251,7 @@ bool ntp_client(const char *server, const bool settime) // Set address to send to/receive from if(connect(s, saddr->ai_addr, saddr->ai_addrlen) != 0) { - printf("ERROR: Cannot connect to NTP server\n"); + log_err("Cannot connect to NTP server\n"); close(s); return false; } @@ -275,10 +299,10 @@ bool ntp_client(const char *server, const bool settime) if(valid == 0) { - printf("No valid NTP replies received, check server and network connectivity\n\n"); + log_err("No valid NTP replies received, check server and network connectivity\n"); return false; } - printf("Received %u/%d valid NTP replies\n\n", valid, NTP_AVERGAGE_COUNT); + log_info("Received %u/%d valid NTP replies\n", valid, NTP_AVERGAGE_COUNT); theta_avg /= valid; delta_avg /= valid; @@ -295,8 +319,8 @@ bool ntp_client(const char *server, const bool settime) theta_stdev = sqrt(theta_stdev / valid); delta_stdev = sqrt(delta_stdev / valid); - printf("Average time offset: (%e +/- %e s)\n", theta_avg, theta_stdev); - printf("Average round-trip delay: (%e +/- %e s)\n", delta_avg, delta_stdev); + log_info("Average time offset: (%e +/- %e s)", theta_avg, theta_stdev); + log_info("Average round-trip delay: (%e +/- %e s)", delta_avg, delta_stdev); // Set time if requested if(settime) @@ -315,12 +339,14 @@ bool ntp_client(const char *server, const bool settime) unix_time.tv_usec = NTPtoUSEC(ntp_time); // Print new time - display_time("Setting local time to", ntp_time); + char time_str[TIMESTR_SIZE]; + format_NTP_time(time_str, ntp_time); + log_info("Setting local time to: %s", time_str); // Set time if(settimeofday(&unix_time, NULL) != 0) { - printf("Failed to set time: %s\n", + log_err("Failed to set time: %s", errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); return false; } diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index cc82c9b9..caca5169 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -11,6 +11,10 @@ #ifndef NTP_H #define NTP_H +#include "FTL.h" +// TIMESTR_SIZE +#include "log.h" + // uint64_t #include // bool @@ -19,6 +23,9 @@ // Get current time in NTP (64bit) format uint64_t gettime64(void); +// Print NTP timestamp in human-readable form +void print_debug_time(const char *label, const uint32_t *u32p, const uint64_t ntp_time); + // Start NTP server bool ntp_server_start(void); diff --git a/src/ntp/server.c b/src/ntp/server.c index e1d593a5..f6e70937 100644 --- a/src/ntp/server.c +++ b/src/ntp/server.c @@ -8,7 +8,7 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -#include "FTL.h" +#include "ntp/ntp.h" // exit(0) #include // memcpy() @@ -33,10 +33,10 @@ #include // PR_SET_NAME #include - -#include "ntp/ntp.h" -#include "log.h" +// config struct #include "config/config.h" +// PRIi64 +#include // RFC 5905 Appendix A.4: Kernel System Clock Interface uint64_t gettime64(void) @@ -47,8 +47,8 @@ uint64_t gettime64(void) } // Create and send an NTP reply to the client -static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const socklen_t saddrlen, - const unsigned char recv_buf[], const uint64_t *recv_time) +static bool ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const socklen_t saddrlen, + const unsigned char recv_buf[], const uint64_t *recv_time) { // Buffer for the response unsigned char send_buf[48]; @@ -69,7 +69,7 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // Check if the first byte is valid: mode is expected to be 3 ("client") if ((recv_buf[0] & 0x07) != 0x3) { log_warn("Received invalid NTP request: not from an NTP client, ignoring"); - return 1; + return false; } // set LI = 0 (no warning about leap seconds), set version-number to @@ -132,6 +132,8 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // T2 of the server packet and tacks on the transmit timestamp T3 before // sending it to the client. memcpy(u32p, &u32r[8], sizeof(uint64_t)); + if(config.debug.ntp.v.b) + print_debug_time("Reference Timestamp", u32p, 0); u32p += 2; // 0 1 2 3 @@ -145,6 +147,8 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // Time at the client when the request departed for the server, in NTP // timestamp format. (this is the client's transmit time) memcpy(u32p, &u32r[10], sizeof(uint64_t)); + if(config.debug.ntp.v.b) + print_debug_time("Origin Timestamp", u32p, 0); u32p += 2; // 0 1 2 3 @@ -159,6 +163,8 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // timestamp format. (this is the server's receive time) const uint64_t net_recv_time = hton64(*recv_time); memcpy(u32p, &net_recv_time, sizeof(uint64_t)); + if(config.debug.ntp.v.b) + print_debug_time("Receive Timestamp", u32p, 0); u32p += 2; // 0 1 2 3 @@ -174,7 +180,9 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const const uint64_t transmit_time = gettime64(); const uint64_t net_transmit_time = hton64(transmit_time); memcpy(u32p, &net_transmit_time, sizeof(uint64_t)); - // u32p += 2; + if(config.debug.ntp.v.b) + print_debug_time("Transmit Timestamp", u32p, 0); + u32p += 2; // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -205,10 +213,10 @@ static int ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const if(sendto(socket_fd, send_buf, sizeof(send_buf), 0, saddr_p, saddrlen) < 48) { log_err("NTP send error: %s", strerror(errno)); - return 1; + return false; } - return 0; + return true; } // Process incoming NTP requests @@ -226,9 +234,32 @@ static void request_process_loop(int fd, const char *ipstr, const int protocol) // the request const uint64_t recv_time = gettime64(); - struct sockaddr_in sin; - memcpy(&sin, &src_addr, sizeof(sin)); + // Print the request + if(config.debug.ntp.v.b) + { + if(protocol == AF_INET6) + { + struct sockaddr_in6 sin6; + memcpy(&sin6, &src_addr, sizeof(sin6)); + char ip[INET6_ADDRSTRLEN]; + const in_port_t port = ntohs(sin6.sin6_port); + inet_ntop(protocol, &sin6.sin6_addr, ip, sizeof(ip)); + log_debug(DEBUG_NTP, "Received NTP request from [%s]:%u", ip, port); + } + else + { + struct sockaddr_in sin; + memcpy(&sin, &src_addr, sizeof(sin)); + + char ip[INET6_ADDRSTRLEN]; + const in_port_t port = ntohs(sin.sin_port); + inet_ntop(protocol, &sin.sin_addr, ip, sizeof(ip)); + log_debug(DEBUG_NTP, "Received NTP request from %s:%u", ip, port); + } + } + + // Fork a child to handle the request const pid_t pid = fork(); if (pid == 0) { // Child diff --git a/test/pihole.toml b/test/pihole.toml index 3c4b651a..9e5b06fb 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -1053,14 +1053,17 @@ # Reserved debug flag reserved = true ### CHANGED, default = false + # Print information about NTP synchronization + ntp = true ### CHANGED, default = false + # Set all debug flags at once. This is a convenience option to enable all debug flags # at once. Note that this option is not persistent, setting it to true will enable all # *remaining* debug flags but unsetting it will disable *all* debug flags. all = true ### CHANGED, default = false # Configuration statistics: -# 140 total entries out of which 86 entries are default -# --> 54 entries are modified +# 141 total entries out of which 86 entries are default +# --> 55 entries are modified # 2 entries are forced through environment: # - misc.nice # - debug.api From 10e4c732f28de5679dc777bf6b74081390a5d51b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 4 Jun 2024 13:14:50 +0200 Subject: [PATCH 10/46] Use trimmed mean to compute time offset to exclude outliers where packets traveled unusual paths Signed-off-by: DL6ER --- src/ntp/client.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 0a1fd4d5..7120fc54 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -322,6 +322,33 @@ bool ntp_client(const char *server, const bool settime) log_info("Average time offset: (%e +/- %e s)", theta_avg, theta_stdev); log_info("Average round-trip delay: (%e +/- %e s)", delta_avg, delta_stdev); + // Compute trimmed mean (average excluding outliers) + double theta_trim = 0.0, delta_trim = 0.0; + unsigned int trim = 0; + for(unsigned int i = 0; i < NTP_AVERGAGE_COUNT; i++) + { + // Skip invalid values + if(fabs(ntp[i].theta) < ntp[i].precision || + fabs(ntp[i].delta) < ntp[i].precision) + continue; + + // Skip outliers + // We consider values > 2 standard deviations from the mean as + // outliers + if(fabs(ntp[i].theta - theta_avg) > 2 * theta_stdev || + fabs(ntp[i].delta - delta_avg) > 2 * delta_stdev) + continue; + + theta_trim += ntp[i].theta; + delta_trim += ntp[i].delta; + trim++; + } + theta_trim /= trim; + delta_trim /= trim; + + log_info("Trimmed mean time offset: %e s (excluded %u outliers)", theta_trim, NTP_AVERGAGE_COUNT - trim); + log_info("Trimmed mean round-trip delay: %e s (excluded %u outliers)", delta_trim, NTP_AVERGAGE_COUNT - trim); + // Set time if requested if(settime) { @@ -332,7 +359,7 @@ bool ntp_client(const char *server, const bool settime) // Convert from double to native format (signed) and add to the // current time. Note the addition is done in native format to // avoid overflow or loss of precision. - const uint64_t ntp_time = U2LFP(unix_time) + D2LFP(theta_avg); + const uint64_t ntp_time = U2LFP(unix_time) + D2LFP(theta_trim); // Convert NTP to native format unix_time.tv_sec = NTPtoSEC(ntp_time); From de45ba840af9a98b5ac89c9c66e69c0c99fcfa28 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 4 Jun 2024 13:17:09 +0200 Subject: [PATCH 11/46] Add CAP_SYS_TIME to required capabilities to set the system time without being root Signed-off-by: DL6ER --- src/CMakeLists.txt | 2 +- src/capabilities.c | 7 +++++++ test/test_suite.bats | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8d823bd0..008e6d7d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -375,5 +375,5 @@ find_program(SETCAP setcap) install(TARGETS pihole-FTL RUNTIME DESTINATION bin PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE) -install(CODE "execute_process(COMMAND ${SETCAP} CAP_NET_BIND_SERVICE,CAP_NET_RAW,CAP_NET_ADMIN,CAP_SYS_NICE,CAP_CHOWN+eip \$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/bin/pihole-FTL)") +install(CODE "execute_process(COMMAND ${SETCAP} CAP_NET_BIND_SERVICE,CAP_NET_RAW,CAP_NET_ADMIN,CAP_SYS_NICE,CAP_CHOWN,CAP_SYS_TIME+eip \$ENV{DESTDIR}\${CMAKE_INSTALL_PREFIX}/bin/pihole-FTL)") diff --git a/src/capabilities.c b/src/capabilities.c index 79d0532e..57631edb 100644 --- a/src/capabilities.c +++ b/src/capabilities.c @@ -141,6 +141,13 @@ bool check_capabilities(void) log_warn("Required Linux capability CAP_CHOWN not available"); capabilities_okay = false; } + if (!(data->permitted & (1 << CAP_SYS_TIME)) || + !(data->effective & (1 << CAP_SYS_TIME))) + { + // Necessary for setting the system time in the NTP client + log_warn("Required Linux capability CAP_SYS_TIME not available"); + capabilities_okay = false; + } // Free allocated memory free(hdr); diff --git a/test/test_suite.bats b/test/test_suite.bats index f16e4d7f..0456c18e 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -487,7 +487,7 @@ } @test "No WARNING messages in FTL.log (besides known warnings)" { - run bash -c 'grep "WARNING:" /var/log/pihole/FTL.log | grep -v -E "CAP_NET_ADMIN|CAP_NET_RAW|CAP_SYS_NICE|CAP_IPC_LOCK|CAP_CHOWN|CAP_NET_BIND_SERVICE|(Cannot set process priority)|FTLCONF_"' + run bash -c 'grep "WARNING:" /var/log/pihole/FTL.log | grep -v -E "CAP_NET_ADMIN|CAP_NET_RAW|CAP_SYS_NICE|CAP_IPC_LOCK|CAP_CHOWN|CAP_NET_BIND_SERVICE|CAP_SYS_TIME|(Cannot set process priority)|FTLCONF_"' printf "%s\n" "${lines[@]}" [[ "${lines[@]}" == "" ]] } From 12758da50b47de2536e6c43deb02fbc52a444268 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 4 Jun 2024 17:48:58 +0200 Subject: [PATCH 12/46] Use David L. Mills' clock adjustment algorithm (RFC 5905) for gradual clock adjustments if the deviation is small Signed-off-by: DL6ER --- src/ntp/client.c | 92 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 69 insertions(+), 23 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 7120fc54..a3b5fd7f 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -29,6 +29,8 @@ #include // config struct #include "config/config.h" +// ntp_adjtime() +#include struct ntp_sync { @@ -114,6 +116,61 @@ void print_debug_time(const char *label, const uint32_t *u32p, const uint64_t nt (timevar >> 32) & 0xFFFFFFFF, timevar & 0xFFFFFFFF, time_str); } +static bool settime_step(const double offset) +{ + // Get current time + struct timeval unix_time; + gettimeofday(&unix_time, NULL); + + // Convert from double to native format (signed) and add to the + // current time. Note the addition is done in native format to + // avoid overflow or loss of precision. + const uint64_t ntp_time = U2LFP(unix_time) + D2LFP(offset); + + // Convert NTP to native format + unix_time.tv_sec = NTPtoSEC(ntp_time); + unix_time.tv_usec = NTPtoUSEC(ntp_time); + log_debug(DEBUG_NTP, "Stepping system time by %e s", offset); + + // Set time immediately + if(settimeofday(&unix_time, NULL) != 0) + { + log_err("Failed to set time: %s", + errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); + return false; + } + + return true; +} + +static bool settime_skew(const double offset) +{ + // Gradually adjust time using ntp_adjtime() using David + // L. Mills' clock adjustment algorithm (see RFC 5905) + // Deviations will only gradually be corrected at + // maximum slew rate of 500ppm (0.05%), i.e., no faster + // than a correction of 500 microseconds per second, or, in + // other words, 1000 seconds (16 minutes and 40 seconds) to + // correct a 0.5 second offset. + struct timex tx; + memset(&tx, 0, sizeof(tx)); + + // Set mode to adjust time offset + tx.modes = MOD_CLKA | MOD_MICRO; + tx.offset = 1e6 * offset; // Convert to microseconds + log_debug(DEBUG_NTP, "Gradually adjusting system time by %li usec within the next %.1f seconds)", + tx.offset, fabs(1e6 * offset / 500)); + + if(ntp_adjtime(&tx) < 0) + { + log_err("Failed to adjust time: %s", + errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); + return false; + } + + return true; +} + static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) { // NTP Packet buffer @@ -352,31 +409,20 @@ bool ntp_client(const char *server, const bool settime) // Set time if requested if(settime) { - // Get current time - struct timeval unix_time; - gettimeofday(&unix_time, NULL); + // If the clock deviates more than 0.5 seconds from the NTP server, + // the time is updated immediately. Otherwise, the time is updated + // gradually to avoid sudden jumps in the system clock. + // The threshold of 0.5 seconds is hard-wired into the kernel + // since Linux 2.6.26, see man ntp_adjtime(2) for details. + bool success; + if(fabs(theta_trim) > 0.5) + success = settime_step(theta_trim); + else + success = settime_skew(theta_trim); - // Convert from double to native format (signed) and add to the - // current time. Note the addition is done in native format to - // avoid overflow or loss of precision. - const uint64_t ntp_time = U2LFP(unix_time) + D2LFP(theta_trim); - - // Convert NTP to native format - unix_time.tv_sec = NTPtoSEC(ntp_time); - unix_time.tv_usec = NTPtoUSEC(ntp_time); - - // Print new time - char time_str[TIMESTR_SIZE]; - format_NTP_time(time_str, ntp_time); - log_info("Setting local time to: %s", time_str); - - // Set time - if(settimeofday(&unix_time, NULL) != 0) - { - log_err("Failed to set time: %s", - errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); + // Return early if time could not be set + if(!success) return false; - } } // Offset and delay larger than 0.1 seconds are considered as invalid From 3971e67849ef731f087b8489f24e16b4e9a1c700 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 4 Jun 2024 23:32:07 +0200 Subject: [PATCH 13/46] Add NTP background synchronization Signed-off-by: DL6ER --- src/api/docs/content/specs/config.yaml | 13 +++ src/config/config.c | 19 ++++ src/config/config.h | 5 + src/dnsmasq_interface.c | 3 + src/ntp/client.c | 130 ++++++++++++++++++++----- src/ntp/ntp.h | 3 + test/pihole.toml | 16 ++- test/test_suite.bats | 2 +- 8 files changed, 162 insertions(+), 29 deletions(-) diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index c1b4edfa..9901324d 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -345,6 +345,15 @@ components: address: type: string x-format: ipv6 + sync: + type: object + properties: + server: + type: string + interval: + type: integer + count: + type: integer resolver: type: object properties: @@ -687,6 +696,10 @@ components: ipv6: active: true address: "" + sync: + server: "pool.ntp.org" + interval: 3600 + count: 8 resolver: resolveIPv4: true resolveIPv6: true diff --git a/src/config/config.c b/src/config/config.c index a7f29c02..df4d7ef4 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -832,6 +832,25 @@ void initConfig(struct config *conf) memset(&conf->ntp.ipv6.address.d.in6_addr, 0, sizeof(struct in6_addr)); conf->ntp.ipv6.address.c = validate_stub; // Only type-based checking + conf->ntp.sync.server.k = "ntp.sync.server"; + conf->ntp.sync.server.h = "NTP server (hostname, IPv4 or IPv6) to sync with, e.g., \"pool.ntp.org\" or \"[2001:4860:4860::8888]\""; + conf->ntp.sync.server.a = cJSON_CreateStringReference("valid NTP upstream server"); + conf->ntp.sync.server.t = CONF_STRING; + conf->ntp.sync.server.d.s = (char*)"pool.ntp.org"; + conf->ntp.sync.server.c = validate_stub; // Only type-based checking + + conf->ntp.sync.interval.k = "ntp.sync.interval"; + conf->ntp.sync.interval.h = "Interval in seconds to sync with the NTP server"; + conf->ntp.sync.interval.t = CONF_UINT; + conf->ntp.sync.interval.d.ui = 3600; + conf->ntp.sync.interval.c = validate_stub; // Only type-based checking + + conf->ntp.sync.count.k = "ntp.sync.count"; + conf->ntp.sync.count.h = "Number of NTP syncs to perform and average before updating the system time"; + conf->ntp.sync.count.t = CONF_UINT; + conf->ntp.sync.count.d.ui = 8; + conf->ntp.sync.count.c = validate_stub; // Only type-based checking + // struct resolver conf->resolver.resolveIPv6.k = "resolver.resolveIPv6"; diff --git a/src/config/config.h b/src/config/config.h index 6c000146..88bdefd9 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -200,6 +200,11 @@ struct config { struct conf_item active; struct conf_item address; } ipv6; + struct { + struct conf_item server; + struct conf_item interval; + struct conf_item count; + } sync; } ntp; struct { diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index cc527ca4..58384edd 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -2899,6 +2899,9 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw, bool dnsmasq_start) // Initialize NTP server ntp_server_start(); + // Start NTP sync thread + ntp_start_sync_thread(); + // We will use the attributes object later to start all threads in // detached mode pthread_attr_t attr; diff --git a/src/ntp/client.c b/src/ntp/client.c index a3b5fd7f..5ce79a87 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -29,8 +29,8 @@ #include // config struct #include "config/config.h" -// ntp_adjtime() -#include +// adjtime() +#include struct ntp_sync { @@ -145,23 +145,41 @@ static bool settime_step(const double offset) static bool settime_skew(const double offset) { - // Gradually adjust time using ntp_adjtime() using David - // L. Mills' clock adjustment algorithm (see RFC 5905) - // Deviations will only gradually be corrected at - // maximum slew rate of 500ppm (0.05%), i.e., no faster - // than a correction of 500 microseconds per second, or, in - // other words, 1000 seconds (16 minutes and 40 seconds) to - // correct a 0.5 second offset. - struct timex tx; - memset(&tx, 0, sizeof(tx)); + // This function gradually adjusts the system clock. + // + // If the adjustment in delta is positive, then the system clock is + // speeded up by some small percentage (i.e., by adding a small amount + // of time to the clock value in each second) until the adjustment has + // been completed. If the adjustment in delta is negative, then the + // clock is slowed down in a similar fashion. + // + // If a clock adjustment from an earlier adjtime() call is already in + // progress at the time of a later adjtime() call, and delta is not NULL + // for the later call, then the earlier adjustment is stopped, but any + // al‐ ready completed part of that adjustment is not undone. + // + // The adjustment that adjtime() makes to the clock is carried out in + // such a manner that the clock is always monotonically increasing. + // Using adjtime() to adjust the time prevents the problems that can be + // caused for certain applications (e.g., make(1)) by abrupt positive or + // negative jumps in the system time. + // + // adjtime() is intended to be used to make small adjustments to the + // system time. The actual time adjustment rate is implementation-specific + // but is typically on the order of 500 ppm, i.e., 0.5 ms/s. + struct timeval tx; + tx.tv_sec = (long int)offset; + tx.tv_usec = (offset - tx.tv_sec) * 1e6; + if(tx.tv_usec < 0) + { + // Adjust seconds if microseconds are negative + tx.tv_sec--; + tx.tv_usec += 1000000000; + } + log_debug(DEBUG_NTP, "Gradually adjusting system time by %li.%06li s", + (long int)tx.tv_sec, (long int)tx.tv_usec); - // Set mode to adjust time offset - tx.modes = MOD_CLKA | MOD_MICRO; - tx.offset = 1e6 * offset; // Convert to microseconds - log_debug(DEBUG_NTP, "Gradually adjusting system time by %li usec within the next %.1f seconds)", - tx.offset, fabs(1e6 * offset / 500)); - - if(ntp_adjtime(&tx) < 0) + if(adjtime(&tx, NULL) < 0) { log_err("Failed to adjust time: %s", errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); @@ -314,14 +332,23 @@ bool ntp_client(const char *server, const bool settime) } freeaddrinfo(saddr); - struct ntp_sync ntp[NTP_AVERGAGE_COUNT]; - memset(&ntp, 0, sizeof(ntp)); - for(unsigned int i = 0; i < NTP_AVERGAGE_COUNT; i++) + // Send and receive NTP packets + const unsigned int count = config.ntp.sync.count.v.ui; + struct ntp_sync *ntp = calloc(count, sizeof(struct ntp_sync)); + if(ntp == NULL) + { + log_err("Cannot allocate memory for NTP client\n"); + close(s); + return false; + } + memset(ntp, 0, count*sizeof(*ntp)); + for(unsigned int i = 0; i < count; i++) { // Send request if(!request(s, &ntp[i])) { close(s); + free(ntp); return false; } // Get reply @@ -342,7 +369,7 @@ bool ntp_client(const char *server, const bool settime) unsigned int valid = 0; double theta_avg = 0.0, theta_stdev = 0.0; double delta_avg = 0.0, delta_stdev = 0.0; - for(unsigned int i = 0; i < NTP_AVERGAGE_COUNT; i++) + for(unsigned int i = 0; i < count; i++) { // Skip invalid values if(fabs(ntp[i].theta) < ntp[i].precision || @@ -357,13 +384,14 @@ bool ntp_client(const char *server, const bool settime) if(valid == 0) { log_err("No valid NTP replies received, check server and network connectivity\n"); + free(ntp); return false; } - log_info("Received %u/%d valid NTP replies\n", valid, NTP_AVERGAGE_COUNT); + log_info("Received %u/%u valid NTP replies\n", valid, count); theta_avg /= valid; delta_avg /= valid; - for(unsigned int i = 0; i < NTP_AVERGAGE_COUNT; i++) + for(unsigned int i = 0; i < count; i++) { // Skip invalid values if(fabs(ntp[i].theta) < ntp[i].precision || @@ -382,7 +410,7 @@ bool ntp_client(const char *server, const bool settime) // Compute trimmed mean (average excluding outliers) double theta_trim = 0.0, delta_trim = 0.0; unsigned int trim = 0; - for(unsigned int i = 0; i < NTP_AVERGAGE_COUNT; i++) + for(unsigned int i = 0; i < count; i++) { // Skip invalid values if(fabs(ntp[i].theta) < ntp[i].precision || @@ -403,8 +431,11 @@ bool ntp_client(const char *server, const bool settime) theta_trim /= trim; delta_trim /= trim; - log_info("Trimmed mean time offset: %e s (excluded %u outliers)", theta_trim, NTP_AVERGAGE_COUNT - trim); - log_info("Trimmed mean round-trip delay: %e s (excluded %u outliers)", delta_trim, NTP_AVERGAGE_COUNT - trim); + // Free allocated memory + free(ntp); + + log_info("Trimmed mean time offset: %e s (excluded %u outliers)", theta_trim, count - trim); + log_info("Trimmed mean round-trip delay: %e s (excluded %u outliers)", delta_trim, count - trim); // Set time if requested if(settime) @@ -429,3 +460,48 @@ bool ntp_client(const char *server, const bool settime) // during local testing (e.g., when the server is on the same machine) return theta_avg < 0.1 && delta_avg < 0.1; } + +static void *ntp_client_thread(void *arg) +{ + // Set thread name + pthread_setname_np(pthread_self(), "NTP sync"); + + // Run NTP client + while(true) + { + // Run NTP client + if(ntp_client(config.ntp.sync.server.v.s, true)) + break; + + // Sleep before retrying + sleep(config.ntp.sync.interval.v.ui); + } + + return NULL; +} + +bool ntp_start_sync_thread(void) +{ + // Return early if NTP client is disabled + if(config.ntp.sync.server.v.s == NULL || + strlen(config.ntp.sync.server.v.s) == 0 || + config.ntp.sync.interval.v.ui == 0) + return false; + + // Create thread + pthread_t thread; + if(pthread_create(&thread, NULL, ntp_client_thread, NULL) != 0) + { + log_err("Cannot create NTP client thread\n"); + return false; + } + + // Detach thread + if(pthread_detach(thread) != 0) + { + log_err("Cannot detach NTP client thread\n"); + return false; + } + + return true; +} \ No newline at end of file diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index caca5169..363f2d3e 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -32,6 +32,9 @@ bool ntp_server_start(void); // Start NTP client bool ntp_client(const char *server, const bool settime); +// Start NTP sync thread +bool ntp_start_sync_thread(void); + // Number of NTP queries to average. The more queries, the more accurate the // time, but the longer it takes to synchronize. The minimum is 1. #define NTP_AVERGAGE_COUNT 8 diff --git a/test/pihole.toml b/test/pihole.toml index 9e5b06fb..f98191d6 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -480,6 +480,20 @@ # or empty string ("") for wildcard (::) address = "" + [ntp.sync] + # NTP server (hostname, IPv4 or IPv6) to sync with, e.g., "pool.ntp.org" or + # "[2001:4860:4860::8888]" + # + # Possible values are: + # valid NTP upstream server + server = "pool.ntp.org" + + # Interval in seconds to sync with the NTP server + interval = 3600 + + # Number of NTP syncs to perform and average before updating the system time + count = 8 + [resolver] # Should FTL try to resolve IPv4 addresses to hostnames? resolveIPv4 = false ### CHANGED, default = true @@ -1062,7 +1076,7 @@ all = true ### CHANGED, default = false # Configuration statistics: -# 141 total entries out of which 86 entries are default +# 144 total entries out of which 89 entries are default # --> 55 entries are modified # 2 entries are forced through environment: # - misc.nice diff --git a/test/test_suite.bats b/test/test_suite.bats index 0456c18e..5969ba79 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -1171,7 +1171,7 @@ @test "No ERROR messages in FTL.log (besides known/intended error)" { run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" - run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log | grep -c -v -E "(index\.html)|(Failed to create shared memory object)|(FTLCONF_debug_api is invalid)"' + run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log | grep -c -v -E "(index\.html)|(Failed to create shared memory object)|(FTLCONF_debug_api is invalid)|(Failed to adjust time: Insufficient permissions)"' printf "count: %s\n" "${lines[@]}" [[ ${lines[0]} == "0" ]] } From a872c03091404b51d2b99f869cce20fb791b6084 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 4 Jun 2024 23:48:44 +0200 Subject: [PATCH 14/46] Fix formating error Signed-off-by: DL6ER --- src/ntp/client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 5ce79a87..995dd26f 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -274,7 +274,7 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) if(ntp->delta < ntp->precision) ntp->delta = 0; -# // Return early if not verbose + // Return early if not verbose if(!config.debug.ntp.v.b) return true; @@ -504,4 +504,4 @@ bool ntp_start_sync_thread(void) } return true; -} \ No newline at end of file +} From 2269caeb9a2a33208f5294cc5a8e3dd4ffb6f9d6 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 4 Jun 2024 23:56:11 +0200 Subject: [PATCH 15/46] Make NTP sync thread cancelable Signed-off-by: DL6ER --- src/database/database-thread.c | 1 - src/enums.h | 1 + src/ntp/client.c | 19 ++++++++++++++----- src/signals.h | 2 ++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/database/database-thread.c b/src/database/database-thread.c index efdd1bd6..81e80484 100644 --- a/src/database/database-thread.c +++ b/src/database/database-thread.c @@ -78,7 +78,6 @@ static bool analyze_database(sqlite3 *db) } #define DBOPEN_OR_AGAIN() { if(!db) db = dbopen(false, false); if(!db) { thread_sleepms(DB, 5000); continue; } } -#define BREAK_IF_KILLED() { if(killed) break; } #define DBCLOSE_OR_BREAK() { dbclose(&db); BREAK_IF_KILLED(); } void *DB_thread(void *val) diff --git a/src/enums.h b/src/enums.h index 67dd1fbc..d8977f83 100644 --- a/src/enums.h +++ b/src/enums.h @@ -252,6 +252,7 @@ enum thread_types { DNSclient, CONF_READER, TIMER, + NTP, THREADS_MAX } __attribute__ ((packed)); diff --git a/src/ntp/client.c b/src/ntp/client.c index 995dd26f..1c3c031b 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -31,7 +31,8 @@ #include "config/config.h" // adjtime() #include - +// thread_names[] +#include "signals.h" struct ntp_sync { uint64_t org; @@ -464,19 +465,27 @@ bool ntp_client(const char *server, const bool settime) static void *ntp_client_thread(void *arg) { // Set thread name + thread_names[NTP] = "ntp-client"; + thread_running[NTP] = true; + prctl(PR_SET_NAME, thread_names[DB], 0, 0, 0); pthread_setname_np(pthread_self(), "NTP sync"); // Run NTP client - while(true) + while(!killed) { // Run NTP client - if(ntp_client(config.ntp.sync.server.v.s, true)) - break; + ntp_client(config.ntp.sync.server.v.s, true); + + // Intermediate cancellation-point + BREAK_IF_KILLED(); // Sleep before retrying - sleep(config.ntp.sync.interval.v.ui); + thread_sleepms(NTP, 1000 * config.ntp.sync.interval.v.ui); } + log_info("Terminating NTP thread"); + thread_running[NTP] = false; + return NULL; } diff --git a/src/signals.h b/src/signals.h index 4a08e4b9..f52fb2d9 100644 --- a/src/signals.h +++ b/src/signals.h @@ -32,4 +32,6 @@ extern volatile sig_atomic_t thread_cancellable[THREADS_MAX]; extern volatile sig_atomic_t thread_running[THREADS_MAX]; extern const char *thread_names[THREADS_MAX]; +#define BREAK_IF_KILLED() { if(killed) break; } + #endif //SIGNALS_H From f088b79e8f7bf60d4f02ba2c310571019f5636b8 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 6 Jun 2024 06:13:52 +0200 Subject: [PATCH 16/46] Exit synchronization early if no trimmed time offest average is avalable, print progress only when printing to the CLI, and increase delay between successive NTP requests to 0.5 seconds Signed-off-by: DL6ER --- src/args.c | 2 +- src/ntp/client.c | 68 +++++++++++++++++++++++++----------------------- src/ntp/ntp.h | 5 +++- 3 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/args.c b/src/args.c index 622c894b..f6e252a1 100644 --- a/src/args.c +++ b/src/args.c @@ -319,7 +319,7 @@ void parse_args(int argc, char* argv[]) const char *server = "127.0.0.1"; if(argc > 2 && strcmp(argv[2], "--update") != 0) server = argv[2]; - exit(ntp_client(server, update) ? EXIT_SUCCESS : EXIT_FAILURE); + exit(ntp_client(server, update, true) ? EXIT_SUCCESS : EXIT_FAILURE); } // Import teleporter archive through CLI diff --git a/src/ntp/client.c b/src/ntp/client.c index 1c3c031b..55c7ce14 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -67,7 +67,8 @@ static bool request(int fd, struct ntp_sync *ntp) // Send request if(send(fd, buf, 48, 0) != 48) { - log_err("Failed to send data to NTP server: %s", strerror(errno)); + log_err("Failed to send data to NTP server: %s", + errno == EAGAIN ? "Timeout" : strerror(errno)); return false; } @@ -171,14 +172,7 @@ static bool settime_skew(const double offset) struct timeval tx; tx.tv_sec = (long int)offset; tx.tv_usec = (offset - tx.tv_sec) * 1e6; - if(tx.tv_usec < 0) - { - // Adjust seconds if microseconds are negative - tx.tv_sec--; - tx.tv_usec += 1000000000; - } - log_debug(DEBUG_NTP, "Gradually adjusting system time by %li.%06li s", - (long int)tx.tv_sec, (long int)tx.tv_usec); + log_debug(DEBUG_NTP, "Gradually adjusting system time by %.3f ms", 1e3 * offset); if(adjtime(&tx, NULL) < 0) { @@ -198,7 +192,8 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // Receive reply if(recv(fd, buf, 48, 0) < 48) { - log_err("Failed to receive data from NTP server: %s", strerror(errno)); + log_err("Failed to receive data from NTP server: %s", + errno == EAGAIN ? "Timeout" : strerror(errno)); return false; } @@ -235,14 +230,15 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // network byte order if(ntp->org != org) { - log_warn("Received NTP reply does not match request (request %"PRIx64", reply %"PRIx64")", ntp->org, org); + log_warn("Received NTP reply does not match request (request %"PRIx64", reply %"PRIx64"), ignoring", + ntp->org, org); return false; } // Check stratum, mode, version, etc. if((buf[0] & 0x07) != 4) { - log_warn("Received NTP reply has invalid version"); + log_warn("Received NTP reply has invalid version, ignoring"); return false; } @@ -292,7 +288,7 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) return true; } -bool ntp_client(const char *server, const bool settime) +bool ntp_client(const char *server, const bool settime, const bool print) { const int protocol = strchr(server, ':') != NULL ? AF_INET6 : AF_INET; @@ -300,26 +296,26 @@ bool ntp_client(const char *server, const bool settime) const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); if(s == -1) { - log_err("Cannot create UDP socket\n"); + log_err("Cannot create UDP socket"); return false; } - // Set socket timeout to 2 seconds + // Set socket timeout to 5 seconds struct timeval tv; - tv.tv_sec = 2; + tv.tv_sec = 5; tv.tv_usec = 0; if(setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) != 0) { - log_err("Cannot set socket timeout\n"); + log_err("Cannot set socket timeout"); close(s); return false; } // Resolve server address struct addrinfo *saddr; - if(getaddrinfo(server, "123", NULL, &saddr) != 0) + if(getaddrinfo(server, "ntp", NULL, &saddr) != 0) { - log_err("Cannot resolve NTP server address\n"); + log_err("Cannot resolve NTP server address"); close(s); return false; } @@ -327,7 +323,7 @@ bool ntp_client(const char *server, const bool settime) // Set address to send to/receive from if(connect(s, saddr->ai_addr, saddr->ai_addrlen) != 0) { - log_err("Cannot connect to NTP server\n"); + log_err("Cannot connect to NTP server"); close(s); return false; } @@ -338,7 +334,7 @@ bool ntp_client(const char *server, const bool settime) struct ntp_sync *ntp = calloc(count, sizeof(struct ntp_sync)); if(ntp == NULL) { - log_err("Cannot allocate memory for NTP client\n"); + log_err("Cannot allocate memory for NTP client"); close(s); return false; } @@ -356,12 +352,14 @@ bool ntp_client(const char *server, const bool settime) if(!reply(s, &ntp[i], false)) continue; - // Sleep for 100 ms to avoid flooding the server - printf("."); + // Sleep for some time to avoid flooding the server + if(print) + printf("."); fflush(stdout); - usleep(100000); + usleep(NTP_DELAY); } - printf("\n"); + if(print) + printf("\n"); // Close socket close(s); @@ -384,11 +382,11 @@ bool ntp_client(const char *server, const bool settime) if(valid == 0) { - log_err("No valid NTP replies received, check server and network connectivity\n"); + log_warn("No valid NTP replies received, check server and network connectivity"); free(ntp); return false; } - log_info("Received %u/%u valid NTP replies\n", valid, count); + log_info("Received %u/%u valid NTP replies", valid, count); theta_avg /= valid; delta_avg /= valid; @@ -429,12 +427,18 @@ bool ntp_client(const char *server, const bool settime) delta_trim += ntp[i].delta; trim++; } - theta_trim /= trim; - delta_trim /= trim; // Free allocated memory free(ntp); + if(trim == 0) + { + log_warn("No valid NTP replies after outlier removal, check server and network connectivity"); + return false; + } + theta_trim /= trim; + delta_trim /= trim; + log_info("Trimmed mean time offset: %e s (excluded %u outliers)", theta_trim, count - trim); log_info("Trimmed mean round-trip delay: %e s (excluded %u outliers)", delta_trim, count - trim); @@ -474,7 +478,7 @@ static void *ntp_client_thread(void *arg) while(!killed) { // Run NTP client - ntp_client(config.ntp.sync.server.v.s, true); + ntp_client(config.ntp.sync.server.v.s, true, false); // Intermediate cancellation-point BREAK_IF_KILLED(); @@ -501,14 +505,14 @@ bool ntp_start_sync_thread(void) pthread_t thread; if(pthread_create(&thread, NULL, ntp_client_thread, NULL) != 0) { - log_err("Cannot create NTP client thread\n"); + log_err("Cannot create NTP client thread"); return false; } // Detach thread if(pthread_detach(thread) != 0) { - log_err("Cannot detach NTP client thread\n"); + log_err("Cannot detach NTP client thread"); return false; } diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index 363f2d3e..34d6f050 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -30,7 +30,7 @@ void print_debug_time(const char *label, const uint32_t *u32p, const uint64_t nt bool ntp_server_start(void); // Start NTP client -bool ntp_client(const char *server, const bool settime); +bool ntp_client(const char *server, const bool settime, const bool print); // Start NTP sync thread bool ntp_start_sync_thread(void); @@ -39,6 +39,9 @@ bool ntp_start_sync_thread(void); // time, but the longer it takes to synchronize. The minimum is 1. #define NTP_AVERGAGE_COUNT 8 +// Delay between consecutive NTP queries in microseconds +#define NTP_DELAY 500000 + // number of seconds between 1900 and 1970 (MSB=1) #define DIFF_SEC_1900_1970 (2208988800UL) // number of seconds between 1970 and Feb 7, 2036 (6:28:16 UTC) (MSB=0) From ee8f9899ddb21ed093c445217518882d3e460527 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 6 Jun 2024 07:21:37 +0200 Subject: [PATCH 17/46] Add NTP settings category to the API and create all threads in detached mode Signed-off-by: DL6ER --- src/api/config.c | 1 + src/dnsmasq_interface.c | 14 ++++++++------ src/ntp/client.c | 14 ++++---------- src/ntp/ntp.h | 4 ++-- src/ntp/server.c | 8 +++----- src/tools/arp-scan.c | 2 ++ src/tools/dhcp-discover.c | 2 ++ 7 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/api/config.c b/src/api/config.c index f413e502..aa881a82 100644 --- a/src/api/config.c +++ b/src/api/config.c @@ -37,6 +37,7 @@ static struct { { { "dns", "DNS", "DNS server settings" }, { "dhcp", "DHCP", "DHCP server settings" }, + { "ntp", "NTP", "Network Time Sync settings" }, { "resolver", "Resolver", "Resolver settings" }, { "database", "Database", "Database settings" }, { "webserver", "HTTP/API", "Webserver and API settings" }, diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 58384edd..14a82284 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -2896,17 +2896,19 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw, bool dnsmasq_start) // so they will not listen to real-time signals handle_realtime_signals(); - // Initialize NTP server - ntp_server_start(); - - // Start NTP sync thread - ntp_start_sync_thread(); - // We will use the attributes object later to start all threads in // detached mode pthread_attr_t attr; // Initialize thread attributes object with default attribute values pthread_attr_init(&attr); + // Set thread attributes to detached mode + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); + + // Initialize NTP server + ntp_server_start(&attr); + + // Start NTP sync thread + ntp_start_sync_thread(&attr); // Start database thread if database is used if(pthread_create( &threads[DB], &attr, DB_thread, NULL ) != 0) diff --git a/src/ntp/client.c b/src/ntp/client.c index 55c7ce14..46cc9e0b 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -31,6 +31,8 @@ #include "config/config.h" // adjtime() #include +// threads[] +#include "daemon.h" // thread_names[] #include "signals.h" struct ntp_sync @@ -493,7 +495,7 @@ static void *ntp_client_thread(void *arg) return NULL; } -bool ntp_start_sync_thread(void) +bool ntp_start_sync_thread(pthread_attr_t *attr) { // Return early if NTP client is disabled if(config.ntp.sync.server.v.s == NULL || @@ -502,19 +504,11 @@ bool ntp_start_sync_thread(void) return false; // Create thread - pthread_t thread; - if(pthread_create(&thread, NULL, ntp_client_thread, NULL) != 0) + if(pthread_create(&threads[NTP], attr, ntp_client_thread, NULL) != 0) { log_err("Cannot create NTP client thread"); return false; } - // Detach thread - if(pthread_detach(thread) != 0) - { - log_err("Cannot detach NTP client thread"); - return false; - } - return true; } diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index 34d6f050..3fde4c07 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -27,13 +27,13 @@ uint64_t gettime64(void); void print_debug_time(const char *label, const uint32_t *u32p, const uint64_t ntp_time); // Start NTP server -bool ntp_server_start(void); +bool ntp_server_start(pthread_attr_t *attr); // Start NTP client bool ntp_client(const char *server, const bool settime, const bool print); // Start NTP sync thread -bool ntp_start_sync_thread(void); +bool ntp_start_sync_thread(pthread_attr_t *attr); // Number of NTP queries to average. The more queries, the more accurate the // time, but the longer it takes to synchronize. The minimum is 1. diff --git a/src/ntp/server.c b/src/ntp/server.c index f6e70937..294ff005 100644 --- a/src/ntp/server.c +++ b/src/ntp/server.c @@ -353,7 +353,7 @@ static void *ntp_bind_and_listen(void *param) } // Start the NTP server -bool ntp_server_start(void) +bool ntp_server_start(pthread_attr_t *attr) { // Spawn two pthreads, one for IPv4 and one for IPv6 @@ -362,7 +362,7 @@ bool ntp_server_start(void) { // Create a thread for the IPv4 NTP server pthread_t thread; - if (pthread_create(&thread, NULL, ntp_bind_and_listen, (void *)0) != 0) + if (pthread_create(&thread, attr, ntp_bind_and_listen, (void *)0) != 0) { log_err("Can not create NTP server thread for IPv4"); return false; @@ -374,14 +374,12 @@ bool ntp_server_start(void) { // Create a thread for the IPv6 NTP server pthread_t thread; - if (pthread_create(&thread, NULL, ntp_bind_and_listen, (void *)1) != 0) + if (pthread_create(&thread, attr, ntp_bind_and_listen, (void *)1) != 0) { log_err("Can not create NTP server thread for IPv6"); return false; } } - sleep(10); - return true; } diff --git a/src/tools/arp-scan.c b/src/tools/arp-scan.c index 923625a3..c5ac6ec3 100644 --- a/src/tools/arp-scan.c +++ b/src/tools/arp-scan.c @@ -616,6 +616,8 @@ int run_arp_scan(const bool scan_all, const bool extreme_mode) pthread_attr_t attr; // Initialize thread attributes object with default attribute values pthread_attr_init(&attr); + // Set thread attributes to detached mode + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); struct ifaddrs *addrs, *tmp; getifaddrs(&addrs); diff --git a/src/tools/dhcp-discover.c b/src/tools/dhcp-discover.c index c68a74f9..045c8c14 100644 --- a/src/tools/dhcp-discover.c +++ b/src/tools/dhcp-discover.c @@ -725,6 +725,8 @@ int run_dhcp_discover(void) pthread_attr_t attr; // Initialize thread attributes object with default attribute values pthread_attr_init(&attr); + // Set thread attributes to detached mode + pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); // Create processing/printfing lock pthread_mutexattr_t lock_attr; From d923904291b1ddfbbac65ff5f38f939e7949a64e Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 6 Jun 2024 07:32:23 +0200 Subject: [PATCH 18/46] Tweak config option description Signed-off-by: DL6ER --- src/config/config.c | 8 ++++---- test/pihole.toml | 11 ++++++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/config/config.c b/src/config/config.c index df4d7ef4..a8ad6e0f 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -803,7 +803,7 @@ void initConfig(struct config *conf) // struct ntp conf->ntp.ipv4.active.k = "ntp.ipv4.active"; - conf->ntp.ipv4.active.h = "Should FTL act as an NTP server (IPv4)?"; + conf->ntp.ipv4.active.h = "Should FTL act as network time protocol (NTP) server (IPv4)?"; conf->ntp.ipv4.active.t = CONF_BOOL; conf->ntp.ipv4.active.f = FLAG_RESTART_FTL; conf->ntp.ipv4.active.d.b = true; @@ -818,7 +818,7 @@ void initConfig(struct config *conf) conf->ntp.ipv4.address.c = validate_stub; // Only type-based checking conf->ntp.ipv6.active.k = "ntp.ipv6.active"; - conf->ntp.ipv6.active.h = "Should FTL act as an NTP server (IPv6)?"; + conf->ntp.ipv6.active.h = "Should FTL act as network time protocol (NTP) server (IPv6)?"; conf->ntp.ipv6.active.t = CONF_BOOL; conf->ntp.ipv6.active.f = FLAG_RESTART_FTL; conf->ntp.ipv6.active.d.b = true; @@ -833,14 +833,14 @@ void initConfig(struct config *conf) conf->ntp.ipv6.address.c = validate_stub; // Only type-based checking conf->ntp.sync.server.k = "ntp.sync.server"; - conf->ntp.sync.server.h = "NTP server (hostname, IPv4 or IPv6) to sync with, e.g., \"pool.ntp.org\" or \"[2001:4860:4860::8888]\""; + conf->ntp.sync.server.h = "NTP upstream server to sync with, e.g., \"pool.ntp.org\". Note that the NTP server should be located as close as possible to you in order to minimize the time offset possibly introduced by different routing paths."; conf->ntp.sync.server.a = cJSON_CreateStringReference("valid NTP upstream server"); conf->ntp.sync.server.t = CONF_STRING; conf->ntp.sync.server.d.s = (char*)"pool.ntp.org"; conf->ntp.sync.server.c = validate_stub; // Only type-based checking conf->ntp.sync.interval.k = "ntp.sync.interval"; - conf->ntp.sync.interval.h = "Interval in seconds to sync with the NTP server"; + conf->ntp.sync.interval.h = "Interval in seconds between successive syncronization attempts with the NTP server"; conf->ntp.sync.interval.t = CONF_UINT; conf->ntp.sync.interval.d.ui = 3600; conf->ntp.sync.interval.c = validate_stub; // Only type-based checking diff --git a/test/pihole.toml b/test/pihole.toml index f98191d6..b765e555 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -461,7 +461,7 @@ hosts = [] [ntp.ipv4] - # Should FTL act as an NTP server (IPv4)? + # Should FTL act as network time protocol (NTP) server (IPv4)? active = true # IPv4 address to listen on for NTP requests @@ -471,7 +471,7 @@ address = "" [ntp.ipv6] - # Should FTL act as an NTP server (IPv6)? + # Should FTL act as network time protocol (NTP) server (IPv6)? active = true # IPv6 address to listen on for NTP requests @@ -481,14 +481,15 @@ address = "" [ntp.sync] - # NTP server (hostname, IPv4 or IPv6) to sync with, e.g., "pool.ntp.org" or - # "[2001:4860:4860::8888]" + # NTP upstream server to sync with, e.g., "pool.ntp.org". Note that the NTP server + # should be located as close as possible to you in order to minimize the time offset + # possibly introduced by different routing paths. # # Possible values are: # valid NTP upstream server server = "pool.ntp.org" - # Interval in seconds to sync with the NTP server + # Interval in seconds between successive syncronization attempts with the NTP server interval = 3600 # Number of NTP syncs to perform and average before updating the system time From 08f2e37d9fd7e112b5bb28c2f99b415ab21d05c9 Mon Sep 17 00:00:00 2001 From: Dominik Date: Thu, 6 Jun 2024 08:54:31 +0200 Subject: [PATCH 19/46] Apply suggestions from code review Co-authored-by: RD WebDesign Signed-off-by: Dominik --- src/config/config.c | 2 +- test/pihole.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config/config.c b/src/config/config.c index a8ad6e0f..19b17086 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -840,7 +840,7 @@ void initConfig(struct config *conf) conf->ntp.sync.server.c = validate_stub; // Only type-based checking conf->ntp.sync.interval.k = "ntp.sync.interval"; - conf->ntp.sync.interval.h = "Interval in seconds between successive syncronization attempts with the NTP server"; + conf->ntp.sync.interval.h = "Interval in seconds between successive synchronization attempts with the NTP server"; conf->ntp.sync.interval.t = CONF_UINT; conf->ntp.sync.interval.d.ui = 3600; conf->ntp.sync.interval.c = validate_stub; // Only type-based checking diff --git a/test/pihole.toml b/test/pihole.toml index b765e555..d29dc427 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -489,7 +489,7 @@ # valid NTP upstream server server = "pool.ntp.org" - # Interval in seconds between successive syncronization attempts with the NTP server + # Interval in seconds between successive synchronization attempts with the NTP server interval = 3600 # Number of NTP syncs to perform and average before updating the system time From 93f751f90a278facc08a48cb87ac60c777d36853 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 7 Jun 2024 18:49:11 +0200 Subject: [PATCH 20/46] Use adjtimex instead of adjtime as the latter uses the former (see http://git.musl-libc.org/cgit/musl/tree/src/linux/adjtime.c and https://codebrowser.dev/glibc/glibc/time/adjtime.c.html). Also add comment from man rtc(4) about how RTCs are updated at the same time Signed-off-by: DL6ER --- src/ntp/client.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 46cc9e0b..3b331062 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -35,6 +35,8 @@ #include "daemon.h" // thread_names[] #include "signals.h" +// adjtimex() +#include struct ntp_sync { uint64_t org; @@ -151,6 +153,7 @@ static bool settime_skew(const double offset) { // This function gradually adjusts the system clock. // + // Linux uses David L. Mills' clock adjustment algorithm (see RFC 5905). // If the adjustment in delta is positive, then the system clock is // speeded up by some small percentage (i.e., by adding a small amount // of time to the clock value in each second) until the adjustment has @@ -160,23 +163,30 @@ static bool settime_skew(const double offset) // If a clock adjustment from an earlier adjtime() call is already in // progress at the time of a later adjtime() call, and delta is not NULL // for the later call, then the earlier adjustment is stopped, but any - // al‐ ready completed part of that adjustment is not undone. + // already completed part of that adjustment is not undone. // - // The adjustment that adjtime() makes to the clock is carried out in + // The adjustment that adjtimex() makes to the clock is carried out in // such a manner that the clock is always monotonically increasing. - // Using adjtime() to adjust the time prevents the problems that can be + // Using adjtimex() to adjust the time prevents the problems that can be // caused for certain applications (e.g., make(1)) by abrupt positive or // negative jumps in the system time. // - // adjtime() is intended to be used to make small adjustments to the + // adjtimex() is intended to be used to make small adjustments to the // system time. The actual time adjustment rate is implementation-specific // but is typically on the order of 500 ppm, i.e., 0.5 ms/s. - struct timeval tx; - tx.tv_sec = (long int)offset; - tx.tv_usec = (offset - tx.tv_sec) * 1e6; - log_debug(DEBUG_NTP, "Gradually adjusting system time by %.3f ms", 1e3 * offset); + // + // man rtc(4) adds: + // When the kernel's system time is synchronized with an external + // reference using adjtimex() it will update a designated RTC + // periodically every 11 minutes. - if(adjtime(&tx, NULL) < 0) + struct timex tx = { 0 }; + tx.offset = 1000000 * offset; + tx.modes = ADJ_OFFSET_SINGLESHOT; + + log_debug(DEBUG_NTP, "Gradually adjusting system time by %ld us", tx.offset); + + if(adjtimex(&tx) < 0) { log_err("Failed to adjust time: %s", errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); From 671771ceb0fde2bd4ec2f5b9e6bc14fc269aecef Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 7 Jun 2024 18:50:14 +0200 Subject: [PATCH 21/46] Improve NTP synchronization by rejecting synchronization if the standard deviation of the time offset or round-trip delay is larger than 1 second. This ensures the time cannot go off even in cases where the network connectivity is really bad Signed-off-by: DL6ER --- src/ntp/client.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/ntp/client.c b/src/ntp/client.c index 3b331062..b860a90b 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -418,6 +418,15 @@ bool ntp_client(const char *server, const bool settime, const bool print) log_info("Average time offset: (%e +/- %e s)", theta_avg, theta_stdev); log_info("Average round-trip delay: (%e +/- %e s)", delta_avg, delta_stdev); + // Reject synchronization if the standard deviation of the time offset + // or round-trip delay is larger than 1 second + if(theta_stdev > 1.0 || delta_stdev > 1.0) + { + log_warn("Standard deviation of time offset is too large, rejecting synchronization"); + free(ntp); + return false; + } + // Compute trimmed mean (average excluding outliers) double theta_trim = 0.0, delta_trim = 0.0; unsigned int trim = 0; From 791e3a80979655e3ab8e80415b702c02a3fbdf37 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 7 Jun 2024 20:01:04 +0200 Subject: [PATCH 22/46] Add RTC synchronization Signed-off-by: DL6ER --- src/api/docs/content/specs/config.yaml | 13 ++ src/config/config.c | 19 ++ src/config/config.h | 5 + src/ntp/CMakeLists.txt | 1 + src/ntp/client.c | 4 + src/ntp/ntp.h | 3 + src/ntp/rtc.c | 296 +++++++++++++++++++++++++ test/pihole.toml | 15 +- 8 files changed, 355 insertions(+), 1 deletion(-) create mode 100644 src/ntp/rtc.c diff --git a/src/api/docs/content/specs/config.yaml b/src/api/docs/content/specs/config.yaml index 9901324d..dfc4ae7d 100644 --- a/src/api/docs/content/specs/config.yaml +++ b/src/api/docs/content/specs/config.yaml @@ -354,6 +354,15 @@ components: type: integer count: type: integer + rtc: + type: object + properties: + set: + type: boolean + device: + type: string + utc: + type: boolean resolver: type: object properties: @@ -700,6 +709,10 @@ components: server: "pool.ntp.org" interval: 3600 count: 8 + rtc: + set: true + device: "" + utc: true resolver: resolveIPv4: true resolveIPv6: true diff --git a/src/config/config.c b/src/config/config.c index 19b17086..d1e51857 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -851,6 +851,25 @@ void initConfig(struct config *conf) conf->ntp.sync.count.d.ui = 8; conf->ntp.sync.count.c = validate_stub; // Only type-based checking + conf->ntp.rtc.set.k = "ntp.rtc.set"; + conf->ntp.rtc.set.h = "Should FTL update a real-time clock (RTC) if available?"; + conf->ntp.rtc.set.t = CONF_BOOL; + conf->ntp.rtc.set.d.b = true; + conf->ntp.rtc.set.c = validate_stub; // Only type-based checking + + conf->ntp.rtc.device.k = "ntp.rtc.device"; + conf->ntp.rtc.device.h = "Path to the RTC device to update. Leave empty for auto-discovery"; + conf->ntp.rtc.device.a = cJSON_CreateStringReference("Path to the RTC device, e.g., \"/dev/rtc0\""); + conf->ntp.rtc.device.t = CONF_STRING; + conf->ntp.rtc.device.d.s = (char*)""; + conf->ntp.rtc.device.c = validate_stub; // Only type-based checking + + conf->ntp.rtc.utc.k = "ntp.rtc.utc"; + conf->ntp.rtc.utc.h = "Should the RTC be set to UTC?"; + conf->ntp.rtc.utc.t = CONF_BOOL; + conf->ntp.rtc.utc.d.b = true; + conf->ntp.rtc.utc.c = validate_stub; // Only type-based checking + // struct resolver conf->resolver.resolveIPv6.k = "resolver.resolveIPv6"; diff --git a/src/config/config.h b/src/config/config.h index 88bdefd9..3a68fa8e 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -205,6 +205,11 @@ struct config { struct conf_item interval; struct conf_item count; } sync; + struct { + struct conf_item set; + struct conf_item device; + struct conf_item utc; + } rtc; } ntp; struct { diff --git a/src/ntp/CMakeLists.txt b/src/ntp/CMakeLists.txt index 7eca589a..5cdc5d12 100644 --- a/src/ntp/CMakeLists.txt +++ b/src/ntp/CMakeLists.txt @@ -11,6 +11,7 @@ set(ntp_sources server.c client.c + rtc.c ntp.h ) diff --git a/src/ntp/client.c b/src/ntp/client.c index b860a90b..a23b37ae 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -480,6 +480,10 @@ bool ntp_client(const char *server, const bool settime, const bool print) // Return early if time could not be set if(!success) return false; + + // Finally, adjust RTC if configured + if(config.ntp.rtc.set.v.b) + ntp_sync_rtc(); } // Offset and delay larger than 0.1 seconds are considered as invalid diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index 3fde4c07..b87a7878 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -35,6 +35,9 @@ bool ntp_client(const char *server, const bool settime, const bool print); // Start NTP sync thread bool ntp_start_sync_thread(pthread_attr_t *attr); +// Sync RTC time +bool ntp_sync_rtc(void); + // Number of NTP queries to average. The more queries, the more accurate the // time, but the longer it takes to synchronize. The minimum is 1. #define NTP_AVERGAGE_COUNT 8 diff --git a/src/ntp/rtc.c b/src/ntp/rtc.c new file mode 100644 index 00000000..abeb457e --- /dev/null +++ b/src/ntp/rtc.c @@ -0,0 +1,296 @@ +/* Pi-hole: A black hole for Internet advertisements +* (c) 2024 Pi-hole, LLC (https://pi-hole.net) +* Network-wide ad blocking via your own hardware. +* +* FTL Engine +* Real Time Clock (RTC) functions +* The routines in this file have been inspired by man pages +* and the source of the hwclock which is part of the util-linux +* project (https://github.com/util-linux/util-linux/) +* +* This file is copyright under the latest version of the EUPL. +* Please see LICENSE file for your rights under this license. */ + +#include "ntp/ntp.h" + +// ioctl() +#include +// RTC +#include +// O_WRONLY +#include +// struct config +#include "config/config.h" + +// List of RTC devices from +// https://github.com/util-linux/util-linux/blob/41e7686c9ad1ea7892b9d8941c266869bf6a28dd/sys-utils/hwclock-rtc.c#L85-L93 +static const char * const rtc_devices[] = { +#ifdef __ia64__ + "/dev/efirtc", + "/dev/misc/efirtc", +#endif + "/dev/rtc0", + "/dev/rtc", + "/dev/misc/rtc" +}; + +static void print_tm_time(const char *label, const struct tm *tm) +{ + char timestr[TIMESTR_SIZE] = { 0 }; + strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S", tm); + log_info("%s %s", label, timestr); +} + +// Try to find the RTC device and open it +static int open_rtc(void) +{ + int rtc_fd = -1; + + // Get current user's UID and GID + const uid_t uid = getuid(); + const gid_t gid = getgid(); + + // If the user has specified an RTC device, try to open it + if(config.ntp.rtc.device.v.s != NULL && + strlen(config.ntp.rtc.device.v.s) > 0) + { + // Open the RTC device + rtc_fd = open(config.ntp.rtc.device.v.s, O_RDONLY); + if (rtc_fd != -1) + { + log_debug(DEBUG_NTP, "Successfully opened RTC at \"%s\"", + config.ntp.rtc.device.v.s); + return rtc_fd; + } + + // If the open failed because of permissions, try to change them + // momentarily. On some embedded systems, the RTC device is owned by + // root exclusively and users do not have permission to even open it. + // Without being able to access the RTC, the capability to set the + // time (CAP_SYS_TIME) is useless. + if(errno == EACCES) + { + // Get current owner of the device + struct stat st = { 0 }; + if(stat(config.ntp.rtc.device.v.s, &st) == -1) + { + log_debug(DEBUG_NTP, "stat(\"%s\") failed: %s", + config.ntp.rtc.device.v.s, strerror(errno)); + return -1; + } + + if(chown(config.ntp.rtc.device.v.s, uid, gid) == -1) + { + log_debug(DEBUG_NTP, "chown(\"%s\", %u, %u) failed: %s", + config.ntp.rtc.device.v.s, uid, gid, strerror(errno)); + return -1; + } + + rtc_fd = open(config.ntp.rtc.device.v.s, O_RDONLY); + if (rtc_fd != -1) + { + log_debug(DEBUG_NTP, "Successfully opened RTC at \"%s\"", + config.ntp.rtc.device.v.s); + } + + // Chown the device back to the original owner + if(chown(config.ntp.rtc.device.v.s, st.st_uid, st.st_gid) == -1) + { + log_debug(DEBUG_NTP, "chown(\"%s\", %u, %u) failed: %s", + config.ntp.rtc.device.v.s, st.st_uid, st.st_gid, strerror(errno)); + return -1; + } + + // Return the RTC file descriptor (can be -1) + return rtc_fd; + } + + log_debug(DEBUG_NTP, "Failed to open RTC at \"%s\": %s", + config.ntp.rtc.device.v.s, strerror(errno)); + + return -1; + } + + // If the user has not specified an RTC device, try to open the default + // ones + for(size_t i = 0; i < ArraySize(rtc_devices); i++) + { + rtc_fd = open(rtc_devices[i], O_RDONLY); + if (rtc_fd != -1) + { + log_debug(DEBUG_NTP, "Successfully opened RTC at \"%s\"", + rtc_devices[i]); + break; + } + + // If the open failed because of permissions, try to change them + // momentarily + if(errno == EACCES) + { + // Get current owner of the device + struct stat st = { 0 }; + if(stat(rtc_devices[i], &st) == -1) + { + log_debug(DEBUG_NTP, "stat(\"%s\") failed: %s", + rtc_devices[i], strerror(errno)); + return -1; + } + + if(chown(rtc_devices[i], uid, gid) == -1) + { + log_debug(DEBUG_NTP, "chown(\"%s\", %u, %u) failed: %s", + rtc_devices[i], uid, gid, strerror(errno)); + return -1; + } + + rtc_fd = open(rtc_devices[i], O_RDONLY); + if (rtc_fd != -1) + { + log_debug(DEBUG_NTP, "Successfully opened RTC at \"%s\"", + rtc_devices[i]); + } + + // Chown the device back to the original owner + if(chown(rtc_devices[i], st.st_uid, st.st_gid) == -1) + { + log_debug(DEBUG_NTP, "chown(\"%s\", %u, %u) failed: %s", + rtc_devices[i], st.st_uid, st.st_gid, strerror(errno)); + return -1; + } + + // Return the RTC file descriptor (can be -1) + return rtc_fd; + } + + log_debug(DEBUG_NTP, "Failed to open RTC at \"%s\": %s", + rtc_devices[i], strerror(errno)); + } + + return rtc_fd; +} + +static bool read_rtc(struct tm *tm) +{ + // Open the RTC device + const int rtc_fd = open_rtc(); + if(rtc_fd == -1) + return false; + + // Read the RTC time + struct rtc_time rtc_tm = { 0 }; + const int rc = ioctl(rtc_fd, RTC_RD_TIME, &rtc_tm); + if(rc == -1) + { + log_debug(DEBUG_NTP, "ioctl(RTC_RD_NAME) failed: %s", + strerror(errno)); + close(rtc_fd); + return false; + } + + // Convert the kernel's struct tm to the standard struct tm + tm->tm_sec = rtc_tm.tm_sec; + tm->tm_min = rtc_tm.tm_min; + tm->tm_hour = rtc_tm.tm_hour; + tm->tm_mday = rtc_tm.tm_mday; + tm->tm_mon = rtc_tm.tm_mon; + tm->tm_year = rtc_tm.tm_year; + tm->tm_wday = rtc_tm.tm_wday; + tm->tm_yday = rtc_tm.tm_yday; + tm->tm_isdst = -1; // the RTC does not provide this information + print_tm_time("Current RTC time is", tm); + + // Close the RTC device + close(rtc_fd); + + return true; +} + +// Set the Hardware Clock to the broken down time . +// Use ioctls to "rtc" device to set the time. +static bool set_rtc(const struct tm *new_time) +{ + // Open the RTC device + const int rtc_fd = open_rtc(); + if(rtc_fd == -1) + return false; + + // Set the RTC time from the broken down time + struct rtc_time rtc_tm = { 0 }; + rtc_tm.tm_sec = new_time->tm_sec; + rtc_tm.tm_min = new_time->tm_min; + rtc_tm.tm_hour = new_time->tm_hour; + rtc_tm.tm_mday = new_time->tm_mday; + rtc_tm.tm_mon = new_time->tm_mon; + rtc_tm.tm_year = new_time->tm_year; + rtc_tm.tm_wday = new_time->tm_wday; + rtc_tm.tm_yday = new_time->tm_yday; + rtc_tm.tm_isdst = new_time->tm_isdst; + + // Set the RTC time + const int rc = ioctl(rtc_fd, RTC_SET_TIME, &rtc_tm); + if(rc == -1) + { + log_debug(DEBUG_NTP, "ioctl(RTC_SET_TIME) failed: %s", + strerror(errno)); + close(rtc_fd); + return false; + } + print_tm_time("RTC time set to", new_time); + + // Close the RTC device + close(rtc_fd); + return true; +} + +bool ntp_sync_rtc(void) +{ + // Wait until the beginning of the next second as the RTC only has a + // resolution of one second + struct timespec ts = { 0 }; + clock_gettime(CLOCK_REALTIME, &ts); + ts.tv_sec++; + ts.tv_nsec = 0; + clock_nanosleep(CLOCK_REALTIME, TIMER_ABSTIME, &ts, NULL); + + // Time to which we will set Hardware Clock, in broken down format + struct tm new_time = { 0 }; + const time_t newtime = time(NULL); + if(config.ntp.rtc.utc.v.b) + // UTC + gmtime_r(&newtime, &new_time); + else + // Local time + localtime_r(&newtime, &new_time); + + // Read the current time from the RTC + struct tm rtc_time = { 0 }; + if(!read_rtc(&rtc_time)) + { + log_debug(DEBUG_NTP, "Failed to read RTC time"); + return false; + } + + // If the RTC time is the same as the current time, we don't need to set + // it. We don't use memcmp() here because the tm struct may contain + // additional fields that are not filled in by the RTC (e.g. tm_isdst). + if(rtc_time.tm_sec == new_time.tm_sec && + rtc_time.tm_min == new_time.tm_min && + rtc_time.tm_hour == new_time.tm_hour && + rtc_time.tm_mday == new_time.tm_mday && + rtc_time.tm_mon == new_time.tm_mon && + rtc_time.tm_year == new_time.tm_year) + { + // The RTC time is already correct, return early + log_debug(DEBUG_NTP, "RTC time is already correct"); + return true; + } + + // Set the RTC time + if(!set_rtc(&new_time)) + { + log_debug(DEBUG_NTP, "Failed to set RTC time"); + return false; + } + + return true; +} diff --git a/test/pihole.toml b/test/pihole.toml index d29dc427..681e3a0c 100644 --- a/test/pihole.toml +++ b/test/pihole.toml @@ -495,6 +495,19 @@ # Number of NTP syncs to perform and average before updating the system time count = 8 + [ntp.rtc] + # Should FTL update a real-time clock (RTC) if available? + set = true + + # Path to the RTC device to update. Leave empty for auto-discovery + # + # Possible values are: + # Path to the RTC device, e.g., "/dev/rtc0" + device = "" + + # Should the RTC be set to UTC? + utc = true + [resolver] # Should FTL try to resolve IPv4 addresses to hostnames? resolveIPv4 = false ### CHANGED, default = true @@ -1077,7 +1090,7 @@ all = true ### CHANGED, default = false # Configuration statistics: -# 144 total entries out of which 89 entries are default +# 147 total entries out of which 92 entries are default # --> 55 entries are modified # 2 entries are forced through environment: # - misc.nice From a05bf8dd24439f5faca14b4b0aeca91b059eff5a Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 10 Jun 2024 16:57:11 +0200 Subject: [PATCH 23/46] Improve shutdown sequence of threads Signed-off-by: DL6ER --- src/daemon.c | 10 +++++++--- src/ntp/client.c | 3 +-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index 0345e046..f329f1ae 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -272,9 +272,14 @@ static void terminate_threads(void) log_info("Waiting for threads to join"); for(int i = 0; i < THREADS_MAX; i++) { + log_debug(DEBUG_EXTRA, "Joining %s thread (%d)", thread_names[i], i); // Skip threads that have never been started or which are already stopped - if(!thread_running[i]) + if(threads[i] == 0 || !thread_running[i]) + { + log_debug(DEBUG_EXTRA, "Skipping thread as it %s", + threads[i] == 0 ? "was never started" : "is not running"); continue; + } // Cancel thread if it is idle if(thread_cancellable[i]) @@ -297,8 +302,7 @@ static void terminate_threads(void) ts.tv_sec += 2; // Try to join thread and cancel it if it is still busy - const int s = pthread_timedjoin_np(threads[i], NULL, &ts); - if(s != 0) + if(pthread_timedjoin_np(threads[i], NULL, &ts) != 0) { log_info("Thread %s (%d) is still busy, cancelling it.", thread_names[i], i); diff --git a/src/ntp/client.c b/src/ntp/client.c index a23b37ae..4f253cd2 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -496,8 +496,7 @@ static void *ntp_client_thread(void *arg) // Set thread name thread_names[NTP] = "ntp-client"; thread_running[NTP] = true; - prctl(PR_SET_NAME, thread_names[DB], 0, 0, 0); - pthread_setname_np(pthread_self(), "NTP sync"); + prctl(PR_SET_NAME, thread_names[NTP], 0, 0, 0); // Run NTP client while(!killed) From 27db8a43cec224f04a057a2cb65addc7e8ebffa5 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 10 Jun 2024 17:12:09 +0200 Subject: [PATCH 24/46] Copy root delay/dispersion errors from upstream server (after first upstream NTP synchronization) Signed-off-by: DL6ER --- src/ntp/client.c | 46 ++++++++++++++++++++++++++++++++++++---------- src/ntp/ntp.h | 4 ++++ src/ntp/server.c | 26 ++++++++++++++------------ 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 4f253cd2..dc3db113 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -122,24 +122,29 @@ void print_debug_time(const char *label, const uint32_t *u32p, const uint64_t nt (timevar >> 32) & 0xFFFFFFFF, timevar & 0xFFFFFFFF, time_str); } -static bool settime_step(const double offset) +static uint64_t get_new_time(struct timeval *unix_time, const double offset) { // Get current time - struct timeval unix_time; - gettimeofday(&unix_time, NULL); + gettimeofday(unix_time, NULL); // Convert from double to native format (signed) and add to the // current time. Note the addition is done in native format to // avoid overflow or loss of precision. - const uint64_t ntp_time = U2LFP(unix_time) + D2LFP(offset); + const uint64_t ntp_time = U2LFP(*unix_time) + D2LFP(offset); // Convert NTP to native format - unix_time.tv_sec = NTPtoSEC(ntp_time); - unix_time.tv_usec = NTPtoUSEC(ntp_time); + unix_time->tv_sec = NTPtoSEC(ntp_time); + unix_time->tv_usec = NTPtoUSEC(ntp_time); + + return ntp_time; +} + +static bool settime_step(struct timeval *unix_time, const double offset) +{ log_debug(DEBUG_NTP, "Stepping system time by %e s", offset); // Set time immediately - if(settimeofday(&unix_time, NULL) != 0) + if(settimeofday(unix_time, NULL) != 0) { log_err("Failed to set time: %s", errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); @@ -221,9 +226,16 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // Compute precision of server clock in seconds 2^rho ntp->precision = pow(2, rho); + // Get root delay and dispersion (in network-byte-order !) + memcpy(&ntp_root_delay, &buf[4], sizeof(ntp_root_delay)); + memcpy(&ntp_root_dispersion, &buf[8], sizeof(ntp_root_dispersion)); + // Extract Transmit Timestamp - // org = Origin Timestamp (Transmit Timestamp @ Client) uint64_t netbuffer; + // ref = Reference Timestamp (Time at which the clock was last set or corrected) + memcpy(&netbuffer, &buf[16], sizeof(netbuffer)); + const uint64_t ref = ntoh64(netbuffer); + // org = Origin Timestamp (Transmit Timestamp @ Client) memcpy(&netbuffer, &buf[24], sizeof(netbuffer)); const uint64_t org = ntoh64(netbuffer); // rec = Receive Timestamp (Receive Timestamp @ Server) @@ -287,6 +299,9 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) if(!config.debug.ntp.v.b) return true; + // Print current time at server + print_debug_time("Server reference time", NULL, ref); + // Print current time at client print_debug_time("Current time at client", NULL, dst); @@ -296,6 +311,10 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // Print offset and delay log_debug(DEBUG_NTP, "Time offset: %e s", ntp->theta); log_debug(DEBUG_NTP, "Round-trip delay: %e s", ntp->delta); + const uint32_t root_delay = ntohl(ntp_root_delay); + log_debug(DEBUG_NTP, "Root delay: %e s", LFP2D(root_delay)); + const uint32_t root_dispersion = ntohl(ntp_root_dispersion); + log_debug(DEBUG_NTP, "Root dispersion: %e s", LFP2D(root_dispersion)); return true; } @@ -398,7 +417,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) free(ntp); return false; } - log_info("Received %u/%u valid NTP replies", valid, count); + log_info("Received %u/%u valid NTP replies from %s", valid, count, server); theta_avg /= valid; delta_avg /= valid; @@ -466,6 +485,10 @@ bool ntp_client(const char *server, const bool settime, const bool print) // Set time if requested if(settime) { + // Calculate corrected time + struct timeval unix_time; + const uint64_t ntp_time = get_new_time(&unix_time, theta_trim); + // If the clock deviates more than 0.5 seconds from the NTP server, // the time is updated immediately. Otherwise, the time is updated // gradually to avoid sudden jumps in the system clock. @@ -473,7 +496,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) // since Linux 2.6.26, see man ntp_adjtime(2) for details. bool success; if(fabs(theta_trim) > 0.5) - success = settime_step(theta_trim); + success = settime_step(&unix_time, theta_trim); else success = settime_skew(theta_trim); @@ -481,6 +504,9 @@ bool ntp_client(const char *server, const bool settime, const bool print) if(!success) return false; + // Update last NTP sync time + ntp_last_sync = ntp_time; + // Finally, adjust RTC if configured if(config.ntp.rtc.set.v.b) ntp_sync_rtc(); diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index b87a7878..d8dbf451 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -65,6 +65,10 @@ bool ntp_sync_rtc(void); #define hton64(x) ((((uint64_t)htonl(x)) << 32) + htonl((x) >> 32)) #define ntoh64(x) ((((uint64_t)ntohl(x)) << 32) + ntohl((x) >> 32)) +extern uint64_t ntp_last_sync; +extern uint32_t ntp_root_delay; +extern uint32_t ntp_root_dispersion; + #endif // NTP_H diff --git a/src/ntp/server.c b/src/ntp/server.c index 294ff005..048a4bdf 100644 --- a/src/ntp/server.c +++ b/src/ntp/server.c @@ -38,6 +38,10 @@ // PRIi64 #include +uint64_t ntp_last_sync = 0u; +uint32_t ntp_root_delay = 0u; +uint32_t ntp_root_dispersion = 0u; + // RFC 5905 Appendix A.4: Kernel System Clock Interface uint64_t gettime64(void) { @@ -100,11 +104,12 @@ static bool ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // | Root Dispersion | // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // Assume Root Delay (total roundtrip delay to the primary reference - // source) = 0, Root Dispersion (the nominal error relative to the - // primary reference source) = 0 as we don't have these numbers - *u32p++ = 0.0; - *u32p++ = 0.0; + // Set Root Delay (total roundtrip delay to the primary reference + // source) and Root Dispersion (the nominal error relative to the + // primary reference source) to the values obtained from the upstream + // NTP server. These values are already in network byte order. + *u32p++ = ntp_root_delay; + *u32p++ = ntp_root_dispersion; // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -115,7 +120,7 @@ static bool ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // Reference ID = 'LOCL" (LOCAL CLOCK) // A four-octet, left-justified, zero-padded ASCII string assigned to // the reference clock - memcpy(u32p++, "LOCL", 4); + memcpy(u32p++, "LOCL", sizeof(uint32_t)); // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 @@ -126,12 +131,9 @@ static bool ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ // Time when the system clock was last set or corrected, in NTP - // timestamp format. As this is not a stratum 1 server, we don't have - // a hardware clock to set this value. - // A stateless server copies T3 and T4 from the client packet to T1 and - // T2 of the server packet and tacks on the transmit timestamp T3 before - // sending it to the client. - memcpy(u32p, &u32r[8], sizeof(uint64_t)); + // timestamp format. + const uint64_t last_sync = hton64(ntp_last_sync); + memcpy(u32p, &last_sync, sizeof(uint64_t)); if(config.debug.ntp.v.b) print_debug_time("Reference Timestamp", u32p, 0); u32p += 2; From fcc0a5ab2f19528b05817a3c37b7d14107d52ab3 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 10 Jun 2024 17:18:41 +0200 Subject: [PATCH 25/46] Use "fresh" sockets for NTP client requests to avoid reusing the same ephermal port for mulitple requests Signed-off-by: DL6ER --- src/ntp/client.c | 54 ++++++++++++++++++++++++++++++------------------ 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index dc3db113..d2e22879 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -319,16 +319,15 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) return true; } -bool ntp_client(const char *server, const bool settime, const bool print) +static int getsock(const struct addrinfo *saddr) { - const int protocol = strchr(server, ':') != NULL ? AF_INET6 : AF_INET; - // Create UDP socket + const int protocol = saddr->ai_addrlen == sizeof(struct sockaddr_in6) ? AF_INET6 : AF_INET; const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); if(s == -1) { log_err("Cannot create UDP socket"); - return false; + return -1; } // Set socket timeout to 5 seconds @@ -339,16 +338,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) { log_err("Cannot set socket timeout"); close(s); - return false; - } - - // Resolve server address - struct addrinfo *saddr; - if(getaddrinfo(server, "ntp", NULL, &saddr) != 0) - { - log_err("Cannot resolve NTP server address"); - close(s); - return false; + return -1; } // Set address to send to/receive from @@ -356,32 +346,56 @@ bool ntp_client(const char *server, const bool settime, const bool print) { log_err("Cannot connect to NTP server"); close(s); + return -1; + } + + // Return socket + return s; +} + +bool ntp_client(const char *server, const bool settime, const bool print) +{ + // Resolve server address + struct addrinfo *saddr; + if(getaddrinfo(server, "ntp", NULL, &saddr) != 0) + { + log_err("Cannot resolve NTP server address"); return false; } - freeaddrinfo(saddr); - // Send and receive NTP packets const unsigned int count = config.ntp.sync.count.v.ui; struct ntp_sync *ntp = calloc(count, sizeof(struct ntp_sync)); if(ntp == NULL) { log_err("Cannot allocate memory for NTP client"); - close(s); return false; } - memset(ntp, 0, count*sizeof(*ntp)); + + // Send and receive NTP packets for(unsigned int i = 0; i < count; i++) { + // Create socket + const int s = getsock(saddr); + if(s == -1) + continue; + // Send request if(!request(s, &ntp[i])) { close(s); free(ntp); + freeaddrinfo(saddr); return false; } // Get reply if(!reply(s, &ntp[i], false)) + { + close(s); continue; + } + + // Close socket + close(s); // Sleep for some time to avoid flooding the server if(print) @@ -392,8 +406,8 @@ bool ntp_client(const char *server, const bool settime, const bool print) if(print) printf("\n"); - // Close socket - close(s); + // Free allocated memory + freeaddrinfo(saddr); // Compute average and standard deviation unsigned int valid = 0; From d737bf524cbe341033950eb0027d346a6eaa523c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 10 Jun 2024 17:23:58 +0200 Subject: [PATCH 26/46] Determine root dispersion and error based on our own most recent time synchronization as described by RFC 5905, Scn. 4 (page 9) Signed-off-by: DL6ER --- src/ntp/client.c | 13 +++++++++---- src/ntp/ntp.h | 3 +++ src/ntp/server.c | 6 +++--- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index d2e22879..817f701f 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -226,10 +226,6 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // Compute precision of server clock in seconds 2^rho ntp->precision = pow(2, rho); - // Get root delay and dispersion (in network-byte-order !) - memcpy(&ntp_root_delay, &buf[4], sizeof(ntp_root_delay)); - memcpy(&ntp_root_dispersion, &buf[8], sizeof(ntp_root_dispersion)); - // Extract Transmit Timestamp uint64_t netbuffer; // ref = Reference Timestamp (Time at which the clock was last set or corrected) @@ -521,6 +517,15 @@ bool ntp_client(const char *server, const bool settime, const bool print) // Update last NTP sync time ntp_last_sync = ntp_time; + // Compute our server's root dispersion and delay + // Both quantities are the maximum error and maximum delay of + // the server's time relative to the reference time. The root + // dispersion is the maximum error of the server's time relative + // to the reference time, while the root delay is the maximum + // delay of the server's time relative to the reference time + ntp_root_delay = D2FP(theta_trim); + ntp_root_dispersion = D2FP(theta_stdev); + // Finally, adjust RTC if configured if(config.ntp.rtc.set.v.b) ntp_sync_rtc(); diff --git a/src/ntp/ntp.h b/src/ntp/ntp.h index d8dbf451..72445423 100644 --- a/src/ntp/ntp.h +++ b/src/ntp/ntp.h @@ -51,6 +51,9 @@ bool ntp_sync_rtc(void); #define DIFF_SEC_1970_2036 (2085978496UL) // Timestamp conversion macroni (RFC 5905, Appendix A) +#define FRIC 65536. // 2^16 as a double +#define D2FP(r) ((uint32_t)((r) * FRIC)) // NTP short +#define FP2D(r) ((double)(r) / FRIC) #define FRAC 4294967296. // 2^32 as double #define D2LFP(a) ((uint64_t)((a) * FRAC)) // NTP timestamp #define LFP2D(a) ((double)(a) / FRAC) diff --git a/src/ntp/server.c b/src/ntp/server.c index 048a4bdf..87b34831 100644 --- a/src/ntp/server.c +++ b/src/ntp/server.c @@ -107,9 +107,9 @@ static bool ntp_reply(const int socket_fd, const struct sockaddr *saddr_p, const // Set Root Delay (total roundtrip delay to the primary reference // source) and Root Dispersion (the nominal error relative to the // primary reference source) to the values obtained from the upstream - // NTP server. These values are already in network byte order. - *u32p++ = ntp_root_delay; - *u32p++ = ntp_root_dispersion; + // NTP server. + *u32p++ = htonl(ntp_root_delay); + *u32p++ = htonl(ntp_root_dispersion); // 0 1 2 3 // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 From b8eee89e07117d6ede18ff69eecc1aea7503115a Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 10 Jun 2024 19:35:40 +0200 Subject: [PATCH 27/46] Do not detach threads because we want to join them during shutdown Signed-off-by: DL6ER --- src/daemon.c | 3 ++- src/dnsmasq_interface.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index f329f1ae..a4d294ab 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -265,7 +265,6 @@ pid_t FTL_gettid(void) static void terminate_threads(void) { - struct timespec ts; // Terminate threads before closing database connections and finishing shared memory killed = true; // Try to join threads to ensure cancellation has succeeded @@ -290,6 +289,8 @@ static void terminate_threads(void) } // Cancel thread if we cannot set a timeout for joining + struct timespec ts; + memset(&ts, 0, sizeof(ts)); if (clock_gettime(CLOCK_REALTIME, &ts) == -1) { log_info("Thread %s (%d) is busy, cancelling it (cannot set timeout).", diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 14a82284..853fba2d 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -2900,9 +2900,9 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw, bool dnsmasq_start) // detached mode pthread_attr_t attr; // Initialize thread attributes object with default attribute values + // Do NOT detach threads as we want to join them during shutdown with a + // fixed timeout to give them time to clean up and finish their work pthread_attr_init(&attr); - // Set thread attributes to detached mode - pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED); // Initialize NTP server ntp_server_start(&attr); From 9bc0d4c25d38e2694244fef547d9c389f9806928 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 11 Jun 2024 04:18:55 +0200 Subject: [PATCH 28/46] Fix root delay/dispersion debug printing Signed-off-by: DL6ER --- src/ntp/client.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 817f701f..2028edc0 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -226,6 +226,11 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // Compute precision of server clock in seconds 2^rho ntp->precision = pow(2, rho); + // Extract root delay and root dispersion of server clock + uint32_t srv_root_delay, srv_root_dispersion; + memcpy(&srv_root_delay, &buf[4], sizeof(srv_root_delay)); + memcpy(&srv_root_dispersion, &buf[8], sizeof(srv_root_dispersion)); + // Extract Transmit Timestamp uint64_t netbuffer; // ref = Reference Timestamp (Time at which the clock was last set or corrected) @@ -307,10 +312,10 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // Print offset and delay log_debug(DEBUG_NTP, "Time offset: %e s", ntp->theta); log_debug(DEBUG_NTP, "Round-trip delay: %e s", ntp->delta); - const uint32_t root_delay = ntohl(ntp_root_delay); - log_debug(DEBUG_NTP, "Root delay: %e s", LFP2D(root_delay)); - const uint32_t root_dispersion = ntohl(ntp_root_dispersion); - log_debug(DEBUG_NTP, "Root dispersion: %e s", LFP2D(root_dispersion)); + const uint32_t root_delay = ntohl(srv_root_delay); + log_debug(DEBUG_NTP, "Root delay: %e s", FP2D(root_delay)); + const uint32_t root_dispersion = ntohl(srv_root_dispersion); + log_debug(DEBUG_NTP, "Root dispersion: %e s", FP2D(root_dispersion)); return true; } @@ -539,7 +544,6 @@ bool ntp_client(const char *server, const bool settime, const bool print) static void *ntp_client_thread(void *arg) { // Set thread name - thread_names[NTP] = "ntp-client"; thread_running[NTP] = true; prctl(PR_SET_NAME, thread_names[NTP], 0, 0, 0); From 90ba90a0c760d4a547beceb8f5d7e9bac384f92b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 11 Jun 2024 04:20:04 +0200 Subject: [PATCH 29/46] Pre-define thread names so they can always be shown during shutdown, even if a thread was never started, remove unused CONF_READER thread slot Signed-off-by: DL6ER --- src/database/database-thread.c | 1 - src/enums.h | 1 - src/gc.c | 1 - src/resolve.c | 1 - src/signals.c | 8 +++++++- src/signals.h | 2 +- 6 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/database/database-thread.c b/src/database/database-thread.c index 81e80484..f8768902 100644 --- a/src/database/database-thread.c +++ b/src/database/database-thread.c @@ -83,7 +83,6 @@ static bool analyze_database(sqlite3 *db) void *DB_thread(void *val) { // Set thread name - thread_names[DB] = "database"; thread_running[DB] = true; prctl(PR_SET_NAME, thread_names[DB], 0, 0, 0); diff --git a/src/enums.h b/src/enums.h index d8977f83..b9ad1443 100644 --- a/src/enums.h +++ b/src/enums.h @@ -250,7 +250,6 @@ enum thread_types { DB, GC, DNSclient, - CONF_READER, TIMER, NTP, THREADS_MAX diff --git a/src/gc.c b/src/gc.c index a142ca23..3d4f0962 100644 --- a/src/gc.c +++ b/src/gc.c @@ -481,7 +481,6 @@ static bool check_files_on_same_device(const char *path1, const char *path2) void *GC_thread(void *val) { // Set thread name - thread_names[GC] = "housekeeper"; thread_running[GC] = true; prctl(PR_SET_NAME, thread_names[GC], 0, 0, 0); diff --git a/src/resolve.c b/src/resolve.c index f77cadfb..453e477e 100644 --- a/src/resolve.c +++ b/src/resolve.c @@ -1053,7 +1053,6 @@ static void resolveUpstreams(const bool onlynew) void *DNSclient_thread(void *val) { // Set thread name - thread_names[DNSclient] = "DNS client"; thread_running[DNSclient] = true; prctl(PR_SET_NAME, thread_names[DNSclient], 0, 0, 0); diff --git a/src/signals.c b/src/signals.c index 9c638445..ec3b35f8 100644 --- a/src/signals.c +++ b/src/signals.c @@ -35,7 +35,13 @@ volatile int exit_code = EXIT_SUCCESS; volatile sig_atomic_t thread_cancellable[THREADS_MAX] = { false }; volatile sig_atomic_t thread_running[THREADS_MAX] = { false }; -const char *thread_names[THREADS_MAX] = { "" }; +const char * const thread_names[THREADS_MAX] = { + "database", + "housekeeper", + "DNS client", + "timer", + "NTP client" + }; // Return the (null-terminated) name of the calling thread // The name is stored in the buffer as well as returned for convenience diff --git a/src/signals.h b/src/signals.h index f52fb2d9..76664887 100644 --- a/src/signals.h +++ b/src/signals.h @@ -30,7 +30,7 @@ extern volatile sig_atomic_t want_to_reload_lists; extern volatile sig_atomic_t thread_cancellable[THREADS_MAX]; extern volatile sig_atomic_t thread_running[THREADS_MAX]; -extern const char *thread_names[THREADS_MAX]; +extern const char * const thread_names[THREADS_MAX]; #define BREAK_IF_KILLED() { if(killed) break; } From 2516dcf3e290074547e9fe1cb8db4bbe9b72333b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 11 Jun 2024 04:23:40 +0200 Subject: [PATCH 30/46] Always join threads if they have ever been started to avoid resource leaking Signed-off-by: DL6ER --- src/daemon.c | 5 ++--- src/signals.c | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/daemon.c b/src/daemon.c index a4d294ab..5e7cf2db 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -273,10 +273,9 @@ static void terminate_threads(void) { log_debug(DEBUG_EXTRA, "Joining %s thread (%d)", thread_names[i], i); // Skip threads that have never been started or which are already stopped - if(threads[i] == 0 || !thread_running[i]) + if(threads[i] == 0) { - log_debug(DEBUG_EXTRA, "Skipping thread as it %s", - threads[i] == 0 ? "was never started" : "is not running"); + log_debug(DEBUG_EXTRA, "Skipping thread as it was never started"); continue; } diff --git a/src/signals.c b/src/signals.c index ec3b35f8..8e14cfe2 100644 --- a/src/signals.c +++ b/src/signals.c @@ -38,9 +38,9 @@ volatile sig_atomic_t thread_running[THREADS_MAX] = { false }; const char * const thread_names[THREADS_MAX] = { "database", "housekeeper", - "DNS client", + "dns-client", "timer", - "NTP client" + "ntp-client" }; // Return the (null-terminated) name of the calling thread From 126d4d87ce84445d3f97a368a29182cc64fe36b8 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 12 Jun 2024 18:21:46 +0200 Subject: [PATCH 31/46] Mark timer thread as running Signed-off-by: DL6ER --- src/timers.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/timers.c b/src/timers.c index fe66d760..17b16704 100644 --- a/src/timers.c +++ b/src/timers.c @@ -84,7 +84,8 @@ void get_blockingmode_timer(double *delay, bool *target_status) void *timer(void *val) { // Set thread name - prctl(PR_SET_NAME, "int.timer", 0, 0, 0); + thread_running[GC] = true; + prctl(PR_SET_NAME, thread_names[TIMER], 0, 0, 0); // Save timestamp as we do not want to store immediately // to the database @@ -105,9 +106,11 @@ void *timer(void *val) set_blockingstatus(timer_target_status); timer_delay = -1.0; } - sleepms(SLEEPING_TIME * 1000); + thread_sleepms(TIMER, SLEEPING_TIME * 1000); } + log_info("Terminating timer thread"); + thread_running[GC] = false; return NULL; } From e70c364af40f71a3669a0a56f1f675f9f44aaa43 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 13 Jun 2024 06:26:28 +0200 Subject: [PATCH 32/46] Add message table entries for selected NTP warnings/errors Signed-off-by: DL6ER --- src/database/message-table.c | 60 ++++++++++++++++++++++++++++++++++++ src/database/message-table.h | 1 + src/enums.h | 1 + src/ntp/client.c | 59 +++++++++++++++++++++++++++-------- src/ntp/server.c | 30 ++++++++++++++---- 5 files changed, 133 insertions(+), 18 deletions(-) diff --git a/src/database/message-table.c b/src/database/message-table.c index 9271bd96..e6dab7de 100644 --- a/src/database/message-table.c +++ b/src/database/message-table.c @@ -97,6 +97,8 @@ static const char *get_message_type_str(const enum message_type type) return "CERTIFICATE_DOMAIN_MISMATCH"; case CONNECTION_ERROR_MESSAGE: return "CONNECTION_ERROR"; + case NTP_MESSAGE: + return "NTP"; case MAX_MESSAGE: default: return "UNKNOWN"; @@ -131,6 +133,8 @@ static enum message_type get_message_type_from_string(const char *typestr) return CERTIFICATE_DOMAIN_MISMATCH_MESSAGE; else if (strcmp(typestr, "CONNECTION_ERROR") == 0) return CONNECTION_ERROR_MESSAGE; + else if (strcmp(typestr, "NTP") == 0) + return NTP_MESSAGE; else return MAX_MESSAGE; } @@ -230,6 +234,14 @@ static unsigned char message_blob_types[MAX_MESSAGE][5] = SQLITE_NULL, // not used SQLITE_NULL, // not used SQLITE_NULL // not used + }, + { + // NTP: The message column contains the warning/error + SQLITE_TEXT, // level (warning/error) + SQLITE_TEXT, // component (server/client) + SQLITE_NULL, // not used + SQLITE_NULL, // not used + SQLITE_NULL // not used } }; // Create message table in the database @@ -900,6 +912,20 @@ static void format_connection_error(char *plain, const int sizeof_plain, char *h free(escaped_server); } +static void format_ntp_message(char *plain, const int sizeof_plain, char *html, const int sizeof_html, + const char *message, const char *level, const char *who) +{ + if(snprintf(plain, sizeof_plain, "%s NTP %s: %s", level, who, message) > sizeof_plain) + log_warn("format_ntp_message(): Buffer too small to hold plain message, warning truncated"); + + // Return early if HTML text is not required + if(sizeof_html < 1 || html == NULL) + return; + + if(snprintf(html, sizeof_html, "%s in NTP %s:
%s
", level, who, message) > sizeof_html) + log_warn("format_ntp_message(): Buffer too small to hold HTML message, warning truncated"); +} + int count_messages(const bool filter_dnsmasq_warnings) { int count = 0; @@ -1147,6 +1173,18 @@ bool format_messages(cJSON *array) break; } + case NTP_MESSAGE: + { + const char *message = (const char*)sqlite3_column_text(stmt, 3); + const char *level = (const char*)sqlite3_column_text(stmt, 4); + const char *who = (const char*)sqlite3_column_text(stmt, 5); + + format_ntp_message(plain, sizeof(plain), html, sizeof(html), + message, level, who); + + break; + } + case MAX_MESSAGE: // Fall through default: log_warn("format_messages() - Unknown message type: %s", mtypestr); @@ -1423,3 +1461,25 @@ void log_connection_error(const char *server, const char *reason, const char *er if(rowid == -1) log_err("logg_connection_error(): Failed to add message to database"); } + +void log_ntp_message(const bool error, const bool server, const char *message) +{ + const char *who = server ? "server" : "client"; + const char *level = error ? "Error" : "Warning"; + + // Create message + char buf[2048]; + format_ntp_message(buf, sizeof(buf), NULL, 0, message, level, who); + + // Log to FTL.log + if(error) + log_err("%s", buf); + else + log_warn("%s", buf); + + // Log to database + const int rowid = add_message(NTP_MESSAGE, message, level, who); + + if(rowid == -1) + log_err("log_ntp_message(): Failed to add message to database"); +} diff --git a/src/database/message-table.h b/src/database/message-table.h index d92bbe8d..5354af6f 100644 --- a/src/database/message-table.h +++ b/src/database/message-table.h @@ -30,5 +30,6 @@ void log_resource_shortage(const double load, const int nprocs, const int shmem, void logg_inaccessible_adlist(const int dbindex, const char *address); void log_certificate_domain_mismatch(const char *certfile, const char *domain); void log_connection_error(const char *server, const char *reason, const char *error); +void log_ntp_message(const bool error, const bool server, const char *message); #endif //MESSAGETABLE_H diff --git a/src/enums.h b/src/enums.h index b9ad1443..406e8f31 100644 --- a/src/enums.h +++ b/src/enums.h @@ -276,6 +276,7 @@ enum message_type { DISK_MESSAGE_EXTENDED, CERTIFICATE_DOMAIN_MISMATCH_MESSAGE, CONNECTION_ERROR_MESSAGE, + NTP_MESSAGE, MAX_MESSAGE, } __attribute__ ((packed)); diff --git a/src/ntp/client.c b/src/ntp/client.c index 2028edc0..c6d53c25 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -37,6 +37,8 @@ #include "signals.h" // adjtimex() #include +// log_ntp_message() +#include "database/message-table.h" struct ntp_sync { uint64_t org; @@ -146,8 +148,11 @@ static bool settime_step(struct timeval *unix_time, const double offset) // Set time immediately if(settimeofday(unix_time, NULL) != 0) { - log_err("Failed to set time: %s", - errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); + char errbuf[1024]; + strncpy(errbuf, "Failed to set time during NTP sync: ", sizeof(errbuf)); + strncat(errbuf, errno == EPERM ? "Insufficient permissions" : strerror(errno), sizeof(errbuf) - strlen(errbuf) - 1); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, false, errbuf); return false; } @@ -193,8 +198,11 @@ static bool settime_skew(const double offset) if(adjtimex(&tx) < 0) { - log_err("Failed to adjust time: %s", - errno == EPERM ? "Insufficient permissions, try running with sudo" : strerror(errno)); + char errbuf[1024]; + strncpy(errbuf, "Failed to adjust time during NTP sync: ", sizeof(errbuf)); + strncat(errbuf, errno == EPERM ? "Insufficient permissions" : strerror(errno), sizeof(errbuf) - strlen(errbuf) - 1); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, false, errbuf); return false; } @@ -220,7 +228,10 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) { // Accepted limits are 2^-32 (~ 0.2 nanoseconds) // to 2^0 (= 1 second) - log_warn("Received NTP reply has invalid precision: 2^(%i), assuming microsecond accuracy", rho); + char errbuf[1024]; + snprintf(errbuf, sizeof(errbuf), "Received NTP reply has invalid precision: 2^(%i), assuming microsecond accuracy", rho); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(false, false, errbuf); rho = -19; } // Compute precision of server clock in seconds 2^rho @@ -327,7 +338,11 @@ static int getsock(const struct addrinfo *saddr) const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); if(s == -1) { - log_err("Cannot create UDP socket"); + char errbuf[1024]; + strncpy(errbuf, "Cannot create UDP socket: ", sizeof(errbuf)); + strncat(errbuf, strerror(errno), sizeof(errbuf) - strlen(errbuf) - 1); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, false, errbuf); return -1; } @@ -337,7 +352,11 @@ static int getsock(const struct addrinfo *saddr) tv.tv_usec = 0; if(setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)) != 0) { - log_err("Cannot set socket timeout"); + char errbuf[1024]; + strncpy(errbuf, "Cannot set socket timeout: ", sizeof(errbuf)); + strncat(errbuf, strerror(errno), sizeof(errbuf) - strlen(errbuf) - 1); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, false, errbuf); close(s); return -1; } @@ -345,7 +364,11 @@ static int getsock(const struct addrinfo *saddr) // Set address to send to/receive from if(connect(s, saddr->ai_addr, saddr->ai_addrlen) != 0) { - log_err("Cannot connect to NTP server"); + char errbuf[1024]; + strncpy(errbuf, "Canot connect to NTP server: ", sizeof(errbuf)); + strncat(errbuf, strerror(errno), sizeof(errbuf) - strlen(errbuf) - 1); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, false, errbuf); close(s); return -1; } @@ -357,10 +380,22 @@ static int getsock(const struct addrinfo *saddr) bool ntp_client(const char *server, const bool settime, const bool print) { // Resolve server address + int eai; struct addrinfo *saddr; - if(getaddrinfo(server, "ntp", NULL, &saddr) != 0) + if((eai = getaddrinfo(server, "ntp", NULL, &saddr)) != 0) { - log_err("Cannot resolve NTP server address"); + char errbuf[1024]; + strncpy(errbuf, "Cannot resolve NTP server address: ", sizeof(errbuf)); + strncat(errbuf, errno == EAI_SYSTEM ? strerror(errno) : gai_strerror(eai), + sizeof(errbuf) - strlen(errbuf) - 1); + if(eai == EAI_NONAME || eai == EAI_NODATA) + { + strncat(errbuf, " \"", sizeof(errbuf) - strlen(errbuf) - 1); + strncat(errbuf, server, sizeof(errbuf) - strlen(errbuf) - 1); + strncat(errbuf, "\"", sizeof(errbuf) - strlen(errbuf) - 1); + } + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, false, errbuf); return false; } @@ -428,7 +463,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) if(valid == 0) { - log_warn("No valid NTP replies received, check server and network connectivity"); + log_ntp_message(false, false, "No valid NTP replies received, check server and network connectivity"); free(ntp); return false; } @@ -456,7 +491,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) // or round-trip delay is larger than 1 second if(theta_stdev > 1.0 || delta_stdev > 1.0) { - log_warn("Standard deviation of time offset is too large, rejecting synchronization"); + log_ntp_message(false, false, "Standard deviation of time offset is too large, rejecting synchronization"); free(ntp); return false; } diff --git a/src/ntp/server.c b/src/ntp/server.c index 87b34831..652c9525 100644 --- a/src/ntp/server.c +++ b/src/ntp/server.c @@ -37,6 +37,8 @@ #include "config/config.h" // PRIi64 #include +// log_ntp_message() +#include "database/message-table.h" uint64_t ntp_last_sync = 0u; uint32_t ntp_root_delay = 0u; @@ -285,8 +287,12 @@ static void *ntp_bind_and_listen(void *param) const int s = socket(protocol, SOCK_DGRAM, IPPROTO_UDP); if(s == -1) { - log_warn("Cannot create NTP socket (%s), IPv%i NTP server not available", + char errbuf[1024]; + snprintf(errbuf, sizeof(errbuf), + "Cannot create NTP socket (%s), IPv%i NTP server not available", strerror(errno), protocol == AF_INET ? 4 : 6); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, true, errbuf); return NULL; } @@ -310,8 +316,12 @@ static void *ntp_bind_and_listen(void *param) errno = 0; if(bind(s, (struct sockaddr *)&bind_addr, sizeof(bind_addr)) != 0) { - log_warn("Cannot bind to IPv4 address %s:123 (%s), IPv4 NTP server not available", + char errbuf[1024]; + snprintf(errbuf, sizeof(errbuf), + "Cannot bind to IPv4 address %s:123 (%s), IPv4 NTP server not available", ipstr, strerror(errno)); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, true, errbuf); return NULL; } } @@ -326,7 +336,11 @@ static void *ntp_bind_and_listen(void *param) int opt = 1; if(setsockopt(s, IPPROTO_IPV6, IPV6_V6ONLY, &opt, sizeof(opt)) != 0) { - log_warn("Cannot set socket option IPV6_V6ONLY (%s), IPv6 NTP server not available", strerror(errno)); + char errbuf[1024]; + strncpy(errbuf, "Cannot set socket option IPV6_V6ONLY, IPv6 NTP server not available: ", sizeof(errbuf)); + strncat(errbuf, strerror(errno), sizeof(errbuf) - strlen(errbuf) - 1); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, true, errbuf); return NULL; } @@ -342,8 +356,12 @@ static void *ntp_bind_and_listen(void *param) errno = 0; if(bind(s, (struct sockaddr *)&bind_addr, sizeof(bind_addr)) != 0) { - log_warn("Cannot bind to IPv6 address %s:123 (%s), IPv6 NTP server not available", + char errbuf[1024]; + snprintf(errbuf, sizeof(errbuf), + "Cannot bind to IPv6 address %s:123 (%s), IPv6 NTP server not available", ipstr, strerror(errno)); + errbuf[sizeof(errbuf) - 1] = '\0'; + log_ntp_message(true, true, errbuf); return NULL; } } @@ -366,7 +384,7 @@ bool ntp_server_start(pthread_attr_t *attr) pthread_t thread; if (pthread_create(&thread, attr, ntp_bind_and_listen, (void *)0) != 0) { - log_err("Can not create NTP server thread for IPv4"); + log_ntp_message(true, true, "Cannot create NTP server thread for IPv4"); return false; } } @@ -378,7 +396,7 @@ bool ntp_server_start(pthread_attr_t *attr) pthread_t thread; if (pthread_create(&thread, attr, ntp_bind_and_listen, (void *)1) != 0) { - log_err("Can not create NTP server thread for IPv6"); + log_ntp_message(true, true, "Cannot create NTP server thread for IPv6"); return false; } } From 3ae3afa1b529adccb3d47136ea4aef2dc2649003 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 13 Jun 2024 06:30:43 +0200 Subject: [PATCH 33/46] Fix harmless incorrect warning when generating HTML regex messages Signed-off-by: DL6ER --- src/database/message-table.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/database/message-table.c b/src/database/message-table.c index e6dab7de..81c4e3fa 100644 --- a/src/database/message-table.c +++ b/src/database/message-table.c @@ -549,7 +549,7 @@ static void format_regex_message(char *plain, const int sizeof_plain, char *html } if(snprintf(html, sizeof_html, "Encountered an error when processing regex %s filter with ID %d: 
%s
Error message: 
%s
", - dbindex, type, dbindex, escaped_regex, escaped_warning)) + dbindex, type, dbindex, escaped_regex, escaped_warning) > sizeof_html) log_warn("format_regex_message(): Buffer too small to hold HTML message, warning truncated"); free(escaped_regex); From e05d9314120953635a00b9c3a7e297524f185c65 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 13 Jun 2024 06:31:44 +0200 Subject: [PATCH 34/46] Spellchecking Signed-off-by: DL6ER --- src/ntp/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index c6d53c25..1272d30c 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -365,7 +365,7 @@ static int getsock(const struct addrinfo *saddr) if(connect(s, saddr->ai_addr, saddr->ai_addrlen) != 0) { char errbuf[1024]; - strncpy(errbuf, "Canot connect to NTP server: ", sizeof(errbuf)); + strncpy(errbuf, "Cannot connect to NTP server: ", sizeof(errbuf)); strncat(errbuf, strerror(errno), sizeof(errbuf) - strlen(errbuf) - 1); errbuf[sizeof(errbuf) - 1] = '\0'; log_ntp_message(true, false, errbuf); From 09a1f6fe5e8b09cb5843116c9004e400179d6b3b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 13 Jun 2024 07:35:47 +0200 Subject: [PATCH 35/46] Adjust CI tests due to modified NTP error message text Signed-off-by: DL6ER --- test/test_suite.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_suite.bats b/test/test_suite.bats index 5969ba79..569bf002 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -1171,7 +1171,7 @@ @test "No ERROR messages in FTL.log (besides known/intended error)" { run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" - run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log | grep -c -v -E "(index\.html)|(Failed to create shared memory object)|(FTLCONF_debug_api is invalid)|(Failed to adjust time: Insufficient permissions)"' + run bash -c 'grep "ERROR: " /var/log/pihole/FTL.log | grep -c -v -E "(index\.html)|(Failed to create shared memory object)|(FTLCONF_debug_api is invalid)|(Failed to set|adjust time during NTP sync: Insufficient permissions)"' printf "count: %s\n" "${lines[@]}" [[ ${lines[0]} == "0" ]] } From b45695c3bded7a036a3fcf1571d519c26ced1b63 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 25 Jun 2024 10:01:53 +0200 Subject: [PATCH 36/46] Restart FTL if system time has been updated by more than one hour using the internal NTP synchronization method. This ensures FTL can import the real most recent 24 hours data of history after a restart on a system lacking a real hardware clock Signed-off-by: DL6ER --- src/ntp/client.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index 1272d30c..c312c8ac 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -484,8 +484,8 @@ bool ntp_client(const char *server, const bool settime, const bool print) theta_stdev = sqrt(theta_stdev / valid); delta_stdev = sqrt(delta_stdev / valid); - log_info("Average time offset: (%e +/- %e s)", theta_avg, theta_stdev); - log_info("Average round-trip delay: (%e +/- %e s)", delta_avg, delta_stdev); + log_debug(DEBUG_NTP, "Average time offset: (%e +/- %e) s", theta_avg, theta_stdev); + log_debug(DEBUG_NTP, "Average round-trip delay: (%e +/- %e) s", delta_avg, delta_stdev); // Reject synchronization if the standard deviation of the time offset // or round-trip delay is larger than 1 second @@ -529,8 +529,8 @@ bool ntp_client(const char *server, const bool settime, const bool print) theta_trim /= trim; delta_trim /= trim; - log_info("Trimmed mean time offset: %e s (excluded %u outliers)", theta_trim, count - trim); - log_info("Trimmed mean round-trip delay: %e s (excluded %u outliers)", delta_trim, count - trim); + log_info("Time offset: %e ms (excluded %u outliers)", 1e3*theta_trim, count - trim); + log_info("Round-trip delay: %e ms (excluded %u outliers)", 1e3*delta_trim, count - trim); // Set time if requested if(settime) @@ -585,9 +585,31 @@ static void *ntp_client_thread(void *arg) // Run NTP client while(!killed) { + + // Get time before NTP sync + const time_t before = time(NULL); + // Run NTP client ntp_client(config.ntp.sync.server.v.s, true, false); + // Get time after NTP sync + const time_t after = time(NULL); + + // If the time was updated by more than one hour, restart FTL to + // import recent data. This is relevant when the system time was + // set to an incorrect value (e.g., due to a dead CMOS battery + // or overall missing RTC) and the time was off. + if(after - before > 3600) + { + log_info("System time was updated by more than one hour, restarting FTL to import recent data"); + // Set the restart flag to true + exit_code = RESTART_FTL_CODE; + // Send SIGTERM to FTL + kill(main_pid(), SIGTERM); + // Kill the NTP thread + killed = true; + } + // Intermediate cancellation-point BREAK_IF_KILLED(); From 23ddd85837f720887a791061204f0631271adb03 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 26 Jun 2024 08:49:31 +0200 Subject: [PATCH 37/46] Check if database is actually writable when we request this Signed-off-by: DL6ER --- src/database/common.c | 9 ++++++ src/database/message-table.c | 59 +++++++++++------------------------- 2 files changed, 26 insertions(+), 42 deletions(-) diff --git a/src/database/common.c b/src/database/common.c index f209a50f..bcf7d61d 100644 --- a/src/database/common.c +++ b/src/database/common.c @@ -103,6 +103,15 @@ sqlite3* _dbopen(const bool readonly, const bool create, const char *func, const return NULL; } + // If the database is opened in read-write mode, actually check if it is + // writable. If it is not, close the database and return an error + if(!readonly && sqlite3_db_readonly(db, NULL)) + { + log_err("Cannot open database in read-write mode"); + dbclose(&db); + return NULL; + } + // Explicitly set busy handler to value defined in FTL.h rc = sqlite3_busy_timeout(db, DATABASE_BUSY_TIMEOUT); if( rc != SQLITE_OK ) diff --git a/src/database/message-table.c b/src/database/message-table.c index 81c4e3fa..031d7fb8 100644 --- a/src/database/message-table.c +++ b/src/database/message-table.c @@ -332,10 +332,8 @@ static int _add_message(const enum message_type type, sqlite3 *db; // Open database connection if((db = dbopen(false, false)) == NULL) - { - log_err("add_message() - Failed to open DB"); + // Reason for failure is logged in dbopen() return -1; - } // Ensure there are no duplicates when adding messages sqlite3_stmt* stmt = NULL; @@ -1244,9 +1242,7 @@ void logg_regex_warning(const char *type, const char *warning, const int dbindex return; // Add to database - const int rowid = add_message(REGEX_MESSAGE, regex, type, warning, dbindex); - if(rowid == -1) - log_err("logg_regex_warning(): Failed to add message to database"); + add_message(REGEX_MESSAGE, regex, type, warning, dbindex); } void logg_subnet_warning(const char *ip, const int matching_count, const char *matching_ids, @@ -1264,10 +1260,8 @@ void logg_subnet_warning(const char *ip, const int matching_count, const char *m log_warn("%s", buf); // Log to database - const int rowid = add_message(SUBNET_MESSAGE, ip, matching_count, names, matching_ids, chosen_match_text, chosen_match_id); + add_message(SUBNET_MESSAGE, ip, matching_count, names, matching_ids, chosen_match_text, chosen_match_id); - if(rowid == -1) - log_err("logg_subnet_warning(): Failed to add message to database"); free(names); } @@ -1286,10 +1280,8 @@ void logg_hostname_warning(const char *ip, const char *name, const unsigned int log_warn("%s", buf); // Log to database - const int rowid = add_message(HOSTNAME_MESSAGE, ip, name, (const int)pos); + add_message(HOSTNAME_MESSAGE, ip, name, (const int)pos); - if(rowid == -1) - log_err("logg_hostname_warning(): Failed to add message to database"); } void logg_fatal_dnsmasq_message(const char *message) @@ -1302,10 +1294,8 @@ void logg_fatal_dnsmasq_message(const char *message) log_crit("%s", buf); // Log to database - const int rowid = add_message_no_args(DNSMASQ_CONFIG_MESSAGE, message); + add_message_no_args(DNSMASQ_CONFIG_MESSAGE, message); - if(rowid == -1) - log_err("logg_fatal_dnsmasq_message(): Failed to add message to database"); } void logg_rate_limit_message(const char *clientIP, const unsigned int rate_limit_count) @@ -1320,10 +1310,8 @@ void logg_rate_limit_message(const char *clientIP, const unsigned int rate_limit log_info("%s", buf); // Log to database - const int rowid = add_message(RATE_LIMIT_MESSAGE, clientIP, config.dns.rateLimit.count.v.ui, config.dns.rateLimit.interval.v.ui, turnaround); + add_message(RATE_LIMIT_MESSAGE, clientIP, config.dns.rateLimit.count.v.ui, config.dns.rateLimit.interval.v.ui, turnaround); - if(rowid == -1) - log_err("logg_rate_limit_message(): Failed to add message to database"); } void logg_warn_dnsmasq_message(char *message) @@ -1336,10 +1324,8 @@ void logg_warn_dnsmasq_message(char *message) log_warn("%s", buf); // Log to database - const int rowid = add_message_no_args(DNSMASQ_WARN_MESSAGE, message); + add_message_no_args(DNSMASQ_WARN_MESSAGE, message); - if(rowid == -1) - log_err("logg_warn_dnsmasq_message(): Failed to add message to database"); } void log_resource_shortage(const double load, const int nprocs, const int shmem, const int disk, const char *path, const char *msg) @@ -1355,10 +1341,9 @@ void log_resource_shortage(const double load, const int nprocs, const int shmem, log_warn("%s", buf); // Log to database - const int rowid = add_message(LOAD_MESSAGE, "excessive load", load, nprocs); + add_message(LOAD_MESSAGE, "excessive load", load, nprocs); + - if(rowid == -1) - log_err("log_resource_shortage(): Failed to add message to database"); } else if(shmem > -1) { @@ -1368,10 +1353,9 @@ void log_resource_shortage(const double load, const int nprocs, const int shmem, log_warn("%s", buf); // Log to database - const int rowid = add_message(SHMEM_MESSAGE, path, shmem, msg); + add_message(SHMEM_MESSAGE, path, shmem, msg); + - if(rowid == -1) - log_err("log_resource_shortage(): Failed to add message to database"); } else if(disk > -1) { @@ -1405,12 +1389,11 @@ void log_resource_shortage(const double load, const int nprocs, const int shmem, log_warn("%s", buf); // Log to database - const int rowid = fsdetails != NULL ? + fsdetails != NULL ? add_message(DISK_MESSAGE_EXTENDED, path, disk, msg, fsdetails->mnt_type, fsdetails->mnt_dir) : add_message(DISK_MESSAGE, path, disk, msg); - if(rowid == -1) - log_err("log_resource_shortage(): Failed to add message to database"); + } } @@ -1424,10 +1407,8 @@ void logg_inaccessible_adlist(const int dbindex, const char *address) log_warn("%s", buf); // Log to database - const int rowid = add_message(INACCESSIBLE_ADLIST_MESSAGE, address, dbindex); + add_message(INACCESSIBLE_ADLIST_MESSAGE, address, dbindex); - if(rowid == -1) - log_err("logg_inaccessible_adlist(): Failed to add message to database"); } void log_certificate_domain_mismatch(const char *certfile, const char *domain) @@ -1440,10 +1421,8 @@ void log_certificate_domain_mismatch(const char *certfile, const char *domain) log_warn("%s", buf); // Log to database - const int rowid = add_message(CERTIFICATE_DOMAIN_MISMATCH_MESSAGE, certfile, domain); + add_message(CERTIFICATE_DOMAIN_MISMATCH_MESSAGE, certfile, domain); - if(rowid == -1) - log_err("log_certificate_domain_mismatch(): Failed to add message to database"); } void log_connection_error(const char *server, const char *reason, const char *error) @@ -1456,10 +1435,8 @@ void log_connection_error(const char *server, const char *reason, const char *er log_warn("%s", buf); // Log to database - const int rowid = add_message(CONNECTION_ERROR_MESSAGE, server, reason, error); + add_message(CONNECTION_ERROR_MESSAGE, server, reason, error); - if(rowid == -1) - log_err("logg_connection_error(): Failed to add message to database"); } void log_ntp_message(const bool error, const bool server, const char *message) @@ -1478,8 +1455,6 @@ void log_ntp_message(const bool error, const bool server, const char *message) log_warn("%s", buf); // Log to database - const int rowid = add_message(NTP_MESSAGE, message, level, who); + add_message(NTP_MESSAGE, message, level, who); - if(rowid == -1) - log_err("log_ntp_message(): Failed to add message to database"); } From 5c2da0d074c684c128f5e297e8240524e904e6a2 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 26 Jun 2024 09:04:18 +0200 Subject: [PATCH 38/46] Only use valid replies. Before, invalid replies would have contributed to the mean with a value of 0.0ms deviation, i.e., reducing any existing real time difference Signed-off-by: DL6ER --- src/ntp/client.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index c312c8ac..365e76d0 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -41,6 +41,7 @@ #include "database/message-table.h" struct ntp_sync { + bool valid; uint64_t org; uint64_t xmt; double theta; @@ -50,7 +51,7 @@ struct ntp_sync // Create minimal NTP request, see server implementation for details about the // packet structure -static bool request(int fd, struct ntp_sync *ntp) +static bool request(int fd, const char *server, struct ntp_sync *ntp) { // NTP Packet buffer unsigned char buf[48] = {0}; @@ -73,8 +74,8 @@ static bool request(int fd, struct ntp_sync *ntp) // Send request if(send(fd, buf, 48, 0) != 48) { - log_err("Failed to send data to NTP server: %s", - errno == EAGAIN ? "Timeout" : strerror(errno)); + log_err("Failed to send data to NTP server %s: %s", + server, errno == EAGAIN ? "Timeout" : strerror(errno)); return false; } @@ -209,7 +210,7 @@ static bool settime_skew(const double offset) return true; } -static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) +static bool reply(int fd, const char *server, struct ntp_sync *ntp, const bool verbose) { // NTP Packet buffer unsigned char buf[48]; @@ -217,8 +218,8 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // Receive reply if(recv(fd, buf, 48, 0) < 48) { - log_err("Failed to receive data from NTP server: %s", - errno == EAGAIN ? "Timeout" : strerror(errno)); + log_err("Failed to receive data from NTP server %s: %s", + server, errno == EAGAIN ? "Timeout" : strerror(errno)); return false; } @@ -296,6 +297,9 @@ static bool reply(int fd, struct ntp_sync *ntp, const bool verbose) // technologies are highly variable ntp->delta = ( T4 - T1 ) - ( T3 - T2 ); + // This reply is valid + ntp->valid = true; + // In some scenarios where the initial frequency offset of the client is // relatively large and the actual propagation time small, it is // possible for the delay computation to become negative. For instance, @@ -416,7 +420,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) continue; // Send request - if(!request(s, &ntp[i])) + if(!request(s, server, &ntp[i])) { close(s); free(ntp); @@ -424,7 +428,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) return false; } // Get reply - if(!reply(s, &ntp[i], false)) + if(!reply(s, server, &ntp[i], false)) { close(s); continue; @@ -453,7 +457,8 @@ bool ntp_client(const char *server, const bool settime, const bool print) { // Skip invalid values if(fabs(ntp[i].theta) < ntp[i].precision || - fabs(ntp[i].delta) < ntp[i].precision) + fabs(ntp[i].delta) < ntp[i].precision || + !ntp[i].valid) continue; theta_avg += ntp[i].theta; @@ -475,7 +480,8 @@ bool ntp_client(const char *server, const bool settime, const bool print) { // Skip invalid values if(fabs(ntp[i].theta) < ntp[i].precision || - fabs(ntp[i].delta) < ntp[i].precision) + fabs(ntp[i].delta) < ntp[i].precision || + !ntp[i].valid) continue; theta_stdev += pow(ntp[i].theta - theta_avg, 2); @@ -503,7 +509,8 @@ bool ntp_client(const char *server, const bool settime, const bool print) { // Skip invalid values if(fabs(ntp[i].theta) < ntp[i].precision || - fabs(ntp[i].delta) < ntp[i].precision) + fabs(ntp[i].delta) < ntp[i].precision || + !ntp[i].valid) continue; // Skip outliers From b3d72cb7b60cdc26c06873dc02066035f6913663 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 26 Jun 2024 09:35:13 +0200 Subject: [PATCH 39/46] Load queries only after first NTP synchronization (if enabled) Signed-off-by: DL6ER --- src/database/query-table.c | 24 ++++++++++++++++++++++++ src/database/query-table.h | 1 + src/dnsmasq_interface.c | 7 ------- src/ntp/client.c | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/database/query-table.c b/src/database/query-table.c index 342730a4..d5e1ccfc 100644 --- a/src/database/query-table.c +++ b/src/database/query-table.c @@ -21,8 +21,11 @@ #include "overTime.h" #include "database/common.h" #include "timers.h" +// runGC() +#include "gc.h" static sqlite3 *_memdb = NULL; +static bool store_in_database = false; static double new_last_timestamp = 0; static unsigned int new_total = 0, new_blocked = 0; static unsigned long last_mem_db_idx = 0, last_disk_db_idx = 0; @@ -1367,6 +1370,11 @@ bool queries_to_database(void) log_debug(DEBUG_DATABASE, "Not storing query in database as there are none"); return true; } + if(!store_in_database) + { + log_debug(DEBUG_DATABASE, "Not storing query in database as this is disabled"); + return true; + } // Loop over recent queries and store new or changed ones in the // in-memory database @@ -1626,3 +1634,19 @@ bool queries_to_database(void) return true; } + +void load_queries_from_disk(void) +{ + // Compensate for possible jumps in time + runGC(time(NULL), NULL, false); + + // Skip if we are not supposed to load queries from disk + if(!config.database.DBimport.v.b) + return; + + // Try to import queries from long-term database if available + import_queries_from_disk(); + DB_read_queries(); + + store_in_database = true; +} \ No newline at end of file diff --git a/src/database/query-table.h b/src/database/query-table.h index 9fecb9ea..71458e28 100644 --- a/src/database/query-table.h +++ b/src/database/query-table.h @@ -119,6 +119,7 @@ bool add_additional_info_column(sqlite3 *db); void DB_read_queries(void); void update_disk_db_idx(void); bool queries_to_database(void); +void load_queries_from_disk(void); bool optimize_queries_table(sqlite3 *db); bool create_addinfo_table(sqlite3 *db); diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index ad21e847..0c8132db 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -2904,13 +2904,6 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw, bool dnsmasq_start) // Flush messages stored in the long-term database flush_message_table(); - // Try to import queries from long-term database if available - if(config.database.DBimport.v.b) - { - import_queries_from_disk(); - DB_read_queries(); - } - // Initialize in-memory database starting index update_disk_db_idx(); diff --git a/src/ntp/client.c b/src/ntp/client.c index 365e76d0..f1fb042f 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -39,6 +39,8 @@ #include // log_ntp_message() #include "database/message-table.h" +// load_queries_from_disk() +#include "database/query-table.h" struct ntp_sync { bool valid; @@ -590,6 +592,7 @@ static void *ntp_client_thread(void *arg) prctl(PR_SET_NAME, thread_names[NTP], 0, 0, 0); // Run NTP client + bool first_run = true; while(!killed) { @@ -599,6 +602,13 @@ static void *ntp_client_thread(void *arg) // Run NTP client ntp_client(config.ntp.sync.server.v.s, true, false); + // Load queries from database after first NTP synchronization + if(first_run) + { + load_queries_from_disk(); + first_run = false; + } + // Get time after NTP sync const time_t after = time(NULL); @@ -636,12 +646,16 @@ bool ntp_start_sync_thread(pthread_attr_t *attr) if(config.ntp.sync.server.v.s == NULL || strlen(config.ntp.sync.server.v.s) == 0 || config.ntp.sync.interval.v.ui == 0) + { + load_queries_from_disk(); return false; + } // Create thread if(pthread_create(&threads[NTP], attr, ntp_client_thread, NULL) != 0) { log_err("Cannot create NTP client thread"); + load_queries_from_disk(); return false; } From 6d164a352937448b95756a65c75a30545048b089 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 26 Jun 2024 09:37:01 +0200 Subject: [PATCH 40/46] Remove restarting step as queries are now loaded *after* NTP time synchronization Signed-off-by: DL6ER --- src/ntp/client.c | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/src/ntp/client.c b/src/ntp/client.c index f1fb042f..532689a2 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -8,7 +8,7 @@ * This file is copyright under the latest version of the EUPL. * Please see LICENSE file for your rights under this license. */ -#include "ntp/ntp.h" +#include "ntp.h" // close() #include // clock_gettime() @@ -595,10 +595,6 @@ static void *ntp_client_thread(void *arg) bool first_run = true; while(!killed) { - - // Get time before NTP sync - const time_t before = time(NULL); - // Run NTP client ntp_client(config.ntp.sync.server.v.s, true, false); @@ -609,24 +605,6 @@ static void *ntp_client_thread(void *arg) first_run = false; } - // Get time after NTP sync - const time_t after = time(NULL); - - // If the time was updated by more than one hour, restart FTL to - // import recent data. This is relevant when the system time was - // set to an incorrect value (e.g., due to a dead CMOS battery - // or overall missing RTC) and the time was off. - if(after - before > 3600) - { - log_info("System time was updated by more than one hour, restarting FTL to import recent data"); - // Set the restart flag to true - exit_code = RESTART_FTL_CODE; - // Send SIGTERM to FTL - kill(main_pid(), SIGTERM); - // Kill the NTP thread - killed = true; - } - // Intermediate cancellation-point BREAK_IF_KILLED(); From 14e716729c0501b6c69a05a87dbaa03ee9579b2c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Wed, 26 Jun 2024 10:21:12 +0200 Subject: [PATCH 41/46] Styling Signed-off-by: DL6ER --- src/database/query-table.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/database/query-table.c b/src/database/query-table.c index d5e1ccfc..5c707dc2 100644 --- a/src/database/query-table.c +++ b/src/database/query-table.c @@ -1649,4 +1649,4 @@ void load_queries_from_disk(void) DB_read_queries(); store_in_database = true; -} \ No newline at end of file +} From 6eff0296b3e74ce37f1258550bdf3ef22711169c Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 27 Jun 2024 16:33:16 +0200 Subject: [PATCH 42/46] Check availablity of CAP_SYS_TIME when NTP client is invoked from CLI Signed-off-by: DL6ER --- src/args.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/args.c b/src/args.c index f6e252a1..dcd625b4 100644 --- a/src/args.c +++ b/src/args.c @@ -68,6 +68,8 @@ #include "resolve.h" // ntp_client() #include "ntp/ntp.h" +// check_capability() +#include "capabilities.h" // defined in dnsmasq.c extern void print_dnsmasq_version(const char *yellow, const char *green, const char *bold, const char *normal); @@ -310,6 +312,18 @@ void parse_args(int argc, char* argv[]) // Create test NTP client if((argc > 1 && argc < 5) && strcmp(argv[1], "ntp") == 0) { + // Ensure we have the necessary capabilities + if(!check_capability(CAP_SYS_TIME)) + { + puts("Insufficient capabilities to run NTP client"); + const char *bold = cli_bold(); + const char *normal = cli_normal(); + printf("Try: %ssudo%s ", bold, normal); + for(int i = 0; i < argc; i++) + printf("%s ", argv[i]); + puts(""); + exit(EXIT_FAILURE); + } // Enable stdout printing cli_mode = true; log_ctrl(false, true); From 4e5526854422f2d929ed15d880dd0ba52c04335a Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 27 Jun 2024 16:34:54 +0200 Subject: [PATCH 43/46] Clarify which server is used when invoked via CLI Signed-off-by: DL6ER --- src/args.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/args.c b/src/args.c index dcd625b4..dbb74476 100644 --- a/src/args.c +++ b/src/args.c @@ -333,6 +333,7 @@ void parse_args(int argc, char* argv[]) const char *server = "127.0.0.1"; if(argc > 2 && strcmp(argv[2], "--update") != 0) server = argv[2]; + printf("Using NTP server: %s\n", server); exit(ntp_client(server, update, true) ? EXIT_SUCCESS : EXIT_FAILURE); } From ec0e0c98bf72ff38315847abdd5ab66ce398ca8d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 27 Jun 2024 16:36:02 +0200 Subject: [PATCH 44/46] Print database statistics after historic queries have been loaded from disk Signed-off-by: DL6ER --- src/database/query-table.c | 3 +++ src/dnsmasq_interface.c | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/database/query-table.c b/src/database/query-table.c index 5c707dc2..435f560a 100644 --- a/src/database/query-table.c +++ b/src/database/query-table.c @@ -1648,5 +1648,8 @@ void load_queries_from_disk(void) import_queries_from_disk(); DB_read_queries(); + // Log some information about the imported queries (if any) + log_counter_info(); + store_in_database = true; } diff --git a/src/dnsmasq_interface.c b/src/dnsmasq_interface.c index 0c8132db..72bb41ce 100644 --- a/src/dnsmasq_interface.c +++ b/src/dnsmasq_interface.c @@ -2907,9 +2907,6 @@ void FTL_fork_and_bind_sockets(struct passwd *ent_pw, bool dnsmasq_start) // Initialize in-memory database starting index update_disk_db_idx(); - // Log some information about the imported queries (if any) - log_counter_info(); - // Handle real-time signals in this process (and its children) // Helper processes are already split from the main instance // so they will not listen to real-time signals From 8e63dd99f9360bf736008c0bedaab58dd92b277d Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 27 Jun 2024 17:35:56 +0200 Subject: [PATCH 45/46] Adjust tests Signed-off-by: DL6ER --- test/test_suite.bats | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/test_suite.bats b/test/test_suite.bats index 569bf002..a740a024 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -22,12 +22,6 @@ [[ ${lines[1]} == "" ]] } -@test "Starting tests without prior history" { - run bash -c 'grep -c "Total DNS queries: 0" /var/log/pihole/FTL.log' - printf "%s\n" "${lines[@]}" - [[ ${lines[0]} == "1" ]] -} - @test "Initial blocking status is enabled" { run bash -c 'grep -c "Blocking status is enabled" /var/log/pihole/FTL.log' printf "%s\n" "${lines[@]}" @@ -40,7 +34,7 @@ [[ ${lines[0]} == *"Compiled 2 allow and 11 deny regex"* ]] } -@test "denied domain is blocked" { +@test "Denied domain is blocked" { run bash -c "dig denied.ftl @127.0.0.1 +short" printf "%s\n" "${lines[@]}" [[ ${lines[0]} == "0.0.0.0" ]] From 3f7d317b8ad11cc9daf8ef0e9166d030eb5853a2 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 27 Jun 2024 18:42:56 +0200 Subject: [PATCH 46/46] Check capabilities only when user requested updating time time Signed-off-by: DL6ER --- src/args.c | 18 +++++++++++------- test/test_suite.bats | 2 +- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/args.c b/src/args.c index dbb74476..a261d3be 100644 --- a/src/args.c +++ b/src/args.c @@ -312,8 +312,15 @@ void parse_args(int argc, char* argv[]) // Create test NTP client if((argc > 1 && argc < 5) && strcmp(argv[1], "ntp") == 0) { + // Parse arguments + const bool update = (argc > 2 && strcmp(argv[2], "--update") == 0) || + (argc > 3 && strcmp(argv[3], "--update") == 0); + const char *server = "127.0.0.1"; + if(argc > 2 && strcmp(argv[2], "--update") != 0) + server = argv[2]; + // Ensure we have the necessary capabilities - if(!check_capability(CAP_SYS_TIME)) + if(update && !check_capability(CAP_SYS_TIME)) { puts("Insufficient capabilities to run NTP client"); const char *bold = cli_bold(); @@ -324,16 +331,13 @@ void parse_args(int argc, char* argv[]) puts(""); exit(EXIT_FAILURE); } + + printf("Using NTP server: %s\n", server); + // Enable stdout printing cli_mode = true; log_ctrl(false, true); readFTLconf(&config, false); - const bool update = (argc > 2 && strcmp(argv[2], "--update") == 0) || - (argc > 3 && strcmp(argv[3], "--update") == 0); - const char *server = "127.0.0.1"; - if(argc > 2 && strcmp(argv[2], "--update") != 0) - server = argv[2]; - printf("Using NTP server: %s\n", server); exit(ntp_client(server, update, true) ? EXIT_SUCCESS : EXIT_FAILURE); } diff --git a/test/test_suite.bats b/test/test_suite.bats index a740a024..c850a709 100644 --- a/test/test_suite.bats +++ b/test/test_suite.bats @@ -1354,7 +1354,7 @@ } @test "Check NTP server is broadcasting correct time" { - run bash -c './pihole-FTL ntp 127.0.0.1' + run bash -c './pihole-FTL ntp 127.0.0.1 --dry-run' printf "%s\n" "${lines[@]}" [[ $status == 0 ]] }