Re: [FFmpeg-devel] [PATCH v2 1/2] avutil/random_seed: add av_random()

[James Almer <jamrial@gmail.com>]

On 7/4/2023 5:54 PM, Hendrik Leppkes wrote:
> On Tue, Jul 4, 2023 at 10:41 PM James Almer <jamrial@gmail.com> wrote:
>>
>> Uses the existing code for av_get_random_seed() to return a buffer with
>> cryptographically secure random data, or an error if none could be generated.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavutil/random_seed.c | 54 ++++++++++++++++++++++++++++-------------
>>   libavutil/random_seed.h | 12 +++++++++
>>   2 files changed, 49 insertions(+), 17 deletions(-)
>>
>> diff --git a/libavutil/random_seed.c b/libavutil/random_seed.c
>> index 66dd504ef0..0ed8f89cc6 100644
>> --- a/libavutil/random_seed.c
>> +++ b/libavutil/random_seed.c
>> @@ -46,20 +46,22 @@
>>   #define TEST 0
>>   #endif
>>
>> -static int read_random(uint32_t *dst, const char *file)
>> -{
>>   #if HAVE_UNISTD_H
>> +static int read_random(uint8_t *dst, size_t len, const char *file)
>> +{
>>       int fd = avpriv_open(file, O_RDONLY);
>> -    int err = -1;
>> +    ssize_t err = -1;
>>
>> +    if (len > SSIZE_MAX)
>> +        return -1;
>>       if (fd == -1)
>>           return -1;
>> -    err = read(fd, dst, sizeof(*dst));
>> +    err = read(fd, dst, len);
>>       close(fd);
>> +    if (err == -1)
>> +        return AVERROR(errno);
>>
>> -    return err;
>> -#else
>> -    return -1;
>> +    return err == len;
>>   #endif
>>   }
>>
>> @@ -118,29 +120,47 @@ static uint32_t get_generic_seed(void)
>>       return AV_RB32(digest) + AV_RB32(digest + 16);
>>   }
>>
>> -uint32_t av_get_random_seed(void)
>> +int av_random(uint8_t* buf, size_t len)
>>   {
>> -    uint32_t seed;
>> +    int err = AVERROR_UNKNOWN;
>>
>>   #if HAVE_BCRYPT
>>       BCRYPT_ALG_HANDLE algo_handle;
>>       NTSTATUS ret = BCryptOpenAlgorithmProvider(&algo_handle, BCRYPT_RNG_ALGORITHM,
>>                                                  MS_PRIMITIVE_PROVIDER, 0);
>>       if (BCRYPT_SUCCESS(ret)) {
>> -        NTSTATUS ret = BCryptGenRandom(algo_handle, (UCHAR*)&seed, sizeof(seed), 0);
>> +        NTSTATUS ret = BCryptGenRandom(algo_handle, (PUCHAR)buf, len, 0);
>>           BCryptCloseAlgorithmProvider(algo_handle, 0);
>>           if (BCRYPT_SUCCESS(ret))
>> -            return seed;
>> +            return 0;
>>       }
>>   #endif
>>
>>   #if HAVE_ARC4RANDOM
>> -    return arc4random();
>> +    arc4random_buf(buf, len);
>> +    return 0;
>> +#endif
>> +
>> +#if HAVE_UNISTD_H
>> +    err = read_random(buf, len, "/dev/urandom");
>> +    if (err == 1)
>> +        return 0;
>> +    err = read_random(buf, len, "/dev/random");
>> +    if (err == 1)
>> +        return 0;
>> +    if (err == 0)
>> +           err = AVERROR_UNKNOWN;
>>   #endif
>>
>> -    if (read_random(&seed, "/dev/urandom") == sizeof(seed))
>> -        return seed;
>> -    if (read_random(&seed, "/dev/random")  == sizeof(seed))
>> -        return seed;
>> -    return get_generic_seed();
>> +    return err;
>> +}
>> +
>> +uint32_t av_get_random_seed(void)
>> +{
>> +    uint32_t seed;
>> +
>> +    if (av_random((uint8_t *)&seed, sizeof(seed)) < 0)
>> +        return get_generic_seed();
>> +
>> +    return seed;
>>   }
>> diff --git a/libavutil/random_seed.h b/libavutil/random_seed.h
>> index 0462a048e0..ce982bb82f 100644
>> --- a/libavutil/random_seed.h
>> +++ b/libavutil/random_seed.h
>> @@ -36,6 +36,18 @@
>>    */
>>   uint32_t av_get_random_seed(void);
>>
>> +/**
>> + * Generate cryptographically secure random data, i.e. suitable for use as
>> + * encryption keys and similar.
>> + *
>> + * @param buf buffer into which the random data will be written
>> + * @param len size of buf in bytes
>> + *
>> + * @retval 0 success, and len bytes of random data was written into buf, or
>> + *         a negative AVERROR code if random data could not be generated.
>> + */
>> +int av_random(uint8_t* buf, size_t len);
> 
> av_random seems like a pretty generic name for something thats
> requiring to be cryptographically secure and otherwise fail. I would
> expect a more specific name for that purpose. There is plenty other
> uses of randomness in multimedia, noise, dithering, etc, which don't
> need crypto security. The function doesn't have to handle those, but
> maybe it should be specific in what it does handle?

Maybe av_random_buf()? I don't want too much bikeshedding on the name.
_______________________________________________
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".