| [chrony-dev] [PATCH 2/4] Add padding to cmdmon requests to prevent amplification attack |
[ Thread Index |
Date Index
| More chrony.tuxfamily.org/chrony-dev Archives
]
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.
---
candm.h | 30 ++++++++++---
client.c | 13 ++++--
cmdmon.c | 22 ++++++---
pktlength.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
pktlength.h | 2 +
5 files changed, 197 insertions(+), 17 deletions(-)
diff --git a/candm.h b/candm.h
index 262e0bd..4b1c66a 100644
--- a/candm.h
+++ b/candm.h
@@ -360,13 +360,24 @@ typedef struct {
Version 5 : auth data moved to the end of the packet to allow hashes with
different sizes, extended sources, tracking and activity reports, dropped
subnets accessed and client accesses
+
+ Version 6 : added padding to requests to prevent amplification attack,
+ changed maximum number of samples in manual list to 16
*/
-#define PROTO_VERSION_NUMBER 5
+#define PROTO_VERSION_NUMBER 6
+
+/* The oldest protocol versions that are compatible enough with the current
+ version to report a version mismatch for the server and the client */
+#define PROTO_VERSION_MISMATCH_COMPAT_SERVER 5
+#define PROTO_VERSION_MISMATCH_COMPAT_CLIENT 4
+
+/* The first protocol version using padding in requests */
+#define PROTO_VERSION_PADDING 6
-/* The oldest protocol version that is compatible enough with
- the current version to report a version mismatch */
-#define PROTO_VERSION_MISMATCH_COMPAT 4
+/* The maximum length of padding in request packet, currently
+ defined by CLIENT_ACCESSES_BY_INDEX and MANUAL_LIST */
+#define MAX_PADDING_LENGTH 396
/* ================================================== */
@@ -424,8 +435,13 @@ typedef struct {
REQ_ReselectDistance reselect_distance;
} data; /* Command specific parameters */
- /* authentication of the packet, there is no hole after the actual data
- from the data union, this field only sets the maximum auth size */
+ /* The following fields only set the maximum size of the packet.
+ There are no holes between them and the actual data. */
+
+ /* Padding used to prevent traffic amplification */
+ uint8_t padding[MAX_PADDING_LENGTH];
+
+ /* Authentication data */
uint8_t auth[MAX_HASH_LENGTH];
} CMD_Request;
@@ -585,6 +601,7 @@ typedef struct {
uint32_t next_index; /* the index 1 beyond those processed on this call */
uint32_t n_clients; /* the number of valid entries in the following array */
RPY_ClientAccesses_Client clients[MAX_CLIENT_ACCESSES];
+ int32_t EOR;
} RPY_ClientAccessesByIndex;
#define MAX_MANUAL_LIST_SAMPLES 16
@@ -599,6 +616,7 @@ typedef struct {
typedef struct {
uint32_t n_samples;
RPY_ManualListSample samples[MAX_MANUAL_LIST_SAMPLES];
+ int32_t EOR;
} RPY_ManualList;
typedef struct {
diff --git a/client.c b/client.c
index 467bfdd..c7bc74d 100644
--- a/client.c
+++ b/client.c
@@ -1257,6 +1257,7 @@ submit_request(CMD_Request *request, CMD_Reply *reply, int *reply_auth_ok)
int read_length;
int expected_length;
int command_length;
+ int padding_length;
int auth_length;
struct timeval tv;
int timeout;
@@ -1278,6 +1279,13 @@ submit_request(CMD_Request *request, CMD_Reply *reply, int *reply_auth_ok)
n_attempts = 0;
do {
+ command_length = PKL_CommandLength(request);
+ padding_length = PKL_CommandPaddingLength(request);
+ assert(command_length > 0 && command_length > padding_length);
+
+ /* Zero the padding to avoid sending uninitialized data. This needs to be
+ done before generating auth data as it includes the padding. */
+ memset(((char *)request) + command_length - padding_length, 0, padding_length);
/* Decide whether to authenticate */
if (password) {
@@ -1291,9 +1299,6 @@ submit_request(CMD_Request *request, CMD_Reply *reply, int *reply_auth_ok)
auth_length = 0;
}
- command_length = PKL_CommandLength(request);
- assert(command_length > 0);
-
/* add empty MD5 auth so older servers will not drop the request
due to bad length */
if (!auth_length) {
@@ -1401,7 +1406,7 @@ submit_request(CMD_Request *request, CMD_Reply *reply, int *reply_auth_ok)
}
bad_header = ((reply->version != PROTO_VERSION_NUMBER &&
- !(reply->version >= PROTO_VERSION_MISMATCH_COMPAT &&
+ !(reply->version >= PROTO_VERSION_MISMATCH_COMPAT_CLIENT &&
ntohs(reply->status) == STT_BADPKTVERSION)) ||
(reply->pkt_type != PKT_TYPE_CMD_REPLY) ||
(reply->res1 != 0) ||
diff --git a/cmdmon.c b/cmdmon.c
index 9696ff5..a6e3f84 100644
--- a/cmdmon.c
+++ b/cmdmon.c
@@ -265,11 +265,25 @@ prepare_socket(int family)
void
CAM_Initialise(int family)
{
+ int i;
+
assert(!initialised);
initialised = 1;
assert(sizeof (permissions) / sizeof (permissions[0]) == N_REQUEST_TYPES);
+ for (i = 0; i < N_REQUEST_TYPES; i++) {
+ CMD_Request r;
+ int command_length, padding_length;
+
+ r.version = PROTO_VERSION_NUMBER;
+ r.command = htons(i);
+ command_length = PKL_CommandLength(&r);
+ padding_length = PKL_CommandPaddingLength(&r);
+ assert(padding_length <= MAX_PADDING_LENGTH && padding_length <= command_length);
+ assert(command_length == 0 || command_length >= offsetof(CMD_Reply, data));
+ }
+
utoken = (unsigned long) time(NULL);
issued_tokens = returned_tokens = issue_pointer = 0;
@@ -1718,6 +1732,7 @@ read_from_cmd_socket(void *anything)
}
if (expected_length < offsetof(CMD_Request, data) ||
+ read_length < offsetof(CMD_Reply, data) ||
rx_message.pkt_type != PKT_TYPE_CMD_REQUEST ||
rx_message.res1 != 0 ||
rx_message.res2 != 0) {
@@ -1755,12 +1770,9 @@ read_from_cmd_socket(void *anything)
if (allowed)
CLG_LogCommandAccess(&remote_ip, CLG_CMD_BAD_PKT, cooked_now.tv_sec);
- if (rx_message.version >= PROTO_VERSION_MISMATCH_COMPAT) {
+ if (rx_message.version >= PROTO_VERSION_MISMATCH_COMPAT_SERVER) {
tx_message.status = htons(STT_BADPKTVERSION);
- /* add empty MD5 auth so older clients will not drop
- the reply due to bad length */
- memset(((char *)&tx_message) + PKL_ReplyLength(&tx_message), 0, 16);
- transmit_reply(&tx_message, &where_from, 16);
+ transmit_reply(&tx_message, &where_from, 0);
}
return;
}
diff --git a/pktlength.c b/pktlength.c
index 761dacd..03a5a7e 100644
--- 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);
@@ -160,6 +160,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
PKL_ReplyLength(CMD_Reply *r)
{
int type;
diff --git a/pktlength.h b/pktlength.h
index 026468d..fad4c30 100644
--- a/pktlength.h
+++ b/pktlength.h
@@ -33,6 +33,8 @@
extern int PKL_CommandLength(CMD_Request *r);
+extern int PKL_CommandPaddingLength(CMD_Request *r);
+
extern int PKL_ReplyLength(CMD_Reply *r);
#endif /* GOT_PKTLENGTH_H */
--
1.8.4.2
--
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble? Email listmaster@xxxxxxxxxxxxxxxxxxxx.