[FFmpeg-devel] [PATCH 3/3] lavf/tls: verify TLS connections by default whenever possible
Rodger Combs
rodger.combs at gmail.com
Sat Aug 19 03:03:07 EEST 2017
> On Aug 16, 2017, at 06:29, wm4 <nfxjfg at googlemail.com> wrote:
>
> On Wed, 16 Aug 2017 02:19:18 -0500
> Rodger Combs <rodger.combs at gmail.com <mailto:rodger.combs at gmail.com>> wrote:
>
>> This makes a reasonable effort to set the default configuration to behave
>> securely, while maintaining the ability for consumers to produce builds using
>> the old behavior without making changes to their runtime code.
>>
>> On Secure Transport and Secure Channel, we use a system-provided trust store,
>> so we don't have to worry about providing our own. On OpenSSL and GNUTLS, we
>> search for a default CA bundle path in the same locations as curl does in their
>> configure script. If this fails (or the user disabled it by setting the path
>> to an empty string), we turn off verification by default.
>>
>> The user can also set an explicit default CA path (which applies on any TLS
>> engine), and explicitly enable or disable the new verify-by-default behavior.
>>
>> When verification is turned off at compile-time (as opposed to runtime), we
>> log a warning indicating that this is the case, and informing the user of how
>> they can turn on verification.
>>
>> Other options that were considered, but deemed too complex (for now) include:
>> - Including a default trust store for OpenSSL and GNUTLS within the libavformat
>> library, to be read from the build machine or fetched online at compile-time
>> (a la nodejs).
>> - Installing a library in the ffmpeg data directory, to be either copied from
>> the build machine or fetched online at compile-time (a la curl).
>> - Providing an "rpath"-style mechanism to set the default CA bundle path
>> relative to the running executable.
>> ---
>> configure | 29 +++++++++++++++++++++++++++++
>> libavformat/tls.c | 15 +++++++++++++++
>> libavformat/tls.h | 6 +++---
>> 3 files changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 7201941c36..aff5bf3bc7 100755
>> --- a/configure
>> +++ b/configure
>> @@ -374,6 +374,8 @@ Advanced options (experts only):
>> disable buffer boundary checking in bitreaders
>> (faster, but may crash)
>> --sws-max-filter-size=N the max filter size swscale uses [$sws_max_filter_size_default]
>> + --disable-tls-verify disable verifying TLS certificates by default [autodetect]
>> + --ca-bundle-path=PATH path to the default trusted certificate authority list in PEM format [autodetect]
>>
>> Optimization options (experts only):
>> --disable-asm disable all assembly optimizations
>> @@ -1631,6 +1633,7 @@ FEATURE_LIST="
>> small
>> static
>> swscale_alpha
>> + tls_verify
>> "
>>
>> LIBRARY_LIST="
>> @@ -2200,6 +2203,7 @@ CMDLINE_SET="
>> build_suffix
>> cc
>> objcc
>> + ca_bundle_path
>> cpu
>> cross_prefix
>> custom_allocator
>> @@ -6593,6 +6597,25 @@ enabled lavfi_indev && prepend avdevice_deps "avfilter"
>>
>> enabled opus_decoder && prepend avcodec_deps "swresample"
>>
>> +enabled_any tls_securetransport_protocol tls_schannel_protocol && enable_weak tls_verify
>> +enabled_any tls_openssl_protocol tls_gnutls_protocol && test -z ${ca_bundle_path+x} && {
>> + for file in /etc/ssl/certs/ca-certificates.crt \
>> + /etc/pki/tls/certs/ca-bundle.crt \
>> + /usr/share/ssl/certs/ca-bundle.crt \
>> + /usr/local/share/certs/ca-root-nss.crt \
>> + ${prefix}/share/certs/ca-root-nss.crt \
>> + /etc/ssl/cert.pem \
>> + ${prefix}/share/curl/curl-ca-bundle.crt \
>> + /usr/share/curl/curl-ca-bundle.crt \
>> + /usr/local/share/curl/curl-ca-bundle.crt; do
>> + if test -f "$file"; then
>> + ca_bundle_path="$file"
>> + break
>> + fi
>> + done;
>> +}
>> +enabled_any tls_openssl_protocol tls_gnutls_protocol && test -n "${ca_bundle_path}" && enable_weak tls_verify
>> +
>
> Wouldn't it be better to search these at runtime? Doing it at compile
> time probably _will_ break cross compiles by default, unless the person
> building it knows about --ca-bundle-path and actually sets it correctly.
Hmm, I could search at runtime by default, or disable this logic when cross-compiling (and expect people to provide the arg, but at least it wouldn't break anything; just print the warning).
>
>> expand_deps(){
>> lib_deps=${1}_deps
>> eval "deps=\$$lib_deps"
>> @@ -6693,6 +6716,8 @@ echo "postprocessing support ${postproc-no}"
>> echo "network support ${network-no}"
>> echo "threading support ${thread_type-no}"
>> echo "safe bitstream reader ${safe_bitstream_reader-no}"
>> +echo "TLS cert verification ${tls_verify-no}"
>> +echo "CA bundle path ${ca_bundle_path-none}"
>> echo "texi2html enabled ${texi2html-no}"
>> echo "perl enabled ${perl-no}"
>> echo "pod2man enabled ${pod2man-no}"
>> @@ -6909,6 +6934,10 @@ cat > $TMPH <<EOF
>> #define SWS_MAX_FILTER_SIZE $sws_max_filter_size
>> EOF
>>
>> +test -n "$ca_bundle_path" &&
>> + echo "#define CA_BUNDLE_PATH \"$(c_escape $ca_bundle_path)\"" >>$TMPH ||
>> + echo "#define CA_BUNDLE_PATH NULL" >>$TMPH
>> +
>> test -n "$assert_level" &&
>> echo "#define ASSERT_LEVEL $assert_level" >>$TMPH
>>
>> diff --git a/libavformat/tls.c b/libavformat/tls.c
>> index 10e0792e29..b1ce6529c8 100644
>> --- a/libavformat/tls.c
>> +++ b/libavformat/tls.c
>> @@ -64,6 +64,21 @@ int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AV
>>
>> set_options(c, uri);
>>
>> + if (c->verify < 0) {
>> + c->verify = CONFIG_TLS_VERIFY;
>> +#if CONFIG_TLS_VERIFY == 0
>> + av_log(parent, AV_LOG_WARNING, "ffmpeg was configured without TLS verification enabled,\n"
>> + "so this connection will be made insecurely.\n"
>> + "To make this connection securely, enable the 'tls_verify' option%s"
>> + ".\n",
>> +#if (CONFIG_TLS_GNUTLS_PROTOCOL || CONFIG_TLS_OPENSSL_PROTOCOL)
>> + (CA_BUNDLE_PATH == NULL) ?
>> + "\nand specify a path to a trusted-root bundle with the 'ca_file' option" :
>> +#endif
>> + "");
>> +#endif
>> + }
>> +
>> if (c->listen)
>> snprintf(opts, sizeof(opts), "?listen=1");
>>
>> diff --git a/libavformat/tls.h b/libavformat/tls.h
>> index 53d0634f49..033d18c8db 100644
>> --- a/libavformat/tls.h
>> +++ b/libavformat/tls.h
>> @@ -45,9 +45,9 @@ typedef struct TLSShared {
>>
>> #define TLS_OPTFL (AV_OPT_FLAG_DECODING_PARAM | AV_OPT_FLAG_ENCODING_PARAM)
>> #define TLS_COMMON_OPTIONS(pstruct, options_field) \
>> - {"ca_file", "Certificate Authority database file", offsetof(pstruct, options_field . ca_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \
>> - {"cafile", "Certificate Authority database file", offsetof(pstruct, options_field . ca_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \
>> - {"tls_verify", "Verify the peer certificate", offsetof(pstruct, options_field . verify), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags = TLS_OPTFL }, \
>> + {"ca_file", "Certificate Authority database file", offsetof(pstruct, options_field . ca_file), AV_OPT_TYPE_STRING, { .str = CA_BUNDLE_PATH }, .flags = TLS_OPTFL }, \
>> + {"cafile", "Certificate Authority database file", offsetof(pstruct, options_field . ca_file), AV_OPT_TYPE_STRING, { .str = CA_BUNDLE_PATH }, .flags = TLS_OPTFL }, \
>> + {"tls_verify", "Verify the peer certificate", offsetof(pstruct, options_field . verify), AV_OPT_TYPE_BOOL, { .i64 = -1 }, -1, 1, .flags = TLS_OPTFL }, \
>> {"cert_file", "Certificate file", offsetof(pstruct, options_field . cert_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \
>> {"key_file", "Private key file", offsetof(pstruct, options_field . key_file), AV_OPT_TYPE_STRING, .flags = TLS_OPTFL }, \
>> {"listen", "Listen for incoming connections", offsetof(pstruct, options_field . listen), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags = TLS_OPTFL }, \
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
More information about the ffmpeg-devel
mailing list