From 786c40023e1348e7613805446ae821af7030b5d3 Mon Sep 17 00:00:00 2001 From: Fabiano Fidêncio Date: Mar 29 2018 18:15:21 +0000 Subject: KCM: Do not use 2048 as fixed size for the payload The KCM code has the limit set as 2048 only inside #ifdef __APPLE__, while it should be normally set as 10 * 1024 * 1024, as seen in: https://github.com/krb5/krb5/blob/master/src/lib/krb5/ccache/cc_kcm.c#L53 Last but not least, doesn't make much sense to use a fixed value as the first 4 bytes received are the payload size ... so let's just allocate the needed size instead of having a fixed value. Resolves: https://pagure.io/SSSD/sssd/issue/3671 Signed-off-by: Fabiano Fidêncio Reviewed-by: Jakub Hrozek --- diff --git a/src/responder/kcm/kcmsrv_cmd.c b/src/responder/kcm/kcmsrv_cmd.c index 3ecba9d..728979d 100644 --- a/src/responder/kcm/kcmsrv_cmd.c +++ b/src/responder/kcm/kcmsrv_cmd.c @@ -38,7 +38,7 @@ /* The maximum length of a request or reply as defined by the RPC * protocol. This is the same constant size as MIT KRB5 uses */ -#define KCM_PACKET_MAX_SIZE 2048 +#define KCM_PACKET_MAX_SIZE 10*1024*1024 /* KCM operation, its raw input and raw output and result */ struct kcm_op_io { @@ -125,7 +125,6 @@ struct kcm_reqbuf { struct kcm_iovec v_len; /* Includes the major, minor versions etc */ - uint8_t msgbuf[KCM_PACKET_MAX_SIZE]; struct kcm_iovec v_msg; }; @@ -238,7 +237,6 @@ struct kcm_repbuf { uint8_t rcbuf[KCM_RETCODE_SIZE]; struct kcm_iovec v_rc; - uint8_t msgbuf[KCM_PACKET_MAX_SIZE]; struct kcm_iovec v_msg; }; @@ -259,11 +257,13 @@ static errno_t kcm_failbuf_construct(errno_t ret, /* retcode is 0 if the operation at least ran, non-zero if there * was some kind of internal KCM error, like input couldn't be parsed */ -static errno_t kcm_output_construct(struct kcm_op_io *op_io, +static errno_t kcm_output_construct(TALLOC_CTX *mem_ctx, + struct kcm_op_io *op_io, struct kcm_repbuf *repbuf) { - size_t c; + uint8_t *rep; size_t replen; + size_t c; replen = sss_iobuf_get_len(op_io->reply); if (replen > KCM_PACKET_MAX_SIZE) { @@ -281,14 +281,22 @@ static errno_t kcm_output_construct(struct kcm_op_io *op_io, SAFEALIGN_SETMEM_UINT32(repbuf->rcbuf, 0, &c); if (replen > 0) { + rep = talloc_zero_array(mem_ctx, uint8_t, replen); + if (rep == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to allocate memory for the message\n"); + return ENOMEM; + } + c = 0; - SAFEALIGN_MEMCPY_CHECK(repbuf->msgbuf, + SAFEALIGN_MEMCPY_CHECK(rep, sss_iobuf_get_data(op_io->reply), replen, - repbuf->v_msg.kiov_len, + replen, &c); - /* Length of the buffer to send to KCM client */ + /* Set the buffer and its length to send to KCM client */ + repbuf->v_msg.kiov_base = rep; repbuf->v_msg.kiov_len = replen; } @@ -321,24 +329,6 @@ static void kcm_reply_error(struct cli_ctx *cctx, TEVENT_FD_WRITEABLE(cctx->cfde); } -static void kcm_send_reply(struct cli_ctx *cctx, - struct kcm_op_io *op_io, - struct kcm_repbuf *repbuf) -{ - errno_t ret; - - DEBUG(SSSDBG_TRACE_INTERNAL, "Sending a reply\n"); - ret = kcm_output_construct(op_io, repbuf); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, - "Cannot construct the reply buffer, terminating client\n"); - kcm_reply_error(cctx, ret, repbuf); - return; - } - - TEVENT_FD_WRITEABLE(cctx->cfde); -} - /** * Request-reply dispatcher */ @@ -356,6 +346,26 @@ struct kcm_req_ctx { struct kcm_op_io op_io; }; +static void kcm_send_reply(struct kcm_req_ctx *req_ctx) +{ + struct cli_ctx *cctx; + errno_t ret; + + DEBUG(SSSDBG_TRACE_INTERNAL, "Sending a reply\n"); + + cctx = req_ctx->cctx; + + ret = kcm_output_construct(cctx, &req_ctx->op_io, &req_ctx->repbuf); + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Cannot construct the reply buffer, terminating client\n"); + kcm_reply_error(cctx, ret, &req_ctx->repbuf); + return; + } + + TEVENT_FD_WRITEABLE(cctx->cfde); +} + static void kcm_cmd_request_done(struct tevent_req *req); static errno_t kcm_cmd_dispatch(struct kcm_ctx *kctx, @@ -385,11 +395,9 @@ static errno_t kcm_cmd_dispatch(struct kcm_ctx *kctx, static void kcm_cmd_request_done(struct tevent_req *req) { struct kcm_req_ctx *req_ctx; - struct cli_ctx *cctx; errno_t ret; req_ctx = tevent_req_callback_data(req, struct kcm_req_ctx); - cctx = req_ctx->cctx; ret = kcm_cmd_recv(req_ctx, req, &req_ctx->op_io.reply); @@ -397,15 +405,19 @@ static void kcm_cmd_request_done(struct tevent_req *req) if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "KCM operation failed [%d]: %s\n", ret, sss_strerror(ret)); - kcm_reply_error(cctx, ret, &req_ctx->repbuf); + kcm_reply_error(req_ctx->cctx, ret, &req_ctx->repbuf); return; } - kcm_send_reply(cctx, &req_ctx->op_io, &req_ctx->repbuf); + kcm_send_reply(req_ctx); } -static errno_t kcm_recv_data(int fd, struct kcm_reqbuf *reqbuf) +static errno_t kcm_recv_data(TALLOC_CTX *mem_ctx, + int fd, + struct kcm_reqbuf *reqbuf) { + uint8_t *msg; + uint32_t msglen; errno_t ret; ret = kcm_read_iovec(fd, &reqbuf->v_len); @@ -416,6 +428,24 @@ static errno_t kcm_recv_data(int fd, struct kcm_reqbuf *reqbuf) return ret; } + msglen = kcm_input_get_payload_len(&reqbuf->v_len); + if (msglen > KCM_PACKET_MAX_SIZE) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Request exceeds the KCM protocol limit, aborting\n"); + return E2BIG; + } + + msg = talloc_zero_array(mem_ctx, uint8_t, msglen); + if (msg == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Failed to allocate memory for the message\n"); + return ENOMEM; + } + + /* Set the buffer and its expected len to receive the data */ + reqbuf->v_msg.kiov_base = msg; + reqbuf->v_msg.kiov_len = msglen; + ret = kcm_read_iovec(fd, &reqbuf->v_msg); if (ret != EOK) { /* Not all errors are fatal, hence we don't print DEBUG messages @@ -443,21 +473,12 @@ static struct kcm_req_ctx *kcm_new_req(struct cli_ctx *cctx, req->reqbuf.v_len.kiov_base = req->reqbuf.lenbuf; req->reqbuf.v_len.kiov_len = KCM_MSG_LEN_SIZE; - req->reqbuf.v_msg.kiov_base = req->reqbuf.msgbuf; - req->reqbuf.v_msg.kiov_len = KCM_PACKET_MAX_SIZE; - req->repbuf.v_len.kiov_base = req->repbuf.lenbuf; req->repbuf.v_len.kiov_len = KCM_MSG_LEN_SIZE; req->repbuf.v_rc.kiov_base = req->repbuf.rcbuf; req->repbuf.v_rc.kiov_len = KCM_RETCODE_SIZE; - req->repbuf.v_msg.kiov_base = req->repbuf.msgbuf; - /* Length of the msg iobuf will be adjusted later, so far use the full - * length so that constructing the reply can use that capacity - */ - req->repbuf.v_msg.kiov_len = KCM_PACKET_MAX_SIZE; - req->cctx = cctx; req->kctx = kctx; @@ -485,7 +506,7 @@ static void kcm_recv(struct cli_ctx *cctx) cctx->state_ctx = req; } - ret = kcm_recv_data(cctx->cfd, &req->reqbuf); + ret = kcm_recv_data(req, cctx->cfd, &req->reqbuf); switch (ret) { case ENODATA: DEBUG(SSSDBG_TRACE_ALL, "Client closed connection.\n");