| Message ID | 20180604025547.19018-1-lq@chinaffmpeg.org |
|---|---|
| State | New |
| Headers | show |
On 04.06.2018 04:55, Steven Liu wrote: > Signed-off-by: Steven Liu <lq@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() ? Best regards, Thomas.
On Mon, Jun 4, 2018 at 12:16 PM, Thomas Volkert <silvo@gmx.net> wrote: > > On 04.06.2018 04:55, Steven Liu wrote: >> Signed-off-by: Steven Liu <lq@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. - Hendrik
2018-06-04 18:16 GMT+08:00 Thomas Volkert <silvo@gmx.net>: > > On 04.06.2018 04:55, Steven Liu wrote: >> Signed-off-by: Steven Liu <lq@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 think the randomize in hls is same as other module using av_get_random_seed, so maybe use av_get_random_seed is ok here. Thanks Steven > > Best regards, > Thomas. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
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@gmx.net> wrote: > > > > On 04.06.2018 04:55, Steven Liu wrote: > >> Signed-off-by: Steven Liu <lq@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. [...]
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@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. on a different topic, if someone wants to improve av_get_random_seed() in a way different from adding libs as sources of randomness data from image sensors (v4l* for example) and PCM audio. Should be usefull sources of entropy. Ive seen CCD imagers of phones being used as particle detectors. So theres seems to be some evidence behind this generating actual truely random numbers Thanks [...]
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@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.
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)
Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- configure | 1 - libavformat/hlsenc.c | 27 ++++++++++++--------------- 2 files changed, 12 insertions(+), 16 deletions(-)