From: Jennifer Richards Date: Tue, 6 Dec 2016 22:33:32 +0000 (-0500) Subject: Fix bugs related to handling TID responses. Fix a few memory leaks. X-Git-Tag: v2.1.1~14 X-Git-Url: http://www.project-moonshot.org/gitweb/?p=trust_router.git;a=commitdiff_plain;h=96f51354ae589732739aaf1dcb48f5be9491278b Fix bugs related to handling TID responses. Fix a few memory leaks. --- diff --git a/common/tr_dh.c b/common/tr_dh.c index 257b77c..e8f2b79 100644 --- a/common/tr_dh.c +++ b/common/tr_dh.c @@ -277,11 +277,13 @@ int tr_dh_pub_hash(TID_REQ *request, unsigned char *bn_bytes = talloc_zero_size(request, BN_num_bytes(pub)); unsigned char *digest = talloc_zero_size(request, SHA_DIGEST_LENGTH+1); assert(bn_bytes && digest); - BN_bn2bin(pub, bn_bytes); - SHA1(bn_bytes, BN_num_bytes(pub), digest); - *out_digest = digest; - *out_len = SHA_DIGEST_LENGTH; - return 0; + BN_bn2bin(pub, bn_bytes); + SHA1(bn_bytes, BN_num_bytes(pub), digest); + *out_digest = digest; + *out_len = SHA_DIGEST_LENGTH; + + talloc_free(bn_bytes); + return 0; } void tr_dh_free(DH *dh) diff --git a/common/tr_mq.c b/common/tr_mq.c index fd8a42c..944ba32 100644 --- a/common/tr_mq.c +++ b/common/tr_mq.c @@ -122,6 +122,9 @@ TR_MQ *tr_mq_new(TALLOC_CTX *mem_ctx) mq->head=NULL; mq->tail=NULL; mq->last_hi_prio=NULL; + + mq->notify_cb=NULL; + mq->notify_cb_arg=NULL; } return mq; } diff --git a/common/tr_msg.c b/common/tr_msg.c index d8b114b..dbc7938 100644 --- a/common/tr_msg.c +++ b/common/tr_msg.c @@ -359,7 +359,7 @@ static int tr_msg_decode_one_server(json_t *jsrvr, TID_SRVR_BLK *srvr) srvr->aaa_server_addr=talloc_strdup(srvr, json_string_value(jsrvr_addr)); srvr->key_name = tr_new_name((char *)json_string_value(jsrvr_kn)); srvr->aaa_server_dh = tr_msg_decode_dh(jsrvr_dh); - srvr->path = json_object_get(jsrvr, "path"); + tid_srvr_blk_set_path(srvr, json_object_get(jsrvr, "path")); jsrvr_expire = json_object_get(jsrvr, "key_expiration"); if (jsrvr_expire && json_is_string(jsrvr_expire)) { if (!g_time_val_from_iso8601(json_string_value(jsrvr_expire), @@ -410,7 +410,7 @@ static TID_SRVR_BLK *tr_msg_decode_servers(TALLOC_CTX *mem_ctx, json_t *jservers for (i = 0; i < num_servers; i++) { jsrvr = json_array_get(jservers, i); - new_srvr=tid_srvr_blk_new(NULL); + new_srvr=tid_srvr_blk_new(tmp_ctx); if (new_srvr==NULL) { servers=NULL; /* it's all in tmp_ctx, so we can just let go */ goto cleanup; @@ -531,18 +531,18 @@ static TID_RESP *tr_msg_decode_tidresp(json_t *jresp) tr_debug("tr_msg_decode_tidresp(): Error! result = %s.", json_string_value(jresult)); if ((NULL != (jerr_msg = json_object_get(jresp, "err_msg"))) || (!json_is_string(jerr_msg))) { - tresp->err_msg = tr_new_name((char *)json_string_value(jerr_msg)); + tresp->err_msg = tr_new_name(json_string_value(jerr_msg)); } } - tresp->rp_realm = tr_new_name((char *)json_string_value(jrp_realm)); - tresp->realm = tr_new_name((char *)json_string_value(jrealm)); - tresp->comm = tr_new_name((char *)json_string_value(jcomm)); + tresp->rp_realm = tr_new_name(json_string_value(jrp_realm)); + tresp->realm = tr_new_name(json_string_value(jrealm)); + tresp->comm = tr_new_name(json_string_value(jcomm)); /* store optional "orig_coi" field */ if ((NULL != (jorig_coi = json_object_get(jresp, "orig_coi"))) && (!json_is_object(jorig_coi))) { - tresp->orig_coi = tr_new_name((char *)json_string_value(jorig_coi)); + tresp->orig_coi = tr_new_name(json_string_value(jorig_coi)); } return tresp; @@ -1246,6 +1246,9 @@ TR_MSG *tr_msg_decode(char *jbuf, size_t buflen) msg->msg_type = TR_UNKNOWN; msg->msg_rep = NULL; } + + json_decref(jmsg); + return msg; } diff --git a/gsscon/gsscon_passive.c b/gsscon/gsscon_passive.c index 1ede6d3..7cdff69 100755 --- a/gsscon/gsscon_passive.c +++ b/gsscon/gsscon_passive.c @@ -56,136 +56,136 @@ const char *gServiceName = NULL; -int gsscon_passive_authenticate (int inSocket, - gss_buffer_desc inNameBuffer, - gss_ctx_id_t *outGSSContext, - client_cb_fn clientCb, - void *clientCbData) +int gsscon_passive_authenticate (int inSocket, + gss_buffer_desc inNameBuffer, + gss_ctx_id_t *outGSSContext, + client_cb_fn clientCb, + void *clientCbData) { - int err = 0; - OM_uint32 majorStatus; - OM_uint32 minorStatus = 0, minorStatusToo = 0; - gss_ctx_id_t gssContext = GSS_C_NO_CONTEXT; - gss_name_t clientName = GSS_C_NO_NAME, serviceName = GSS_C_NO_NAME; - gss_cred_id_t acceptorCredentials = NULL; - gss_buffer_desc clientDisplayName = {0, NULL}; - char *inputTokenBuffer = NULL; - size_t inputTokenBufferLength = 0; - gss_buffer_desc inputToken; /* buffer received from the server */ + int err = 0; + OM_uint32 majorStatus; + OM_uint32 minorStatus = 0, minorStatusToo = 0; + gss_ctx_id_t gssContext = GSS_C_NO_CONTEXT; + gss_name_t clientName = GSS_C_NO_NAME, serviceName = GSS_C_NO_NAME; + gss_cred_id_t acceptorCredentials = NULL; + gss_buffer_desc clientDisplayName = {0, NULL}; + char *inputTokenBuffer = NULL; + size_t inputTokenBufferLength = 0; + gss_buffer_desc inputToken; /* buffer received from the server */ - if (inSocket < 0 ) { err = EINVAL; } - if (!outGSSContext) { err = EINVAL; } + if (inSocket < 0 ) { err = EINVAL; } + if (!outGSSContext) { err = EINVAL; } - if (!err) { - majorStatus = gss_import_name (&minorStatus, &inNameBuffer, (gss_OID) GSS_C_NT_HOSTBASED_SERVICE, &serviceName); - if (majorStatus != GSS_S_COMPLETE) { - gsscon_print_gss_errors ("gss_import_name(serviceName)", majorStatus, minorStatus); - err = minorStatus ? minorStatus : majorStatus; - } + if (!err) { + majorStatus = gss_import_name (&minorStatus, &inNameBuffer, (gss_OID) GSS_C_NT_HOSTBASED_SERVICE, &serviceName); + if (majorStatus != GSS_S_COMPLETE) { + gsscon_print_gss_errors ("gss_import_name(serviceName)", majorStatus, minorStatus); + err = minorStatus ? minorStatus : majorStatus; } + } - if (!err) { - majorStatus = gss_acquire_cred ( &minorStatus, serviceName, - GSS_C_INDEFINITE, GSS_C_NO_OID_SET, - GSS_C_ACCEPT, &acceptorCredentials, - NULL /*mechs out*/, NULL /*time out*/); - if (majorStatus != GSS_S_COMPLETE) { - gsscon_print_gss_errors ("gss_acquire_cred", majorStatus, minorStatus); - err = minorStatus ? minorStatus : majorStatus; - } + if (!err) { + majorStatus = gss_acquire_cred ( &minorStatus, serviceName, + GSS_C_INDEFINITE, GSS_C_NO_OID_SET, + GSS_C_ACCEPT, &acceptorCredentials, + NULL /*mechs out*/, NULL /*time out*/); + if (majorStatus != GSS_S_COMPLETE) { + gsscon_print_gss_errors ("gss_acquire_cred", majorStatus, minorStatus); + err = minorStatus ? minorStatus : majorStatus; } + } - /* - * The main authentication loop: - * - * GSS is a multimechanism API. The number of packet exchanges required to - * authenticatevaries between mechanisms. As a result, we need to loop reading - * input tokens from the client, calling gss_accept_sec_context on the input - * tokens and send the resulting output tokens back to the client until we - * get GSS_S_COMPLETE or an error. - * - * When we are done, save the client principal so we can make authorization - * checks. - */ + /* + * The main authentication loop: + * + * GSS is a multimechanism API. The number of packet exchanges required to + * authenticatevaries between mechanisms. As a result, we need to loop reading + * input tokens from the client, calling gss_accept_sec_context on the input + * tokens and send the resulting output tokens back to the client until we + * get GSS_S_COMPLETE or an error. + * + * When we are done, save the client principal so we can make authorization + * checks. + */ - majorStatus = GSS_S_CONTINUE_NEEDED; - while (!err && (majorStatus != GSS_S_COMPLETE)) { - /* Clean up old input buffer */ - if (inputTokenBuffer != NULL) { - free (inputTokenBuffer); - inputTokenBuffer = NULL; /* don't double-free */ - } + majorStatus = GSS_S_CONTINUE_NEEDED; + while (!err && (majorStatus != GSS_S_COMPLETE)) { + /* Clean up old input buffer */ + if (inputTokenBuffer != NULL) { + free (inputTokenBuffer); + inputTokenBuffer = NULL; /* don't double-free */ + } - err = gsscon_read_token (inSocket, &inputTokenBuffer, &inputTokenBufferLength); + err = gsscon_read_token (inSocket, &inputTokenBuffer, &inputTokenBufferLength); - if (!err) { - /* Set up input buffers for the next run through the loop */ - inputToken.value = inputTokenBuffer; - inputToken.length = inputTokenBufferLength; - } + if (!err) { + /* Set up input buffers for the next run through the loop */ + inputToken.value = inputTokenBuffer; + inputToken.length = inputTokenBufferLength; + } - if (!err) { - /* buffer to send to the server */ - gss_buffer_desc outputToken = { 0, NULL }; + if (!err) { + /* buffer to send to the server */ + gss_buffer_desc outputToken = { 0, NULL }; - /* - * accept_sec_context does the actual work of taking the client's - * request and generating an appropriate reply. */ - majorStatus = gss_accept_sec_context (&minorStatus, - &gssContext, - acceptorCredentials, - &inputToken, - GSS_C_NO_CHANNEL_BINDINGS, - &clientName, - NULL /* actual_mech_type */, - &outputToken, - NULL /* req_flags */, - NULL /* time_rec */, - NULL /* delegated_cred_handle */); + /* + * accept_sec_context does the actual work of taking the client's + * request and generating an appropriate reply. */ + majorStatus = gss_accept_sec_context (&minorStatus, + &gssContext, + acceptorCredentials, + &inputToken, + GSS_C_NO_CHANNEL_BINDINGS, + &clientName, + NULL /* actual_mech_type */, + &outputToken, + NULL /* req_flags */, + NULL /* time_rec */, + NULL /* delegated_cred_handle */); - if ((outputToken.length > 0) && (outputToken.value != NULL)) { - /* Send the output token to the client (even on error) */ - err = gsscon_write_token (inSocket, outputToken.value, outputToken.length); + if ((outputToken.length > 0) && (outputToken.value != NULL)) { + /* Send the output token to the client (even on error) */ + err = gsscon_write_token (inSocket, outputToken.value, outputToken.length); - /* free the output token */ - gss_release_buffer (&minorStatusToo, &outputToken); - } - } + /* free the output token */ + gss_release_buffer (&minorStatusToo, &outputToken); + } + } - if ((majorStatus != GSS_S_COMPLETE) && (majorStatus != GSS_S_CONTINUE_NEEDED)) { - gsscon_print_gss_errors ("gss_accept_sec_context", majorStatus, minorStatus); - err = minorStatus ? minorStatus : majorStatus; - } + if ((majorStatus != GSS_S_COMPLETE) && (majorStatus != GSS_S_CONTINUE_NEEDED)) { + gsscon_print_gss_errors ("gss_accept_sec_context", majorStatus, minorStatus); + err = minorStatus ? minorStatus : majorStatus; } + } - if (!err) { - majorStatus = gss_display_name(&minorStatus, clientName, &clientDisplayName, NULL); - if (GSS_ERROR(majorStatus)) { - gsscon_print_gss_errors("gss_display_name", majorStatus, minorStatus); - err = EINVAL; - } - if (!err) - err = clientCb(clientName, &clientDisplayName, clientCbData); + if (!err) { + majorStatus = gss_display_name(&minorStatus, clientName, &clientDisplayName, NULL); + if (GSS_ERROR(majorStatus)) { + gsscon_print_gss_errors("gss_display_name", majorStatus, minorStatus); + err = EINVAL; } + if (!err) + err = clientCb(clientName, &clientDisplayName, clientCbData); + } - if (!err) { - *outGSSContext = gssContext; - gssContext = NULL; - } else { - gsscon_print_error (err, "Authenticate failed"); - } + if (!err) { + *outGSSContext = gssContext; + gssContext = NULL; + } else { + gsscon_print_error (err, "Authenticate failed"); + } - if (inputTokenBuffer) { free (inputTokenBuffer); } - if (gssContext != GSS_C_NO_CONTEXT) { - gss_delete_sec_context (&minorStatus, &gssContext, GSS_C_NO_BUFFER); } -if (clientName != GSS_C_NO_NAME) - gss_release_name(&minorStatus, &clientName); -if (clientDisplayName.value != NULL) - gss_release_buffer(&minorStatus, &clientDisplayName); - gss_release_name( &minorStatus, &serviceName); - gss_release_cred( &minorStatus, &acceptorCredentials); + if (inputTokenBuffer) { free (inputTokenBuffer); } + if (gssContext != GSS_C_NO_CONTEXT) { + gss_delete_sec_context (&minorStatus, &gssContext, GSS_C_NO_BUFFER); } + if (clientName != GSS_C_NO_NAME) + gss_release_name(&minorStatus, &clientName); + if (clientDisplayName.value != NULL) + gss_release_buffer(&minorStatus, &clientDisplayName); + gss_release_name( &minorStatus, &serviceName); + gss_release_cred( &minorStatus, &acceptorCredentials); - return err; + return err; } @@ -195,18 +195,18 @@ if (clientDisplayName.value != NULL) static int ClientPrincipalIsAuthorizedForService (const char *inClientPrincipal) { - int err = 0; - /* - * Here is where the server checks to see if the client principal should - * be allowed to use your service. Typically it should check both the name - * and the realm, since with cross-realm shared keys, a user at another - * realm may be trying to contact your service. - */ - err = 0; + int err = 0; + /* + * Here is where the server checks to see if the client principal should + * be allowed to use your service. Typically it should check both the name + * and the realm, since with cross-realm shared keys, a user at another + * realm may be trying to contact your service. + */ + err = 0; - return err; + return err; } /* --------------------------------------------------------------------------- */ @@ -215,56 +215,56 @@ int gsscon_authorize (gss_ctx_id_t inContext, int *outAuthorized, int *outAuthorizationError) { - int err = 0; - OM_uint32 majorStatus; - OM_uint32 minorStatus = 0; - gss_name_t clientName = NULL; - gss_name_t serviceName = NULL; - char *clientPrincipal = NULL; - char *servicePrincipal = NULL; + int err = 0; + OM_uint32 majorStatus; + OM_uint32 minorStatus = 0; + gss_name_t clientName = NULL; + gss_name_t serviceName = NULL; + char *clientPrincipal = NULL; + char *servicePrincipal = NULL; - if (!inContext ) { err = EINVAL; } - if (!outAuthorized ) { err = EINVAL; } - if (!outAuthorizationError) { err = EINVAL; } + if (!inContext ) { err = EINVAL; } + if (!outAuthorized ) { err = EINVAL; } + if (!outAuthorizationError) { err = EINVAL; } - if (!err) { - /* Get the client and service principals used to authenticate */ - majorStatus = gss_inquire_context (&minorStatus, - inContext, - &clientName, - &serviceName, - NULL, NULL, NULL, NULL, NULL); - if (majorStatus != GSS_S_COMPLETE) { - err = minorStatus ? minorStatus : majorStatus; - } + if (!err) { + /* Get the client and service principals used to authenticate */ + majorStatus = gss_inquire_context (&minorStatus, + inContext, + &clientName, + &serviceName, + NULL, NULL, NULL, NULL, NULL); + if (majorStatus != GSS_S_COMPLETE) { + err = minorStatus ? minorStatus : majorStatus; } + } - if (!err) { - /* Pull the client principal string out of the gss name */ - gss_buffer_desc nameToken; + if (!err) { + /* Pull the client principal string out of the gss name */ + gss_buffer_desc nameToken; - majorStatus = gss_display_name (&minorStatus, - clientName, - &nameToken, - NULL); - if (majorStatus != GSS_S_COMPLETE) { - err = minorStatus ? minorStatus : majorStatus; - } + majorStatus = gss_display_name (&minorStatus, + clientName, + &nameToken, + NULL); + if (majorStatus != GSS_S_COMPLETE) { + err = minorStatus ? minorStatus : majorStatus; + } - if (!err) { - clientPrincipal = malloc (nameToken.length + 1); - if (clientPrincipal == NULL) { err = ENOMEM; } - } + if (!err) { + clientPrincipal = malloc (nameToken.length + 1); + if (clientPrincipal == NULL) { err = ENOMEM; } + } - if (!err) { - memcpy (clientPrincipal, nameToken.value, nameToken.length); - clientPrincipal[nameToken.length] = '\0'; - } + if (!err) { + memcpy (clientPrincipal, nameToken.value, nameToken.length); + clientPrincipal[nameToken.length] = '\0'; + } - if (nameToken.value) { gss_release_buffer (&minorStatus, &nameToken); } - } + if (nameToken.value) { gss_release_buffer (&minorStatus, &nameToken); } + } - if (!err) { + if (!err) { // /* Pull the service principal string out of the gss name */ // gss_buffer_desc nameToken; // @@ -290,24 +290,24 @@ int gsscon_authorize (gss_ctx_id_t inContext, // } - int authorizationErr = 0; - authorizationErr = ClientPrincipalIsAuthorizedForService (clientPrincipal); + int authorizationErr = 0; + authorizationErr = ClientPrincipalIsAuthorizedForService (clientPrincipal); // printf ("'%s' is%s authorized for service '%s'\n", // clientPrincipal, authorizationErr ? " NOT" : "", servicePrincipal); // - *outAuthorized = !authorizationErr; - *outAuthorizationError = authorizationErr; - } + *outAuthorized = !authorizationErr; + *outAuthorizationError = authorizationErr; + } - if (serviceName ) { gss_release_name (&minorStatus, &serviceName); } - if (clientName ) { gss_release_name (&minorStatus, &clientName); } - if (clientPrincipal ) { free (clientPrincipal); } - if (servicePrincipal) { free (servicePrincipal); } + if (serviceName ) { gss_release_name (&minorStatus, &serviceName); } + if (clientName ) { gss_release_name (&minorStatus, &clientName); } + if (clientPrincipal ) { free (clientPrincipal); } + if (servicePrincipal) { free (servicePrincipal); } - return err; + return err; } diff --git a/tid/example/tids_main.c b/tid/example/tids_main.c index d8f2345..000b0f3 100644 --- a/tid/example/tids_main.c +++ b/tid/example/tids_main.c @@ -148,6 +148,7 @@ static int handle_authorizations(TID_REQ *req, const unsigned char *dh_hash, if (SQLITE_DONE != sqlite3_result) tr_crit("sqlite3: failed to write to database"); sqlite3_reset(authorization_insert); + sqlite3_clear_bindings(authorization_insert); } return 0; } @@ -161,7 +162,7 @@ static int tids_req_handler (TIDS_INSTANCE *tids, unsigned char *s_keybuf = NULL; int s_keylen = 0; char key_id[12]; - unsigned char *pub_digest; + unsigned char *pub_digest=NULL; size_t pub_digest_len; @@ -236,14 +237,16 @@ static int tids_req_handler (TIDS_INSTANCE *tids, if (NULL != insert_stmt) { int sqlite3_result; gchar *expiration_str = g_time_val_to_iso8601(&resp->servers->key_expiration); - sqlite3_bind_text(insert_stmt, 1, key_id, -1, SQLITE_TRANSIENT); + sqlite3_bind_text(insert_stmt, 1, key_id, -1, SQLITE_TRANSIENT); sqlite3_bind_blob(insert_stmt, 2, s_keybuf, s_keylen, SQLITE_TRANSIENT); sqlite3_bind_blob(insert_stmt, 3, pub_digest, pub_digest_len, SQLITE_TRANSIENT); - sqlite3_bind_text(insert_stmt, 4, expiration_str, -1, SQLITE_TRANSIENT); + sqlite3_bind_text(insert_stmt, 4, expiration_str, -1, SQLITE_TRANSIENT); + g_free(expiration_str); /* bind_text already made its own copy */ sqlite3_result = sqlite3_step(insert_stmt); if (SQLITE_DONE != sqlite3_result) tr_crit("sqlite3: failed to write to database"); sqlite3_reset(insert_stmt); + sqlite3_clear_bindings(insert_stmt); } /* Print out the key. */ @@ -253,6 +256,12 @@ static int tids_req_handler (TIDS_INSTANCE *tids, // } // fprintf(stderr, "\n"); + if (s_keybuf!=NULL) + free(s_keybuf); + + if (pub_digest!=NULL) + talloc_free(pub_digest); + return s_keylen; } diff --git a/tid/tid_resp.c b/tid/tid_resp.c index 3970616..03118b3 100644 --- a/tid/tid_resp.c +++ b/tid/tid_resp.c @@ -82,19 +82,17 @@ void tid_resp_free(TID_RESP *resp) TID_RESP *tid_resp_dup(TALLOC_CTX *mem_ctx, TID_RESP *resp) { - TALLOC_CTX *tmp_ctx=NULL; TID_RESP *newresp=NULL; if (resp==NULL) return NULL; - tmp_ctx=talloc_new(NULL); - newresp=talloc(tmp_ctx, TID_RESP); + newresp=tid_resp_new(mem_ctx); if (NULL!=newresp) { newresp->result=resp->result; newresp->err_msg=tr_dup_name(resp->err_msg); - newresp->rp_realm=tr_dup_name(resp->err_msg); + newresp->rp_realm=tr_dup_name(resp->rp_realm); newresp->realm=tr_dup_name(resp->realm); newresp->comm=tr_dup_name(resp->comm); newresp->orig_coi=tr_dup_name(resp->orig_coi); @@ -102,7 +100,6 @@ TID_RESP *tid_resp_dup(TALLOC_CTX *mem_ctx, TID_RESP *resp) tid_resp_set_cons(newresp, resp->cons); tid_resp_set_error_path(newresp, resp->error_path); } - talloc_free(tmp_ctx); return newresp; } @@ -238,6 +235,7 @@ TR_EXPORT TID_SRVR_BLK *tid_srvr_blk_dup(TALLOC_CTX *mem_ctx, TID_SRVR_BLK *srvr new->key_name=tr_dup_name(srvr->key_name); new->aaa_server_dh=tr_dh_dup(srvr->aaa_server_dh); new->key_expiration=srvr->key_expiration; + tid_srvr_blk_set_path(new, srvr->path); tid_srvr_blk_add(new->next, tid_srvr_blk_dup(mem_ctx, srvr->next)); @@ -255,7 +253,12 @@ TR_EXPORT TID_SRVR_BLK *tid_srvr_blk_add_func(TID_SRVR_BLK *head, TID_SRVR_BLK * while (this->next!=NULL) this=this->next; + this->next=new; + while (this!=NULL) { + talloc_steal(head, this); + this=this->next; + } return head; } diff --git a/tid/tids.c b/tid/tids.c index 2e226ef..64c7764 100644 --- a/tid/tids.c +++ b/tid/tids.c @@ -77,37 +77,12 @@ static TID_RESP *tids_create_response (TIDS_INSTANCE *tids, TID_REQ *req) cleanup: if ((!success) && (resp!=NULL)) { - if (resp->rp_realm!=NULL) - tr_free_name(resp->rp_realm); - if (resp->realm!=NULL) - tr_free_name(resp->realm); - if (resp->comm!=NULL) - tr_free_name(resp->comm); - if (resp->orig_coi!=NULL) - tr_free_name(resp->orig_coi); talloc_free(resp); resp=NULL; } return resp; } -static void tids_destroy_response(TIDS_INSTANCE *tids, TID_RESP *resp) -{ - if (resp) { - if (resp->err_msg) - tr_free_name(resp->err_msg); - if (resp->rp_realm) - tr_free_name(resp->rp_realm); - if (resp->realm) - tr_free_name(resp->realm); - if (resp->comm) - tr_free_name(resp->comm); - if (resp->orig_coi) - tr_free_name(resp->orig_coi); - talloc_free(resp); - } -} - static int tids_listen(TIDS_INSTANCE *tids, int port, int *fd_out, size_t max_fd) { int rc = 0; @@ -217,8 +192,11 @@ static int tids_auth_connection (TIDS_INSTANCE *inst, if (rc = gsscon_passive_authenticate(conn, nameBuffer, gssctx, tids_auth_cb, inst)) { tr_debug("tids_auth_connection: Error from gsscon_passive_authenticate(), rc = %d.", rc); + free(name); return -1; } + free(name); + nameBuffer.value=NULL; nameBuffer.length=0; if (rc = gsscon_authorize(*gssctx, &auth, &autherr)) { tr_debug("tids_auth_connection: Error from gsscon_authorize, rc = %d, autherr = %d.", @@ -323,7 +301,7 @@ int tids_send_err_response (TIDS_INSTANCE *tids, TID_REQ *req, const char *err_m rc = tids_send_response(tids, req, resp); - tids_destroy_response(tids, resp); + tid_resp_free(resp); return rc; } @@ -425,8 +403,7 @@ static void tids_handle_connection (TIDS_INSTANCE *tids, int conn) /* Fall through to free the response, either way. */ } - tids_destroy_response(tids, resp); - tr_msg_free_decoded(mreq); + tr_msg_free_decoded(mreq); /* takes resp with it */ return; } } diff --git a/tr/tr_tid.c b/tr/tr_tid.c index 27c7a45..7278a45 100644 --- a/tr/tr_tid.c +++ b/tr/tr_tid.c @@ -156,6 +156,15 @@ static void *tr_tids_req_fwd_thread(void *arg) talloc_steal(tmp_ctx, args); /* take responsibility for the cookie */ + /* create the cookie we will use for our response */ + cookie=talloc(tmp_ctx, TR_RESP_COOKIE); + if (cookie==NULL) { + tr_notice("tr_tids_req_fwd_thread: unable to allocate response cookie."); + success=0; + goto cleanup; + } + cookie->thread_id=args->thread_id; + /* Create a TID client instance */ if (tidc==NULL) { tr_crit("tr_tids_req_fwd_thread: Unable to allocate TIDC instance."); @@ -178,13 +187,6 @@ static void *tr_tids_req_fwd_thread(void *arg) }; /* Send a TID request. */ - cookie=talloc(tmp_ctx, TR_RESP_COOKIE); - if (cookie==NULL) { - tr_notice("tr_tids_req_fwd_thread: unable to allocate response cookie."); - success=0; - goto cleanup; - } - cookie->thread_id=args->thread_id; if (0 > (rc = tidc_fwd_request(tidc, args->fwd_req, tr_tidc_resp_handler, (void *)cookie))) { tr_notice("Error from tidc_fwd_request, rc = %d.", rc); success=0; @@ -227,6 +229,15 @@ static TID_RC tr_tids_merge_resps(TID_RESP *r1, TID_RESP *r2) if ((r1->result!=TID_SUCCESS) || (r2->result!=TID_SUCCESS)) return TID_ERROR; + if (r1==NULL) tr_debug("******* r1 null"); + if (r1->realm==NULL) tr_debug("******* r1->realm null"); + if (r1->rp_realm==NULL) tr_debug("******* r1->rp_realm null"); + if (r1->comm==NULL) tr_debug("******* r1->comm null"); + if (r2==NULL) tr_debug("******* r2 null"); + if (r2->realm==NULL) tr_debug("******* r2->realm null"); + if (r2->rp_realm==NULL) tr_debug("******* r2->rp_realm null"); + if (r2->comm==NULL) tr_debug("******* r2->comm null"); + if ((0!=tr_name_cmp(r1->rp_realm, r2->rp_realm)) || (0!=tr_name_cmp(r1->realm, r2->realm)) || (0!=tr_name_cmp(r1->comm, r2->comm))) @@ -459,7 +470,7 @@ static int tr_tids_req_handler(TIDS_INSTANCE *tids, aaa_cookie[n_aaa]->mq=mq; aaa_cookie[n_aaa]->aaa_hostname=tr_dup_name(this_aaa->hostname); aaa_cookie[n_aaa]->dh_params=tr_dh_dup(orig_req->tidc_dh); - aaa_cookie[n_aaa]->fwd_req=tid_dup_req(aaa_cookie, fwd_req); + aaa_cookie[n_aaa]->fwd_req=tid_dup_req(aaa_cookie[n_aaa], fwd_req); /* Take the cookie out of tmp_ctx before starting thread. If thread starts, it becomes * responsible for freeing it until it queues a response. */ @@ -499,8 +510,14 @@ static int tr_tids_req_handler(TIDS_INSTANCE *tids, } } else if (0==strcmp(tr_mq_msg_get_message(msg), TR_TID_MQMSG_FAILURE)) { /* failure */ - payload=talloc_get_type_abort(tr_mq_msg_get_payload(msg), TR_RESP_COOKIE); n_failed++; + payload=talloc_get_type(tr_mq_msg_get_payload(msg), TR_RESP_COOKIE); + if (payload==NULL) { + /* this means the thread was unable to allocate a response cookie, and we thus cannot determine which thread it was. This is bad and should never happen in a working system.. Give up. */ + tr_notice("tr_tids_req_handler: TID request thread sent invalid reply. Aborting!"); + retval=-1; + goto cleanup; + } tr_notice("tr_tids_req_handler: TID request for AAA server %d failed.", payload->thread_id); } else {