[FFmpeg-devel] [PATCH 1/2] avformat/hlsenc: fall back to av_get_random_seed() when generating AES128 key

[James Almer]

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?