Cleanup libcrypto memory management. Remove redundant NULL checks

before calling *_free() functions.  Use 'get0' functions where it
makes sense to avoid some frees.

Feedback and ok tb@

3 files changed, 64 insertions, 118 deletions

diff --git a/sbin/iked/ca.c b/sbin/iked/ca.c
index a2b419018d7..b06247136f9 100644
--- a/sbin/iked/ca.c
+++ b/sbin/iked/ca.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ca.c,v 1.83 2021/12/08 19:17:35 tobhe Exp $	*/
+/*	$OpenBSD: ca.c,v 1.84 2021/12/13 17:35:34 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -187,10 +187,8 @@ ca_reset(struct privsep *ps)
 	    store->ca_pubkey.id_type == IKEV2_ID_NONE)
 		fatalx("ca_reset: keys not loaded");
 
-	if (store->ca_cas != NULL)
-		X509_STORE_free(store->ca_cas);
-	if (store->ca_certs != NULL)
-		X509_STORE_free(store->ca_certs);
+	X509_STORE_free(store->ca_cas);
+	X509_STORE_free(store->ca_certs);
 
 	if ((store->ca_cas = X509_STORE_new()) == NULL)
 		fatalx("ca_reset: failed to get ca store");
@@ -483,9 +481,8 @@ ca_getcert(struct iked *env, struct imsg *imsg)
 		cert = ca_by_subjectaltname(store->ca_certs, &id);
 		if (cert) {
 			log_debug("%s: found local cert", __func__);
-			if ((certkey = X509_get_pubkey(cert)) != NULL) {
+			if ((certkey = X509_get0_pubkey(cert)) != NULL) {
 				ret = ca_pubkey_serialize(certkey, &key);
-				EVP_PKEY_free(certkey);
 				if (ret == 0) {
 					ptr = ibuf_data(key.id_buf);
 					len = ibuf_length(key.id_buf);
@@ -1045,7 +1042,7 @@ ca_cert_local(struct iked *env, X509  *cert)
 	if ((localpub = ca_id_to_pkey(&store->ca_pubkey)) == NULL)
 		goto done;
 
-	if ((certkey = X509_get_pubkey(cert)) == NULL) {
+	if ((certkey = X509_get0_pubkey(cert)) == NULL) {
 		log_info("%s: no public key in cert", __func__);
 		goto done;
 	}
@@ -1057,10 +1054,8 @@ ca_cert_local(struct iked *env, X509  *cert)
 
 	ret = 1;
  done:
-	if (certkey != NULL)
-		EVP_PKEY_free(certkey);
-	if (localpub != NULL)
-		EVP_PKEY_free(localpub);
+	EVP_PKEY_free(localpub);
+
 	return (ret);
 }
 
@@ -1092,8 +1087,7 @@ ca_cert_info(const char *msg, X509 *cert)
 		log_info("%s: subject: %s", msg, buf);
 	ca_x509_subjectaltname_log(cert, msg);
 out:
-	if (rawserial)
-		BIO_free(rawserial);
+	BIO_free(rawserial);
 }
 
 int
@@ -1112,10 +1106,9 @@ ca_subjectpubkey_digest(X509 *x509, uint8_t *md, unsigned int *size)
 	 * that includes the public key type (eg. RSA) and the
 	 * public key value (see 3.7 of RFC7296).
 	 */
-	if ((pkey = X509_get_pubkey(x509)) == NULL)
+	if ((pkey = X509_get0_pubkey(x509)) == NULL)
 		return (-1);
 	buflen = i2d_PUBKEY(pkey, &buf);
-	EVP_PKEY_free(pkey);
 	if (buflen == 0)
 		return (-1);
 	if (!EVP_Digest(buf, buflen, md, size, EVP_sha1(), NULL)) {
@@ -1196,7 +1189,7 @@ ca_pubkey_serialize(EVP_PKEY *key, struct iked_id *id)
 		ibuf_release(id->id_buf);
 		id->id_buf = NULL;
 
-		if ((rsa = EVP_PKEY_get1_RSA(key)) == NULL)
+		if ((rsa = EVP_PKEY_get0_RSA(key)) == NULL)
 			goto done;
 		if ((len = i2d_RSAPublicKey(rsa, NULL)) <= 0)
 			goto done;
@@ -1218,7 +1211,7 @@ ca_pubkey_serialize(EVP_PKEY *key, struct iked_id *id)
 		ibuf_release(id->id_buf);
 		id->id_buf = NULL;
 
-		if ((ec = EVP_PKEY_get1_EC_KEY(key)) == NULL)
+		if ((ec = EVP_PKEY_get0_EC_KEY(key)) == NULL)
 			goto done;
 		if ((len = i2d_EC_PUBKEY(ec, NULL)) <= 0)
 			goto done;
@@ -1245,10 +1238,7 @@ ca_pubkey_serialize(EVP_PKEY *key, struct iked_id *id)
 
 	ret = 0;
  done:
-	if (rsa != NULL)
-		RSA_free(rsa);
-	if (ec != NULL)
-		EC_KEY_free(ec);
+
 	return (ret);
 }
 
@@ -1268,7 +1258,7 @@ ca_privkey_serialize(EVP_PKEY *key, struct iked_id *id)
 		ibuf_release(id->id_buf);
 		id->id_buf = NULL;
 
-		if ((rsa = EVP_PKEY_get1_RSA(key)) == NULL)
+		if ((rsa = EVP_PKEY_get0_RSA(key)) == NULL)
 			goto done;
 		if ((len = i2d_RSAPrivateKey(rsa, NULL)) <= 0)
 			goto done;
@@ -1290,7 +1280,7 @@ ca_privkey_serialize(EVP_PKEY *key, struct iked_id *id)
 		ibuf_release(id->id_buf);
 		id->id_buf = NULL;
 
-		if ((ec = EVP_PKEY_get1_EC_KEY(key)) == NULL)
+		if ((ec = EVP_PKEY_get0_EC_KEY(key)) == NULL)
 			goto done;
 		if ((len = i2d_ECPrivateKey(ec, NULL)) <= 0)
 			goto done;
@@ -1317,10 +1307,7 @@ ca_privkey_serialize(EVP_PKEY *key, struct iked_id *id)
 
 	ret = 0;
  done:
-	if (rsa != NULL)
-		RSA_free(rsa);
-	if (ec != NULL)
-		EC_KEY_free(ec);
+
 	return (ret);
 }
 
@@ -1354,14 +1341,11 @@ ca_id_to_pkey(struct iked_id *pubkey)
 	out = localkey;
 	localkey = NULL;
  done:
-	if (localkey != NULL)
-		EVP_PKEY_free(localkey);
-	if (localrsa != NULL)
-		RSA_free(localrsa);
-	if (localec != NULL)
-		EC_KEY_free(localec);
-	if (rawcert != NULL)
-		BIO_free(rawcert);
+	EVP_PKEY_free(localkey);
+	RSA_free(localrsa);
+	EC_KEY_free(localec);
+	BIO_free(rawcert);
+
 	return (out);
 }
 
@@ -1403,11 +1387,8 @@ ca_privkey_to_method(struct iked_id *privkey)
 	    print_map(method, ikev2_auth_map));
 
  out:
-	if (ec != NULL)
-		EC_KEY_free(ec);
-	if (rawcert != NULL)
-		BIO_free(rawcert);
-
+	EC_KEY_free(ec);
+	BIO_free(rawcert);
 	return (method);
 }
 
@@ -1630,19 +1611,13 @@ ca_validate_pubkey(struct iked *env, struct iked_static_id *id,
 		ca_sslerror(__func__);
  done:
 	ibuf_release(idp.id_buf);
-	if (localkey != NULL)
-		EVP_PKEY_free(localkey);
-	if (peerrsa != NULL)
-		RSA_free(peerrsa);
-	if (peerec != NULL)
-		EC_KEY_free(peerec);
-	if (localrsa != NULL)
-		RSA_free(localrsa);
-	if (rawcert != NULL) {
-		BIO_free(rawcert);
-		if (peerkey != NULL)
-			EVP_PKEY_free(peerkey);
-	}
+	EVP_PKEY_free(localkey);
+	RSA_free(peerrsa);
+	EC_KEY_free(peerec);
+	RSA_free(localrsa);
+	BIO_free(rawcert);
+	if (len > 0)
+		EVP_PKEY_free(peerkey);
 
 	return (ret);
 }
@@ -1682,12 +1657,11 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id,
 	}
 
 	if (id != NULL) {
-		if ((pkey = X509_get_pubkey(cert)) == NULL) {
+		if ((pkey = X509_get0_pubkey(cert)) == NULL) {
 			errstr = "no public key in cert";
 			goto done;
 		}
 		ret = ca_validate_pubkey(env, id, pkey, 0, NULL);
-		EVP_PKEY_free(pkey);
 		if (ret == 0) {
 			errstr = "in public key file, ok";
 			goto done;
@@ -1759,14 +1733,10 @@ ca_validate_cert(struct iked *env, struct iked_static_id *id,
 	}
  err:
 
-	if (rawcert != NULL) {
-		BIO_free(rawcert);
-		if (cert != NULL)
-			X509_free(cert);
-	}
-
-	if (csc != NULL)
-		X509_STORE_CTX_free(csc);
+	if (len > 0)
+		X509_free(cert);
+	BIO_free(rawcert);
+	X509_STORE_CTX_free(csc);
 
 	return (ret);
 }
diff --git a/sbin/iked/crypto.c b/sbin/iked/crypto.c
index 59cac673029..87fb7650c3f 100644
--- a/sbin/iked/crypto.c
+++ b/sbin/iked/crypto.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: crypto.c,v 1.38 2021/12/01 16:42:12 deraadt Exp $	*/
+/*	$OpenBSD: crypto.c,v 1.39 2021/12/13 17:35:34 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2010-2013 Reyk Floeter <reyk@openbsd.org>
@@ -319,8 +319,7 @@ hash_free(struct iked_hash *hash)
 {
 	if (hash == NULL)
 		return;
-	if (hash->hash_ctx != NULL)
-		HMAC_CTX_free(hash->hash_ctx);
+	HMAC_CTX_free(hash->hash_ctx);
 	ibuf_release(hash->hash_key);
 	free(hash);
 }
@@ -764,9 +763,8 @@ dsa_free(struct iked_dsa *dsa)
 	if (dsa->dsa_hmac) {
 		HMAC_CTX_free((HMAC_CTX *)dsa->dsa_ctx);
 	} else {
-		EVP_MD_CTX_destroy((EVP_MD_CTX *)dsa->dsa_ctx);
-		if (dsa->dsa_key)
-			EVP_PKEY_free(dsa->dsa_key);
+		EVP_MD_CTX_free((EVP_MD_CTX *)dsa->dsa_ctx);
+		EVP_PKEY_free(dsa->dsa_key);
 	}
 
 	ibuf_release(dsa->dsa_keydata);
@@ -842,8 +840,7 @@ dsa_setkey(struct iked_dsa *dsa, void *key, size_t keylen, uint8_t type)
 		goto err;
 	}
 
-	if (cert != NULL)
-		X509_free(cert);
+	X509_free(cert);
 	BIO_free(rawcert);	/* temporary for parsing */
 
 	return (dsa->dsa_keydata);
@@ -853,16 +850,11 @@ dsa_setkey(struct iked_dsa *dsa, void *key, size_t keylen, uint8_t type)
  err:
 	log_debug("%s: error", __func__);
 
-	if (rsa != NULL)
-		RSA_free(rsa);
-	if (ec != NULL)
-		EC_KEY_free(ec);
-	if (pkey != NULL)
-		EVP_PKEY_free(pkey);
-	if (cert != NULL)
-		X509_free(cert);
-	if (rawcert != NULL)
-		BIO_free(rawcert);
+	RSA_free(rsa);
+	EC_KEY_free(ec);
+	EVP_PKEY_free(pkey);
+	X509_free(cert);
+	BIO_free(rawcert);
 	ibuf_release(dsa->dsa_keydata);
 	dsa->dsa_keydata = NULL;
 	return (NULL);
@@ -1078,8 +1070,8 @@ _dsa_sign_ecdsa(struct iked_dsa *dsa, uint8_t *ptr, size_t len)
 	ret = 0;
  done:
 	free(tmp);
-	if (obj)
-		ECDSA_SIG_free(obj);
+	ECDSA_SIG_free(obj);
+
 	return (ret);
 }
 
@@ -1180,8 +1172,8 @@ _dsa_verify_prepare(struct iked_dsa *dsa, uint8_t **sigp, size_t *lenp,
 	BN_clear_free(r);
 	BN_clear_free(s);
 	free(ptr);
-	if (obj)
-		ECDSA_SIG_free(obj);
+	ECDSA_SIG_free(obj);
+
 	return (ret);
 }
 
diff --git a/sbin/iked/ocsp.c b/sbin/iked/ocsp.c
index 06d77af3996..0b05a620ffa 100644
--- a/sbin/iked/ocsp.c
+++ b/sbin/iked/ocsp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ocsp.c,v 1.22 2021/11/19 21:16:25 tobhe Exp $ */
+/*	$OpenBSD: ocsp.c,v 1.23 2021/12/13 17:35:34 tobhe Exp $ */
 
 /*
  * Copyright (c) 2014 Markus Friedl
@@ -322,20 +322,16 @@ ocsp_validate_cert(struct iked *env, void *data, size_t len,
 	ret = proc_composev(&env->sc_ps, PROC_PARENT, IMSG_OCSP_FD,
 	    iov, iovcnt);
 
-	if (aia)
-		X509_email_free(aia);	/* free stack of openssl strings */
+	X509_email_free(aia);	/* free stack of openssl strings */
 
 	return (ret);
 
  err:
 	ca_sslerror(__func__);
 	free(ioe);
-	if (rawcert != NULL)
-		BIO_free(rawcert);
-	if (cert != NULL)
-		X509_free(cert);
-	if (id != NULL)
-		OCSP_CERTID_free(id);
+	BIO_free(rawcert);
+	X509_free(cert);
+	OCSP_CERTID_free(id);
 	ocsp_validate_finish(ocsp, 0);	/* failed */
 	return (-1);
 }
@@ -349,15 +345,9 @@ ocsp_free(struct iked_ocsp *ocsp)
 			close(ocsp->ocsp_sock->sock_fd);
 			free(ocsp->ocsp_sock);
 		}
-		if (ocsp->ocsp_cbio != NULL)
-			BIO_free_all(ocsp->ocsp_cbio);
-
-		if (ocsp->ocsp_req_ctx != NULL)
-			OCSP_REQ_CTX_free(ocsp->ocsp_req_ctx);
-
-		if (ocsp->ocsp_req != NULL)
-			OCSP_REQUEST_free(ocsp->ocsp_req);
-
+		BIO_free_all(ocsp->ocsp_cbio);
+		OCSP_REQ_CTX_free(ocsp->ocsp_req_ctx);
+		OCSP_REQUEST_free(ocsp->ocsp_req);
 		free(ocsp);
 	}
 }
@@ -471,12 +461,10 @@ ocsp_load_certs(const char *file)
 	}
 
  done:
-	if (bio)
-		BIO_free(bio);
-	if (xis)
-		sk_X509_INFO_pop_free(xis, X509_INFO_free);
-	if (certs && sk_X509_num(certs) <= 0) {
-		sk_X509_pop_free(certs, X509_free);
+	BIO_free(bio);
+	sk_X509_INFO_pop_free(xis, X509_INFO_free);
+	if (sk_X509_num(certs) <= 0) {
+		sk_X509_free(certs);
 		certs = NULL;
 	}
 	return (certs);
@@ -599,14 +587,10 @@ ocsp_parse_response(struct iked_ocsp *ocsp, OCSP_RESPONSE *resp)
 	if (!valid) {
 		log_debug("%s: status: %s", __func__, errstr);
 	}
-	if (store)
-		X509_STORE_free(store);
-	if (verify_other)
-		sk_X509_pop_free(verify_other, X509_free);
-	if (resp)
-		OCSP_RESPONSE_free(resp);
-	if (bs)
-		OCSP_BASICRESP_free(bs);
+	X509_STORE_free(store);
+	sk_X509_pop_free(verify_other, X509_free);
+	OCSP_RESPONSE_free(resp);
+	OCSP_BASICRESP_free(bs);
 
 	ocsp_validate_finish(ocsp, valid);
 }