[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