Commit 786c400 KCM: Do not use 2048 as fixed size for the payload

1 file Authored by fidencio 2 months ago , Committed by jhrozek 2 months ago ,
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 <fidencio@redhat.com>

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

    
  1 @@ -38,7 +38,7 @@
  2   /* The maximum length of a request or reply as defined by the RPC
  3    * protocol. This is the same constant size as MIT KRB5 uses
  4    */
  5 - #define KCM_PACKET_MAX_SIZE 2048
  6 + #define KCM_PACKET_MAX_SIZE 10*1024*1024
  7   
  8   /* KCM operation, its raw input and raw output and result */
  9   struct kcm_op_io {
 10 @@ -125,7 +125,6 @@
 11       struct kcm_iovec v_len;
 12   
 13       /* Includes the major, minor versions etc */
 14 -     uint8_t msgbuf[KCM_PACKET_MAX_SIZE];
 15       struct kcm_iovec v_msg;
 16   };
 17   
 18 @@ -238,7 +237,6 @@
 19       uint8_t rcbuf[KCM_RETCODE_SIZE];
 20       struct kcm_iovec v_rc;
 21   
 22 -     uint8_t msgbuf[KCM_PACKET_MAX_SIZE];
 23       struct kcm_iovec v_msg;
 24   };
 25   
 26 @@ -259,11 +257,13 @@
 27   /* retcode is 0 if the operation at least ran, non-zero if there
 28    * was some kind of internal KCM error, like input couldn't be parsed
 29    */
 30 - static errno_t kcm_output_construct(struct kcm_op_io *op_io,
 31 + static errno_t kcm_output_construct(TALLOC_CTX *mem_ctx,
 32 +                                     struct kcm_op_io *op_io,
 33                                       struct kcm_repbuf *repbuf)
 34   {
 35 -     size_t c;
 36 +     uint8_t *rep;
 37       size_t replen;
 38 +     size_t c;
 39   
 40       replen = sss_iobuf_get_len(op_io->reply);
 41       if (replen > KCM_PACKET_MAX_SIZE) {
 42 @@ -281,14 +281,22 @@
 43       SAFEALIGN_SETMEM_UINT32(repbuf->rcbuf, 0, &c);
 44   
 45       if (replen > 0) {
 46 +         rep = talloc_zero_array(mem_ctx, uint8_t, replen);
 47 +         if (rep == NULL) {
 48 +             DEBUG(SSSDBG_CRIT_FAILURE,
 49 +                   "Failed to allocate memory for the message\n");
 50 +             return ENOMEM;
 51 +         }
 52 + 
 53           c = 0;
 54 -         SAFEALIGN_MEMCPY_CHECK(repbuf->msgbuf,
 55 +         SAFEALIGN_MEMCPY_CHECK(rep,
 56                                  sss_iobuf_get_data(op_io->reply),
 57                                  replen,
 58 -                                repbuf->v_msg.kiov_len,
 59 +                                replen,
 60                                  &c);
 61   
 62 -         /* Length of the buffer to send to KCM client */
 63 +         /* Set the buffer and its length to send to KCM client */
 64 +         repbuf->v_msg.kiov_base = rep;
 65           repbuf->v_msg.kiov_len = replen;
 66       }
 67   
 68 @@ -321,24 +329,6 @@
 69       TEVENT_FD_WRITEABLE(cctx->cfde);
 70   }
 71   
 72 - static void kcm_send_reply(struct cli_ctx *cctx,
 73 -                            struct kcm_op_io *op_io,
 74 -                            struct kcm_repbuf *repbuf)
 75 - {
 76 -     errno_t ret;
 77 - 
 78 -     DEBUG(SSSDBG_TRACE_INTERNAL, "Sending a reply\n");
 79 -     ret = kcm_output_construct(op_io, repbuf);
 80 -     if (ret != EOK) {
 81 -         DEBUG(SSSDBG_CRIT_FAILURE,
 82 -               "Cannot construct the reply buffer, terminating client\n");
 83 -         kcm_reply_error(cctx, ret, repbuf);
 84 -         return;
 85 -     }
 86 - 
 87 -     TEVENT_FD_WRITEABLE(cctx->cfde);
 88 - }
 89 - 
 90   /**
 91    * Request-reply dispatcher
 92    */
 93 @@ -356,6 +346,26 @@
 94       struct kcm_op_io op_io;
 95   };
 96   
 97 + static void kcm_send_reply(struct kcm_req_ctx *req_ctx)
 98 + {
 99 +     struct cli_ctx *cctx;
100 +     errno_t ret;
101 + 
102 +     DEBUG(SSSDBG_TRACE_INTERNAL, "Sending a reply\n");
103 + 
104 +     cctx = req_ctx->cctx;
105 + 
106 +     ret = kcm_output_construct(cctx, &req_ctx->op_io, &req_ctx->repbuf);
107 +     if (ret != EOK) {
108 +         DEBUG(SSSDBG_CRIT_FAILURE,
109 +               "Cannot construct the reply buffer, terminating client\n");
110 +         kcm_reply_error(cctx, ret, &req_ctx->repbuf);
111 +         return;
112 +     }
113 + 
114 +     TEVENT_FD_WRITEABLE(cctx->cfde);
115 + }
116 + 
117   static void kcm_cmd_request_done(struct tevent_req *req);
118   
119   static errno_t kcm_cmd_dispatch(struct kcm_ctx *kctx,
120 @@ -385,11 +395,9 @@
121   static void kcm_cmd_request_done(struct tevent_req *req)
122   {
123       struct kcm_req_ctx *req_ctx;
124 -     struct cli_ctx *cctx;
125       errno_t ret;
126   
127       req_ctx = tevent_req_callback_data(req, struct kcm_req_ctx);
128 -     cctx = req_ctx->cctx;
129   
130       ret = kcm_cmd_recv(req_ctx, req,
131                          &req_ctx->op_io.reply);
132 @@ -397,15 +405,19 @@
133       if (ret != EOK) {
134           DEBUG(SSSDBG_OP_FAILURE,
135                 "KCM operation failed [%d]: %s\n", ret, sss_strerror(ret));
136 -         kcm_reply_error(cctx, ret, &req_ctx->repbuf);
137 +         kcm_reply_error(req_ctx->cctx, ret, &req_ctx->repbuf);
138           return;
139       }
140   
141 -     kcm_send_reply(cctx, &req_ctx->op_io, &req_ctx->repbuf);
142 +     kcm_send_reply(req_ctx);
143   }
144   
145 - static errno_t kcm_recv_data(int fd, struct kcm_reqbuf *reqbuf)
146 + static errno_t kcm_recv_data(TALLOC_CTX *mem_ctx,
147 +                              int fd,
148 +                              struct kcm_reqbuf *reqbuf)
149   {
150 +     uint8_t *msg;
151 +     uint32_t msglen;
152       errno_t ret;
153   
154       ret = kcm_read_iovec(fd, &reqbuf->v_len);
155 @@ -416,6 +428,24 @@
156           return ret;
157       }
158   
159 +     msglen = kcm_input_get_payload_len(&reqbuf->v_len);
160 +     if (msglen > KCM_PACKET_MAX_SIZE) {
161 +         DEBUG(SSSDBG_CRIT_FAILURE,
162 +               "Request exceeds the KCM protocol limit, aborting\n");
163 +         return E2BIG;
164 +     }
165 + 
166 +     msg = talloc_zero_array(mem_ctx, uint8_t, msglen);
167 +     if (msg == NULL) {
168 +         DEBUG(SSSDBG_CRIT_FAILURE,
169 +               "Failed to allocate memory for the message\n");
170 +         return ENOMEM;
171 +     }
172 + 
173 +     /* Set the buffer and its expected len to receive the data */
174 +     reqbuf->v_msg.kiov_base = msg;
175 +     reqbuf->v_msg.kiov_len = msglen;
176 + 
177       ret = kcm_read_iovec(fd, &reqbuf->v_msg);
178       if (ret != EOK) {
179           /* Not all errors are fatal, hence we don't print DEBUG messages
180 @@ -443,21 +473,12 @@
181       req->reqbuf.v_len.kiov_base = req->reqbuf.lenbuf;
182       req->reqbuf.v_len.kiov_len = KCM_MSG_LEN_SIZE;
183   
184 -     req->reqbuf.v_msg.kiov_base = req->reqbuf.msgbuf;
185 -     req->reqbuf.v_msg.kiov_len = KCM_PACKET_MAX_SIZE;
186 - 
187       req->repbuf.v_len.kiov_base = req->repbuf.lenbuf;
188       req->repbuf.v_len.kiov_len = KCM_MSG_LEN_SIZE;
189   
190       req->repbuf.v_rc.kiov_base = req->repbuf.rcbuf;
191       req->repbuf.v_rc.kiov_len = KCM_RETCODE_SIZE;
192   
193 -     req->repbuf.v_msg.kiov_base = req->repbuf.msgbuf;
194 -     /* Length of the msg iobuf will be adjusted later, so far use the full
195 -      * length so that constructing the reply can use that capacity
196 -      */
197 -     req->repbuf.v_msg.kiov_len = KCM_PACKET_MAX_SIZE;
198 - 
199       req->cctx = cctx;
200       req->kctx = kctx;
201   
202 @@ -485,7 +506,7 @@
203           cctx->state_ctx = req;
204       }
205   
206 -     ret = kcm_recv_data(cctx->cfd, &req->reqbuf);
207 +     ret = kcm_recv_data(req, cctx->cfd, &req->reqbuf);
208       switch (ret) {
209       case ENODATA:
210           DEBUG(SSSDBG_TRACE_ALL, "Client closed connection.\n");