> On Jan 17, 2019, at 03:09, Nicolas George <geo...@nsup.org> 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 <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.
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
