On Thu, 13 Aug 2020 at 20:32, Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> > Depending on the RNG type, the RNG must be seeded. The seed is provided
> > using the setsockopt interface to set the key. For example, the
> > ansi_cprng requires a seed. The DRBGs do not require a seed, but may be
> > -seeded.
> > +seeded. The seed is also known as a *Personalization String* in DRBG800-90A
> > +standard.
>
> Isn't the standard called "NIST SP 800-90A"?
> "DRBG800-90A" doesn't return many hits on Google.
Fixed, thanks!
> > +For the purpose of CAVP testing, the concatenation of *Entropy* and *Nonce*
> > +can be provided to the RNG via ALG_SET_DRBG_ENTROPY setsockopt interface. This
> > +requires a kernel built with CONFIG_CRYPTO_USER_API_CAVP_DRBG, and
> > +CAP_SYS_ADMIN permission.
> > +
> > +*Additional Data* can be provided using the send()/sendmsg() system calls.
>
> This doesn't make it clear whether the support for "Additional Data" is
> dependent on CONFIG_CRYPTO_USER_API_CAVP_DRBG or not.
Fixed.
> > +config CRYPTO_USER_API_CAVP_DRBG
> > + tristate "Enable CAVP testing of DRBG"
> > + depends on CRYPTO_USER_API_RNG && CRYPTO_DRBG
> > + help
> > + This option enables extra API for CAVP testing via the user-space
> > + interface: resetting of DRBG entropy, and providing Additional Data.
> > + This should only be enabled for CAVP testing. You should say
> > + no unless you know what this is.
>
> Using "tristate" here is incorrect because this option is not a module itself.
> It's an option *for* a module. So it needs to be "bool" instead.
Done.
> Also, since this is an option to refine CRYPTO_USER_API_RNG, how about renaming
> it to "CRYPTO_USER_API_RNG_CAVP", and moving it to just below the definition of
> "CRYPTO_USER_API_RNG" so that they show up adjacent to each other?
Sounds good, done.
> > +static int rng_test_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> > + int flags)
> > +{
> > + struct sock *sk = sock->sk;
> > + struct alg_sock *ask = alg_sk(sk);
> > + struct rng_ctx *ctx = ask->private;
> > + int err;
> > +
> > + lock_sock(sock->sk);
> > + err = _rng_recvmsg(ctx->drng, msg, len, ctx->addtl, ctx->addtl_len);
> > + rng_reset_addtl(ctx);
> > + release_sock(sock->sk);
> > +
> > + return err ? err : len;
>
> Shouldn't this just return the value that _rng_recvmsg() returned?
>
> Also 'err' is conventionally just for 0 or -errno codes. Use 'ret' if the
> variable can also hold a length.
Done.
