[FFmpeg-devel] [PATCH] lavf/tls_mbedtls: add support for mbedtls version 3

[FFmpeg-devel] [PATCH] lavf/tls_mbedtls: add support for mbedtls version 3

[Timo Rothenpieler]

- certs.h is gone. Only contains test data, and was not used at all.
- config.h is renamed. Was seemingly not used, so can be removed.
- MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE is gone, instead
  MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE will be thrown.
- mbedtls_pk_parse_keyfile now needs to be passed a properly seeded
  RNG. Hence, move the call to after RNG seeding.

Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org>
---
 libavformat/tls_mbedtls.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/libavformat/tls_mbedtls.c b/libavformat/tls_mbedtls.c
index 5754d0d018..8503523b6d 100644
--- a/libavformat/tls_mbedtls.c
+++ b/libavformat/tls_mbedtls.c
@@ -19,8 +19,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
-#include <mbedtls/certs.h>
-#include <mbedtls/config.h>
+#include <mbedtls/version.h>
 #include <mbedtls/ctr_drbg.h>
 #include <mbedtls/entropy.h>
 #include <mbedtls/net_sockets.h>
@@ -130,9 +129,15 @@ static void handle_pk_parse_error(URLContext *h, int ret)
 static void handle_handshake_error(URLContext *h, int ret)
 {
     switch (ret) {
+#if MBEDTLS_VERSION_MAJOR < 3
     case MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE:
         av_log(h, AV_LOG_ERROR, "None of the common ciphersuites is usable. Was the local certificate correctly set?\n");
         break;
+#else
+    case MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE:
+        av_log(h, AV_LOG_ERROR, "TLS handshake failed.\n");
+        break;
+#endif
     case MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE:
         av_log(h, AV_LOG_ERROR, "A fatal alert message was received from the peer, has the peer a correct certificate?\n");
         break;
@@ -195,16 +200,6 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
         }
     }
 
-    // load key file
-    if (shr->key_file) {
-        if ((ret = mbedtls_pk_parse_keyfile(&tls_ctx->priv_key,
-                                            shr->key_file,
-                                            tls_ctx->priv_key_pw)) != 0) {
-            handle_pk_parse_error(h, ret);
-            goto fail;
-        }
-    }
-
     // seed the random number generator
     if ((ret = mbedtls_ctr_drbg_seed(&tls_ctx->ctr_drbg_context,
                                      mbedtls_entropy_func,
@@ -214,6 +209,21 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
         goto fail;
     }
 
+    // load key file
+    if (shr->key_file) {
+        if ((ret = mbedtls_pk_parse_keyfile(&tls_ctx->priv_key,
+                                            shr->key_file,
+                                            tls_ctx->priv_key_pw
+#if MBEDTLS_VERSION_MAJOR >= 3
+                                            , mbedtls_ctr_drbg_random,
+                                            &tls_ctx->ctr_drbg_context
+#endif
+                                            )) != 0) {
+            handle_pk_parse_error(h, ret);
+            goto fail;
+        }
+    }
+
     if ((ret = mbedtls_ssl_config_defaults(&tls_ctx->ssl_config,
                                            shr->listen ? MBEDTLS_SSL_IS_SERVER : MBEDTLS_SSL_IS_CLIENT,
                                            MBEDTLS_SSL_TRANSPORT_STREAM,
-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavf/tls_mbedtls: add support for mbedtls version 3

[Timo Rothenpieler]

On 24.04.2022 01:32, Timo Rothenpieler wrote:
> - certs.h is gone. Only contains test data, and was not used at all.
> - config.h is renamed. Was seemingly not used, so can be removed.
> - MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE is gone, instead
>    MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE will be thrown.
> - mbedtls_pk_parse_keyfile now needs to be passed a properly seeded
>    RNG. Hence, move the call to after RNG seeding.
> 

I'd really like some review on this, and merge it rather sooner than later.

Also, I think this should be backported to all currently maintained 
branches, since mbedtls<3 is no longer maintained, so lack of support 
for recent versions is a potential security issue.

Would hate to just push it without review, even though it looks trivial 
enough to me, it is touching TLS code.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] lavf/tls_mbedtls: add support for mbedtls version 3

[Jan Ekström]

On Sun, Apr 24, 2022 at 2:33 AM Timo Rothenpieler <timo@rothenpieler.org> wrote:
>
> - certs.h is gone. Only contains test data, and was not used at all.
> - config.h is renamed. Was seemingly not used, so can be removed.
> - MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE is gone, instead
>   MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE will be thrown.
> - mbedtls_pk_parse_keyfile now needs to be passed a properly seeded
>   RNG. Hence, move the call to after RNG seeding.
>
> Signed-off-by: Timo Rothenpieler <timo@rothenpieler.org>
> ---
>  libavformat/tls_mbedtls.c | 34 ++++++++++++++++++++++------------
>  1 file changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/libavformat/tls_mbedtls.c b/libavformat/tls_mbedtls.c
> index 5754d0d018..8503523b6d 100644
> --- a/libavformat/tls_mbedtls.c
> +++ b/libavformat/tls_mbedtls.c
> @@ -19,8 +19,7 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>
> -#include <mbedtls/certs.h>
> -#include <mbedtls/config.h>
> +#include <mbedtls/version.h>
>  #include <mbedtls/ctr_drbg.h>
>  #include <mbedtls/entropy.h>
>  #include <mbedtls/net_sockets.h>
> @@ -130,9 +129,15 @@ static void handle_pk_parse_error(URLContext *h, int ret)
>  static void handle_handshake_error(URLContext *h, int ret)
>  {
>      switch (ret) {
> +#if MBEDTLS_VERSION_MAJOR < 3
>      case MBEDTLS_ERR_SSL_NO_USABLE_CIPHERSUITE:
>          av_log(h, AV_LOG_ERROR, "None of the common ciphersuites is usable. Was the local certificate correctly set?\n");
>          break;
> +#else
> +    case MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE:
> +        av_log(h, AV_LOG_ERROR, "TLS handshake failed.\n");
> +        break;
> +#endif
>      case MBEDTLS_ERR_SSL_FATAL_ALERT_MESSAGE:
>          av_log(h, AV_LOG_ERROR, "A fatal alert message was received from the peer, has the peer a correct certificate?\n");
>          break;
> @@ -195,16 +200,6 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
>          }
>      }
>
> -    // load key file
> -    if (shr->key_file) {
> -        if ((ret = mbedtls_pk_parse_keyfile(&tls_ctx->priv_key,
> -                                            shr->key_file,
> -                                            tls_ctx->priv_key_pw)) != 0) {
> -            handle_pk_parse_error(h, ret);
> -            goto fail;
> -        }
> -    }
> -
>      // seed the random number generator
>      if ((ret = mbedtls_ctr_drbg_seed(&tls_ctx->ctr_drbg_context,
>                                       mbedtls_entropy_func,
> @@ -214,6 +209,21 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
>          goto fail;
>      }
>
> +    // load key file
> +    if (shr->key_file) {
> +        if ((ret = mbedtls_pk_parse_keyfile(&tls_ctx->priv_key,
> +                                            shr->key_file,
> +                                            tls_ctx->priv_key_pw
> +#if MBEDTLS_VERSION_MAJOR >= 3
> +                                            , mbedtls_ctr_drbg_random,
> +                                            &tls_ctx->ctr_drbg_context
> +#endif
> +                                            )) != 0) {
> +            handle_pk_parse_error(h, ret);
> +            goto fail;
> +        }
> +    }
> +

Seems correct enough as the RNG is then utilized in the TLS
configuration as well.

As long as the mbedtls API documentation notes that it is OK to
utilize the same RNG for both this as well as the TLS configuration
itself, this seems fine.

Jan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".