From f55d5503336233c90fb84aec15c2f0a18116831e Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Tue, 17 Apr 2018 12:27:15 -0400 Subject: [PATCH] Use TR_MSG instead of encoded strings in GSS request handler interface Also some further cleanup of header files and data types. --- common/tr_gss.c | 41 +++++++++++++++++++++++++++++++---------- include/mon_internal.h | 8 ++++---- include/tr_gss.h | 2 +- mon/mons.c | 27 +++++++++++++++++++++++++-- tid/tids.c | 46 +++++++++++++++++++++++++--------------------- 5 files changed, 86 insertions(+), 38 deletions(-) diff --git a/common/tr_gss.c b/common/tr_gss.c index 39a13a9..e2483b9 100644 --- a/common/tr_gss.c +++ b/common/tr_gss.c @@ -221,6 +221,9 @@ void tr_gss_handle_connection(int conn, TALLOC_CTX *tmp_ctx = talloc_new(NULL); gss_ctx_id_t gssctx = GSS_C_NO_CONTEXT; char *req_str = NULL; + size_t req_len = 0; + TR_MSG *req_msg = NULL; + TR_MSG *resp_msg = NULL; char *resp_str = NULL; if (tr_gss_auth_connection(conn, @@ -236,34 +239,52 @@ void tr_gss_handle_connection(int conn, tr_debug("tr_gss_handle_connection: Connection authorized"); // TODO: should there be a timeout on this? - while (1) { /* continue until an error breaks us out */ + do { + /* continue until an error breaks us out */ // try to read a request req_str = tr_gss_read_req(tmp_ctx, conn, gssctx); - if ( req_str == NULL) { + if (req_str == NULL) { // an error occurred, give up tr_notice("tr_gss_handle_connection: Error reading request"); goto cleanup; - } else if (strlen(req_str) > 0) { - // we got a request message, exit the loop and process it - break; } - // no error, but no message, keep waiting for one - talloc_free(req_str); // this would be cleaned up anyway, but may as well free it + req_len = strlen(req_str); + + /* If we got no characters, we will loop again. Free the empty response for the next loop. */ + if (req_len == 0) + talloc_free(req_str); + + } while (req_len == 0); + + /* Decode the request */ + req_msg = tr_msg_decode(tmp_ctx, req_str, req_len); + if (req_msg == NULL) { + tr_notice("tr_gss_handle_connection: Error decoding response"); + goto cleanup; } /* Hand off the request for processing and get the response */ - resp_str = req_cb(tmp_ctx, req_str, req_cookie); + resp_msg = req_cb(tmp_ctx, req_msg, req_cookie); - if (resp_str == NULL) { + if (resp_msg == NULL) { // no response, clean up goto cleanup; } + /* Encode the response */ + resp_str = tr_msg_encode(tmp_ctx, resp_msg); + if (resp_str == NULL) { + /* We apparently can't encode a response, so just return */ + tr_err("tr_gss_handle_connection: Error encoding response"); + goto cleanup; + } + // send the response if (tr_gss_write_resp(conn, gssctx, resp_str)) { - tr_notice("tr_gss_handle_connection: Error writing response"); + tr_err("tr_gss_handle_connection: Error writing response"); + goto cleanup; } cleanup: diff --git a/include/mon_internal.h b/include/mon_internal.h index 0b19830..27fc00e 100644 --- a/include/mon_internal.h +++ b/include/mon_internal.h @@ -41,8 +41,8 @@ #include #include #include - -//#include +#include +#include #include #include #include @@ -126,8 +126,8 @@ struct mons_instance { const char *hostname; unsigned int port; TR_GSS_NAMES *authorized_gss_names; - void *tids; // TODO sort out header file cycles and use typed pointers - void *trps; // TODO sort out header file cycles and use typed pointers + TIDS_INSTANCE *tids; + TRPS_INSTANCE *trps; MONS_REQ_FUNC *req_handler; MONS_AUTH_FUNC *auth_handler; void *cookie; diff --git a/include/tr_gss.h b/include/tr_gss.h index 30abc6d..d035a4e 100644 --- a/include/tr_gss.h +++ b/include/tr_gss.h @@ -38,7 +38,7 @@ #include typedef int (TR_GSS_AUTH_FN)(gss_name_t, TR_NAME *, void *); -typedef char *(TR_GSS_HANDLE_REQ_FN)(TALLOC_CTX *, const char *, void *); +typedef TR_MSG *(TR_GSS_HANDLE_REQ_FN)(TALLOC_CTX *, TR_MSG *, void *); void tr_gss_handle_connection(int conn, const char *acceptor_name, const char *acceptor_realm, TR_GSS_AUTH_FN auth_cb, void *auth_cookie, TR_GSS_HANDLE_REQ_FN req_cb, void *req_cookie); diff --git a/mon/mons.c b/mon/mons.c index d79cbcb..d2a5c8a 100644 --- a/mon/mons.c +++ b/mon/mons.c @@ -44,6 +44,8 @@ #include #include +#include "mons_handlers.h" + /** * Allocate a new MONS_INSTANCE * @@ -78,9 +80,30 @@ MONS_INSTANCE *mons_new(TALLOC_CTX *mem_ctx) * @param data pointer to a MONS_INSTANCE * @return pointer to the response string or null to send no response */ -static char *mons_req_cb(TALLOC_CTX *mem_ctx, const char *req_str, void *data) +static TR_MSG *mons_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *req_msg, void *data) { - return "This is a response."; + TALLOC_CTX *tmp_ctx = talloc_new(NULL); + //MONS_INSTANCE *mons = talloc_get_type_abort(data, MONS_INSTANCE); + MON_REQ *req = NULL; + TR_MSG *resp_msg = NULL; /* This is the response value */ + + /* Validate inputs */ + if (req_msg == NULL) + goto cleanup; + + req = tr_msg_get_mon_req(req_msg); + if (req == NULL) { + /* this is an internal error */ + tr_err("mons_req_cb: Received incorrect message type (was %d, expected %d)", + tr_msg_get_msg_type(req_msg), + MON_REQUEST); + /* TODO send an error response */ + goto cleanup; + } + +cleanup: + talloc_free(tmp_ctx); + return resp_msg; } /** diff --git a/tid/tids.c b/tid/tids.c index a94c902..2e5dbea 100644 --- a/tid/tids.c +++ b/tid/tids.c @@ -255,41 +255,45 @@ int tids_send_response (TIDS_INSTANCE *tids, TID_REQ *req, TID_RESP *resp) * @param data pointer to a TIDS_INSTANCE * @return pointer to the response string or null to send no response */ -static char *tids_req_cb(TALLOC_CTX *mem_ctx, const char *req_str, void *data) +static TR_MSG *tids_req_cb(TALLOC_CTX *mem_ctx, TR_MSG *mreq, void *data) { + TALLOC_CTX *tmp_ctx = talloc_new(NULL); TIDS_INSTANCE *tids = talloc_get_type_abort(data, TIDS_INSTANCE); - TR_MSG *mreq = NULL; TID_REQ *req = NULL; TID_RESP *resp = NULL; - char *resp_str = NULL; + TR_MSG *resp_msg = NULL; /* this is the return value */ int rc = 0; - mreq = tr_msg_decode(NULL, req_str, strlen(req_str)); // allocates memory on success! - if (mreq == NULL) { - tr_debug("tids_req_cb: Error decoding request."); - return NULL; - } - /* If this isn't a TID Request, just drop it. */ if (mreq->msg_type != TID_REQUEST) { - tr_msg_free_decoded(mreq); tr_debug("tids_req_cb: Not a TID request, dropped."); - return NULL; + goto cleanup; } /* Get a handle on the request itself. Don't free req - it belongs to mreq */ req = tr_msg_get_req(mreq); - /* Allocate a response structure and populate common fields. The resp is in req's talloc context, - * which will be cleaned up when mreq is freed. */ - resp = tids_create_response(req, req); + /* Allocate a response message */ + resp_msg = talloc(tmp_ctx, TR_MSG); + if (resp_msg == NULL) { + /* We cannot create a response message, so all we can really do is emit + * an error message and return. */ + tr_crit("tids_req_cb: Error allocating response message."); + goto cleanup; + } + + /* Allocate a response structure and populate common fields. Put it in the + * response message's talloc context. */ + resp = tids_create_response(resp_msg, req); if (resp == NULL) { /* If we were unable to create a response, we cannot reply. Log an * error if we can, then drop the request. */ - tr_msg_free_decoded(mreq); tr_crit("tids_req_cb: Error creating response structure."); - return NULL; + resp_msg = NULL; /* the contents are in tmp_ctx, so they will still be cleaned up */ + goto cleanup; } + /* Now officially assign the response to the message. */ + tr_msg_set_resp(resp_msg, resp); /* Handle the request and fill in resp */ rc = tids_handle_request(tids, req, resp); @@ -298,12 +302,12 @@ static char *tids_req_cb(TALLOC_CTX *mem_ctx, const char *req_str, void *data) /* Fall through, to send the response, either way */ } - /* Convert the completed response into an encoded response */ - resp_str = tids_encode_response(mem_ctx, resp); + /* put the response message in the caller's context */ + talloc_steal(mem_ctx, resp_msg); - /* Finished; free the request and return */ - tr_msg_free_decoded(mreq); // this frees req and resp, too - return resp_str; +cleanup: + talloc_free(tmp_ctx); + return resp_msg; } TIDS_INSTANCE *tids_new(TALLOC_CTX *mem_ctx) -- 2.1.4