Subject: Re: bug#22202: 24.5; SECURITY ISSUE -- Emacs Server vulnerable to random number generator attack on Windows systems

From: Eli Zaretskii <[email-masked]>
Date: Mon, 18 Jan 2016 17:45:33 +0200

> From: Paul Eggert <[email-masked]>
> Cc: [email-masked], Richard Copley <[email-masked]>,
>  [email-masked], [email-masked]
> Date: Sun, 17 Jan 2016 12:26:31 -0800
> 
> Eli, thanks for improving the initial seed for (random t) in Emacs. I noticed that with this change, my Emacs was opening /dev/urandom twice, because GnuTLS does something similar during startup. Also, it was reading more data from /dev/urandom than it needed, due to stdio buffering. So I installed the attached patch, which defers to GnuTLS and falls back on doing things by hand (without stdio) only if GnuTLS is not available or fails. I assume this approach works under MS-Windows; if not please let me know and I'll try to fix it.

I wish we'd discuss such things before committing and not after.
What's the rush?  It's not like Emacs was broken or something.  It
just isn't worth it.

More to to the point, the more I think about this, the less I like
this change.  Here's why:

  . I see nothing wrong with having 2 (or more) independent reads from
    /dev/urandom:

    . GnuTLS is a separate library, so it's free to do that for its
      own purposes; we shouldn't care.  Besides, what if tomorrow
      there will be a 3rd library that would need to access
      /dev/urandom?

    . Reading from /dev/urandom never blocks, so doing this one more
      time during startup should be a non-issue.

  . Calling gnutls_rnd makes Emacs dependent on GnuTLS in ways that we
    don't necessarily want: GnuTLS is a library for TLS, not for
    cryptography.  What that function does on each platform is not
    documented anywhere in the GnuTLS manual that I could see; I
    needed to read the sources to find out.  What if tomorrow GnuTLS
    changes its implementation?  They are a separate project, they are
    not under any obligation to tell us.

  . This change means that we now load GnuTLS at startup, even if no
    TLS connections are or will be used.  Why should we unnecessarily
    bloat our memory footprint, even in minor ways?

  . It breaks the Windows build -- since GnuTLS is dynamically loaded
    on Windows, any GnuTLS function must be called through macros set
    up in gnutls.c, which sysdep.c knows nothing about.  (This is
    relatively simple to fix, but doing so requires adding yet more
    ugly #ifdef's to an already not-so-pretty little mess.)

So I tend to just say NO here.  Let's use the relatively simple code
we had before (with your I/O changes, I don't object to those, of
course), and leave GnuTLS to its own devices.  WDYT?

> Would you mind if I removed the newly-added details about current time and process ID from the documentation? The idea is that this is internal implementation detail that users should not rely on.

I didn't really add anything: the fact that we used time and PID was
spelled out in the function's doc string for the last 20-odd years.  I
just added that to the ELisp manual, to make it more consistent with
the doc string, and also because it's hard to tell that we are using
system entropy when available without saying anything about what we do
when it isn't.  (And IMO we should mention the system entropy because
that's an important feature users should know about.)

Why is it suddenly a concern that users will know that we use time and
PID as fallback?