diff mbox series

[FFmpeg-devel,v3,3/3] avutil/random_seed: add support for gcrypt and OpenSSL as source of randomness

Message ID 20230704232614.2785-3-jamrial@gmail.com
State Accepted
Commit b7f4d5fa7e04498fddd60d298bc4a95241a581da
Headers show
Series [FFmpeg-devel,v3,1/3] avutil/random_seed: use fread() in read_random() | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

James Almer July 4, 2023, 11:26 p.m. UTC
Signed-off-by: James Almer <jamrial@gmail.com>
---
 configure               |  2 +-
 libavutil/random_seed.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

Comments

Anton Khirnov July 5, 2023, 12:56 p.m. UTC | #1
Quoting James Almer (2023-07-05 01:26:14)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  configure               |  2 +-
>  libavutil/random_seed.c | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 107d533b3e..d6e78297fe 100755
> --- a/configure
> +++ b/configure
> @@ -3892,7 +3892,7 @@ avfilter_deps="avutil"
>  avfilter_suggest="libm stdatomic"
>  avformat_deps="avcodec avutil"
>  avformat_suggest="libm network zlib stdatomic"
> -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
> +avutil_suggest="clock_gettime ffnvcodec gcrypt libm libdrm libmfx opencl openssl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"

What will this do exactly?
James Almer July 5, 2023, 1:03 p.m. UTC | #2
On 7/5/2023 9:56 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-07-05 01:26:14)
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   configure               |  2 +-
>>   libavutil/random_seed.c | 16 ++++++++++++++++
>>   2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 107d533b3e..d6e78297fe 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3892,7 +3892,7 @@ avfilter_deps="avutil"
>>   avfilter_suggest="libm stdatomic"
>>   avformat_deps="avcodec avutil"
>>   avformat_suggest="libm network zlib stdatomic"
>> -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
>> +avutil_suggest="clock_gettime ffnvcodec gcrypt libm libdrm libmfx opencl openssl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
> 
> What will this do exactly?

It's a soft dependency. Only if OpenSSL or GCrypt are enabled they will 
be added to avutil's library dependencies.
Notice the bcrypt entry, also used in random_seed, and all the hwcontext 
backends.
Anton Khirnov July 6, 2023, 7:52 a.m. UTC | #3
Quoting James Almer (2023-07-05 15:03:07)
> On 7/5/2023 9:56 AM, Anton Khirnov wrote:
> > Quoting James Almer (2023-07-05 01:26:14)
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>   configure               |  2 +-
> >>   libavutil/random_seed.c | 16 ++++++++++++++++
> >>   2 files changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/configure b/configure
> >> index 107d533b3e..d6e78297fe 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -3892,7 +3892,7 @@ avfilter_deps="avutil"
> >>   avfilter_suggest="libm stdatomic"
> >>   avformat_deps="avcodec avutil"
> >>   avformat_suggest="libm network zlib stdatomic"
> >> -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
> >> +avutil_suggest="clock_gettime ffnvcodec gcrypt libm libdrm libmfx opencl openssl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
> > 
> > What will this do exactly?
> 
> It's a soft dependency. Only if OpenSSL or GCrypt are enabled they will 
> be added to avutil's library dependencies.
> Notice the bcrypt entry, also used in random_seed, and all the hwcontext 
> backends.

Ok then.
James Almer July 6, 2023, 5:03 p.m. UTC | #4
On 7/6/2023 4:52 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-07-05 15:03:07)
>> On 7/5/2023 9:56 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-07-05 01:26:14)
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    configure               |  2 +-
>>>>    libavutil/random_seed.c | 16 ++++++++++++++++
>>>>    2 files changed, 17 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 107d533b3e..d6e78297fe 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -3892,7 +3892,7 @@ avfilter_deps="avutil"
>>>>    avfilter_suggest="libm stdatomic"
>>>>    avformat_deps="avcodec avutil"
>>>>    avformat_suggest="libm network zlib stdatomic"
>>>> -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
>>>> +avutil_suggest="clock_gettime ffnvcodec gcrypt libm libdrm libmfx opencl openssl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
>>>
>>> What will this do exactly?
>>
>> It's a soft dependency. Only if OpenSSL or GCrypt are enabled they will
>> be added to avutil's library dependencies.
>> Notice the bcrypt entry, also used in random_seed, and all the hwcontext
>> backends.
> 
> Ok then.

Any opinion on the order of function calls? Should i leave /dev/urandom 
as last resort after all (if any) external lib implementations failed, 
considering what Remí mentioned?
Marton Balint July 6, 2023, 5:56 p.m. UTC | #5
On Thu, 6 Jul 2023, James Almer wrote:

> On 7/6/2023 4:52 AM, Anton Khirnov wrote:
>>  Quoting James Almer (2023-07-05 15:03:07)
>>>  On 7/5/2023 9:56 AM, Anton Khirnov wrote:
>>>>  Quoting James Almer (2023-07-05 01:26:14)
>>>>>  Signed-off-by: James Almer <jamrial@gmail.com>
>>>>>  ---
>>>>>     configure               |  2 +-
>>>>>     libavutil/random_seed.c | 16 ++++++++++++++++
>>>>>     2 files changed, 17 insertions(+), 1 deletion(-)
>>>>>
>>>>>  diff --git a/configure b/configure
>>>>>  index 107d533b3e..d6e78297fe 100755
>>>>>  --- a/configure
>>>>>  +++ b/configure
>>>>>  @@ -3892,7 +3892,7 @@ avfilter_deps="avutil"
>>>>>     avfilter_suggest="libm stdatomic"
>>>>>     avformat_deps="avcodec avutil"
>>>>>     avformat_suggest="libm network zlib stdatomic"
>>>>>  -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl
>>>>>  user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia
>>>>>  bcrypt stdatomic"
>>>>>  +avutil_suggest="clock_gettime ffnvcodec gcrypt libm libdrm libmfx
>>>>>  opencl openssl user32 vaapi vulkan videotoolbox corefoundation
>>>>>  corevideo coremedia bcrypt stdatomic"
>>>>
>>>>  What will this do exactly?
>>>
>>>  It's a soft dependency. Only if OpenSSL or GCrypt are enabled they will
>>>  be added to avutil's library dependencies.
>>>  Notice the bcrypt entry, also used in random_seed, and all the hwcontext
>>>  backends.
>>
>>  Ok then.
>
> Any opinion on the order of function calls? Should i leave /dev/urandom as 
> last resort after all (if any) external lib implementations failed, 
> considering what Remí mentioned?

I think it is fine as is. Buffering can be turned off with
setvbuf(f, NULL, _IONBF, 0); so that should not relevant.

Regards,
Marton
James Almer July 6, 2023, 6:36 p.m. UTC | #6
On 7/6/2023 2:56 PM, Marton Balint wrote:
> 
> 
> On Thu, 6 Jul 2023, James Almer wrote:
> 
>> On 7/6/2023 4:52 AM, Anton Khirnov wrote:
>>>  Quoting James Almer (2023-07-05 15:03:07)
>>>>  On 7/5/2023 9:56 AM, Anton Khirnov wrote:
>>>>>  Quoting James Almer (2023-07-05 01:26:14)
>>>>>>  Signed-off-by: James Almer <jamrial@gmail.com>
>>>>>>  ---
>>>>>>     configure               |  2 +-
>>>>>>     libavutil/random_seed.c | 16 ++++++++++++++++
>>>>>>     2 files changed, 17 insertions(+), 1 deletion(-)
>>>>>>
>>>>>>  diff --git a/configure b/configure
>>>>>>  index 107d533b3e..d6e78297fe 100755
>>>>>>  --- a/configure
>>>>>>  +++ b/configure
>>>>>>  @@ -3892,7 +3892,7 @@ avfilter_deps="avutil"
>>>>>>     avfilter_suggest="libm stdatomic"
>>>>>>     avformat_deps="avcodec avutil"
>>>>>>     avformat_suggest="libm network zlib stdatomic"
>>>>>>  -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl
>>>>>>  user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia
>>>>>>  bcrypt stdatomic"
>>>>>>  +avutil_suggest="clock_gettime ffnvcodec gcrypt libm libdrm libmfx
>>>>>>  opencl openssl user32 vaapi vulkan videotoolbox corefoundation
>>>>>>  corevideo coremedia bcrypt stdatomic"
>>>>>
>>>>>  What will this do exactly?
>>>>
>>>>  It's a soft dependency. Only if OpenSSL or GCrypt are enabled they 
>>>> will
>>>>  be added to avutil's library dependencies.
>>>>  Notice the bcrypt entry, also used in random_seed, and all the 
>>>> hwcontext
>>>>  backends.
>>>
>>>  Ok then.
>>
>> Any opinion on the order of function calls? Should i leave 
>> /dev/urandom as last resort after all (if any) external lib 
>> implementations failed, considering what Remí mentioned?
> 
> I think it is fine as is. Buffering can be turned off with
> setvbuf(f, NULL, _IONBF, 0); so that should not relevant.

Ok, applied as is, then.
diff mbox series

Patch

diff --git a/configure b/configure
index 107d533b3e..d6e78297fe 100755
--- a/configure
+++ b/configure
@@ -3892,7 +3892,7 @@  avfilter_deps="avutil"
 avfilter_suggest="libm stdatomic"
 avformat_deps="avcodec avutil"
 avformat_suggest="libm network zlib stdatomic"
-avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
+avutil_suggest="clock_gettime ffnvcodec gcrypt libm libdrm libmfx opencl openssl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt stdatomic"
 postproc_deps="avutil gpl"
 postproc_suggest="libm stdatomic"
 swresample_deps="avutil"
diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
index f5c291263e..2980e565e0 100644
--- a/libavutil/random_seed.c
+++ b/libavutil/random_seed.c
@@ -30,6 +30,11 @@ 
 #include <windows.h>
 #include <bcrypt.h>
 #endif
+#if CONFIG_GCRYPT
+#include <gcrypt.h>
+#elif CONFIG_OPENSSL
+#include <openssl/rand.h>
+#endif
 #include <fcntl.h>
 #include <math.h>
 #include <time.h>
@@ -143,6 +148,17 @@  int av_random_bytes(uint8_t* buf, size_t len)
 #endif
 
     err = read_random(buf, len, "/dev/urandom");
+    if (!err)
+        return err;
+
+#if CONFIG_GCRYPT
+    gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
+    return 0;
+#elif CONFIG_OPENSSL
+    if (RAND_bytes(buf, len) == 1)
+        return 0;
+    err = AVERROR_EXTERNAL;
+#endif
 
     return err;
 }