Re: [PATCH 3/4] Revise crypt.texi.

[Florian Weimer <fweimer at redhat dot com>]

On 05/25/2018 04:52 AM, Rical Jasan wrote:
> On 05/21/2018 10:38 AM, Zack Weinberg wrote:
> ...
> > diff --git a/crypt/crypt.h b/crypt/crypt.h
> ...
> > diff --git a/inet/ruserpass.c b/inet/ruserpass.c
> ...
> > diff --git a/manual/contrib.texi b/manual/contrib.texi
> ...
> > diff --git a/manual/crypt.texi b/manual/crypt.texi
> ...
>
> I have no comments or complaints on anything up to this point.
>
> > @@ -123,93 +202,120 @@ for a password and prints ``Access granted.'' if the user types
> >   
> >   @node Unpredictable Bytes
> >   @section Generating Unpredictable Bytes
> > -
> > -Some cryptographic applications (such as session key generation) need
> > -unpredictable bytes.
> > -
> > -In general, application code should use a deterministic random bit
> > -generator, which could call the @code{getentropy} function described
> > -below internally to obtain randomness to seed the generator.  The
> > -@code{getrandom} function is intended for low-level applications which
> > -need additional control over the blocking behavior.
> > +@cindex randomness source
> > +@cindex random numbers, cryptographic
> > +@cindex pseudo-random numbers, cryptographic
> > +@cindex cryptographic random number generator
> > +@cindex deterministic random bit generator
> > +@cindex CRNG
> > +@cindex CSPRNG
> > +@cindex DRBG
> > +
> > +Cryptographic applications often need some random data that will be as
> > +difficult as possible for a hostile eavesdropper to guess.  For
> > +instance, encryption keys should be chosen at random, and the ``salt''
> > +strings used by @code{crypt} (@pxref{Passphrase Storage}) should also
> > +be chosen at random.
> > +
> > +The pseudo-random number generators described in @ref{Pseudo-Random
> > +Numbers} are not unpredictable enough for these applications.  They
>
> @ref will still render with the "see" verb in some formats, so it's
> generally a bad choice.  Maybe:
>
> Some pseudo-random number generators do not provide unpredictable-enough
> output for cryptographic applications; @pxref{Pseudo-Random Numbers}.
> Such applications...

Thanks, applied.

> > +need to use a @dfn{cryptographic random number generator} (CRNG), also
> > +sometimes called a @dfn{cryptographically strong pseudo-random number
> > +generator} (CSPRNG) or @dfn{deterministic random bit generator} (DRBG).
> > +
> > +Currently, @theglibc{} does not provide a cryptographic random number
> > +generator.  What it does provide is functions that read random data
>
> "are functions"

I'm going to use this:

“
Currently, @theglibc{} does not provide a cryptographic random number
generator, but it does provide functions that read random data
from a @dfn{randomness source} supplied by the operating system.  The
”

> > +from a @dfn{randomness source} supplied by the operating system.  The
> > +randomness source is a CRNG at heart, but it also also continually
>
> Double "also".

Fixed.

> >   @table @code
> >   @item ENOSYS
> > -The kernel does not implement the required system call.
> > +The operating system does not implement a randomness source, or does
> > +not support this way of accessing it.  (For instance, the system call
> > +used by this function was added to the Linux kernel in version 3.17.)
>
> I'm not sure if I'm missing a (possibly not so) subtle point here, but I
> feel like this is more words to say the same thing.  What other ways of
> accessing an operating system's (as opposed to the kernel's) source of
> randomness would glibc take advantage of?  It kind of sounds like we're
> not doing due diligence in that light.  I could just be reading this
> wrong, though.

I think it's about covering the Hurd, which does not have kernel support 
for getrandom.  But I don't feel strongly about this.

> >   @item EFAULT
> >   The combination of @var{buffer} and @var{length} arguments specifies
> >   an invalid memory range.
> >   
> >   @item EIO
> > -More than 256 bytes of randomness have been requested, or the buffer
> > -could not be overwritten with random data for an unspecified reason.
> > -
> > +@var{length} is larger than 256, or the kernel entropy pool has
> > +suffered a catastrophic failure.
>
> Someone else may need to comment on this.  The original version leaves
> it pretty wide open as to why one would get EIO.  Is it really only just
> because LENGTH was greater than 256 or there was a problem in a specific
> subsystem of the kernel?  If so, then OK.
>
> I do appreciate that this would be the first use of "catastrophic
> failure" in an errno description.  :)

I think both versions are okay.  I expect that Zack looked at the kernel 
sources here.

> > +The @code{getentropy} function is declared in the header file
> > +@file{sys/random.h}.  It is derived from OpenBSD.
>
> Should the @standards be adjusted?  If this isn't purely a GNU
> extension, I think credit should be given where credit's due.

Well, we already give credit to OpenBSD here.

> Does BSD declare it in sys/random.h?

No, that header was pioneered by Solaris.

> >   @table @code
> >   @item GRND_RANDOM
> > -Use the @file{/dev/random} (blocking) pool instead of the
> > -@file{/dev/urandom} (non-blocking) pool to obtain randomness.  If the
> > -@code{GRND_RANDOM} flag is specified, the @code{getrandom} function can
> > -block even after the randomness source has been initialized.
> > +Use the @file{/dev/random} (blocking) source instead of the
> > +@file{/dev/urandom} (non-blocking) source to obtain randomness.
>
> Referring to the (entropy) "pool" seems more natural in this context;
> did you change it to "source" strictly for consistency?  OK either way,
> but it's noticeable.

I think “source” is more useful to the casual reader who doesn't know 
about implementation strategies for CSPRNGs.

> > +If this flag is specified, the call may block, potentially for quite
> > +some time.  If it is not specified, the call can only block when the
> > +system has just booted and the randomness source has not yet been
> > +initialized.
>
> I think the prior information that using this flag can cause the call to
> block "even after the randomness source has been initialized" is
> valuable.

Agreed, this is critical information.  I'm going with this:

“
If this flag is specified, the call may block, potentially for quite
some time, even after the randomness source has been initialized.  If it
is not specified, the call can only block when the system has just
booted and the randomness source has not yet been initialized.
”

> > +The @code{getrandom} function is declared in the header file
> > +@file{sys/random.h}.  It is a GNU extension.
> > +
> >   @end deftypefun
>
> Good, moving this to the bottom of the description, which is where this
> information is typically located.
>
> Does the extra blank line make a noticeable improvement in the rendered
> output?  (Some places in the manual have @comments saying to preserve
> them for that reason.)

I think current makeinfo is way less sensitive about input whitespace 
then it used to be.  The blank line does not survive into HTML and Info 
output (but the latter is difficult to tell because there's no text 
after it in the node).

Thanks,
Florian