Update NEWS

Error messages caused by incoming packets need to be rate limited
to avoid filling up disk space.

NEWS

@@ -1,3 +1,12 @@
+New in version 1.23.1
+=====================
+
+Security fixes
+--------------
+* Don't reply to invalid cmdmon packets (CVE-2010-0292)
+* Limit client log memory size (CVE-2010-0293)
+* Limit rate of syslog messages (CVE-2010-0294)
+
 New in version 1.23
 ===================
 

chrony.texi

@@ -1177,6 +1177,7 @@ directives can occur in any order in the file.
 * manual directive::            Allow manual entry using chronyc's settime cmd.
 * maxupdateskew directive::     Stop bad estimates upsetting machine clock
 * noclientlog directive::       Prevent chronyd from gathering data about clients
+* clientloglimit directive::    Set client log memory limit
 * peer directive::              Specify an NTP peer
 * pidfile directive::           Specify the file where chronyd's pid is written
 * port directive::              Set port to use for NTP packets
@@ -2066,6 +2067,21 @@ This directive, which takes no arguments, specifies that client accesses
 are not to be logged.  Normally they are logged, allowing statistics to
 be reported using the @code{clients} command in @code{chronyc}.
 @c }}}
+@c {{{ clientloglimit
+@node clientloglimit directive
+@subsection clientloglimit
+This directive specifies the maximum size of the memory allocated to
+log client accesses.  When the limit is reached, only information for
+clients that have already been logged will be updated.  If 0 is
+specified, the memory size will be unlimited.  The default is 524288
+bytes.
+
+An example of the use of this directive is
+
+@example
+clientloglimit 1048576
+@end example
+@c }}}
 @c {{{ peer
 @node peer directive
 @subsection peer

clientlog.c

@@ -40,6 +40,7 @@
 #include "memory.h"
 #include "reports.h"
 #include "util.h"
+#include "logging.h"
 
 /* Number of bits of address per layer of the table.  This value has
    been chosen on the basis that a server will predominantly be serving
@@ -86,6 +87,13 @@ static int max_nodes = 0;
 /* Flag indicating whether facility is turned on or not */
 static int active = 0;
 
+/* Flag indicating whether memory allocation limit has been reached
+   and no new nodes or subnets should be allocated */
+static int alloc_limit_reached;
+
+static unsigned long alloc_limit;
+static unsigned long alloced;
+
 /* ================================================== */
 
 static void
@@ -128,6 +136,9 @@ CLG_Initialise(void)
   max_nodes = 0;
   n_nodes = 0;
 
+  alloced = 0;
+  alloc_limit = CNF_GetClientLogLimit();
+  alloc_limit_reached = 0;
 }
 
 /* ================================================== */
@@ -140,11 +151,25 @@ CLG_Finalise(void)
 
 /* ================================================== */
 
+static void check_alloc_limit() {
+  if (alloc_limit_reached)
+    return;
+
+  if (alloced >= alloc_limit) {
+    LOG(LOGS_WARN, LOGF_ClientLog, "Client log memory limit reached");
+    alloc_limit_reached = 1;
+  }
+}
+
+/* ================================================== */
+
 static void
 create_subnet(Subnet *parent_subnet, int the_entry)
 {
   parent_subnet->entry[the_entry] = (void *) MallocNew(Subnet);
   clear_subnet((Subnet *) parent_subnet->entry[the_entry]);
+  alloced += sizeof (Subnet);
+  check_alloc_limit();
 }
 
 /* ================================================== */
@@ -157,6 +182,8 @@ create_node(Subnet *parent_subnet, int the_entry)
   parent_subnet->entry[the_entry] = (void *) new_node;
   clear_node(new_node);
 
+  alloced += sizeof (Node);
+
   if (n_nodes == max_nodes) {
     if (nodes) {
       max_nodes += NODE_TABLE_INCREMENT;
@@ -168,8 +195,10 @@ create_node(Subnet *parent_subnet, int the_entry)
       max_nodes = NODE_TABLE_INCREMENT;
       nodes = MallocArray(Node *, max_nodes);
     }
+    alloced += sizeof (Node *) * (max_nodes - n_nodes);
   }
   nodes[n_nodes++] = (Node *) new_node;
+  check_alloc_limit();
 }
 
 /* ================================================== */
@@ -195,11 +224,15 @@ find_subnet(Subnet *subnet, CLG_IP_Addr addr, int bits_left)
 
   if (new_bits_left > 0) {
     if (!subnet->entry[this_subnet]) {
+      if (alloc_limit_reached)
+        return NULL;
       create_subnet(subnet, this_subnet);
     }
     return find_subnet((Subnet *) subnet->entry[this_subnet], new_subnet, new_bits_left);
   } else {
     if (!subnet->entry[this_subnet]) {
+      if (alloc_limit_reached)
+        return NULL;
       create_node(subnet, this_subnet);
     }
     return subnet->entry[this_subnet];
@@ -248,6 +281,8 @@ CLG_LogNTPClientAccess (CLG_IP_Addr client, time_t now)
   Node *node;
   if (active) {
     node = (Node *) find_subnet(&top_subnet, client, 32);
+    if (node == NULL)
+      return;
     node->ip_addr = client;
     ++node->client_hits;
     node->last_ntp_hit = now;
@@ -262,6 +297,8 @@ CLG_LogNTPPeerAccess(CLG_IP_Addr client, time_t now)
   Node *node;
   if (active) {
     node = (Node *) find_subnet(&top_subnet, client, 32);
+    if (node == NULL)
+      return;
     node->ip_addr = client;
     ++node->peer_hits;
     node->last_ntp_hit = now;
@@ -276,6 +313,8 @@ CLG_LogCommandAccess(CLG_IP_Addr client, CLG_Command_Type type, time_t now)
   Node *node;
   if (active) {
     node = (Node *) find_subnet(&top_subnet, client, 32);
+    if (node == NULL)
+      return;
     node->ip_addr = client;
     node->last_cmd_hit = now;
     switch (type) {

cmdmon.c

@@ -654,7 +654,7 @@ transmit_reply(CMD_Reply *msg, struct sockaddr_in *where_to)
   status = sendto(sock_fd, (void *) msg, tx_message_length, 0,
                   (struct sockaddr *) where_to, sizeof(struct sockaddr_in));
 
-  if (status < 0) {
+  if (status < 0 && !LOG_RateLimited()) {
     remote_ip = ntohl(where_to->sin_addr.s_addr);
     remote_port = ntohs(where_to->sin_port);
     LOG(LOGS_WARN, LOGF_CmdMon, "Could not send response to %s:%hu", UTI_IPToDottedQuad(remote_ip), remote_port);
@@ -1593,6 +1593,7 @@ read_from_cmd_socket(void *anything)
   int valid_ts;
   int authenticated;
   int localhost;
+  int allowed;
   unsigned short rx_command;
   unsigned long rx_message_token;
   unsigned long tx_message_token;
@@ -1642,20 +1643,43 @@ read_from_cmd_socket(void *anything)
 
   localhost = (remote_ip == 0x7f000001UL);
 
-  if ((!ADF_IsAllowed(access_auth_table, remote_ip)) &&
-      (!localhost)) {
+  allowed = ADF_IsAllowed(access_auth_table, remote_ip) || localhost;
+
+  if ((read_length < offsetof(CMD_Request, data)) ||
+      (rx_message.version != PROTO_VERSION_NUMBER) ||
+      (rx_message.pkt_type != PKT_TYPE_CMD_REQUEST) ||
+      (rx_message.res1 != 0) ||
+      (rx_message.res2 != 0)) {
+
+    /* We don't know how to process anything like this */
+    if (allowed)
+      CLG_LogCommandAccess(remote_ip, CLG_CMD_BAD_PKT, cooked_now.tv_sec);
+    
+    return;
+  }
+
+  if (read_length != expected_length) {
+    if (!LOG_RateLimited()) {
+      LOG(LOGS_WARN, LOGF_CmdMon, "Read incorrectly sized packet from %s:%hu", UTI_IPToDottedQuad(remote_ip), remote_port);
+    }
+    if (allowed)
+      CLG_LogCommandAccess(remote_ip, CLG_CMD_BAD_PKT, cooked_now.tv_sec);
+    /* For now, just ignore the packet.  We may want to send a reply
+       back eventually */
+    return;
+  }
+
+  if (!allowed) {
     /* The client is not allowed access, so don't waste any more time
        on him.  Note that localhost is always allowed access
        regardless of the defined access rules - otherwise, we could
        shut ourselves out completely! */
 
-    /* We ought to find another way to log this, there is an attack
-       here against the host because an adversary can just keep
-       hitting us with bad packets until our log file(s) fill up. */
-
-    LOG(LOGS_WARN, LOGF_CmdMon, "Command packet received from unauthorised host %s port %d",
-        UTI_IPToDottedQuad(remote_ip),
-        remote_port);
+    if (!LOG_RateLimited()) {
+      LOG(LOGS_WARN, LOGF_CmdMon, "Command packet received from unauthorised host %s port %d",
+          UTI_IPToDottedQuad(remote_ip),
+          remote_port);
+    }
 
     tx_message.status = htons(STT_NOHOSTACCESS);
     transmit_reply(&tx_message, &where_from);
@@ -1664,25 +1688,6 @@ read_from_cmd_socket(void *anything)
   }
 
 
-  if (read_length != expected_length) {
-    LOG(LOGS_WARN, LOGF_CmdMon, "Read incorrectly sized packet from %s:%hu", UTI_IPToDottedQuad(remote_ip), remote_port);
-    CLG_LogCommandAccess(remote_ip, CLG_CMD_BAD_PKT, cooked_now.tv_sec);
-    /* For now, just ignore the packet.  We may want to send a reply
-       back eventually */
-    return;
-  }
-
-  if ((rx_message.version != PROTO_VERSION_NUMBER) ||
-      (rx_message.pkt_type != PKT_TYPE_CMD_REQUEST) ||
-      (rx_message.res1 != 0) ||
-      (rx_message.res2 != 0)) {
-
-    /* We don't know how to process anything like this */
-    CLG_LogCommandAccess(remote_ip, CLG_CMD_BAD_PKT, cooked_now.tv_sec);
-    
-    return;
-  }
-
   rx_command = ntohs(rx_message.command);
 
   /* OK, we have a valid message.  Now dispatch on message type and process it. */
@@ -1759,7 +1764,7 @@ read_from_cmd_socket(void *anything)
       tx_message_length = PKL_ReplyLength(prev_tx_message);
       status = sendto(sock_fd, (void *) prev_tx_message, tx_message_length, 0,
                       (struct sockaddr *) &where_from, sizeof(where_from));
-      if (status < 0) {
+      if (status < 0 && !LOG_RateLimited()) {
         LOG(LOGS_WARN, LOGF_CmdMon, "Could not send response to %s:%hu", UTI_IPToDottedQuad(remote_ip), remote_port);
       }
       return;
@@ -1809,7 +1814,7 @@ read_from_cmd_socket(void *anything)
     tx_message.status = htons(STT_INVALID);
     tx_message.reply = htons(RPY_NULL);
   } else {
-    int allowed = 0;
+    allowed = 0;
 
     /* Check level of authority required to issue the command */
     switch(permissions[rx_command]) {
@@ -1879,7 +1884,7 @@ read_from_cmd_socket(void *anything)
 
         case REQ_LOGON:
           /* If the log-on fails, record the reason why */
-          if (!issue_token) {
+          if (!issue_token && !LOG_RateLimited()) {
             LOG(LOGS_WARN, LOGF_CmdMon,
                 "Bad command logon from %s port %d (md5_ok=%d valid_ts=%d)\n",
                 UTI_IPToDottedQuad(remote_ip),

conf.c

@@ -83,6 +83,7 @@ static void parse_cmddeny(const char *);
 static void parse_cmdport(const char *);
 static void parse_rtconutc(const char *);
 static void parse_noclientlog(const char *);
+static void parse_clientloglimit(const char *);
 static void parse_logchange(const char *);
 static void parse_mailonchange(const char *);
 static void parse_bindaddress(const char *);
@@ -146,6 +147,9 @@ static double mail_change_threshold = 0.0;
    memory */
 static int no_client_log = 0;
 
+/* Limit memory allocated for the clients log */
+static unsigned long client_log_limit = 524288;
+
 /* IP address (host order) for binding the NTP socket to.  0 means INADDR_ANY
    will be used */
 static unsigned long bind_address = 0UL;
@@ -200,6 +204,7 @@ static const Command commands[] = {
   {"cmdport", 7, parse_cmdport},
   {"rtconutc", 8, parse_rtconutc},
   {"noclientlog", 11, parse_noclientlog},
+  {"clientloglimit", 14, parse_clientloglimit},
   {"logchange", 9, parse_logchange},
   {"mailonchange", 12, parse_mailonchange},
   {"bindaddress", 11, parse_bindaddress},
@@ -634,6 +639,21 @@ parse_noclientlog(const char *line)
 
 /* ================================================== */
 
+static void
+parse_clientloglimit(const char *line)
+{
+  if (sscanf(line, "%lu", &client_log_limit) != 1) {
+    LOG(LOGS_WARN, LOGF_Configure, "Could not read clientlog memory limit at line %d", line_number);
+  }
+
+  if (client_log_limit == 0) {
+    /* unlimited */
+    client_log_limit = (unsigned long)-1;
+  }
+}
+
+/* ================================================== */
+
 static void
 parse_logchange(const char *line)
 {
@@ -1195,6 +1215,14 @@ CNF_GetNoClientLog(void)
 
 /* ================================================== */
 
+unsigned long
+CNF_GetClientLogLimit(void)
+{
+  return client_log_limit;
+}
+
+/* ================================================== */
+
 void
 CNF_GetBindAddress(unsigned long *addr)
 {

conf.h

@@ -59,6 +59,7 @@ extern int CNF_GetRTCOnUTC(void);
 extern void CNF_GetLogChange(int *enabled, double *threshold);
 extern void CNF_GetMailOnChange(int *enabled, double *threshold, char **user);
 extern int CNF_GetNoClientLog(void);
+extern unsigned long CNF_GetClientLogLimit(void);
 extern void CNF_GetBindAddress(unsigned long *addr);
 extern void CNF_GetBindCommandAddress(unsigned long *addr);
 extern char *CNF_GetPidFile(void);

logging.c

@@ -40,6 +40,8 @@ static int initialised = 0;
 
 static int is_detached = 0;
 
+static time_t last_limited = 0;
+
 #ifdef WINNT
 static FILE *logfile;
 #endif
@@ -214,3 +216,19 @@ LOG_GoDaemon(void)
 }
 
 /* ================================================== */
+
+int
+LOG_RateLimited(void)
+{
+  time_t now;
+
+  now = time(NULL);
+
+  if (last_limited + 10 > now && last_limited <= now)
+    return 1;
+
+  last_limited = now;
+  return 0;
+}
+
+/* ================================================== */

logging.h

@@ -53,6 +53,7 @@ typedef enum {
   LOGF_Local,
   LOGF_Util,
   LOGF_Main,
+  LOGF_ClientLog,
   LOGF_Configure,
   LOGF_CmdMon,
   LOGF_Acquire,
@@ -84,6 +85,9 @@ extern void LOG_Position(const char *filename, int line_number, const char *func
 
 extern void LOG_GoDaemon(void);
 
+/* Return zero once per 10 seconds */
+extern int LOG_RateLimited(void);
+
 /* Line logging macro.  If the compiler is GNU C, we take advantage of
    being able to get the function name also. */
 #if defined(__GNUC__)

ntp_core.c

@@ -1358,7 +1358,7 @@ process_known
                         &inst->local_ntp_tx,
                         &inst->remote_addr);
 
-      } else {
+      } else if (!LOG_RateLimited()) {
         LOG(LOGS_WARN, LOGF_NtpCore, "NTP packet received from unauthorised host %s port %d",
             UTI_IPToDottedQuad(inst->remote_addr.ip_addr),
             inst->remote_addr.port);
@@ -1526,7 +1526,7 @@ NCR_ProcessNoauthUnknown(NTP_Packet *message, struct timeval *now, NTP_Remote_Ad
                       remote_addr);
       
     }
-  } else {
+  } else if (!LOG_RateLimited()) {
     LOG(LOGS_WARN, LOGF_NtpCore, "NTP packet received from unauthorised host %s port %d",
         UTI_IPToDottedQuad(remote_addr->ip_addr),
         remote_addr->port);

ntp_io.c

@@ -243,7 +243,8 @@ NIO_SendNormalPacket(NTP_Packet *packet, NTP_Remote_Address *remote_addr)
   remote.sin_addr.s_addr = htonl(remote_addr->ip_addr);
 
   if (sendto(sock_fd, (void *) packet, NTP_NORMAL_PACKET_SIZE, 0,
-             (struct sockaddr *) &remote, sizeof(remote)) < 0) {
+             (struct sockaddr *) &remote, sizeof(remote)) < 0 &&
+      !LOG_RateLimited()) {
     LOG(LOGS_WARN, LOGF_NtpIO, "Could not send to %s:%d : %s",
         UTI_IPToDottedQuad(remote_addr->ip_addr), remote_addr->port, strerror(errno));
   }
@@ -266,7 +267,8 @@ NIO_SendAuthenticatedPacket(NTP_Packet *packet, NTP_Remote_Address *remote_addr)
   remote.sin_addr.s_addr = htonl(remote_addr->ip_addr);
 
   if (sendto(sock_fd, (void *) packet, sizeof(NTP_Packet), 0,
-             (struct sockaddr *) &remote, sizeof(remote)) < 0) {
+             (struct sockaddr *) &remote, sizeof(remote)) < 0 &&
+      !LOG_RateLimited()) {
     LOG(LOGS_WARN, LOGF_NtpIO, "Could not send to %s:%d : %s",
         UTI_IPToDottedQuad(remote_addr->ip_addr), remote_addr->port, strerror(errno));
   }