[Git][NTPsec/ntpsec][master] 7 commits: Fixed peer_active update timing in ntpd/ntp_peer.c
Eric S. Raymond (@esr)
gitlab at mg.gitlab.com
Mon Apr 27 13:55:17 UTC 2026
Eric S. Raymond pushed to branch master at NTPsec / ntpsec
Commits:
e43e0737 by Eric S. Raymond at 2026-04-26T19:53:34-04:00
Fixed peer_active update timing in ntpd/ntp_peer.c
Without this change, pool and noselect peers could be counted as
active even though later code expects them to be skipped, and
unpeer might not decrement them. This can break the maxclock and
pool growth limts.
Bug found by ChatGPT 5.5, which considered it severity 'High'.
- - - - -
23006456 by Eric S. Raymond at 2026-04-26T20:30:49-04:00
Prevent failure of cert validation & long-running NTS pool peers.
NTS pool children were created with hostname == NULL, then only
FLAG_NTS and nts_state were copied earlyWhen cookies run out,
restart_nts_ke() set FLAG_LOOKUP, so the child rekeyed against the
numeric IP address instead of the original NTS_KE hostname and
lost per-pool NTS config.
Bug found by ChatGPT 5.5, which considered it severity 'High'.
- - - - -
c1c91e74 by Eric S. Raymond at 2026-04-26T22:13:16-04:00
dns_probe() now clears active ifd pthread_create() fails...
...so DNS/NTS lookups do not remain permanently busy.
Bug found by ChatGPT 5.5, which considered it severity 'High'.
- - - - -
9de458b5 by Eric S. Raymond at 2026-04-26T23:28:52-04:00
dns_check() now clear active if pthread_join() fails...
...so DNS/NTS lookups do not stay permanerly busy.
Bug found by ChatGPT 5.5, which considered it severity 'High'.
- - - - -
06958095 by Eric S. Raymond at 2026-04-26T23:34:43-04:00
Prevent a crash bug. Add a NULL cgeck after SSL_new().
On allocation failure, it now logs the OpenSSL error, closes the
socket, increments probers_bad, and exits cleanly.
Bug found by ChatGPT 5.5, which considered it severity 'Medium'.
- - - - -
276e81d2 by Eric S. Raymond at 2026-04-27T09:37:19-04:00
Extra NTS cookies wwre logged as ignored, but not consumed.
This meant the parser could misread cookie data as he next record
and reject the response.
Bug found by ChatGPT 5.5, which considered it severity 'Medium/Low'.
- - - - -
5ac1f369 by Eric S. Raymond at 2026-04-27T09:47:18-04:00
Handle both cases of successful NTS socket connect.
Forva nonblocking socket, connect() has two successful patterns:
1. Comnon case: returns -1 with errno -- EINPROGRESS
2. Imnmediare case: return 0
Previous code only accepted case 1, Udf connect() returned 9, the
condition -1 != err is rrue, so it logged failure and returned false.
Bug found by ChatGPT 5.5, which considered it severity 'Medium'.
- - - - -
5 changed files:
- ntpd/ntp_dns.c
- ntpd/ntp_peer.c
- ntpd/ntp_proto.c
- ntpd/nts_client.c
- tests/ntpd/nts_client.c
Changes:
=====================================
ntpd/ntp_dns.c
=====================================
@@ -80,6 +80,7 @@ bool dns_probe(struct peer* pp)
msyslog(LOG_ERR, "DNS: dns_probe: error from pthread_create: %s, %s",
hostname, strerror(rc));
pthread_sigmask(SIG_SETMASK, &saved_sig_mask, NULL);
+ active = NULL;
return true; /* don't try again */
}
pthread_sigmask(SIG_SETMASK, &saved_sig_mask, NULL);
@@ -103,7 +104,8 @@ void dns_check(void)
rc = pthread_join(worker, NULL);
if (0 != rc) {
msyslog(LOG_ERR, "DNS: dns_check: join failed %s", strerror(rc));
- return; /* leaves active set */
+ active = NULL;
+ return;
}
#ifndef DISABLE_NTS
@@ -201,4 +203,3 @@ static void* dns_lookup(void* arg)
*/
return (void *)NULL;
}
-
=====================================
ntpd/ntp_peer.c
=====================================
@@ -608,9 +608,6 @@ newpeer(
}
UNLINK_HEAD_SLIST(peer, peer_free, p_link);
peer_free_count--;
- if (!(FLAG_NOSELECT & peer->cfg.flags)
- && !(MDF_POOL & peer->cast_flags))
- peer_active++;
if (FLAG_PREEMPT & ctl->flags)
peer_preempt++;
@@ -635,6 +632,9 @@ newpeer(
peer->nts_state.count = -1;
peer->cast_flags = cast_flags;
+ if (!(FLAG_NOSELECT & peer->cfg.flags)
+ && !(MDF_POOL & peer->cast_flags))
+ peer_active++;
set_peerdstadr(peer,
select_peerinterface(peer, srcadr, dstadr));
=====================================
ntpd/ntp_proto.c
=====================================
@@ -2512,18 +2512,22 @@ dns_take_pool(
pctl.minpoll = pool->cfg.minpoll;
pctl.maxpoll = pool->cfg.maxpoll;
pctl.flags = FLAG_PREEMPT | (FLAG_IBURST & pool->cfg.flags);
+ if (pool->cfg.flags & FLAG_NTS) {
+ pctl.flags |= pool->cfg.flags &
+ (FLAG_NTS | FLAG_NTS_ASK | FLAG_NTS_REQ | FLAG_NTS_NOVAL);
+ pctl.nts_cfg = pool->cfg.nts_cfg;
+ }
pctl.mode = 0;
pctl.peerkey = 0;
peer = newpeer(rmtadr, NULL, lcladr,
MODE_CLIENT, &pctl, MDF_UCAST, false);
if (pool->cfg.flags & FLAG_NTS) {
- /* pool is in NTS mode
- NTS info is in pool struct
- copy to new peer, clean out pool struct */
- peer->cfg.flags |= FLAG_NTS;
- memcpy(&peer->nts_state, &pool->nts_state, sizeof(peer->nts_state));
- ZERO(pool->nts_state);
- pool->nts_state.count = -1;
+ /* Keep the NTS-KE name for later cookie renewal. */
+ if (NULL != pool->hostname)
+ peer->hostname = estrdup(pool->hostname);
+ memcpy(&peer->nts_state, &pool->nts_state, sizeof(peer->nts_state));
+ ZERO(pool->nts_state);
+ pool->nts_state.count = -1;
}
peer_xmit(peer);
if (peer->cfg.flags & FLAG_IBURST)
@@ -2921,4 +2925,3 @@ void maybe_log_junk(const char *tag, struct recvbuf *rbufp) {
msyslog(LOG_INFO,
"%s: %s", tag, buf);
}
-
=====================================
ntpd/nts_client.c
=====================================
@@ -139,6 +139,13 @@ bool nts_probe(struct peer * peer) {
ssl = SSL_new(ctx);
SSL_CTX_free(ctx);
}
+ if (NULL == ssl) {
+ msyslog(LOG_ERR, "NTSc: SSL_new failed");
+ nts_log_ssl_error();
+ close(server);
+ ntske_cnt.probes_bad++;
+ return false;
+ }
set_hostname(ssl, hostname);
SSL_set_fd(ssl, server);
@@ -420,39 +427,42 @@ bool connect_TCP_socket(int sockfd, struct addrinfo *addr) {
return false;
}
err = connect(sockfd, addr->ai_addr, addr->ai_addrlen);
- /* The normal case is -1 and errno == EINPROGRESS
+ /* The usual nonblocking case is -1 and errno == EINPROGRESS.
+ * A fast local connection can also succeed immediately.
* Getting connected should be possible if the scheduler
* avoids us for long enough.
* Other errors may be possible. No route?
* I haven't seen that yet. HGM, 2020 Jan 19
*/
- if (-1 != err || EINPROGRESS != errno) {
+ if (-1 == err && EINPROGRESS != errno) {
ntp_strerror_r(errno, errbuf, sizeof(errbuf));
msyslog(LOG_INFO, "NTSc: connect_TCP_socket: connect failed: %s", errbuf);
return false;
}
- FD_ZERO(&fdset);
- FD_SET(sockfd, &fdset);
- timeout.tv_sec = NTS_KE_TIMEOUT;
- timeout.tv_usec = 0;
+ if (-1 == err) {
+ FD_ZERO(&fdset);
+ FD_SET(sockfd, &fdset);
+ timeout.tv_sec = NTS_KE_TIMEOUT;
+ timeout.tv_usec = 0;
- if (0 == select(sockfd + 1, NULL, &fdset, NULL, &timeout)) {
- msyslog(LOG_INFO, "NTSc: connect_TCP_socket: timeout");
- return false;
- }
+ if (0 == select(sockfd + 1, NULL, &fdset, NULL, &timeout)) {
+ msyslog(LOG_INFO, "NTSc: connect_TCP_socket: timeout");
+ return false;
+ }
- /* It's ready, either connected or error. */
- if (-1 == getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &so_error, &so_len)) {
- ntp_strerror_r(errno, errbuf, sizeof(errbuf));
- msyslog(LOG_INFO, "NTSc: connect_TCP_socket: getsockopt failed: %s", errbuf);
- return false;
- }
+ /* It's ready, either connected or error. */
+ if (-1 == getsockopt(sockfd, SOL_SOCKET, SO_ERROR, &so_error, &so_len)) {
+ ntp_strerror_r(errno, errbuf, sizeof(errbuf));
+ msyslog(LOG_INFO, "NTSc: connect_TCP_socket: getsockopt failed: %s", errbuf);
+ return false;
+ }
- if (0 != so_error) {
- ntp_strerror_r(so_error, errbuf, sizeof(errbuf));
- msyslog(LOG_INFO, "NTSc: connect_TCP_socket: connect failed: %s", errbuf);
- return false;
+ if (0 != so_error) {
+ ntp_strerror_r(so_error, errbuf, sizeof(errbuf));
+ msyslog(LOG_INFO, "NTSc: connect_TCP_socket: connect failed: %s", errbuf);
+ return false;
+ }
}
err = fcntl(sockfd, F_SETFL, 0); /* turn off O_NONBLOCK */
@@ -778,6 +788,8 @@ bool nts_client_process_response_core(uint8_t *buff, int transferred, struct pee
idx = peer->nts_state.writeIdx;
if (NTS_MAX_COOKIES <= peer->nts_state.count) {
msyslog(LOG_ERR, "NTSc: Extra cookie ignored.");
+ buf.next += length;
+ buf.left -= length;
break;
}
next_bytes(&buf, (uint8_t*)&peer->nts_state.cookies[idx], length);
=====================================
tests/ntpd/nts_client.c
=====================================
@@ -209,20 +209,43 @@ TEST(nts_client, nts_client_process_response_core) {
/* run */
success = nts_client_process_response_core(buf7, sizeof(buf7), &peer);
TEST_ASSERT_EQUAL(false, success);
- /* ===== Test: nts_new_cookie, have max cookies ===== */
- /* data */
- uint8_t buf8[] = {
- 0x80, nts_new_cookie, 0, 4, 0, 8, 10, 20, 30, 40, 50, 60, 70, 80,
- 0x80, nts_end_of_message, 0, 0
- };
- peer.nts_state.writeIdx = 0;
- peer.nts_state.count = NTS_MAX_COOKIES;
+ /* ===== Test: nts_new_cookie, extra cookies are ignored ===== */
+ uint8_t buf8[6 + 6 + (NTS_MAX_COOKIES + 1) * 8 + 4];
+ int used8 = 0;
+ buf8[used8++] = 0x80;
+ buf8[used8++] = nts_next_protocol_negotiation;
+ buf8[used8++] = 0;
+ buf8[used8++] = 2;
+ buf8[used8++] = 0;
+ buf8[used8++] = nts_protocol_NTP;
+ buf8[used8++] = 0x80;
+ buf8[used8++] = nts_algorithm_negotiation;
+ buf8[used8++] = 0;
+ buf8[used8++] = 2;
+ buf8[used8++] = 0;
+ buf8[used8++] = AEAD_AES_SIV_CMAC_256;
+ for (int i = 0; i < NTS_MAX_COOKIES + 1; i++) {
+ buf8[used8++] = 0x80;
+ buf8[used8++] = nts_new_cookie;
+ buf8[used8++] = 0;
+ buf8[used8++] = 4;
+ buf8[used8++] = 0;
+ buf8[used8++] = (uint8_t)i;
+ buf8[used8++] = 10;
+ buf8[used8++] = 20;
+ }
+ buf8[used8++] = 0x80;
+ buf8[used8++] = nts_end_of_message;
+ buf8[used8++] = 0;
+ buf8[used8++] = 0;
/* run */
- success = nts_client_process_response_core(buf8, sizeof(buf8), &peer);
+ success = nts_client_process_response_core(buf8, used8, &peer);
/* check */
- TEST_ASSERT_EQUAL(false, success);
-//$ TEST_ASSERT_EQUAL(0, peer.nts_state.writeIdx);
- TEST_ASSERT_NOT_EQUAL(10, peer.nts_state.cookies[0][0]);
+ TEST_ASSERT_EQUAL(true, success);
+ TEST_ASSERT_EQUAL_INT32(NTS_MAX_COOKIES, peer.nts_state.count);
+ TEST_ASSERT_EQUAL_INT32(0, peer.nts_state.writeIdx);
+ TEST_ASSERT_EQUAL_INT8(NTS_MAX_COOKIES - 1,
+ peer.nts_state.cookies[NTS_MAX_COOKIES - 1][1]);
/* ===== Test: nts_end_of_message, wrong length ===== */
/* data */
uint8_t buf9[] = {
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/compare/32a950f65ad6fe90d04e90c597416f0377d008e0...5ac1f369637288033b018be3831055e9015c1cf6
--
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/compare/32a950f65ad6fe90d04e90c597416f0377d008e0...5ac1f369637288033b018be3831055e9015c1cf6
You're receiving this email because of your account on gitlab.com. Manage all notifications: https://gitlab.com/-/profile/notifications | Help: https://gitlab.com/help
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20260427/efa2b7f0/attachment-0001.htm>
More information about the vc
mailing list