tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: question about /src/sys/kern/subr_cprng.c 1.15



On Mon, Mar 25, 2013 at 01:45:50PM +0100, Joachim Kuebart wrote:
> On 25 March 2013 13:32, Joachim Kuebart <joachim.kuebart%gmail.com@localhost> 
> wrote:
> > just read about this on the news and have a question about
> > abovementioned diff: in the call to cprng_entropy_try from
> > cprng_strong_create, shouldn't the parameter 'hard' be false unless
> > CPRNG_INIT_ANY is specified? The original code as well as the call
> > from cprng_strong would suggest that RNG_EXTRACT_ANY is only to be
> > used when the corresponding _ANY flag is present.
> 
> d'oh, managed to confuse myself -- what I wrote is what the code
> currently does, but I think 'hard' should be *true* unless
> CPRNG_INIT_ANY is specified, i.e.
> 
> - r = cprng_entropy_try(key, sizeof(key), c->flags & CPRNG_INIT_ANY);
> + r = cprng_entropy_try(key, sizeof(key), !(c->flags & CPRNG_INIT_ANY));

I think that's right.  Someone else just brought this to my attention
as well.  I am embarrassed to note that several of us missed this bug
while considering the patch for the security issue.

However, in this case I think we are lucky and there is no real
security consequence of the inverted flag evaluation.  That is because
of two things:

1) The new code always retrieves the requested number of bits from the
   pool, whether "hard" is set or not.

2) The pool startup code has a separate notion of an initial entropy
   threshold, and immediately rekeys any stream generators that may
   have been keyed before that threshold was hit, as soon as it is hit.

The combined effect of those two facts and the backwards flag here is to
make our code behave like Darwin's, FreeBSD's, OpenSSL's, and OpenBSD's,
enforcing an "initial entropy" rule and then returning cryptographically
strong random numbers to all callers regardless of which device node
they might use to ask for them.  It's not what I intended, which was to
preserve the old behavior of letting callers ask for special,
"information-theoretic" random numbers via /dev/random (at this point, I
think only Linux still allows this), but it is certainly safe.  Unless
there is another bug in here that missed my fingers and the eyes of the
other reviewers.  I have looked the code over yet again and I do not
think there is.  Sometimes you screw up but you still get lucky, which
is, I believe, what happened here.

But I am travelling with limited net access and would dearly appreciate
additional eyes on this, so, please tell me if you think the above
analysis is not correct with regard to what the code actually does (I
do not, however, want to argue about the relative merits of
information-theoretic vs. cryptographic randomness right now).

I think the bug can still cause /dev/urandom reads to block, which may
confuse callers.  So it should be fixed.  Given that there does not seem
to be a real security impact, though, let's go slowly and carefully and
agree first that there is a bug, and exactly what it is, then fix it.  ;-)

Thor


Home | Main Index | Thread Index | Old Index