diff --git a/0009-KCM-Remove-mem_ctx-from-kcm_new_req.patch b/0009-KCM-Remove-mem_ctx-from-kcm_new_req.patch new file mode 100644 index 0000000..fab80a1 --- /dev/null +++ b/0009-KCM-Remove-mem_ctx-from-kcm_new_req.patch @@ -0,0 +1,50 @@ +From 48cff40315cfbfcfae3582935efda961757ceec6 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= +Date: Tue, 13 Mar 2018 21:11:16 +0100 +Subject: [PATCH 09/15] KCM: Remove mem_ctx from kcm_new_req() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Let's remove the mem_ctx argument as we really want cctx to be the +memory context here, so that if the client disconnects the request goes +away. + +Signed-off-by: Fabiano Fidêncio + +Reviewed-by: Jakub Hrozek +--- + src/responder/kcm/kcmsrv_cmd.c | 10 ++++++---- + 1 file changed, 6 insertions(+), 4 deletions(-) + +diff --git a/src/responder/kcm/kcmsrv_cmd.c b/src/responder/kcm/kcmsrv_cmd.c +index 0b933f0b4..d4ebb79bf 100644 +--- a/src/responder/kcm/kcmsrv_cmd.c ++++ b/src/responder/kcm/kcmsrv_cmd.c +@@ -423,8 +423,10 @@ static errno_t kcm_recv_data(int fd, struct kcm_reqbuf *reqbuf) + return EOK; + } + +-static struct kcm_req_ctx *kcm_new_req(TALLOC_CTX *mem_ctx, +- struct cli_ctx *cctx, ++/* Mind that kcm_new_req() does not take a mem_ctx argument on purpose as we ++ * really want the cctx to be the memory context here so that if the client ++ * disconnects, the request goes away. */ ++static struct kcm_req_ctx *kcm_new_req(struct cli_ctx *cctx, + struct kcm_ctx *kctx) + { + struct kcm_req_ctx *req; +@@ -467,8 +469,8 @@ static void kcm_recv(struct cli_ctx *cctx) + kctx = talloc_get_type(cctx->rctx->pvt_ctx, struct kcm_ctx); + req = talloc_get_type(cctx->state_ctx, struct kcm_req_ctx); + if (req == NULL) { +- /* A new request comes in, setup data structures */ +- req = kcm_new_req(cctx, cctx, kctx); ++ /* A new request comes in, setup data structures. */ ++ req = kcm_new_req(cctx, kctx); + if (req == NULL) { + DEBUG(SSSDBG_CRIT_FAILURE, + "Cannot set up client connection\n"); +-- +2.14.3 + diff --git a/0010-KCM-Introduce-kcm_input_get_payload_len.patch b/0010-KCM-Introduce-kcm_input_get_payload_len.patch new file mode 100644 index 0000000..e0ea83e --- /dev/null +++ b/0010-KCM-Introduce-kcm_input_get_payload_len.patch @@ -0,0 +1,61 @@ +From 7fa69ab8152392b11490950ff8aeeef7e0ad14de Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= +Date: Tue, 13 Mar 2018 23:13:35 +0100 +Subject: [PATCH 10/15] KCM: Introduce kcm_input_get_payload_len() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +As this piece of code will be useful for us in the future patches of +this series, let's move it to a new function. + +Signed-off-by: Fabiano Fidêncio + +Reviewed-by: Jakub Hrozek +--- + src/responder/kcm/kcmsrv_cmd.c | 20 ++++++++++++-------- + 1 file changed, 12 insertions(+), 8 deletions(-) + +diff --git a/src/responder/kcm/kcmsrv_cmd.c b/src/responder/kcm/kcmsrv_cmd.c +index d4ebb79bf..3ecba9df2 100644 +--- a/src/responder/kcm/kcmsrv_cmd.c ++++ b/src/responder/kcm/kcmsrv_cmd.c +@@ -129,23 +129,27 @@ struct kcm_reqbuf { + struct kcm_iovec v_msg; + }; + ++static uint32_t kcm_input_get_payload_len(struct kcm_iovec *v) ++{ ++ size_t lc = 0; ++ uint32_t len_be = 0; ++ ++ /* The first 4 bytes before the payload is message length */ ++ SAFEALIGN_COPY_UINT32_CHECK(&len_be, v->kiov_base, v->kiov_len, &lc); ++ ++ return be32toh(len_be); ++} ++ + static errno_t kcm_input_parse(struct kcm_reqbuf *reqbuf, + struct kcm_op_io *op_io) + { +- size_t lc = 0; + size_t mc = 0; + uint16_t opcode_be = 0; +- uint32_t len_be = 0; + uint32_t msglen; + uint8_t proto_maj = 0; + uint8_t proto_min = 0; + +- /* The first 4 bytes before the payload is message length */ +- SAFEALIGN_COPY_UINT32_CHECK(&len_be, +- reqbuf->v_len.kiov_base, +- reqbuf->v_len.kiov_len, +- &lc); +- msglen = be32toh(len_be); ++ msglen = kcm_input_get_payload_len(&reqbuf->v_len); + DEBUG(SSSDBG_TRACE_LIBS, + "Received message with length %"PRIu32"\n", msglen); + +-- +2.14.3 + diff --git a/0011-KCM-Do-not-use-2048-as-fixed-size-for-the-payload.patch b/0011-KCM-Do-not-use-2048-as-fixed-size-for-the-payload.patch new file mode 100644 index 0000000..964bddb --- /dev/null +++ b/0011-KCM-Do-not-use-2048-as-fixed-size-for-the-payload.patch @@ -0,0 +1,243 @@ +From 9f078d2e9ec7e1803b6c7e2f8a51e0e185723e76 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= +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 + +Reviewed-by: Jakub Hrozek +--- + 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 + diff --git a/0012-KCM-Adjust-REPLY_MAX-to-the-one-used-in-krb5.patch b/0012-KCM-Adjust-REPLY_MAX-to-the-one-used-in-krb5.patch new file mode 100644 index 0000000..7df0643 --- /dev/null +++ b/0012-KCM-Adjust-REPLY_MAX-to-the-one-used-in-krb5.patch @@ -0,0 +1,55 @@ +From d910ef0667a902b4ac0551f3e8d11121bb02214c Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= +Date: Wed, 14 Mar 2018 09:21:45 +0100 +Subject: [PATCH 12/15] KCM: Adjust REPLY_MAX to the one used in krb5 +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +krb5 has its MAX_REPLY_SIZE set as 10*1024*1024, as seen in: +https://github.com/krb5/krb5/blob/master/src/lib/krb5/ccache/cc_kcm.c#L53 + +Related: +https://pagure.io/SSSD/sssd/issue/3386 + +Signed-off-by: Fabiano Fidêncio + +Reviewed-by: Jakub Hrozek +--- + src/responder/kcm/kcmsrv_ops.c | 5 ++++- + src/util/tev_curl.c | 3 ++- + 2 files changed, 6 insertions(+), 2 deletions(-) + +diff --git a/src/responder/kcm/kcmsrv_ops.c b/src/responder/kcm/kcmsrv_ops.c +index 7a78e9d6b..1e229adc4 100644 +--- a/src/responder/kcm/kcmsrv_ops.c ++++ b/src/responder/kcm/kcmsrv_ops.c +@@ -31,7 +31,10 @@ + #include "responder/kcm/kcmsrv_ops.h" + #include "responder/kcm/kcmsrv_ccache.h" + +-#define KCM_REPLY_MAX 16384 ++/* This limit comes from: ++ * https://github.com/krb5/krb5/blob/master/src/lib/krb5/ccache/cc_kcm.c#L53 ++ */ ++#define KCM_REPLY_MAX 10*1024*1024 + + struct kcm_op_ctx { + struct kcm_resp_ctx *kcm_data; +diff --git a/src/util/tev_curl.c b/src/util/tev_curl.c +index 4c2f1ec9f..f8bede6c5 100644 +--- a/src/util/tev_curl.c ++++ b/src/util/tev_curl.c +@@ -35,7 +35,8 @@ + #include "util/tev_curl.h" + + #define TCURL_IOBUF_CHUNK 1024 +-#define TCURL_IOBUF_MAX 16384 ++/* This limit in the same one as KCM_REPLY_MAX */ ++#define TCURL_IOBUF_MAX 10*1024*1024 + + static bool global_is_curl_initialized; + +-- +2.14.3 + diff --git a/0504-KCM-temporary-increase-hardcoded-buffers.patch b/0504-KCM-temporary-increase-hardcoded-buffers.patch deleted file mode 100644 index 70e3f15..0000000 --- a/0504-KCM-temporary-increase-hardcoded-buffers.patch +++ /dev/null @@ -1,41 +0,0 @@ -From 3f2845f98ad28e57cf6a2a3ce33ff01d417c4a45 Mon Sep 17 00:00:00 2001 -From: Lukas Slebodnik -Date: Tue, 21 Nov 2017 17:48:16 +0100 -Subject: [PATCH] KCM: temporary increase hardcoded buffers - -Temporary workaround: -https://pagure.io/SSSD/sssd/issue/3386 ---- - src/responder/kcm/kcmsrv_ops.c | 2 +- - src/util/tev_curl.c | 2 +- - 2 files changed, 2 insertions(+), 2 deletions(-) - -diff --git a/src/responder/kcm/kcmsrv_ops.c b/src/responder/kcm/kcmsrv_ops.c -index 7a78e9d6b36b4aa3d31ad467216244f733f4a57b..5af567c0d19d347e28cdeada22d15807fb8bc0d5 100644 ---- a/src/responder/kcm/kcmsrv_ops.c -+++ b/src/responder/kcm/kcmsrv_ops.c -@@ -31,7 +31,7 @@ - #include "responder/kcm/kcmsrv_ops.h" - #include "responder/kcm/kcmsrv_ccache.h" - --#define KCM_REPLY_MAX 16384 -+#define KCM_REPLY_MAX 131072 - - struct kcm_op_ctx { - struct kcm_resp_ctx *kcm_data; -diff --git a/src/util/tev_curl.c b/src/util/tev_curl.c -index 4c2f1ec9ff0127ccfd72010460ed75dad43e9ce3..a51003f4118d4dc0dcb697469b861d277cd1b917 100644 ---- a/src/util/tev_curl.c -+++ b/src/util/tev_curl.c -@@ -35,7 +35,7 @@ - #include "util/tev_curl.h" - - #define TCURL_IOBUF_CHUNK 1024 --#define TCURL_IOBUF_MAX 16384 -+#define TCURL_IOBUF_MAX 131072 - - static bool global_is_curl_initialized; - --- -2.15.0 - diff --git a/sssd.spec b/sssd.spec index 283a113..a4e74d6 100644 --- a/sssd.spec +++ b/sssd.spec @@ -50,10 +50,13 @@ Patch0005: 0005-TESTS-Move-get_call_output-to-util.py.patch Patch0006: 0006-TESTS-Make-get_call_output-more-flexible-about-the-s.patch Patch0007: 0007-TESTS-Add-a-basic-test-of-sssctl-domain-list.patch Patch0008: 0008-KCM-Use-json_loadb-when-dealing-with-sss_iobuf-data.patch +Patch0009: 0009-KCM-Remove-mem_ctx-from-kcm_new_req.patch +Patch0010: 0010-KCM-Introduce-kcm_input_get_payload_len.patch +Patch0011: 0011-KCM-Do-not-use-2048-as-fixed-size-for-the-payload.patch +Patch0012: 0012-KCM-Adjust-REPLY_MAX-to-the-one-used-in-krb5.patch Patch0502: 0502-SYSTEMD-Use-capabilities.patch Patch0503: 0503-Disable-stopping-idle-socket-activated-responders.patch -Patch0504: 0504-KCM-temporary-increase-hardcoded-buffers.patch ### Dependencies ### @@ -1257,6 +1260,7 @@ fi - Resolves: upstream#3658 - Application domain is not interpreted correctly - Resolves: upstream#3687 - KCM: Don't pass a non null terminated string to json_loads() +- Resolves: upstream#3386 - KCM: Payload buffer is too small * Fri Mar 9 2018 Fabiano Fidêncio - 1.16.1-1 - New upstream release 1.16.1