Re: [PATCH v2 0/2] Generate temporary files using a CSPRNG

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi brian,

On Tue, 4 Jan 2022, brian m. carlson wrote:

> Currently, when we generate a temporary file name, we use the seconds,
> microseconds, and the PID to generate a unique value.  The resulting
> value, while changing frequently, is actually predictable and on some
> systems, it may be possible to cause a DoS by creating all potential
> temporary files when the temporary file is being created in TMPDIR.
>
> The solution to this is to use the system CSPRNG to generate the
> temporary file name.  This is the approach taken by FreeBSD, NetBSD, and
> OpenBSD, and glibc also recently switched to this approach from an
> approach that resembled ours in many ways.
>
> [...]

This gets my happy Reviewed-By:, and I very much liked the thoroughness
and diligence of your work. It does an excellent job of motivating the
change, and documents the consideration and diligence that has gone into
the patches. Thank you for this refreshing patch series, which provided me
with a nice start into the new year.

To show that I did not just glance over this for five minutes and then
moved on to the next email in order to throw as many emails at the Git
mailing list as possible, I'd like to state that I mulled for a while over
the change in `compat/winansi.c`. It first got me concerned that the
changes to `git-compat-util.h` might be done at the wrong layer: if
something as unrelated to the winansi emulation as temporary files
creation requires fiddling, so that it can continue to include the headers
it requires, wouldn't it make more sense to create a new file called
`csprng.c` and add the new conditional `#include`s _there? Alas, it _is_
Git's custom to hide platform-dependent parts in `git-compat-util.h` as
much as possible. And personal opinions should always stand back in favor
of consistency of the code base. In short: I agree with your choice to add
the conditional `#include`s to `git-compat-util.h` even if it requires a
somewhat stray (but of course well-documented, thank you for that!)
`#undef` in `compat/winansi.c`.

Thanks!
Dscho