Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
@ 2023-07-02 19:30 Marton Balint
  2023-07-02 19:30 ` [FFmpeg-devel] [PATCH 2/2] avformat/hlsenc: remove openssl/gcrypt random key generation Marton Balint
                   ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Marton Balint @ 2023-07-02 19:30 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marton Balint

It should be OK to use av_get_random_seed() to generate the key instead of
using openSSL/Gcrypt functions. This removes the hard dependancy of those libs
for key generation functionality.

Fixes ticket #10441.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/hlsenc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 1e0848ce3d..0b22c71186 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -40,6 +40,7 @@
 #include "libavutil/intreadwrite.h"
 #include "libavutil/opt.h"
 #include "libavutil/log.h"
+#include "libavutil/random_seed.h"
 #include "libavutil/time.h"
 #include "libavutil/time_internal.h"
 
@@ -710,18 +711,18 @@ fail:
     return ret;
 }
 
-static int randomize(uint8_t *buf, int len)
+static void randomize(uint8_t *buf, int len)
 {
 #if CONFIG_GCRYPT
     gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
-    return 0;
+    return;
 #elif CONFIG_OPENSSL
     if (RAND_bytes(buf, len))
-        return 0;
-#else
-    return AVERROR(ENOSYS);
+        return;
 #endif
-    return AVERROR(EINVAL);
+    av_assert0(len % 4 == 0);
+    for (int i = 0; i < len; i += 4)
+        AV_WB32(buf + i, av_get_random_seed());
 }
 
 static int do_encrypt(AVFormatContext *s, VariantStream *vs)
@@ -775,10 +776,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
     if (!*hls->key_string) {
         AVDictionary *options = NULL;
         if (!hls->key) {
-            if ((ret = randomize(key, sizeof(key))) < 0) {
-                av_log(s, AV_LOG_ERROR, "Cannot generate a strong random key\n");
-                return ret;
-            }
+            randomize(key, sizeof(key));
         } else {
             memcpy(key, hls->key, sizeof(key));
         }
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] avformat/hlsenc: remove openssl/gcrypt random key generation
  2023-07-02 19:30 [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key Marton Balint
@ 2023-07-02 19:30 ` Marton Balint
  2023-07-03  2:21   ` Steven Liu
  2023-07-03  2:20 ` [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key Steven Liu
  2023-07-03 19:33 ` James Almer
  2 siblings, 1 reply; 28+ messages in thread
From: Marton Balint @ 2023-07-02 19:30 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marton Balint

av_get_random_seed() should be sufficent and that is used everywhere in the
codebase for similar cases.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 configure            |  1 -
 libavformat/hlsenc.c | 13 -------------
 2 files changed, 14 deletions(-)

diff --git a/configure b/configure
index 107d533b3e..b331b2e9db 100755
--- a/configure
+++ b/configure
@@ -3507,7 +3507,6 @@ gxf_muxer_select="pcm_rechunk_bsf"
 hds_muxer_select="flv_muxer"
 hls_demuxer_select="adts_header ac3_parser mov_demuxer mpegts_demuxer"
 hls_muxer_select="mov_muxer mpegts_muxer"
-hls_muxer_suggest="gcrypt openssl"
 image2_alias_pix_demuxer_select="image2_demuxer"
 image2_brender_pix_demuxer_select="image2_demuxer"
 imf_demuxer_deps="libxml2"
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 0b22c71186..f2284e8ea0 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/avstring.h"
@@ -713,13 +707,6 @@ fail:
 
 static void randomize(uint8_t *buf, int len)
 {
-#if CONFIG_GCRYPT
-    gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
-    return;
-#elif CONFIG_OPENSSL
-    if (RAND_bytes(buf, len))
-        return;
-#endif
     av_assert0(len % 4 == 0);
     for (int i = 0; i < len; i += 4)
         AV_WB32(buf + i, av_get_random_seed());
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-02 19:30 [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key Marton Balint
  2023-07-02 19:30 ` [FFmpeg-devel] [PATCH 2/2] avformat/hlsenc: remove openssl/gcrypt random key generation Marton Balint
@ 2023-07-03  2:20 ` Steven Liu
  2023-07-03 19:23   ` Marton Balint
  2023-07-03 19:33 ` James Almer
  2 siblings, 1 reply; 28+ messages in thread
From: Steven Liu @ 2023-07-03  2:20 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Marton Balint

Marton Balint <cus@passwd.hu> 于2023年7月3日周一 03:30写道:
>
> It should be OK to use av_get_random_seed() to generate the key instead of
> using openSSL/Gcrypt functions. This removes the hard dependancy of those libs
> for key generation functionality.
>
> Fixes ticket #10441.
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/hlsenc.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 1e0848ce3d..0b22c71186 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -40,6 +40,7 @@
>  #include "libavutil/intreadwrite.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/log.h"
> +#include "libavutil/random_seed.h"
>  #include "libavutil/time.h"
>  #include "libavutil/time_internal.h"
>
> @@ -710,18 +711,18 @@ fail:
>      return ret;
>  }
>
> -static int randomize(uint8_t *buf, int len)
> +static void randomize(uint8_t *buf, int len)
>  {
>  #if CONFIG_GCRYPT
>      gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
> -    return 0;
> +    return;
>  #elif CONFIG_OPENSSL
>      if (RAND_bytes(buf, len))
> -        return 0;
> -#else
> -    return AVERROR(ENOSYS);
> +        return;
>  #endif
> -    return AVERROR(EINVAL);
> +    av_assert0(len % 4 == 0);
> +    for (int i = 0; i < len; i += 4)
> +        AV_WB32(buf + i, av_get_random_seed());
>  }
>
>  static int do_encrypt(AVFormatContext *s, VariantStream *vs)
> @@ -775,10 +776,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
>      if (!*hls->key_string) {
>          AVDictionary *options = NULL;
>          if (!hls->key) {
> -            if ((ret = randomize(key, sizeof(key))) < 0) {
> -                av_log(s, AV_LOG_ERROR, "Cannot generate a strong random key\n");
> -                return ret;
> -            }
> +            randomize(key, sizeof(key));
>          } else {
>              memcpy(key, hls->key, sizeof(key));
>          }
Hi Marton,

Should remove braces too. I cannot sure how to make it more simpler as
!hls->key ?  randomize(key, sizeof(key)) : memcpy(key, hls->key,
sizeof(key)); ?


Thanks
Steven
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avformat/hlsenc: remove openssl/gcrypt random key generation
  2023-07-02 19:30 ` [FFmpeg-devel] [PATCH 2/2] avformat/hlsenc: remove openssl/gcrypt random key generation Marton Balint
@ 2023-07-03  2:21   ` Steven Liu
  0 siblings, 0 replies; 28+ messages in thread
From: Steven Liu @ 2023-07-03  2:21 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Marton Balint

Marton Balint <cus@passwd.hu> 于2023年7月3日周一 03:30写道:
>
> av_get_random_seed() should be sufficent and that is used everywhere in the
> codebase for similar cases.
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  configure            |  1 -
>  libavformat/hlsenc.c | 13 -------------
>  2 files changed, 14 deletions(-)
>
> diff --git a/configure b/configure
> index 107d533b3e..b331b2e9db 100755
> --- a/configure
> +++ b/configure
> @@ -3507,7 +3507,6 @@ gxf_muxer_select="pcm_rechunk_bsf"
>  hds_muxer_select="flv_muxer"
>  hls_demuxer_select="adts_header ac3_parser mov_demuxer mpegts_demuxer"
>  hls_muxer_select="mov_muxer mpegts_muxer"
> -hls_muxer_suggest="gcrypt openssl"
>  image2_alias_pix_demuxer_select="image2_demuxer"
>  image2_brender_pix_demuxer_select="image2_demuxer"
>  imf_demuxer_deps="libxml2"
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 0b22c71186..f2284e8ea0 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/avstring.h"
> @@ -713,13 +707,6 @@ fail:
>
>  static void randomize(uint8_t *buf, int len)
>  {
> -#if CONFIG_GCRYPT
> -    gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
> -    return;
> -#elif CONFIG_OPENSSL
> -    if (RAND_bytes(buf, len))
> -        return;
> -#endif
>      av_assert0(len % 4 == 0);
>      for (int i = 0; i < len; i += 4)
>          AV_WB32(buf + i, av_get_random_seed());
> --
> 2.35.3
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".


LGTM


Thanks
Steven
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03  2:20 ` [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key Steven Liu
@ 2023-07-03 19:23   ` Marton Balint
  0 siblings, 0 replies; 28+ messages in thread
From: Marton Balint @ 2023-07-03 19:23 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On Mon, 3 Jul 2023, Steven Liu wrote:

> Marton Balint <cus@passwd.hu> 于2023年7月3日周一 03:30写道:
>>
>> It should be OK to use av_get_random_seed() to generate the key instead of
>> using openSSL/Gcrypt functions. This removes the hard dependancy of those libs
>> for key generation functionality.
>>
>> Fixes ticket #10441.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---

[...]

>>  static int do_encrypt(AVFormatContext *s, VariantStream *vs)
>> @@ -775,10 +776,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
>>      if (!*hls->key_string) {
>>          AVDictionary *options = NULL;
>>          if (!hls->key) {
>> -            if ((ret = randomize(key, sizeof(key))) < 0) {
>> -                av_log(s, AV_LOG_ERROR, "Cannot generate a strong random key\n");
>> -                return ret;
>> -            }
>> +            randomize(key, sizeof(key));
>>          } else {
>>              memcpy(key, hls->key, sizeof(key));
>>          }
> Hi Marton,
>
> Should remove braces too. I cannot sure how to make it more simpler as
> !hls->key ?  randomize(key, sizeof(key)) : memcpy(key, hls->key,
> sizeof(key)); ?

I will just remove the braces, looks more readable to me than using the 
ternary operator.

Will apply the series, thanks.

Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-02 19:30 [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key Marton Balint
  2023-07-02 19:30 ` [FFmpeg-devel] [PATCH 2/2] avformat/hlsenc: remove openssl/gcrypt random key generation Marton Balint
  2023-07-03  2:20 ` [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key Steven Liu
@ 2023-07-03 19:33 ` James Almer
  2023-07-03 20:15   ` Anton Khirnov
  2023-07-03 20:20   ` Marton Balint
  2 siblings, 2 replies; 28+ messages in thread
From: James Almer @ 2023-07-03 19:33 UTC (permalink / raw)
  To: ffmpeg-devel

On 7/2/2023 4:30 PM, Marton Balint wrote:
> It should be OK to use av_get_random_seed() to generate the key instead of
> using openSSL/Gcrypt functions. This removes the hard dependancy of those libs
> for key generation functionality.
> 
> Fixes ticket #10441.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>   libavformat/hlsenc.c | 18 ++++++++----------
>   1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 1e0848ce3d..0b22c71186 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -40,6 +40,7 @@
>   #include "libavutil/intreadwrite.h"
>   #include "libavutil/opt.h"
>   #include "libavutil/log.h"
> +#include "libavutil/random_seed.h"
>   #include "libavutil/time.h"
>   #include "libavutil/time_internal.h"
>   
> @@ -710,18 +711,18 @@ fail:
>       return ret;
>   }
>   
> -static int randomize(uint8_t *buf, int len)
> +static void randomize(uint8_t *buf, int len)
>   {
>   #if CONFIG_GCRYPT
>       gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
> -    return 0;
> +    return;
>   #elif CONFIG_OPENSSL
>       if (RAND_bytes(buf, len))
> -        return 0;
> -#else
> -    return AVERROR(ENOSYS);
> +        return;
>   #endif
> -    return AVERROR(EINVAL);
> +    av_assert0(len % 4 == 0);
> +    for (int i = 0; i < len; i += 4)
> +        AV_WB32(buf + i, av_get_random_seed());

Maybe instead use a PRNG, like the following:

AVLFG c;
av_lfg_init(&c, av_get_random_seed());
for (int i = 0; i < len; i += 4)
     AV_WB32(buf + i, av_lfg_get(&c));
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03 19:33 ` James Almer
@ 2023-07-03 20:15   ` Anton Khirnov
  2023-07-03 20:54     ` Marton Balint
  2023-07-03 20:20   ` Marton Balint
  1 sibling, 1 reply; 28+ messages in thread
From: Anton Khirnov @ 2023-07-03 20:15 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting James Almer (2023-07-03 21:33:04)
> On 7/2/2023 4:30 PM, Marton Balint wrote:
> > It should be OK to use av_get_random_seed() to generate the key instead of
> > using openSSL/Gcrypt functions. This removes the hard dependancy of those libs
> > for key generation functionality.
> > 
> > Fixes ticket #10441.
> > 
> > Signed-off-by: Marton Balint <cus@passwd.hu>
> > ---
> >   libavformat/hlsenc.c | 18 ++++++++----------
> >   1 file changed, 8 insertions(+), 10 deletions(-)
> > 
> > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> > index 1e0848ce3d..0b22c71186 100644
> > --- a/libavformat/hlsenc.c
> > +++ b/libavformat/hlsenc.c
> > @@ -40,6 +40,7 @@
> >   #include "libavutil/intreadwrite.h"
> >   #include "libavutil/opt.h"
> >   #include "libavutil/log.h"
> > +#include "libavutil/random_seed.h"
> >   #include "libavutil/time.h"
> >   #include "libavutil/time_internal.h"
> >   
> > @@ -710,18 +711,18 @@ fail:
> >       return ret;
> >   }
> >   
> > -static int randomize(uint8_t *buf, int len)
> > +static void randomize(uint8_t *buf, int len)
> >   {
> >   #if CONFIG_GCRYPT
> >       gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
> > -    return 0;
> > +    return;
> >   #elif CONFIG_OPENSSL
> >       if (RAND_bytes(buf, len))
> > -        return 0;
> > -#else
> > -    return AVERROR(ENOSYS);
> > +        return;
> >   #endif
> > -    return AVERROR(EINVAL);
> > +    av_assert0(len % 4 == 0);
> > +    for (int i = 0; i < len; i += 4)
> > +        AV_WB32(buf + i, av_get_random_seed());
> 
> Maybe instead use a PRNG, like the following:
> 
> AVLFG c;
> av_lfg_init(&c, av_get_random_seed());
> for (int i = 0; i < len; i += 4)
>      AV_WB32(buf + i, av_lfg_get(&c));

We really REALLY should not be taking any shortcuts when generating
keys.

Ideally we shouldn't be generating them ourselves in the first place, as
we are not a crypto library. This patch seems like a step backward to
me.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03 19:33 ` James Almer
  2023-07-03 20:15   ` Anton Khirnov
@ 2023-07-03 20:20   ` Marton Balint
  1 sibling, 0 replies; 28+ messages in thread
From: Marton Balint @ 2023-07-03 20:20 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On Mon, 3 Jul 2023, James Almer wrote:

> On 7/2/2023 4:30 PM, Marton Balint wrote:
>>  It should be OK to use av_get_random_seed() to generate the key instead of
>>  using openSSL/Gcrypt functions. This removes the hard dependancy of those
>>  libs
>>  for key generation functionality.
>>
>>  Fixes ticket #10441.
>>
>>  Signed-off-by: Marton Balint <cus@passwd.hu>
>>  ---
>>    libavformat/hlsenc.c | 18 ++++++++----------
>>    1 file changed, 8 insertions(+), 10 deletions(-)
>>
>>  diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>  index 1e0848ce3d..0b22c71186 100644
>>  --- a/libavformat/hlsenc.c
>>  +++ b/libavformat/hlsenc.c
>>  @@ -40,6 +40,7 @@
>>    #include "libavutil/intreadwrite.h"
>>    #include "libavutil/opt.h"
>>    #include "libavutil/log.h"
>>  +#include "libavutil/random_seed.h"
>>    #include "libavutil/time.h"
>>    #include "libavutil/time_internal.h"
>>
>>  @@ -710,18 +711,18 @@ fail:
>>        return ret;
>>    }
>>
>>  -static int randomize(uint8_t *buf, int len)
>>  +static void randomize(uint8_t *buf, int len)
>>    {
>>    #if CONFIG_GCRYPT
>>        gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
>>  -    return 0;
>>  +    return;
>>    #elif CONFIG_OPENSSL
>>        if (RAND_bytes(buf, len))
>>  -        return 0;
>>  -#else
>>  -    return AVERROR(ENOSYS);
>>  +        return;
>>    #endif
>>  -    return AVERROR(EINVAL);
>>  +    av_assert0(len % 4 == 0);
>>  +    for (int i = 0; i < len; i += 4)
>>  +        AV_WB32(buf + i, av_get_random_seed());
>
> Maybe instead use a PRNG, like the following:
>
> AVLFG c;
> av_lfg_init(&c, av_get_random_seed());
> for (int i = 0; i < len; i += 4)
>    AV_WB32(buf + i, av_lfg_get(&c));

If randomize() were to be used for arbitrary lengths, I'd agree, but here 
it is only used for key generation (only 128-bit keys in the current 
code), so I think av_get_random_seed() for the whole keysize is fine, and 
more secure.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03 20:15   ` Anton Khirnov
@ 2023-07-03 20:54     ` Marton Balint
  2023-07-03 21:09       ` Anton Khirnov
  0 siblings, 1 reply; 28+ messages in thread
From: Marton Balint @ 2023-07-03 20:54 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On Mon, 3 Jul 2023, Anton Khirnov wrote:

> Quoting James Almer (2023-07-03 21:33:04)
>> On 7/2/2023 4:30 PM, Marton Balint wrote:
>>> It should be OK to use av_get_random_seed() to generate the key instead of
>>> using openSSL/Gcrypt functions. This removes the hard dependancy of those libs
>>> for key generation functionality.
>>>
>>> Fixes ticket #10441.
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>>   libavformat/hlsenc.c | 18 ++++++++----------
>>>   1 file changed, 8 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>> index 1e0848ce3d..0b22c71186 100644
>>> --- a/libavformat/hlsenc.c
>>> +++ b/libavformat/hlsenc.c
>>> @@ -40,6 +40,7 @@
>>>   #include "libavutil/intreadwrite.h"
>>>   #include "libavutil/opt.h"
>>>   #include "libavutil/log.h"
>>> +#include "libavutil/random_seed.h"
>>>   #include "libavutil/time.h"
>>>   #include "libavutil/time_internal.h"
>>>
>>> @@ -710,18 +711,18 @@ fail:
>>>       return ret;
>>>   }
>>>
>>> -static int randomize(uint8_t *buf, int len)
>>> +static void randomize(uint8_t *buf, int len)
>>>   {
>>>   #if CONFIG_GCRYPT
>>>       gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
>>> -    return 0;
>>> +    return;
>>>   #elif CONFIG_OPENSSL
>>>       if (RAND_bytes(buf, len))
>>> -        return 0;
>>> -#else
>>> -    return AVERROR(ENOSYS);
>>> +        return;
>>>   #endif
>>> -    return AVERROR(EINVAL);
>>> +    av_assert0(len % 4 == 0);
>>> +    for (int i = 0; i < len; i += 4)
>>> +        AV_WB32(buf + i, av_get_random_seed());
>>
>> Maybe instead use a PRNG, like the following:
>>
>> AVLFG c;
>> av_lfg_init(&c, av_get_random_seed());
>> for (int i = 0; i < len; i += 4)
>>      AV_WB32(buf + i, av_lfg_get(&c));
>
> We really REALLY should not be taking any shortcuts when generating
> keys.
>
> Ideally we shouldn't be generating them ourselves in the first place, as
> we are not a crypto library. This patch seems like a step backward to
> me.

My patch use av_get_random_seed() which uses what the underlying OS 
provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
BSD/Mac. You really think that these are significantly worse than 
OpenSSL/GCrypt, so it should not be allowed to fallback to?

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03 20:54     ` Marton Balint
@ 2023-07-03 21:09       ` Anton Khirnov
  2023-07-03 21:52         ` Marton Balint
  2023-07-03 23:50         ` [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when " Michael Niedermayer
  0 siblings, 2 replies; 28+ messages in thread
From: Anton Khirnov @ 2023-07-03 21:09 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Marton Balint (2023-07-03 22:54:41)
> On Mon, 3 Jul 2023, Anton Khirnov wrote:
> My patch use av_get_random_seed() which uses what the underlying OS 
> provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
> BSD/Mac.

IOW it's a jungle of various paths, some of which are not guaranteed to
be cryptographically secure. I see no such guarantees for arc4random()
from a brief web search, and the fallback get_generic_seed() certainly
is not either. Granted it's only used on obscure architectures, but
still.

The doxy even says
> This function tries to provide a good seed at a best effort bases.

> You really think that these are significantly worse than
> OpenSSL/GCrypt, so it should not be allowed to fallback to?

I think we should be using cryptographically secure PRNG for generating
encryption keys, or fail when they are not available. If you want to get
rid of the openssl dependency, IMO the best solution is a new
  int av_random(uint8_t* buf, size_t len);
that guarantees either cryptographically secure randomness or an error.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03 21:09       ` Anton Khirnov
@ 2023-07-03 21:52         ` Marton Balint
  2023-07-04 19:02           ` James Almer
  2023-07-03 23:50         ` [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when " Michael Niedermayer
  1 sibling, 1 reply; 28+ messages in thread
From: Marton Balint @ 2023-07-03 21:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On Mon, 3 Jul 2023, Anton Khirnov wrote:

> Quoting Marton Balint (2023-07-03 22:54:41)
>> On Mon, 3 Jul 2023, Anton Khirnov wrote:
>> My patch use av_get_random_seed() which uses what the underlying OS
>> provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for
>> BSD/Mac.
>
> IOW it's a jungle of various paths, some of which are not guaranteed to
> be cryptographically secure. I see no such guarantees for arc4random()

It depends on OS and version most likely.

> from a brief web search, and the fallback get_generic_seed() certainly
> is not either.
> Granted it's only used on obscure architectures, but
> still.

I am no expert on the subject, but even the generic code seems reasonably 
secure. It gathers entropy, it uses a crypto hash to get the output. And 
as you said, even that only used for obscure cases.

>
> The doxy even says
>> This function tries to provide a good seed at a best effort bases.
>
>> You really think that these are significantly worse than
>> OpenSSL/GCrypt, so it should not be allowed to fallback to?
>
> I think we should be using cryptographically secure PRNG for generating
> encryption keys, or fail when they are not available. If you want to get
> rid of the openssl dependency, IMO the best solution is a new
>  int av_random(uint8_t* buf, size_t len);
> that guarantees either cryptographically secure randomness or an error.

Sorry, seems a bit overdesign for me, so I won't be pursuing this further.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03 21:09       ` Anton Khirnov
  2023-07-03 21:52         ` Marton Balint
@ 2023-07-03 23:50         ` Michael Niedermayer
  2023-07-04  5:54           ` Anton Khirnov
  1 sibling, 1 reply; 28+ messages in thread
From: Michael Niedermayer @ 2023-07-03 23:50 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 1743 bytes --]

On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> Quoting Marton Balint (2023-07-03 22:54:41)
> > On Mon, 3 Jul 2023, Anton Khirnov wrote:
> > My patch use av_get_random_seed() which uses what the underlying OS 
> > provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
> > BSD/Mac.
> 
> IOW it's a jungle of various paths, some of which are not guaranteed to
> be cryptographically secure. I see no such guarantees for arc4random()
> from a brief web search, and the fallback get_generic_seed() certainly
> is not either. Granted it's only used on obscure architectures, but
> still.
> 
> The doxy even says
> > This function tries to provide a good seed at a best effort bases.
> 
> > You really think that these are significantly worse than
> > OpenSSL/GCrypt, so it should not be allowed to fallback to?
> 
> I think we should be using cryptographically secure PRNG for generating
> encryption keys, or fail when they are not available. If you want to get
> rid of the openssl dependency, IMO the best solution is a new
>   int av_random(uint8_t* buf, size_t len);
> that guarantees either cryptographically secure randomness or an error.

"guarantees cryptographically secure randomness" ?
If one defined "cryptographically secure" as "not broken publically as of today"

Iam saying that as i think "guarantees" can be misleading in what it means

thx

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

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03 23:50         ` [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when " Michael Niedermayer
@ 2023-07-04  5:54           ` Anton Khirnov
  2023-07-04  9:08             ` Kieran Kunhya
                               ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Anton Khirnov @ 2023-07-04  5:54 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Michael Niedermayer (2023-07-04 01:50:57)
> On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> > Quoting Marton Balint (2023-07-03 22:54:41)
> > > On Mon, 3 Jul 2023, Anton Khirnov wrote:
> > > My patch use av_get_random_seed() which uses what the underlying OS 
> > > provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
> > > BSD/Mac.
> > 
> > IOW it's a jungle of various paths, some of which are not guaranteed to
> > be cryptographically secure. I see no such guarantees for arc4random()
> > from a brief web search, and the fallback get_generic_seed() certainly
> > is not either. Granted it's only used on obscure architectures, but
> > still.
> > 
> > The doxy even says
> > > This function tries to provide a good seed at a best effort bases.
> > 
> > > You really think that these are significantly worse than
> > > OpenSSL/GCrypt, so it should not be allowed to fallback to?
> > 
> > I think we should be using cryptographically secure PRNG for generating
> > encryption keys, or fail when they are not available. If you want to get
> > rid of the openssl dependency, IMO the best solution is a new
> >   int av_random(uint8_t* buf, size_t len);
> > that guarantees either cryptographically secure randomness or an error.
> 
> "guarantees cryptographically secure randomness" ?
> If one defined "cryptographically secure" as "not broken publically as of today"
> 
> Iam saying that as i think "guarantees" can be misleading in what it means

I feel your snark is very much misplaced.

I recall way more instances of broken crypto caused by overconfident
non-experts with an attitude like yours ("those silly crypto libraries,
broken all the time, how hard can it be really") than by actual
vulnerabilities in actual crypto libraries.

In fact the highest-profile break I remember (Debian key entropy bug)
was caused precisely by non-experts fiddling with code they did not
understand.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-04  5:54           ` Anton Khirnov
@ 2023-07-04  9:08             ` Kieran Kunhya
  2023-07-04 14:37             ` James Almer
  2023-07-04 23:50             ` Michael Niedermayer
  2 siblings, 0 replies; 28+ messages in thread
From: Kieran Kunhya @ 2023-07-04  9:08 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, 4 Jul 2023 at 06:54, Anton Khirnov <anton@khirnov.net> wrote:

> Quoting Michael Niedermayer (2023-07-04 01:50:57)
> > On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> > > Quoting Marton Balint (2023-07-03 22:54:41)
> > > > On Mon, 3 Jul 2023, Anton Khirnov wrote:
> > > > My patch use av_get_random_seed() which uses what the underlying OS
> > > > provides, BCrypt for Windows, /dev/urandom for Linux, arc4random()
> for
> > > > BSD/Mac.
> > >
> > > IOW it's a jungle of various paths, some of which are not guaranteed to
> > > be cryptographically secure. I see no such guarantees for arc4random()
> > > from a brief web search, and the fallback get_generic_seed() certainly
> > > is not either. Granted it's only used on obscure architectures, but
> > > still.
> > >
> > > The doxy even says
> > > > This function tries to provide a good seed at a best effort bases.
> > >
> > > > You really think that these are significantly worse than
> > > > OpenSSL/GCrypt, so it should not be allowed to fallback to?
> > >
> > > I think we should be using cryptographically secure PRNG for generating
> > > encryption keys, or fail when they are not available. If you want to
> get
> > > rid of the openssl dependency, IMO the best solution is a new
> > >   int av_random(uint8_t* buf, size_t len);
> > > that guarantees either cryptographically secure randomness or an error.
> >
> > "guarantees cryptographically secure randomness" ?
> > If one defined "cryptographically secure" as "not broken publically as
> of today"
> >
> > Iam saying that as i think "guarantees" can be misleading in what it
> means
>
> I feel your snark is very much misplaced.
>
> I recall way more instances of broken crypto caused by overconfident
> non-experts with an attitude like yours ("those silly crypto libraries,
> broken all the time, how hard can it be really") than by actual
> vulnerabilities in actual crypto libraries.
>
> In fact the highest-profile break I remember (Debian key entropy bug)
> was caused precisely by non-experts fiddling with code they did not
> understand.
>

+1

Kieran
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-04  5:54           ` Anton Khirnov
  2023-07-04  9:08             ` Kieran Kunhya
@ 2023-07-04 14:37             ` James Almer
  2023-07-04 15:31               ` Anton Khirnov
  2023-07-04 23:50             ` Michael Niedermayer
  2 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-07-04 14:37 UTC (permalink / raw)
  To: ffmpeg-devel

On 7/4/2023 2:54 AM, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-04 01:50:57)
>> On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
>>> Quoting Marton Balint (2023-07-03 22:54:41)
>>>> On Mon, 3 Jul 2023, Anton Khirnov wrote:
>>>> My patch use av_get_random_seed() which uses what the underlying OS
>>>> provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for
>>>> BSD/Mac.
>>>
>>> IOW it's a jungle of various paths, some of which are not guaranteed to
>>> be cryptographically secure. I see no such guarantees for arc4random()
>>> from a brief web search, and the fallback get_generic_seed() certainly
>>> is not either. Granted it's only used on obscure architectures, but
>>> still.
>>>
>>> The doxy even says
>>>> This function tries to provide a good seed at a best effort bases.
>>>
>>>> You really think that these are significantly worse than
>>>> OpenSSL/GCrypt, so it should not be allowed to fallback to?
>>>
>>> I think we should be using cryptographically secure PRNG for generating
>>> encryption keys, or fail when they are not available. If you want to get
>>> rid of the openssl dependency, IMO the best solution is a new
>>>    int av_random(uint8_t* buf, size_t len);
>>> that guarantees either cryptographically secure randomness or an error.
>>
>> "guarantees cryptographically secure randomness" ?
>> If one defined "cryptographically secure" as "not broken publically as of today"
>>
>> Iam saying that as i think "guarantees" can be misleading in what it means
> 
> I feel your snark is very much misplaced.
> 
> I recall way more instances of broken crypto caused by overconfident
> non-experts with an attitude like yours ("those silly crypto libraries,
> broken all the time, how hard can it be really") than by actual
> vulnerabilities in actual crypto libraries.
> 
> In fact the highest-profile break I remember (Debian key entropy bug)
> was caused precisely by non-experts fiddling with code they did not
> understand.

Maybe the gcrypt and openssl API calls used here can instead be moved to 
av_get_random_seed(), which would reduce (or outright remove) the cases 
/dev/random or get_generic_seed() are called and result in essentially 
no changes to this functionality here?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-04 14:37             ` James Almer
@ 2023-07-04 15:31               ` Anton Khirnov
  0 siblings, 0 replies; 28+ messages in thread
From: Anton Khirnov @ 2023-07-04 15:31 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting James Almer (2023-07-04 16:37:03)
> On 7/4/2023 2:54 AM, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2023-07-04 01:50:57)
> >> On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> >>> Quoting Marton Balint (2023-07-03 22:54:41)
> >>>> On Mon, 3 Jul 2023, Anton Khirnov wrote:
> >>>> My patch use av_get_random_seed() which uses what the underlying OS
> >>>> provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for
> >>>> BSD/Mac.
> >>>
> >>> IOW it's a jungle of various paths, some of which are not guaranteed to
> >>> be cryptographically secure. I see no such guarantees for arc4random()
> >>> from a brief web search, and the fallback get_generic_seed() certainly
> >>> is not either. Granted it's only used on obscure architectures, but
> >>> still.
> >>>
> >>> The doxy even says
> >>>> This function tries to provide a good seed at a best effort bases.
> >>>
> >>>> You really think that these are significantly worse than
> >>>> OpenSSL/GCrypt, so it should not be allowed to fallback to?
> >>>
> >>> I think we should be using cryptographically secure PRNG for generating
> >>> encryption keys, or fail when they are not available. If you want to get
> >>> rid of the openssl dependency, IMO the best solution is a new
> >>>    int av_random(uint8_t* buf, size_t len);
> >>> that guarantees either cryptographically secure randomness or an error.
> >>
> >> "guarantees cryptographically secure randomness" ?
> >> If one defined "cryptographically secure" as "not broken publically as of today"
> >>
> >> Iam saying that as i think "guarantees" can be misleading in what it means
> > 
> > I feel your snark is very much misplaced.
> > 
> > I recall way more instances of broken crypto caused by overconfident
> > non-experts with an attitude like yours ("those silly crypto libraries,
> > broken all the time, how hard can it be really") than by actual
> > vulnerabilities in actual crypto libraries.
> > 
> > In fact the highest-profile break I remember (Debian key entropy bug)
> > was caused precisely by non-experts fiddling with code they did not
> > understand.
> 
> Maybe the gcrypt and openssl API calls used here can instead be moved to 
> av_get_random_seed(), which would reduce (or outright remove) the cases 
> /dev/random or get_generic_seed() are called and result in essentially 

I see nothing wrong with using /dev/random, it's probably the most
trustworthy source on most machines. Though on linux it's probably
even better to use getrandom() where available.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-03 21:52         ` Marton Balint
@ 2023-07-04 19:02           ` James Almer
  2023-07-04 19:30             ` Marton Balint
  0 siblings, 1 reply; 28+ messages in thread
From: James Almer @ 2023-07-04 19:02 UTC (permalink / raw)
  To: ffmpeg-devel

On 7/3/2023 6:52 PM, Marton Balint wrote:
> 
> 
> On Mon, 3 Jul 2023, Anton Khirnov wrote:
> 
>> Quoting Marton Balint (2023-07-03 22:54:41)
>>> On Mon, 3 Jul 2023, Anton Khirnov wrote:
>>> My patch use av_get_random_seed() which uses what the underlying OS
>>> provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for
>>> BSD/Mac.
>>
>> IOW it's a jungle of various paths, some of which are not guaranteed to
>> be cryptographically secure. I see no such guarantees for arc4random()
> 
> It depends on OS and version most likely.
> 
>> from a brief web search, and the fallback get_generic_seed() certainly
>> is not either.
>> Granted it's only used on obscure architectures, but
>> still.
> 
> I am no expert on the subject, but even the generic code seems 
> reasonably secure. It gathers entropy, it uses a crypto hash to get the 
> output. And as you said, even that only used for obscure cases.
> 
>>
>> The doxy even says
>>> This function tries to provide a good seed at a best effort bases.
>>
>>> You really think that these are significantly worse than
>>> OpenSSL/GCrypt, so it should not be allowed to fallback to?
>>
>> I think we should be using cryptographically secure PRNG for generating
>> encryption keys, or fail when they are not available. If you want to get
>> rid of the openssl dependency, IMO the best solution is a new
>>  int av_random(uint8_t* buf, size_t len);
>> that guarantees either cryptographically secure randomness or an error.
> 
> Sorry, seems a bit overdesign for me, so I won't be pursuing this further.

I just sent a patchset implementing a new av_random() function that uses 
the existing av_get_random_seed() entropy sources, and adds the ones 
used by hlsenc too.
If that goes in, then this patchset can go in as well afterwards.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-04 19:02           ` James Almer
@ 2023-07-04 19:30             ` Marton Balint
  2023-07-06 17:01               ` [FFmpeg-devel] [PATCH] avformat/hlsenc: use av_random_bytes() for " Marton Balint
  0 siblings, 1 reply; 28+ messages in thread
From: Marton Balint @ 2023-07-04 19:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On Tue, 4 Jul 2023, James Almer wrote:

> On 7/3/2023 6:52 PM, Marton Balint wrote:
>>
>>
>>  On Mon, 3 Jul 2023, Anton Khirnov wrote:
>>
>>>  Quoting Marton Balint (2023-07-03 22:54:41)
>>>>  On Mon, 3 Jul 2023, Anton Khirnov wrote:
>>>>  My patch use av_get_random_seed() which uses what the underlying OS
>>>>  provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for
>>>>  BSD/Mac.
>>>
>>>  IOW it's a jungle of various paths, some of which are not guaranteed to
>>>  be cryptographically secure. I see no such guarantees for arc4random()
>>
>>  It depends on OS and version most likely.
>>
>>>  from a brief web search, and the fallback get_generic_seed() certainly
>>>  is not either.
>>>  Granted it's only used on obscure architectures, but
>>>  still.
>>
>>  I am no expert on the subject, but even the generic code seems reasonably
>>  secure. It gathers entropy, it uses a crypto hash to get the output. And
>>  as you said, even that only used for obscure cases.
>> 
>>>
>>>  The doxy even says
>>>>  This function tries to provide a good seed at a best effort bases.
>>>
>>>>  You really think that these are significantly worse than
>>>>  OpenSSL/GCrypt, so it should not be allowed to fallback to?
>>>
>>>  I think we should be using cryptographically secure PRNG for generating
>>>  encryption keys, or fail when they are not available. If you want to get
>>>  rid of the openssl dependency, IMO the best solution is a new
>>>   int av_random(uint8_t* buf, size_t len);
>>>  that guarantees either cryptographically secure randomness or an error.
>>
>>  Sorry, seems a bit overdesign for me, so I won't be pursuing this further.
>
> I just sent a patchset implementing a new av_random() function that uses the 
> existing av_get_random_seed() entropy sources, and adds the ones used by 
> hlsenc too.
> If that goes in, then this patchset can go in as well afterwards.

Thanks! I will rework the patch to use av_random() then.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-04  5:54           ` Anton Khirnov
  2023-07-04  9:08             ` Kieran Kunhya
  2023-07-04 14:37             ` James Almer
@ 2023-07-04 23:50             ` Michael Niedermayer
  2023-07-05  9:22               ` Anton Khirnov
  2 siblings, 1 reply; 28+ messages in thread
From: Michael Niedermayer @ 2023-07-04 23:50 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 2884 bytes --]

On Tue, Jul 04, 2023 at 07:54:06AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-04 01:50:57)
> > On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> > > Quoting Marton Balint (2023-07-03 22:54:41)
> > > > On Mon, 3 Jul 2023, Anton Khirnov wrote:
> > > > My patch use av_get_random_seed() which uses what the underlying OS 
> > > > provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
> > > > BSD/Mac.
> > > 
> > > IOW it's a jungle of various paths, some of which are not guaranteed to
> > > be cryptographically secure. I see no such guarantees for arc4random()
> > > from a brief web search, and the fallback get_generic_seed() certainly
> > > is not either. Granted it's only used on obscure architectures, but
> > > still.
> > > 
> > > The doxy even says
> > > > This function tries to provide a good seed at a best effort bases.
> > > 
> > > > You really think that these are significantly worse than
> > > > OpenSSL/GCrypt, so it should not be allowed to fallback to?
> > > 
> > > I think we should be using cryptographically secure PRNG for generating
> > > encryption keys, or fail when they are not available. If you want to get
> > > rid of the openssl dependency, IMO the best solution is a new
> > >   int av_random(uint8_t* buf, size_t len);
> > > that guarantees either cryptographically secure randomness or an error.
> > 
> > "guarantees cryptographically secure randomness" ?
> > If one defined "cryptographically secure" as "not broken publically as of today"
> > 
> > Iam saying that as i think "guarantees" can be misleading in what it means
> 
> I feel your snark is very much misplaced.
> 

> I recall way more instances of broken crypto caused by overconfident
> non-experts with an attitude like yours ("those silly crypto libraries,
> broken all the time, how hard can it be really") than by actual
> vulnerabilities in actual crypto libraries.
> 
> In fact the highest-profile break I remember (Debian key entropy bug)
> was caused precisely by non-experts fiddling with code they did not
> understand.

There is no snark here, at least that was not the intend
Also what you say in these 2 paragraphs is true but isnt really
related to what i said or meant to say

these cryptographically secure PRNGS are secure as long as the
currently used components and assumtations they are build on havnt
been broken.
Can i do better? no. but that doesnt mean that these
are going to be unbroken in 30 years.
just look 30 years in the past what percentage of what was believed to
be secure 30 years ago has been broken today. or 50 or 100years
thats really what i meant

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Nations do behave wisely once they have exhausted all other alternatives. 
-- Abba Eban

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-04 23:50             ` Michael Niedermayer
@ 2023-07-05  9:22               ` Anton Khirnov
  2023-07-05 22:54                 ` Michael Niedermayer
  0 siblings, 1 reply; 28+ messages in thread
From: Anton Khirnov @ 2023-07-05  9:22 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Michael Niedermayer (2023-07-05 01:50:12)
> On Tue, Jul 04, 2023 at 07:54:06AM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2023-07-04 01:50:57)
> > > On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> > > > Quoting Marton Balint (2023-07-03 22:54:41)
> > > > > On Mon, 3 Jul 2023, Anton Khirnov wrote:
> > > > > My patch use av_get_random_seed() which uses what the underlying OS 
> > > > > provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
> > > > > BSD/Mac.
> > > > 
> > > > IOW it's a jungle of various paths, some of which are not guaranteed to
> > > > be cryptographically secure. I see no such guarantees for arc4random()
> > > > from a brief web search, and the fallback get_generic_seed() certainly
> > > > is not either. Granted it's only used on obscure architectures, but
> > > > still.
> > > > 
> > > > The doxy even says
> > > > > This function tries to provide a good seed at a best effort bases.
> > > > 
> > > > > You really think that these are significantly worse than
> > > > > OpenSSL/GCrypt, so it should not be allowed to fallback to?
> > > > 
> > > > I think we should be using cryptographically secure PRNG for generating
> > > > encryption keys, or fail when they are not available. If you want to get
> > > > rid of the openssl dependency, IMO the best solution is a new
> > > >   int av_random(uint8_t* buf, size_t len);
> > > > that guarantees either cryptographically secure randomness or an error.
> > > 
> > > "guarantees cryptographically secure randomness" ?
> > > If one defined "cryptographically secure" as "not broken publically as of today"
> > > 
> > > Iam saying that as i think "guarantees" can be misleading in what it means
> > 
> > I feel your snark is very much misplaced.
> > 
> 
> > I recall way more instances of broken crypto caused by overconfident
> > non-experts with an attitude like yours ("those silly crypto libraries,
> > broken all the time, how hard can it be really") than by actual
> > vulnerabilities in actual crypto libraries.
> > 
> > In fact the highest-profile break I remember (Debian key entropy bug)
> > was caused precisely by non-experts fiddling with code they did not
> > understand.
> 
> There is no snark here, at least that was not the intend
> Also what you say in these 2 paragraphs is true but isnt really
> related to what i said or meant to say
> 
> these cryptographically secure PRNGS are secure as long as the
> currently used components and assumtations they are build on havnt
> been broken.
> Can i do better? no. but that doesnt mean that these
> are going to be unbroken in 30 years.
> just look 30 years in the past what percentage of what was believed to
> be secure 30 years ago has been broken today. or 50 or 100years
> thats really what i meant

I still don't see what point are you trying to make here.
Yes, any practical cryptographic algorithm could potentially be broken
at some point. And everything in real life is imperfect, because we do
not live in the world of ideal forms.
But I don't see what practical steps could or should be taken in
response to this.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-05  9:22               ` Anton Khirnov
@ 2023-07-05 22:54                 ` Michael Niedermayer
  2023-07-06  7:52                   ` Anton Khirnov
  0 siblings, 1 reply; 28+ messages in thread
From: Michael Niedermayer @ 2023-07-05 22:54 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 4117 bytes --]

On Wed, Jul 05, 2023 at 11:22:44AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-05 01:50:12)
> > On Tue, Jul 04, 2023 at 07:54:06AM +0200, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2023-07-04 01:50:57)
> > > > On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> > > > > Quoting Marton Balint (2023-07-03 22:54:41)
> > > > > > On Mon, 3 Jul 2023, Anton Khirnov wrote:
> > > > > > My patch use av_get_random_seed() which uses what the underlying OS 
> > > > > > provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
> > > > > > BSD/Mac.
> > > > > 
> > > > > IOW it's a jungle of various paths, some of which are not guaranteed to
> > > > > be cryptographically secure. I see no such guarantees for arc4random()
> > > > > from a brief web search, and the fallback get_generic_seed() certainly
> > > > > is not either. Granted it's only used on obscure architectures, but
> > > > > still.
> > > > > 
> > > > > The doxy even says
> > > > > > This function tries to provide a good seed at a best effort bases.
> > > > > 
> > > > > > You really think that these are significantly worse than
> > > > > > OpenSSL/GCrypt, so it should not be allowed to fallback to?
> > > > > 
> > > > > I think we should be using cryptographically secure PRNG for generating
> > > > > encryption keys, or fail when they are not available. If you want to get
> > > > > rid of the openssl dependency, IMO the best solution is a new
> > > > >   int av_random(uint8_t* buf, size_t len);
> > > > > that guarantees either cryptographically secure randomness or an error.
> > > > 
> > > > "guarantees cryptographically secure randomness" ?
> > > > If one defined "cryptographically secure" as "not broken publically as of today"
> > > > 
> > > > Iam saying that as i think "guarantees" can be misleading in what it means
> > > 
> > > I feel your snark is very much misplaced.
> > > 
> > 
> > > I recall way more instances of broken crypto caused by overconfident
> > > non-experts with an attitude like yours ("those silly crypto libraries,
> > > broken all the time, how hard can it be really") than by actual
> > > vulnerabilities in actual crypto libraries.
> > > 
> > > In fact the highest-profile break I remember (Debian key entropy bug)
> > > was caused precisely by non-experts fiddling with code they did not
> > > understand.
> > 
> > There is no snark here, at least that was not the intend
> > Also what you say in these 2 paragraphs is true but isnt really
> > related to what i said or meant to say
> > 
> > these cryptographically secure PRNGS are secure as long as the
> > currently used components and assumtations they are build on havnt
> > been broken.
> > Can i do better? no. but that doesnt mean that these
> > are going to be unbroken in 30 years.
> > just look 30 years in the past what percentage of what was believed to
> > be secure 30 years ago has been broken today. or 50 or 100years
> > thats really what i meant
> 
> I still don't see what point are you trying to make here.
> Yes, any practical cryptographic algorithm could potentially be broken
> at some point. And everything in real life is imperfect, because we do
> not live in the world of ideal forms.

> But I don't see what practical steps could or should be taken in
> response to this.

for us i dont know but a user could
instead of putting critical data in a system that might be broken in 30 years
just write it down on paper and burn and grind the paper when its not needed anymore
(which may or may not be an option)

nothing is perfect but there are methods to transfer and destroy data which have a
long track record of being secure and are simple.

I think we should not make it sound like encrypting with these random numbers
is as good as not storing/transmitting or using bits from fliping a real fair coin

thx

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

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-05 22:54                 ` Michael Niedermayer
@ 2023-07-06  7:52                   ` Anton Khirnov
  2023-07-06 23:34                     ` Kieran Kunhya
  2023-07-07  0:55                     ` Michael Niedermayer
  0 siblings, 2 replies; 28+ messages in thread
From: Anton Khirnov @ 2023-07-06  7:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Michael Niedermayer (2023-07-06 00:54:47)
> On Wed, Jul 05, 2023 at 11:22:44AM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2023-07-05 01:50:12)
> > > On Tue, Jul 04, 2023 at 07:54:06AM +0200, Anton Khirnov wrote:
> > > > Quoting Michael Niedermayer (2023-07-04 01:50:57)
> > > > > On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> > > > > > Quoting Marton Balint (2023-07-03 22:54:41)
> > > > > > > On Mon, 3 Jul 2023, Anton Khirnov wrote:
> > > > > > > My patch use av_get_random_seed() which uses what the underlying OS 
> > > > > > > provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
> > > > > > > BSD/Mac.
> > > > > > 
> > > > > > IOW it's a jungle of various paths, some of which are not guaranteed to
> > > > > > be cryptographically secure. I see no such guarantees for arc4random()
> > > > > > from a brief web search, and the fallback get_generic_seed() certainly
> > > > > > is not either. Granted it's only used on obscure architectures, but
> > > > > > still.
> > > > > > 
> > > > > > The doxy even says
> > > > > > > This function tries to provide a good seed at a best effort bases.
> > > > > > 
> > > > > > > You really think that these are significantly worse than
> > > > > > > OpenSSL/GCrypt, so it should not be allowed to fallback to?
> > > > > > 
> > > > > > I think we should be using cryptographically secure PRNG for generating
> > > > > > encryption keys, or fail when they are not available. If you want to get
> > > > > > rid of the openssl dependency, IMO the best solution is a new
> > > > > >   int av_random(uint8_t* buf, size_t len);
> > > > > > that guarantees either cryptographically secure randomness or an error.
> > > > > 
> > > > > "guarantees cryptographically secure randomness" ?
> > > > > If one defined "cryptographically secure" as "not broken publically as of today"
> > > > > 
> > > > > Iam saying that as i think "guarantees" can be misleading in what it means
> > > > 
> > > > I feel your snark is very much misplaced.
> > > > 
> > > 
> > > > I recall way more instances of broken crypto caused by overconfident
> > > > non-experts with an attitude like yours ("those silly crypto libraries,
> > > > broken all the time, how hard can it be really") than by actual
> > > > vulnerabilities in actual crypto libraries.
> > > > 
> > > > In fact the highest-profile break I remember (Debian key entropy bug)
> > > > was caused precisely by non-experts fiddling with code they did not
> > > > understand.
> > > 
> > > There is no snark here, at least that was not the intend
> > > Also what you say in these 2 paragraphs is true but isnt really
> > > related to what i said or meant to say
> > > 
> > > these cryptographically secure PRNGS are secure as long as the
> > > currently used components and assumtations they are build on havnt
> > > been broken.
> > > Can i do better? no. but that doesnt mean that these
> > > are going to be unbroken in 30 years.
> > > just look 30 years in the past what percentage of what was believed to
> > > be secure 30 years ago has been broken today. or 50 or 100years
> > > thats really what i meant
> > 
> > I still don't see what point are you trying to make here.
> > Yes, any practical cryptographic algorithm could potentially be broken
> > at some point. And everything in real life is imperfect, because we do
> > not live in the world of ideal forms.
> 
> > But I don't see what practical steps could or should be taken in
> > response to this.
> 
> for us i dont know but a user could
> instead of putting critical data in a system that might be broken in 30 years
> just write it down on paper and burn and grind the paper when its not needed anymore
> (which may or may not be an option)
> 
> nothing is perfect but there are methods to transfer and destroy data which have a
> long track record of being secure and are simple.
> 
> I think we should not make it sound like encrypting with these random numbers
> is as good as not storing/transmitting or using bits from fliping a real fair coin

We are not claiming that. We are claiming that the random numbers
generated are (to the best of our ability, and that of the underlying
libraries we rely on) cryptographically secure. This means suitable for
use in state of the art cryptographic algorithms like AES.
I do not think it makes sense to mistrust CSPRNGs, yet still trust AES.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* [FFmpeg-devel] [PATCH] avformat/hlsenc: use av_random_bytes() for generating AES128 key
  2023-07-04 19:30             ` Marton Balint
@ 2023-07-06 17:01               ` Marton Balint
  2023-07-14 19:39                 ` Marton Balint
  0 siblings, 1 reply; 28+ messages in thread
From: Marton Balint @ 2023-07-06 17:01 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marton Balint

av_random_bytes() can use OS provided strong random functions and does not
depend soley on openssl/gcrypt external libraries.

Fixes ticket #10441.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 configure            |  1 -
 libavformat/hlsenc.c | 23 ++---------------------
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/configure b/configure
index 107d533b3e..b331b2e9db 100755
--- a/configure
+++ b/configure
@@ -3507,7 +3507,6 @@ gxf_muxer_select="pcm_rechunk_bsf"
 hds_muxer_select="flv_muxer"
 hls_demuxer_select="adts_header ac3_parser mov_demuxer mpegts_demuxer"
 hls_muxer_select="mov_muxer mpegts_muxer"
-hls_muxer_suggest="gcrypt openssl"
 image2_alias_pix_demuxer_select="image2_demuxer"
 image2_brender_pix_demuxer_select="image2_demuxer"
 imf_demuxer_deps="libxml2"
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 1e0848ce3d..27d97f5f72 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/avstring.h"
@@ -40,6 +34,7 @@
 #include "libavutil/intreadwrite.h"
 #include "libavutil/opt.h"
 #include "libavutil/log.h"
+#include "libavutil/random_seed.h"
 #include "libavutil/time.h"
 #include "libavutil/time_internal.h"
 
@@ -710,20 +705,6 @@ fail:
     return ret;
 }
 
-static int randomize(uint8_t *buf, int len)
-{
-#if CONFIG_GCRYPT
-    gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
-    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)
 {
     HLSContext *hls = s->priv_data;
@@ -775,7 +756,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
     if (!*hls->key_string) {
         AVDictionary *options = NULL;
         if (!hls->key) {
-            if ((ret = randomize(key, sizeof(key))) < 0) {
+            if ((ret = av_random_bytes(key, sizeof(key))) < 0) {
                 av_log(s, AV_LOG_ERROR, "Cannot generate a strong random key\n");
                 return ret;
             }
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-06  7:52                   ` Anton Khirnov
@ 2023-07-06 23:34                     ` Kieran Kunhya
  2023-07-07  0:55                     ` Michael Niedermayer
  1 sibling, 0 replies; 28+ messages in thread
From: Kieran Kunhya @ 2023-07-06 23:34 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Thu, 6 Jul 2023, 08:52 Anton Khirnov, <anton@khirnov.net> wrote:

>
> We are not claiming that. We are claiming that the random numbers
> generated are (to the best of our ability, and that of the underlying
> libraries we rely on) cryptographically secure. This means suitable for
> use in state of the art cryptographic algorithms like AES.
> I do not think it makes sense to mistrust CSPRNGs, yet still trust AES.


Agreed, I don't think it's ffmpeg's job to get into a philosophical
discussion about the state of the art of random number generation.

Kieran

> ".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-06  7:52                   ` Anton Khirnov
  2023-07-06 23:34                     ` Kieran Kunhya
@ 2023-07-07  0:55                     ` Michael Niedermayer
  2023-07-07  8:05                       ` Anton Khirnov
  1 sibling, 1 reply; 28+ messages in thread
From: Michael Niedermayer @ 2023-07-07  0:55 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 5309 bytes --]

On Thu, Jul 06, 2023 at 09:52:12AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-06 00:54:47)
> > On Wed, Jul 05, 2023 at 11:22:44AM +0200, Anton Khirnov wrote:
> > > Quoting Michael Niedermayer (2023-07-05 01:50:12)
> > > > On Tue, Jul 04, 2023 at 07:54:06AM +0200, Anton Khirnov wrote:
> > > > > Quoting Michael Niedermayer (2023-07-04 01:50:57)
> > > > > > On Mon, Jul 03, 2023 at 11:09:54PM +0200, Anton Khirnov wrote:
> > > > > > > Quoting Marton Balint (2023-07-03 22:54:41)
> > > > > > > > On Mon, 3 Jul 2023, Anton Khirnov wrote:
> > > > > > > > My patch use av_get_random_seed() which uses what the underlying OS 
> > > > > > > > provides, BCrypt for Windows, /dev/urandom for Linux, arc4random() for 
> > > > > > > > BSD/Mac.
> > > > > > > 
> > > > > > > IOW it's a jungle of various paths, some of which are not guaranteed to
> > > > > > > be cryptographically secure. I see no such guarantees for arc4random()
> > > > > > > from a brief web search, and the fallback get_generic_seed() certainly
> > > > > > > is not either. Granted it's only used on obscure architectures, but
> > > > > > > still.
> > > > > > > 
> > > > > > > The doxy even says
> > > > > > > > This function tries to provide a good seed at a best effort bases.
> > > > > > > 
> > > > > > > > You really think that these are significantly worse than
> > > > > > > > OpenSSL/GCrypt, so it should not be allowed to fallback to?
> > > > > > > 
> > > > > > > I think we should be using cryptographically secure PRNG for generating
> > > > > > > encryption keys, or fail when they are not available. If you want to get
> > > > > > > rid of the openssl dependency, IMO the best solution is a new
> > > > > > >   int av_random(uint8_t* buf, size_t len);
> > > > > > > that guarantees either cryptographically secure randomness or an error.
> > > > > > 
> > > > > > "guarantees cryptographically secure randomness" ?
> > > > > > If one defined "cryptographically secure" as "not broken publically as of today"
> > > > > > 
> > > > > > Iam saying that as i think "guarantees" can be misleading in what it means
> > > > > 
> > > > > I feel your snark is very much misplaced.
> > > > > 
> > > > 
> > > > > I recall way more instances of broken crypto caused by overconfident
> > > > > non-experts with an attitude like yours ("those silly crypto libraries,
> > > > > broken all the time, how hard can it be really") than by actual
> > > > > vulnerabilities in actual crypto libraries.
> > > > > 
> > > > > In fact the highest-profile break I remember (Debian key entropy bug)
> > > > > was caused precisely by non-experts fiddling with code they did not
> > > > > understand.
> > > > 
> > > > There is no snark here, at least that was not the intend
> > > > Also what you say in these 2 paragraphs is true but isnt really
> > > > related to what i said or meant to say
> > > > 
> > > > these cryptographically secure PRNGS are secure as long as the
> > > > currently used components and assumtations they are build on havnt
> > > > been broken.
> > > > Can i do better? no. but that doesnt mean that these
> > > > are going to be unbroken in 30 years.
> > > > just look 30 years in the past what percentage of what was believed to
> > > > be secure 30 years ago has been broken today. or 50 or 100years
> > > > thats really what i meant
> > > 
> > > I still don't see what point are you trying to make here.
> > > Yes, any practical cryptographic algorithm could potentially be broken
> > > at some point. And everything in real life is imperfect, because we do
> > > not live in the world of ideal forms.
> > 
> > > But I don't see what practical steps could or should be taken in
> > > response to this.
> > 
> > for us i dont know but a user could
> > instead of putting critical data in a system that might be broken in 30 years
> > just write it down on paper and burn and grind the paper when its not needed anymore
> > (which may or may not be an option)
> > 
> > nothing is perfect but there are methods to transfer and destroy data which have a
> > long track record of being secure and are simple.
> > 
> > I think we should not make it sound like encrypting with these random numbers
> > is as good as not storing/transmitting or using bits from fliping a real fair coin
> 
> We are not claiming that. We are claiming that the random numbers
> generated are (to the best of our ability, and that of the underlying
> libraries we rely on) cryptographically secure. This means suitable for
> use in state of the art cryptographic algorithms like AES.
> I do not think it makes sense to mistrust CSPRNGs, yet still trust AES.

The litteral wording was
"that guarantees either cryptographically secure randomness or an error."

that was what i refered to.

the wording used now:
"to the best of our ability, and that of the underlying libraries we rely on) cryptographically secure."

is perfectly fine with me.
I would have the same issue if someone said AES gurantees ...

thx


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-07  0:55                     ` Michael Niedermayer
@ 2023-07-07  8:05                       ` Anton Khirnov
  2023-07-07 14:42                         ` Michael Niedermayer
  0 siblings, 1 reply; 28+ messages in thread
From: Anton Khirnov @ 2023-07-07  8:05 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Quoting Michael Niedermayer (2023-07-07 02:55:46)
> 
> The litteral wording was
> "that guarantees either cryptographically secure randomness or an error."
> 
> that was what i refered to.
> 
> the wording used now:
> "to the best of our ability, and that of the underlying libraries we rely on) cryptographically secure."
> 
> is perfectly fine with me.
> I would have the same issue if someone said AES gurantees ...

IMO the two formulations are equivalent whenever it comes to practical
computing. An algorithm can be mathematically proven to be sound*, but
any practical computing scheme on actual hardware is always subject to
software bugs, system misconfiguration, hardware bugs, hardware failure,
etc.

We use similar wording in other documentation, where e.g. we might
guarantee that some function returns a NULL-terminated string or so.
That guarantee is always under the implicit condition that there are no
bugs and the code runs in the expected environment. The same
considerations apply here.

* assuming there are no bugs in your proof

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key
  2023-07-07  8:05                       ` Anton Khirnov
@ 2023-07-07 14:42                         ` Michael Niedermayer
  0 siblings, 0 replies; 28+ messages in thread
From: Michael Niedermayer @ 2023-07-07 14:42 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


[-- Attachment #1.1: Type: text/plain, Size: 2529 bytes --]

On Fri, Jul 07, 2023 at 10:05:50AM +0200, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2023-07-07 02:55:46)
> > 
> > The litteral wording was
> > "that guarantees either cryptographically secure randomness or an error."
> > 
> > that was what i refered to.
> > 
> > the wording used now:
> > "to the best of our ability, and that of the underlying libraries we rely on) cryptographically secure."
> > 
> > is perfectly fine with me.
> > I would have the same issue if someone said AES gurantees ...
> 
> IMO the two formulations are equivalent whenever it comes to practical
> computing. An algorithm can be mathematically proven to be sound*, but
> any practical computing scheme on actual hardware is always subject to
> software bugs, system misconfiguration, hardware bugs, hardware failure,
> etc.
>

> We use similar wording in other documentation, where e.g. we might
> guarantee that some function returns a NULL-terminated string or so.
> That guarantee is always under the implicit condition that there are no
> bugs and the code runs in the expected environment. The same
> considerations apply here.

Theres a big difference between a bug in our implementation
And us claiming some cryptographic primitive is secure.
It was said previously that we shouldnt do things we lack the experties
on and rather delegate to cryptographic libraries writen and audited by
experts. (where it matters for security not just for playback)
But claiming CSPRNG or AES or anything else is guranteed secure is
exactly such a claim that is not within our experties.

If you claim your code produces a null terminated string that i believe
you (within the bounds you mentioned) but if you tell me AES will always
be secure i wont believe you that unless you have the mathemtical proofs
to back that up (and i read and understood them).

Now all that flawlessness with security primitives from proper security libs and
stuff needs to be taken with a grain of salt too.
just 4 months ago i found 2 issues with teh random number generator in the hardware
password manager that i use.
So yeah maybe people feels iam too nitpicky here but honestly id rather be nitpicky
on security stuff

thx

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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: use av_random_bytes() for generating AES128 key
  2023-07-06 17:01               ` [FFmpeg-devel] [PATCH] avformat/hlsenc: use av_random_bytes() for " Marton Balint
@ 2023-07-14 19:39                 ` Marton Balint
  0 siblings, 0 replies; 28+ messages in thread
From: Marton Balint @ 2023-07-14 19:39 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On Thu, 6 Jul 2023, Marton Balint wrote:

> av_random_bytes() can use OS provided strong random functions and does not
> depend soley on openssl/gcrypt external libraries.
>
> Fixes ticket #10441.

Will apply.

Regards,
Marton

>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> configure            |  1 -
> libavformat/hlsenc.c | 23 ++---------------------
> 2 files changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/configure b/configure
> index 107d533b3e..b331b2e9db 100755
> --- a/configure
> +++ b/configure
> @@ -3507,7 +3507,6 @@ gxf_muxer_select="pcm_rechunk_bsf"
> hds_muxer_select="flv_muxer"
> hls_demuxer_select="adts_header ac3_parser mov_demuxer mpegts_demuxer"
> hls_muxer_select="mov_muxer mpegts_muxer"
> -hls_muxer_suggest="gcrypt openssl"
> image2_alias_pix_demuxer_select="image2_demuxer"
> image2_brender_pix_demuxer_select="image2_demuxer"
> imf_demuxer_deps="libxml2"
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 1e0848ce3d..27d97f5f72 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/avstring.h"
> @@ -40,6 +34,7 @@
> #include "libavutil/intreadwrite.h"
> #include "libavutil/opt.h"
> #include "libavutil/log.h"
> +#include "libavutil/random_seed.h"
> #include "libavutil/time.h"
> #include "libavutil/time_internal.h"
>
> @@ -710,20 +705,6 @@ fail:
>     return ret;
> }
>
> -static int randomize(uint8_t *buf, int len)
> -{
> -#if CONFIG_GCRYPT
> -    gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
> -    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)
> {
>     HLSContext *hls = s->priv_data;
> @@ -775,7 +756,7 @@ static int do_encrypt(AVFormatContext *s, VariantStream *vs)
>     if (!*hls->key_string) {
>         AVDictionary *options = NULL;
>         if (!hls->key) {
> -            if ((ret = randomize(key, sizeof(key))) < 0) {
> +            if ((ret = av_random_bytes(key, sizeof(key))) < 0) {
>                 av_log(s, AV_LOG_ERROR, "Cannot generate a strong random key\n");
>                 return ret;
>             }
> -- 
> 2.35.3
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2023-07-14 19:42 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-02 19:30 [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key Marton Balint
2023-07-02 19:30 ` [FFmpeg-devel] [PATCH 2/2] avformat/hlsenc: remove openssl/gcrypt random key generation Marton Balint
2023-07-03  2:21   ` Steven Liu
2023-07-03  2:20 ` [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key Steven Liu
2023-07-03 19:23   ` Marton Balint
2023-07-03 19:33 ` James Almer
2023-07-03 20:15   ` Anton Khirnov
2023-07-03 20:54     ` Marton Balint
2023-07-03 21:09       ` Anton Khirnov
2023-07-03 21:52         ` Marton Balint
2023-07-04 19:02           ` James Almer
2023-07-04 19:30             ` Marton Balint
2023-07-06 17:01               ` [FFmpeg-devel] [PATCH] avformat/hlsenc: use av_random_bytes() for " Marton Balint
2023-07-14 19:39                 ` Marton Balint
2023-07-03 23:50         ` [FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when " Michael Niedermayer
2023-07-04  5:54           ` Anton Khirnov
2023-07-04  9:08             ` Kieran Kunhya
2023-07-04 14:37             ` James Almer
2023-07-04 15:31               ` Anton Khirnov
2023-07-04 23:50             ` Michael Niedermayer
2023-07-05  9:22               ` Anton Khirnov
2023-07-05 22:54                 ` Michael Niedermayer
2023-07-06  7:52                   ` Anton Khirnov
2023-07-06 23:34                     ` Kieran Kunhya
2023-07-07  0:55                     ` Michael Niedermayer
2023-07-07  8:05                       ` Anton Khirnov
2023-07-07 14:42                         ` Michael Niedermayer
2023-07-03 20:20   ` Marton Balint

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git