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

[Thomas Volkert]

On 04.06.2018 15:38, Michael Niedermayer wrote:
> On Mon, Jun 04, 2018 at 12:16:36PM +0200, Thomas Volkert 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() ?
> we need a (preferrably single) API to get good random numbers from.
> all libs may need to access it so libavutil is probably the place for it
> and av_get_random_seed() is already there ...
>
> having a list of #if/elif/else accesing multiple sources of randomness
> should if it is needed be in av_get_random_seed() not duplicated every
> time a random number is needed.
Yes, I agree completely. The differentiation between the implementations
should be hidden behind av_get_random_seed().
But we should be careful when simplifying code at this point.

Best regards,
Thomas.