[m-rev.] for review: add a system RNG implementation for the C backends on Linux

[Peter Wang]

On Sun, 31 Jan 2021 15:15:34 +1100 Julien Fischer <jfischer at opturion.com> wrote:
> 
> For review by anyone.
> 
> ------------------------
> 
> Add a system RNG implementation for the C backends on Linux.
> 
> Add a system RNG implementation for the C backends on Linux that works by
> reading random bits from /dev/urandom. We will eventually provide a version
> that reads from getrandom() on Linux systems that provide that; this
> implementation is a fallback for those system that do not.  (It may also
> be what we use on other Unix-like system that do not provide anything
> better.)
> 
> runtime/mercury_random.h:
> runtime/mercury_random.c:
>      Add new runtime module that provides the interface to the system
>      RNG for the C backends.
> 
>      Add two implementations of the system RNG, one that reads from
>      /dev/urandom and one that just aborts.  The former is used on
>      Linux and the latter is used everywhere else (for now).
> 
> runtime/Mmakefile:
>      Add the new files.
> 
> library/random.system_rng.m:
>      Add C foreign procs for the predicates in this module that
>      forward all their work to the new runtime module.
> 
> Julien.
> 
> diff --git a/library/random.system_rng.m b/library/random.system_rng.m
> index 872058c..affd2c4 100644
> --- a/library/random.system_rng.m
> +++ b/library/random.system_rng.m
> @@ -84,16 +84,19 @@
> 
>   :- import_module bool.
>   :- import_module exception.
> +:- import_module list.
> +:- import_module string.
> 
>   %---------------------------------------------------------------------------%
> 
> -:- pragma foreign_type("Java", system_rng,
> -    "java.security.SecureRandom").
> +:- pragma foreign_decl("C", "#include \"mercury_random.h\"").
> +

IMO the C code doesn't need to be in the runtime directory.


> diff --git a/runtime/mercury_random.c b/runtime/mercury_random.c
> index e69de29..85a1025 100644
> --- a/runtime/mercury_random.c
> +++ b/runtime/mercury_random.c
> @@ -0,0 +1,214 @@
> +// vim: ts=4 sw=4 expandtab ft=c
> +
> +// Copyright (C) 2021 The Mercury team.
> +// This file is distributed under the terms specified in COPYING.LIB.
> +
> +// mercury_random.c
> +

There are a few stray space characters at EOLs in this file.

> +#if defined(MR_SYSRAND_IMPL_URANDOM)
> +
> +static MR_Bool
> +read_bytes_from_urandom(int fd, void *buffer, size_t n,
> +    MR_String *err_msg);

It's not specific to urandom to maybe rename it to read_bytes
or read_bytes_from_fd?

> +
> +MR_SystemRandomHandle
> +MR_random_open(MR_Bool *succeeded, MR_String *err_msg)
> +{
> +    int fd;
> +    char errbuf[MR_STRERROR_BUF_SIZE]; 
> +    MR_SystemRandomHandle handle;
> +
> +    do {
> +        fd = open("/dev/urandom", O_RDONLY);
> +    } while (fd == -1 && MR_is_eintr(errno));
> +
> +    if (fd == -1) {
> +        MR_strerror(errno, errbuf, sizeof(errbuf));

You must use the return value of MR_strerror() as it may not
fill the given buffer.

> +        MR_save_transient_hp();
> +        MR_make_aligned_string_copy(*err_msg, errbuf);
> +        MR_restore_transient_hp();
> +        *succeeded = MR_NO;
> +        handle = NULL;
> +    } else {
> +        handle =
> +            MR_GC_malloc(sizeof(struct MR_SystemRandomHandle_Struct));

Could use MR_GC_NEW.

> +        handle->MR_srh_fd = fd;
> +        *err_msg = MR_make_string_const("");
> +        *succeeded = MR_YES;
> +    }
> + 
> +    return handle;
> +}
> +
> +MR_Bool
> +MR_random_close(MR_SystemRandomHandle handle, MR_String *err_msg)
> +{
> +    char errbuf[MR_STRERROR_BUF_SIZE]; 
> +
> +    if (close(handle->MR_srh_fd) == -1) {
> +        MR_strerror(errno, errbuf, sizeof(errbuf));

As above.

> +        MR_save_transient_hp();
> +        MR_make_aligned_string_copy(*err_msg, errbuf);
> +        MR_restore_transient_hp();
> +        return MR_NO;
> +    } else {
> +        handle->MR_srh_fd = -1;
> +        *err_msg = MR_make_string_const("");
> +        return MR_YES;
> +    }
> +}
> +

> +uint8_t
> +MR_random_generate_uint8(MR_SystemRandomHandle handle,
> +    MR_Bool *succeeded, MR_String *err_msg)
> +{
> +    unsigned char buffer[1];
> +    if (read_bytes_from_urandom(handle->MR_srh_fd, buffer, 1, err_msg)) {
> +        *succeeded = MR_TRUE;
> +        return (uint8_t) buffer[0];
> +    } else {
> +        *succeeded = MR_FALSE;
> +        return 0;
> +    }
> +}

Why not have a single function which takes a buffer and length?
The caller can use a union type consisting of the uintN_t and the
unsigned char buffer:

    union {
	uint16_t x;
	unsigned char buf[2];
    } u;
    succeeded =
	MR_random_generate_bytes(handle, u.buf, sizeof(u.buf), &err_msg);

> +MR_Bool
> +read_bytes_from_urandom(int fd, void *buffer, size_t n,
> +    MR_String *err_msg)
> +{
> +    int i;
> +    char errbuf[MR_STRERROR_BUF_SIZE]; 
> +
> +    for (i = 0; i < n; ) {
> +        size_t to_read = n - i;
> +        ssize_t bytes_read = read(fd, buffer, to_read);
> +        if (bytes_read == -1) {
> +            if (MR_is_eintr(errno)) {
> +                continue;
> +            } else {
> +                MR_strerror(errno, errbuf, sizeof(errbuf));

As above.

> +                MR_save_transient_hp();
> +                MR_make_aligned_string_copy(*err_msg, errbuf);
> +                MR_restore_transient_hp();
> +                return MR_FALSE;
> +            }
> +        }
> +        i += bytes_read;
> +    }
> +
> +    *err_msg = MR_make_string_const("");
> +    return MR_TRUE;
> +}
> +
> +#else // MR_SYSRAND_IMPL_NONE

> diff --git a/runtime/mercury_random.h b/runtime/mercury_random.h
> index e69de29..8e29c78 100644
> --- a/runtime/mercury_random.h
> +++ b/runtime/mercury_random.h
> @@ -0,0 +1,97 @@
> +// vim: ts=4 sw=4 expandtab ft=c
> +
> +// Copyright (C) 2021 The Mercury team.
> +// This file is distributed under the terms specified in COPYING.LIB.
> +
> +// mercury_random.h - code for interacting with the system RNG.
> +
> +#ifndef MR_MERCURY_RANDOM_H
> +#define MERCURY_RANDOM_H
> +
> +#include "mercury_conf_param.h"
> +#include "mercury_types.h"
> +#include "mercury_windows.h"
> +
> +#include <stdint.h>
> +
> +////////////////////////////////////////////////////////////////////////////
> +//
> +// The following macros define if the system random number exists on this
> +// system and, if so, how it is accessed.
> +//
> +// Only one of the following must be defined.
> +//
> +// MR_SYSRAND_IMPL_ARC4RANDOM (NYI)
> +//    the system RNG is implemented by calling the arc4random() family of
> +//    functions. Note: this for when arc4random() is provided by libc (as on
> +//    macOS and the BSDs), not for when it is provided as a separate library
> +//    (e.g. libbsd on Linux).
> +//
> +// MR_SYSRAND_IMPL_RAND_S (NYI)
> +//    the system RNG is implemented by calling the rand_s() function
> +//    (Windows only).
> +//
> +// MR_SYSRAND_IMPL_GETRANDOM (NYI)
> +//    the system RNG is implemented by calling getrandom() (sufficiently
> +//    recent Linux kernels only).
> +//
> +// MR_SYSRAND_IMPL_URANDOM
> +//     the system RNG is implemented by reading from /dev/urandom.
> +//     (XXX currently only tested, and enabled, on Linux.)
> +//
> +// MR_SYSRAND_IMPL_NONE
> +//     there is no system RNG is not available on this platform.
> +
> +#if defined(__linux__)
> +    #define MR_SYSRAND_IMPL_URANDOM
> +#else
> +    #define MR_SYSRAND_IMPL_NONE
> +#endif

I've used /dev/urandom on Linux, FreeBSD, OpenBSD, MacOS X and AIX,
and Solaris and NetBSD also have it, so you can enable
MR_SYSRAND_IMPL_URANDOM on anything but Windows.

Peter