From 9f078d2e9ec7e1803b6c7e2f8a51e0e185723e76 Mon Sep 17 00:00:00 2001

From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>

Date: Wed, 14 Mar 2018 00:57:39 +0100

Subject: [PATCH 11/15] KCM: Do not use 2048 as fixed size for the payload

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

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 <fidencio@redhat.com>

Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>

---

 src/responder/kcm/kcmsrv_cmd.c | 103 +++++++++++++++++++++++++----------------

 1 file changed, 62 insertions(+), 41 deletions(-)

diff --git a/src/responder/kcm/kcmsrv_cmd.c b/src/responder/kcm/kcmsrv_cmd.c

index 3ecba9df2..728979da9 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");

-- 

2.14.3