Fix segfaults related to copying DH parameters.
authorJennifer Richards <jennifer@painless-security.com>
Thu, 1 Dec 2016 21:21:40 +0000 (16:21 -0500)
committerJennifer Richards <jennifer@painless-security.com>
Thu, 1 Dec 2016 21:21:40 +0000 (16:21 -0500)
common/tr_dh.c
common/tr_msg.c
include/tr_util.h
include/trust_router/tr_dh.h
tid/example/tidc_main.c
tid/tid_resp.c
tid/tidc.c
tr/tr_tid.c

index c76ab27..257b77c 100644 (file)
@@ -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);
 }
index 0d18466..d8b114b 100644 (file)
@@ -36,6 +36,7 @@
 #include <arpa/inet.h>
 #include <string.h>
 #include <openssl/dh.h>
+#include <openssl/crypto.h>
 #include <jansson.h>
 #include <assert.h>
 #include <talloc.h>
@@ -47,6 +48,7 @@
 #include <trust_router/tr_name.h>
 #include <trp_internal.h>
 #include <trust_router/tr_constraint.h>
+#include <trust_router/tr_dh.h>
 #include <tr_debug.h>
 
 /* 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;
   }
 
index 3b0e597..372c8ec 100644 (file)
@@ -37,8 +37,8 @@
 
 #include <trust_router/tr_versioning.h>
 
-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 */
index dc4fbd9..7de6249 100644 (file)
 #include <trust_router/tr_versioning.h>
 #include <trust_router/tid.h>
 
-
+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);
index b4a7ece..92056c2 100644 (file)
@@ -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;
index ad2f28f..3970616 100644 (file)
@@ -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;
index fcc78c9..de3295b 100644 (file)
@@ -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;
index ab836d3..27c7a45 100644 (file)
@@ -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