Hey,
tb@ noticed that we do a lot of redundant explicit NULL checks before
calling libcrypto *_free() functions. A few of the free() calls can also
be avoided by using X509_get0_pubkey() instead of X509_get_pubkey().
ok?
Index: ca.c
===================================================================
RCS file: /cvs/src/sbin/iked/ca.c,v
retrieving revision 1.83
diff -u -p -r1.83 ca.c
--- ca.c 8 Dec 2021 19:17:35 -0000 1.83
+++ ca.c 13 Dec 2021 13:25:55 -0000
@@ -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
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 *c
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 *c
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, uint
* 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)) {
@@ -1245,10 +1238,9 @@ ca_pubkey_serialize(EVP_PKEY *key, struc
ret = 0;
done:
- if (rsa != NULL)
- RSA_free(rsa);
- if (ec != NULL)
- EC_KEY_free(ec);
+ RSA_free(rsa);
+ EC_KEY_free(ec);
+
return (ret);
}
@@ -1317,10 +1309,9 @@ ca_privkey_serialize(EVP_PKEY *key, stru
ret = 0;
done:
- if (rsa != NULL)
- RSA_free(rsa);
- if (ec != NULL)
- EC_KEY_free(ec);
+ RSA_free(rsa);
+ EC_KEY_free(ec);
+
return (ret);
}
@@ -1354,14 +1345,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 +1391,8 @@ ca_privkey_to_method(struct iked_id *pri
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 +1615,12 @@ ca_validate_pubkey(struct iked *env, str
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);
+ EVP_PKEY_free(peerkey);
return (ret);
}
@@ -1682,12 +1660,11 @@ ca_validate_cert(struct iked *env, struc
}
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 +1736,9 @@ ca_validate_cert(struct iked *env, struc
}
err:
- if (rawcert != NULL) {
- BIO_free(rawcert);
- if (cert != NULL)
- X509_free(cert);
- }
-
- if (csc != NULL)
- X509_STORE_CTX_free(csc);
+ BIO_free(rawcert);
+ X509_free(cert);
+ X509_STORE_CTX_free(csc);
return (ret);
}
Index: crypto.c
===================================================================
RCS file: /cvs/src/sbin/iked/crypto.c,v
retrieving revision 1.38
diff -u -p -r1.38 crypto.c
--- crypto.c 1 Dec 2021 16:42:12 -0000 1.38
+++ crypto.c 13 Dec 2021 13:25:55 -0000
@@ -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);
}
@@ -765,8 +764,7 @@ dsa_free(struct iked_dsa *dsa)
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_PKEY_free(dsa->dsa_key);
}
ibuf_release(dsa->dsa_keydata);
@@ -842,8 +840,7 @@ dsa_setkey(struct iked_dsa *dsa, void *k
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 *k
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, ui
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
BN_clear_free(r);
BN_clear_free(s);
free(ptr);
- if (obj)
- ECDSA_SIG_free(obj);
+ ECDSA_SIG_free(obj);
+
return (ret);
}
Index: ocsp.c
===================================================================
RCS file: /cvs/src/sbin/iked/ocsp.c,v
retrieving revision 1.22
diff -u -p -r1.22 ocsp.c
--- ocsp.c 19 Nov 2021 21:16:25 -0000 1.22
+++ ocsp.c 13 Dec 2021 13:25:55 -0000
@@ -322,20 +322,16 @@ ocsp_validate_cert(struct iked *env, voi
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,10 +461,8 @@ ocsp_load_certs(const char *file)
}
done:
- if (bio)
- BIO_free(bio);
- if (xis)
- sk_X509_INFO_pop_free(xis, X509_INFO_free);
+ BIO_free(bio);
+ sk_X509_INFO_pop_free(xis, X509_INFO_free);
if (certs && sk_X509_num(certs) <= 0) {
sk_X509_pop_free(certs, X509_free);
certs = NULL;
@@ -599,14 +587,10 @@ ocsp_parse_response(struct iked_ocsp *oc
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);
}
