[FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[Marvin Scholz]

From: Daniel N Pettersson <danielnp@axis.com>

Co-Authored-By: Marvin Scholz <epirat07@gmail.com>
---
 libavformat/tls_openssl.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index a0fa3285d5..7614caf089 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -921,8 +921,15 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
     ret = init_bio_method(h);
     if (ret < 0)
         goto fail;
-    if (!c->listen && !c->numerichost)
+    if (!c->listen && !c->numerichost) {
+        if (!SSL_set1_host(p->ssl, c->host)) {
+            av_log(h, AV_LOG_ERROR, "Failed to set hostname for TLS/SSL verification: %s\n",
+                openssl_get_error(p));
+            ret = AVERROR(EIO);
+            goto fail;
+        }
         SSL_set_tlsext_host_name(p->ssl, c->host);
+    }
     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.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/3] avformat/tls_openssl: verify setting hostname for SNI

[Marvin Scholz]

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

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index 7614caf089..e65914f11a 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -928,7 +928,11 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
             ret = AVERROR(EIO);
             goto fail;
         }
-        SSL_set_tlsext_host_name(p->ssl, c->host);
+        if (!SSL_set_tlsext_host_name(p->ssl, c->host)) {
+            av_log(h, AV_LOG_ERROR, "Failed to set hostname for SNI: %s\n", openssl_get_error(p));
+            ret = AVERROR(EIO);
+            goto fail;
+        }
     }
     ret = c->listen ? SSL_accept(p->ssl) : SSL_connect(p->ssl);
     if (ret == 0) {
-- 
2.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/3] avformat/tls_openssl: load default verify locations

[Marvin Scholz]

When no explicit CAs file is set, load the default locations,
else there is no way for verification to succeed.

This matches the behavior of other TLS backends.
---
 libavformat/tls_openssl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index e65914f11a..d112043977 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -698,6 +698,12 @@ static av_cold int openssl_init_ca_key_cert(URLContext *h)
     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", openssl_get_error(p));
+    } else {
+        if (!SSL_CTX_set_default_verify_paths(p->ctx)) {
+            // Only log the failure but do not error out, as this is not fatal
+            av_log(h, AV_LOG_WARNING, "Failure setting default verify locations: %s\n",
+                openssl_get_error(p));
+        }
     }
 
     if (c->cert_file) {
-- 
2.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[Nicolas George]

Marvin Scholz (HE12025-07-08):
> From: Daniel N Pettersson <danielnp@axis.com>
> 
> Co-Authored-By: Marvin Scholz <epirat07@gmail.com>
> ---
>  libavformat/tls_openssl.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
> index a0fa3285d5..7614caf089 100644
> --- a/libavformat/tls_openssl.c
> +++ b/libavformat/tls_openssl.c
> @@ -921,8 +921,15 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
>      ret = init_bio_method(h);
>      if (ret < 0)
>          goto fail;
> -    if (!c->listen && !c->numerichost)
> +    if (!c->listen && !c->numerichost) {

> +        if (!SSL_set1_host(p->ssl, c->host)) {

Must be optional.

> +            av_log(h, AV_LOG_ERROR, "Failed to set hostname for TLS/SSL verification: %s\n",
> +                openssl_get_error(p));

> +            ret = AVERROR(EIO);

AVERROR_EXTERNAL

> +            goto fail;
> +        }
>          SSL_set_tlsext_host_name(p->ssl, c->host);
> +    }
>      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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/tls_openssl: verify setting hostname for SNI

[Nicolas George]

Marvin Scholz (HE12025-07-08):
> ---
>  libavformat/tls_openssl.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
> index 7614caf089..e65914f11a 100644
> --- a/libavformat/tls_openssl.c
> +++ b/libavformat/tls_openssl.c
> @@ -928,7 +928,11 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
>              ret = AVERROR(EIO);
>              goto fail;
>          }
> -        SSL_set_tlsext_host_name(p->ssl, c->host);
> +        if (!SSL_set_tlsext_host_name(p->ssl, c->host)) {
> +            av_log(h, AV_LOG_ERROR, "Failed to set hostname for SNI: %s\n", openssl_get_error(p));

> +            ret = AVERROR(EIO);

AVERROR_EXTERNAL

> +            goto fail;
> +        }
>      }
>      ret = c->listen ? SSL_accept(p->ssl) : SSL_connect(p->ssl);
>      if (ret == 0) {
> -- 
> 2.39.5 (Apple Git-154)
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
  “I dont see why” isnt an argument. Proposing better is.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[Marvin Scholz]

On 8 Jul 2025, at 20:16, Nicolas George wrote:

> Marvin Scholz (HE12025-07-08):
>> From: Daniel N Pettersson <danielnp@axis.com>
>>
>> Co-Authored-By: Marvin Scholz <epirat07@gmail.com>
>> ---
>>  libavformat/tls_openssl.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
>> index a0fa3285d5..7614caf089 100644
>> --- a/libavformat/tls_openssl.c
>> +++ b/libavformat/tls_openssl.c
>> @@ -921,8 +921,15 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
>>      ret = init_bio_method(h);
>>      if (ret < 0)
>>          goto fail;
>> -    if (!c->listen && !c->numerichost)
>> +    if (!c->listen && !c->numerichost) {
>
>> +        if (!SSL_set1_host(p->ssl, c->host)) {
>
> Must be optional.

Can you clarify?

>
>> +            av_log(h, AV_LOG_ERROR, "Failed to set hostname for TLS/SSL verification: %s\n",
>> +                openssl_get_error(p));
>
>> +            ret = AVERROR(EIO);
>
> AVERROR_EXTERNAL
>
>> +            goto fail;
>> +        }
>>          SSL_set_tlsext_host_name(p->ssl, c->host);
>> +    }
>>      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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[Nicolas George]

Marvin Scholz (HE12025-07-08):
> Can you clarify?

wget(1):

       --no-check-certificate
           Don't check the server certificate against the available certificate
           authorities.  Also don't require the URL  host  name  to  match  the
           common name presented by the certificate.

curl(1):
       -k, --insecure
              (TLS SFTP SCP) By default, every secure connection curl makes  is
              verified  to  be secure before the transfer takes place. This op‐
              tion makes curl skip the verification step  and  proceed  without
              checking.

w3m/README:
   ssl_verify_server ON/OFF
       Verify the SSL server (default: ON)

And so on.

Should probably be off by default for now, too.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[Marvin Scholz]

On 8 Jul 2025, at 20:21, Nicolas George wrote:

> Marvin Scholz (HE12025-07-08):
>> Can you clarify?

That's already possible with `-tls_verify 0`
(which is actually the default, arguably shouldn't be IMHO but
thats a different topic)

>
> wget(1):
>
>        --no-check-certificate
>            Don't check the server certificate against the available certificate
>            authorities.  Also don't require the URL  host  name  to  match  the
>            common name presented by the certificate.
>
> curl(1):
>        -k, --insecure
>               (TLS SFTP SCP) By default, every secure connection curl makes  is
>               verified  to  be secure before the transfer takes place. This op‐
>               tion makes curl skip the verification step  and  proceed  without
>               checking.
>
> w3m/README:
>    ssl_verify_server ON/OFF
>        Verify the SSL server (default: ON)
>
> And so on.
>
> Should probably be off by default for now, too.
>
> Regards,
>
> -- 
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/tls_openssl: verify setting hostname for SNI

[Marvin Scholz]

On 8 Jul 2025, at 20:16, Nicolas George wrote:

> Marvin Scholz (HE12025-07-08):
>> ---
>>  libavformat/tls_openssl.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
>> index 7614caf089..e65914f11a 100644
>> --- a/libavformat/tls_openssl.c
>> +++ b/libavformat/tls_openssl.c
>> @@ -928,7 +928,11 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
>>              ret = AVERROR(EIO);
>>              goto fail;
>>          }
>> -        SSL_set_tlsext_host_name(p->ssl, c->host);
>> +        if (!SSL_set_tlsext_host_name(p->ssl, c->host)) {
>> +            av_log(h, AV_LOG_ERROR, "Failed to set hostname for SNI: %s\n", openssl_get_error(p));
>
>> +            ret = AVERROR(EIO);
>
> AVERROR_EXTERNAL
>

Indeed, will send a new set with the error code changed.

We should probably eventually change most of the other ones to that
as well, as right now nearly all are AVERROR(EIO) even if its not an
IO error at all.

>> +            goto fail;
>> +        }
>>      }
>>      ret = c->listen ? SSL_accept(p->ssl) : SSL_connect(p->ssl);
>>      if (ret == 0) {
>> -- 
>> 2.39.5 (Apple Git-154)
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
> -- 
>   “I dont see why” isnt an argument. Proposing better is.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 1/3] avformat/tls_openssl: add host verification

[Marvin Scholz]

From: Daniel N Pettersson <danielnp@axis.com>

Co-Authored-By: Marvin Scholz <epirat07@gmail.com>
---
 libavformat/tls_openssl.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index a0fa3285d5..7f10d80f9e 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -906,8 +906,7 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
     }
     ret = openssl_init_ca_key_cert(h);
     if (ret < 0) 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);
@@ -921,8 +920,15 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
     ret = init_bio_method(h);
     if (ret < 0)
         goto fail;
-    if (!c->listen && !c->numerichost)
+    if (!c->listen && !c->numerichost) {
+        if (!SSL_set1_host(p->ssl, c->host)) {
+            av_log(h, AV_LOG_ERROR, "Failed to set hostname for TLS/SSL verification: %s\n",
+                openssl_get_error(p));
+            ret = AVERROR_EXTERNAL;
+            goto fail;
+        }
         SSL_set_tlsext_host_name(p->ssl, c->host);
+    }
     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.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 2/3] avformat/tls_openssl: verify setting hostname for SNI

[Marvin Scholz]

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

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index 7f10d80f9e..248d1eedf9 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -927,7 +927,11 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
             ret = AVERROR_EXTERNAL;
             goto fail;
         }
-        SSL_set_tlsext_host_name(p->ssl, c->host);
+        if (!SSL_set_tlsext_host_name(p->ssl, c->host)) {
+            av_log(h, AV_LOG_ERROR, "Failed to set hostname for SNI: %s\n", openssl_get_error(p));
+            ret = AVERROR_EXTERNAL;
+            goto fail;
+        }
     }
     ret = c->listen ? SSL_accept(p->ssl) : SSL_connect(p->ssl);
     if (ret == 0) {
-- 
2.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 3/3] avformat/tls_openssl: load default verify locations

[Marvin Scholz]

When no explicit CAs file is set, load the default locations,
else there is no way for verification to succeed.

This matches the behavior of other TLS backends.
---
 libavformat/tls_openssl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index 248d1eedf9..d360dd320c 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -698,6 +698,12 @@ static av_cold int openssl_init_ca_key_cert(URLContext *h)
     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", openssl_get_error(p));
+    } else {
+        if (!SSL_CTX_set_default_verify_paths(p->ctx)) {
+            // Only log the failure but do not error out, as this is not fatal
+            av_log(h, AV_LOG_WARNING, "Failure setting default verify locations: %s\n",
+                openssl_get_error(p));
+        }
     }
 
     if (c->cert_file) {
-- 
2.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[Nicolas George]

Marvin Scholz (HE12025-07-08):
> That's already possible with `-tls_verify 0`

Then the commit message inadequately explains what the patch does.
Please clarify.

> (which is actually the default, arguably shouldn't be IMHO but
> thats a different topic)

A transition period where only a warning is printed would be necessary.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/tls_openssl: verify setting hostname for SNI

[Nicolas George]

Marvin Scholz (HE12025-07-08):
> We should probably eventually change most of the other ones to that
> as well, as right now nearly all are AVERROR(EIO) even if its not an
> IO error at all.

You are certainly right. A lot of this code predates the addition of
AVERROR_EXTERNAL.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[Marvin Scholz]

On 8 Jul 2025, at 20:33, Nicolas George wrote:

> Marvin Scholz (HE12025-07-08):
>> That's already possible with `-tls_verify 0`
>
> Then the commit message inadequately explains what the patch does.
> Please clarify.

Sure, I will add a more verbose message.

However note that verification was already done before
my patch, when enabled, just not taking the host into account
but all other aspects of the cert.

>
>> (which is actually the default, arguably shouldn't be IMHO but
>> thats a different topic)
>
> A transition period where only a warning is printed would be necessary.

How could that work though? Warn for every tls use
in ffmpeg unless the user explicitly specifies
-tls_verify 1 or -tls_verify 0?
I think a lot of people would complain about that?

But I agree we probably can't just change the behavior without making
people aware of it before…

>
> Regards,
>
> -- 
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/tls_openssl: add host verification

[Nicolas George]

Marvin Scholz (HE12025-07-08):
> Sure, I will add a more verbose message.

Thanks.

> How could that work though? Warn for every tls use
> in ffmpeg unless the user explicitly specifies
> -tls_verify 1 or -tls_verify 0?
> I think a lot of people would complain about that?

Warn each time verification fails unless explicitly specified 0 or 1.

Regards,

-- 
  Nicolas George
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 1/3] avformat/tls_openssl: add hostname for verification

[Marvin Scholz]

From: Daniel N Pettersson <danielnp@axis.com>

When verification is enabled (using -tls_verify 1) now
the hostname will be verified properly too, while before
only other aspects of the certificate were checked.

Co-Authored-By: Marvin Scholz <epirat07@gmail.com>
---
 libavformat/tls_openssl.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index a0fa3285d5..7f10d80f9e 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -906,8 +906,7 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
     }
     ret = openssl_init_ca_key_cert(h);
     if (ret < 0) 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);
@@ -921,8 +920,15 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
     ret = init_bio_method(h);
     if (ret < 0)
         goto fail;
-    if (!c->listen && !c->numerichost)
+    if (!c->listen && !c->numerichost) {
+        if (!SSL_set1_host(p->ssl, c->host)) {
+            av_log(h, AV_LOG_ERROR, "Failed to set hostname for TLS/SSL verification: %s\n",
+                openssl_get_error(p));
+            ret = AVERROR_EXTERNAL;
+            goto fail;
+        }
         SSL_set_tlsext_host_name(p->ssl, c->host);
+    }
     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.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 2/3] avformat/tls_openssl: verify setting hostname for SNI

[Marvin Scholz]

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

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index 7f10d80f9e..248d1eedf9 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -927,7 +927,11 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op
             ret = AVERROR_EXTERNAL;
             goto fail;
         }
-        SSL_set_tlsext_host_name(p->ssl, c->host);
+        if (!SSL_set_tlsext_host_name(p->ssl, c->host)) {
+            av_log(h, AV_LOG_ERROR, "Failed to set hostname for SNI: %s\n", openssl_get_error(p));
+            ret = AVERROR_EXTERNAL;
+            goto fail;
+        }
     }
     ret = c->listen ? SSL_accept(p->ssl) : SSL_connect(p->ssl);
     if (ret == 0) {
-- 
2.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v3 3/3] avformat/tls_openssl: load default verify locations

[Marvin Scholz]

When no explicit CAs file is set, load the default locations,
else there is no way for verification to succeed.

This matches the behavior of other TLS backends.
---
 libavformat/tls_openssl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
index 248d1eedf9..d360dd320c 100644
--- a/libavformat/tls_openssl.c
+++ b/libavformat/tls_openssl.c
@@ -698,6 +698,12 @@ static av_cold int openssl_init_ca_key_cert(URLContext *h)
     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", openssl_get_error(p));
+    } else {
+        if (!SSL_CTX_set_default_verify_paths(p->ctx)) {
+            // Only log the failure but do not error out, as this is not fatal
+            av_log(h, AV_LOG_WARNING, "Failure setting default verify locations: %s\n",
+                openssl_get_error(p));
+        }
     }
 
     if (c->cert_file) {
-- 
2.39.5 (Apple Git-154)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".