diff mbox

[FFmpeg-devel] openssl: Allow newer TLS versions than TLSv1

Message ID 914b948d-3aca-9336-b136-61d84ca9d39b@jkqxz.net
State Accepted
Headers show

Commit Message

Mark Thompson Oct. 29, 2016, 8:53 a.m. UTC
The use of TLSv1_*_method() disallows newer protocol versions; instead
use SSLv23_*_method() and then explicitly disable the deprecated
protocol versions which should not be supported.

Fixes ticket #5915.
---
On 28/10/16 22:15, Hendrik Leppkes wrote:
> I should have looked further when commenting on the other patch - I guess. :)
> Looks good to me, the OpenSSL API seems to be rather confusing in this
> regard. Maybe a comment might be  useful to indicate why this is done.

Hopefully this is clearer.

Thanks,

- Mark


 libavformat/tls_openssl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer Oct. 29, 2016, 9:57 p.m. UTC | #1
On Sat, Oct 29, 2016 at 09:53:30AM +0100, Mark Thompson wrote:
> The use of TLSv1_*_method() disallows newer protocol versions; instead
> use SSLv23_*_method() and then explicitly disable the deprecated
> protocol versions which should not be supported.
> 
> Fixes ticket #5915.
> ---
> On 28/10/16 22:15, Hendrik Leppkes wrote:
> > I should have looked further when commenting on the other patch - I guess. :)
> > Looks good to me, the OpenSSL API seems to be rather confusing in this
> > regard. Maybe a comment might be  useful to indicate why this is done.
> 
> Hopefully this is clearer.
> 
> Thanks,
> 
> - Mark
> 
> 
>  libavformat/tls_openssl.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

should be ok

thx

[...]
Mark Thompson Oct. 30, 2016, 1:21 p.m. UTC | #2
On 29/10/16 22:57, Michael Niedermayer wrote:
> On Sat, Oct 29, 2016 at 09:53:30AM +0100, Mark Thompson wrote:
>> The use of TLSv1_*_method() disallows newer protocol versions; instead
>> use SSLv23_*_method() and then explicitly disable the deprecated
>> protocol versions which should not be supported.
>>
>> Fixes ticket #5915.
>> ---
>> On 28/10/16 22:15, Hendrik Leppkes wrote:
>>> I should have looked further when commenting on the other patch - I guess. :)
>>> Looks good to me, the OpenSSL API seems to be rather confusing in this
>>> regard. Maybe a comment might be  useful to indicate why this is done.
>>
>> Hopefully this is clearer.
>>
>> Thanks,
>>
>> - Mark
>>
>>
>>  libavformat/tls_openssl.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> should be ok
> 
> thx

Applied.

Thanks,

- Mark
diff mbox

Patch

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index c551ac7..178ca9e 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -233,12 +233,17 @@  static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
     if ((ret = ff_tls_open_underlying(c, h, uri, options)) < 0)
         goto fail;

-    p->ctx = SSL_CTX_new(c->listen ? TLSv1_server_method() : TLSv1_client_method());
+    // We want to support all versions of TLS >= 1.0, but not the deprecated
+    // and insecure SSLv2 and SSLv3.  Despite the name, SSLv23_*_method()
+    // enables support for all versions of SSL and TLS, and we then disable
+    // support for the old protocols immediately after creating the context.
+    p->ctx = SSL_CTX_new(c->listen ? SSLv23_server_method() : SSLv23_client_method());
     if (!p->ctx) {
         av_log(h, AV_LOG_ERROR, "%s\n", ERR_error_string(ERR_get_error(), NULL));
         ret = AVERROR(EIO);
         goto fail;
     }
+    SSL_CTX_set_options(p->ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
     if (c->ca_file) {
         if (!SSL_CTX_load_verify_locations(p->ctx, c->ca_file, NULL))
             av_log(h, AV_LOG_ERROR, "SSL_CTX_load_verify_locations %s\n", ERR_error_string(ERR_get_error(), NULL));