Subject: Re: [ISC-Bugs #46047] improve crypto-rand UI and clarify RNG use in general
Content-Disposition: inline
content-type: text/plain; charset="utf-8"
User-Agent: Mutt/1.5.23 (2014-03-12)
Date: Wed, 27 Sep 2017 18:23:56 +0000
In-Reply-To: <rt-4.4.1-30085-1506523120-1399.46047-5-0@isc.org>
Return-Path: <each@isc.org>
X-RT-Original-Encoding: utf-8
Message-ID: <20170927182356.GA95908@isc.org>
From each@isc.org  Wed Sep 27 18:23:56 2017
X-RT-Interface: Email
X-RT-Incoming-Encryption: Not encrypted
From: "Evan Hunt" <each@isc.org>
X-Original-To: bind9-public@bugs.isc.org
References: <RT-Ticket-46047@isc.org> <rt-4.4.1-49214-1505851568-605.46047-5-0@isc.org> <rt-4.4.1-76225-1506447380-162.46047-5-0@isc.org> <rt-4.4.1-30085-1506523120-1399.46047-5-0@isc.org>
To: "Francis Dupont via RT" <bind9-public@isc.org>
Delivered-To: bind9-public@bugs.isc.org
MIME-Version: 1.0
Received: from bikeshed.isc.org (bikeshed.isc.org [149.20.48.19]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client CN "mail.isc.org", Issuer "RapidSSL CA" (not verified)) by bugs.isc.org (Postfix) with ESMTPS id A706AD78B0A for <bind9-public@bugs.isc.org>; Wed, 27 Sep 2017 18:23:56 +0000 (UTC)
Received: by bikeshed.isc.org (Postfix, from userid 10292) id 6D865216C1E; Wed, 27 Sep 2017 18:23:56 +0000 (UTC)
RT-Message-ID: <rt-4.4.1-33589-1506536637-420.46047-0-0@isc.org>
Content-Length: 1214

On Wed, Sep 27, 2017 at 02:38:40PM +0000, Francis Dupont via RT wrote:
>  - it is a matter of taste but IMHO in generate_salt
>   i and n should be size_t (or at least unsigned).

I'm fine with this.

>  - you removed the ifdef SC_PLATFORM_CRYPTORANDOM
>   in dst__entropy_getdata. Please put it back as it simplifies
>   the code and makes sure that dst library never uses a
>   not crypto library PNRG. BTW don't confuse
>   isc_entropy_usehook and isc_entropy_sethook, only
>   the first changes the source of entropy.

I removed it because, when we're running with cryptorandom,
isc_entropy_getdata() is hooked to dst_random_getdata() anyway, and if
we're *not* running with cryptorandom, dst_random_getdata() returns
NOTIMPLEMENTED, which means it's best if dst_random_getdata() isn't
called directly.

Doing it the way I did seems safer and cleaner, and it only adds the
overhead of a single function call.

Come to think of it, I'd prefer it if it were called dst__random_getdata,
to discourage people from relying on it or calling it directly. Just use
it as an internal hook function only.

If you absolutely insist, I'll put it back the way it was, but I really
do think this is architecturally better.