Re: [FFmpeg-devel] [PATCH 3/4] lavf/tls_openssl: on 1.1 or later, verify the server's hostname
Rodger Combs (12019-01-17): > What kind of misconfiguration are you referring to? The actual A server with a certificate on a different name. > verification is still gated behind the tls_verify option; if that's > set to 0, this won't actually do anything. Ok, that is good. Thanks for clarifying. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/4] lavf/tls_openssl: on 1.1 or later, verify the server's hostname
> On Jan 17, 2019, at 03:09, Nicolas George wrote: > > Signed PGP part > 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 >> #include >> #include >> +#include >> >> 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 >= 0x101fL >> +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 >= 0x101fL >> +// 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. What kind of misconfiguration are you referring to? The actual verification is still gated behind the tls_verify option; if that's set to 0, this won't actually do anything. (Well, it'll result in OpenSSL potentially generating a different error code internally, and then discarding it because we didn't enable verification). > >> 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 > > signature.asc Description: Message signed with OpenPGP ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 3/4] lavf/tls_openssl: on 1.1 or later, verify the server's hostname
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 > #include > #include > +#include > > 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 >= 0x101fL > +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 >= 0x101fL > +// 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 signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/4] lavf/tls_openssl: on 1.1 or later, verify the server's hostname
--- 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 #include #include +#include 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 >= 0x101fL +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 >= 0x101fL +// 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; +} +} 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"); -- 2.19.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel