[FFmpeg-devel] [PATCH 3/3] lavf/tls: verify TLS connections by default whenever possible
wm4
nfxjfg at googlemail.com
Wed Aug 16 14:29:22 EEST 2017
On Wed, 16 Aug 2017 02:19:18 -0500
Rodger Combs <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.
> 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 }, \
More information about the ffmpeg-devel
mailing list