| Message ID | 20220423233208.27071-1-timo@rothenpieler.org |
|---|---|
| State | Accepted |
| Commit | 4fc806bbe612784286f123af92a0d5d0da333064 |
| Headers | show |
| Series | [FFmpeg-devel] lavf/tls_mbedtls: add support for mbedtls version 3 | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
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.
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
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,
- 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(-)