Add padding to cmdmon requests to prevent amplification attack

To prevent an attacker using chronyd in an amplification attack, change the protocol to include padding in request packets so that the largest possible reply is not larger than the request. Request packets that don't include this padding are ignored as invalid. This is an incompatible change in the protocol. Clients from chrony 1.27, 1.28 and 1.29 will receive NULL reply with STT_BADPKTVERSION and print "Protocol version mismatch". Clients from 1.26 and older will not receive a reply as it would be larger than the request if it was padded to be compatible with their protocol.
2025-12-03 17:55:07 -05:00 · 2014-01-24 13:55:15 +01:00
parent 3e23430926
commit dba458d50c
5 changed files with 197 additions and 17 deletions
--- a/pktlength.c
+++ b/pktlength.c
@@ -35,8 +35,8 @@

 /* ================================================== */

-int
-PKL_CommandLength(CMD_Request *r)
+static int
+command_unpadded_length(CMD_Request *r)
 {
  int type;
  type = ntohs(r->command);
@@ -157,6 +157,149 @@ PKL_CommandLength(CMD_Request *r)
 }


+/* ================================================== */
+
+int
+PKL_CommandLength(CMD_Request *r)
+{
+  int command_length;
+
+  command_length = command_unpadded_length(r);
+  if (!command_length)
+    return 0;
+
+  return command_length + PKL_CommandPaddingLength(r);
+}
+
+/* ================================================== */
+
+#define PADDING_LENGTH_(request_length, reply_length) \
+  ((request_length) < (reply_length) ? (reply_length) - (request_length) : 0)
+
+#define PADDING_LENGTH(request_data, reply_data) \
+  PADDING_LENGTH_(offsetof(CMD_Request, request_data), offsetof(CMD_Reply, reply_data))
+
+int
+PKL_CommandPaddingLength(CMD_Request *r)
+{
+  int type;
+
+  if (r->version < PROTO_VERSION_PADDING)
+    return 0;
+
+  type = ntohs(r->command);
+
+  if (type < 0 || type >= N_REQUEST_TYPES)
+    return 0;
+
+  switch (type) {
+    case REQ_NULL:
+      return PADDING_LENGTH(data, data.null.EOR);
+    case REQ_ONLINE:
+      return PADDING_LENGTH(data.online.EOR, data.null.EOR);
+    case REQ_OFFLINE:
+      return PADDING_LENGTH(data.offline.EOR, data.null.EOR);
+    case REQ_BURST:
+      return PADDING_LENGTH(data.burst.EOR, data.null.EOR);
+    case REQ_MODIFY_MINPOLL:
+      return PADDING_LENGTH(data.modify_minpoll.EOR, data.null.EOR);
+    case REQ_MODIFY_MAXPOLL:
+      return PADDING_LENGTH(data.modify_maxpoll.EOR, data.null.EOR);
+    case REQ_DUMP:
+      return PADDING_LENGTH(data.dump.EOR, data.null.EOR);
+    case REQ_MODIFY_MAXDELAY:
+      return PADDING_LENGTH(data.modify_maxdelay.EOR, data.null.EOR);
+    case REQ_MODIFY_MAXDELAYRATIO:
+      return PADDING_LENGTH(data.modify_maxdelayratio.EOR, data.null.EOR);
+    case REQ_MODIFY_MAXDELAYDEVRATIO:
+      return PADDING_LENGTH(data.modify_maxdelaydevratio.EOR, data.null.EOR);
+    case REQ_MODIFY_MAXUPDATESKEW:
+      return PADDING_LENGTH(data.modify_maxupdateskew.EOR, data.null.EOR);
+    case REQ_LOGON:
+      return PADDING_LENGTH(data.logon.EOR, data.null.EOR);
+    case REQ_SETTIME:
+      return PADDING_LENGTH(data.settime.EOR, data.manual_timestamp.EOR);
+    case REQ_LOCAL:
+      return PADDING_LENGTH(data.local.EOR, data.null.EOR);
+    case REQ_MANUAL:
+      return PADDING_LENGTH(data.manual.EOR, data.null.EOR);
+    case REQ_N_SOURCES:
+      return PADDING_LENGTH(data.n_sources.EOR, data.n_sources.EOR);
+    case REQ_SOURCE_DATA:
+      return PADDING_LENGTH(data.source_data.EOR, data.source_data.EOR);
+    case REQ_REKEY:
+      return PADDING_LENGTH(data.rekey.EOR, data.null.EOR);
+    case REQ_ALLOW:
+      return PADDING_LENGTH(data.allow_deny.EOR, data.null.EOR);
+    case REQ_ALLOWALL:
+      return PADDING_LENGTH(data.allow_deny.EOR, data.null.EOR);
+    case REQ_DENY:
+      return PADDING_LENGTH(data.allow_deny.EOR, data.null.EOR);
+    case REQ_DENYALL:
+      return PADDING_LENGTH(data.allow_deny.EOR, data.null.EOR);
+    case REQ_CMDALLOW:
+      return PADDING_LENGTH(data.allow_deny.EOR, data.null.EOR);
+    case REQ_CMDALLOWALL:
+      return PADDING_LENGTH(data.allow_deny.EOR, data.null.EOR);
+    case REQ_CMDDENY:
+      return PADDING_LENGTH(data.allow_deny.EOR, data.null.EOR);
+    case REQ_CMDDENYALL:
+      return PADDING_LENGTH(data.allow_deny.EOR, data.null.EOR);
+    case REQ_ACCHECK:
+      return PADDING_LENGTH(data.ac_check.EOR, data.null.EOR);
+    case REQ_CMDACCHECK:
+      return PADDING_LENGTH(data.ac_check.EOR, data.null.EOR);
+    case REQ_ADD_SERVER:
+      return PADDING_LENGTH(data.ntp_source.EOR, data.null.EOR);
+    case REQ_ADD_PEER:
+      return PADDING_LENGTH(data.ntp_source.EOR, data.null.EOR);
+    case REQ_DEL_SOURCE:
+      return PADDING_LENGTH(data.del_source.EOR, data.null.EOR);
+    case REQ_WRITERTC:
+      return PADDING_LENGTH(data.writertc.EOR, data.null.EOR);
+    case REQ_DFREQ:
+      return PADDING_LENGTH(data.dfreq.EOR, data.null.EOR);
+    case REQ_DOFFSET:
+      return PADDING_LENGTH(data.doffset.EOR, data.null.EOR);
+    case REQ_TRACKING:
+      return PADDING_LENGTH(data.tracking.EOR, data.tracking.EOR);
+    case REQ_SOURCESTATS:
+      return PADDING_LENGTH(data.sourcestats.EOR, data.sourcestats.EOR);
+    case REQ_RTCREPORT:
+      return PADDING_LENGTH(data.rtcreport.EOR, data.rtc.EOR);
+    case REQ_TRIMRTC:
+      return PADDING_LENGTH(data.trimrtc.EOR, data.null.EOR);
+    case REQ_CYCLELOGS:
+      return PADDING_LENGTH(data.cyclelogs.EOR, data.null.EOR);
+    case REQ_SUBNETS_ACCESSED:
+    case REQ_CLIENT_ACCESSES:
+      /* No longer supported */
+      return 0;
+    case REQ_CLIENT_ACCESSES_BY_INDEX:
+      return PADDING_LENGTH(data.client_accesses_by_index.EOR, data.client_accesses_by_index.EOR);
+    case REQ_MANUAL_LIST:
+      return PADDING_LENGTH(data.manual_list.EOR, data.manual_list.EOR);
+    case REQ_MANUAL_DELETE:
+      return PADDING_LENGTH(data.manual_delete.EOR, data.null.EOR);
+    case REQ_MAKESTEP:
+      return PADDING_LENGTH(data.make_step.EOR, data.null.EOR);
+    case REQ_ACTIVITY:
+      return PADDING_LENGTH(data.activity.EOR, data.activity.EOR);
+    case REQ_RESELECT:
+      return PADDING_LENGTH(data.reselect.EOR, data.null.EOR);
+    case REQ_RESELECTDISTANCE:
+      return PADDING_LENGTH(data.reselect_distance.EOR, data.null.EOR);
+    case REQ_MODIFY_MINSTRATUM:
+      return PADDING_LENGTH(data.modify_minstratum.EOR, data.null.EOR);
+    case REQ_MODIFY_POLLTARGET:
+      return PADDING_LENGTH(data.modify_polltarget.EOR, data.null.EOR);
+    default:
+      /* If we fall through the switch, it most likely means we've forgotten to implement a new case */
+      assert(0);
+      return 0;
+  }
+}
+
 /* ================================================== */

 int