Passkey: Add child timeout handler

If passkey auth times out, the SIGCHLD handler needs to be destroyed otherwise the SIGCHLD handler tries to access the tevent_req which was already freed from the timeout. Resolves: #6889 Reviewed-by: Iker Pedrosa <[email protected]> Reviewed-by: Sumit Bose <[email protected]>
SSSD · Aug 31, 2023 · b516f1e · b516f1e
1 parent eebb43d
commit b516f1e
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 4 deletions.
diff --git a/src/responder/pam/pamsrv_passkey.c b/src/responder/pam/pamsrv_passkey.c
@@ -701,6 +701,7 @@ void pam_passkey_get_user_done(struct tevent_req *req)
 struct pam_passkey_auth_send_state {
     struct pam_data *pd;
     struct tevent_context *ev;
+    struct tevent_timer *timeout_handler;
     struct sss_child_ctx_old *child_ctx;
     struct child_io_fds *io;
     const char *logfile;
@@ -993,6 +994,24 @@ pam_passkey_auth_send(TALLOC_CTX *mem_ctx,
     return req;
 }
 
+static void
+passkey_child_timeout(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 pam_passkey_auth_send_state *state =
+            tevent_req_data(req, struct pam_passkey_auth_send_state);
+
+    DEBUG(SSSDBG_CRIT_FAILURE, "Timeout reached for passkey child, "
+                               "consider increasing passkey_child_timeout\n");
+    child_handler_destroy(state->child_ctx);
+    state->child_ctx = NULL;
+    state->child_status = ETIMEDOUT;
+    tevent_req_error(req, ERR_PASSKEY_CHILD_TIMEOUT);
+}
+
 static errno_t passkey_child_exec(struct tevent_req *req)
 {
     struct pam_passkey_auth_send_state *state;
@@ -1003,7 +1022,6 @@ static errno_t passkey_child_exec(struct tevent_req *req)
     uint8_t *write_buf = NULL;
     size_t write_buf_len = 0;
     struct timeval tv;
-    bool endtime;
     int ret;
 
     state = tevent_req_data(req, struct pam_passkey_auth_send_state);
@@ -1043,7 +1061,7 @@ static errno_t passkey_child_exec(struct tevent_req *req)
 
         /* Set up SIGCHLD handler */
         if (state->kerberos_pa) {
-            ret = child_handler_setup(state->ev, child_pid, NULL, NULL, NULL);
+            ret = child_handler_setup(state->ev, child_pid, NULL, req, &state->child_ctx);
         } else {
             ret = child_handler_setup(state->ev, child_pid,
                                       pam_passkey_auth_done, req,
@@ -1059,8 +1077,9 @@ static errno_t passkey_child_exec(struct tevent_req *req)
 
         /* Set up timeout handler */
         tv = tevent_timeval_current_ofs(state->timeout, 0);
-        endtime = tevent_req_set_endtime(req, state->ev, tv);
-        if (endtime == false) {
+        state->timeout_handler = tevent_add_timer(state->ev, req, tv,
+                                                  passkey_child_timeout, req);
+        if (state->timeout_handler == NULL) {
             ret = ERR_PASSKEY_CHILD;
             goto done;
         }

diff --git a/src/util/util_errors.c b/src/util/util_errors.c
@@ -93,6 +93,7 @@ struct err_string error_to_str[] = {
     { "p11_child timeout" }, /* ERR_P11_CHILD_TIMEOUT */
     { "PIN locked" }, /* ERR_P11_PIN_LOCKED */
     { "passkey_child failed" }, /* ERR_PASSKEY_CHILD */
+    { "passkey_child timeout" }, /* ERR_PASSKEY_CHILD_TIMEOUT */
     { "Address family not supported" }, /* ERR_ADDR_FAMILY_NOT_SUPPORTED */
     { "Message sender is the bus" }, /* ERR_SBUS_SENDER_BUS */
     { "Subdomain is inactive" }, /* ERR_SUBDOM_INACTIVE */

diff --git a/src/util/util_errors.h b/src/util/util_errors.h
@@ -114,6 +114,7 @@ enum sssd_errors {
     ERR_P11_CHILD_TIMEOUT,
     ERR_P11_PIN_LOCKED,
     ERR_PASSKEY_CHILD,
+    ERR_PASSKEY_CHILD_TIMEOUT,
     ERR_ADDR_FAMILY_NOT_SUPPORTED,
     ERR_SBUS_SENDER_BUS,
     ERR_SUBDOM_INACTIVE,