Blob Blame Raw
From 0897d5131489c7eac21d558625c30d23b0a1774d Mon Sep 17 00:00:00 2001
From: Your Name <you@example.com>
Date: Tue, 14 Apr 2020 13:17:14 +0000
Subject: [PATCH 35/39] Cleanup the SCEP helper curl and talloc contexts when
 finished

The talloc context was freed in only a few cases and the curl
context was never freed.
---
 src/scep.c     | 127 ++++++++++++++++++++++++++++++++-----------------
 src/submit-h.c |  15 +++++-
 src/submit-h.h |   1 +
 3 files changed, 97 insertions(+), 46 deletions(-)

diff --git a/src/scep.c b/src/scep.c
index 0b8bef9..4d00692 100644
--- a/src/scep.c
+++ b/src/scep.c
@@ -199,7 +199,7 @@ int
 main(int argc, const char **argv)
 {
 	const char *url = NULL, *results = NULL, *results2 = NULL;
-	struct cm_submit_h_context *hctx;
+	struct cm_submit_h_context *hctx = NULL;
 	int c, verbose = 0, results_length = 0, results_length2 = 0, i;
 	int prefer_non_renewal = 0, can_renewal = 0;
 	int response_code = 0, response_code2 = 0;
@@ -225,7 +225,8 @@ main(int argc, const char **argv)
 	size_t payload_length;
 	long error;
 	PKCS7 *p7;
-	poptContext pctx;
+	int rval = CM_SUBMIT_STATUS_UNCONFIGURED;
+	poptContext pctx = NULL;
 	struct poptOption popts[] = {
 		{"url", 'u', POPT_ARG_STRING, &url, 0, "service location", "URL"},
 		{"ca-identifier", 'i', POPT_ARG_STRING, &id, 0, "name to use when querying for capabilities", "IDENTIFIER"},
@@ -388,8 +389,8 @@ main(int argc, const char **argv)
 			}
 			if ((message == NULL) || (strlen(message) == 0)) {
 				printf(_("Error reading request.  Expected PKCS7 data containing a GetInitialCert pkiMessage, got nothing.\n"));
-				free(cainfo);
-				return CM_SUBMIT_STATUS_NEED_SCEP_MESSAGES;
+				rval = CM_SUBMIT_STATUS_NEED_SCEP_MESSAGES;
+				goto done;
 			}
 			/* First step: read capabilities for our use. */
 			params = talloc_asprintf(ctx, "operation=" OP_GET_CA_CAPS);
@@ -408,8 +409,8 @@ main(int argc, const char **argv)
 			}
 			if ((message == NULL) || (strlen(message) == 0)) {
 				printf(_("Error reading request.  Expected PKCS7 data containing a PKCSReq pkiMessage, got nothing.\n"));
-				free(cainfo);
-				return CM_SUBMIT_STATUS_NEED_SCEP_MESSAGES;
+				rval = CM_SUBMIT_STATUS_NEED_SCEP_MESSAGES;
+				goto done;
 			}
 			/* First step: read capabilities for our use. */
 			params = talloc_asprintf(ctx, "operation=" OP_GET_CA_CAPS);
@@ -420,8 +421,8 @@ main(int argc, const char **argv)
 	/* Supply help output, if it's needed. */
 	if (missing_args) {
 		poptPrintUsage(pctx, stdout, 0);
-		free(cainfo);
-		return CM_SUBMIT_STATUS_UNCONFIGURED;
+		rval = CM_SUBMIT_STATUS_UNCONFIGURED;
+		goto done;
 	}
 
 	/* Check the rekey PKCSReq message, if we have one. */
@@ -505,7 +506,6 @@ main(int argc, const char **argv)
 				verbose > 1 ?
 				cm_submit_h_curl_verbose_on :
 				cm_submit_h_curl_verbose_off);
-	free(cainfo);
 	cm_submit_h_run(hctx);
 	content_type = cm_submit_h_result_type(hctx);
 	if (content_type == NULL) {
@@ -551,7 +551,8 @@ main(int argc, const char **argv)
 		}
 		if ((tmp2 == NULL) || (strlen(tmp2) == 0)) {
 			printf(_("Error reading request.  Expected PKCS7 data containing a GetInitialCert pkiMessage, got nothing.\n"));
-			return CM_SUBMIT_STATUS_NEED_SCEP_MESSAGES;
+			rval = CM_SUBMIT_STATUS_NEED_SCEP_MESSAGES;
+			goto done;
 		} else
 		if (verbose > 0) {
 			if (tmp2 == rekey_message) {
@@ -576,7 +577,8 @@ main(int argc, const char **argv)
 		}
 		if ((tmp2 == NULL) || (strlen(tmp2) == 0)) {
 			printf(_("Error reading request.  Expected PKCS7 data containing a PKCSReq pkiMessage, got nothing.\n"));
-			return CM_SUBMIT_STATUS_NEED_SCEP_MESSAGES;
+			rval = CM_SUBMIT_STATUS_NEED_SCEP_MESSAGES;
+			goto done;
 		} else
 		if (verbose > 0) {
 			if (tmp2 == rekey_message) {
@@ -638,7 +640,8 @@ main(int argc, const char **argv)
 			       cm_submit_h_result_code(hctx),
 			       url);
 		}
-		return CM_SUBMIT_STATUS_UNREACHABLE;
+		rval = CM_SUBMIT_STATUS_UNREACHABLE;
+		goto done;
 	}
 	switch (op) {
 	case op_unset:
@@ -651,16 +654,19 @@ main(int argc, const char **argv)
 			       response_code, url);
 			if (response_code == 500) {
 				/* The server might recover, right? */
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			} else {
 				/* Maybe not? */
-				return CM_SUBMIT_STATUS_REJECTED;
+				rval = CM_SUBMIT_STATUS_REJECTED;
+				goto done;
 			}
 		}
 		if (results == NULL) {
 			printf(_("Internal error: no response to \"%s?%s\".\n"),
 			       url, params);
-			return CM_SUBMIT_STATUS_REJECTED;
+			rval = CM_SUBMIT_STATUS_REJECTED;
+			goto done;
 		}
 		break;
 	case op_get_cert_initial:
@@ -685,10 +691,12 @@ main(int argc, const char **argv)
 				fprintf(stderr, "Result is surprisingly large, "
 					"suppressing it.\n");
 			}
-			return CM_SUBMIT_STATUS_REJECTED;
+			rval = CM_SUBMIT_STATUS_REJECTED;
+			goto done;
 		}
 		printf("%s\n", results);
-		return CM_SUBMIT_STATUS_ISSUED;
+		rval = CM_SUBMIT_STATUS_ISSUED;
+		goto done;
 		break;
 	case op_get_ca_certs:
 		if ((strcasecmp(content_type,
@@ -697,7 +705,8 @@ main(int argc, const char **argv)
 				"application/x-x509-ca-ra-cert") != 0)) {
 			printf(_("Server reply was of unexpected MIME type "
 				 "\"%s\".\n"), content_type);
-			return CM_SUBMIT_STATUS_UNREACHABLE;
+			rval = CM_SUBMIT_STATUS_UNREACHABLE;
+			goto done;
 		}
 		if (racert == NULL) {
 			racertp = &racert;
@@ -710,7 +719,8 @@ main(int argc, const char **argv)
 						 n_buffers + 1);
 			if ((buffers == NULL) || (lengths == NULL)) {
 				fprintf(stderr, "Out of memory.\n");
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			buffers[n_buffers] = (unsigned char *) racert;
 			lengths[n_buffers] = strlen(racert);
@@ -727,7 +737,8 @@ main(int argc, const char **argv)
 						 n_buffers + 1);
 			if ((buffers == NULL) || (lengths == NULL)) {
 				fprintf(stderr, "Out of memory.\n");
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			buffers[n_buffers] = (unsigned char *) cacert;
 			lengths[n_buffers] = strlen(cacert);
@@ -741,7 +752,8 @@ main(int argc, const char **argv)
 						 n_buffers + 1);
 			if ((buffers == NULL) || (lengths == NULL)) {
 				fprintf(stderr, "Out of memory.\n");
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			buffers[n_buffers] = (unsigned char *) results;
 			lengths[n_buffers] = results_length;
@@ -755,7 +767,8 @@ main(int argc, const char **argv)
 						 n_buffers + 1);
 			if ((buffers == NULL) || (lengths == NULL)) {
 				fprintf(stderr, "Out of memory.\n");
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			buffers[n_buffers] = (unsigned char *) results2;
 			lengths[n_buffers] = results_length2;
@@ -850,7 +863,8 @@ main(int argc, const char **argv)
 						 n_buffers + 1);
 			if ((buffers == NULL) || (lengths == NULL)) {
 				fprintf(stderr, "Out of memory.\n");
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			buffers[n_buffers] = (unsigned char *) results2;
 			lengths[n_buffers] = results_length2;
@@ -882,11 +896,11 @@ main(int argc, const char **argv)
 					}
 				}
 			}
-			talloc_free(ctx);
-			return CM_SUBMIT_STATUS_ISSUED;
+			rval = CM_SUBMIT_STATUS_ISSUED;
+			goto done;
 		} else {
-			talloc_free(ctx);
-			return CM_SUBMIT_STATUS_UNREACHABLE;
+			rval = CM_SUBMIT_STATUS_UNREACHABLE;
+			goto done;
 		}
 		break;
 	case op_get_cert_initial:
@@ -957,42 +971,50 @@ main(int argc, const char **argv)
 				fprintf(stderr, "%s", s);
 				cm_log(1, "%s", s);
 				free(s);
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			if ((msgtype == NULL) ||
 			    (strcmp(msgtype, SCEP_MSGTYPE_CERTREP) != 0)) {
 				printf(_("Error: reply was not a CertRep (%s).\n"),
 				       msgtype ? msgtype : "none");
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			if (tx == NULL) {
 				printf(_("Error: reply is missing transactionId.\n"));
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			if (sent_tx != NULL) {
 				if (strcmp(sent_tx, tx) != 0) {
 					printf(_("Error: reply contains a "
 						 "different transactionId.\n"));
-					return CM_SUBMIT_STATUS_UNREACHABLE;
+					rval = CM_SUBMIT_STATUS_UNREACHABLE;
+					goto done;
 				}
 			}
 			if (pkistatus == NULL) {
 				printf(_("Error: reply is missing pkiStatus.\n"));
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			if (recipient_nonce == NULL) {
 				printf(_("Error: reply is missing recipientNonce.\n"));
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			if ((recipient_nonce_length != sent_nonce_length) ||
 			    (memcmp(recipient_nonce, sent_nonce,
 				    sent_nonce_length) != 0)) {
 				printf(_("Error: reply nonce doesn't match request.\n"));
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			if (sender_nonce == NULL) {
 				printf(_("Error: reply is missing senderNonce.\n"));
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 			if (strcmp(pkistatus, SCEP_PKISTATUS_PENDING) == 0) {
 				if (verbose > 0) {
@@ -1002,7 +1024,8 @@ main(int argc, const char **argv)
 				s = cm_store_base64_from_bin(ctx, sender_nonce,
 							     sender_nonce_length);
 				printf("%s\n", s);
-				return CM_SUBMIT_STATUS_WAIT;
+				rval = CM_SUBMIT_STATUS_WAIT;
+				goto done;
 			} else
 			if (strcmp(pkistatus, SCEP_PKISTATUS_FAILURE) == 0) {
 				if (verbose > 0) {
@@ -1050,7 +1073,8 @@ main(int argc, const char **argv)
 					printf(_("Server returned failure code \"%s\".\n"),
 					       failinfo);
 				}
-				return CM_SUBMIT_STATUS_REJECTED;
+				rval = CM_SUBMIT_STATUS_REJECTED;
+				goto done;
 			} else
 			if (strcmp(pkistatus, SCEP_PKISTATUS_SUCCESS) == 0) {
 				if (verbose > 0) {
@@ -1067,7 +1091,8 @@ main(int argc, const char **argv)
 					s = cm_submit_u_pem_from_base64("PKCS7", 0, s);
 					fprintf(stderr, "Full reply:\n%s", s);
 					free(s);
-					return CM_SUBMIT_STATUS_UNREACHABLE;
+					rval = CM_SUBMIT_STATUS_UNREACHABLE;
+					goto done;
 				}
 				if (!PKCS7_type_is_enveloped(p7)) {
 					printf(_("Error: signed-data payload is not enveloped-data.\n"));
@@ -1079,7 +1104,8 @@ main(int argc, const char **argv)
 					s = cm_submit_u_pem_from_base64("PKCS7", 0, s);
 					fprintf(stderr, "Full reply:\n%s", s);
 					free(s);
-					return CM_SUBMIT_STATUS_UNREACHABLE;
+					rval = CM_SUBMIT_STATUS_UNREACHABLE;
+					goto done;
 				}
 				if ((p7->d.enveloped == NULL) ||
 				    (p7->d.enveloped->enc_data == NULL) ||
@@ -1094,29 +1120,42 @@ main(int argc, const char **argv)
 					s = cm_submit_u_pem_from_base64("PKCS7", 0, s);
 					fprintf(stderr, "Full reply:\n%s", s);
 					free(s);
-					return CM_SUBMIT_STATUS_UNREACHABLE;
+					rval = CM_SUBMIT_STATUS_UNREACHABLE;
+					goto done;
 				}
 				s = cm_store_base64_from_bin(ctx, payload,
 							     payload_length);
 				s = cm_submit_u_pem_from_base64("PKCS7", 0, s);
 				printf("%s", s);
 				free(s);
-				return CM_SUBMIT_STATUS_ISSUED;
+				rval = CM_SUBMIT_STATUS_ISSUED;
+				goto done;
 			} else {
 				if (verbose > 0) {
 					fprintf(stderr, "SCEP status is \"%s\".\n", pkistatus);
 				}
 				printf(_("Error: pkiStatus \"%s\" not recognized.\n"),
 				       pkistatus);
-				return CM_SUBMIT_STATUS_UNREACHABLE;
+				rval = CM_SUBMIT_STATUS_UNREACHABLE;
+				goto done;
 			}
 		} else {
 			printf(_("Server reply was of unexpected MIME type "
 				 "\"%s\".\n"), content_type);
 			printf("Full reply:\n%.*s", results_length2, results2);
-			return CM_SUBMIT_STATUS_UNREACHABLE;
+			rval = CM_SUBMIT_STATUS_UNREACHABLE;
+			goto done;
 		}
 		break;
 	}
-	return CM_SUBMIT_STATUS_UNCONFIGURED;
+
+done:
+	if (pctx) {
+		poptFreeContext(pctx);
+	}
+	free(cainfo);
+	free(id);
+	cm_submit_h_cleanup(hctx);
+	talloc_free(ctx);
+	return rval;
 }
diff --git a/src/submit-h.c b/src/submit-h.c
index 33f9b39..9b507db 100644
--- a/src/submit-h.c
+++ b/src/submit-h.c
@@ -298,6 +298,15 @@ cm_submit_h_result_type(struct cm_submit_h_context *ctx)
 	return ret;
 }
 
+void
+cm_submit_h_cleanup(struct cm_submit_h_context *ctx)
+{
+
+	if (ctx != NULL && ctx->curl != NULL) {
+		curl_easy_cleanup(ctx->curl);
+	}
+}
+
 #ifdef CM_SUBMIT_H_MAIN
 int
 main(int argc, const char **argv)
@@ -307,7 +316,7 @@ main(int argc, const char **argv)
 	enum cm_submit_h_opt_negotiate negotiate;
 	enum cm_submit_h_opt_delegate negotiate_delegate;
 	enum cm_submit_h_opt_clientauth clientauth;
-	int c, fd, l, verbose = 0, length = 0;
+	int c, fd, l, verbose = 0, length = 0, rval = 0;
 	char *ctype, *accept, *capath, *cainfo, *sslcert, *sslkey, *sslpass;
 	char *pinfile;
 	const char *method, *url;
@@ -423,6 +432,8 @@ main(int argc, const char **argv)
 			cm_submit_h_result_code(ctx),
 			cm_submit_h_result_code_text(ctx));
 	}
-	return cm_submit_h_result_code(ctx);
+	rval = cm_submit_h_result_code(ctx);
+	cm_submit_h_cleanup(ctx);
+	return rval;
 }
 #endif
diff --git a/src/submit-h.h b/src/submit-h.h
index 1283c53..931cc89 100644
--- a/src/submit-h.h
+++ b/src/submit-h.h
@@ -61,5 +61,6 @@ int cm_submit_h_result_code(struct cm_submit_h_context *ctx);
 const char *cm_submit_h_result_code_text(struct cm_submit_h_context *ctx);
 const char *cm_submit_h_results(struct cm_submit_h_context *ctx, int *length);
 const char *cm_submit_h_result_type(struct cm_submit_h_context *ctx);
+void cm_submit_h_cleanup(struct cm_submit_h_context *ctx);
 
 #endif
-- 
2.21.1