[FFmpeg-devel] [PATCH 3/4] lavf/tls_openssl: on 1.1 or later, verify the server's hostname

[Nicolas George]

Rodger Combs (12019-01-17):
> ---
>  libavformat/tls_openssl.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
> index 493f43e610..bdc4985bad 100644
> --- a/libavformat/tls_openssl.c
> +++ b/libavformat/tls_openssl.c
> @@ -35,6 +35,7 @@
>  #include <openssl/bio.h>
>  #include <openssl/ssl.h>
>  #include <openssl/err.h>
> +#include <openssl/x509v3.h>
>  
>  static int openssl_init;
>  
> @@ -269,8 +270,6 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
>          ret = AVERROR(EIO);
>          goto fail;
>      }
> -    // Note, this doesn't check that the peer certificate actually matches
> -    // the requested hostname.
>      if (c->verify)
>          SSL_CTX_set_verify(p->ctx, SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL);
>      p->ssl = SSL_new(p->ctx);
> @@ -294,8 +293,23 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
>      bio->ptr = c->tcp;
>  #endif
>      SSL_set_bio(p->ssl, bio, bio);
> -    if (!c->listen && !c->numerichost)
> -        SSL_set_tlsext_host_name(p->ssl, c->host);

> +    if (!c->listen && !c->numerichost) {
> +#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
> +        X509_VERIFY_PARAM *param = SSL_get0_param(p->ssl);
> +        X509_VERIFY_PARAM_set_hostflags(param, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
> +#endif
> +        if (
> +#if OPENSSL_VERSION_NUMBER >= 0x1010000fL
> +            // Note, if on OpenSSL prior to 1.1.0, we won't check that
> +            // the peer certificate actually matches the requested hostname.
> +            !X509_VERIFY_PARAM_set1_host(param, c->host, 0) ||
> +#endif
> +            !SSL_set_tlsext_host_name(p->ssl, c->host)) {
> +            av_log(h, AV_LOG_ERROR, "%s\n", ERR_error_string(ERR_get_error(), NULL));
> +            ret = AVERROR(EIO);
> +            goto fail;
> +        }
> +    }

I think AVERROR(EIO) is not the best choice. EPROTO would seem obvious,
but not supported on windows. Otherwise EPERM.

More importantly: with this change, users will no longer be able to
access misconfigured servers. An option to let them bypass the test
would be necessary.

>      ret = c->listen ? SSL_accept(p->ssl) : SSL_connect(p->ssl);
>      if (ret == 0) {
>          av_log(h, AV_LOG_ERROR, "Unable to negotiate TLS/SSL session\n");

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190117/a1f32d3f/attachment.sig>