[FFmpeg-devel] [PATCH 2/2] configure: libbsd support for arc4random()

[Ganesh Ajjanagadde]

On Wed, Dec 9, 2015 at 1:56 PM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> On Wed, Dec 9, 2015 at 11:00 AM, wm4 <nfxjfg at googlemail.com> wrote:
>> On Wed, 9 Dec 2015 09:39:47 -0500
>> Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>>
>>> On Wed, Dec 9, 2015 at 8:28 AM, wm4 <nfxjfg at googlemail.com> wrote:
>>> > On Tue, 8 Dec 2015 09:01:47 -0500
>>> > Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
>>> >
>>> >> On Tue, Dec 8, 2015 at 8:16 AM, Clément Bœsch <u at pkh.me> wrote:
>>> >> > On Tue, Dec 08, 2015 at 07:34:51AM -0500, Ganesh Ajjanagadde wrote:
>>> >> >> On Tue, Dec 8, 2015 at 7:27 AM, wm4 <nfxjfg at googlemail.com> wrote:
>>> >> >> > On Sun,  6 Dec 2015 22:56:33 -0500
>>> >> >> > Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>>> >> >> >
>>> >> >> >> On non-BSD machines, there exists a package libbsd for providing BSD
>>> >> >> >> functionality. This can be used to get support for arc4random.
>>> >> >> >>
>>> >> >> >> Thus, an opt-in --enable-libbsd is added to configure for this
>>> >> >> >> functionality.
>>> >> >> >>
>>> >> >> >> Tested on GNU/Linux.
>>> >> >> >>
>>> >> >> >
>>> >> >> > This doesn't seem worth the trouble at all. Who is even going to use
>>> >> >> > it, and why, and what additional hidden bugs will it cause?
>>> >> >>
>>> >> >> arc4random is a far superior interface, in that it can never fail. See
>>> >> >> http://www.linuxfromscratch.org/hints/downloads/files/entropy.txt for
>>> >> >> details.
>>> >> >> As for hidden bugs, apart from configuration/detection issues, I see none.
>>> >> >> If anything, it is easier to use /dev/urandom incorrectly.
>>> >> >> Ultimately any code change is a tradeoff, with different people
>>> >> >> feeling differently about various things.
>>> >> >> I still feel that it is worth inclusion due to its technical merits.
>>> >> >>
>>> >> >
>>> >> > Note that the behaviour of arc4random differs between implementations.
>>> >> >
>>> >> > http://insanecoding.blogspot.gr/2014/05/libressl-porting-update.html
>>> >>
>>> >> That is for libressl, not libbsd, though the remark may still apply.
>>> >> And there is no real fundamental issue: FFmpeg's seeding code anyway
>>> >> varies between platforms, in the worst case using time.
>>> >>
>>> >> Second, a getrandom system call has been added to the kernel, so
>>> >> libbsd/libressl should upgrade to the new interface over time.
>>> >>
>>> >> Whatever, if people don't want this, I will drop 2/2 but keep within
>>> >> my own tree for a future date potentially.
>>> >>
>>> >> 1/2 is still very much worthwhile: platforms supporting natively
>>> >> arc4random should use it (e.g the BSD's).
>>> >
>>> > You should wait until glibc supports the getrandom syscall, instead of
>>> > using a wrapper lib that is merely meant to make porting BSD programs
>>> > simpler. (Looking at libbsd, it does try to use the getrandom syscall,
>>> > but also tries potentially dangerous fallbacks like using the system
>>> > time as random seed? Isn't that exactly the wrong thing if you want
>>> > 100% correctness?)
>>>
>>> I looked at the libbsd code right now. Strictly speaking, you are
>>> right, but it falls back to time of day only on very rare platforms.
>>> In particular, it uses /dev/urandom when available. Thus, it turns out
>>
>> So what, we do that too? Just because libbsd accumulated a shitload of
>> hacks it doesn't mean we have to use it, neither does it have to mean
>> it's "better".
>
> libbsd uses a "shitload of hacks" because there is no portable way of
> generating good random number seeds efficiently and of good quality.
> The documentation states that FFmpeg makes a best effort for providing
> a good random number seed. By best effort, I define it along the
> dimensions of seed quality and speed. libbsd provides improvements
> along one axis for FFmpeg, keeps the other axis fixed afaik, and so I
> still consider the change worthy.
>
> I can't speak for your definition of "better", which you have not clarified.

For completeness, sample benchmark on GNU/Linux (via
avutil/random_seed-test, START/STOP):
   2255 decicycles in av_get_random_seed,     255 runs,      1 skips

old:
  41445 decicycles in av_get_random_seed,     256 runs,      0 skips

representing a ~ 18x speedup.

>
>>
>> Anyway, I'm strictly against making FFmpeg depend on a BSD
>> compatibility library, even optionally. It makes no sense. I'm out of
>> here.

BTW, if you had such a strong opinion about it, please say it out
right at the very beginning instead of pretending to be interested and
later showing your true inclinations. The knowledge needed for this
response could have been obtained right when I posted the patch, or
even from 0/2, where I detail the rationale.

>
> I request a review for 1/2.
> You have a tendency to have strong reactions regarding hacks and/or
> external dependencies for things, as long as it is not mpv. I thus
> also request a second opinion on this.
>
>>
>>> that using it over /dev/urandom should actually be a Pareto
>>> improvement (ref: getentropy in _rs_stir). Or more generally, using it
>>> when available via --enable-libbsd will never lower FFmpeg's random
>>> seeding quality.
>>>
>>> I am now 90-10 for 2/2.
>>>
>>> > _______________________________________________
>>> > ffmpeg-devel mailing list
>>> > ffmpeg-devel at ffmpeg.org
>>> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel