From 04c7e28f83062dc42f2380d1bb3a6bf0190852c0 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Mon, 18 Dec 2017 02:25:15 +0000 Subject: upstream commit pass negotiated signing algorithm though to sshkey_verify() and check that the negotiated algorithm matches the type in the signature (only matters for RSA SHA1/SHA2 sigs). ok markus@ OpenBSD-Commit-ID: 735fb15bf4adc060d3bee9d047a4bcaaa81b1af9 --- kexdhc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'kexdhc.c') diff --git a/kexdhc.c b/kexdhc.c index 9864ee2ec..5e1a353a5 100644 --- a/kexdhc.c +++ b/kexdhc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kexdhc.c,v 1.20 2017/05/30 14:23:52 markus Exp $ */ +/* $OpenBSD: kexdhc.c,v 1.21 2017/12/18 02:25:15 djm Exp $ */ /* * Copyright (c) 2001 Markus Friedl. All rights reserved. * @@ -183,7 +183,7 @@ input_kex_dh(int type, u_int32_t seq, struct ssh *ssh) goto out; if ((r = sshkey_verify(server_host_key, signature, slen, hash, hashlen, - ssh->compat)) != 0) + kex->hostkey_alg, ssh->compat)) != 0) goto out; /* save session id */ -- cgit v1.2.3 From 7cd31632e3a6607170ed0c9ed413a7ded5b9b377 Mon Sep 17 00:00:00 2001 From: "jsing@openbsd.org" Date: Wed, 7 Feb 2018 02:06:50 +0000 Subject: upstream commit 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@ OpenBSD-Commit-ID: 954f1c51b94297d0ae1f749271e184141e0cadae --- cipher.c | 11 ++++------- dh.c | 8 +++----- kex.c | 8 +++----- kexdhc.c | 8 +++----- kexdhs.c | 8 +++----- kexecdhc.c | 17 ++++++----------- kexecdhs.c | 14 +++++--------- kexgexc.c | 14 +++++--------- kexgexs.c | 8 +++----- ssh-dss.c | 8 +++----- ssh-ecdsa.c | 8 +++----- ssh-pkcs11.c | 5 ++--- sshkey.c | 53 ++++++++++++++++++----------------------------------- 13 files changed, 61 insertions(+), 109 deletions(-) (limited to 'kexdhc.c') diff --git a/cipher.c b/cipher.c index aa8cfcf67..f3d4f69a5 100644 --- a/cipher.c +++ b/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 * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -310,8 +310,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); @@ -416,10 +415,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/dh.c b/dh.c index eebee2377..46afba033 100644 --- a/dh.c +++ b/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. * @@ -135,10 +135,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/kex.c b/kex.c index 83c6199f3..15ea28b07 100644 --- a/kex.c +++ b/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. * @@ -587,11 +587,9 @@ kex_free(struct kex *kex) u_int mode; #ifdef WITH_OPENSSL - if (kex->dh) - DH_free(kex->dh); + DH_free(kex->dh); #ifdef OPENSSL_HAS_ECC - if (kex->ec_client_key) - EC_KEY_free(kex->ec_client_key); + EC_KEY_free(kex->ec_client_key); #endif /* OPENSSL_HAS_ECC */ #endif /* WITH_OPENSSL */ for (mode = 0; mode < MODE_MAX; mode++) { diff --git a/kexdhc.c b/kexdhc.c index 5e1a353a5..9a9f1ea78 100644 --- a/kexdhc.c +++ b/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. * @@ -203,14 +203,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/kexdhs.c b/kexdhs.c index 81ce56d7a..da8f4c439 100644 --- a/kexdhs.c +++ b/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. * @@ -208,14 +208,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/kexecdhc.c b/kexecdhc.c index 67669b3bf..ac146a362 100644 --- a/kexecdhc.c +++ b/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. @@ -89,8 +89,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; } @@ -206,18 +205,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/kexecdhs.c b/kexecdhs.c index dc24a3af6..af4f30309 100644 --- a/kexecdhs.c +++ b/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. @@ -187,18 +187,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/kexgexc.c b/kexgexc.c index 6f8cf48a6..762a9a322 100644 --- a/kexgexc.c +++ b/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. @@ -134,10 +134,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; } @@ -250,14 +248,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/kexgexs.c b/kexgexs.c index c5dd00578..d7b48ea88 100644 --- a/kexgexs.c +++ b/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. @@ -237,14 +237,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/ssh-dss.c b/ssh-dss.c index cda498a87..9f832ee2b 100644 --- a/ssh-dss.c +++ b/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. * @@ -107,8 +107,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; } @@ -186,8 +185,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/ssh-ecdsa.c b/ssh-ecdsa.c index d7bf3c69b..3d3b78d7b 100644 --- a/ssh-ecdsa.c +++ b/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. @@ -101,8 +101,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; } @@ -180,8 +179,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/ssh-pkcs11.c b/ssh-pkcs11.c index b37491c5d..65a7b5897 100644 --- a/ssh-pkcs11.c +++ b/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. * @@ -532,8 +532,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/sshkey.c b/sshkey.c index 91e0073ff..fb987d6b7 100644 --- a/sshkey.c +++ b/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. @@ -469,8 +469,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; } @@ -483,8 +482,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; } @@ -578,21 +576,18 @@ 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; # ifdef OPENSSL_HAS_ECC 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 /* OPENSSL_HAS_ECC */ @@ -1248,8 +1243,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 @@ -1257,8 +1251,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 @@ -1267,8 +1260,7 @@ sshkey_read(struct sshkey *ret, char **cpp) break; # ifdef OPENSSL_HAS_ECC 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; @@ -1410,10 +1402,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; } @@ -1441,8 +1431,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; } @@ -1521,8 +1510,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 /* OPENSSL_HAS_ECC */ @@ -1933,8 +1921,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; @@ -2011,8 +1998,7 @@ sshkey_from_blob_internal(struct sshbuf *b, struct sshkey **keyp, free(curve); free(pk); #if defined(WITH_OPENSSL) && defined(OPENSSL_HAS_ECC) - if (q != NULL) - EC_POINT_free(q); + EC_POINT_free(q); #endif /* WITH_OPENSSL && OPENSSL_HAS_ECC */ return ret; } @@ -2765,8 +2751,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) { @@ -2854,8 +2839,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; } @@ -3550,8 +3534,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; } -- cgit v1.2.3