From: jsing Date: Tue, 4 Jan 2022 12:53:31 +0000 (+0000) Subject: Return 0 on failure from send/get kex functions in the legacy stack. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=9ccbb9b1bfb3a09ebfc6b3bafcba58958c9b88d1;p=openbsd Return 0 on failure from send/get kex functions in the legacy stack. In the legacy stack, a message handling function returns -1 for failure, 0 for need more data and 1 for success (although in extra special cases 2 may also be used). However, the various send/get kex functions only need to indicate success or failure - switch these to return 0 on failure (rather than -1) and use normal result testing. This leaves GOST unchanged for now, as that code is special and needs extra work. ok inoguchi@ tb@ --- diff --git a/lib/libssl/ssl_clnt.c b/lib/libssl/ssl_clnt.c index 3e4a4b37905..80a16f1042d 100644 --- a/lib/libssl/ssl_clnt.c +++ b/lib/libssl/ssl_clnt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_clnt.c,v 1.125 2022/01/04 11:17:11 jsing Exp $ */ +/* $OpenBSD: ssl_clnt.c,v 1.126 2022/01/04 12:53:31 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1258,7 +1258,7 @@ ssl3_get_server_kex_dhe(SSL *s, EVP_PKEY **pkey, CBS *cbs) sc->peer_dh_tmp = dh; - return (1); + return 1; decode_err: SSLerror(s, SSL_R_BAD_PACKET_LENGTH); @@ -1267,14 +1267,14 @@ ssl3_get_server_kex_dhe(SSL *s, EVP_PKEY **pkey, CBS *cbs) err: DH_free(dh); - return (-1); + return 0; } static int ssl3_get_server_kex_ecdhe_ecp(SSL *s, SESS_CERT *sc, int nid, CBS *public) { EC_KEY *ecdh = NULL; - int ret = -1; + int ret = 0; /* Extract the server's ephemeral ECDH public key. */ if ((ecdh = EC_KEY_new()) == NULL) { @@ -1320,10 +1320,10 @@ ssl3_get_server_kex_ecdhe_ecx(SSL *s, SESS_CERT *sc, int nid, CBS *public) goto err; } - return (1); + return 1; err: - return (-1); + return 0; } static int @@ -1371,10 +1371,10 @@ ssl3_get_server_kex_ecdhe(SSL *s, EVP_PKEY **pkey, CBS *cbs) } if (nid == NID_X25519) { - if (ssl3_get_server_kex_ecdhe_ecx(s, sc, nid, &public) != 1) + if (!ssl3_get_server_kex_ecdhe_ecx(s, sc, nid, &public)) goto err; } else { - if (ssl3_get_server_kex_ecdhe_ecp(s, sc, nid, &public) != 1) + if (!ssl3_get_server_kex_ecdhe_ecp(s, sc, nid, &public)) goto err; } @@ -1391,13 +1391,13 @@ ssl3_get_server_kex_ecdhe(SSL *s, EVP_PKEY **pkey, CBS *cbs) /* XXX - Anonymous ECDH, so no certificate or pkey. */ *pkey = NULL; - return (1); + return 1; decode_err: ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); SSLerror(s, SSL_R_BAD_PACKET_LENGTH); err: - return (-1); + return 0; } int @@ -1465,10 +1465,10 @@ ssl3_get_server_key_exchange(SSL *s) param_len = CBS_len(&cbs); if (alg_k & SSL_kDHE) { - if (ssl3_get_server_kex_dhe(s, &pkey, &cbs) != 1) + if (!ssl3_get_server_kex_dhe(s, &pkey, &cbs)) goto err; } else if (alg_k & SSL_kECDHE) { - if (ssl3_get_server_kex_ecdhe(s, &pkey, &cbs) != 1) + if (!ssl3_get_server_kex_ecdhe(s, &pkey, &cbs)) goto err; } else if (alg_k != 0) { al = SSL_AD_UNEXPECTED_MESSAGE; @@ -1904,7 +1904,7 @@ ssl3_send_client_kex_rsa(SSL *s, SESS_CERT *sess_cert, CBB *cbb) uint16_t max_legacy_version; EVP_PKEY *pkey = NULL; RSA *rsa; - int ret = -1; + int ret = 0; int enc_len; CBB epms; @@ -1960,7 +1960,7 @@ ssl3_send_client_kex_rsa(SSL *s, SESS_CERT *sess_cert, CBB *cbb) EVP_PKEY_free(pkey); free(enc_pms); - return (ret); + return ret; } static int @@ -1970,7 +1970,7 @@ ssl3_send_client_kex_dhe(SSL *s, SESS_CERT *sess_cert, CBB *cbb) DH *dh_srvr; uint8_t *key = NULL; size_t key_len = 0; - int ret = -1; + int ret = 0; /* Ensure that we have an ephemeral key from the server for DHE. */ if ((dh_srvr = sess_cert->peer_dh_tmp) == NULL) { @@ -1999,7 +1999,7 @@ ssl3_send_client_kex_dhe(SSL *s, SESS_CERT *sess_cert, CBB *cbb) DH_free(dh_clnt); freezero(key, key_len); - return (ret); + return ret; } static int @@ -2008,7 +2008,7 @@ ssl3_send_client_kex_ecdhe_ecp(SSL *s, SESS_CERT *sc, CBB *cbb) EC_KEY *ecdh = NULL; uint8_t *key = NULL; size_t key_len = 0; - int ret = -1; + int ret = 0; CBB ecpoint; if ((ecdh = EC_KEY_new()) == NULL) { @@ -2039,14 +2039,14 @@ ssl3_send_client_kex_ecdhe_ecp(SSL *s, SESS_CERT *sc, CBB *cbb) freezero(key, key_len); EC_KEY_free(ecdh); - return (ret); + return ret; } static int ssl3_send_client_kex_ecdhe_ecx(SSL *s, SESS_CERT *sc, CBB *cbb) { uint8_t *public_key = NULL, *private_key = NULL, *shared_key = NULL; - int ret = -1; + int ret = 0; CBB ecpoint; /* Generate X25519 key pair and derive shared key. */ @@ -2078,7 +2078,7 @@ ssl3_send_client_kex_ecdhe_ecx(SSL *s, SESS_CERT *sc, CBB *cbb) freezero(private_key, X25519_KEY_LENGTH); freezero(shared_key, X25519_KEY_LENGTH); - return (ret); + return ret; } static int @@ -2096,10 +2096,10 @@ ssl3_send_client_kex_ecdhe(SSL *s, SESS_CERT *sc, CBB *cbb) goto err; } - return (1); + return 1; err: - return (-1); + return 0; } static int @@ -2237,13 +2237,13 @@ ssl3_send_client_key_exchange(SSL *s) goto err; if (alg_k & SSL_kRSA) { - if (ssl3_send_client_kex_rsa(s, sess_cert, &kex) != 1) + if (!ssl3_send_client_kex_rsa(s, sess_cert, &kex)) goto err; } else if (alg_k & SSL_kDHE) { - if (ssl3_send_client_kex_dhe(s, sess_cert, &kex) != 1) + if (!ssl3_send_client_kex_dhe(s, sess_cert, &kex)) goto err; } else if (alg_k & SSL_kECDHE) { - if (ssl3_send_client_kex_ecdhe(s, sess_cert, &kex) != 1) + if (!ssl3_send_client_kex_ecdhe(s, sess_cert, &kex)) goto err; } else if (alg_k & SSL_kGOST) { if (ssl3_send_client_kex_gost(s, sess_cert, &kex) != 1) diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c index 330f9176d89..0496985351b 100644 --- a/lib/libssl/ssl_srvr.c +++ b/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.129 2021/12/26 15:10:59 tb Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.130 2022/01/04 12:53:31 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1361,7 +1361,7 @@ ssl3_send_server_kex_dhe(SSL *s, CBB *cbb) err: DH_free(dh); - return -1; + return 0; } static int @@ -1417,12 +1417,12 @@ ssl3_send_server_kex_ecdhe_ecp(SSL *s, int nid, CBB *cbb) if (!CBB_flush(cbb)) goto err; - return (1); + return 1; fatal_err: ssl3_send_alert(s, SSL3_AL_FATAL, al); err: - return (-1); + return 0; } static int @@ -1431,7 +1431,7 @@ ssl3_send_server_kex_ecdhe_ecx(SSL *s, int nid, CBB *cbb) uint8_t *public_key = NULL, *private_key = NULL; uint16_t curve_id; CBB ecpoint; - int ret = -1; + int ret = 0; /* Generate an X25519 key pair. */ if (S3I(s)->tmp.x25519 != NULL) { @@ -1469,7 +1469,7 @@ ssl3_send_server_kex_ecdhe_ecx(SSL *s, int nid, CBB *cbb) free(public_key); freezero(private_key, X25519_KEY_LENGTH); - return (ret); + return ret; } static int @@ -1518,10 +1518,10 @@ ssl3_send_server_key_exchange(SSL *s) type = S3I(s)->hs.cipher->algorithm_mkey; if (type & SSL_kDHE) { - if (ssl3_send_server_kex_dhe(s, &cbb_params) != 1) + if (!ssl3_send_server_kex_dhe(s, &cbb_params)) goto err; } else if (type & SSL_kECDHE) { - if (ssl3_send_server_kex_ecdhe(s, &cbb_params) != 1) + if (!ssl3_send_server_kex_ecdhe(s, &cbb_params)) goto err; } else { al = SSL_AD_HANDSHAKE_FAILURE; @@ -1775,7 +1775,7 @@ ssl3_get_client_kex_rsa(SSL *s, CBS *cbs) freezero(pms, pms_len); - return (1); + return 1; decode_err: al = SSL_AD_DECODE_ERROR; @@ -1785,7 +1785,7 @@ ssl3_get_client_kex_rsa(SSL *s, CBS *cbs) err: freezero(pms, pms_len); - return (-1); + return 0; } static int @@ -1796,7 +1796,7 @@ ssl3_get_client_kex_dhe(SSL *s, CBS *cbs) int invalid_key; uint8_t *key = NULL; size_t key_len = 0; - int ret = -1; + int ret = 0; if ((dh_srvr = S3I(s)->tmp.dh) == NULL) { ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE); @@ -1844,7 +1844,7 @@ ssl3_get_client_kex_ecdhe_ecp(SSL *s, CBS *cbs) EC_KEY *ecdh_peer = NULL; EC_KEY *ecdh; CBS public; - int ret = -1; + int ret = 0; /* * Use the ephemeral values we saved when generating the @@ -1887,7 +1887,7 @@ ssl3_get_client_kex_ecdhe_ecp(SSL *s, CBS *cbs) freezero(key, key_len); EC_KEY_free(ecdh_peer); - return (ret); + return ret; } static int @@ -1895,7 +1895,7 @@ ssl3_get_client_kex_ecdhe_ecx(SSL *s, CBS *cbs) { uint8_t *shared_key = NULL; CBS ecpoint; - int ret = -1; + int ret = 0; if (!CBS_get_u8_length_prefixed(cbs, &ecpoint)) goto err; @@ -1920,7 +1920,7 @@ ssl3_get_client_kex_ecdhe_ecx(SSL *s, CBS *cbs) err: freezero(shared_key, X25519_KEY_LENGTH); - return (ret); + return ret; } static int @@ -2023,13 +2023,13 @@ ssl3_get_client_key_exchange(SSL *s) alg_k = S3I(s)->hs.cipher->algorithm_mkey; if (alg_k & SSL_kRSA) { - if (ssl3_get_client_kex_rsa(s, &cbs) != 1) + if (!ssl3_get_client_kex_rsa(s, &cbs)) goto err; } else if (alg_k & SSL_kDHE) { - if (ssl3_get_client_kex_dhe(s, &cbs) != 1) + if (!ssl3_get_client_kex_dhe(s, &cbs)) goto err; } else if (alg_k & SSL_kECDHE) { - if (ssl3_get_client_kex_ecdhe(s, &cbs) != 1) + if (!ssl3_get_client_kex_ecdhe(s, &cbs)) goto err; } else if (alg_k & SSL_kGOST) { if (ssl3_get_client_kex_gost(s, &cbs) != 1)