[Openvpn-devel] Change CTR DRBG update function call to new mbedtls 2.16.0 API

  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(-)
  

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