[Bug 3010] remote configuration trustedkey/requestkey values are not properly validated

bk: 56c78c41-oKNCUhyU5kKQCxLjnp0Fw

diff --git a/ChangeLog b/ChangeLog
index c70fe8fc563cf0a16def01a652b989e8bc667f06..d5a2f9a2323a80c6a4f27f914b4347706f72a4ab 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,6 +2,8 @@
 
 * [Bug 2994] Systems with HAVE_SIGNALED_IO fail to compile. perlinger@ntp.org
 * [Bug 2995] Fixes to compile on Windows
+* [Bug 3010] remote configuration trustedkey/requestkey values
+  are not properly validated. perlinger@ntp.org
 
 ---
 (4.2.8p6) 2016/01/20 Released by Harlan Stenn <stenn@ntp.org>

diff --git a/ntpd/ntp_request.c b/ntpd/ntp_request.c
index ba968e2c8e8aaa39b3ec863f1c236445019eefbd..4641da2adf98289028d40403ee7a270801f44a27 100644 (file)
--- a/ntpd/ntp_request.c
+++ b/ntpd/ntp_request.c
@@ -2297,34 +2297,62 @@ do_setclr_trap(
        return;
 }
 
-
-
 /*
- * set_request_keyid - set the keyid used to authenticate requests
+ * Validate a request packet for a new request or control key:
+ *  - only one item allowed
+ *  - key must be valid (that is, known, and not in the autokey range)
  */
 static void
-set_request_keyid(
-       sockaddr_u *srcadr,
-       endpt *inter,
+set_keyid_checked(
+       keyid_t        *into,
+       const char     *what,
+       sockaddr_u     *srcadr,
+       endpt          *inter,
        struct req_pkt *inpkt
        )
 {
        keyid_t *pkeyid;
+       keyid_t  tmpkey;
 
-       /*
-        * Restrict ourselves to one item only.
-        */
+       /* restrict ourselves to one item only */
        if (INFO_NITEMS(inpkt->err_nitems) > 1) {
-               msyslog(LOG_ERR, "set_request_keyid: err_nitems > 1");
+               msyslog(LOG_ERR, "set_keyid_checked[%s]: err_nitems > 1",
+                       what);
                req_ack(srcadr, inter, inpkt, INFO_ERR_FMT);
                return;
        }
 
+       /* plug the new key from the packet */
        pkeyid = (keyid_t *)&inpkt->u;
-       info_auth_keyid = ntohl(*pkeyid);
+       tmpkey = ntohl(*pkeyid);
+
+       /* validate the new key id, claim data error on failure */
+       if (tmpkey < 1 || tmpkey > NTP_MAXKEY || !auth_havekey(tmpkey)) {
+               msyslog(LOG_ERR, "set_keyid_checked[%s]: invalid key id: %ld",
+                       what, (long)tmpkey);
+               req_ack(srcadr, inter, inpkt, INFO_ERR_NODATA);
+               return;
+       }
+
+       /* if we arrive here, the key is good -- use it */
+       *into = tmpkey;
        req_ack(srcadr, inter, inpkt, INFO_OKAY);
 }
 
+/*
+ * set_request_keyid - set the keyid used to authenticate requests
+ */
+static void
+set_request_keyid(
+       sockaddr_u *srcadr,
+       endpt *inter,
+       struct req_pkt *inpkt
+       )
+{
+       set_keyid_checked(&info_auth_keyid, "request",
+                         srcadr, inter, inpkt);
+}
+
 
 
 /*
@@ -2337,20 +2365,8 @@ set_control_keyid(
        struct req_pkt *inpkt
        )
 {
-       keyid_t *pkeyid;
-
-       /*
-        * Restrict ourselves to one item only.
-        */
-       if (INFO_NITEMS(inpkt->err_nitems) > 1) {
-               msyslog(LOG_ERR, "set_control_keyid: err_nitems > 1");
-               req_ack(srcadr, inter, inpkt, INFO_ERR_FMT);
-               return;
-       }
-
-       pkeyid = (keyid_t *)&inpkt->u;
-       ctl_auth_keyid = ntohl(*pkeyid);
-       req_ack(srcadr, inter, inpkt, INFO_OKAY);
+       set_keyid_checked(&ctl_auth_keyid, "control",
+                         srcadr, inter, inpkt);
 }