From 83b78ef2ffdc620130c6d4ffb2b37d29343d6964 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Feb 04 2010 14:39:25 +0000 Subject: - don't reply to invalid cmdmon packets (#555367, CVE-2010-0292) - limit client log memory size (#555367, CVE-2010-0293) - limit rate of syslog messages (#555367, CVE-2010-0294) --- diff --git a/chrony-1.23-clientloglimit.patch b/chrony-1.23-clientloglimit.patch new file mode 100644 index 0000000..2a9598e --- /dev/null +++ b/chrony-1.23-clientloglimit.patch @@ -0,0 +1,266 @@ +From 7269f9aad2cb3e7afe7fec601e52df4154502abc Mon Sep 17 00:00:00 2001 +From: Miroslav Lichvar +Date: Thu, 14 Jan 2010 15:34:56 +0100 +Subject: [PATCH 3/3] Add option to limit clientlog memory + +--- + chrony.texi | 16 ++++++++++++++++ + clientlog.c | 39 +++++++++++++++++++++++++++++++++++++++ + conf.c | 28 ++++++++++++++++++++++++++++ + conf.h | 1 + + logging.h | 1 + + 5 files changed, 85 insertions(+), 0 deletions(-) + +diff --git a/chrony.texi b/chrony.texi +index 909a0cc..f4d4a2c 100644 +--- a/chrony.texi ++++ b/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 +diff --git a/clientlog.c b/clientlog.c +index a66c883..97649ec 100644 +--- a/clientlog.c ++++ b/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) { +diff --git a/conf.c b/conf.c +index e34927e..3aed626 100644 +--- a/conf.c ++++ b/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}, +@@ -635,6 +640,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) + { + if (sscanf(line, "%lf", &log_change_threshold) == 1) { +@@ -1195,6 +1215,14 @@ CNF_GetNoClientLog(void) + + /* ================================================== */ + ++unsigned long ++CNF_GetClientLogLimit(void) ++{ ++ return client_log_limit; ++} ++ ++/* ================================================== */ ++ + void + CNF_GetBindAddress(unsigned long *addr) + { +diff --git a/conf.h b/conf.h +index 166cd1b..1e7c14d 100644 +--- a/conf.h ++++ b/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); +diff --git a/logging.h b/logging.h +index 41975ec..ac82ff6 100644 +--- a/logging.h ++++ b/logging.h +@@ -53,6 +53,7 @@ typedef enum { + LOGF_Local, + LOGF_Util, + LOGF_Main, ++ LOGF_ClientLog, + LOGF_Configure, + LOGF_CmdMon, + LOGF_Acquire, +-- +1.6.5.2 + diff --git a/chrony-1.23-invalidcmdmon.patch b/chrony-1.23-invalidcmdmon.patch new file mode 100644 index 0000000..7d391e6 --- /dev/null +++ b/chrony-1.23-invalidcmdmon.patch @@ -0,0 +1,93 @@ +From 7864c7a70ce00369194e734eb2842ecc5f8db531 Mon Sep 17 00:00:00 2001 +From: Miroslav Lichvar +Date: Wed, 13 Jan 2010 17:40:20 +0100 +Subject: [PATCH 1/3] Don't reply to invalid chronyc packets + +--- + cmdmon.c | 49 +++++++++++++++++++++++++++---------------------- + 1 files changed, 27 insertions(+), 22 deletions(-) + +diff --git a/cmdmon.c b/cmdmon.c +index e88d7c3..c9ce0e9 100644 +--- a/cmdmon.c ++++ b/cmdmon.c +@@ -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,8 +1643,31 @@ 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) { ++ 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 +@@ -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. */ +@@ -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]) { +-- +1.6.5.2 + diff --git a/chrony-1.23-syslograte.patch b/chrony-1.23-syslograte.patch new file mode 100644 index 0000000..2e89d65 --- /dev/null +++ b/chrony-1.23-syslograte.patch @@ -0,0 +1,172 @@ +From 0b710499f994823bd938fc6895f097eefb9cc59f Mon Sep 17 00:00:00 2001 +From: Miroslav Lichvar +Date: Wed, 13 Jan 2010 19:02:07 +0100 +Subject: [PATCH 2/3] Limit rate of syslog messages + +Error messages caused by incoming packets need to be rate limited +to avoid filling up disk space. +--- + cmdmon.c | 22 +++++++++++----------- + logging.c | 18 ++++++++++++++++++ + logging.h | 3 +++ + ntp_core.c | 4 ++-- + ntp_io.c | 6 ++++-- + 5 files changed, 38 insertions(+), 15 deletions(-) + +diff --git a/cmdmon.c b/cmdmon.c +index c9ce0e9..f79d282 100644 +--- a/cmdmon.c ++++ b/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); +@@ -1659,7 +1659,9 @@ 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); ++ 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 +@@ -1673,13 +1675,11 @@ read_from_cmd_socket(void *anything) + 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); +@@ -1764,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; +@@ -1884,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), +diff --git a/logging.c b/logging.c +index d432762..8311640 100644 +--- a/logging.c ++++ b/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; ++} ++ ++/* ================================================== */ +diff --git a/logging.h b/logging.h +index 6e73dbb..41975ec 100644 +--- a/logging.h ++++ b/logging.h +@@ -84,6 +84,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__) +diff --git a/ntp_core.c b/ntp_core.c +index 60d433c..9576296 100644 +--- a/ntp_core.c ++++ b/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); +diff --git a/ntp_io.c b/ntp_io.c +index afb6ad1..aaa3cbc 100644 +--- a/ntp_io.c ++++ b/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)); + } +-- +1.6.5.2 + diff --git a/chrony.spec b/chrony.spec index db77053..765fc79 100644 --- a/chrony.spec +++ b/chrony.spec @@ -1,6 +1,6 @@ Name: chrony Version: 1.23 -Release: 5.20081106gitbe42b4%{?dist} +Release: 6.20081106gitbe42b4%{?dist} Summary: An NTP client/server Group: System Environment/Daemons @@ -20,6 +20,9 @@ Patch3: chrony-1.23-gethost.patch Patch4: chrony-1.23-res.patch Patch5: chrony-1.23-cap.patch Patch6: chrony-1.23-s390.patch +Patch8: chrony-1.23-invalidcmdmon.patch +Patch9: chrony-1.23-syslograte.patch +Patch10: chrony-1.23-clientloglimit.patch BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) BuildRequires: libcap-devel readline-devel bison texinfo @@ -44,6 +47,9 @@ cp -p %{SOURCE6} . %patch4 -p1 -b .res %patch5 -p1 -b .cap %patch6 -p1 -b .s390 +%patch8 -p1 -b .invalidcmdmon +%patch9 -p1 -b .syslograte +%patch10 -p1 -b .clientloglimit # don't link with ncurses sed -i 's|-lncurses||' configure @@ -125,6 +131,11 @@ fi %dir %attr(-,chrony,chrony) %{_localstatedir}/log/chrony %changelog +* Thu Feb 04 2010 Miroslav Lichvar 1.23-6.20081106gitbe42b4 +- don't reply to invalid cmdmon packets (#555367, CVE-2010-0292) +- limit client log memory size (#555367, CVE-2010-0293) +- limit rate of syslog messages (#555367, CVE-2010-0294) + * Mon Jun 08 2009 Dan Horak 1.23-5.20081106gitbe42b4 - add patch with support for s390/s390x