]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
auth: sasl-server-request - Track and enforce valid SASL interaction state
authorStephan Bosch <stephan.bosch@open-xchange.com>
Mon, 23 Oct 2023 19:04:15 +0000 (21:04 +0200)
committertimo.sirainen <timo.sirainen@open-xchange.com>
Thu, 9 Oct 2025 08:41:22 +0000 (08:41 +0000)
src/auth/auth-request.c
src/auth/auth-request.h
src/auth/auth-sasl.c
src/auth/sasl-server-private.h
src/auth/sasl-server-request.c
src/auth/sasl-server.h

index 037a98dd30ae4176f9c7b6edc316bf57af35a8dd..7af85e935c534d5589cbdd2123b3f16798d044a8 100644 (file)
@@ -298,8 +298,6 @@ static void auth_request_success_continue(struct auth_policy_check_ctx *ctx)
        if (ctx->success_data->used > 0 && !request->fields.final_resp_ok) {
                /* we'll need one more SASL round, since client doesn't support
                   the final SASL response */
-               i_assert(!request->final_resp_sent);
-               request->final_resp_sent = TRUE;
                auth_request_handler_reply_continue(request,
                        ctx->success_data->data, ctx->success_data->used);
                return;
@@ -324,8 +322,6 @@ void auth_request_fail_with_reply(struct auth_request *request,
        if (final_data_size > 0 && !request->fields.final_resp_ok) {
                /* Otherwise, we need to send the data as part of a normal
                   challenge and wait for a dummy client response. */
-               i_assert(!request->final_resp_sent);
-               request->final_resp_sent = TRUE;
                auth_request_handler_reply_continue(request, final_data,
                                                    final_data_size);
                return;
@@ -418,15 +414,6 @@ void auth_request_continue(struct auth_request *request,
 {
        i_assert(request->state == AUTH_REQUEST_STATE_MECH_CONTINUE);
 
-       if (request->final_resp_sent) {
-               if (!request->fields.successful) {
-                       auth_request_fail(request);
-                       return;
-               }
-               auth_request_success(request, "", 0);
-               return;
-       }
-
        auth_request_refresh_last_access(request);
        auth_sasl_request_continue(request, data, data_size);
 }
index c30a6e36cd5c4b1a61e6f3c7eea54f0c32c50270..308d4d934c20da72175d2842cb38c24020708eeb 100644 (file)
@@ -241,9 +241,6 @@ struct auth_request {
           needs to be tracked outside regular extra fields, because they get
           rolled back on passdb failure. */
        bool failure_nodelay:1;
-       /* Sent final response (challenge). Waiting for dummy client response.
-        */
-       bool final_resp_sent:1;
 
        bool event_finished_sent:1;
 
index e6fa3534ed5ed8f7bc5702ff91d1c211aebf4576..c985041e3ec825a270533519e81caa83805e5268 100644 (file)
@@ -145,7 +145,8 @@ auth_sasl_request_output(struct sasl_server_req_ctx *rctx,
                auth_request_internal_failure(request);
                break;
        case SASL_SERVER_OUTPUT_FAILURE:
-               auth_request_fail(request);
+               auth_request_fail_with_reply(
+                       request, output->data, output->data_size);
                break;
        case SASL_SERVER_OUTPUT_CONTINUE:
                auth_request_handler_reply_continue(request, output->data,
index d48b8294db7c2d648f8507b531279236b102a5c4..7b908dd48199f91f6f5f15ca9bdd5b25f9e52bac 100644 (file)
@@ -19,8 +19,14 @@ struct sasl_server_request {
        struct sasl_server_mech_request *mech;
        struct event *event;
 
+       enum sasl_server_request_state state;
+       unsigned int sequence;
+
        enum sasl_server_passdb_type passdb_type;
        sasl_server_mech_passdb_callback_t *passdb_callback;
+
+       bool failed:1;
+       bool finished_with_data:1;
 };
 
 struct sasl_server_mech_reg {
index 5a5deefb6b611677cbd3e866f198f7e446570804..f8d0b48d81270736c97197df34115441ea254465 100644 (file)
@@ -158,6 +158,9 @@ void sasl_server_request_initial(struct sasl_server_req_ctx *rctx,
        struct sasl_server_mech_request *mreq = req->mech;
        const struct sasl_server_mech *mech = mreq->mech;
 
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_NEW);
+       req->state = SASL_SERVER_REQUEST_STATE_SERVER;
+
        if (sasl_server_request_fail_on_nuls(req, data, data_size))
                return;
 
@@ -174,6 +177,19 @@ void sasl_server_request_input(struct sasl_server_req_ctx *rctx,
        struct sasl_server_mech_request *mreq = req->mech;
        const struct sasl_server_mech *mech = mreq->mech;
 
+       if (req->state == SASL_SERVER_REQUEST_STATE_FINISHED &&
+           req->finished_with_data) {
+               req->state = SASL_SERVER_REQUEST_STATE_SERVER;
+               if (!req->failed)
+                       sasl_server_request_success(mreq, "", 0);
+               else
+                       sasl_server_request_failure(mreq);
+               return;
+       }
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_CLIENT);
+       i_assert(!req->finished_with_data);
+       req->state = SASL_SERVER_REQUEST_STATE_SERVER;
+
        if (sasl_server_request_fail_on_nuls(req, data, data_size))
                return;
 
@@ -207,7 +223,11 @@ bool sasl_server_request_set_authid(struct sasl_server_mech_request *mreq,
 
        i_assert(req->rctx != NULL);
        i_assert(funcs->request_set_authid != NULL);
-       return funcs->request_set_authid(req->rctx, authid_type, authid);
+       if (!funcs->request_set_authid(req->rctx, authid_type, authid)) {
+               req->failed = TRUE;
+               return FALSE;
+       }
+       return TRUE;
 }
 
 bool sasl_server_request_set_authzid(struct sasl_server_mech_request *mreq,
@@ -219,7 +239,11 @@ bool sasl_server_request_set_authzid(struct sasl_server_mech_request *mreq,
 
        i_assert(req->rctx != NULL);
        i_assert(funcs->request_set_authzid != NULL);
-       return funcs->request_set_authzid(req->rctx, authzid);
+       if (!funcs->request_set_authzid(req->rctx, authzid)) {
+               req->failed = TRUE;
+               return FALSE;
+       }
+       return TRUE;
 }
 
 void sasl_server_request_set_realm(struct sasl_server_mech_request *mreq,
@@ -286,6 +310,13 @@ void sasl_server_request_output(struct sasl_server_mech_request *mreq,
 
        i_assert(req->rctx != NULL);
 
+       i_assert(!req->failed);
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_NEW ||
+                req->state == SASL_SERVER_REQUEST_STATE_SERVER ||
+                req->state == SASL_SERVER_REQUEST_STATE_PASSDB);
+       req->state = SASL_SERVER_REQUEST_STATE_CLIENT;
+       req->sequence++;
+
        const struct sasl_server_output output = {
                .status = SASL_SERVER_OUTPUT_CONTINUE,
                .data = data,
@@ -304,6 +335,17 @@ void sasl_server_request_success(struct sasl_server_mech_request *mreq,
 
        i_assert(req->rctx != NULL);
 
+       i_assert(!req->failed);
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_NEW ||
+                req->state == SASL_SERVER_REQUEST_STATE_SERVER ||
+                req->state == SASL_SERVER_REQUEST_STATE_PASSDB);
+       req->state = SASL_SERVER_REQUEST_STATE_FINISHED;
+       req->sequence++;
+       if (data_size > 0) {
+               i_assert(!req->finished_with_data);
+               req->finished_with_data = TRUE;
+       }
+
        const struct sasl_server_output output = {
                .status = SASL_SERVER_OUTPUT_SUCCESS,
                .data = data,
@@ -324,6 +366,18 @@ sasl_server_request_failure_common(struct sasl_server_mech_request *mreq,
 
        i_assert(req->rctx != NULL);
 
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_NEW ||
+                req->state == SASL_SERVER_REQUEST_STATE_SERVER ||
+                req->state == SASL_SERVER_REQUEST_STATE_PASSDB);
+       req->state = SASL_SERVER_REQUEST_STATE_FINISHED;
+       req->sequence++;
+       req->failed = TRUE;
+       if (data_size > 0) {
+               i_assert(status != SASL_SERVER_OUTPUT_INTERNAL_FAILURE);
+               i_assert(!req->finished_with_data);
+               req->finished_with_data = TRUE;
+       }
+
        const struct sasl_server_output output = {
                .status = status,
                .data = data,
@@ -360,6 +414,11 @@ verify_plain_callback(struct sasl_server_req_ctx *rctx,
 {
        struct sasl_server_request *req = rctx->request;
 
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_PASSDB);
+       req->state = SASL_SERVER_REQUEST_STATE_SERVER;
+       if (result->status == SASL_PASSDB_RESULT_INTERNAL_FAILURE)
+               req->failed = TRUE;
+
        i_assert(req->passdb_type == SASL_SERVER_PASSDB_TYPE_VERIFY_PLAIN);
        req->passdb_callback(req->mech, result);
 }
@@ -374,6 +433,11 @@ void sasl_server_request_verify_plain(
 
        i_assert(req->rctx != NULL);
 
+       i_assert(!req->failed);
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_NEW ||
+                req->state == SASL_SERVER_REQUEST_STATE_SERVER);
+       req->state = SASL_SERVER_REQUEST_STATE_PASSDB;
+
        req->passdb_type = SASL_SERVER_PASSDB_TYPE_VERIFY_PLAIN;
        req->passdb_callback = callback;
 
@@ -388,6 +452,11 @@ lookup_credentials_callback(struct sasl_server_req_ctx *rctx,
 {
        struct sasl_server_request *req = rctx->request;
 
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_PASSDB);
+       req->state = SASL_SERVER_REQUEST_STATE_SERVER;
+       if (result->status == SASL_PASSDB_RESULT_INTERNAL_FAILURE)
+               req->failed = TRUE;
+
        i_assert(req->passdb_type ==
                 SASL_SERVER_PASSDB_TYPE_LOOKUP_CREDENTIALS);
        req->passdb_callback(req->mech, result);
@@ -403,6 +472,11 @@ void sasl_server_request_lookup_credentials(
 
        i_assert(req->rctx != NULL);
 
+       i_assert(!req->failed);
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_NEW ||
+                req->state == SASL_SERVER_REQUEST_STATE_SERVER);
+       req->state = SASL_SERVER_REQUEST_STATE_PASSDB;
+
        req->passdb_type = SASL_SERVER_PASSDB_TYPE_LOOKUP_CREDENTIALS;
        req->passdb_callback = callback;
 
@@ -417,6 +491,11 @@ set_credentials_callback(struct sasl_server_req_ctx *rctx,
 {
        struct sasl_server_request *req = rctx->request;
 
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_PASSDB);
+       req->state = SASL_SERVER_REQUEST_STATE_SERVER;
+       if (result->status == SASL_PASSDB_RESULT_INTERNAL_FAILURE)
+               req->failed = TRUE;
+
        i_assert(req->passdb_type == SASL_SERVER_PASSDB_TYPE_SET_CREDENTIALS);
        req->passdb_callback(req->mech, result);
 }
@@ -432,6 +511,11 @@ void sasl_server_request_set_credentials(
 
        i_assert(req->rctx != NULL);
 
+       i_assert(!req->failed);
+       i_assert(req->state == SASL_SERVER_REQUEST_STATE_NEW ||
+                req->state == SASL_SERVER_REQUEST_STATE_SERVER);
+       req->state = SASL_SERVER_REQUEST_STATE_PASSDB;
+
        req->passdb_type = SASL_SERVER_PASSDB_TYPE_SET_CREDENTIALS;
        req->passdb_callback = callback;
 
index 97ddf25fae4e41cf7b9ba5873ce8a1c410ebce15..2431e3d3a60e2a028b2cd8fbd706f1384b937876 100644 (file)
@@ -91,6 +91,21 @@ enum sasl_server_authid_type {
        SASL_SERVER_AUTHID_TYPE_EXTERNAL,
 };
 
+enum sasl_server_request_state {
+       /* Request is newly created */
+       SASL_SERVER_REQUEST_STATE_NEW = 0,
+       /* Server needs to act next on this request */
+       SASL_SERVER_REQUEST_STATE_SERVER,
+       /* Client needs to act next on this request */
+       SASL_SERVER_REQUEST_STATE_CLIENT,
+       /* Server is waiting for passdb lookup */
+       SASL_SERVER_REQUEST_STATE_PASSDB,
+       /* Request is finished */
+       SASL_SERVER_REQUEST_STATE_FINISHED,
+       /* Request is aborted */
+       SASL_SERVER_REQUEST_STATE_ABORTED,
+};
+
 struct sasl_server_req_ctx {
        const struct sasl_server_mech *mech;
        const char *mech_name;