diff options
author | Simo Sorce <ssorce@redhat.com> | 2009-12-18 10:23:41 -0500 |
---|---|---|
committer | Stephen Gallagher <sgallagh@redhat.com> | 2009-12-18 15:41:30 -0500 |
commit | bb78a286dbc90d248bd7a9d29344de87051920f2 (patch) | |
tree | 79dc659b368e88e7fe247fe535fc48b83862d50e /server | |
parent | 6b94e84a0455ebb68506e70cf8ccc2a4656a2c91 (diff) | |
download | sssd-bb78a286dbc90d248bd7a9d29344de87051920f2.tar.gz sssd-bb78a286dbc90d248bd7a9d29344de87051920f2.tar.xz sssd-bb78a286dbc90d248bd7a9d29344de87051920f2.zip |
Fix ldap child memory hierarchy and other issues
The timeout handler was not a child of the request so it could fire even though
the request was already freed.
The code wouldn't use async writes to the children so it could incur in a short
write with no way to detect or recover from it.
Also fixed style of some helper functions to pass explicit paramters instead of
a general structure.
Add common code to do async writes to pipes.
Fixed async write issue for the krb5_child as well.
Fix also sdap_kinit_done(), a return statement was missing and we were mixing
SDAP_AUTH and errno return codes in state->result
Remove usless helper function that just replicates talloc_strndup()
Diffstat (limited to 'server')
-rw-r--r-- | server/providers/child_common.c | 149 | ||||
-rw-r--r-- | server/providers/child_common.h | 9 | ||||
-rw-r--r-- | server/providers/krb5/krb5_auth.c | 77 | ||||
-rw-r--r-- | server/providers/krb5/krb5_child.c | 83 | ||||
-rw-r--r-- | server/providers/ldap/ldap_child.c | 64 | ||||
-rw-r--r-- | server/providers/ldap/sdap_async_connection.c | 10 | ||||
-rw-r--r-- | server/providers/ldap/sdap_async_private.h | 22 | ||||
-rw-r--r-- | server/providers/ldap/sdap_child_helpers.c | 374 |
8 files changed, 482 insertions, 306 deletions
diff --git a/server/providers/child_common.c b/server/providers/child_common.c index 15e0eefe7..154f56dd7 100644 --- a/server/providers/child_common.c +++ b/server/providers/child_common.c @@ -33,23 +33,100 @@ #include "db/sysdb.h" #include "providers/child_common.h" -uint8_t *copy_buffer_and_add_zero(TALLOC_CTX *mem_ctx, - const uint8_t *src, size_t len) +/* Async communication with the child process via a pipe */ + +struct write_pipe_state { + int fd; + uint8_t *buf; + size_t len; + size_t written; +}; + +static void write_pipe_handler(struct tevent_context *ev, + struct tevent_fd *fde, + uint16_t flags, void *pvt); + +struct tevent_req *write_pipe_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + uint8_t *buf, size_t len, int fd) { - uint8_t *str; + struct tevent_req *req; + struct write_pipe_state *state; + struct tevent_fd *fde; + + req = tevent_req_create(mem_ctx, &state, struct write_pipe_state); + if (req == NULL) return NULL; + + state->fd = fd; + state->buf = buf; + state->len = len; + state->written = 0; - str = talloc_size(mem_ctx, len + 1); - if (str == NULL) { - DEBUG(1, ("talloc_size failed.\n")); - return NULL; + fde = tevent_add_fd(ev, state, fd, TEVENT_FD_WRITE, + write_pipe_handler, req); + if (fde == NULL) { + DEBUG(1, ("tevent_add_fd failed.\n")); + goto fail; } - memcpy(str, src, len); - str[len] = '\0'; - return str; + return req; + +fail: + talloc_zfree(req); + return NULL; } -/* Async communication with the child process via a pipe */ +static void write_pipe_handler(struct tevent_context *ev, + struct tevent_fd *fde, + uint16_t flags, void *pvt) +{ + struct tevent_req *req = talloc_get_type(pvt, struct tevent_req); + struct write_pipe_state *state = tevent_req_data(req, + struct write_pipe_state); + ssize_t size; + + if (flags & TEVENT_FD_READ) { + DEBUG(1, ("write_pipe_done called with TEVENT_FD_READ," + " this should not happen.\n")); + tevent_req_error(req, EINVAL); + return; + } + + size = write(state->fd, + state->buf + state->written, + state->len - state->written); + if (size == -1) { + if (errno == EAGAIN || errno == EINTR) return; + DEBUG(1, ("write failed [%d][%s].\n", errno, strerror(errno))); + tevent_req_error(req, errno); + return; + + } else if (size >= 0) { + state->written += size; + if (state->written > state->len) { + DEBUG(1, ("write to much, this should never happen.\n")); + tevent_req_error(req, EINVAL); + return; + } + } else { + DEBUG(1, ("unexpected return value of write [%d].\n", size)); + tevent_req_error(req, EINVAL); + return; + } + + if (state->len == state->written) { + DEBUG(6, ("All data has been sent!\n")); + tevent_req_done(req); + return; + } +} + +int write_pipe_recv(struct tevent_req *req) +{ + TEVENT_REQ_RETURN_ON_ERROR(req); + + return EOK; +} struct read_pipe_state { int fd; @@ -57,9 +134,9 @@ struct read_pipe_state { size_t len; }; -static void read_pipe_done(struct tevent_context *ev, - struct tevent_fd *fde, - uint16_t flags, void *pvt); +static void read_pipe_handler(struct tevent_context *ev, + struct tevent_fd *fde, + uint16_t flags, void *pvt); struct tevent_req *read_pipe_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, int fd) @@ -77,7 +154,7 @@ struct tevent_req *read_pipe_send(TALLOC_CTX *mem_ctx, if (state->buf == NULL) goto fail; fde = tevent_add_fd(ev, state, fd, TEVENT_FD_READ, - read_pipe_done, req); + read_pipe_handler, req); if (fde == NULL) { DEBUG(1, ("tevent_add_fd failed.\n")); goto fail; @@ -90,37 +167,49 @@ fail: return NULL; } -static void read_pipe_done(struct tevent_context *ev, - struct tevent_fd *fde, - uint16_t flags, void *pvt) +static void read_pipe_handler(struct tevent_context *ev, + struct tevent_fd *fde, + uint16_t flags, void *pvt) { - ssize_t size; struct tevent_req *req = talloc_get_type(pvt, struct tevent_req); - struct read_pipe_state *state = tevent_req_data(req, struct read_pipe_state); + struct read_pipe_state *state = tevent_req_data(req, + struct read_pipe_state); + ssize_t size; + errno_t err; if (flags & TEVENT_FD_WRITE) { - DEBUG(1, ("read_pipe_done called with TEVENT_FD_WRITE, this should not happen.\n")); + DEBUG(1, ("read_pipe_done called with TEVENT_FD_WRITE," + " this should not happen.\n")); tevent_req_error(req, EINVAL); return; } - size = read(state->fd, state->buf + state->len, talloc_get_size(state->buf) - state->len); + size = read(state->fd, + state->buf + state->len, + MAX_CHILD_MSG_SIZE - state->len); if (size == -1) { - if (errno == EAGAIN || errno == EINTR) return; - DEBUG(1, ("read failed [%d][%s].\n", errno, strerror(errno))); - tevent_req_error(req, errno); + err = errno; + if (err == EAGAIN || err == EINTR) { + return; + } + + DEBUG(1, ("read failed [%d][%s].\n", err, strerror(err))); + tevent_req_error(req, err); return; + } else if (size > 0) { state->len += size; - if (state->len > talloc_get_size(state->buf)) { + if (state->len > MAX_CHILD_MSG_SIZE) { DEBUG(1, ("read to much, this should never happen.\n")); tevent_req_error(req, EINVAL); return; } - return; + } else if (size == 0) { + DEBUG(6, ("EOF received, client finished\n")); tevent_req_done(req); return; + } else { DEBUG(1, ("unexpected return value of read [%d].\n", size)); tevent_req_error(req, EINVAL); @@ -131,12 +220,12 @@ static void read_pipe_done(struct tevent_context *ev, int read_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, uint8_t **buf, ssize_t *len) { - struct read_pipe_state *state = tevent_req_data(req, - struct read_pipe_state); + struct read_pipe_state *state; + state = tevent_req_data(req, struct read_pipe_state); TEVENT_REQ_RETURN_ON_ERROR(req); - *buf = talloc_move(mem_ctx, &state->buf); + *buf = talloc_steal(mem_ctx, state->buf); *len = state->len; return EOK; diff --git a/server/providers/child_common.h b/server/providers/child_common.h index 894255b57..a441df3c5 100644 --- a/server/providers/child_common.h +++ b/server/providers/child_common.h @@ -46,15 +46,14 @@ struct io_buffer { size_t size; }; -uint8_t *copy_buffer_and_add_zero(TALLOC_CTX *mem_ctx, - const uint8_t *src, size_t len); - /* Async communication with the child process via a pipe */ -struct read_pipe_state; +struct tevent_req *write_pipe_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + uint8_t *buf, size_t len, int fd); +int write_pipe_recv(struct tevent_req *req); struct tevent_req *read_pipe_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, int fd); - int read_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, uint8_t **buf, ssize_t *len); diff --git a/server/providers/krb5/krb5_auth.c b/server/providers/krb5/krb5_auth.c index 71f4b919a..74981b19b 100644 --- a/server/providers/krb5/krb5_auth.c +++ b/server/providers/krb5/krb5_auth.c @@ -575,56 +575,86 @@ static errno_t fork_child(struct krb5child_req *kr) } struct handle_child_state { + struct tevent_context *ev; struct krb5child_req *kr; - ssize_t len; uint8_t *buf; + ssize_t len; }; +static void handle_child_step(struct tevent_req *subreq); static void handle_child_done(struct tevent_req *subreq); static struct tevent_req *handle_child_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, struct krb5child_req *kr) { - int ret; - struct tevent_req *req; - struct tevent_req *subreq; + struct tevent_req *req, *subreq; struct handle_child_state *state; struct io_buffer *buf; + int ret; + + req = tevent_req_create(mem_ctx, &state, struct handle_child_state); + if (req == NULL) { + return NULL; + } + + state->ev = ev; + state->kr = kr; + state->buf = NULL; + state->len = 0; ret = create_send_buffer(kr, &buf); if (ret != EOK) { DEBUG(1, ("create_send_buffer failed.\n")); - return NULL; + goto fail; } ret = fork_child(kr); if (ret != EOK) { DEBUG(1, ("fork_child failed.\n")); - return NULL; + goto fail; } - ret = write(kr->write_to_child_fd, buf->data, buf->size); - close(kr->write_to_child_fd); - kr->write_to_child_fd = -1; - if (ret == -1) { - DEBUG(1, ("write failed [%d][%s].\n", errno, strerror(errno))); - return NULL; + subreq = write_pipe_send(state, ev, buf->data, buf->size, + kr->write_to_child_fd); + if (!subreq) { + ret = ENOMEM; + goto fail; } + tevent_req_set_callback(subreq, handle_child_step, req); - req = tevent_req_create(mem_ctx, &state, struct handle_child_state); - if (req == NULL) { - return NULL; + return req; + +fail: + tevent_req_error(req, ret); + tevent_req_post(req, ev); + return req; +} + +static void handle_child_step(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data(subreq, + struct tevent_req); + struct handle_child_state *state = tevent_req_data(req, + struct handle_child_state); + int ret; + + ret = write_pipe_recv(subreq); + talloc_zfree(subreq); + if (ret != EOK) { + tevent_req_error(req, ret); + return; } - state->kr = kr; + close(state->kr->write_to_child_fd); + state->kr->write_to_child_fd = -1; - subreq = read_pipe_send(state, ev, kr->read_from_child_fd); - if (tevent_req_nomem(subreq, req)) { - return tevent_req_post(req, ev); + subreq = read_pipe_send(state, state->ev, state->kr->read_from_child_fd); + if (!subreq) { + tevent_req_error(req, ENOMEM); + return; } tevent_req_set_callback(subreq, handle_child_done, req); - return req; } static void handle_child_done(struct tevent_req *subreq) @@ -637,14 +667,14 @@ static void handle_child_done(struct tevent_req *subreq) ret = read_pipe_recv(subreq, state, &state->buf, &state->len); talloc_zfree(subreq); - talloc_zfree(state->kr->timeout_handler); - close(state->kr->read_from_child_fd); - state->kr->read_from_child_fd = -1; if (ret != EOK) { tevent_req_error(req, ret); return; } + close(state->kr->read_from_child_fd); + state->kr->read_from_child_fd = -1; + tevent_req_done(req); return; } @@ -961,6 +991,7 @@ static void krb5_child_done(struct tevent_req *req) int dp_err = DP_ERR_FATAL; ret = handle_child_recv(req, pd, &buf, &len); + talloc_zfree(kr->timeout_handler); talloc_zfree(req); if (ret != EOK) { DEBUG(1, ("child failed (%d [%s])\n", ret, strerror(ret))); diff --git a/server/providers/krb5/krb5_child.c b/server/providers/krb5/krb5_child.c index 6eb420bce..6b4c7cc0f 100644 --- a/server/providers/krb5/krb5_child.c +++ b/server/providers/krb5/krb5_child.c @@ -339,8 +339,9 @@ static struct response *prepare_response_message(struct krb5_req *kr, static errno_t sendresponse(int fd, krb5_error_code kerr, int pam_status, struct krb5_req *kr) { - int ret; struct response *resp; + size_t written; + int ret; resp = prepare_response_message(kr, kerr, pam_status); if (resp == NULL) { @@ -348,10 +349,18 @@ static errno_t sendresponse(int fd, krb5_error_code kerr, int pam_status, return ENOMEM; } - ret = write(fd, resp->buf, resp->size); - if (ret == -1) { - DEBUG(1, ("write failed [%d][%s].\n", errno, strerror(errno))); - return errno; + written = 0; + while (written < resp->size) { + ret = write(fd, resp->buf + written, resp->size - written); + if (ret == -1) { + if (errno == EAGAIN || errno == EINTR) { + continue; + } + ret = errno; + DEBUG(1, ("write failed [%d][%s].\n", ret, strerror(ret))); + return ret; + } + written += ret; } return EOK; @@ -680,79 +689,71 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct pam_data *pd, char **ccname, char **keytab, uint32_t *validate) { size_t p = 0; - uint32_t *len; + uint32_t len; if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); - pd->cmd = *len; + pd->cmd = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); - pd->pw_uid = *len; + pd->pw_uid = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); - pd->gr_gid = *len; + pd->gr_gid = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); - *validate = *len; + *validate = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); + len = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); - if ((p + *len ) > size) return EINVAL; - pd->upn = (char *) copy_buffer_and_add_zero(pd, buf+p, - sizeof(char) * (*len)); + if ((p + len ) > size) return EINVAL; + pd->upn = talloc_strndup(pd, (char *)(buf + p), len); if (pd->upn == NULL) return ENOMEM; - p += *len; + p += len; if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); + len = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); - if ((p + *len ) > size) return EINVAL; - *ccname = (char *) copy_buffer_and_add_zero(pd, buf+p, - sizeof(char) * (*len)); + if ((p + len ) > size) return EINVAL; + *ccname = talloc_strndup(pd, (char *)(buf + p), len); if (*ccname == NULL) return ENOMEM; - p += *len; + p += len; if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); + len = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); - if ((p + *len ) > size) return EINVAL; - *keytab = (char *) copy_buffer_and_add_zero(pd, buf+p, - sizeof(char) * (*len)); + if ((p + len ) > size) return EINVAL; + *keytab = talloc_strndup(pd, (char *)(buf + p), len); if (*keytab == NULL) return ENOMEM; - p += *len; + p += len; if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); + len = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); - if ((p + *len) > size) return EINVAL; - pd->authtok = copy_buffer_and_add_zero(pd, buf+p, sizeof(char) * (*len)); + if ((p + len) > size) return EINVAL; + pd->authtok = (uint8_t *)talloc_strndup(pd, (char *)(buf + p), len); if (pd->authtok == NULL) return ENOMEM; - pd->authtok_size = *len + 1; - p += *len; + pd->authtok_size = len + 1; + p += len; if (pd->cmd == SSS_PAM_CHAUTHTOK) { if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); + len = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); - if ((p + *len) > size) return EINVAL; - pd->newauthtok = copy_buffer_and_add_zero(pd, buf+p, - sizeof(char) * (*len)); + if ((p + len) > size) return EINVAL; + pd->newauthtok = (uint8_t *)talloc_strndup(pd, (char *)(buf + p), len); if (pd->newauthtok == NULL) return ENOMEM; - pd->newauthtok_size = *len + 1; - p += *len; + pd->newauthtok_size = len + 1; + p += len; } else { pd->newauthtok = NULL; pd->newauthtok_size = 0; @@ -938,6 +939,8 @@ int main(int argc, const char *argv[]) poptFreeContext(pc); + DEBUG(7, ("krb5_child started.\n")); + pd = talloc(NULL, struct pam_data); if (pd == NULL) { DEBUG(1, ("malloc failed.\n")); diff --git a/server/providers/ldap/ldap_child.c b/server/providers/ldap/ldap_child.c index 9c11bf401..4a577b87f 100644 --- a/server/providers/ldap/ldap_child.c +++ b/server/providers/ldap/ldap_child.c @@ -42,10 +42,11 @@ struct input_buffer { const char *keytab_name; }; -static errno_t unpack_buffer(uint8_t *buf, size_t size, struct input_buffer *ibuf) +static errno_t unpack_buffer(uint8_t *buf, size_t size, + struct input_buffer *ibuf) { size_t p = 0; - uint32_t *len; + uint32_t len; /* realm_str size and length */ DEBUG(7, ("total buffer size: %d\n", size)); @@ -53,47 +54,44 @@ static errno_t unpack_buffer(uint8_t *buf, size_t size, struct input_buffer *ibu DEBUG(1, ("Error: buffer too big!\n")); return EINVAL; } - len = ((uint32_t *)(buf+p)); + len = ((uint32_t *)(buf + p))[0]; p += sizeof(uint32_t); - DEBUG(7, ("realm_str size: %d\n", *len)); - if (*len) { - if ((p + *len ) > size) return EINVAL; - ibuf->realm_str = (char *) copy_buffer_and_add_zero(ibuf, buf+p, - sizeof(char) * (*len)); + DEBUG(7, ("realm_str size: %d\n", len)); + if (len) { + if ((p + len ) > size) return EINVAL; + ibuf->realm_str = talloc_strndup(ibuf, (char *)(buf + p), len); DEBUG(7, ("got realm_str: %s\n", ibuf->realm_str)); if (ibuf->realm_str == NULL) return ENOMEM; - p += *len; + p += len; } /* princ_str size and length */ if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); + len = ((uint32_t *)(buf + p))[0]; p += sizeof(uint32_t); - DEBUG(7, ("princ_str size: %d\n", *len)); - if (*len) { - if ((p + *len ) > size) return EINVAL; - ibuf->princ_str = (char *) copy_buffer_and_add_zero(ibuf, buf+p, - sizeof(char) * (*len)); + DEBUG(7, ("princ_str size: %d\n", len)); + if (len) { + if ((p + len ) > size) return EINVAL; + ibuf->princ_str = talloc_strndup(ibuf, (char *)(buf + p), len); DEBUG(7, ("got princ_str: %s\n", ibuf->princ_str)); if (ibuf->princ_str == NULL) return ENOMEM; - p += *len; + p += len; } /* keytab_name size and length */ if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); + len = ((uint32_t *)(buf + p))[0]; p += sizeof(uint32_t); - DEBUG(7, ("keytab_name size: %d\n", *len)); - if (*len) { - if ((p + *len ) > size) return EINVAL; - ibuf->keytab_name = (char *) copy_buffer_and_add_zero(ibuf, buf+p, - sizeof(char) * (*len)); + DEBUG(7, ("keytab_name size: %d\n", len)); + if (len) { + if ((p + len ) > size) return EINVAL; + ibuf->keytab_name = talloc_strndup(ibuf, (char *)(buf + p), len); DEBUG(7, ("got keytab_name: %s\n", ibuf->keytab_name)); if (ibuf->keytab_name == NULL) return ENOMEM; - p += *len; + p += len; } return EOK; @@ -321,6 +319,7 @@ int main(int argc, const char *argv[]) const char *ccname = NULL; struct input_buffer *ibuf = NULL; struct response *resp = NULL; + size_t written; struct poptOption long_options[] = { POPT_AUTOHELP @@ -346,6 +345,8 @@ int main(int argc, const char *argv[]) poptFreeContext(pc); + DEBUG(7, ("ldap_child started.\n")); + main_ctx = talloc_new(NULL); if (main_ctx == NULL) { DEBUG(1, ("talloc_new failed.\n")); @@ -414,11 +415,18 @@ int main(int argc, const char *argv[]) return ENOMEM; } - ret = write(STDOUT_FILENO, resp->buf, resp->size); - if (ret == -1) { - ret = errno; - DEBUG(1, ("write failed [%d][%s].\n", ret, strerror(ret))); - return errno; + written = 0; + while (written < resp->size) { + ret = write(STDOUT_FILENO, resp->buf + written, resp->size - written); + if (ret == -1) { + if (errno == EAGAIN || errno == EINTR) { + continue; + } + ret = errno; + DEBUG(1, ("write failed [%d][%s].\n", ret, strerror(ret))); + return ret; + } + written += ret; } close(STDOUT_FILENO); diff --git a/server/providers/ldap/sdap_async_connection.c b/server/providers/ldap/sdap_async_connection.c index 134979039..7ccb301bd 100644 --- a/server/providers/ldap/sdap_async_connection.c +++ b/server/providers/ldap/sdap_async_connection.c @@ -581,8 +581,7 @@ struct tevent_req *sdap_kinit_send(TALLOC_CTX *memctx, } } - subreq = sdap_krb5_get_tgt_send(state, ev, timeout, - realm, principal, keytab); + subreq = sdap_get_tgt_send(state, ev, realm, principal, keytab, timeout); if (!subreq) { talloc_zfree(req); return NULL; @@ -603,19 +602,20 @@ static void sdap_kinit_done(struct tevent_req *subreq) int result; char *ccname = NULL; - ret = sdap_krb5_get_tgt_recv(subreq, state, &result, &ccname); + ret = sdap_get_tgt_recv(subreq, state, &result, &ccname); talloc_zfree(subreq); if (ret != EOK) { - state->result = ret; + state->result = SDAP_AUTH_FAILED; DEBUG(1, ("child failed (%d [%s])\n", ret, strerror(ret))); tevent_req_error(req, ret); + return; } if (result == EOK) { ret = setenv("KRB5CCNAME", ccname, 1); if (ret == -1) { DEBUG(2, ("Unable to set env. variable KRB5CCNAME!\n")); - state->result = EFAULT; + state->result = SDAP_AUTH_FAILED; tevent_req_error(req, EFAULT); } diff --git a/server/providers/ldap/sdap_async_private.h b/server/providers/ldap/sdap_async_private.h index af8d2b8e4..9237f15d4 100644 --- a/server/providers/ldap/sdap_async_private.h +++ b/server/providers/ldap/sdap_async_private.h @@ -44,16 +44,16 @@ int sdap_get_rootdse_recv(struct tevent_req *req, /* from sdap_child_helpers.c */ -struct tevent_req *sdap_krb5_get_tgt_send(TALLOC_CTX *mem_ctx, - struct tevent_context *ev, - int timeout, - const char *realm_str, - const char *princ_str, - const char *keytab_name); - -int sdap_krb5_get_tgt_recv(struct tevent_req *req, - TALLOC_CTX *mem_ctx, - int *result, - char **ccname); +struct tevent_req *sdap_get_tgt_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + const char *realm_str, + const char *princ_str, + const char *keytab_name, + int timeout); + +int sdap_get_tgt_recv(struct tevent_req *req, + TALLOC_CTX *mem_ctx, + int *result, + char **ccname); #endif /* _SDAP_ASYNC_PRIVATE_H_ */ diff --git a/server/providers/ldap/sdap_child_helpers.c b/server/providers/ldap/sdap_child_helpers.c index 0972ec723..862dacf9e 100644 --- a/server/providers/ldap/sdap_child_helpers.c +++ b/server/providers/ldap/sdap_child_helpers.c @@ -42,76 +42,42 @@ #define LDAP_CHILD_USER "nobody" #endif -struct sdap_child_req { +struct sdap_child { /* child info */ - pid_t child_pid; + pid_t pid; int read_from_child_fd; int write_to_child_fd; - - /* for handling timeout */ - struct tevent_context *ev; - int timeout; - struct tevent_timer *timeout_handler; - struct tevent_req *req; - - /* parameters */ - const char *realm_str; - const char *princ_str; - const char *keytab_name; }; -static int sdap_child_req_destructor(void *ptr) -{ - struct sdap_child_req *cr = talloc_get_type(ptr, struct sdap_child_req); - - if (cr == NULL) return EOK; - - child_cleanup(cr->read_from_child_fd, cr->write_to_child_fd); - memset(cr, 0, sizeof(struct sdap_child_req)); - - return EOK; -} - -static void sdap_child_timeout(struct tevent_context *ev, - struct tevent_timer *te, - struct timeval tv, void *pvt) +static void sdap_close_fd(int *fd) { - struct sdap_child_req *lr = talloc_get_type(pvt, struct sdap_child_req); int ret; - if (lr->timeout_handler == NULL) { + if (*fd == -1) { + DEBUG(6, ("fd already closed\n")); return; } - DEBUG(9, ("timeout for ldap child [%d] reached.\n", lr->child_pid)); - - ret = kill(lr->child_pid, SIGKILL); - if (ret == -1) { - DEBUG(1, ("kill failed [%d][%s].\n", errno, strerror(errno))); + ret = close(*fd); + if (ret) { + ret = errno; + DEBUG(2, ("Closing fd %d, return error %d (%s)\n", + *fd, ret, strerror(ret))); } - tevent_req_error(lr->req, EIO); + *fd = -1; } -static errno_t activate_child_timeout_handler(struct sdap_child_req *child_req) +static int sdap_child_destructor(void *ptr) { - struct timeval tv; + struct sdap_child *child = talloc_get_type(ptr, struct sdap_child); - tv = tevent_timeval_current(); - tv = tevent_timeval_add(&tv, - child_req->timeout, - 0); - child_req->timeout_handler = tevent_add_timer(child_req->ev, child_req, tv, - sdap_child_timeout, child_req); - if (child_req->timeout_handler == NULL) { - DEBUG(1, ("tevent_add_timer failed.\n")); - return ENOMEM; - } + child_cleanup(child->read_from_child_fd, child->write_to_child_fd); - return EOK; + return 0; } -static errno_t fork_ldap_child(struct sdap_child_req *child_req) +static errno_t sdap_fork_child(struct sdap_child *child) { int pipefd_to_child[2]; int pipefd_from_child[2]; @@ -135,7 +101,7 @@ static errno_t fork_ldap_child(struct sdap_child_req *child_req) pid = fork(); if (pid == 0) { /* child */ - err = exec_child(child_req, + err = exec_child(child, pipefd_to_child, pipefd_from_child, LDAP_CHILD, ldap_child_debug_fd); if (err != EOK) { @@ -144,13 +110,13 @@ static errno_t fork_ldap_child(struct sdap_child_req *child_req) return err; } } else if (pid > 0) { /* parent */ - child_req->child_pid = pid; - child_req->read_from_child_fd = pipefd_from_child[0]; + child->pid = pid; + child->read_from_child_fd = pipefd_from_child[0]; close(pipefd_from_child[1]); - child_req->write_to_child_fd = pipefd_to_child[1]; + child->write_to_child_fd = pipefd_to_child[1]; close(pipefd_to_child[0]); - fd_nonblocking(child_req->read_from_child_fd); - fd_nonblocking(child_req->write_to_child_fd); + fd_nonblocking(child->read_from_child_fd); + fd_nonblocking(child->write_to_child_fd); } else { /* error */ err = errno; @@ -161,27 +127,36 @@ static errno_t fork_ldap_child(struct sdap_child_req *child_req) return EOK; } -static errno_t create_ldap_send_buffer(struct sdap_child_req *child_req, struct io_buffer **io_buf) +static errno_t create_tgt_req_send_buffer(TALLOC_CTX *mem_ctx, + const char *realm_str, + const char *princ_str, + const char *keytab_name, + struct io_buffer **io_buf) { struct io_buffer *buf; size_t rp; + int len; - buf = talloc(child_req, struct io_buffer); + buf = talloc(mem_ctx, struct io_buffer); if (buf == NULL) { DEBUG(1, ("talloc failed.\n")); return ENOMEM; } - buf->size = 3*sizeof(int); - if (child_req->realm_str) - buf->size += strlen(child_req->realm_str); - if (child_req->princ_str) - buf->size += strlen(child_req->princ_str); - if (child_req->keytab_name) - buf->size += strlen(child_req->keytab_name); + buf->size = 3 * sizeof(uint32_t); + if (realm_str) { + buf->size += strlen(realm_str); + } + if (princ_str) { + buf->size += strlen(princ_str); + } + if (keytab_name) { + buf->size += strlen(keytab_name); + } + DEBUG(7, ("buffer size: %d\n", buf->size)); - buf->data = talloc_size(child_req, buf->size); + buf->data = talloc_size(buf, buf->size); if (buf->data == NULL) { DEBUG(1, ("talloc_size failed.\n")); talloc_free(buf); @@ -189,34 +164,41 @@ static errno_t create_ldap_send_buffer(struct sdap_child_req *child_req, struct } rp = 0; + /* realm */ - ((uint32_t *)(&buf->data[rp]))[0] = - (uint32_t) (child_req->realm_str ? - strlen(child_req->realm_str) : 0); - rp += sizeof(uint32_t); - if (child_req->realm_str) { - memcpy(&buf->data[rp], child_req->realm_str, strlen(child_req->realm_str)); - rp += strlen(child_req->realm_str); + if (realm_str) { + len = strlen(realm_str); + ((uint32_t *)(&buf->data[rp]))[0] = len; + rp += sizeof(uint32_t); + memcpy(&buf->data[rp], realm_str, len); + rp += len; + } else { + ((uint32_t *)(&buf->data[rp]))[0] = 0; + rp += sizeof(uint32_t); } /* principal */ - ((uint32_t *)(&buf->data[rp]))[0] = - (uint32_t) (child_req->princ_str ? - strlen(child_req->princ_str) : 0); - rp += sizeof(uint32_t); - if (child_req->princ_str) { - memcpy(&buf->data[rp], child_req->princ_str, strlen(child_req->princ_str)); - rp += strlen(child_req->princ_str); + if (princ_str) { + len = strlen(princ_str); + ((uint32_t *)(&buf->data[rp]))[0] = len; + rp += sizeof(uint32_t); + memcpy(&buf->data[rp], princ_str, len); + rp += len; + } else { + ((uint32_t *)(&buf->data[rp]))[0] = 0; + rp += sizeof(uint32_t); } /* keytab */ - ((uint32_t *)(&buf->data[rp]))[0] = - (uint32_t) (child_req->keytab_name ? - strlen(child_req->keytab_name) : 0); - rp += sizeof(uint32_t); - if (child_req->keytab_name) { - memcpy(&buf->data[rp], child_req->keytab_name, strlen(child_req->keytab_name)); - rp += strlen(child_req->keytab_name); + if (keytab_name) { + len = strlen(keytab_name); + ((uint32_t *)(&buf->data[rp]))[0] = len; + rp += sizeof(uint32_t); + memcpy(&buf->data[rp], keytab_name, len); + rp += len; + } else { + ((uint32_t *)(&buf->data[rp]))[0] = 0; + rp += sizeof(uint32_t); } *io_buf = buf; @@ -225,151 +207,172 @@ static errno_t create_ldap_send_buffer(struct sdap_child_req *child_req, struct static int parse_child_response(TALLOC_CTX *mem_ctx, uint8_t *buf, ssize_t size, - int *result, - char **ccache) + int *result, char **ccache) { size_t p = 0; - uint32_t *len; - uint32_t *res; + uint32_t len; + uint32_t res; char *ccn; - /* ccache size the ccache itself*/ + /* operatoin result code */ if ((p + sizeof(uint32_t)) > size) return EINVAL; - res = ((uint32_t *)(buf+p)); + res = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); - /* ccache size the ccache itself*/ + /* ccache name size */ if ((p + sizeof(uint32_t)) > size) return EINVAL; - len = ((uint32_t *)(buf+p)); + len = *((uint32_t *)(buf + p)); p += sizeof(uint32_t); - if ((p + *len ) > size) return EINVAL; + if ((p + len ) > size) return EINVAL; - ccn = talloc_size(mem_ctx, sizeof(char) * (*len + 1)); + ccn = talloc_size(mem_ctx, sizeof(char) * (len + 1)); if (ccn == NULL) { DEBUG(1, ("talloc_size failed.\n")); return ENOMEM; } - memcpy(ccn, buf+p, sizeof(char) * (*len + 1)); - ccn[*len] = '\0'; + memcpy(ccn, buf+p, sizeof(char) * (len + 1)); + ccn[len] = '\0'; - *result = *res; + *result = res; *ccache = ccn; return EOK; } /* ==The-public-async-interface============================================*/ -struct sdap_krb5_get_tgt_state { - struct sdap_child_req *lr; +struct sdap_get_tgt_state { + struct tevent_context *ev; + struct sdap_child *child; ssize_t len; uint8_t *buf; }; -static void sdap_krb5_get_tgt_done(struct tevent_req *subreq); - -struct tevent_req *sdap_krb5_get_tgt_send(TALLOC_CTX *mem_ctx, - struct tevent_context *ev, - int timeout, - const char *realm_str, - const char *princ_str, - const char *keytab_name) +static errno_t set_tgt_child_timeout(struct tevent_req *req, + struct tevent_context *ev, + int timeout); +static void sdap_get_tgt_step(struct tevent_req *subreq); +static void sdap_get_tgt_done(struct tevent_req *subreq); + +struct tevent_req *sdap_get_tgt_send(TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + const char *realm_str, + const char *princ_str, + const char *keytab_name, + int timeout) { - struct sdap_child_req *child_req = NULL; - struct sdap_krb5_get_tgt_state *state = NULL; + struct tevent_req *req, *subreq; + struct sdap_get_tgt_state *state; + struct io_buffer *buf; int ret; - struct io_buffer *buf = NULL; - struct tevent_req *req = NULL; - struct tevent_req *subreq = NULL; - /* prepare the data to pass to child */ - child_req = talloc_zero(mem_ctx, struct sdap_child_req); - if (!child_req) goto fail; - - child_req->ev = ev; - child_req->read_from_child_fd = -1; - child_req->write_to_child_fd = -1; - child_req->realm_str = realm_str; - child_req->princ_str = princ_str; - child_req->keytab_name = keytab_name; - child_req->timeout = timeout; - talloc_set_destructor((TALLOC_CTX *) child_req, sdap_child_req_destructor); - - ret = create_ldap_send_buffer(child_req, &buf); - if (ret != EOK) { - DEBUG(1, ("create_ldap_send_buffer failed.\n")); + req = tevent_req_create(mem_ctx, &state, struct sdap_get_tgt_state); + if (!req) { return NULL; } - ret = fork_ldap_child(child_req); - if (ret != EOK) { - DEBUG(1, ("fork_ldap_child failed.\n")); + state->ev = ev; + + state->child = talloc_zero(state, struct sdap_child); + if (!state->child) { + ret = ENOMEM; goto fail; } - ret = write(child_req->write_to_child_fd, buf->data, buf->size); - close(child_req->write_to_child_fd); - child_req->write_to_child_fd = -1; - if (ret == -1) { - ret = errno; - DEBUG(1, ("write failed [%d][%s].\n", ret, strerror(ret))); - return NULL; - } + state->child->read_from_child_fd = -1; + state->child->write_to_child_fd = -1; + talloc_set_destructor((TALLOC_CTX *)state->child, sdap_child_destructor); - req = tevent_req_create(mem_ctx, &state, struct sdap_krb5_get_tgt_state); - if (req == NULL) { - return NULL; + /* prepare the data to pass to child */ + ret = create_tgt_req_send_buffer(state, + realm_str, princ_str, keytab_name, + &buf); + if (ret != EOK) { + DEBUG(1, ("create_tgt_req_send_buffer failed.\n")); + goto fail; } - state->lr = child_req; + ret = sdap_fork_child(state->child); + if (ret != EOK) { + DEBUG(1, ("sdap_fork_child failed.\n")); + goto fail; + } - child_req->req = req; - ret = activate_child_timeout_handler(child_req); + ret = set_tgt_child_timeout(req, ev, timeout); if (ret != EOK) { DEBUG(1, ("activate_child_timeout_handler failed.\n")); - return NULL; + goto fail; } - subreq = read_pipe_send(state, ev, child_req->read_from_child_fd); - if (tevent_req_nomem(subreq, req)) { - return tevent_req_post(req, ev); + subreq = write_pipe_send(state, ev, buf->data, buf->size, + state->child->write_to_child_fd); + if (!subreq) { + ret = ENOMEM; + goto fail; } - tevent_req_set_callback(subreq, sdap_krb5_get_tgt_done, req); + tevent_req_set_callback(subreq, sdap_get_tgt_step, req); return req; + fail: - return NULL; + tevent_req_error(req, ret); + tevent_req_post(req, ev); + return req; +} + +static void sdap_get_tgt_step(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data(subreq, + struct tevent_req); + struct sdap_get_tgt_state *state = tevent_req_data(req, + struct sdap_get_tgt_state); + int ret; + + ret = write_pipe_recv(subreq); + talloc_zfree(subreq); + if (ret != EOK) { + tevent_req_error(req, ret); + return; + } + + sdap_close_fd(&state->child->write_to_child_fd); + + subreq = read_pipe_send(state, state->ev, + state->child->read_from_child_fd); + if (!subreq) { + tevent_req_error(req, ENOMEM); + return; + } + tevent_req_set_callback(subreq, sdap_get_tgt_done, req); } -static void sdap_krb5_get_tgt_done(struct tevent_req *subreq) +static void sdap_get_tgt_done(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req); - struct sdap_krb5_get_tgt_state *state = tevent_req_data(req, - struct sdap_krb5_get_tgt_state); + struct sdap_get_tgt_state *state = tevent_req_data(req, + struct sdap_get_tgt_state); int ret; ret = read_pipe_recv(subreq, state, &state->buf, &state->len); talloc_zfree(subreq); - talloc_zfree(state->lr->timeout_handler); - close(state->lr->read_from_child_fd); - state->lr->read_from_child_fd = -1; if (ret != EOK) { tevent_req_error(req, ret); return; } + sdap_close_fd(&state->child->read_from_child_fd); + tevent_req_done(req); - return; } -int sdap_krb5_get_tgt_recv(struct tevent_req *req, +int sdap_get_tgt_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, int *result, char **ccname) { - struct sdap_krb5_get_tgt_state *state = tevent_req_data(req, - struct sdap_krb5_get_tgt_state); + struct sdap_get_tgt_state *state = tevent_req_data(req, + struct sdap_get_tgt_state); char *ccn; int res; int ret; @@ -388,6 +391,49 @@ int sdap_krb5_get_tgt_recv(struct tevent_req *req, return EOK; } + + +static void get_tgt_timeout_handler(struct tevent_context *ev, + struct tevent_timer *te, + struct timeval tv, void *pvt) +{ + struct tevent_req *req = talloc_get_type(pvt, struct tevent_req); + struct sdap_get_tgt_state *state = tevent_req_data(req, + struct sdap_get_tgt_state); + int ret; + + DEBUG(9, ("timeout for tgt child [%d] reached.\n", state->child->pid)); + + ret = kill(state->child->pid, SIGKILL); + if (ret == -1) { + DEBUG(1, ("kill failed [%d][%s].\n", errno, strerror(errno))); + } + + tevent_req_error(req, ETIMEDOUT); +} + +static errno_t set_tgt_child_timeout(struct tevent_req *req, + struct tevent_context *ev, + int timeout) +{ + struct tevent_timer *te; + struct timeval tv; + + DEBUG(6, ("Setting %d seconds timeout for tgt child\n", timeout)); + + tv = tevent_timeval_current_ofs(timeout, 0); + + te = tevent_add_timer(ev, req, tv, get_tgt_timeout_handler, req); + if (te == NULL) { + DEBUG(1, ("tevent_add_timer failed.\n")); + return ENOMEM; + } + + return EOK; +} + + + /* Setup child logging */ int setup_child(struct sdap_id_ctx *ctx) { |