From d1b5ed3fafb1d14898003030380cb65dd400f95d Mon Sep 17 00:00:00 2001 From: agessaman Date: Wed, 9 Sep 2026 14:45:22 -0700 Subject: [PATCH] fix(mqtt): validate NTP replies before trusting them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NTPClient::forceUpdate() treated any non-empty datagram arriving on its fixed local port 1337 as a time response: it ignored the read length, the version, the mode, the stratum, the leap indicator and any request/response correlation, then handed the bytes at offset 40 to the bridge. The bridge only checked that the derived epoch was at least 2026-01-01 before calling settimeofday() and writing the RTC. A host probe against the installed library source accepted a one-byte non-NTP datagram and produced epoch 2085978496. Anyone able to land a UDP datagram during a query window could set a bogus clock, which then feeds JWT issuance, certificate validity and packet timestamps. Replace that path with probeNtpServer(): one exchange per server on a fresh ephemeral socket, closed again on every exit. A reply is accepted only if it is a full 48 bytes, from the address and port queried, NTPv3/v4 mode 4, from a synchronised server (no leap alarm, stratum 1-15), echoing the random transmit timestamp of the request, with an epoch inside a plausible range. Rejected datagrams leave the clock alone and do not end the wait, so an early bogus packet cannot pre-empt the real answer. Era-1 (post-2036) timestamps convert forward instead of wrapping into 1900. The diagnostic now runs the same validated probe, so `get mqtt.ntp.diag` answers the question it is asked — would this server be trusted? — instead of reporting a datagram nothing checked. It also no longer leaves its UDP socket open after the probe, and each server's per-probe result carries the reason it failed ("DNS failed", "unsolicited reply", "server unsynced", ...). The diagnostic still blocks its caller; making it asynchronous is a separate change. The acceptance rules live in NtpValidation.h (pure, host-tested), including the review's one-byte-datagram reproduction. --- MQTT_IMPLEMENTATION.md | 3 +- src/helpers/NtpValidation.h | 141 ++++++++++++++ src/helpers/bridges/MQTTBridge.cpp | 168 ++++++++++++---- src/helpers/bridges/MQTTBridge.h | 20 +- .../test_ntp_validation.cpp | 184 ++++++++++++++++++ 5 files changed, 474 insertions(+), 42 deletions(-) create mode 100644 src/helpers/NtpValidation.h create mode 100644 test/test_ntp_validation/test_ntp_validation.cpp diff --git a/MQTT_IMPLEMENTATION.md b/MQTT_IMPLEMENTATION.md index 9d4d16bc8..56f43569a 100644 --- a/MQTT_IMPLEMENTATION.md +++ b/MQTT_IMPLEMENTATION.md @@ -524,7 +524,7 @@ These settings apply across all MQTT slots: - `get mqtt.neighbors` - Get periodic neighbors publishing setting (on/off; neighbors-enabled builds) - `get mqtt.neighbors.interval` - Get neighbors publish interval in hours (neighbors-enabled builds) - `get mqtt.ntp` - Get effective NTP server hostname -- `get mqtt.ntp.diag` - Probe every configured NTP server for connectivity (does not change the clock; serial console shows each server's reported time, LoRa shows a compact ` ok|fail` list) +- `get mqtt.ntp.diag` - Probe every configured NTP server for connectivity (does not change the clock; serial console shows each server's reported time, or why a probe was rejected — `DNS failed`, `unsolicited reply`, `server unsynced`, ... — and LoRa shows a compact ` ok|fail` list) - `get mqtt.owner` - Get owner public key (serial console only) - `get mqtt.email` - Get owner email address (serial console only) @@ -884,6 +884,7 @@ the radio actually performs in that case. - Automatic time synchronization with NTP servers (required for JWT authentication) - Default primary: `pool.ntp.org`; built-in fallbacks (tried sequentially on failure): `time.google.com`, `time.cloudflare.com`, `time.aws.com`, `time.nist.gov` - Periodic time updates (every hour) on the effective primary only; system time is kept in UTC +- Replies are validated before they are trusted: the datagram must be a full-length NTPv3/v4 server reply from the queried address and port, from a synchronised server (stratum 1-15, no leap alarm), echoing the random transmit timestamp of the request, with a plausible epoch. Anything else is discarded and the clock, the RTC and JWT issuance are left alone - Configure and diagnose with `set mqtt.ntp` / `get mqtt.ntp` / `get mqtt.ntp.diag` — see [MQTT Shared Commands](#mqtt-shared-commands) ### Authentication diff --git a/src/helpers/NtpValidation.h b/src/helpers/NtpValidation.h new file mode 100644 index 000000000..e81510b7f --- /dev/null +++ b/src/helpers/NtpValidation.h @@ -0,0 +1,141 @@ +#pragma once + +#include +#include + +// Client-side NTPv4 request building and response validation. +// +// Pure logic: no Arduino, WiFi or UDP headers, so every rejection case is +// host-testable. The caller owns the socket and supplies the datagram plus +// whether it arrived from the server it asked (see MQTTBridge::probeNtpServer). +// +// This exists because the NTPClient library accepted *any* non-empty datagram +// on its fixed local port as time: it ignored the read length, the mode, the +// stratum, the leap indicator and the request/response correlation, so a +// one-byte packet from anywhere set the clock — which then feeds JWT issuance, +// certificate validity and the RTC. Everything below is the validation that +// path never had. +namespace NtpValidation { + +static const size_t kPacketSize = 48; + +// Seconds between the NTP epoch (1900-01-01) and the Unix epoch (1970-01-01). +static const uint32_t kUnixEpochOffset = 2208988800UL; +// Wrap distance for NTP era 1, which starts 2036-02-07: era-1 seconds count +// from there, so a small NTP seconds field is a *future* time, not a past one. +static const uint32_t kEra1UnixOffset = 2085978496UL; + +enum Reject : uint8_t { + kAccepted = 0, + kShortPacket, // fewer than 48 bytes: not an NTP response at all + kWrongSource, // not from the address/port we queried + kBadVersion, // NTP version outside 3..4 + kBadMode, // not mode 4 (server) + kLeapAlarm, // LI=3: the server itself is unsynchronised + kBadStratum, // 0 (kiss-of-death) or >= 16 (unsynchronised) + kOriginMismatch, // did not echo the transmit timestamp we sent + kZeroTransmit, // no transmit timestamp + kEpochTooEarly, // before the caller's floor + kEpochTooLate, // implausibly far in the future +}; + +// The transmit timestamp we send and the server must echo back in its originate +// field. Random, so an off-path sender cannot answer a query it never saw. +struct Nonce { + uint32_t seconds; + uint32_t fraction; +}; + +struct Result { + Reject reject; + uint32_t epoch; // Unix seconds; only meaningful when reject == kAccepted +}; + +static inline uint32_t readU32(const uint8_t* p) { + return ((uint32_t)p[0] << 24) | ((uint32_t)p[1] << 16) | + ((uint32_t)p[2] << 8) | (uint32_t)p[3]; +} + +static inline void writeU32(uint8_t* p, uint32_t v) { + p[0] = (uint8_t)(v >> 24); + p[1] = (uint8_t)(v >> 16); + p[2] = (uint8_t)(v >> 8); + p[3] = (uint8_t)v; +} + +// LI=0 (no warning), VN=4, Mode=3 (client). The NTPClient request claimed LI=3, +// advertising an alarm condition it had no business asserting. +static inline void buildRequest(uint8_t out[kPacketSize], Nonce nonce) { + memset(out, 0, kPacketSize); + out[0] = 0x23; + out[1] = 0; // stratum: unspecified + out[2] = 6; // poll interval + out[3] = 0xEC; // precision + writeU32(out + 40, nonce.seconds); // transmit timestamp = our nonce + writeU32(out + 44, nonce.fraction); +} + +// Unix time for an NTP seconds field, handling the 2036 era rollover instead of +// wrapping into 1900 the way an unchecked subtraction does. +static inline uint32_t unixFromNtpSeconds(uint32_t ntp_seconds) { + return ntp_seconds >= kUnixEpochOffset ? (ntp_seconds - kUnixEpochOffset) + : (ntp_seconds + kEra1UnixOffset); +} + +// `source_matches` is the caller's check that the datagram came from the +// address and port it queried; it is a parameter rather than a lookup so this +// stays free of socket types. +static inline Result validate(const uint8_t* data, size_t len, bool source_matches, + Nonce nonce, uint32_t min_epoch, uint32_t max_epoch) { + Result r; + r.epoch = 0; + + if (data == nullptr || len < kPacketSize) { r.reject = kShortPacket; return r; } + if (!source_matches) { r.reject = kWrongSource; return r; } + + const uint8_t li = (uint8_t)((data[0] >> 6) & 0x03); + const uint8_t version = (uint8_t)((data[0] >> 3) & 0x07); + const uint8_t mode = (uint8_t)(data[0] & 0x07); + const uint8_t stratum = data[1]; + + if (version < 3 || version > 4) { r.reject = kBadVersion; return r; } + if (mode != 4) { r.reject = kBadMode; return r; } + if (li == 3) { r.reject = kLeapAlarm; return r; } + // Stratum 0 carries a kiss-of-death code (RATE/DENY/RSTR) rather than time; + // 16 and above means the server has no time to give. + if (stratum == 0 || stratum >= 16) { r.reject = kBadStratum; return r; } + + if (readU32(data + 24) != nonce.seconds || + readU32(data + 28) != nonce.fraction) { r.reject = kOriginMismatch; return r; } + + const uint32_t transmit_seconds = readU32(data + 40); + if (transmit_seconds == 0) { r.reject = kZeroTransmit; return r; } + + const uint32_t epoch = unixFromNtpSeconds(transmit_seconds); + if (epoch < min_epoch) { r.reject = kEpochTooEarly; return r; } + if (epoch > max_epoch) { r.reject = kEpochTooLate; return r; } + + r.reject = kAccepted; + r.epoch = epoch; + return r; +} + +// Short enough for a LoRa-bounded diagnostic reply. +static inline const char* rejectReason(Reject reject) { + switch (reject) { + case kAccepted: return "ok"; + case kShortPacket: return "short packet"; + case kWrongSource: return "wrong source"; + case kBadVersion: return "bad version"; + case kBadMode: return "not a server reply"; + case kLeapAlarm: return "server unsynced"; + case kBadStratum: return "bad stratum"; + case kOriginMismatch: return "unsolicited reply"; + case kZeroTransmit: return "no timestamp"; + case kEpochTooEarly: return "time too old"; + case kEpochTooLate: return "time too far ahead"; + default: return "invalid"; + } +} + +} // namespace NtpValidation diff --git a/src/helpers/bridges/MQTTBridge.cpp b/src/helpers/bridges/MQTTBridge.cpp index 7fa5d69e0..c2deeaed9 100644 --- a/src/helpers/bridges/MQTTBridge.cpp +++ b/src/helpers/bridges/MQTTBridge.cpp @@ -2,13 +2,13 @@ #include "MQTTErrorLabels.h" #include "../WifiPowerSavePolicy.h" #include "../MQTTConnectionPolicy.h" +#include "../NtpValidation.h" #include "../MQTTMessageBuilder.h" #include "../MQTTPacketQueuePolicy.h" #include "../MQTTReplyFormat.h" #include "../MQTTRuntimeBufferLifecycle.h" #include "../MQTTTopicRouter.h" #include "../TxtDataHelpers.h" -#include #include #include #include @@ -24,6 +24,7 @@ #ifdef ESP_PLATFORM #include #include +#include #include #include #include @@ -57,6 +58,10 @@ static constexpr size_t kNtpBuiltinFallbackCount = static_assert(MQTTBridge::kMaxNtpServers >= 1 + (int)kNtpBuiltinFallbackCount, "kMaxNtpServers must hold the custom primary plus all built-in fallbacks"); +// Shared so the retry loop can recognise "this name did not resolve" by pointer +// and stop retrying: nothing was sent, so a second attempt changes nothing. +static const char* const kNtpDnsFailedReason = "DNS failed"; + static bool ntpHostnameEquals(const char* a, const char* b) { if (!a || !b) return false; return strcasecmp(a, b) == 0; @@ -674,7 +679,7 @@ MQTTBridge::MQTTBridge(NodePrefs *prefs, MQTTPrefs *obs, mesh::PacketManager *mg _obs(obs), _queue_count(0), _last_status_publish(0), _last_status_retry(0), _status_interval(300000), - _ntp_client(_ntp_udp, effectiveNtpPrimary(obs), 0, 60000), _last_ntp_sync(0), _ntp_synced(false), _ntp_sync_pending(false), _slots_setup_done(false), _max_active_slots(RUNTIME_MQTT_SLOTS), + _last_ntp_sync(0), _ntp_synced(false), _ntp_sync_pending(false), _slots_setup_done(false), _max_active_slots(RUNTIME_MQTT_SLOTS), _ntp_force_requested(false), _ntp_force_done(false), _ntp_force_result(false), _ntp_diag_requested(false), _ntp_diag_done(false), _ntp_diag_count(0), // Default to UTC; setRules() will be called from syncTimeWithNTP when a @@ -4026,6 +4031,106 @@ void MQTTBridge::refreshNTP() { MQTT_DEBUG_PRINTLN("NTP refresh triggered (async SNTP)"); } +// One validated NTP exchange with one named server, on a fresh ephemeral socket. +// +// Replaces NTPClient, which accepted any datagram that arrived on its fixed +// local port 1337 as time — no length, mode, stratum, leap or request/response +// check — and fed it straight into the system clock, the RTC and JWT issuance +// (F07). Everything that makes a reply trustworthy is checked here or in +// NtpValidation; a rejected reply leaves the clock alone and the loop keeps +// listening until the deadline, so an early bogus datagram cannot cancel the +// real answer. +// +// Returns false with *why set to a short reason for the diagnostic. The socket +// is opened and closed inside this call: no listener outlives the probe, and +// each server gets its own local port, so no reply can be credited to the wrong +// name (F08). +bool MQTTBridge::probeNtpServer(const char* server, uint32_t min_epoch, + uint32_t* epoch_out, const char** why) { + if (why) *why = "no reply"; + if (!server || server[0] == '\0') { if (why) *why = "no server"; return false; } + + bool have_expected_ip = false; + IPAddress expected_ip; + #ifdef ESP_PLATFORM + // Authoritative, not advisory. WiFiUDP leaves remote_ip/remote_port at the + // previous destination when a name fails to resolve, so sending anyway asks + // whichever server resolved last and credits its genuine reply to this name. + // Observed on d4: `set mqtt.ntp bogus.invalid` reported success with a correct + // epoch, answered by the pool address left over from boot. + if (!WiFi.hostByName(server, expected_ip)) { + if (why) *why = kNtpDnsFailedReason; + return false; + } + have_expected_ip = true; + #endif + + // Ephemeral local port: nothing to aim unsolicited traffic at between probes. + if (!_ntp_udp.begin(0)) { + if (why) *why = "no socket"; + return false; + } + + NtpValidation::Nonce nonce; + #ifdef ESP_PLATFORM + nonce.seconds = esp_random(); + nonce.fraction = esp_random(); + #else + nonce.seconds = (uint32_t)millis() * 2654435761UL; + nonce.fraction = (uint32_t)random(0, 0x7FFFFFFF); + #endif + + uint8_t packet[NtpValidation::kPacketSize]; + NtpValidation::buildRequest(packet, nonce); + + bool sent; + if (have_expected_ip) { + sent = _ntp_udp.beginPacket(expected_ip, kNtpPort) != 0; + } else { + sent = _ntp_udp.beginPacket(server, kNtpPort) != 0; + } + if (sent) { + _ntp_udp.write(packet, sizeof(packet)); + sent = _ntp_udp.endPacket() != 0; + } + if (!sent) { + _ntp_udp.stop(); + if (why) *why = "send failed"; + return false; + } + + bool accepted = false; + const unsigned long started = millis(); + while (millis() - started < kNtpProbeTimeoutMs) { + delay(10); + int len = _ntp_udp.parsePacket(); + if (len <= 0) continue; + + uint8_t reply[NtpValidation::kPacketSize]; + const int read_len = _ntp_udp.read(reply, sizeof(reply)); + // Discard any tail: WiFiUDP::parsePacket() refuses to read the next datagram + // while an unread one is still buffered, so an oversized reply would + // otherwise block the rest of this wait. + _ntp_udp.flush(); + const bool source_ok = (!have_expected_ip || _ntp_udp.remoteIP() == expected_ip) && + _ntp_udp.remotePort() == kNtpPort; + NtpValidation::Result r = NtpValidation::validate( + reply, read_len > 0 ? (size_t)read_len : 0, source_ok, nonce, + min_epoch, kNtpMaxValidEpoch); + if (r.reject == NtpValidation::kAccepted) { + if (epoch_out) *epoch_out = r.epoch; + if (why) *why = "ok"; + accepted = true; + break; + } + // Keep waiting: a rejected datagram must not consume this server's chance. + if (why) *why = NtpValidation::rejectReason(r.reject); + } + + _ntp_udp.stop(); + return accepted; +} + bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only) { if (!WiFi.isConnected()) { MQTT_DEBUG_PRINTLN("Cannot sync time - WiFi not connected"); @@ -4058,49 +4163,32 @@ bool MQTTBridge::syncTimeWithNTP(bool force, bool primary_only) { bool ntp_ok = false; unsigned long epochTime = 0; - const unsigned long kMinValidEpoch = 1767225600; // 2026-01-01 00:00:00 UTC + const uint32_t kMinValidEpoch = kNtpMinValidEpoch; const char* ntp_server_used = nullptr; - _ntp_client.begin(); const int kMaxNtpRetriesPerServer = 2; for (int s = 0; s < server_count && !ntp_ok; s++) { const char* server = servers[s]; - #ifdef ESP_PLATFORM - // Authoritative, not advisory. NTPClient::sendNTPPacket() ignores what - // beginPacket() returns, and WiFiUDP leaves remote_ip/remote_port at the previous - // destination when a name fails to resolve — so asking an unresolvable host sends - // the request to whichever server resolved last, and that server's genuine reply - // gets credited to this name. Observed on d4: `set mqtt.ntp bogus.invalid` reported - // success with a correct epoch, answered by the pool address left over from boot. - // Skipping is what keeps the credit honest; the name that answered is the name - // recorded. - IPAddress resolved_ip; - if (!WiFi.hostByName(server, resolved_ip)) { - MQTT_DEBUG_PRINTLN("NTP: %s does not resolve — skipping, not attempting a send", server); - continue; - } - #endif - - _ntp_client.setPoolServerName(server); - for (int attempt = 1; attempt <= kMaxNtpRetriesPerServer && !ntp_ok; attempt++) { if (attempt > 1) { MQTT_DEBUG_PRINTLN("NTP retry %d/%d on %s...", attempt, kMaxNtpRetriesPerServer, server); delay(1000); } - if (_ntp_client.forceUpdate()) { - epochTime = _ntp_client.getEpochTime(); - if (epochTime >= kMinValidEpoch) { - ntp_ok = true; - ntp_server_used = server; - } + uint32_t probed_epoch = 0; + const char* why = nullptr; + if (probeNtpServer(server, kMinValidEpoch, &probed_epoch, &why)) { + epochTime = probed_epoch; + ntp_ok = true; + ntp_server_used = server; + } else { + MQTT_DEBUG_PRINTLN("NTP: %s rejected (%s)", server, why ? why : "no reply"); + if (why == kNtpDnsFailedReason) break; // no send happened; try the next server } } } - _ntp_client.end(); - // Fallback: use ESP32 built-in SNTP (configTime) when NTPClient fails + // Fallback: use ESP32 built-in SNTP (configTime) when no server passed validation #ifdef ESP_PLATFORM if (!ntp_ok) { MQTT_DEBUG_PRINTLN("NTP client failed, trying SNTP fallback..."); @@ -4304,15 +4392,18 @@ void MQTTBridge::runNtpDiagProbe() { int count = 0; fillNtpServerList(_obs, servers, count); - _ntp_client.begin(); for (int i = 0; i < count; i++) { - _ntp_client.setPoolServerName(servers[i]); - bool ok = _ntp_client.forceUpdate(); + // Same validated probe the real sync uses, so the diagnostic answers the + // question an operator is actually asking: would this server be trusted? + uint32_t epoch = 0; + const char* why = nullptr; + bool ok = probeNtpServer(servers[i], kNtpMinValidEpoch, &epoch, &why); NtpDiagResult& r = _ntp_diag_results[i]; strncpy(r.server, servers[i], sizeof(r.server) - 1); r.server[sizeof(r.server) - 1] = '\0'; r.ok = ok; - r.epoch = ok ? (uint32_t)_ntp_client.getEpochTime() : 0; + r.epoch = ok ? epoch : 0; + r.why = why; // static literal from NtpValidation/probeNtpServer } _ntp_diag_count = count; } @@ -4346,12 +4437,13 @@ bool MQTTBridge::ntpDiag(char* reply, size_t reply_size, bool verbose) { const NtpDiagResult& r = _ntp_diag_results[i]; if (r.ok) { time_t t = (time_t)r.epoch; - struct tm* tmv = gmtime(&t); + struct tm tmv; + gmtime_r(&t, &tmv); // caller-owned storage: gmtime()'s buffer is shared Serial.printf(" %-20s OK %04d-%02d-%02d %02d:%02d:%02d UTC\r\n", - r.server, tmv->tm_year + 1900, tmv->tm_mon + 1, tmv->tm_mday, - tmv->tm_hour, tmv->tm_min, tmv->tm_sec); + r.server, tmv.tm_year + 1900, tmv.tm_mon + 1, tmv.tm_mday, + tmv.tm_hour, tmv.tm_min, tmv.tm_sec); } else { - Serial.printf(" %-20s FAIL\r\n", r.server); + Serial.printf(" %-20s FAIL %s\r\n", r.server, r.why ? r.why : "no reply"); } } snprintf(reply, reply_size, "> NTP diag: %d/%d OK (see console)", ok_count, _ntp_diag_count); diff --git a/src/helpers/bridges/MQTTBridge.h b/src/helpers/bridges/MQTTBridge.h index 9dd3f29c4..39d169e20 100644 --- a/src/helpers/bridges/MQTTBridge.h +++ b/src/helpers/bridges/MQTTBridge.h @@ -4,7 +4,6 @@ #include "helpers/bridges/BridgeBase.h" #include #include -#include #include #include #include "helpers/JWTHelper.h" @@ -92,6 +91,13 @@ public: private: static const size_t AUTH_TOKEN_SIZE = 768; + // NTP acceptance bounds. A reply outside them is rejected outright rather + // than allowed to set the clock, the RTC and every JWT minted afterwards. + static const uint16_t kNtpPort = 123; + static const uint32_t kNtpProbeTimeoutMs = 1000; + static const uint32_t kNtpMinValidEpoch = 1767225600UL; // 2026-01-01 UTC + static const uint32_t kNtpMaxValidEpoch = 4102444800UL; // 2100-01-01 UTC + // Connection slot - each slot holds one MQTT connection struct MQTTSlot { PsychicMqttClient* client; @@ -216,9 +222,9 @@ private: #endif int _queue_count; // Protected by queue operations or mutex - // NTP time sync + // NTP time sync. The socket is opened per probe and closed again (see + // probeNtpServer); nothing listens between syncs. WiFiUDP _ntp_udp; - NTPClient _ntp_client; unsigned long _last_ntp_sync; bool _ntp_synced; bool _ntp_sync_pending; // Flag to trigger NTP sync from loop() instead of event handler @@ -266,6 +272,10 @@ private: char server[64]; bool ok; uint32_t epoch; // server-reported UTC epoch when ok + // Why a probe failed, as a static literal ("DNS failed", "unsolicited + // reply", ...). Previously every failure looked alike, and a name that never + // resolved could be credited with another server's reply. + const char* why; }; NtpDiagResult _ntp_diag_results[kMaxNtpServers]; int _ntp_diag_count; @@ -509,6 +519,10 @@ private: bool isAnySlotConnected(); void refreshNTP(); // Lightweight periodic NTP refresh (non-blocking) void runNtpDiagProbe(); // Probe every server for connectivity; never sets the clock. Core 0 only. + // One validated NTP exchange with one server on a fresh ephemeral socket. + // Core 0 only; never touches the clock. *why receives a static reason literal. + bool probeNtpServer(const char* server, uint32_t min_epoch, + uint32_t* epoch_out, const char** why); // Populates dst_out/std_out with TimeChangeRules for the given IANA or // abbreviation string. Returns false if the string is not recognized // (callers should fall back to UTC). Zero-allocation. diff --git a/test/test_ntp_validation/test_ntp_validation.cpp b/test/test_ntp_validation/test_ntp_validation.cpp new file mode 100644 index 000000000..d2222dd83 --- /dev/null +++ b/test/test_ntp_validation/test_ntp_validation.cpp @@ -0,0 +1,184 @@ +#include "helpers/NtpValidation.h" + +#include + +#include + +using namespace NtpValidation; + +namespace { + +const uint32_t MIN_EPOCH = 1767225600UL; // 2026-01-01, the bridge's floor +const uint32_t MAX_EPOCH = 4102444800UL; // 2100-01-01 +const Nonce NONCE = { 0xA1B2C3D4UL, 0x0F1E2D3CUL }; + +// A well-formed reply to buildRequest(): stratum 2, mode 4, our nonce echoed. +void makeReply(uint8_t out[48], uint32_t unix_epoch = 1789000000UL) { + memset(out, 0, 48); + out[0] = (0 << 6) | (4 << 3) | 4; // LI=0, VN=4, mode=4 (server) + out[1] = 2; // stratum + writeU32(out + 24, NONCE.seconds); // originate = our transmit timestamp + writeU32(out + 28, NONCE.fraction); + writeU32(out + 32, unix_epoch + kUnixEpochOffset); // receive + writeU32(out + 40, unix_epoch + kUnixEpochOffset); // transmit +} + +Result check(const uint8_t* data, size_t len, bool source_matches = true) { + return validate(data, len, source_matches, NONCE, MIN_EPOCH, MAX_EPOCH); +} + +} // namespace + +TEST(NtpValidation, RequestIsAClientPacketCarryingTheNonce) { + uint8_t req[48]; + memset(req, 0xFF, sizeof(req)); + buildRequest(req, NONCE); + + EXPECT_EQ(0u, (req[0] >> 6) & 0x03); // LI = 0, not the library's bogus alarm + EXPECT_EQ(4u, (req[0] >> 3) & 0x07); // version 4 + EXPECT_EQ(3u, req[0] & 0x07); // mode 3 = client + EXPECT_EQ(NONCE.seconds, readU32(req + 40)); + EXPECT_EQ(NONCE.fraction, readU32(req + 44)); +} + +TEST(NtpValidation, ValidReplyIsAccepted) { + uint8_t pkt[48]; + makeReply(pkt, 1789000000UL); + + Result r = check(pkt, sizeof(pkt)); + + EXPECT_EQ(kAccepted, r.reject); + EXPECT_EQ(1789000000UL, r.epoch); +} + +// F07's reproduction: the installed NTPClient accepted a one-byte non-NTP +// datagram and derived epoch 2085978496 from the uninitialised buffer. +TEST(NtpValidation, OneByteNonNtpDatagramIsRejected) { + const uint8_t junk[1] = { 0x00 }; + + Result r = check(junk, sizeof(junk)); + + EXPECT_EQ(kShortPacket, r.reject); + EXPECT_EQ(0u, r.epoch); +} + +TEST(NtpValidation, ShortAndEmptyDatagramsAreRejected) { + uint8_t pkt[48]; + makeReply(pkt); + + for (size_t len = 0; len < 48; len++) { + EXPECT_EQ(kShortPacket, check(pkt, len).reject) << "len " << len; + } + EXPECT_EQ(kShortPacket, check(nullptr, 48).reject); +} + +TEST(NtpValidation, ReplyFromAnotherSourceIsRejected) { + uint8_t pkt[48]; + makeReply(pkt); + + EXPECT_EQ(kWrongSource, check(pkt, sizeof(pkt), /*source_matches=*/false).reject); +} + +// An off-path sender that never saw the query cannot echo the nonce. +TEST(NtpValidation, UnsolicitedReplyIsRejected) { + uint8_t pkt[48]; + makeReply(pkt); + writeU32(pkt + 24, 0); + writeU32(pkt + 28, 0); + + EXPECT_EQ(kOriginMismatch, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt); + writeU32(pkt + 28, NONCE.fraction ^ 1u); // one bit off is still not ours + EXPECT_EQ(kOriginMismatch, check(pkt, sizeof(pkt)).reject); +} + +TEST(NtpValidation, ClientModeAndWrongVersionAreRejected) { + uint8_t pkt[48]; + + makeReply(pkt); + pkt[0] = (0 << 6) | (4 << 3) | 3; // our own request echoed back + EXPECT_EQ(kBadMode, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt); + pkt[0] = (0 << 6) | (5 << 3) | 4; // version 5 does not exist + EXPECT_EQ(kBadVersion, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt); + pkt[0] = (0 << 6) | (3 << 3) | 4; // NTPv3 servers are fine + EXPECT_EQ(kAccepted, check(pkt, sizeof(pkt)).reject); +} + +TEST(NtpValidation, UnsynchronisedServersAreRejected) { + uint8_t pkt[48]; + + makeReply(pkt); + pkt[0] |= (3 << 6); // LI = 3: alarm, clock not set + EXPECT_EQ(kLeapAlarm, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt); + pkt[1] = 0; // stratum 0: kiss-of-death + EXPECT_EQ(kBadStratum, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt); + pkt[1] = 16; // unsynchronised + EXPECT_EQ(kBadStratum, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt); + pkt[1] = 15; // last usable stratum + EXPECT_EQ(kAccepted, check(pkt, sizeof(pkt)).reject); +} + +TEST(NtpValidation, MissingTransmitTimestampIsRejected) { + uint8_t pkt[48]; + makeReply(pkt); + writeU32(pkt + 40, 0); + + EXPECT_EQ(kZeroTransmit, check(pkt, sizeof(pkt)).reject); +} + +// The bridge's floor is the only defence against a stale or rolled-back clock, +// and nothing previously stopped an absurd forward jump. +TEST(NtpValidation, ImplausibleTimesAreRejected) { + uint8_t pkt[48]; + + makeReply(pkt, MIN_EPOCH - 1); + EXPECT_EQ(kEpochTooEarly, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt, MIN_EPOCH); + EXPECT_EQ(kAccepted, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt, MAX_EPOCH + 1); + EXPECT_EQ(kEpochTooLate, check(pkt, sizeof(pkt)).reject); + + makeReply(pkt, MAX_EPOCH); + EXPECT_EQ(kAccepted, check(pkt, sizeof(pkt)).reject); +} + +// Era 1 begins 2036-02-07; a naive subtraction turns those seconds into 1900. +TEST(NtpValidation, Era1TimestampsConvertForward) { + EXPECT_EQ(0u, unixFromNtpSeconds(kUnixEpochOffset)); + EXPECT_EQ(2085978496UL, unixFromNtpSeconds(0)); // 2036-02-07 + EXPECT_GT(unixFromNtpSeconds(1000), unixFromNtpSeconds(0xFFFFFFFFu)); + + uint8_t pkt[48]; + makeReply(pkt); + writeU32(pkt + 40, 1000); // 1000 s into era 1 + Result r = check(pkt, sizeof(pkt)); + EXPECT_EQ(kAccepted, r.reject); + EXPECT_EQ(2085979496UL, r.epoch); +} + +TEST(NtpValidation, EveryRejectionHasAShortReason) { + for (int i = 0; i <= (int)kEpochTooLate; i++) { + const char* reason = rejectReason((Reject)i); + ASSERT_NE(nullptr, reason); + EXPECT_GT(strlen(reason), 0u); + EXPECT_LE(strlen(reason), 20u) << "reject " << i; + } +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +}