| Message ID | a865831e-4cca-4184-b99f-89bae5280a86@skybound.link |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] libavformat/tls_mbedtls: Changes the return code handling of mbedtls_x509_crt_parse_file | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configure_x86 | warning | Failed to apply patch |
LGTM after checking documentation of mbedtls_x509_crt_parse_file()
> av_log(h, AV_LOG_DEBUG, "mbedtls_x509_crt_parse_file skipped %d
certificate(s)\n", ret);
Is it worth it making this a higher log level? Or is it too much noise?
Thinking if it’s important security information to share with the user.
Could do. What level were you thinking? WARN? On 14/07/2024 17:29, Marth64 wrote: >> av_log(h, AV_LOG_DEBUG, "mbedtls_x509_crt_parse_file skipped %d > certificate(s)\n", ret); > > Is it worth it making this a higher log level? Or is it too much noise? > Thinking if it’s important security information to share with the user. > _______________________________________________ > 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".
> Could do. What level were you thinking? WARN?
How about,
```
av_log(h, AV_LOG_WARNING, "Failed to process %d certificate(s) from
the CA bundle, ignoring these certificates\n", ret);
```
Thank you,
Sounds good. Sorry first time contributing, should I make another patch with the change and send that through again with git send-email? Thanks Mohit On Tue, Jul 16, 2024 at 2:26 AM Marth64 <marth64@proxyid.net> wrote: > > Could do. What level were you thinking? WARN? > > How about, > ``` > av_log(h, AV_LOG_WARNING, "Failed to process %d certificate(s) from > the CA bundle, ignoring these certificates\n", ret); > ``` > > > Thank you, > _______________________________________________ > 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". >
Disclaimer: I am a light contributor and do not have access to actually merge this patch, only making suggestions based on my learnings. > Sounds good. Sorry first time contributing, should I make another patch > with the change and send that through again with git send-email? Yes, except when you do git format-patch, specify "-v2" at the end and provide the message ID of your original email, so it processes as a reply. This will prefix the patch subject with "V2" to let readers know you have updated it. Thank you!
diff --git a/libavformat/tls_mbedtls.c b/libavformat/tls_mbedtls.c index 567b95b..97094e3 100644 --- a/libavformat/tls_mbedtls.c +++ b/libavformat/tls_mbedtls.c @@ -223,9 +223,11 @@ static int tls_open(URLContext *h, const char *uri, int flags, AVDictionary **op // load trusted CA if (shr->ca_file) { - if ((ret = mbedtls_x509_crt_parse_file(&tls_ctx->ca_cert, shr->ca_file)) != 0) { + if ((ret = mbedtls_x509_crt_parse_file(&tls_ctx->ca_cert, shr->ca_file)) < 0) { av_log(h, AV_LOG_ERROR, "mbedtls_x509_crt_parse_file for CA cert returned %d\n", ret); goto fail; + } else if (ret > 0) { + av_log(h, AV_LOG_DEBUG, "mbedtls_x509_crt_parse_file
mbedtls_x509_crt_parse_file returns an error with negative numbers, and positive numbers indicate the number of failed certificates to load from certificate specific issues, such as critical extensions. This would fix ticket #11079. Signed-off-by: Mohit Gupta <git@skybound.link> --- libavformat/tls_mbedtls.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) skipped %d certificate(s)\n", ret); } } -- 2.45.2