From a1b1b820c2abe67aff1ac4e517360ee969adaed2 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Thu, 1 Dec 2016 16:21:40 -0500 Subject: [PATCH] Fix segfaults related to copying DH parameters. --- common/tr_dh.c | 71 ++++++++++++++++++++++++++++++++++++++++---- common/tr_msg.c | 18 ++++++----- include/tr_util.h | 4 +-- include/trust_router/tr_dh.h | 6 ++-- tid/example/tidc_main.c | 2 +- tid/tid_resp.c | 22 +++++++------- tid/tidc.c | 2 +- tr/tr_tid.c | 4 +-- 8 files changed, 96 insertions(+), 33 deletions(-) diff --git a/common/tr_dh.c b/common/tr_dh.c index c76ab27..257b77c 100644 --- a/common/tr_dh.c +++ b/common/tr_dh.c @@ -77,6 +77,11 @@ unsigned char tr_2048_dhprime[2048/8] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }; +DH *tr_dh_new(void) +{ + return DH_new(); +} + DH *tr_create_dh_params(unsigned char *priv_key, size_t keylen) { @@ -170,10 +175,64 @@ void tr_destroy_dh_params(DH *dh) { } } -DH *tr_dup_dh_params(DH *dh) { - if (dh!=NULL) - return DHparams_dup(dh); - return NULL; +DH *tr_dh_dup(DH *in) +{ + DH *out=DH_new(); + + if (out==NULL) + return NULL; + + if (in->g==NULL) + out->g=NULL; + else { + out->g=BN_dup(in->g); + if (out->g==NULL) { + DH_free(out); + return NULL; + } + } + + if (in->p==NULL) + out->p=NULL; + else { + out->p=BN_dup(in->p); + if (out->p==NULL) { + DH_free(out); + return NULL; + } + } + + if (in->q==NULL) + out->q=NULL; + else { + out->q=BN_dup(in->q); + if (out->q==NULL) { + DH_free(out); + return NULL; + } + } + + if (in->priv_key==NULL) + out->priv_key=NULL; + else { + out->priv_key=BN_dup(in->priv_key); + if (out->priv_key==NULL) { + DH_free(out); + return NULL; + } + } + + if (in->pub_key==NULL) + out->pub_key=NULL; + else { + out->pub_key=BN_dup(in->pub_key); + if (out->g==NULL) { + DH_free(out); + return NULL; + } + } + + return out; } int tr_compute_dh_key(unsigned char **pbuf, @@ -225,7 +284,7 @@ int tr_dh_pub_hash(TID_REQ *request, return 0; } -void tr_dh_free(unsigned char *dh_buf) +void tr_dh_free(DH *dh) { - free(dh_buf); + DH_free(dh); } diff --git a/common/tr_msg.c b/common/tr_msg.c index 0d18466..d8b114b 100644 --- a/common/tr_msg.c +++ b/common/tr_msg.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,7 @@ #include #include #include +#include #include /* JSON helpers */ @@ -156,19 +158,23 @@ static json_t *tr_msg_encode_dh(DH *dh) { json_t *jdh = NULL; json_t *jbn = NULL; + char *s=NULL; if ((!dh) || (!dh->p) || (!dh->g) || (!dh->pub_key)) return NULL; jdh = json_object(); - jbn = json_string(BN_bn2hex(dh->p)); + jbn = json_string(s=BN_bn2hex(dh->p)); + OPENSSL_free(s); json_object_set_new(jdh, "dh_p", jbn); - jbn = json_string(BN_bn2hex(dh->g)); + jbn = json_string(s=BN_bn2hex(dh->g)); + OPENSSL_free(s); json_object_set_new(jdh, "dh_g", jbn); - jbn = json_string(BN_bn2hex(dh->pub_key)); + jbn = json_string(s=BN_bn2hex(dh->pub_key)); + OPENSSL_free(s); json_object_set_new(jdh, "dh_pub_key", jbn); return jdh; @@ -181,19 +187,17 @@ static DH *tr_msg_decode_dh(json_t *jdh) json_t *jg = NULL; json_t *jpub_key = NULL; - if (!(dh = malloc(sizeof(DH)))) { + if (!(dh=tr_dh_new())) { tr_crit("tr_msg_decode_dh(): Error allocating DH structure."); return NULL; } - memset(dh, 0, sizeof(DH)); - /* store required fields from dh object */ if ((NULL == (jp = json_object_get(jdh, "dh_p"))) || (NULL == (jg = json_object_get(jdh, "dh_g"))) || (NULL == (jpub_key = json_object_get(jdh, "dh_pub_key")))) { tr_debug("tr_msg_decode_dh(): Error parsing dh_info."); - free(dh); + tr_dh_free(dh); return NULL; } diff --git a/include/tr_util.h b/include/tr_util.h index 3b0e597..372c8ec 100644 --- a/include/tr_util.h +++ b/include/tr_util.h @@ -37,8 +37,8 @@ #include -void tr_bin_to_hex(const unsigned char * bin, size_t binlen, - char * hex_out, size_t hex_len); +TR_EXPORT void tr_bin_to_hex(const unsigned char * bin, size_t binlen, + char * hex_out, size_t hex_len); TR_EXPORT int tr_cmp_timespec(struct timespec *ts1, struct timespec *ts2); #endif /* TR_UTIL_H */ diff --git a/include/trust_router/tr_dh.h b/include/trust_router/tr_dh.h index dc4fbd9..7de6249 100644 --- a/include/trust_router/tr_dh.h +++ b/include/trust_router/tr_dh.h @@ -40,14 +40,14 @@ #include #include - +TR_EXPORT DH *tr_dh_new(void); +TR_EXPORT void tr_dh_free(DH *dh); TR_EXPORT DH *tr_create_dh_params(unsigned char *key, size_t len); TR_EXPORT DH *tr_create_matching_dh(unsigned char *key, size_t len, DH *in_dh); TR_EXPORT void tr_destroy_dh_params(DH *dh); -TR_EXPORT DH *tr_dup_dh_params(DH *dh); +TR_EXPORT DH *tr_dh_dup(DH *in); TR_EXPORT int tr_compute_dh_key(unsigned char **pbuf, BIGNUM *pub_key, DH *priv_dh); -TR_EXPORT void tr_dh_free(unsigned char *dh_buf); int TR_EXPORT tr_dh_pub_hash(TID_REQ *request, unsigned char **out_digest, size_t *out_llen); diff --git a/tid/example/tidc_main.c b/tid/example/tidc_main.c index b4a7ece..92056c2 100644 --- a/tid/example/tidc_main.c +++ b/tid/example/tidc_main.c @@ -190,7 +190,7 @@ int main (int argc, printf("TIDC Client:\nServer = %s, rp_realm = %s, target_realm = %s, community = %s, port = %i\n", opts.server, opts.rp_realm, opts.target_realm, opts.community, opts.port); /* Create a TID client instance & the client DH */ - tidc = tidc_create(); + tidc = tidc_create(NULL); if (NULL == (tidc->client_dh = tr_create_dh_params(NULL, 0))) { printf("Error creating client DH params.\n"); return 1; diff --git a/tid/tid_resp.c b/tid/tid_resp.c index ad2f28f..3970616 100644 --- a/tid/tid_resp.c +++ b/tid/tid_resp.c @@ -203,7 +203,7 @@ static int tid_srvr_blk_destructor(void *obj) return 0; } -TID_SRVR_BLK *tid_srvr_blk_new(TALLOC_CTX *mem_ctx) +TR_EXPORT TID_SRVR_BLK *tid_srvr_blk_new(TALLOC_CTX *mem_ctx) { TID_SRVR_BLK *srvr=talloc(mem_ctx, TID_SRVR_BLK); @@ -219,12 +219,12 @@ TID_SRVR_BLK *tid_srvr_blk_new(TALLOC_CTX *mem_ctx) return srvr; } -void tid_srvr_blk_free(TID_SRVR_BLK *srvr) +TR_EXPORT void tid_srvr_blk_free(TID_SRVR_BLK *srvr) { talloc_free(srvr); } -TID_SRVR_BLK *tid_srvr_blk_dup(TALLOC_CTX *mem_ctx, TID_SRVR_BLK *srvr) +TR_EXPORT TID_SRVR_BLK *tid_srvr_blk_dup(TALLOC_CTX *mem_ctx, TID_SRVR_BLK *srvr) { TID_SRVR_BLK *new=NULL; @@ -236,7 +236,7 @@ TID_SRVR_BLK *tid_srvr_blk_dup(TALLOC_CTX *mem_ctx, TID_SRVR_BLK *srvr) if (srvr->aaa_server_addr!=NULL) new->aaa_server_addr=talloc_strdup(new, srvr->aaa_server_addr); new->key_name=tr_dup_name(srvr->key_name); - new->aaa_server_dh=tr_dup_dh_params(srvr->aaa_server_dh); + 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); @@ -246,7 +246,7 @@ TID_SRVR_BLK *tid_srvr_blk_dup(TALLOC_CTX *mem_ctx, TID_SRVR_BLK *srvr) } /* use the macro */ -TID_SRVR_BLK *tid_srvr_blk_add_func(TID_SRVR_BLK *head, TID_SRVR_BLK *new) +TR_EXPORT TID_SRVR_BLK *tid_srvr_blk_add_func(TID_SRVR_BLK *head, TID_SRVR_BLK *new) { TID_SRVR_BLK *this=head; @@ -259,7 +259,7 @@ TID_SRVR_BLK *tid_srvr_blk_add_func(TID_SRVR_BLK *head, TID_SRVR_BLK *new) return head; } -void tid_srvr_blk_set_path(TID_SRVR_BLK *block, json_t *path) +TR_EXPORT void tid_srvr_blk_set_path(TID_SRVR_BLK *block, json_t *path) { if (block->path!=NULL) json_decref(block->path); @@ -268,14 +268,14 @@ void tid_srvr_blk_set_path(TID_SRVR_BLK *block, json_t *path) json_incref(block->path); } -const json_t *tid_srvr_get_path( const TID_SRVR_BLK *block) +TR_EXPORT const json_t *tid_srvr_get_path( const TID_SRVR_BLK *block) { if (!block) return NULL; return block->path; } -void tid_resp_set_cons(TID_RESP *resp, TR_CONSTRAINT_SET *cons) +TR_EXPORT void tid_resp_set_cons(TID_RESP *resp, TR_CONSTRAINT_SET *cons) { json_t *jc=(json_t *)cons; @@ -287,7 +287,7 @@ void tid_resp_set_cons(TID_RESP *resp, TR_CONSTRAINT_SET *cons) json_incref(jc); } -void tid_resp_set_error_path(TID_RESP *resp, json_t *ep) +TR_EXPORT void tid_resp_set_error_path(TID_RESP *resp, json_t *ep) { if (resp->error_path!=NULL) json_decref(resp->error_path); @@ -296,14 +296,14 @@ void tid_resp_set_error_path(TID_RESP *resp, json_t *ep) json_incref(resp->error_path); } -json_t *tid_resp_get_error_path(const TID_RESP *resp) +TR_EXPORT json_t *tid_resp_get_error_path(const TID_RESP *resp) { if (!resp) return NULL; return resp->error_path; } -json_t *tid_resp_get_a_path(const TID_RESP *const_resp) +TR_EXPORT json_t *tid_resp_get_a_path(const TID_RESP *const_resp) { size_t index; TID_SRVR_BLK *server; diff --git a/tid/tidc.c b/tid/tidc.c index fcc78c9..de3295b 100644 --- a/tid/tidc.c +++ b/tid/tidc.c @@ -119,7 +119,7 @@ int tidc_send_request (TIDC_INSTANCE *tidc, goto error; } - tid_req->tidc_dh = tidc->client_dh; + tid_req->tidc_dh = tr_dh_dup(tidc->client_dh); rc = tidc_fwd_request(tidc, tid_req, resp_handler, cookie); goto cleanup; diff --git a/tr/tr_tid.c b/tr/tr_tid.c index ab836d3..27c7a45 100644 --- a/tr/tr_tid.c +++ b/tr/tr_tid.c @@ -185,7 +185,7 @@ static void *tr_tids_req_fwd_thread(void *arg) goto cleanup; } cookie->thread_id=args->thread_id; - if (0 > (rc = tidc_fwd_request(tidc, args->fwd_req, &tr_tidc_resp_handler, (void *)cookie))) { + 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; goto cleanup; @@ -458,7 +458,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_dup_dh_params(orig_req->tidc_dh); + 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); /* Take the cookie out of tmp_ctx before starting thread. If thread starts, it becomes -- 2.1.4