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

[Rical Jasan <rj at 2c3t dot io>]

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...

> +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"

> +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".

> +``re-seeds'' itself from physical sources of randomness, such as
> +electronic noise and clock jitter.  This means applications do not
> +need to do anything to ensure that the random numbers it produces are
> +different on each run.
> +
> +The catch, however, is that these functions will only produce
> +relatively short random strings in any one call.  Often this is not a
> +problem, but applications that need more than a few kilobytes of
> +cryptographically strong random data should call these functions once
> +and use their output to seed a CRNG.
> +
> +Most applications should use @code{getentropy}.  The @code{getrandom}
> +function is intended for low-level applications which need additional
> +control over blocking behavior.
>  
>  @deftypefun int getentropy (void *@var{buffer}, size_t @var{length})
>  @standards{GNU, sys/random.h}
>  @safety{@mtsafe{}@assafe{}@acsafe{}}
>  
> -This function writes @var{length} bytes of random data to the array
> -starting at @var{buffer}, which must be at most 256 bytes long.  The
> -function returns zero on success.  On failure, it returns @code{-1} and
> -@code{errno} is updated accordingly.
> -
> -The @code{getentropy} function is declared in the header file
> -@file{sys/random.h}.  It is derived from OpenBSD.
> -
> -The @code{getentropy} function is not a cancellation point.  A call to
> -@code{getentropy} can block if the system has just booted and the kernel
> -entropy pool has not yet been initialized.  In this case, the function
> -will keep blocking even if a signal arrives, and return only after the
> -entropy pool has been initialized.
> -
> -The @code{getentropy} function can fail with several errors, some of
> -which are listed below.
> +This function writes exactly @var{length} bytes of random data to the
> +array starting at @var{buffer}.  @var{length} can be no more than 256.
> +On success, it returns zero.  On failure, it returns @math{-1}, and
> +@code{errno} is set to indicate the problem.  Some of the possible
> +errors are listed below.
>  
>  @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.

>  @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.  :)

>  @end table
>  
> +A call to @code{getentropy} can only block when the system has just
> +booted and the randomness source has not yet been initialized.
> +However, if it does block, it cannot be interrupted by signals or
> +thread cancellation.  Programs intended to run in very early stages of
> +the boot process may need to use @code{getrandom} in non-blocking mode
> +instead, and be prepared to cope with random data not being available
> +at all.

Good.

> +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.  Does BSD
declare it in sys/random.h?

>  @end deftypefun
>  
>  @deftypefun ssize_t getrandom (void *@var{buffer}, size_t @var{length}, unsigned int @var{flags})
>  @standards{GNU, sys/random.h}
>  @safety{@mtsafe{}@assafe{}@acsafe{}}
>  
> -This function writes @var{length} bytes of random data to the array
> -starting at @var{buffer}.  On success, this function returns the number
> -of bytes which have been written to the buffer (which can be less than
> -@var{length}).  On error, @code{-1} is returned, and @code{errno} is
> -updated accordingly.
> -
> -The @code{getrandom} function is declared in the header file
> -@file{sys/random.h}.  It is a GNU extension.
> -
> -The following flags are defined for the @var{flags} argument:
> +This function writes up to @var{length} bytes of random data to the
> +array starting at @var{buffer}.  The @var{flags} argument should be
> +either zero, or the bitwise OR of some of the following flags:
>  
>  @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.

> +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.  It's more to the point, IMO, as it directly addresses the use
of this flag.  (Though without a GRND_URANDOM flag, it is fair to
address the non-use of this flag here, which I see you pulled from below.)

>  @item GRND_NONBLOCK
>  Instead of blocking, return to the caller immediately if no data is
>  available.
>  @end table
>  
> -The @code{getrandom} function is a cancellation point.
> +Unlike @code{getentropy}, the @code{getrandom} function is a
> +cancellation point, and if it blocks, it can be interrupted by
> +signals.

Good.

> -Obtaining randomness from the @file{/dev/urandom} pool (i.e., a call
> -without the @code{GRND_RANDOM} flag) can block if the system has just
> -booted and the pool has not yet been initialized.

(Moved to GRND_RANDOM discussion.)  OK.  Otherwise, I would have
considered moving it earlier in the description, but it would be
difficult to avoid disrupting the flow leading to the flags discussion.

> -The @code{getrandom} function can fail with several errors, some of
> -which are listed below.  In addition, the function may not fill the
> -buffer completely and return a value less than @var{length}.
> +On success, @code{getrandom} returns the number of bytes which have
> +been written to the buffer, which may be less than @var{length}.  On
> +error, it returns @math{-1}, and @code{errno} is set to indicate the
> +problem.  Some of the possible errors are:

I like the way you used "up to LENGTH" earlier, and moved the
conversation about return values here without being redundant.

>  @table @code
>  @item ENOSYS
> -The kernel does not implement the @code{getrandom} 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.)

(See comments on ENOSYS for getentropy.)

>  @item EAGAIN
>  No random data was available and @code{GRND_NONBLOCK} was specified in
> @@ -228,4 +334,7 @@ the kernel randomness pool is initialized, this can happen even if
>  The @var{flags} argument contains an invalid combination of flags.
>  @end table
>  
> +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.)

> diff --git a/manual/examples/genpass.c b/manual/examples/genpass.c

...

I don't have time to review the examples right now, and there's even
more after that, so I'll try to finish this over the weekend.  It's a
pretty heavy rewrite.  So far, there have been a few things I don't
think needed to be so heavily rewritten, but it's quality.  :)  So far,
so good.

Rical