[FFmpeg-devel] [PATCH v3] avformat/hlsenc: reimplement randomize of hls use av_get_random_seed

[Michael Niedermayer]

On Mon, Jun 04, 2018 at 12:43:41PM +0200, Hendrik Leppkes wrote:
> On Mon, Jun 4, 2018 at 12:16 PM, Thomas Volkert <silvo at gmx.net> wrote:
> >
> > On 04.06.2018 04:55, Steven Liu wrote:
> >> Signed-off-by: Steven Liu <lq at chinaffmpeg.org>
> >> ---
> >>  configure            |  1 -
> >>  libavformat/hlsenc.c | 27 ++++++++++++---------------
> >>  2 files changed, 12 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/configure b/configure
> >> index 22eeca22a5..a3d0f5837a 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -3127,7 +3127,6 @@ fifo_muxer_deps="threads"
> >>  flac_demuxer_select="flac_parser"
> >>  hds_muxer_select="flv_muxer"
> >>  hls_muxer_select="mpegts_muxer"
> >> -hls_muxer_suggest="gcrypt openssl"
> >>  image2_alias_pix_demuxer_select="image2_demuxer"
> >>  image2_brender_pix_demuxer_select="image2_demuxer"
> >>  ipod_muxer_select="mov_muxer"
> >> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> >> index 2268f898b0..c04c561586 100644
> >> --- a/libavformat/hlsenc.c
> >> +++ b/libavformat/hlsenc.c
> >> @@ -27,12 +27,6 @@
> >>  #include <unistd.h>
> >>  #endif
> >>
> >> -#if CONFIG_GCRYPT
> >> -#include <gcrypt.h>
> >> -#elif CONFIG_OPENSSL
> >> -#include <openssl/rand.h>
> >> -#endif
> >> -
> >>  #include "libavutil/avassert.h"
> >>  #include "libavutil/mathematics.h"
> >>  #include "libavutil/parseutils.h"
> >> @@ -569,18 +563,21 @@ fail:
> >>      return ret;
> >>  }
> >>
> >> +
> >>  static int randomize(uint8_t *buf, int len)
> >>  {
> >> -#if CONFIG_GCRYPT
> >> -    gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
> >> +    uint32_t tmp_number[4];
> >> +    int i = 0;
> >> +
> >> +    if (len != 16)
> >> +        return AVERROR(EINVAL);
> >> +
> >> +    for (i = 0; i < 4; i++)
> >> +        tmp_number[i] = av_get_random_seed();
> >> +
> >> +    memcpy(buf, tmp_number, len);
> >> +
> >>      return 0;
> >> -#elif CONFIG_OPENSSL
> >> -    if (RAND_bytes(buf, len))
> >> -        return 0;
> >> -#else
> >> -    return AVERROR(ENOSYS);
> >> -#endif
> >> -    return AVERROR(EINVAL);
> >>  }
> >>
> >>  static int do_encrypt(AVFormatContext *s, VariantStream *vs)
> >
> > Does av_get_random_seed() provide the same random quality level as
> > gcry_randomize() / RAND_bytes() / mbedtls_havege_random() ?
> >
> 
> I can't really comment on quality, but av_get_random_seed does not use
> any cryptographic randomness sources - unless you're on Windows.
> On Linux it'll read /dev/urandom or /dev/random or make up its own
> seed based on time as a hash, depending on whats available.
> 
> If  av_get_random_seed is supposed to be used cryptographically, it
> should probably be expanded to use crypto APIs - although it being in
> avutil might make that annoying linking-wise.

is it just me or does something here feel wrong, i dont mean anything
related to our code but rather the apparent general acceptance that
generation of good random numbers requires non system crypto libs.

IM(H)O its the job os the OS to provide unbiased random numbers.
the OS also has all the hardware to its disposal to gather entropy,
its in a better position to do this than a user level lib.


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180604/c085a180/attachment.sig>