
Critical PRNG Bug in NetBSD Kernel - tshtf
http://ftp.netbsd.org/pub/NetBSD/security/advisories/NetBSD-SA2013-003.txt.asc
======
yk

        Due to a misplaced parenthesis, if insufficient GOOD 
        bits were available to satisfy a request, the 
        keying/rekeying code requested either 32 or 64 ANY bits, 
        rather than the balance of bits required to key the 
        stream generator.
    

I think this paragraph is a nice reminder, how hard crypto can be.

~~~
caf
In particular CSPRNGs, because the output of a fatally flawed CSPRNG and a
secure CSPRNG can look very similar.

This is also why CSPRNGs are a great place to hide backdoors.

~~~
yk
Backdoor in the sense of leaking state?

And is there a case in the wild ( except the PS3 hack?)?

~~~
ctz
> except the PS3 hack?

To be clear, the PS3 problem was not a problem of randomness quality or a PRNG
backdoor. It was illegal nonce reuse in the zero-knowledge proof-of-key-
possession protocol embedded in DSA signatures.

~~~
wolf550e
Did you mean to use the word "illegal"?

~~~
ctz
Sorry 'illegal' in the sense of being contrary to the requirements of the
crypto protocol, in the same way that you can have the concept of an 'illegal
operation' in a CPU ISA.

------
kilovoltaire
I enjoyed the Thanks To section:

    
    
        Thor Lancelot Simon for causing, finding and fixing the bug

------
beering
The advisory says that reading from /dev/random is fine, but reading from
/dev/urandom is affected. Shouldn't cryptographic applications be using
/dev/random to begin with? I was under the impression that /dev/urandom is
only for when low-quality randomness is acceptable.

~~~
marshray
The only time that ought to make a difference in a modern properly-implemented
CSPRNG is right after system startup when very little unpredictability has
made it into the pool.

Consequently, system boot scripts are just about the worst possible place to
generate new keys if they didn't already exist.

~~~
gizmo686
From the manpage of /dev/random "If a seed file is saved across reboots as
recommended below (all major Linux distributions have done this since 2000 at
least), the output is cryptographically secure against attackers without local
root access as soon as it is reloaded in the boot sequence, and perfectly
adequate for network encryption session keys. Since reads from /dev/random may
block, users will usually want to open it in nonblocking mode (or perform a
read with timeout), and provide some sort of user notification if the desired
entropy is not immediately available."

~~~
marshray
Only if the seed file was was generated by a secure kernel during a clean
shutdown and kept secret during that whole time. There's a lot to go wrong
there, especially if you're an embedded system running on flash.

I also seem to recall host keys being generated on _first_ boot. Perhaps some
installers are smart enough to prime that seed file.

------
jimktrains2
> Due to a misplaced parenthesis...

I know this isn't the (main) take away message, but it does make me feel a
little better about some errors I've made in the past to know that even
heavily vetted code can have these types of errors.

~~~
CasimirCelerity
I don't know if this code would qualify as "heavily vetted." Thor Lancelot
Simon wrote it himself and imported it himself into the tree for NetBSD v6 and
onwards. Probably hasn't seen that many eyes for review.

~~~
jimktrains2
I just assumed kernel code would have at least someone else looking over it
before it's merged in.

~~~
sliverstorm
Maybe it does, but they won't be looking for misplaced parens.

------
jey
Anyone have a link to the actual patch?

~~~
Mithrandir
[http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/subr_cprng....](http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/subr_cprng.c.diff?r1=1.14&r2=1.15&only_with_tag=MAIN&f=h)

    
    
        Fix a security issue: when we are reseeding a PRNG seeded early in boot
        before we had ever had any entropy, if something else has consumed the
        entropy that triggered the immediate reseed, we can reseed with as little
        as sizeof(int) bytes of entropy.

~~~
jey
The misplaced right-paren is in this line:

    
    
        rnd_extract_data(key + r, sizeof(key - r), RND_EXTRACT_ANY);

------
vincie
Does vim have a plugin to detect unbalanced parens/brackets/braces etc?

~~~
ams6110
The parens were not unbalanced, that would very likely not even compile. They
were misplaced, a much more subtle mistake.

------
xkcdfanboy
Just in case anyone is curious, the error was having `sizeof(key - r)` instead
of `sizeof(key) - r`

Rookie mistake!

~~~
riffraff
I wonder, what sort of technology would have avoided this? The "key -r" bit
seems a bit suspicious type wise

~~~
marshray
Yeah, why would you care about the size of a temporary arithmetic expression?

~~~
ElectroPrime
Well... sizeof of an expression is mostly useful for not repeating yourself in
cases when you need the size of a type you already have some way of
referencing, as in

    
    
      a = malloc(sizeof(*a));
    

It's also useful for various hacks (stuff like compile-time assert macros),
since it doesn't evaluate the expression, only its type.

~~~
marshray
Sure, I can think of plenty of ways it could arise via metaprogramming. I
don't know if I would want such a warning enabled in my own template-heavy C++
code.

But it perhaps it could be useful for those following the NetBSD style. :-)

------
martinced
_"Due to a misplaced parenthesis..."_

Is this RNG written in Lisp!?

Sorry, couldn't resist ; ) (big fan of Clojure and elisp here btw)

~~~
marshray

         ERROR: LINE 3, COLUMN 26
         ERROR: UNMATCHED RIGHT PARENTHESIS ENCOUNTERED.
         ERROR: WHILE PROCESSING INPUT:
                  1         2         3         4         5         6         7
         123456789012345678901234567890123456789012345678901234567890123456789012
         Sorry, couldn't resist ; ) (big fan of Clojure and elisp here btw)
                                  ^
                                  |
         ERROR: PROCESSING WILL CONTINUE WITH THE NEXT AVAILABLE INPUT TOKEN.
         ERROR: ALL YOUR BASE ARE BELONG TO US.

