| Message ID | 1513246897-28171-1-git-send-email-steffan@karger.me |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] ssl_openssl: fix compiler warning by removing getbio() wrapper | expand |
Am 14.12.17 um 11:21 schrieb Steffan Karger: > An API change in openssl 1.1 made the BIO_METHOD * returned by BIO_f_ssl() > and BIO_s_mem() const, as well as the BIO_METHOD * argment of BIO_new() > const. This meant that our getbio() function would either have an API > inconsistent with 1.0 or 1.1. > > The wrapper was basically an ASSERT, so fix this by replacing the wrapper > with an ASSERT. > Looks good. The wrapper does also not add much clarity or otherwise functionality. Acked-by: Arne Schwabe ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Thu, Dec 14, 2017 at 11:21:37AM +0100, Steffan Karger wrote: > - if (!ret) > - { > - crypto_msg(M_FATAL, "Error creating %s BIO", desc); > - } I'm not sure how likely these are going to fail, but the crypto_msg() had the benefit of actually telling us what the underlying library returned as error code... If this won't ever happen except under very exceptional conditions, it won't matter much, though. gert
Hi, On 14-12-17 16:44, Gert Doering wrote: > On Thu, Dec 14, 2017 at 11:21:37AM +0100, Steffan Karger wrote: >> - if (!ret) >> - { >> - crypto_msg(M_FATAL, "Error creating %s BIO", desc); >> - } > > I'm not sure how likely these are going to fail, but the crypto_msg() > had the benefit of actually telling us what the underlying library > returned as error code... > > If this won't ever happen except under very exceptional conditions, it > won't matter much, though. Fair point, but this is very unlikely to fail indeed. We use standard openssl-provided BIO_METHOD's, and do not do threading. The one error I could imagine is a malloc failure. I still think it's fine to just go with the ASSERT(). -Steffan ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Your patch has been applied to the master and release/2.4 branch.
commit 006d6a57b8835c15222359bfb42c95005723394c (master)
commit 041b94c0ab1981c35f4294afc3f9e4f73ba9056c (release/2.4)
Author: Steffan Karger
Date: Thu Dec 14 11:21:37 2017 +0100
ssl_openssl: fix compiler warning by removing getbio() wrapper
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <1513246897-28171-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16083.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c index 34c31b9..711bba1 100644 --- a/src/openvpn/ssl_openssl.c +++ b/src/openvpn/ssl_openssl.c @@ -1416,23 +1416,6 @@ bio_debug_oc(const char *mode, BIO *bio) #endif /* ifdef BIO_DEBUG */ /* - * OpenVPN's interface to SSL/TLS authentication, - * encryption, and decryption is exclusively - * through "memory BIOs". - */ -static BIO * -getbio(BIO_METHOD *type, const char *desc) -{ - BIO *ret; - ret = BIO_new(type); - if (!ret) - { - crypto_msg(M_FATAL, "Error creating %s BIO", desc); - } - return ret; -} - -/* * Write to an OpenSSL BIO in non-blocking mode. */ static int @@ -1573,9 +1556,9 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl, const struct tls_root_ctx *ssl_ * from verify callback*/ SSL_set_ex_data(ks_ssl->ssl, mydata_index, session); - ks_ssl->ssl_bio = getbio(BIO_f_ssl(), "ssl_bio"); - ks_ssl->ct_in = getbio(BIO_s_mem(), "ct_in"); - ks_ssl->ct_out = getbio(BIO_s_mem(), "ct_out"); + ASSERT((ks_ssl->ssl_bio = BIO_new(BIO_f_ssl()))); + ASSERT((ks_ssl->ct_in = BIO_new(BIO_s_mem()))); + ASSERT((ks_ssl->ct_out = BIO_new(BIO_s_mem()))); #ifdef BIO_DEBUG bio_debug_oc("open ssl_bio", ks_ssl->ssl_bio);
An API change in openssl 1.1 made the BIO_METHOD * returned by BIO_f_ssl() and BIO_s_mem() const, as well as the BIO_METHOD * argment of BIO_new() const. This meant that our getbio() function would either have an API inconsistent with 1.0 or 1.1. The wrapper was basically an ASSERT, so fix this by replacing the wrapper with an ASSERT. Signed-off-by: Steffan Karger <steffan@karger.me> --- src/openvpn/ssl_openssl.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)