| Message ID | E1lSJph-0004I0-FL@sfs-ml-2.v29.lw.sourceforge.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Openvpn-devel] Change CTR DRBG update function call to new mbedtls 2.16.0 API | expand |
Am 02.04.21 um 15:26 schrieb Max Fillinger: > From: Uipko Berghuis <uipko.berghuis@fox-it.com> > > In mbedtls 2.16.0 mbedtls_ctr_drbg_update() changed to > mbedtls_ctr_drbg_update_ret(). Change the function name and handle > the new return value error code. > --- > src/openvpn/ssl_mbedtls.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c > index 5d7af351..56e9f045 100644 > --- a/src/openvpn/ssl_mbedtls.c > +++ b/src/openvpn/ssl_mbedtls.c > @@ -950,7 +950,10 @@ tls_ctx_personalise_random(struct tls_root_ctx *ctx) > > if (0 != memcmp(old_sha256_hash, sha256_hash, sizeof(sha256_hash))) > { > - mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32); > + if (!mbed_ok(mbedtls_ctr_drbg_update_ret(cd_ctx, sha256_hash, 32))) > + { > + msg(M_WARN, "WARNING: failed to personalise random, could not update CTR_DRBG"); > + } > memcpy(old_sha256_hash, sha256_hash, sizeof(old_sha256_hash)); > } > } > This change will break compilation with anything that is < 2.16.0. Arne
> Am 02.04.21 um 15:26 schrieb Max Fillinger: > > From: Uipko Berghuis <uipko.berghuis@fox-it.com> > > > > In mbedtls 2.16.0 mbedtls_ctr_drbg_update() changed to > > mbedtls_ctr_drbg_update_ret(). Change the function name and handle the > > new return value error code. > > --- > > src/openvpn/ssl_mbedtls.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c > > index 5d7af351..56e9f045 100644 > > --- a/src/openvpn/ssl_mbedtls.c > > +++ b/src/openvpn/ssl_mbedtls.c > > @@ -950,7 +950,10 @@ tls_ctx_personalise_random(struct tls_root_ctx > > *ctx) > > > > if (0 != memcmp(old_sha256_hash, sha256_hash, > sizeof(sha256_hash))) > > { > > - mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32); > > + if (!mbed_ok(mbedtls_ctr_drbg_update_ret(cd_ctx, > sha256_hash, 32))) > > + { > > + msg(M_WARN, "WARNING: failed to personalise random, > could not update CTR_DRBG"); > > + } > > memcpy(old_sha256_hash, sha256_hash, > sizeof(old_sha256_hash)); > > } > > } > > > > This change will break compilation with anything that is < 2.16.0. This function is deprecated in 2.16. I don't mind keeping this change to OpenVPN-NL for now, but for future reference, what's the best solution when a new version of mbedtls removes the function?
Hi, On Tue, Apr 06, 2021 at 10:55:52AM +0000, Maximilian Fillinger wrote: > > This change will break compilation with anything that is < 2.16.0. > > This function is deprecated in 2.16. I don't mind keeping this change to > OpenVPN-NL for now, but for future reference, what's the best solution > when a new version of mbedtls removes the function? It's still available in 2.26.0... so when they decide to really remove it, we'll have to see - what is the oldest mbedtls version we aim to support - if that is "2.16 or later", we can just apply the change as is - if that is "before 2.16", we need to add compat code - either #ifdef on mbedTLS version (if possible), or adding function detection to configure.ac (as we do for OpenSSL). Both is somewhat annoying. Now... what *is* the oldest mbedtls version we should reasonably support? For OpenSSL, we're stuck to 1.0.2 for the time being as that's still the primary (and bugfix-backported) version on FreeBSD 11 and on RHEL versions still supported. For mbedTLS I have no idea. gert
Hi, On 06/04/2021 13:14, Gert Doering wrote: > Now... what *is* the oldest mbedtls version we should reasonably support? > > For OpenSSL, we're stuck to 1.0.2 for the time being as that's still > the primary (and bugfix-backported) version on FreeBSD 11 and on RHEL > versions still supported. For mbedTLS I have no idea. Good question. I was wondering the same. Debian 10 (stable) is on mbedtls-2.16.0 CentOS 8 is on mbedtls-2.16.9 Fedora EPEL 8 (and up to Fedora 35) is on mbedtls-2.16.9 ** Ubuntu 18.04 is on mbedtls-2.8.0 ** Ubuntu 20.04 is on mbedtls-2.16.4 At this point I believe that assuming mbedtls >= 2.16.0 is meaningful. Distros shipping something older are probably not going to ship a recent OpenVPN either. Opinions? Cheers,
Am 06.04.21 um 13:51 schrieb Antonio Quartulli: > Hi, > > On 06/04/2021 13:14, Gert Doering wrote: >> Now... what *is* the oldest mbedtls version we should reasonably support? >> >> For OpenSSL, we're stuck to 1.0.2 for the time being as that's still >> the primary (and bugfix-backported) version on FreeBSD 11 and on RHEL >> versions still supported. For mbedTLS I have no idea. > > Good question. I was wondering the same. > > Debian 10 (stable) is on mbedtls-2.16.0 > CentOS 8 is on mbedtls-2.16.9 > Fedora EPEL 8 (and up to Fedora 35) is on mbedtls-2.16.9 > > ** Ubuntu 18.04 is on mbedtls-2.8.0 ** > Ubuntu 20.04 is on mbedtls-2.16.4 > > At this point I believe that assuming mbedtls >= 2.16.0 is meaningful. > > Distros shipping something older are probably not going to ship a recent > OpenVPN either. > > Opinions? > If we adjust the minimum mbed TLS version we should also change the check in configure.ac that checks for the minimum version as well. mbed TLS 2.16.0 has been released end of 2018, so that version is "only" a bit over two years old. Currently mbed TLS 2.7 is still a supported LTS release, so if it is not too much effort, I think we should still support it. Arne
Hi, On 06-04-2021 12:55, Maximilian Fillinger wrote: >> Am 02.04.21 um 15:26 schrieb Max Fillinger: >>> From: Uipko Berghuis <uipko.berghuis@fox-it.com> >>> >>> In mbedtls 2.16.0 mbedtls_ctr_drbg_update() changed to >>> mbedtls_ctr_drbg_update_ret(). Change the function name and handle the >>> new return value error code. >>> --- >>> src/openvpn/ssl_mbedtls.c | 5 ++++- >>> 1 file changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c >>> index 5d7af351..56e9f045 100644 >>> --- a/src/openvpn/ssl_mbedtls.c >>> +++ b/src/openvpn/ssl_mbedtls.c >>> @@ -950,7 +950,10 @@ tls_ctx_personalise_random(struct tls_root_ctx >>> *ctx) >>> >>> if (0 != memcmp(old_sha256_hash, sha256_hash, >> sizeof(sha256_hash))) >>> { >>> - mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32); >>> + if (!mbed_ok(mbedtls_ctr_drbg_update_ret(cd_ctx, >> sha256_hash, 32))) >>> + { >>> + msg(M_WARN, "WARNING: failed to personalise random, >> could not update CTR_DRBG"); >>> + } >>> memcpy(old_sha256_hash, sha256_hash, >> sizeof(old_sha256_hash)); >>> } >>> } >>> >> >> This change will break compilation with anything that is < 2.16.0. > > This function is deprecated in 2.16. I don't mind keeping this change to > OpenVPN-NL for now, but for future reference, what's the best solution > when a new version of mbedtls removes the function? I'd say add a compat-wrapper, like we have many for openssl. Possibly in compat-mbedtls.h (mimicing the openssl code) or just in crypto_mbedtls.h if we don't have many. Something like (untested/"pseudo"code): #if MBEDTLS_VERSION < 2.16 static inline int mbedtls_ctr_drbg_update_ret(ctx, h, len) { mbedtls_ctr_drbg_update(ctx, h, len); return 0; } #endif -Steffan
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c index 5d7af351..56e9f045 100644 --- a/src/openvpn/ssl_mbedtls.c +++ b/src/openvpn/ssl_mbedtls.c @@ -950,7 +950,10 @@ tls_ctx_personalise_random(struct tls_root_ctx *ctx) if (0 != memcmp(old_sha256_hash, sha256_hash, sizeof(sha256_hash))) { - mbedtls_ctr_drbg_update(cd_ctx, sha256_hash, 32); + if (!mbed_ok(mbedtls_ctr_drbg_update_ret(cd_ctx, sha256_hash, 32))) + { + msg(M_WARN, "WARNING: failed to personalise random, could not update CTR_DRBG"); + } memcpy(old_sha256_hash, sha256_hash, sizeof(old_sha256_hash)); } }
From: Uipko Berghuis <uipko.berghuis@fox-it.com> In mbedtls 2.16.0 mbedtls_ctr_drbg_update() changed to mbedtls_ctr_drbg_update_ret(). Change the function name and handle the new return value error code. --- src/openvpn/ssl_mbedtls.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)