Browse Source

bug fixes, clean valgrind output

cecylia 7 years ago
parent
commit
e7a8636ef4
4 changed files with 62 additions and 19 deletions
  1. 42 13
      relay_station/crypto.c
  2. 8 2
      relay_station/flow.c
  3. 1 2
      relay_station/relay.c
  4. 11 2
      relay_station/slitheen-proxy.c

+ 42 - 13
relay_station/crypto.c

@@ -419,6 +419,8 @@ int verify_finish_hash(flow *f, uint8_t *hs, int32_t incoming){
 		//replace existing MAC with modified one
 		memcpy(p, output, fin_length);
 
+		free(extra_input);
+
 	}
 
 	free(output);
@@ -639,7 +641,7 @@ err:
 	if((pub_key != NULL) && (dh_srvr == NULL)){
 		BN_free(pub_key);
 	}
-	if((priv_key != NULL) && (dh_clnt == NULL) && (EC_KEY_get0_private_key(clnt_ecdh) == NULL)){
+	if((priv_key != NULL) && ((dh_clnt == NULL) || (EC_KEY_get0_private_key(clnt_ecdh) == NULL))){
 		BN_free(priv_key);
 	}
 
@@ -875,8 +877,7 @@ int init_ciphers(flow *f){
 	if(c == NULL){
 		/*This *shouldn't* happen, but might if a serverHello msg isn't received
 		 * or if a session is resumed in a strange way */
-		remove_flow(f);
-		return 0;
+		return 1;
 	}
 
 	/* Generate Keys */
@@ -1140,8 +1141,10 @@ void generate_client_super_keys(uint8_t *secret, client *c){
 
 int super_encrypt(client *c, uint8_t *data, uint32_t len){
 
-	EVP_CIPHER_CTX *hdr_ctx;
-	EVP_CIPHER_CTX *bdy_ctx;
+	int retval = 1;
+
+	EVP_CIPHER_CTX *hdr_ctx = NULL;
+	EVP_CIPHER_CTX *bdy_ctx = NULL;
 	
 	int32_t out_len;
 	size_t mac_len;
@@ -1164,7 +1167,8 @@ int super_encrypt(client *c, uint8_t *data, uint32_t len){
 	
 	if(!EVP_CipherUpdate(hdr_ctx, p, &out_len, p, SLITHEEN_HEADER_LEN)){
 		printf("Failed!\n");
-		return 0;
+		retval = 0;
+		goto end;
 	}
 
 #ifdef DEBUG
@@ -1176,7 +1180,8 @@ int super_encrypt(client *c, uint8_t *data, uint32_t len){
 #endif
 
 	if(len == 0){ //only encrypt header: body contains garbage bytes
-		return 1;
+		retval = 1;
+		goto end;
 	}
 
 	//encrypt the body
@@ -1202,7 +1207,8 @@ int super_encrypt(client *c, uint8_t *data, uint32_t len){
 
 	if(!EVP_CipherUpdate(bdy_ctx, p, &out_len, p, len)){
 		printf("Failed!\n");
-		return 0;
+		goto end;
+		retval = 0;
 	}
 
 #ifdef DEBUG
@@ -1215,17 +1221,40 @@ int super_encrypt(client *c, uint8_t *data, uint32_t len){
 #endif
 	
 	//MAC at the end
-	EVP_DigestSignUpdate(c->mac_ctx, p, out_len);
+	EVP_MD_CTX mac_ctx;
+	EVP_MD_CTX_init(&mac_ctx);
+
+	EVP_MD_CTX_copy_ex(&mac_ctx, c->mac_ctx);
+
+	EVP_DigestSignUpdate(&mac_ctx, p, out_len);
 
-	EVP_DigestSignFinal(c->mac_ctx, output, &mac_len);
+	EVP_DigestSignFinal(&mac_ctx, output, &mac_len);
+
+	EVP_MD_CTX_cleanup(&mac_ctx);
 
 	p += out_len;
 	memcpy(p, output, 16);
 
-	EVP_CIPHER_CTX_free(bdy_ctx);
-	EVP_CIPHER_CTX_free(hdr_ctx);
+#ifdef DEBUG_PARSE
+    printf("Computed mac:\n");
+    for(int i=0; i< 16; i++){
+        printf("%02x ", output[i]);
+    }   
+    printf("\n");
+    fflush(stdout);
+#endif
 
-	return 1;
+end:
+	if(hdr_ctx != NULL){
+		EVP_CIPHER_CTX_cleanup(hdr_ctx);
+		OPENSSL_free(hdr_ctx);
+	}
+	if(bdy_ctx != NULL){
+		EVP_CIPHER_CTX_cleanup(bdy_ctx);
+		OPENSSL_free(bdy_ctx);
+	}
+
+	return retval;
 }
 
 /** Checks a handshake message to see if it is tagged or a

+ 8 - 2
relay_station/flow.c

@@ -121,6 +121,7 @@ flow *add_flow(struct packet_info *info) {
 	const EVP_MD *md = EVP_sha384();
 	EVP_DigestInit_ex(new_flow->finish_md_ctx, md, NULL);
 
+	new_flow->cipher = NULL;
 	new_flow->clnt_read_ctx = NULL;
 	new_flow->clnt_write_ctx = NULL;
 	new_flow->srvr_read_ctx = NULL;
@@ -389,7 +390,6 @@ int update_flow(flow *f, uint8_t *record, uint8_t incoming) {
 					printf("Error? (%x:%d -> %x:%d)...\n", f->src_ip.s_addr, ntohs(f->src_port), f->dst_ip.s_addr, ntohs(f->dst_port));
 					remove_flow(f);
 					goto err;
-					break;
 			}
 			break;
 		case APP:
@@ -431,6 +431,7 @@ int update_flow(flow *f, uint8_t *record, uint8_t incoming) {
 		default:
 			printf("Error: Not a Record (%x:%d -> %x:%d)\n", f->src_ip.s_addr, ntohs(f->src_port), f->dst_ip.s_addr, ntohs(f->dst_port));
 			fflush(stdout);
+			remove_flow(f);
 			goto err;
 	}
 	return 0;
@@ -458,6 +459,7 @@ int remove_flow(flow *f) {
 		free(tmp);
 		tmp = f->upstream_app_data->first_packet;
 	}
+	free(f->upstream_app_data);
 
 	tmp = f->downstream_app_data->first_packet;
 	while(tmp != NULL){
@@ -466,6 +468,7 @@ int remove_flow(flow *f) {
 		free(tmp);
 		tmp = f->downstream_app_data->first_packet;
 	}
+	free(f->downstream_app_data);
 
 	//Clean up cipher ctxs
 	EVP_MD_CTX_cleanup(f->finish_md_ctx);
@@ -1073,7 +1076,10 @@ int add_packet(flow *f, struct packet_info *info){
 						free(copy_info->app_data);
 						free(copy_info);
 					} else {
-						update_flow(f, record, incoming);
+						if(update_flow(f, record, incoming)){
+							free(record);
+							return 1;//error occurred and flow was removed
+						}
 
 						//check to see if last finished message received
 						if(f->application ==1){

+ 1 - 2
relay_station/relay.c

@@ -172,7 +172,7 @@ int read_header(flow *f, struct packet_info *info){
 		//check to see if the new record is too long
 		record_hdr = (struct record_header*) p;
 		record_length = RECORD_LEN(record_hdr);
-		if(record_length > info->app_data_len){
+		if(record_length + RECORD_HEADER_LEN > info->app_data_len){
 
 			//add info to upstream queue
 			queue_block *new_block = emalloc(sizeof(queue_block));
@@ -182,7 +182,6 @@ int read_header(flow *f, struct packet_info *info){
 			memcpy(block_data, p, info->app_data_len);
 
 			new_block->len = info->app_data_len;
-			new_block->offset = record_length; //re-appropriate this for len of record
 			new_block->data = block_data;
 			new_block->next = NULL;
 

+ 11 - 2
relay_station/slitheen-proxy.c

@@ -282,8 +282,8 @@ void process_packet(struct packet_info *info){
 						data_to_fill -= data_to_fill;
 					} else {
 						p += saved_data->seq_num - seq_num;
-						seq_num += saved_data->seq_num - seq_num;
 						data_to_fill -= saved_data->seq_num - seq_num;
+						seq_num += saved_data->seq_num - seq_num;
 					}
 				} else if ( seq_num == saved_data->seq_num) {
 
@@ -329,6 +329,8 @@ void process_packet(struct packet_info *info){
 
 		if(data_to_process){
 
+			uint8_t removed = 0;
+
 			if(p != info->app_data){
 				printf("UH OH something weird might happen\n");
 			}
@@ -338,7 +340,9 @@ void process_packet(struct packet_info *info){
 			} else {
 
 				/* Pass data to packet chain */
-				add_packet(observed, info);
+				if(add_packet(observed, info)){//removed_flow
+					removed = 1;
+				}
 			}
 
 			/* Update TCP state */
@@ -348,6 +352,11 @@ void process_packet(struct packet_info *info){
 			} else {
 				/* add packet to application data queue */
 
+				//check if flow was removed
+				if(removed){
+					return;
+				}
+
 				//add new app block
 				packet *new_block = ecalloc(1, sizeof(packet));
 				new_block->seq_num = seq_num;