[Git][NTPsec/ntpsec][master] 2 commits: Fix bug that left nts_state.writeIdx out of bounds
Hal Murray
gitlab at mg.gitlab.com
Wed Feb 20 04:18:04 UTC 2019
Hal Murray pushed to branch master at NTPsec / ntpsec
Commits:
49b6b63a by Hal Murray at 2019-02-20T03:42:50Z
Fix bug that left nts_state.writeIdx out of bounds
- - - - -
4f1606d2 by Hal Murray at 2019-02-20T04:16:24Z
Response side of NTS working.
- - - - -
5 changed files:
- include/ntpd.h
- include/nts.h
- ntpd/ntp_extens.c
- ntpd/ntp_proto.c
- ntpd/nts_client.c
Changes:
=====================================
include/ntpd.h
=====================================
@@ -439,6 +439,8 @@ extern const uint8_t num_refclock_conf;
bool extens_init(void);
int extens_client_send(struct peer *peer, struct pkt *xpkt);
bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng);
+int extens_server_send(struct ntspacket_t *ntspacket, struct pkt *xpkt);
+bool extens_client_recv(struct peer *peer, uint8_t *pkt, int lng);
/* nts.c */
void nts_init(void);
=====================================
include/nts.h
=====================================
@@ -53,13 +53,12 @@ struct ntsclient_t {
/* Server-side state per packet */
struct ntspacket_t {
bool valid;
- int extra;
int uidlen;
uint8_t UID[NTS_UID_MAX_LENGTH];
+ int needed;
+ int aead;
int keylen;
uint8_t c2s[NTS_MAX_KEYLEN], s2c[NTS_MAX_KEYLEN];
- int aead;
- uint8_t cookie[NTS_MAX_COOKIELEN];
};
=====================================
ntpd/ntp_extens.c
=====================================
@@ -29,7 +29,15 @@
#define NTP_EX_HDR_LNG 4
#define NTP_EX_U16_LNG 2
-
+/* Statistics */
+uint64_t client_extens_sent = 0;
+uint64_t client_extens_xtra = 0;
+uint64_t client_extens_recv = 0;
+uint64_t client_extens_recv_good = 0;
+uint64_t server_extens_sent = 0;
+uint64_t server_extens_xtra = 0;
+uint64_t server_extens_recv = 0;
+uint64_t server_extens_recv_good = 0;
enum NtpExtFieldType {
Unique_Identifier = 10,
@@ -105,6 +113,7 @@ int extens_client_send(struct peer *peer, struct pkt *xpkt) {
buf.left -= left;
used = buf.next-xpkt->exten;
+ client_extens_sent++;
return used;
}
@@ -112,10 +121,16 @@ bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng) {
struct BufCtl_t buf;
uint16_t aead;
int noncelen, cmaclen;
+ bool sawcookie, sawAEEF;
+
+ server_extens_recv++;
buf.next = pkt+LEN_PKT_NOMAC;
buf.left = lng-LEN_PKT_NOMAC;
+ sawcookie = sawAEEF = false;
+ ntspacket->uidlen = 0;
+
while (buf.left > 0) {
uint16_t type;
bool critical = false;
@@ -125,7 +140,7 @@ bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng) {
bool ok;
type = nts_next_record(&buf, &length);
- if (length > buf.left || length < 0)
+ if (length&3 || length > buf.left || length < 0)
return false;
if (NTS_CRITICAL & type) {
critical = true;
@@ -139,6 +154,8 @@ bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng) {
nts_next_bytes(&buf, ntspacket->UID, length);
break;
case NTS_Cookie:
+ if (sawcookie)
+ return false; /* second cookie */
ok = nts_unpack_cookie(buf.next, length,
&aead,
ntspacket->c2s, ntspacket->s2c, &ntspacket->keylen);
@@ -146,16 +163,18 @@ bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng) {
return false;
buf.next += length;
buf.left -= length;
- ntspacket->valid = true;
+ sawcookie = true;
+ ntspacket->needed = 1;
ntspacket->aead = aead;
break;
case NTS_Cookie_Placeholder:
- ntspacket->extra++;
+ /* doesn't check length */
+ ntspacket->needed++;
buf.next += length;
buf.left -= length;
break;
case NTS_AEEF:
- if (!ntspacket->valid)
+ if (!sawcookie)
return false; /* no cookie yet, no c2s */
if (length != NTP_EX_HDR_LNG+NONCE_LENGTH+CMAC_LENGTH)
return false;
@@ -181,23 +200,201 @@ bool extens_server_recv(struct ntspacket_t *ntspacket, uint8_t *pkt, int lng) {
return false;
if (0 != outlen)
return false;
+ /* we already used 2 length slots way above*/
+ length -= (NTP_EX_U16_LNG+NTP_EX_U16_LNG);
buf.next += length;
buf.left -= length;
- if (0 != buf.left)
+ if (0 != buf.left)
return false; /* Reject extens after AEEF block */
+ sawAEEF = true;
break;
default:
if (critical)
return false;
buf.next += length;
buf.left -= length;
+ return false; // FIXME - for now, it's probably a bug
}
}
- if (!ntspacket->valid)
+ if (!sawAEEF)
return false;
-
+// printf("ESRx: %d, %d, %d\n",
+// lng-LEN_PKT_NOMAC, ntspacket->needed, ntspacket->keylen);
+ ntspacket->valid = true;
+ server_extens_recv_good++;
return true;
}
+int extens_server_send(struct ntspacket_t *ntspacket, struct pkt *xpkt) {
+ struct BufCtl_t buf;
+ int used, adlength;
+ size_t left;
+ uint8_t *nonce, *packet;
+ uint8_t *plaintext, *ciphertext;;
+ uint8_t cookie[NTS_MAX_COOKIELEN];
+ int cookielen, plainleng, aeadlen;
+ bool ok;
+
+ /* get first cookie now so we have length */
+ cookielen = nts_make_cookie(cookie, ntspacket->aead,
+ ntspacket->c2s, ntspacket->s2c, ntspacket->keylen);
+
+ packet = (uint8_t*)xpkt;
+ buf.next = xpkt->exten;
+ buf.left = MAX_EXT_LEN;
+
+ /* UID */
+ if (0 < ntspacket->uidlen)
+ nts_append_record_bytes(&buf, Unique_Identifier,
+ ntspacket->UID, ntspacket->uidlen);
+
+ adlength = buf.next-packet; /* up to here is Additional Data */
+
+ /* length of whole AEEF */
+ plainleng = ntspacket->needed*(NTP_EX_HDR_LNG+cookielen);
+ /* length of whole AEEF header */
+ aeadlen = NTP_EX_U16_LNG*2+NONCE_LENGTH+CMAC_LENGTH + plainleng;
+ nts_append_header(&buf, NTS_AEEF, aeadlen);
+ nts_append_uint16(&buf, NONCE_LENGTH);
+ nts_append_uint16(&buf, plainleng);
+
+ nonce = buf.next;
+ RAND_bytes(nonce, NONCE_LENGTH);
+ buf.next += NONCE_LENGTH;
+ buf.left -= NONCE_LENGTH;
+
+ ciphertext = buf.next; /* cipher text starts here */
+ left = buf.left;
+ buf.next += CMAC_LENGTH; /* skip space for CMAC */
+ buf.left -= CMAC_LENGTH;
+ plaintext = buf.next; /* encrypt in place */
+
+ nts_append_record_bytes(&buf, NTS_Cookie,
+ cookie, cookielen);
+ for (int i=1; i<ntspacket->needed; i++) {
+ nts_make_cookie(cookie, ntspacket->aead,
+ ntspacket->c2s, ntspacket->s2c, ntspacket->keylen);
+ nts_append_record_bytes(&buf, NTS_Cookie,
+ cookie, cookielen);
+ }
+
+//printf("ESSa: %d, %d, %d, %d\n",
+// adlength, plainleng, cookielen, ntspacket->needed);
+
+ ok = AES_SIV_Encrypt(wire_ctx,
+ ciphertext, &left, /* left: in: max out length, out: length used */
+ ntspacket->s2c, ntspacket->keylen,
+ nonce, NONCE_LENGTH,
+ plaintext, plainleng,
+ packet, adlength);
+ if (!ok) {
+ msyslog(LOG_ERR, "NTS: extens_server_send - Error from AES_SIV_Encrypt");
+ nts_log_ssl_error();
+ /* I don't think this should happen,
+ * so crash rather than work incorrectly.
+ * Hal, 2019-Feb-17
+ * Similar code in nts_cookie
+ */
+ exit(1);
+ }
+
+ used = buf.next-xpkt->exten;
+
+// printf("ESSx: %lu, %d\n", (long unsigned)left, used);
+
+ server_extens_sent++;
+ return used;
+}
+
+bool extens_client_recv(struct peer *peer, uint8_t *pkt, int lng) {
+ struct BufCtl_t buf;
+ int idx;
+ bool sawAEEF = false;
+
+ client_extens_recv++;
+
+ buf.next = pkt+LEN_PKT_NOMAC;
+ buf.left = lng-LEN_PKT_NOMAC;
+
+ while (buf.left > 0) {
+ uint16_t type;
+ bool critical = false;
+ int length, adlength, noncelen;
+ uint8_t *nonce, *ciphertext, *plaintext;
+ size_t outlen;
+ bool ok;
+
+ type = nts_next_record(&buf, &length);
+ if (length&3 || length > buf.left || length < 0)
+ return false;
+ if (NTS_CRITICAL & type) {
+ critical = true;
+ type &= ~NTS_CRITICAL;
+ }
+// printf("ECR: %d, %d, %d\n", type, length, buf.left);
+ switch (type) {
+ case Unique_Identifier:
+ if (NTS_UID_LENGTH != length)
+ return false;
+ if (0 != memcmp(buf.next, peer->nts_state.UID, NTS_UID_LENGTH))
+ return false;
+ buf.next += length;
+ buf.left -= length;
+ break;
+ case NTS_Cookie:
+ if (!sawAEEF)
+ return false; /* reject unencrypted cookies */
+ if (NTS_MAX_COOKIES <= peer->nts_state.count)
+ return false; /* reject extra cookies */
+ if (length != peer->nts_state.cookielen)
+ return false; /* reject length change */
+ idx = peer->nts_state.writeIdx++;
+ memcpy((uint8_t*)&peer->nts_state.cookies[idx], buf.next, length);
+ peer->nts_state.writeIdx = peer->nts_state.writeIdx % NTS_MAX_COOKIES;
+ peer->nts_state.count++;
+ buf.next += length;
+ buf.left -= length;
+ break;
+ case NTS_AEEF:
+ adlength = buf.next-NTP_EX_HDR_LNG-pkt; /* backup over header */
+ noncelen = nts_next_uint16(&buf);
+ outlen = nts_next_uint16(&buf);
+ if (noncelen&3 || outlen&3)
+ return false; /* else round up */
+ nonce = buf.next;
+ ciphertext = nonce+noncelen;
+ plaintext = ciphertext+CMAC_LENGTH;
+ outlen = buf.left-NONCE_LENGTH-CMAC_LENGTH;
+// printf("ECRa: %lu, %d\n", (long unsigned)outlen, noncelen);
+ ok = AES_SIV_Decrypt(wire_ctx,
+ plaintext, &outlen,
+ peer->nts_state.s2c, peer->nts_state.keylen,
+ nonce, noncelen,
+ ciphertext, outlen+CMAC_LENGTH,
+ pkt, adlength);
+// printf("ECRb: %d, %lu\n", ok, (long unsigned)outlen);
+ if (!ok)
+ return false;
+ /* setup to process encrypted headers */
+ buf.next += NONCE_LENGTH+CMAC_LENGTH;
+ buf.left -= NONCE_LENGTH+CMAC_LENGTH;
+ sawAEEF = true;
+ break;
+ default:
+ if (critical)
+ return false;
+ buf.next += length;
+ buf.left -= length;
+ return false; // FIXME - for now, it's probably a bug
+ }
+ }
+
+// printf("ECRx: %d, %d %d, %d\n", sawAEEF, peer->nts_state.count,
+// peer->nts_state.writeIdx, peer->nts_state.readIdx);
+ if (!sawAEEF)
+ return false;
+ client_extens_recv_good++;
+ return true;
+}
/* end */
=====================================
ntpd/ntp_proto.c
=====================================
@@ -254,8 +254,6 @@ parse_packet(
rbufp->extens_present = false;
rbufp->ntspacket.valid = false;
- rbufp->ntspacket.extra = 0;
- rbufp->ntspacket.uidlen = 0;
if(PKT_VERSION(pkt->li_vn_mode) > 4) {
/* Unsupported version */
@@ -704,19 +702,23 @@ receive(
switch (PKT_MODE(rbufp->pkt.li_vn_mode)) {
case MODE_ACTIVE: /* remote site using "peer" in config file */
case MODE_CLIENT: /* Request for us as a server. */
- if (rbufp->extens_present) {
- if (!extens_server_recv(&rbufp->ntspacket,
+ if (rbufp->extens_present
+ && !extens_server_recv(&rbufp->ntspacket,
rbufp->recv_buffer, rbufp->recv_length)) {
stat_count.sys_declined++;
break;
- }
}
handle_fastxmit(rbufp, restrict_mask, auth);
stat_count.sys_processed++;
break;
case MODE_SERVER: /* Reply to our request to a server. */
- if (peer == NULL || nts_validate(&peer->cfg.nts_cfg, &peer->nts_state,
- &rbufp->pkt) != 0) {
+ if (NULL == peer) {
+ stat_count.sys_declined++;
+ break;
+ }
+ if (rbufp->extens_present
+ && !extens_client_recv(peer,
+ rbufp->recv_buffer, rbufp->recv_length)) {
stat_count.sys_declined++;
break;
}
@@ -2215,6 +2217,7 @@ fast_xmit(
if (flags & RES_MSSNTP) {
keyid_t keyid = 0;
if (NULL != auth) keyid = auth->keyid;
+ // FIXME need counter
send_via_ntp_signd(rbufp, xmode, keyid, flags, &xpkt);
return;
}
@@ -2228,8 +2231,8 @@ fast_xmit(
*/
sendlen = LEN_PKT_NOMAC;
get_systime(&xmt_tx);
- if (0) {
- sendlen += nts_decorate(NULL, NULL, xpkt.exten, sizeof(xpkt.exten));
+ if (rbufp->ntspacket.valid) {
+ sendlen += extens_server_send(&rbufp->ntspacket, &xpkt);
} else if (NULL != auth) {
sendlen += (size_t)authencrypt(auth, (uint32_t *)&xpkt, (int)sendlen);
}
=====================================
ntpd/nts_client.c
=====================================
@@ -371,6 +371,7 @@ bool nts_client_process_response(struct peer* peer, SSL *ssl) {
nts_next_bytes(&buf, (uint8_t*)&peer->nts_state.cookies[idx], length);
peer->nts_state.valid[idx] = true;
peer->nts_state.writeIdx++;
+ peer->nts_state.writeIdx = peer->nts_state.writeIdx % NTS_MAX_COOKIES;
peer->nts_state.count++;
ntskeyfetches++;
break;
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/8c045f521280aa01c26f36b21257c3dfb550b6c0...4f1606d2cc5b17a4c6dbfab704daeb44479da520
--
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/compare/8c045f521280aa01c26f36b21257c3dfb550b6c0...4f1606d2cc5b17a4c6dbfab704daeb44479da520
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/20190220/2d0179cf/attachment-0001.html>
More information about the vc
mailing list