options: enable tls-verify by default #17909
Conversation
|
Can you limit this to curl backend only? ffmpeg does not sensibly try to search for system CA store, so in practice nothing works unless manually specifying CA file. |
This depends on the TLS backend. schannel clearly will use CA store, while for openssl, on Linux, distributions usually handle it. By the way, Windows CI builds all use schannel, and although zhongfly builds use openssl, it has been patched to use CA store by default.
There is no difference. When curl is built with schannel backend, CA store is used by default, whereas built with openssl backend, CA store is not used unless curl is built with CURL_CA_NATIVE=ON. |
I posted this more as a RFC, because I'm aware some broken systems may have issues with this. However, https is prevalent and having broken CA store or not usable is rather user error at this point in time. FYI, I was recently testing all things with this option enabled, which also affects ffmpeg, and it works fine, but I indeed use schannel on Windows. We could set explicit CA to ffmpeg if curl is linked in, to the same file. There are still issues that we use http.c from lavf in some cases, because of https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23071. All in all, I would prefer to have it enabled, even if it cause breakage. I know it sucks, but user can mitigate it, by updating their settings. Also some remaining issues may be resolved on the packaging side. |
|
Note that FFmpeg will enable TLS verification on next major version bump. FFmpeg/FFmpeg@5621eee And version bump is coming with big steps https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/22455 |
This affects all linux systems out of box. Certificate package is already installed. But CA search still needs to be handled by curl or tls library.
This is where the issue is. mpv should let curl search for CA store for openssl backend, and only enables tls verify be default if this can be done.
This is only done after FFmpeg/FFmpeg@080dc4c which lets it search for CA so tls verify works by default. The option should not be enabled by default on older versions. |
So it's package issue of libcurl, not ours.
I don't think this is valid. This breaks expectation that TLS is verified. We do verify when we feel like it? It should be the conscious decision of the user to either disable verification or fix CA.
Sure, I will gate it with the same api level. |
curl can search for native CA store with
This is similar to the ffmpeg situation. If curl can attempt to perform CA search then tls verify should be enabled by default. If it is not supported then it should be disabled. |
I can enable |
|
Updated, now it enabled by default only when libcurl is used or for lavf > 63 |
|
It would be best to log a warning when tls-verify cannot be enabled by default. |
|
It's build time check, not sure it's worth logging that. Eventually it will go away as mpv migrates to libcurl and ffmpeg itself is updated. In the meantime it remains as it were, disabled. |
This is independent of other CA certificate locations set at run time or build time. Those locations are searched in addition to the native CA store.
We shouldn't default to no verification in current year. User can opt-out if needed. Keep it disable for builds without libcurl when libavformat is older than 63.0.100. This is done to match ffmpeg's timeline for enabling tls_verify by default in lavf, also to ensure we the default verify locations will be loaded on such version.
kasper93
force-pushed
the
tls-verify
branch
2 times, most recently
from
eee5239 to
745c96a
Compare
I'm not sure what the deal with
|
? |
I don't know either. But they seem to want this enabled, so I don't mind doing so. |
In places where Windows does not support Schannel (UWP, Xbox, etc.), using OpenSSL+libcurl and CURLSSLOPT_NATIVE_CA is one way to solve the problem |
5 participants
No description provided.