Remove all guards for calls to OpenSSL free functions - all of these

functions handle NULL, from at least OpenSSL 1.0.1g onwards.

Prompted by dtucker@ asking about guards for RSA_free(), when looking at
openssh-portable pr#84 on github.

ok deraadt@ dtucker@

diff --git a/usr.bin/ssh/cipher.c b/usr.bin/ssh/cipher.c
index ec6c138..cded43b 100644 (file)
--- a/usr.bin/ssh/cipher.c
+++ b/usr.bin/ssh/cipher.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: cipher.c,v 1.108 2017/11/03 02:22:41 djm Exp $ */
+/* $OpenBSD: cipher.c,v 1.109 2018/02/07 02:06:50 jsing Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -304,8 +304,7 @@ cipher_init(struct sshcipher_ctx **ccp, const struct sshcipher *cipher,
        } else {
                if (cc != NULL) {
 #ifdef WITH_OPENSSL
-                       if (cc->evp != NULL)
-                               EVP_CIPHER_CTX_free(cc->evp);
+                       EVP_CIPHER_CTX_free(cc->evp);
 #endif /* WITH_OPENSSL */
                        explicit_bzero(cc, sizeof(*cc));
                        free(cc);
@@ -410,10 +409,8 @@ cipher_free(struct sshcipher_ctx *cc)
        else if ((cc->cipher->flags & CFLAG_AESCTR) != 0)
                explicit_bzero(&cc->ac_ctx, sizeof(cc->ac_ctx));
 #ifdef WITH_OPENSSL
-       if (cc->evp != NULL) {
-               EVP_CIPHER_CTX_free(cc->evp);
-               cc->evp = NULL;
-       }
+       EVP_CIPHER_CTX_free(cc->evp);
+       cc->evp = NULL;
 #endif
        explicit_bzero(cc, sizeof(*cc));
        free(cc);

diff --git a/usr.bin/ssh/dh.c b/usr.bin/ssh/dh.c
index 095f93b..74b4135 100644 (file)
--- a/usr.bin/ssh/dh.c
+++ b/usr.bin/ssh/dh.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dh.c,v 1.62 2016/12/15 21:20:41 dtucker Exp $ */
+/* $OpenBSD: dh.c,v 1.63 2018/02/07 02:06:50 jsing Exp $ */
 /*
  * Copyright (c) 2000 Niels Provos.  All rights reserved.
  *
@@ -131,10 +131,8 @@ parse_prime(int linenum, char *line, struct dhgroup *dhg)
        return 1;
 
  fail:
-       if (dhg->g != NULL)
-               BN_clear_free(dhg->g);
-       if (dhg->p != NULL)
-               BN_clear_free(dhg->p);
+       BN_clear_free(dhg->g);
+       BN_clear_free(dhg->p);
        dhg->g = dhg->p = NULL;
        return 0;
 }

diff --git a/usr.bin/ssh/kex.c b/usr.bin/ssh/kex.c
index f88fefc..44723db 100644 (file)
--- a/usr.bin/ssh/kex.c
+++ b/usr.bin/ssh/kex.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.c,v 1.135 2018/01/23 05:27:21 djm Exp $ */
+/* $OpenBSD: kex.c,v 1.136 2018/02/07 02:06:50 jsing Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  *
@@ -575,10 +575,8 @@ kex_free(struct kex *kex)
        u_int mode;
 
 #ifdef WITH_OPENSSL
-       if (kex->dh)
-               DH_free(kex->dh);
-       if (kex->ec_client_key)
-               EC_KEY_free(kex->ec_client_key);
+       DH_free(kex->dh);
+       EC_KEY_free(kex->ec_client_key);
 #endif
        for (mode = 0; mode < MODE_MAX; mode++) {
                kex_free_newkeys(kex->newkeys[mode]);

diff --git a/usr.bin/ssh/kexdhc.c b/usr.bin/ssh/kexdhc.c
index 6b60f59..dc06b66 100644 (file)
--- a/usr.bin/ssh/kexdhc.c
+++ b/usr.bin/ssh/kexdhc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kexdhc.c,v 1.21 2017/12/18 02:25:15 djm Exp $ */
+/* $OpenBSD: kexdhc.c,v 1.22 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  *
@@ -198,14 +198,12 @@ input_kex_dh(int type, u_int32_t seq, struct ssh *ssh)
        explicit_bzero(hash, sizeof(hash));
        DH_free(kex->dh);
        kex->dh = NULL;
-       if (dh_server_pub)
-               BN_clear_free(dh_server_pub);
+       BN_clear_free(dh_server_pub);
        if (kbuf) {
                explicit_bzero(kbuf, klen);
                free(kbuf);
        }
-       if (shared_secret)
-               BN_clear_free(shared_secret);
+       BN_clear_free(shared_secret);
        sshkey_free(server_host_key);
        free(server_host_key_blob);
        free(signature);

diff --git a/usr.bin/ssh/kexdhs.c b/usr.bin/ssh/kexdhs.c
index 6dae3b2..20a061b 100644 (file)
--- a/usr.bin/ssh/kexdhs.c
+++ b/usr.bin/ssh/kexdhs.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kexdhs.c,v 1.25 2017/05/30 14:23:52 markus Exp $ */
+/* $OpenBSD: kexdhs.c,v 1.26 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  *
@@ -203,14 +203,12 @@ input_kex_dh_init(int type, u_int32_t seq, struct ssh *ssh)
        explicit_bzero(hash, sizeof(hash));
        DH_free(kex->dh);
        kex->dh = NULL;
-       if (dh_client_pub)
-               BN_clear_free(dh_client_pub);
+       BN_clear_free(dh_client_pub);
        if (kbuf) {
                explicit_bzero(kbuf, klen);
                free(kbuf);
        }
-       if (shared_secret)
-               BN_clear_free(shared_secret);
+       BN_clear_free(shared_secret);
        free(server_host_key_blob);
        free(signature);
        return r;

diff --git a/usr.bin/ssh/kexecdhc.c b/usr.bin/ssh/kexecdhc.c
index ee6f848..0ff264b 100644 (file)
--- a/usr.bin/ssh/kexecdhc.c
+++ b/usr.bin/ssh/kexecdhc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kexecdhc.c,v 1.12 2017/12/18 02:25:15 djm Exp $ */
+/* $OpenBSD: kexecdhc.c,v 1.13 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2010 Damien Miller.  All rights reserved.
@@ -85,8 +85,7 @@ kexecdh_client(struct ssh *ssh)
        ssh_dispatch_set(ssh, SSH2_MSG_KEX_ECDH_REPLY, &input_kex_ecdh_reply);
        r = 0;
  out:
-       if (client_key)
-               EC_KEY_free(client_key);
+       EC_KEY_free(client_key);
        return r;
 }
 
@@ -202,18 +201,14 @@ input_kex_ecdh_reply(int type, u_int32_t seq, struct ssh *ssh)
                r = kex_send_newkeys(ssh);
  out:
        explicit_bzero(hash, sizeof(hash));
-       if (kex->ec_client_key) {
-               EC_KEY_free(kex->ec_client_key);
-               kex->ec_client_key = NULL;
-       }
-       if (server_public)
-               EC_POINT_clear_free(server_public);
+       EC_KEY_free(kex->ec_client_key);
+       kex->ec_client_key = NULL;
+       EC_POINT_clear_free(server_public);
        if (kbuf) {
                explicit_bzero(kbuf, klen);
                free(kbuf);
        }
-       if (shared_secret)
-               BN_clear_free(shared_secret);
+       BN_clear_free(shared_secret);
        sshkey_free(server_host_key);
        free(server_host_key_blob);
        free(signature);

diff --git a/usr.bin/ssh/kexecdhs.c b/usr.bin/ssh/kexecdhs.c
index 4392198..89201c7 100644 (file)
--- a/usr.bin/ssh/kexecdhs.c
+++ b/usr.bin/ssh/kexecdhs.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kexecdhs.c,v 1.16 2017/05/30 14:23:52 markus Exp $ */
+/* $OpenBSD: kexecdhs.c,v 1.17 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2010 Damien Miller.  All rights reserved.
@@ -183,18 +183,14 @@ input_kex_ecdh_init(int type, u_int32_t seq, struct ssh *ssh)
                r = kex_send_newkeys(ssh);
  out:
        explicit_bzero(hash, sizeof(hash));
-       if (kex->ec_client_key) {
-               EC_KEY_free(kex->ec_client_key);
-               kex->ec_client_key = NULL;
-       }
-       if (server_key)
-               EC_KEY_free(server_key);
+       EC_KEY_free(kex->ec_client_key);
+       kex->ec_client_key = NULL;
+       EC_KEY_free(server_key);
        if (kbuf) {
                explicit_bzero(kbuf, klen);
                free(kbuf);
        }
-       if (shared_secret)
-               BN_clear_free(shared_secret);
+       BN_clear_free(shared_secret);
        free(server_host_key_blob);
        free(signature);
        return r;

diff --git a/usr.bin/ssh/kexgexc.c b/usr.bin/ssh/kexgexc.c
index 45a5682..bec49ce 100644 (file)
--- a/usr.bin/ssh/kexgexc.c
+++ b/usr.bin/ssh/kexgexc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kexgexc.c,v 1.26 2017/12/18 02:25:15 djm Exp $ */
+/* $OpenBSD: kexgexc.c,v 1.27 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2000 Niels Provos.  All rights reserved.
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
@@ -129,10 +129,8 @@ input_kex_dh_gex_group(int type, u_int32_t seq, struct ssh *ssh)
        ssh_dispatch_set(ssh, SSH2_MSG_KEX_DH_GEX_REPLY, &input_kex_dh_gex_reply);
        r = 0;
 out:
-       if (p)
-               BN_clear_free(p);
-       if (g)
-               BN_clear_free(g);
+       BN_clear_free(p);
+       BN_clear_free(g);
        return r;
 }
 
@@ -245,14 +243,12 @@ input_kex_dh_gex_reply(int type, u_int32_t seq, struct ssh *ssh)
        explicit_bzero(hash, sizeof(hash));
        DH_free(kex->dh);
        kex->dh = NULL;
-       if (dh_server_pub)
-               BN_clear_free(dh_server_pub);
+       BN_clear_free(dh_server_pub);
        if (kbuf) {
                explicit_bzero(kbuf, klen);
                free(kbuf);
        }
-       if (shared_secret)
-               BN_clear_free(shared_secret);
+       BN_clear_free(shared_secret);
        sshkey_free(server_host_key);
        free(server_host_key_blob);
        free(signature);

diff --git a/usr.bin/ssh/kexgexs.c b/usr.bin/ssh/kexgexs.c
index 7f9bce7..025ce9a 100644 (file)
--- a/usr.bin/ssh/kexgexs.c
+++ b/usr.bin/ssh/kexgexs.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kexgexs.c,v 1.31 2017/05/30 14:23:52 markus Exp $ */
+/* $OpenBSD: kexgexs.c,v 1.32 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2000 Niels Provos.  All rights reserved.
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
@@ -232,14 +232,12 @@ input_kex_dh_gex_init(int type, u_int32_t seq, struct ssh *ssh)
  out:
        DH_free(kex->dh);
        kex->dh = NULL;
-       if (dh_client_pub)
-               BN_clear_free(dh_client_pub);
+       BN_clear_free(dh_client_pub);
        if (kbuf) {
                explicit_bzero(kbuf, klen);
                free(kbuf);
        }
-       if (shared_secret)
-               BN_clear_free(shared_secret);
+       BN_clear_free(shared_secret);
        free(server_host_key_blob);
        free(signature);
        return r;

diff --git a/usr.bin/ssh/ssh-dss.c b/usr.bin/ssh/ssh-dss.c
index 6c9604b..d742dee 100644 (file)
--- a/usr.bin/ssh/ssh-dss.c
+++ b/usr.bin/ssh/ssh-dss.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-dss.c,v 1.36 2018/01/23 05:27:21 djm Exp $ */
+/* $OpenBSD: ssh-dss.c,v 1.37 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -101,8 +101,7 @@ ssh_dss_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
        ret = 0;
  out:
        explicit_bzero(digest, sizeof(digest));
-       if (sig != NULL)
-               DSA_SIG_free(sig);
+       DSA_SIG_free(sig);
        sshbuf_free(b);
        return ret;
 }
@@ -180,8 +179,7 @@ ssh_dss_verify(const struct sshkey *key,
 
  out:
        explicit_bzero(digest, sizeof(digest));
-       if (sig != NULL)
-               DSA_SIG_free(sig);
+       DSA_SIG_free(sig);
        sshbuf_free(b);
        free(ktype);
        if (sigblob != NULL) {

diff --git a/usr.bin/ssh/ssh-ecdsa.c b/usr.bin/ssh/ssh-ecdsa.c
index b127269..fef0e00 100644 (file)
--- a/usr.bin/ssh/ssh-ecdsa.c
+++ b/usr.bin/ssh/ssh-ecdsa.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-ecdsa.c,v 1.13 2016/04/21 06:08:02 djm Exp $ */
+/* $OpenBSD: ssh-ecdsa.c,v 1.14 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2010 Damien Miller.  All rights reserved.
@@ -97,8 +97,7 @@ ssh_ecdsa_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
        explicit_bzero(digest, sizeof(digest));
        sshbuf_free(b);
        sshbuf_free(bb);
-       if (sig != NULL)
-               ECDSA_SIG_free(sig);
+       ECDSA_SIG_free(sig);
        return ret;
 }
 
@@ -176,8 +175,7 @@ ssh_ecdsa_verify(const struct sshkey *key,
        explicit_bzero(digest, sizeof(digest));
        sshbuf_free(sigbuf);
        sshbuf_free(b);
-       if (sig != NULL)
-               ECDSA_SIG_free(sig);
+       ECDSA_SIG_free(sig);
        free(ktype);
        return ret;
 }

diff --git a/usr.bin/ssh/ssh-pkcs11.c b/usr.bin/ssh/ssh-pkcs11.c
index 763997e..64cff8c 100644 (file)
--- a/usr.bin/ssh/ssh-pkcs11.c
+++ b/usr.bin/ssh/ssh-pkcs11.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-pkcs11.c,v 1.25 2017/05/31 09:15:42 deraadt Exp $ */
+/* $OpenBSD: ssh-pkcs11.c,v 1.26 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2010 Markus Friedl.  All rights reserved.
  *
@@ -521,8 +521,7 @@ pkcs11_fetch_keys_filter(struct pkcs11_provider *p, CK_ULONG slotidx,
                            == NULL) {
                                error("RSAPublicKey_dup");
                        }
-                       if (x509)
-                               X509_free(x509);
+                       X509_free(x509);
                }
                if (rsa && rsa->n && rsa->e &&
                    pkcs11_rsa_wrap(p, slotidx, &attribs[0], rsa) == 0) {

diff --git a/usr.bin/ssh/sshkey.c b/usr.bin/ssh/sshkey.c
index 0f8769f..20f4fb2 100644 (file)
--- a/usr.bin/ssh/sshkey.c
+++ b/usr.bin/ssh/sshkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.c,v 1.59 2017/12/18 02:25:15 djm Exp $ */
+/* $OpenBSD: sshkey.c,v 1.60 2018/02/07 02:06:51 jsing Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
@@ -449,8 +449,7 @@ sshkey_new(int type)
                if ((rsa = RSA_new()) == NULL ||
                    (rsa->n = BN_new()) == NULL ||
                    (rsa->e = BN_new()) == NULL) {
-                       if (rsa != NULL)
-                               RSA_free(rsa);
+                       RSA_free(rsa);
                        free(k);
                        return NULL;
                }
@@ -463,8 +462,7 @@ sshkey_new(int type)
                    (dsa->q = BN_new()) == NULL ||
                    (dsa->g = BN_new()) == NULL ||
                    (dsa->pub_key = BN_new()) == NULL) {
-                       if (dsa != NULL)
-                               DSA_free(dsa);
+                       DSA_free(dsa);
                        free(k);
                        return NULL;
                }
@@ -558,20 +556,17 @@ sshkey_free(struct sshkey *k)
 #ifdef WITH_OPENSSL
        case KEY_RSA:
        case KEY_RSA_CERT:
-               if (k->rsa != NULL)
-                       RSA_free(k->rsa);
+               RSA_free(k->rsa);
                k->rsa = NULL;
                break;
        case KEY_DSA:
        case KEY_DSA_CERT:
-               if (k->dsa != NULL)
-                       DSA_free(k->dsa);
+               DSA_free(k->dsa);
                k->dsa = NULL;
                break;
        case KEY_ECDSA:
        case KEY_ECDSA_CERT:
-               if (k->ecdsa != NULL)
-                       EC_KEY_free(k->ecdsa);
+               EC_KEY_free(k->ecdsa);
                k->ecdsa = NULL;
                break;
 #endif /* WITH_OPENSSL */
@@ -1222,8 +1217,7 @@ sshkey_read(struct sshkey *ret, char **cpp)
                switch (sshkey_type_plain(ret->type)) {
 #ifdef WITH_OPENSSL
                case KEY_RSA:
-                       if (ret->rsa != NULL)
-                               RSA_free(ret->rsa);
+                       RSA_free(ret->rsa);
                        ret->rsa = k->rsa;
                        k->rsa = NULL;
 #ifdef DEBUG_PK
@@ -1231,8 +1225,7 @@ sshkey_read(struct sshkey *ret, char **cpp)
 #endif
                        break;
                case KEY_DSA:
-                       if (ret->dsa != NULL)
-                               DSA_free(ret->dsa);
+                       DSA_free(ret->dsa);
                        ret->dsa = k->dsa;
                        k->dsa = NULL;
 #ifdef DEBUG_PK
@@ -1240,8 +1233,7 @@ sshkey_read(struct sshkey *ret, char **cpp)
 #endif
                        break;
                case KEY_ECDSA:
-                       if (ret->ecdsa != NULL)
-                               EC_KEY_free(ret->ecdsa);
+                       EC_KEY_free(ret->ecdsa);
                        ret->ecdsa = k->ecdsa;
                        ret->ecdsa_nid = k->ecdsa_nid;
                        k->ecdsa = NULL;
@@ -1382,10 +1374,8 @@ rsa_generate_private_key(u_int bits, RSA **rsap)
        private = NULL;
        ret = 0;
  out:
-       if (private != NULL)
-               RSA_free(private);
-       if (f4 != NULL)
-               BN_free(f4);
+       RSA_free(private);
+       BN_free(f4);
        return ret;
 }
 
@@ -1413,8 +1403,7 @@ dsa_generate_private_key(u_int bits, DSA **dsap)
        private = NULL;
        ret = 0;
  out:
-       if (private != NULL)
-               DSA_free(private);
+       DSA_free(private);
        return ret;
 }
 
@@ -1490,8 +1479,7 @@ ecdsa_generate_private_key(u_int bits, int *nid, EC_KEY **ecdsap)
        private = NULL;
        ret = 0;
  out:
-       if (private != NULL)
-               EC_KEY_free(private);
+       EC_KEY_free(private);
        return ret;
 }
 #endif /* WITH_OPENSSL */
@@ -1896,8 +1884,7 @@ sshkey_from_blob_internal(struct sshbuf *b, struct sshkey **keyp,
                        ret = SSH_ERR_EC_CURVE_MISMATCH;
                        goto out;
                }
-               if (key->ecdsa != NULL)
-                       EC_KEY_free(key->ecdsa);
+               EC_KEY_free(key->ecdsa);
                if ((key->ecdsa = EC_KEY_new_by_curve_name(key->ecdsa_nid))
                    == NULL) {
                        ret = SSH_ERR_EC_CURVE_INVALID;
@@ -1973,8 +1960,7 @@ sshkey_from_blob_internal(struct sshbuf *b, struct sshkey **keyp,
        free(curve);
        free(pk);
 #ifdef WITH_OPENSSL
-       if (q != NULL)
-               EC_POINT_free(q);
+       EC_POINT_free(q);
 #endif /* WITH_OPENSSL */
        return ret;
 }
@@ -2715,8 +2701,7 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
        free(tname);
        free(curve);
 #ifdef WITH_OPENSSL
-       if (exponent != NULL)
-               BN_clear_free(exponent);
+       BN_clear_free(exponent);
 #endif /* WITH_OPENSSL */
        sshkey_free(k);
        if (ed25519_pk != NULL) {
@@ -2804,8 +2789,7 @@ sshkey_ec_validate_public(const EC_GROUP *group, const EC_POINT *public)
        ret = 0;
  out:
        BN_CTX_free(bnctx);
-       if (nq != NULL)
-               EC_POINT_free(nq);
+       EC_POINT_free(nq);
        return ret;
 }
 
@@ -3496,8 +3480,7 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
        }
  out:
        BIO_free(bio);
-       if (pk != NULL)
-               EVP_PKEY_free(pk);
+       EVP_PKEY_free(pk);
        sshkey_free(prv);
        return r;
 }