[Git][NTPsec/ntpsec][master] 2 commits: Cleanup NTPv1 processing
Hal Murray (@hal.murray)
gitlab at mg.gitlab.com
Fri Dec 23 03:20:55 UTC 2022
Hal Murray pushed to branch master at NTPsec / ntpsec
Commits:
1195ff02 by Hal Murray at 2022-12-22T17:04:15-08:00
Cleanup NTPv1 processing
ntpq sysstats now shows NTPv1 activity
add NTPv1 packet count to sysstats log files
- - - - -
b8900bfc by Hal Murray at 2022-12-22T17:44:05-08:00
Fix for gliitches found by Google's oss-fuzz
- - - - -
14 changed files:
- NEWS.adoc
- docs/includes/mon-commands.adoc
- include/ntp.h
- include/ntpd.h
- include/nts.h
- ntpclients/ntpq.py
- ntpd/ntp_control.c
- ntpd/ntp_proto.c
- ntpd/ntp_util.c
- ntpd/nts.c
- ntpd/nts_client.c
- ntpd/nts_extens.c
- ntpd/nts_server.c
- tests/ntpd/nts_client.c
Changes:
=====================================
NEWS.adoc
=====================================
@@ -12,7 +12,9 @@ on user-visible changes.
## Repository Head
-Restore NTPv1 support.
+Restore/cleanup NTPv1 support.
+ ntpq sysstats now shows NTPv1 traffic.
+ NTPv1 counter added to sysstats log file.
NTS supports partial wildcards, for example *.example.com
=====================================
docs/includes/mon-commands.adoc
=====================================
@@ -168,25 +168,26 @@ The BOGON flags are decoded link:decode.html#flash[here].
generation set named _sysstats_:
+
-----------------------------------------------------------
-50928 2132.543 36000 81965 0 9546 56 71793 512 540 10 147 1
+59935 82782.547 3600 36082754 31287166 26510580 4779042 113 19698 1997 428 4773352 0 366120
-----------------------------------------------------------
+
[width="100%",cols="<34%,<33%,<33%",]
|==================================================
-|Item |Units |Description
-|+50928+ |MJD |date
-|+2132.543+|s |time past midnight
-|+3600+ |s |time since reset
-|+81965+ |# |packets received
-|+0+ |# |packets for this host
-|+9546+ |# |current versions
-|+56+ |# |old version
-|+512+ |# |access denied
-|+540+ |# |bad length or format
-|+10+ |# |bad authentication
-|+4+ |# |declined
-|+147+ |# |rate exceeded
-|+1+ |# |kiss-o'-death packets sent
+|Item |Units |Description
+|+59935+ |MJD |date
+|+82782.547+|s |time past midnight
+|+3600+ |s |time since reset
+|+36082754+ |# |packets received
+|+31287166+ |# |packets processed
+|+26510580+ |# |current version
+|+4779042+ |# |old version(s)
+|+113+ |# |access denied
+|+19698+ |# |bad length or format
+|+1997+ |# |bad authentication
+|+428+ |# |declined
+|+4773352+ |# |rate exceeded
+|+0+ |# |kiss-o'-death packets sent
+|+366120+ |# |NTPv1 packets received
|==================================================
+
The first two fields show the date (Modified Julian Day) and time
=====================================
include/ntp.h
=====================================
@@ -61,6 +61,7 @@ void ntp_RAND_priv_bytes(unsigned char *buf, int num);
*/
#define NTP_VERSION 4 /* current version number */
#define NTP_OLDVERSION 1 /* oldest credible version: see #707 */
+#define NTPv1 1 /* Gets special treatment: see receive() */
#define NTP_PORT 123 /* included for non-unix machines */
#define NTP_PORTA "123" /* or unix without /etc/services */
=====================================
include/ntpd.h
=====================================
@@ -171,6 +171,10 @@ stat_sys_form(processed);
stat_sys_form(restricted);
stat_sys_form(newversion);
stat_sys_form(oldversion);
+stat_sys_form(version1);
+stat_sys_form(version1client);
+stat_sys_form(version1zero);
+stat_sys_form(version1symm);
stat_sys_form(badlength);
stat_sys_form(badauth);
stat_sys_form(declined);
=====================================
include/nts.h
=====================================
@@ -65,6 +65,10 @@ bool nts_ke_setup_send(struct BufCtl_t *buf, int aead,
*/
+/* 2 byte type, 2 byte length */
+#define NTS_KE_HDR_LNG 4
+#define NTS_KE_U16_LNG 2
+
/* xxx_append_record_foo makes whole record with one foo */
/* append_foo appends foo to existing partial record */
void ke_append_record_null(BufCtl* buf, uint16_t type);
=====================================
ntpclients/ntpq.py
=====================================
@@ -1553,20 +1553,24 @@ usage: kerninfo
def do_sysstats(self, _line):
"display system uptime and packet counts"
sysstats = (
- ("ss_uptime", "uptime: ", NTP_UPTIME),
+ ("ss_uptime", "uptime: ", NTP_UPTIME),
("ss_numctlreq", "control requests: ", NTP_INT),
)
sysstats2 = (
- ("ss_reset", "sysstats reset: ", NTP_UPTIME),
- ("ss_received", "packets received: ", NTP_PACKETS),
- ("ss_thisver", "current version: ", NTP_PACKETS),
- ("ss_oldver", "older version: ", NTP_PACKETS),
+ ("ss_reset", "sysstats reset: ", NTP_UPTIME),
+ ("ss_received", "packets received: ", NTP_PACKETS),
+ ("ss_thisver", "current version: ", NTP_PACKETS),
+ ("ss_oldver", "older version: ", NTP_PACKETS),
+ ("ss_ver1", "NTPv1 total: ", NTP_PACKETS),
+ ("ss_ver1client","NTPv1 clients: ", NTP_PACKETS),
+ ("ss_ver1zero", "NTPv1 mode0: ", NTP_PACKETS),
+ ("ss_ver1symm", "NTPv1 symm act: ", NTP_PACKETS),
("ss_badformat", "bad length or format: ", NTP_PACKETS),
- ("ss_badauth", "authentication failed:", NTP_PACKETS),
- ("ss_declined", "declined: ", NTP_PACKETS),
- ("ss_restricted", "restricted: ", NTP_PACKETS),
- ("ss_limited", "rate limited: ", NTP_PACKETS),
- ("ss_kodsent", "KoD responses: ", NTP_PACKETS),
+ ("ss_badauth", "authentication failed:", NTP_PACKETS),
+ ("ss_declined", "declined: ", NTP_PACKETS),
+ ("ss_restricted","restricted: ", NTP_PACKETS),
+ ("ss_limited", "rate limited: ", NTP_PACKETS),
+ ("ss_kodsent", "KoD responses: ", NTP_PACKETS),
("ss_processed", "processed for time: ", NTP_PACKETS),
)
self.collect_display(associd=0, variables=sysstats, decodestatus=False)
=====================================
ntpd/ntp_control.c
=====================================
@@ -205,6 +205,7 @@ static const struct ctl_var sys_var[] = {
{ CS_SS_THISVER, RO, "ss_thisver" },
#define CS_SS_OLDVER 37
{ CS_SS_OLDVER, RO, "ss_oldver" },
+/* CS_SS_VER1 is way below */
#define CS_SS_BADFORMAT 38
{ CS_SS_BADFORMAT, RO, "ss_badformat" },
#define CS_SS_BADAUTH 39
@@ -358,6 +359,7 @@ static const struct ctl_var sys_var[] = {
{ CS_SS_THISVER_R, RO, "ss_thisver_r" },
#define CS_SS_OLDVER_R 110
{ CS_SS_OLDVER_R, RO, "ss_oldver_r" },
+/* CS_SS_VER1_R is way below */
#define CS_SS_BADFORMAT_R 111
{ CS_SS_BADFORMAT_R, RO, "ss_badformat_r" },
#define CS_SS_BADAUTH_R 112
@@ -370,46 +372,67 @@ static const struct ctl_var sys_var[] = {
{ CS_SS_LIMITED_R, RO, "ss_limited_r" },
#define CS_SS_KODSENT_R 116
{ CS_SS_KODSENT_R, RO, "ss_kodsent_r" },
-#define CS_SS_PROCESSED_R 117
+#define CS_SS_PROCESSED_R 117
{ CS_SS_PROCESSED_R, RO, "ss_processed_r" },
+
+/* These belong way above. */
+#define CS_SS_VER1 118
+ { CS_SS_VER1, RO, "ss_ver1" },
+#define CS_SS_VER1_R 119
+ { CS_SS_VER1_R, RO, "ss_ver1_r" },
+#define CS_SS_VER1CLIENT 120
+ { CS_SS_VER1CLIENT, RO, "ss_ver1client" },
+#define CS_SS_VER1CLIENT_R 121
+ { CS_SS_VER1CLIENT_R, RO, "ss_ver1client_r" },
+#define CS_SS_VER1ZERO 122
+ { CS_SS_VER1ZERO, RO, "ss_ver1zero" },
+#define CS_SS_VER1ZERO_R 123
+ { CS_SS_VER1ZERO_R, RO, "ss_ver1zero_r" },
+#define CS_SS_VER1SYMM 124
+ { CS_SS_VER1SYMM, RO, "ss_ver1symm" },
+#define CS_SS_VER1SYMM_R 125
+ { CS_SS_VER1SYMM_R, RO, "ss_ver1symm_r" },
+
#ifndef DISABLE_NTS
-#define CS_nts_client_send 118
+#define CS_nts_client_send 126
{ CS_nts_client_send, RO, "nts_client_send" },
-#define CS_nts_client_recv_good 119
+#define CS_nts_client_recv_good 127
{ CS_nts_client_recv_good, RO, "nts_client_recv_good" },
-#define CS_nts_client_recv_bad 120
+#define CS_nts_client_recv_bad 128
{ CS_nts_client_recv_bad, RO, "nts_client_recv_bad" },
-#define CS_nts_server_send 121
+#define CS_nts_server_send 129
{ CS_nts_server_send, RO, "nts_server_send" },
-#define CS_nts_server_recv_good 122
+#define CS_nts_server_recv_good 130
{ CS_nts_server_recv_good, RO, "nts_server_recv_good" },
-#define CS_nts_server_recv_bad 123
+#define CS_nts_server_recv_bad 131
{ CS_nts_server_recv_bad, RO, "nts_server_recv_bad" },
-#define CS_nts_cookie_make 124
+#define CS_nts_cookie_make 132
{ CS_nts_cookie_make, RO, "nts_cookie_make" },
-#define CS_nts_cookie_decode 125
+#define CS_nts_cookie_decode 133
{ CS_nts_cookie_decode, RO, "nts_cookie_decode" },
-#define CS_nts_cookie_decode_old 126
+#define CS_nts_cookie_decode_old 134
{ CS_nts_cookie_decode_old, RO, "nts_cookie_decode_old" },
-#define CS_nts_cookie_decode_old2 127
+#define CS_nts_cookie_decode_old2 135
{ CS_nts_cookie_decode_old2, RO, "nts_cookie_decode_old2" },
-#define CS_nts_cookie_decode_older 128
+#define CS_nts_cookie_decode_older 136
{ CS_nts_cookie_decode_older, RO, "nts_cookie_decode_older" },
-#define CS_nts_cookie_decode_too_old 129
+#define CS_nts_cookie_decode_too_old 137
{ CS_nts_cookie_decode_too_old, RO, "nts_cookie_decode_too_old" },
-#define CS_nts_cookie_decode_error 130
+#define CS_nts_cookie_decode_error 138
{ CS_nts_cookie_decode_error, RO, "nts_cookie_decode_error" },
-#define CS_nts_ke_serves_good 131
+#define CS_nts_ke_serves_good 139
{ CS_nts_ke_serves_good, RO, "nts_ke_serves_good" },
-#define CS_nts_ke_serves_bad 132
+#define CS_nts_ke_serves_bad 140
{ CS_nts_ke_serves_bad, RO, "nts_ke_serves_bad" },
-#define CS_nts_ke_probes_good 133
+#define CS_nts_ke_probes_good 141
{ CS_nts_ke_probes_good, RO, "nts_ke_probes_good" },
-#define CS_nts_ke_probes_bad 134
+#define CS_nts_ke_probes_bad 142
{ CS_nts_ke_probes_bad, RO, "nts_ke_probes_bad" },
#endif
+/* Can't insert things here -- need dummy slots for ifdef case */
+
#define CS_MAXCODE ((sizeof(sys_var)/sizeof(sys_var[0])) - 1)
{ 0, EOV, "" }
};
@@ -1577,6 +1600,16 @@ ctl_putsys(
CASE_UINT(CS_SS_OLDVER, stat_oldversion());
+ CASE_UINT(CS_SS_VER1, stat_version1());
+ CASE_UINT(CS_SS_VER1CLIENT, stat_version1client());
+ CASE_UINT(CS_SS_VER1ZERO, stat_version1zero());
+ CASE_UINT(CS_SS_VER1SYMM, stat_version1symm());
+
+ CASE_UINT(CS_SS_VER1_R, stat_total_version1());
+ CASE_UINT(CS_SS_VER1CLIENT_R, stat_total_version1client());
+ CASE_UINT(CS_SS_VER1ZERO_R, stat_total_version1zero());
+ CASE_UINT(CS_SS_VER1SYMM_R, stat_total_version1symm());
+
CASE_UINT(CS_SS_BADFORMAT, stat_badlength());
CASE_UINT(CS_SS_BADAUTH, stat_badauth());
=====================================
ntpd/ntp_proto.c
=====================================
@@ -144,6 +144,10 @@ struct statistics_counters {
uint64_t sys_restricted; /* restricted packets */
uint64_t sys_newversion; /* current version */
uint64_t sys_oldversion; /* old version */
+ uint64_t sys_version1; /* old cruft: NTPv1 */
+ uint64_t sys_version1client; /* NTPv1: mode 3 many */
+ uint64_t sys_version1zero; /* NTPv1: mode 0 as per RFC */
+ uint64_t sys_version1symm; /* NTPv1: mode 1 old Windows */
uint64_t sys_badlength; /* bad length or format */
uint64_t sys_badauth; /* bad authentication */
uint64_t sys_declined; /* declined */
@@ -178,6 +182,10 @@ stat_sys_dumps(processed)
stat_sys_dumps(restricted)
stat_sys_dumps(newversion)
stat_sys_dumps(oldversion)
+stat_sys_dumps(version1)
+stat_sys_dumps(version1client)
+stat_sys_dumps(version1zero)
+stat_sys_dumps(version1symm)
stat_sys_dumps(badlength)
stat_sys_dumps(badauth)
stat_sys_dumps(declined)
@@ -655,6 +663,37 @@ receive(
stat_proto_total.sys_received++;
+#ifdef NTPv1
+ /*Hack for NTPv1. See #707 */
+ if ((NTPv1 == PKT_VERSION(rbufp->recv_buffer[0])) && \
+ (48 == rbufp->recv_length)) {
+ mode = PKT_MODE(rbufp->recv_buffer[0]);
+ stat_proto_total.sys_version1++;
+ /* There is a lot of crufty old NTPv1 SNTP traffic.
+ * NTPv1 has no mode field. */
+ switch (mode) {
+ case MODE_CLIENT:
+ /* Some packets come with MODE_CLIENT. They will just work. */
+ stat_proto_total.sys_version1client++;
+ break;
+ case MODE_UNSPEC:
+ /* Some packets come with 0. Patch them.
+ * This assumes the client doesn't check the returned mode. */
+ rbufp->recv_buffer[0] = PKT_LI_VN_MODE(0, NTPv1, MODE_CLIENT);
+ stat_proto_total.sys_version1zero++;
+ break;
+ case MODE_ACTIVEx:
+ /* Windows XP and Server 2003 send MODE_ACTIVE
+ * https://kb.meinbergglobal.com/kb/time_sync/timekeeping_on_windows/configuring_w32time_as_ntp_client
+ * Again, this assumes the client doesn't check the returned mode. */
+ rbufp->recv_buffer[0] = PKT_LI_VN_MODE(0, NTPv1, MODE_CLIENT);
+ stat_proto_total.sys_version1symm++;
+ break;
+ default:
+ break;
+ }
+ }
+#endif
if(!is_packet_not_low_rot(rbufp)) {
stat_proto_total.sys_badlength++;
return;
@@ -2267,7 +2306,7 @@ fast_xmit(
* The version is copied from the request.
* There are minor differences between v3 and v4.
* So far, nobody cares.
- * Note: There is significant v1 traffic. See #707
+ * Note: There is significant NTPv1 traffic. See #707
*/
xpkt.li_vn_mode = PKT_LI_VN_MODE(sys_vars.sys_leap,
PKT_VERSION(rbufp->pkt.li_vn_mode), MODE_SERVER);
=====================================
ntpd/ntp_util.c
=====================================
@@ -636,6 +636,7 @@ void record_ref_stats(
* declined
* rate exceeded
* KoD sent
+ * NTPv1 packets
*/
void
record_sys_stats(void)
@@ -650,12 +651,13 @@ record_sys_stats(void)
if (sysstats.fp != NULL) {
fprintf(sysstats.fp,
"%s %u %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64
- " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 "\n",
+ " %" PRIu64 " %" PRIu64 " %" PRIu64 " %" PRIu64 \
+ " %" PRIu64 " %" PRIu64"\n",
timespec_to_MJDtime(&now), stat_stattime(),
stat_received(), stat_processed(), stat_newversion(),
stat_oldversion(), stat_restricted(), stat_badlength(),
stat_badauth(), stat_declined(), stat_limitrejected(),
- stat_kodsent());
+ stat_kodsent(), stat_version1());
fflush(sysstats.fp);
proto_clr_stats();
}
=====================================
ntpd/nts.c
=====================================
@@ -296,10 +296,6 @@ void nts_log_ssl_error(void) {
/* NB: KE length is body length, Extension length includes header. */
-/* 2 byte type, 2 byte length */
-#define NTS_KE_HDR_LNG 4
-#define NTS_KE_U16_LNG 2
-
/* Troubles with signed/unsigned compares when using sizeof() */
void ke_append_record_null(BufCtl* buf, uint16_t type) {
=====================================
ntpd/nts_client.c
=====================================
@@ -616,7 +616,7 @@ bool nts_client_process_response_core(uint8_t *buff, int transferred, struct pee
buf.next = buff;
buf.left = transferred;
- while (buf.left > 0) {
+ while (buf.left >= NTS_KE_HDR_LNG) {
uint16_t type, data, port;
bool critical = false;
int length, keylength;
@@ -729,6 +729,9 @@ bool nts_client_process_response_core(uint8_t *buff, int transferred, struct pee
} /* case */
} /* while */
+ if (buf.left > 0)
+ return false;
+
if (NO_AEAD == peer->nts_state.aead) {
msyslog(LOG_ERR, "NTSc: No AEAD algorithim.");
return false;
=====================================
ntpd/nts_extens.c
=====================================
@@ -142,7 +142,7 @@ bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng) {
ntspacket->uidlen = 0;
ntspacket->needed = 0;
- while (buf.left > 0) {
+ while (buf.left >= NTS_KE_HDR_LNG) {
uint16_t type;
bool critical = false;
int length, adlength;
@@ -259,6 +259,9 @@ bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng) {
if (!sawAEEF) {
return false;
}
+ if (buf.left > 0)
+ return false;
+
// printf("ESRx: %d, %d, %d\n",
// lng-LEN_PKT_NOMAC, ntspacket->needed, ntspacket->keylen);
ntspacket->valid = true;
@@ -361,7 +364,7 @@ bool extens_client_recv(struct peer *peer, uint8_t *pkt, int lng) {
buf.next = pkt+LEN_PKT_NOMAC;
buf.left = lng-LEN_PKT_NOMAC;
- while (buf.left > 0) {
+ while (buf.left >= NTS_KE_HDR_LNG) {
uint16_t type;
bool critical = false;
int length, adlength, noncelen;
@@ -442,6 +445,8 @@ bool extens_client_recv(struct peer *peer, uint8_t *pkt, int lng) {
if (!sawAEEF) {
return false;
}
+ if (buf.left > 0)
+ return false;
nts_client_recv_good++;
nts_client_recv_bad--;
return true;
=====================================
ntpd/nts_server.c
=====================================
@@ -491,7 +491,7 @@ bool create_listener6(int port) {
}
bool nts_ke_process_receive(struct BufCtl_t *buf, int *aead) {
- while (buf->left > 0) {
+ while (buf->left >= NTS_KE_HDR_LNG) {
uint16_t type, data;
int length;
bool critical = false;
@@ -554,6 +554,9 @@ bool nts_ke_process_receive(struct BufCtl_t *buf, int *aead) {
} /* case */
} /* while */
+ if (buf->left > 0)
+ return false;
+
return true;
}
=====================================
tests/ntpd/nts_client.c
=====================================
@@ -117,11 +117,15 @@ TEST(nts_client, nts_client_process_response_core) {
/* ===== Test: all correct ===== */
/* data */
uint8_t buf0[] = {
- 0x80, nts_next_protocol_negotiation, 0, 2, 0, nts_protocol_NTP,
- 0x80, nts_algorithm_negotiation, 0, 2, 0, AEAD_AES_SIV_CMAC_256,
- 0x80, nts_new_cookie, 0, 8, 1, 2, 3, 4, 5, 6, 7, 8,
+ 0x80, nts_next_protocol_negotiation, 0, 2,
+ 0, nts_protocol_NTP,
+ 0x80, nts_algorithm_negotiation, 0, 2,
+ 0, AEAD_AES_SIV_CMAC_256,
+ 0x80, nts_new_cookie, 0, 8,
+ 1, 2, 3, 4, 5, 6, 7, 8,
/* server_negotiation skipped due to getaddrinfo() containment breach */
- 0x80, nts_port_negotiation, 0, 2, 0, 3,
+ 0x80, nts_port_negotiation, 0, 2,
+ 0, 123,
0x80, nts_end_of_message, 0, 0
};
/* run */
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/compare/21962ae0a1cd4d85fe2b2a96aeef8c489fe691ff...b8900bfc21cb3df44a3d17df4acb2a533dfd36a3
--
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/compare/21962ae0a1cd4d85fe2b2a96aeef8c489fe691ff...b8900bfc21cb3df44a3d17df4acb2a533dfd36a3
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20221223/f7a3eed1/attachment-0001.htm>
More information about the vc
mailing list