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

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

[Demetri Obenour]

1. Be logged into the same Windows computer as someone else.
2. Have a process running that is notified whenever a process starts up
3. Have them run `emacs --daemon' or invoke `server-start'.
4. Use the knowledge of the current time and the server's PID to guess
   the authentication key.
5. Connect to the other user's Emacs server.
6. Execute arbitrary code with the other user's privileges.

Emacs does not provide a cryptographically secure random number
generator that can be used on all platforms (including Windows).  On
Unix-like systems, one can use `/dev/urandom', but Windows requires a
Windows API call to obtain entropy, which is not accessable from Emacs.

Emacs should provide a CSPRNG on ALL platforms, and this should be used
for the secret in the Emacs server.  This is a blocker to the use of the
Emacs server on Windows.



In GNU Emacs 24.5.1 (x86_64-redhat-linux-gnu, GTK+ Version 3.16.6)
 of 2015-09-14 on buildvm-10.phx2.fedoraproject.org
Windowing system distributor `Fedora Project', version 11.0.11704000
System Description:	Fedora release 22 (Twenty Two)

Configured using:
 `configure --build=x86_64-redhat-linux-gnu
 --host=x86_64-redhat-linux-gnu --program-prefix=
 --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr
 --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc
 --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64
 --libexecdir=/usr/libexec --localstatedir=/var
 --sharedstatedir=/var/lib --mandir=/usr/share/man
 --infodir=/usr/share/info --with-dbus --with-gif --with-jpeg --with-png
 --with-rsvg --with-tiff --with-xft --with-xpm --with-x-toolkit=gtk3
 --with-gpm=no build_alias=x86_64-redhat-linux-gnu
 host_alias=x86_64-redhat-linux-gnu 'CFLAGS=-DMAIL_USE_LOCKF -O2 -g
 -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4
 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
 -m64 -mtune=generic' LDFLAGS=-Wl,-z,relro'

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Tar

Minor modes in effect:
  display-time-mode: t
  display-battery-mode: t
  global-linum-mode: t
  linum-mode: t
  global-semanticdb-minor-mode: t
  global-semantic-idle-scheduler-mode: t
  show-paren-mode: t
  semantic-mode: t
  global-auto-complete-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  global-visual-line-mode: t
  visual-line-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Type "q" to delete help window.
user-error: Beginning of history; no preceding item
Quit [2 times]
user-error: Beginning of history; no preceding item
Making completion list...
File emacs-24.4.tar.xz is large (37.9M), really open? (y or n) y
XZ uncompressing emacs-24.4.tar.xz...done
Parsing tar file...done
Making completion list...

Load-path shadows:
/home/dobenour/.emacs.d/elpa/ada-mode-5.1.8/ada-ref-man hides /home/dobenour/.emacs.d/elpa/ada-ref-man-2012.0/ada-ref-man
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/prv-emacs hides /usr/share/emacs/site-lisp/auctex/prv-emacs
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/context-en hides /usr/share/emacs/site-lisp/auctex/context-en
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-bar hides /usr/share/emacs/site-lisp/auctex/tex-bar
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/texmathp hides /usr/share/emacs/site-lisp/auctex/texmathp
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/toolbar-x hides /usr/share/emacs/site-lisp/auctex/toolbar-x
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-buf hides /usr/share/emacs/site-lisp/auctex/tex-buf
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/latex hides /usr/share/emacs/site-lisp/auctex/latex
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/context-nl hides /usr/share/emacs/site-lisp/auctex/context-nl
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-jp hides /usr/share/emacs/site-lisp/auctex/tex-jp
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-style hides /usr/share/emacs/site-lisp/auctex/tex-style
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/plain-tex hides /usr/share/emacs/site-lisp/auctex/plain-tex
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-fold hides /usr/share/emacs/site-lisp/auctex/tex-fold
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/context hides /usr/share/emacs/site-lisp/auctex/context
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-mik hides /usr/share/emacs/site-lisp/auctex/tex-mik
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/font-latex hides /usr/share/emacs/site-lisp/auctex/font-latex
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/preview hides /usr/share/emacs/site-lisp/auctex/preview
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex hides /usr/share/emacs/site-lisp/auctex/tex
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-font hides /usr/share/emacs/site-lisp/auctex/tex-font
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-info hides /usr/share/emacs/site-lisp/auctex/tex-info
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/multi-prompt hides /usr/share/emacs/site-lisp/auctex/multi-prompt
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/bib-cite hides /usr/share/emacs/site-lisp/auctex/bib-cite
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/tex-site hides /usr/share/emacs/site-lisp/tex-site
/home/dobenour/.emacs.d/elpa/auctex-11.88.6/auctex hides /usr/share/emacs/site-lisp/site-start.d/auctex
/usr/share/emacs/site-lisp/site-start.d/slime-autoloads hides /usr/share/emacs/site-lisp/slime/slime-autoloads
/usr/share/emacs/site-lisp/site-start.d/slime hides /usr/share/emacs/site-lisp/slime/slime
/usr/share/emacs/site-lisp/site-start.d/hyperspec hides /usr/share/emacs/site-lisp/slime/hyperspec
/usr/share/emacs/site-lisp/site-start.d/maxima-modes hides /usr/share/emacs/site-lisp/maxima/site_start.d/maxima-modes
/home/dobenour/.emacs.d/elpa/ada-mode-5.1.8/ada-mode hides /usr/share/emacs/24.5/lisp/progmodes/ada-mode
/home/dobenour/.emacs.d/elpa/ada-mode-5.1.8/ada-xref hides /usr/share/emacs/24.5/lisp/progmodes/ada-xref
/home/dobenour/.emacs.d/elpa/ada-mode-5.1.8/ada-prj hides /usr/share/emacs/24.5/lisp/progmodes/ada-prj
/home/dobenour/.emacs.d/elpa/ada-mode-5.1.8/ada-stmt hides /usr/share/emacs/24.5/lisp/progmodes/ada-stmt
/home/dobenour/.emacs.d/elpa/org-20150608/ob-exp hides /usr/share/emacs/24.5/lisp/org/ob-exp
/home/dobenour/.emacs.d/elpa/org-20150608/ob-emacs-lisp hides /usr/share/emacs/24.5/lisp/org/ob-emacs-lisp
/home/dobenour/.emacs.d/elpa/org-20150608/ob-js hides /usr/share/emacs/24.5/lisp/org/ob-js
/home/dobenour/.emacs.d/elpa/org-20150608/org-loaddefs hides /usr/share/emacs/24.5/lisp/org/org-loaddefs
/home/dobenour/.emacs.d/elpa/org-20150608/ob-io hides /usr/share/emacs/24.5/lisp/org/ob-io
/home/dobenour/.emacs.d/elpa/org-20150608/org-gnus hides /usr/share/emacs/24.5/lisp/org/org-gnus
/home/dobenour/.emacs.d/elpa/org-20150608/ob-screen hides /usr/share/emacs/24.5/lisp/org/ob-screen
/home/dobenour/.emacs.d/elpa/org-20150608/ob-octave hides /usr/share/emacs/24.5/lisp/org/ob-octave
/home/dobenour/.emacs.d/elpa/org-20150608/org-docview hides /usr/share/emacs/24.5/lisp/org/org-docview
/home/dobenour/.emacs.d/elpa/org-20150608/org-faces hides /usr/share/emacs/24.5/lisp/org/org-faces
/home/dobenour/.emacs.d/elpa/org-20150608/ox-latex hides /usr/share/emacs/24.5/lisp/org/ox-latex
/home/dobenour/.emacs.d/elpa/org-20150608/ox-org hides /usr/share/emacs/24.5/lisp/org/ox-org
/home/dobenour/.emacs.d/elpa/org-20150608/org hides /usr/share/emacs/24.5/lisp/org/org
/home/dobenour/.emacs.d/elpa/org-20150608/ox-texinfo hides /usr/share/emacs/24.5/lisp/org/ox-texinfo
/home/dobenour/.emacs.d/elpa/org-20150608/ob hides /usr/share/emacs/24.5/lisp/org/ob
/home/dobenour/.emacs.d/elpa/org-20150608/ob-haskell hides /usr/share/emacs/24.5/lisp/org/ob-haskell
/home/dobenour/.emacs.d/elpa/org-20150608/ob-comint hides /usr/share/emacs/24.5/lisp/org/ob-comint
/home/dobenour/.emacs.d/elpa/org-20150608/org-crypt hides /usr/share/emacs/24.5/lisp/org/org-crypt
/home/dobenour/.emacs.d/elpa/org-20150608/org-bbdb hides /usr/share/emacs/24.5/lisp/org/org-bbdb
/home/dobenour/.emacs.d/elpa/org-20150608/org-colview hides /usr/share/emacs/24.5/lisp/org/org-colview
/home/dobenour/.emacs.d/elpa/org-20150608/ob-sqlite hides /usr/share/emacs/24.5/lisp/org/ob-sqlite
/home/dobenour/.emacs.d/elpa/org-20150608/ob-tangle hides /usr/share/emacs/24.5/lisp/org/ob-tangle
/home/dobenour/.emacs.d/elpa/org-20150608/org-protocol hides /usr/share/emacs/24.5/lisp/org/org-protocol
/home/dobenour/.emacs.d/elpa/org-20150608/org-entities hides /usr/share/emacs/24.5/lisp/org/org-entities
/home/dobenour/.emacs.d/elpa/org-20150608/ob-sql hides /usr/share/emacs/24.5/lisp/org/ob-sql
/home/dobenour/.emacs.d/elpa/org-20150608/ob-java hides /usr/share/emacs/24.5/lisp/org/ob-java
/home/dobenour/.emacs.d/elpa/org-20150608/ob-perl hides /usr/share/emacs/24.5/lisp/org/ob-perl
/home/dobenour/.emacs.d/elpa/org-20150608/ob-lisp hides /usr/share/emacs/24.5/lisp/org/ob-lisp
/home/dobenour/.emacs.d/elpa/org-20150608/org-capture hides /usr/share/emacs/24.5/lisp/org/org-capture
/home/dobenour/.emacs.d/elpa/org-20150608/org-list hides /usr/share/emacs/24.5/lisp/org/org-list
/home/dobenour/.emacs.d/elpa/org-20150608/ob-core hides /usr/share/emacs/24.5/lisp/org/ob-core
/home/dobenour/.emacs.d/elpa/org-20150608/ob-picolisp hides /usr/share/emacs/24.5/lisp/org/ob-picolisp
/home/dobenour/.emacs.d/elpa/org-20150608/ob-ledger hides /usr/share/emacs/24.5/lisp/org/ob-ledger
/home/dobenour/.emacs.d/elpa/org-20150608/ob-R hides /usr/share/emacs/24.5/lisp/org/ob-R
/home/dobenour/.emacs.d/elpa/org-20150608/org-mhe hides /usr/share/emacs/24.5/lisp/org/org-mhe
/home/dobenour/.emacs.d/elpa/org-20150608/ob-sh hides /usr/share/emacs/24.5/lisp/org/ob-sh
/home/dobenour/.emacs.d/elpa/org-20150608/org-mobile hides /usr/share/emacs/24.5/lisp/org/org-mobile
/home/dobenour/.emacs.d/elpa/org-20150608/org-mouse hides /usr/share/emacs/24.5/lisp/org/org-mouse
/home/dobenour/.emacs.d/elpa/org-20150608/org-timer hides /usr/share/emacs/24.5/lisp/org/org-timer
/home/dobenour/.emacs.d/elpa/org-20150608/ob-sass hides /usr/share/emacs/24.5/lisp/org/ob-sass
/home/dobenour/.emacs.d/elpa/org-20150608/org-irc hides /usr/share/emacs/24.5/lisp/org/org-irc
/home/dobenour/.emacs.d/elpa/org-20150608/org-info hides /usr/share/emacs/24.5/lisp/org/org-info
/home/dobenour/.emacs.d/elpa/org-20150608/org-w3m hides /usr/share/emacs/24.5/lisp/org/org-w3m
/home/dobenour/.emacs.d/elpa/org-20150608/ob-scheme hides /usr/share/emacs/24.5/lisp/org/ob-scheme
/home/dobenour/.emacs.d/elpa/org-20150608/ox-md hides /usr/share/emacs/24.5/lisp/org/ox-md
/home/dobenour/.emacs.d/elpa/org-20150608/org-eshell hides /usr/share/emacs/24.5/lisp/org/org-eshell
/home/dobenour/.emacs.d/elpa/org-20150608/org-datetree hides /usr/share/emacs/24.5/lisp/org/org-datetree
/home/dobenour/.emacs.d/elpa/org-20150608/org-attach hides /usr/share/emacs/24.5/lisp/org/org-attach
/home/dobenour/.emacs.d/elpa/org-20150608/ob-org hides /usr/share/emacs/24.5/lisp/org/ob-org
/home/dobenour/.emacs.d/elpa/org-20150608/org-bibtex hides /usr/share/emacs/24.5/lisp/org/org-bibtex
/home/dobenour/.emacs.d/elpa/org-20150608/ox-publish hides /usr/share/emacs/24.5/lisp/org/ox-publish
/home/dobenour/.emacs.d/elpa/org-20150608/ob-matlab hides /usr/share/emacs/24.5/lisp/org/ob-matlab
/home/dobenour/.emacs.d/elpa/org-20150608/ob-ruby hides /usr/share/emacs/24.5/lisp/org/ob-ruby
/home/dobenour/.emacs.d/elpa/org-20150608/org-rmail hides /usr/share/emacs/24.5/lisp/org/org-rmail
/home/dobenour/.emacs.d/elpa/org-20150608/org-ctags hides /usr/share/emacs/24.5/lisp/org/org-ctags
/home/dobenour/.emacs.d/elpa/org-20150608/org-element hides /usr/share/emacs/24.5/lisp/org/org-element
/home/dobenour/.emacs.d/elpa/org-20150608/ob-python hides /usr/share/emacs/24.5/lisp/org/ob-python
/home/dobenour/.emacs.d/elpa/org-20150608/org-footnote hides /usr/share/emacs/24.5/lisp/org/org-footnote
/home/dobenour/.emacs.d/elpa/org-20150608/ob-mscgen hides /usr/share/emacs/24.5/lisp/org/ob-mscgen
/home/dobenour/.emacs.d/elpa/org-20150608/org-inlinetask hides /usr/share/emacs/24.5/lisp/org/org-inlinetask
/home/dobenour/.emacs.d/elpa/org-20150608/ob-plantuml hides /usr/share/emacs/24.5/lisp/org/ob-plantuml
/home/dobenour/.emacs.d/elpa/org-20150608/ob-latex hides /usr/share/emacs/24.5/lisp/org/ob-latex
/home/dobenour/.emacs.d/elpa/org-20150608/ox-man hides /usr/share/emacs/24.5/lisp/org/ox-man
/home/dobenour/.emacs.d/elpa/org-20150608/org-habit hides /usr/share/emacs/24.5/lisp/org/org-habit
/home/dobenour/.emacs.d/elpa/org-20150608/org-clock hides /usr/share/emacs/24.5/lisp/org/org-clock
/home/dobenour/.emacs.d/elpa/org-20150608/ob-asymptote hides /usr/share/emacs/24.5/lisp/org/ob-asymptote
/home/dobenour/.emacs.d/elpa/org-20150608/org-macro hides /usr/share/emacs/24.5/lisp/org/org-macro
/home/dobenour/.emacs.d/elpa/org-20150608/ob-scala hides /usr/share/emacs/24.5/lisp/org/ob-scala
/home/dobenour/.emacs.d/elpa/org-20150608/org-install hides /usr/share/emacs/24.5/lisp/org/org-install
/home/dobenour/.emacs.d/elpa/org-20150608/ox-html hides /usr/share/emacs/24.5/lisp/org/ox-html
/home/dobenour/.emacs.d/elpa/org-20150608/org-compat hides /usr/share/emacs/24.5/lisp/org/org-compat
/home/dobenour/.emacs.d/elpa/org-20150608/ox-beamer hides /usr/share/emacs/24.5/lisp/org/ox-beamer
/home/dobenour/.emacs.d/elpa/org-20150608/org-feed hides /usr/share/emacs/24.5/lisp/org/org-feed
/home/dobenour/.emacs.d/elpa/org-20150608/org-id hides /usr/share/emacs/24.5/lisp/org/org-id
/home/dobenour/.emacs.d/elpa/org-20150608/ob-lilypond hides /usr/share/emacs/24.5/lisp/org/ob-lilypond
/home/dobenour/.emacs.d/elpa/org-20150608/org-src hides /usr/share/emacs/24.5/lisp/org/org-src
/home/dobenour/.emacs.d/elpa/org-20150608/org-macs hides /usr/share/emacs/24.5/lisp/org/org-macs
/home/dobenour/.emacs.d/elpa/org-20150608/ob-clojure hides /usr/share/emacs/24.5/lisp/org/ob-clojure
/home/dobenour/.emacs.d/elpa/org-20150608/ob-maxima hides /usr/share/emacs/24.5/lisp/org/ob-maxima
/home/dobenour/.emacs.d/elpa/org-20150608/ob-css hides /usr/share/emacs/24.5/lisp/org/ob-css
/home/dobenour/.emacs.d/elpa/org-20150608/org-plot hides /usr/share/emacs/24.5/lisp/org/org-plot
/home/dobenour/.emacs.d/elpa/org-20150608/org-indent hides /usr/share/emacs/24.5/lisp/org/org-indent
/home/dobenour/.emacs.d/elpa/org-20150608/org-archive hides /usr/share/emacs/24.5/lisp/org/org-archive
/home/dobenour/.emacs.d/elpa/org-20150608/org-pcomplete hides /usr/share/emacs/24.5/lisp/org/org-pcomplete
/home/dobenour/.emacs.d/elpa/org-20150608/ob-makefile hides /usr/share/emacs/24.5/lisp/org/ob-makefile
/home/dobenour/.emacs.d/elpa/org-20150608/ox-icalendar hides /usr/share/emacs/24.5/lisp/org/ox-icalendar
/home/dobenour/.emacs.d/elpa/org-20150608/org-agenda hides /usr/share/emacs/24.5/lisp/org/org-agenda
/home/dobenour/.emacs.d/elpa/org-20150608/ob-table hides /usr/share/emacs/24.5/lisp/org/ob-table
/home/dobenour/.emacs.d/elpa/org-20150608/ob-eval hides /usr/share/emacs/24.5/lisp/org/ob-eval
/home/dobenour/.emacs.d/elpa/org-20150608/ox hides /usr/share/emacs/24.5/lisp/org/ox
/home/dobenour/.emacs.d/elpa/org-20150608/ob-awk hides /usr/share/emacs/24.5/lisp/org/ob-awk
/home/dobenour/.emacs.d/elpa/org-20150608/ox-odt hides /usr/share/emacs/24.5/lisp/org/ox-odt
/home/dobenour/.emacs.d/elpa/org-20150608/ob-lob hides /usr/share/emacs/24.5/lisp/org/ob-lob
/home/dobenour/.emacs.d/elpa/org-20150608/ox-ascii hides /usr/share/emacs/24.5/lisp/org/ox-ascii
/home/dobenour/.emacs.d/elpa/org-20150608/org-table hides /usr/share/emacs/24.5/lisp/org/org-table
/home/dobenour/.emacs.d/elpa/org-20150608/ob-shen hides /usr/share/emacs/24.5/lisp/org/ob-shen
/home/dobenour/.emacs.d/elpa/org-20150608/ob-keys hides /usr/share/emacs/24.5/lisp/org/ob-keys
/home/dobenour/.emacs.d/elpa/org-20150608/ob-calc hides /usr/share/emacs/24.5/lisp/org/ob-calc
/home/dobenour/.emacs.d/elpa/org-20150608/ob-ocaml hides /usr/share/emacs/24.5/lisp/org/ob-ocaml
/home/dobenour/.emacs.d/elpa/org-20150608/org-version hides /usr/share/emacs/24.5/lisp/org/org-version
/home/dobenour/.emacs.d/elpa/org-20150608/ob-C hides /usr/share/emacs/24.5/lisp/org/ob-C
/home/dobenour/.emacs.d/elpa/org-20150608/ob-ditaa hides /usr/share/emacs/24.5/lisp/org/ob-ditaa
/home/dobenour/.emacs.d/elpa/org-20150608/ob-ref hides /usr/share/emacs/24.5/lisp/org/ob-ref
/home/dobenour/.emacs.d/elpa/org-20150608/ob-fortran hides /usr/share/emacs/24.5/lisp/org/ob-fortran
/home/dobenour/.emacs.d/elpa/org-20150608/ob-dot hides /usr/share/emacs/24.5/lisp/org/ob-dot
/home/dobenour/.emacs.d/elpa/org-20150608/ob-gnuplot hides /usr/share/emacs/24.5/lisp/org/ob-gnuplot

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils tar-mode jka-compr eieio-opt speedbar sb-image
dframe help-mode info package julia-mode ert find-func ewoc debug rx tls
server idris-autoloads toml-mode-init rust-mode-autoloads time battery
linum semantic/db-mode semantic/db eieio-base semantic/idle
semantic/format ezimage semantic/tag-ls semantic/find semantic/ctxt
saveplace paren semantic/util-modes semantic/util semantic semantic/tag
semantic/lex semantic/fw eieio eieio-core mode-local cedet cus-start
cus-load vc vc-dispatcher advice u-vm-color vm-autoloads vm-vars
vm-version slime warnings byte-opt bytecomp byte-compile cconv derived
cl-extra help-fns easy-mmode easymenu pp comint ansi-color ring
slime-autoloads preview-latex proof-site proof-autoloads pg-vars
hyperspec cl-macs thingatpt browse-url cl gv bbdb-loaddefs
auto-complete-config auto-complete edmacro kmacro cl-loaddefs cl-lib
popup tex-site auto-loads agda2 time-date tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 250685 11999)
 (symbols 48 31904 0)
 (miscs 40 5369 181)
 (strings 32 64890 8146)
 (string-bytes 1 1659171)
 (vectors 16 23573)
 (vector-slots 8 581629 6882)
 (floats 8 129 250)
 (intervals 56 10649 30)
 (buffers 960 16)
 (heap 1024 52593 2624))

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

[Eli Zaretskii]

> From: Demetri Obenour <demetriobenour@gmail.com>
> Date: Fri, 18 Dec 2015 05:05:09 -0500
> 
> 
> 1. Be logged into the same Windows computer as someone else.
> 2. Have a process running that is notified whenever a process starts up
> 3. Have them run `emacs --daemon' or invoke `server-start'.
> 4. Use the knowledge of the current time and the server's PID to guess
>    the authentication key.
> 5. Connect to the other user's Emacs server.
> 6. Execute arbitrary code with the other user's privileges.

Please provide the necessary details for reproducing this problem and
verifying the solution.  What I'm missing:

> 1. Be logged into the same Windows computer as someone else.

How do you do that?  I understand you are describing a situation where
2 users are logged into the same Windows system simultaneously using
the same credentials, is that true?  If so, how to create such a
situation?

> 2. Have a process running that is notified whenever a process starts up
> 3. Have them run `emacs --daemon' or invoke `server-start'.
> 4. Use the knowledge of the current time and the server's PID to guess
>    the authentication key.

I don't think we use the current time and PID for that, but even if we
do, how do you get a hold of the time at the moment of the server
creation to nanosecond resolution?  Please tell how to do that.

Thanks.

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

[Richard Copley]

> Please provide the necessary details for reproducing this problem and
> verifying the solution.  What I'm missing:
>
> > 1. Be logged into the same Windows computer as someone else.
>
> How do you do that?  I understand you are describing a situation where
> 2 users are logged into the same Windows system simultaneously using
> the same credentials, is that true?  If so, how to create such a
> situation?

I don't think that is possible; however, two /different/ accounts can
be logged in to a computer at the same time, via Remote Desktop or
Fast User Switching. (If the computer is a Remote Desktop server then
two users can be simultaneously interacting with their desktops, in
separate sessions. That's not at all uncommon in a business
environment, but I don't think it's relevant to this question.)

> > 2. Have a process running that is notified whenever a process starts up
> > 3. Have them run `emacs --daemon' or invoke `server-start'.
> > 4. Use the knowledge of the current time and the server's PID to guess
> >    the authentication key.
>
> I don't think we use the current time and PID for that, but even if we
> do, how do you get a hold of the time at the moment of the server
> creation to nanosecond resolution?  Please tell how to do that.

We use function "random" (see function "server-generate-key"); its
seed is typically set at startup using the current time and PID (see
"init_random()" in sysdep.c), so it's the time Emacs started that you
would want to know, not the time the server started. You can get the
start time (to the nearest second at least) and PID of any user's
processes using, e.g., Process Explorer.

I'm not sure what resolution timestamp we end up using as the seed.
gettime() might return microsecond timestamps in certain configurations.

I can't speak for Demetri but it seems to me he's imagining an attacker
who is prepared to use a certain amount of brute force. Knowing or
guessing the Emacs start time within a few seconds would reduce the
search space.

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

[Eli Zaretskii]

> Date: Tue, 29 Dec 2015 15:36:12 +0000
> From: Richard Copley <rcopley@gmail.com>
> 
> > Please provide the necessary details for reproducing this problem and
> > verifying the solution.  What I'm missing:
> >
> > > 1. Be logged into the same Windows computer as someone else.
> >
> > How do you do that?  I understand you are describing a situation where
> > 2 users are logged into the same Windows system simultaneously using
> > the same credentials, is that true?  If so, how to create such a
> > situation?
> 
> I don't think that is possible; however, two /different/ accounts can
> be logged in to a computer at the same time, via Remote Desktop or
> Fast User Switching.

Logging in via Remote Desktop usurps the system, AFAIK.  So these
possibilities are not relevant to the issue at hand.

> > > 2. Have a process running that is notified whenever a process starts up
> > > 3. Have them run `emacs --daemon' or invoke `server-start'.
> > > 4. Use the knowledge of the current time and the server's PID to guess
> > >    the authentication key.
> >
> > I don't think we use the current time and PID for that, but even if we
> > do, how do you get a hold of the time at the moment of the server
> > creation to nanosecond resolution?  Please tell how to do that.
> 
> We use function "random" (see function "server-generate-key"); its
> seed is typically set at startup using the current time and PID (see
> "init_random()" in sysdep.c), so it's the time Emacs started that you
> would want to know, not the time the server started. You can get the
> start time (to the nearest second at least) and PID of any user's
> processes using, e.g., Process Explorer.

You need the time to nanosecond resolution to compute the seed.  How
do you do that?

> I'm not sure what resolution timestamp we end up using as the seed.
> gettime() might return microsecond timestamps in certain configurations.

On MS-Windows, gettime calls gettimeofday, which returns the system
clock in 100 nanosecond units.  The actual resolution of the clock is
between 1 ms and 10 ms, but I think it's still an impossible task to
get the exact time we sample the clock during startup with such a high
accuracy.

> I can't speak for Demetri but it seems to me he's imagining an attacker
> who is prepared to use a certain amount of brute force. Knowing or
> guessing the Emacs start time within a few seconds would reduce the
> search space.

As I said, I don't see how such a user could even get access to a
machine without my paying attention.  And that if the services
required for remote access have not been turned off to begin with.

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

[Richard Copley]

On 29 December 2015 at 16:21, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 29 Dec 2015 15:36:12 +0000
>> From: Richard Copley <rcopley@gmail.com>
>>
>> > Please provide the necessary details for reproducing this problem and
>> > verifying the solution.  What I'm missing:
>> >
>> > > 1. Be logged into the same Windows computer as someone else.
>> >
>> > How do you do that?  I understand you are describing a situation where
>> > 2 users are logged into the same Windows system simultaneously using
>> > the same credentials, is that true?  If so, how to create such a
>> > situation?
>>
>> I don't think that is possible; however, two /different/ accounts can
>> be logged in to a computer at the same time, via Remote Desktop or
>> Fast User Switching.
>
> Logging in via Remote Desktop usurps the system, AFAIK.  So these
> possibilities are not relevant to the issue at hand.

That is definitely not correct. In some configurations several users
can connect via remote desktop. I do this every day. It /might/ be
necessary to have a "Professional" and/or Server edition of Windows.
A licensed Terminal Server supports dozens of sessions at once.

Fast User Switching is a different thing. (Type CTRL-ALT-DEL and click
"Switch User".) That, too, might require "Professional".

>> > > 2. Have a process running that is notified whenever a process starts up
>> > > 3. Have them run `emacs --daemon' or invoke `server-start'.
>> > > 4. Use the knowledge of the current time and the server's PID to guess
>> > >    the authentication key.
>> >
>> > I don't think we use the current time and PID for that, but even if we
>> > do, how do you get a hold of the time at the moment of the server
>> > creation to nanosecond resolution?  Please tell how to do that.
>>
>> We use function "random" (see function "server-generate-key"); its
>> seed is typically set at startup using the current time and PID (see
>> "init_random()" in sysdep.c), so it's the time Emacs started that you
>> would want to know, not the time the server started. You can get the
>> start time (to the nearest second at least) and PID of any user's
>> processes using, e.g., Process Explorer.
>
> You need the time to nanosecond resolution to compute the seed.  How
> do you do that?

I haven't tried, but the MSDN docs for GetProcessTimes say it returns the
start time in 100 ns units. I'd guess that's what Process Explorer uses.

>> I'm not sure what resolution timestamp we end up using as the seed.
>> gettime() might return microsecond timestamps in certain configurations.
>
> On MS-Windows, gettime calls gettimeofday, which returns the system
> clock in 100 nanosecond units.  The actual resolution of the clock is
> between 1 ms and 10 ms, but I think it's still an impossible task to
> get the exact time we sample the clock during startup with such a high
> accuracy.

Perhaps you don't need to. Brute force. (Maybe that's ridiculous. I haven't
tried to do the sums. Trying 100 to 1000 different values doesn't sound too
hard.)

>> I can't speak for Demetri but it seems to me he's imagining an attacker
>> who is prepared to use a certain amount of brute force. Knowing or
>> guessing the Emacs start time within a few seconds would reduce the
>> search space.
>
> As I said, I don't see how such a user could even get access to a
> machine without my paying attention.

With respect, that's not correct (explained above).

> And that if the services
> required for remote access have not been turned off to begin with.

Yes obviously, but many organizations do have Remote Desktop
servers their staff can (or must) connect to.

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

[David Engster]

Richard Copley writes:
> On 29 December 2015 at 16:21, Eli Zaretskii <eliz@gnu.org> wrote:
>>> Date: Tue, 29 Dec 2015 15:36:12 +0000
>>> From: Richard Copley <rcopley@gmail.com>
>>>
>
>>> > Please provide the necessary details for reproducing this problem and
>>> > verifying the solution.  What I'm missing:
>>> >
>>> > > 1. Be logged into the same Windows computer as someone else.
>>> >
>>> > How do you do that?  I understand you are describing a situation where
>>> > 2 users are logged into the same Windows system simultaneously using
>>> > the same credentials, is that true?  If so, how to create such a
>>> > situation?
>>>
>>> I don't think that is possible; however, two /different/ accounts can
>>> be logged in to a computer at the same time, via Remote Desktop or
>>> Fast User Switching.
>>
>> Logging in via Remote Desktop usurps the system, AFAIK.  So these
>> possibilities are not relevant to the issue at hand.
>
> That is definitely not correct. In some configurations several users
> can connect via remote desktop. I do this every day. It /might/ be
> necessary to have a "Professional" and/or Server edition of Windows.
> A licensed Terminal Server supports dozens of sessions at once.

That's correct (it requires a Windows Server with enabled terminal
services), but each user session has of course its own process space, so
I don't see how the described attack could work there.

-David

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

[Richard Copley]

>> [...]
>
> That's correct (it requires a Windows Server with enabled terminal
> services), but each user session has of course its own process space, so
> I don't see how the described attack could work there.

Not sure what you mean by process space. As an unprivileged user
you can find other users' Emacs processes without any effort (using
tasklist.exe, for example). If you know on what port an Emacs server
is listening (which is admittedly a difficulty), you can send bytes to it.
I've just done so as an experiment. (I was driving both sessions so I
knew the server port.)

I haven't reproduced the whole attack scenario and I don't pretend
know whether it could work. I don't claim any expertise in software
security. I just wanted to help out by answering Eli's questions.

To get back to the OP's main point, given that we already go to the
trouble of creating this secret, it wouldn't hurt to do it better (on all
systems, for preference). On Windows it really doesn't seem hard.
Sorry, no patch, for legal reasons, but there's a simple example on
the MSDN page for CryptGenRandom.

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

[David Engster]

Richard Copley writes:
>>> [...]
>>
>> That's correct (it requires a Windows Server with enabled terminal
>> services), but each user session has of course its own process space, so
>> I don't see how the described attack could work there.
>
> Not sure what you mean by process space. As an unprivileged user
> you can find other users' Emacs processes without any effort (using
> tasklist.exe, for example). If you know on what port an Emacs server
> is listening (which is admittedly a difficulty), you can send bytes to it.
> I've just done so as an experiment. (I was driving both sessions so I
> knew the server port.)

You logged in with two different user accounts? I always thought
sessions from different users were better isolated from one another and
more similar to Linux containers. If that is not the case, then I agree
the attack scenario looks feasible.

-David

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

[Richard Copley]

> You logged in with two different user accounts?
Yes. I didn't know you could log in two sessions as the same user.
Apparently you can (serverfault.com/questions/116016).

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

[Eli Zaretskii]

> Date: Tue, 29 Dec 2015 21:22:55 +0000
> From: Richard Copley <rcopley@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 22202@debbugs.gnu.org, 
> 	Demetri Obenour <demetriobenour@gmail.com>
> 
> I haven't reproduced the whole attack scenario and I don't pretend
> know whether it could work. I don't claim any expertise in software
> security. I just wanted to help out by answering Eli's questions.
> 
> To get back to the OP's main point, given that we already go to the
> trouble of creating this secret, it wouldn't hurt to do it better (on all
> systems, for preference). On Windows it really doesn't seem hard.
> Sorry, no patch, for legal reasons, but there's a simple example on
> the MSDN page for CryptGenRandom.

Can you audit the patch below?  I know next to nothing about
cryptography, and I'm not sure I understood all the flags involved in
these APIs.

Also, generating random numbers with these APIs is significantly
slower -- about 2.5 msec per number, as opposed to something like
175 microsec using 'rand'.  Should we perhaps provide an option
(by default off) to force using the old, weaker, but faster method?

Thanks.

--- src/w32.c~0	2015-11-29 06:48:07.000000000 +0200
+++ src/w32.c	2015-12-30 17:48:19.297251800 +0200
@@ -224,6 +224,8 @@ typedef struct _REPARSE_DATA_BUFFER {
 
 #include <iphlpapi.h>	/* should be after winsock2.h */
 
+#include <wincrypt.h>
+
 #include <c-strcase.h>
 
 #include "w32.h"
@@ -2093,9 +2095,34 @@ init_user_info (void)
     CloseHandle (token);
 }
 
+static HCRYPTPROV w32_crypto_hprov;
+static int
+w32_init_crypt_random (void)
+{
+  if (!CryptAcquireContext (&w32_crypto_hprov, NULL, NULL, PROV_RSA_FULL,
+			    CRYPT_VERIFYCONTEXT | CRYPT_SILENT))
+    {
+      DebPrint (("CryptAcquireContext failed with error %x\n",
+		 GetLastError ()));
+      return -1;
+    }
+  return 0;
+}
+
 int
 random (void)
 {
+  if (w32_crypto_hprov)
+    w32_init_crypt_random ();
+  if (w32_crypto_hprov)
+    {
+      const DWORD nbytes = 4;	/* see RAND_BITS in sysdep.c */
+      BYTE rand_buffer[nbytes];
+
+      if (CryptGenRandom (w32_crypto_hprov, nbytes, rand_buffer))
+	return *(int *)&rand_buffer[0];
+    }
+  /* Else fall back on rand ()  */
   /* rand () on NT gives us 15 random bits...hack together 30 bits.  */
   return ((rand () << 15) | rand ());
 }
@@ -2103,6 +2130,18 @@ random (void)
 void
 srandom (int seed)
 {
+  if (!w32_crypto_hprov)
+    w32_init_crypt_random ();
+  if (w32_crypto_hprov)
+    {
+      const DWORD nbytes = 4;	/* see RAND_BITS in sysdep.c */
+      BYTE buf[nbytes];
+
+      memcpy (buf, &seed, sizeof buf);
+      CryptGenRandom (w32_crypto_hprov, nbytes, buf);
+    }
+  /* Always seed rand () as well, in case some future call to
+     CryptGenRandom fails and we need to fall back to rand () */
   srand (seed);
 }
 
@@ -9386,6 +9425,8 @@ globals_of_w32 (void)
   extern void dynlib_reset_last_error (void);
   dynlib_reset_last_error ();
 #endif
+
+  w32_crypto_hprov = (HCRYPTPROV)0;
 }
 
 /* For make-serial-process  */

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

[Richard Copley]

> Can you audit the patch below?  I know next to nothing about
> cryptography, and I'm not sure I understood all the flags involved in
> these APIs.

Sure! But please bear in mind I'm not experienced in crypto
either.

With regard to API usage: The call to CryptAcquireContext looks good
to me. (The comment about interoperability in the documentation for
the parameter "pszProvider" does not apply as we are not inter-
operating with anything. Setting "pszContainer" to NULL, as you have
done, is explicitly recommended. The docs for the individual flags
entail the very value of "dwFlags" that you use.) I can see nothing
else to comment on.

Re performance: using CryptGenRandom to provide a seed for srand
is enough to address Demetri's concern. For performance reasons,
as you said, implementing random() with CryptGenRandom is
potentially bad. I think random() itself should not be changed.

That said, rand() makes me uncomfortable (mostly because of bugs in
long-gone implementations, and superstition). Given the chance I would
replace it with an xorshift* generator. The generator at [1] seeded
with 64 bits from CryptGenRandom should give good performance for
random() and (I guess!) an effectively unassailable server secret.

But I have no good reason to claim that rand() is not good enough.

Thank you Eli.

[1]: https://en.wikipedia.org/w/index.php?title=Xorshift&oldid=697235156#xorshift.2A

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

[Richard Copley]

Ah, I forgot to mention one other thing that had occurred to me.
It might not be a good idea to pass the current time to CryptGenRandom
for the optional initial seed. The current time (in various forms) is
already used as seed entropy by the system, and it's conceivable
(though implausible) that we could be destroying entropy by doing
this. It's probably better (and "acceptable" according to the
documentation) not to pass any seed at all.

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

[Eli Zaretskii]

> Date: Wed, 30 Dec 2015 20:47:40 +0000
> From: Richard Copley <rcopley@gmail.com>
> Cc: David Engster <deng@randomsample.de>, 22202@debbugs.gnu.org, 
> 	Demetri Obenour <demetriobenour@gmail.com>
> 
> Re performance: using CryptGenRandom to provide a seed for srand
> is enough to address Demetri's concern. For performance reasons,
> as you said, implementing random() with CryptGenRandom is
> potentially bad. I think random() itself should not be changed.

That'd be the worst of both worlds, IMO: a not-so-good PRNG with no
way whatsoever to get a repeatable sequence.  Am I right?

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

[Richard Copley]

> That'd be the worst of both worlds, IMO: a not-so-good PRNG with no
> way whatsoever to get a repeatable sequence.  Am I right?

Oh dear. Yes. Worse than that, repeatability is part of the contract for
the lisp "random" function, so seed_random() and get_random() are
constrained to be deterministic. Right?

Seems as though init_random() is the only place that could use
CryptGenRandom, which is a pity if you're trying to confine the
changes to w32.c.

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

[Eli Zaretskii]

> Date: Wed, 30 Dec 2015 21:15:13 +0000
> From: Richard Copley <rcopley@gmail.com>
> Cc: David Engster <deng@randomsample.de>, 22202@debbugs.gnu.org, 
> 	Demetri Obenour <demetriobenour@gmail.com>
> 
> > That'd be the worst of both worlds, IMO: a not-so-good PRNG with no
> > way whatsoever to get a repeatable sequence.  Am I right?
> 
> Oh dear. Yes. Worse than that, repeatability is part of the contract for
> the lisp "random" function, so seed_random() and get_random() are
> constrained to be deterministic. Right?
> 
> Seems as though init_random() is the only place that could use
> CryptGenRandom, which is a pity if you're trying to confine the
> changes to w32.c.

How about the following, then?

(Not sure it's a good idea to read from /dev/random, as that could
block; perhaps we should use /dev/urandom instead.)

--- src/sysdep.c~2	2015-11-11 07:57:41.000000000 +0200
+++ src/sysdep.c	2015-12-31 16:09:46.987229300 +0200
@@ -2095,8 +2095,35 @@ seed_random (void *seed, ptrdiff_t seed_
 void
 init_random (void)
 {
-  struct timespec t = current_timespec ();
-  uintmax_t v = getpid () ^ t.tv_sec ^ t.tv_nsec;
+  uintmax_t v;
+  struct timespec t;
+  bool success = false;
+
+#if HAVE_DEV_RANDOM
+  FILE *fp = fopen ("/dev/random", "rb");
+
+  if (fp)
+    {
+      int i;
+
+      for (i = 0, v = 0; i < sizeof (uintmax_t); i++)
+	{
+	  v <<= 8;
+	  v |= fgetc (fp);
+	}
+      fclose (fp);
+      success = true;
+    }
+#elif defined WINDOWSNT
+  if (w32_init_random (&v, sizeof v) == 0)
+    success = true;
+#endif	/* HAVE_DEV_RANDOM || WINDOWSNT */
+  if (!success)
+    {
+      /* Fall back to current time value + PID.  */
+      t = current_timespec ();
+      v = getpid () ^ t.tv_sec ^ t.tv_nsec;
+    }
   seed_random (&v, sizeof v);
 }
 


--- src/w32.c~0	2015-11-29 06:48:07.000000000 +0200
+++ src/w32.c	2015-12-31 16:05:06.775707600 +0200
@@ -224,6 +224,8 @@ typedef struct _REPARSE_DATA_BUFFER {
 
 #include <iphlpapi.h>	/* should be after winsock2.h */
 
+#include <wincrypt.h>
+
 #include <c-strcase.h>
 
 #include "w32.h"
@@ -2093,6 +2095,34 @@ init_user_info (void)
     CloseHandle (token);
 }
 
+static HCRYPTPROV w32_crypto_hprov;
+static int
+w32_init_crypt_random (void)
+{
+  if (!CryptAcquireContext (&w32_crypto_hprov, NULL, NULL, PROV_RSA_FULL,
+			    CRYPT_VERIFYCONTEXT | CRYPT_SILENT))
+    {
+      DebPrint (("CryptAcquireContext failed with error %x\n",
+		 GetLastError ()));
+      w32_crypto_hprov = 0;
+      return -1;
+    }
+  return 0;
+}
+
+int
+w32_init_random (void *buf, ptrdiff_t buflen)
+{
+  if (w32_crypto_hprov)
+    w32_init_crypt_random ();
+  if (w32_crypto_hprov)
+    {
+      if (CryptGenRandom (w32_crypto_hprov, buflen, (BYTE *)buf))
+	return 0;
+    }
+  return -1;
+}
+
 int
 random (void)
 {
@@ -9386,6 +9416,8 @@ globals_of_w32 (void)
   extern void dynlib_reset_last_error (void);
   dynlib_reset_last_error ();
 #endif
+
+  w32_crypto_hprov = (HCRYPTPROV)0;
 }
 
 /* For make-serial-process  */


--- src/w32.h~2	2015-11-29 06:48:07.000000000 +0200
+++ src/w32.h	2015-12-31 16:09:21.960654500 +0200
@@ -222,6 +222,9 @@ extern int w32_memory_info (unsigned lon
 /* Compare 2 UTF-8 strings in locale-dependent fashion.  */
 extern int w32_compare_strings (const char *, const char *, char *, int);
 
+/* Return a cryptographically secure seed for PRNG.  */
+extern int w32_init_random (void *, ptrdiff_t);
+
 #ifdef HAVE_GNUTLS
 #include <gnutls/gnutls.h>
 


--- src/fns.c~	2015-11-22 07:57:20.000000000 +0200
+++ src/fns.c	2015-12-31 16:57:43.607286800 +0200
@@ -50,7 +50,8 @@ All integers representable in Lisp, i.e.
 and `most-positive-fixnum', inclusive, are equally likely.
 
 With positive integer LIMIT, return random number in interval [0,LIMIT).
-With argument t, set the random number seed from the current time and pid.
+With argument t, set the random number seed from the system's entropy
+pool, or from the current time and pid if entropy is unavailable.
 With a string argument, set the seed based on the string's contents.
 Other values of LIMIT are ignored.
 
--- configure.ac~2	2015-12-20 06:45:33.000000000 +0200
+++ configure.ac	2015-12-31 16:06:48.959511800 +0200
@@ -4145,6 +4145,22 @@
 
 AC_TYPE_MBSTATE_T
 
+AC_MSG_CHECKING([whether "/dev/random" is available])
+dev_random=no
+dnl MSYS, being a Cygwin fork, thinks "/dev/random" does exist, so
+dnl don't check this for the MinGW builds.
+if test "${opsys}" != "mingw32"; then
+   if test -r "/dev/random"; then
+      AC_DEFINE(HAVE_DEV_RANDOM, 1, [Define if the system supports the "/dev/random" device.])
+      dev_random=yes
+   fi
+fi
+if test $dev_random = yes; then
+   AC_MSG_RESULT(yes)
+else
+   AC_MSG_RESULT(no)
+fi
+
 dnl Fixme: AC_SYS_POSIX_TERMIOS should probably be used, but it's not clear
 dnl how the tty code is related to POSIX and/or other versions of termios.
 dnl The following looks like a useful start.

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

[Demetrios Obenour]

On Wed, 2015-12-30 at 20:47 +0000, Richard Copley wrote:
> > Can you audit the patch below?  I know next to nothing about
> > cryptography, and I'm not sure I understood all the flags involved
> > in
> > these APIs.
> 
> Sure! But please bear in mind I'm not experienced in crypto
> either.
> 
> With regard to API usage: The call to CryptAcquireContext looks good
> to me. (The comment about interoperability in the documentation for
> the parameter "pszProvider" does not apply as we are not inter-
> operating with anything. Setting "pszContainer" to NULL, as you have
> done, is explicitly recommended. The docs for the individual flags
> entail the very value of "dwFlags" that you use.) I can see nothing
> else to comment on.
> 
> Re performance: using CryptGenRandom to provide a seed for srand
> is enough to address Demetri's concern. For performance reasons,
> as you said, implementing random() with CryptGenRandom is
> potentially bad. I think random() itself should not be changed.
> 
> That said, rand() makes me uncomfortable (mostly because of bugs in
> long-gone implementations, and superstition). Given the chance I
> would
> replace it with an xorshift* generator. The generator at [1] seeded
> with 64 bits from CryptGenRandom should give good performance for
> random() and (I guess!) an effectively unassailable server secret.
> 
> But I have no good reason to claim that rand() is not good enough.
> 
> Thank you Eli.
> 
> [1]: https://en.wikipedia.org/w/index.php?title=Xorshift&oldid=697235
> 156#xorshift.2A
>
The server secret should be entirely obtained from CryptGenRandom (or
the function RtlGenRandom on which it is based).  The server secret is
a cryptographic key and should be generated as such.  Using the same
entropy to seed an insecure PRNG and the server secret is a bad idea --
the server secret could be guessed based on PRNG output.

It would also be nice to expose a CSPRNG to Lisp on all platforms.  I
know that SLIME could use it on Windows, and it would be nice if one
could have a just-do-it API for this purpose.  Speed does not matter
much here.

Demetri

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

[Eli Zaretskii]

> From: Demetrios Obenour <demetriobenour@gmail.com>
> Cc: David Engster <deng@randomsample.de>, 22202@debbugs.gnu.org
> Date: Thu, 31 Dec 2015 12:04:38 -0500
> 
> The server secret should be entirely obtained from CryptGenRandom (or
> the function RtlGenRandom on which it is based).  The server secret is
> a cryptographic key and should be generated as such.  Using the same
> entropy to seed an insecure PRNG and the server secret is a bad idea --
> the server secret could be guessed based on PRNG output.

I don't understand what you are saying.  server.el doesn't use the
secret, it simply invokes the 'random' function several time to
generate the authentication key.  The secret is used to seed the PRNG
during Emacs startup, and it is used only once.

Given this description, how can the secret be guessed, and what are
the implications of that guess (if indeed it's possible) on the
ability of an attacker to control Emacs via the client socket?

> It would also be nice to expose a CSPRNG to Lisp on all platforms.  I
> know that SLIME could use it on Windows, and it would be nice if one
> could have a just-do-it API for this purpose.  Speed does not matter
> much here.

Patches are welcome, but they should include the same feature for
Posix hosts, probably using /dev/random.

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

[Richard Copley]

That last patch would still improve matters. The user would have
to be publishing the output of their PRNG to begin with in order
for the attacker to analyse it and guess the seed. (I don't know
how one could do that but that's no proof that it's impossible.)

What Demetri has just described is what I would do. (Sorry
again that I can't assist with a patch.)

+  if (w32_crypto_hprov)
+    w32_init_crypt_random ();

should be

+  if (! w32_crypto_hprov)
+    w32_init_crypt_random ();

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

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Thu, 31 Dec 2015 17:47:18 +0000
> Cc: Demetrios Obenour <demetriobenour@gmail.com>, David Engster <deng@randomsample.de>, 
> 	22202@debbugs.gnu.org
> 
> That last patch would still improve matters. The user would have
> to be publishing the output of their PRNG to begin with in order
> for the attacker to analyse it and guess the seed. (I don't know
> how one could do that but that's no proof that it's impossible.)

I don't even understand how that could be possible.

> What Demetri has just described is what I would do.

Now I'm confused: do what?  We still need to support 'random' with an
argument, so we cannot get rid of seeding a PRNG with a known value.
And I didn't want to remove srandom.

> +  if (w32_crypto_hprov)
> +    w32_init_crypt_random ();
> 
> should be
> 
> +  if (! w32_crypto_hprov)
> +    w32_init_crypt_random ();

Ah, that's a left-over from debugging.  Thanks.

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

[Eli Zaretskii]

> From: Demetrios Obenour <demetriobenour@gmail.com>
> Cc: David Engster <deng@randomsample.de>, 22202@debbugs.gnu.org
> Date: Thu, 31 Dec 2015 12:04:38 -0500
> 
> It would also be nice to expose a CSPRNG to Lisp on all platforms.  I
> know that SLIME could use it on Windows, and it would be nice if one
> could have a just-do-it API for this purpose.  Speed does not matter
> much here.

Btw, for the record: my speed measurements were wrong: it turned out I
compared an optimized build with an unoptimized one.  When compared
correctly, there's no perceptible performance difference between using
CRT's 'rand' and using the Windows Crypto API to generate random
numbers.

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

[Richard Copley]

>> That last patch would still improve matters. The user would have
>> to be publishing the output of their PRNG to begin with in order
>> for the attacker to analyse it and guess the seed. (I don't know
>> how one could do that but that's no proof that it's impossible.)
>
>I don't even understand how that could be possible.

Me either, but that doesn't make it impossible. (There are articles
on the web demonstrating such feats, if you're interested.)

>> What Demetri has just described is what I would do.
>
>Now I'm confused: do what?

As I understand it: Provide a function callable from lisp that returns
a cryptographically secure sequence of random bytes, of a specified
length. Use that function to generate the server secret.

>We still need to support 'random' with an
>argument, so we cannot get rid of seeding a PRNG with a known value.
>And I didn't want to remove srandom.

Given the above, we could leave "random", etc., as they are, or we
could use a better PRNG and/or seed with system entropy. It would
no longer be tied up with this issue report.

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

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Thu, 31 Dec 2015 19:49:42 +0000
> Cc: Demetrios Obenour <demetriobenour@gmail.com>, David Engster <deng@randomsample.de>, 
> 	22202@debbugs.gnu.org
> 
> >> What Demetri has just described is what I would do.
> >
> >Now I'm confused: do what?
> 
> As I understand it: Provide a function callable from lisp that returns
> a cryptographically secure sequence of random bytes, of a specified
> length. Use that function to generate the server secret.

That's what my patch does.

> >We still need to support 'random' with an
> >argument, so we cannot get rid of seeding a PRNG with a known value.
> >And I didn't want to remove srandom.
> 
> Given the above, we could leave "random", etc., as they are, or we
> could use a better PRNG and/or seed with system entropy. It would
> no longer be tied up with this issue report.

Patches welcome, as I said already.

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

[Richard Copley]

>> >> What Demetri has just described is what I would do.
>> >
>> >Now I'm confused: do what?
>>
>> As I understand it: Provide a function callable from lisp that returns
>> a cryptographically secure sequence of random bytes, of a specified
>> length. Use that function to generate the server secret.
>
> That's what my patch does.

A separate function from "random".

>> >We still need to support 'random' with an
>> >argument, so we cannot get rid of seeding a PRNG with a known value.
>> >And I didn't want to remove srandom.
>>
>> Given the above, we could leave "random", etc., as they are, or we
>> could use a better PRNG and/or seed with system entropy. It would
>> no longer be tied up with this issue report.
>
> Patches welcome, as I said already.

You asked me a question.

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

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Thu, 31 Dec 2015 19:49:42 +0000
> Cc: Demetrios Obenour <demetriobenour@gmail.com>, David Engster <deng@randomsample.de>, 
> 	22202@debbugs.gnu.org
> 
> >> That last patch would still improve matters. The user would have
> >> to be publishing the output of their PRNG to begin with in order
> >> for the attacker to analyse it and guess the seed. (I don't know
> >> how one could do that but that's no proof that it's impossible.)
> >
> >I don't even understand how that could be possible.
> 
> Me either, but that doesn't make it impossible. (There are articles
> on the web demonstrating such feats, if you're interested.)
> 
> >> What Demetri has just described is what I would do.
> >
> >Now I'm confused: do what?
> 
> As I understand it: Provide a function callable from lisp that returns
> a cryptographically secure sequence of random bytes, of a specified
> length. Use that function to generate the server secret.

That'd be an enhancement, not a bug.  Patches to provide such an API
are welcome, now that the infrastructure exists both on Posix hosts
and on MS-Windows (see below), the rest should be easy: one just needs
to follow the established APIs in other Lisp-like environments, I
think.

> >We still need to support 'random' with an
> >argument, so we cannot get rid of seeding a PRNG with a known value.
> >And I didn't want to remove srandom.
> 
> Given the above, we could leave "random", etc., as they are, or we
> could use a better PRNG and/or seed with system entropy. It would
> no longer be tied up with this issue report.

I preferred to make it possible to pass a cryptographically secure
byte stream to 'srandom' instead.  See commit 3ffe81e on the emacs-25
branch.  This leaves the basic 'random' functionality intact, so no
Lisp packages should be affected.

I'm therefore marking this bug as done.  Thanks for the feedback.

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

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 743 bytes --]

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.

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.

[-- Attachment #2: 0001-Prefer-GnuTLS-when-acquiring-random-seed.patch --]
[-- Type: text/x-diff, Size: 4807 bytes --]

From 05e8148a24ebe51fbe758dd16265e8fb81f85953 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 17 Jan 2016 12:12:08 -0800
Subject: [PATCH] Prefer GnuTLS when acquiring random seed

This attempts to improve on the fix for Bug#22202.
* configure.ac (HAVE_DEV_URANDOM): Remove.
Check /dev/urandom existence at run time, not at build time,
since the device could exist in the former but not the latter.
* src/sysdep.c [HAVE_GNUTLS]: Include gnutls/gnutls.h.
(gnutls_rnd) [GNUTLS_VERSION_NUMBER < 0x020c00]: New fallback macro.
(random_seed): New typedef.
(set_random_seed): New static function.
(seed_random): Use them.
(init_random): Use random_seed instead of uintmax_t, so as to
not consume more entropy than needed.  Prefer gnutls_rnd if it
works; this avoids a redundant open of /dev/urandom on
GNU/Linux with modern GnuTLS.
---
 configure.ac | 16 ------------
 src/sysdep.c | 85 ++++++++++++++++++++++++++++++------------------------------
 2 files changed, 43 insertions(+), 58 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6c9b621..8c01aba 100644
--- a/configure.ac
+++ b/configure.ac
@@ -4153,22 +4153,6 @@ fi
 
 AC_TYPE_MBSTATE_T
 
-AC_MSG_CHECKING([whether "/dev/urandom" is available])
-dev_urandom=no
-dnl MSYS, being a Cygwin fork, thinks "/dev/urandom" does exist, so
-dnl don't check this for the MinGW builds.
-if test "${opsys}" != "mingw32"; then
-   if test -r "/dev/urandom"; then
-      AC_DEFINE(HAVE_DEV_URANDOM, 1, [Define if the system supports the "/dev/urandom" device.])
-      dev_urandom=yes
-   fi
-fi
-if test $dev_urandom = yes; then
-   AC_MSG_RESULT(yes)
-else
-   AC_MSG_RESULT(no)
-fi
-
 dnl Fixme: AC_SYS_POSIX_TERMIOS should probably be used, but it's not clear
 dnl how the tty code is related to POSIX and/or other versions of termios.
 dnl The following looks like a useful start.
diff --git a/src/sysdep.c b/src/sysdep.c
index 1fa4229..635443c 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -99,6 +99,15 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "process.h"
 #include "cm.h"
 
+#ifdef HAVE_GNUTLS
+# include <gnutls/gnutls.h>
+#endif
+#if 0x020c00 <= GNUTLS_VERSION_NUMBER
+# include <gnutls/crypto.h>
+#else
+# define gnutls_rnd(level, data, len) (-1)
+#endif
+
 #ifdef WINDOWSNT
 #include <direct.h>
 /* In process.h which conflicts with the local copy.  */
@@ -2068,63 +2077,55 @@ init_signals (bool dumping)
 # endif /* !HAVE_RANDOM */
 #endif /* !RAND_BITS */
 
+#ifdef HAVE_RANDOM
+typedef unsigned int random_seed;
+static void set_random_seed (random_seed arg) { srandom (arg); }
+#elif defined HAVE_LRAND48
+/* Although srand48 uses a long seed, this is unsigned long to avoid
+   undefined behavior on signed integer overflow in init_random.  */
+typedef unsigned long int random_seed;
+static void set_random_seed (random_seed arg) { srand48 (arg); }
+#else
+typedef unsigned int random_seed;
+static void set_random_seed (random_seed arg) { srand (arg); }
+#endif
+
 void
 seed_random (void *seed, ptrdiff_t seed_size)
 {
-#if defined HAVE_RANDOM || ! defined HAVE_LRAND48
-  unsigned int arg = 0;
-#else
-  long int arg = 0;
-#endif
+  random_seed arg = 0;
   unsigned char *argp = (unsigned char *) &arg;
   unsigned char *seedp = seed;
-  ptrdiff_t i;
-  for (i = 0; i < seed_size; i++)
+  for (ptrdiff_t i = 0; i < seed_size; i++)
     argp[i % sizeof arg] ^= seedp[i];
-#ifdef HAVE_RANDOM
-  srandom (arg);
-#else
-# ifdef HAVE_LRAND48
-  srand48 (arg);
-# else
-  srand (arg);
-# endif
-#endif
+  set_random_seed (arg);
 }
 
 void
 init_random (void)
 {
-  uintmax_t v;
-  struct timespec t;
-  bool success = false;
-
-#if HAVE_DEV_URANDOM
-  FILE *fp = fopen ("/dev/urandom", "rb");
-
-  if (fp)
+  random_seed v;
+  if (gnutls_rnd (GNUTLS_RND_NONCE, &v, sizeof v) != 0)
     {
-      int i;
-
-      for (i = 0, v = 0; i < sizeof (uintmax_t); i++)
+      bool success = false;
+#ifndef WINDOWSNT
+      int fd = emacs_open ("/dev/urandom", O_RDONLY | O_BINARY, 0);
+      if (0 <= fd)
 	{
-	  v <<= 8;
-	  v |= fgetc (fp);
+	  success = emacs_read (fd, &v, sizeof v) == sizeof v;
+	  emacs_close (fd);
+	}
+#else
+      success = w32_init_random (&v, sizeof v) == 0;
+#endif
+      if (! success)
+	{
+	  /* Fall back to current time value + PID.  */
+	  struct timespec t = current_timespec ();
+	  v = getpid () ^ t.tv_sec ^ t.tv_nsec;
 	}
-      fclose (fp);
-      success = true;
-    }
-#elif defined WINDOWSNT
-  if (w32_init_random (&v, sizeof v) == 0)
-    success = true;
-#endif	/* HAVE_DEV_URANDOM || WINDOWSNT */
-  if (!success)
-    {
-      /* Fall back to current time value + PID.  */
-      t = current_timespec ();
-      v = getpid () ^ t.tv_sec ^ t.tv_nsec;
     }
-  seed_random (&v, sizeof v);
+  set_random_seed (v);
 }
 
 /*
-- 
2.5.0

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

[Paul Eggert]

Andreas Schwab discovered a problem with my patch in that GnuTLS wasn't 
initialized, and reverted the GnuTLS part of it. As I understand it, newer 
versions of GnuTLS initialize themselves when they are loaded and so do not run 
into the issue; I tested with GnuTLS 3.3.15, which I suppose is new enough. I 
attempted to fix this problem in the followup commit 
130d512045aa376333b664d58c501b3884187592.

Andreas's commit also changed some unrelated style issues, which I reverted; 
that is merely a longrunning stylistic disagreement, and right now is not a good 
time to be changing style in code unrelated to fixes.

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

[Andy Moreton]

On Sun 17 Jan 2016, Paul Eggert wrote:

> Andreas Schwab discovered a problem with my patch in that GnuTLS wasn't
> initialized, and reverted the GnuTLS part of it. As I understand it, newer
> versions of GnuTLS initialize themselves when they are loaded and so do not
> run into the issue; I tested with GnuTLS 3.3.15, which I suppose is new
> enough. I attempted to fix this problem in the followup commit
> 130d512045aa376333b664d58c501b3884187592.

Your patch will break builds configured using --without-gnutls, as the
gnutls headers may not be installed, and should not be included.

In addition, these changes require static or dynamic linking of gnutls,
which breaks the Windows builds (which use runtime imports for the gnutls DLLs).

> Andreas's commit also changed some unrelated style issues, which I reverted;
> that is merely a longrunning stylistic disagreement, and right now is not a
> good time to be changing style in code unrelated to fixes.

This is rudeness from both of you: please solve this disagreement by
talking to each other instead of adding pointless churn to the source tree.

    AndyM

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

[Richard Copley]

On 18 January 2016 at 01:42, Paul Eggert <eggert@cs.ucla.edu> wrote:
> Andreas Schwab discovered a problem with my patch in that GnuTLS wasn't
> initialized, and reverted the GnuTLS part of it. As I understand it, newer
> versions of GnuTLS initialize themselves when they are loaded and so do not
> run into the issue; I tested with GnuTLS 3.3.15, which I suppose is new
> enough. I attempted to fix this problem in the followup commit
> 130d512045aa376333b664d58c501b3884187592.
>
> Andreas's commit also changed some unrelated style issues, which I reverted;
> that is merely a longrunning stylistic disagreement, and right now is not a
> good time to be changing style in code unrelated to fixes.

I can't build from the current sources; the error is:

  CCLD     temacs.exe
sysdep.o: In function `init_random':
C:/emacs/repo/emacs/src/sysdep.c:2108: undefined reference to `gnutls_rnd'
C:/emacs/repo/emacs/src/sysdep.c:2108:(.text+0xf38): relocation
truncated to fit: R_X86_64_PC32 against undefined symbol `gnutls_rnd'
collect2.exe: error: ld returned 1 exit status

Configuration details (from last good build):

In GNU Emacs 25.0.50.1 (x86_64-w64-mingw32)
 of 2016-01-14 built on 60678UHB
Repository revision: dadb841a06aa1ffd6d17c04ef83140dbd1ad7307
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Configured using:
 'configure --prefix /c/emacs/emacs-20160114-182403
 --without-imagemagick --disable-dependency-tracking
 --enable-locallisppath=%emacs_dir%/../site-lisp 'CFLAGS=-Og -g -ggdb''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND DBUS NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS

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

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: 22202@debbugs.gnu.org, Richard Copley <rcopley@gmail.com>,
>  demetriobenour@gmail.com, deng@randomsample.de
> 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?

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

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Mon, 18 Jan 2016 12:04:34 +0000
> 
> In addition, these changes require static or dynamic linking of gnutls,
> which breaks the Windows builds (which use runtime imports for the gnutls DLLs).

This part should be provisionally fixed now.

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

[Eli Zaretskii]

> From: Richard Copley <rcopley@gmail.com>
> Date: Mon, 18 Jan 2016 14:40:05 +0000
> Cc: 22202@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>, 
> 	Demetri Obenour <demetriobenour@gmail.com>, David Engster <deng@randomsample.de>, 
> 	Andreas Schwab <schwab@linux-m68k.org>
> 
> I can't build from the current sources; the error is:
> 
>   CCLD     temacs.exe
> sysdep.o: In function `init_random':
> C:/emacs/repo/emacs/src/sysdep.c:2108: undefined reference to `gnutls_rnd'
> C:/emacs/repo/emacs/src/sysdep.c:2108:(.text+0xf38): relocation
> truncated to fit: R_X86_64_PC32 against undefined symbol `gnutls_rnd'
> collect2.exe: error: ld returned 1 exit status

Please try again.

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

[Richard Copley]

On 18 January 2016 at 16:05, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Richard Copley <rcopley@gmail.com>
>> Date: Mon, 18 Jan 2016 14:40:05 +0000
>> Cc: 22202@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>,
>>       Demetri Obenour <demetriobenour@gmail.com>, David Engster <deng@randomsample.de>,
>>       Andreas Schwab <schwab@linux-m68k.org>
>>
>> I can't build from the current sources; the error is:
>>
>>   CCLD     temacs.exe
>> sysdep.o: In function `init_random':
>> C:/emacs/repo/emacs/src/sysdep.c:2108: undefined reference to `gnutls_rnd'
>> C:/emacs/repo/emacs/src/sysdep.c:2108:(.text+0xf38): relocation
>> truncated to fit: R_X86_64_PC32 against undefined symbol `gnutls_rnd'
>> collect2.exe: error: ld returned 1 exit status
>
> Please try again.

Fixed, thanks.

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

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 2551 bytes --]

Eli Zaretskii wrote:
> I wish we'd discuss such things before committing and not after.

Sorry, I missed the part of the discussion that talked about reading 
/dev/urandom in the first place.

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

It's one more thing to worry about when auditing an Emacs trace. Also, it's two 
file descriptors (both open at the same time) when we can get by with just one.

>      . GnuTLS is a separate library, so it's free to do that for its
>        own purposes; we shouldn't care.

Yes and no. Yes, we shouldn't care how GnuTLS gets entropy; and no, because if 
GnuTLS is available we should be better off using its entropy source than 
rolling our own. The GnuTLS guys are far more expert in this stuff; why reinvent 
the wheel? And if the GnuTLS entropy source is busted, Emacs is already insecure 
in dozens of important ways, so using that source here shouldn't make matters 
significantly worse.

>	 Besides, what if tomorrow
>        there will be a 3rd library that would need to access
>        /dev/urandom?

Not our problem. As you say, libraries are free to do that for their own 
purposes, and we shouldn't care.

>    . GnuTLS is a library for TLS, not for cryptography.

GnuTLS is not just for TLS, it's for secure communications. Getting a nonce is a 
basic building block for such a library. They're not going to remove a basic 
building block.

>      What if tomorrow GnuTLS changes its implementation?

That's fine. We don't need to know the details of how GnuTLS gets its nonces. 
For example, if it starts using the RDRAND instruction available on Ivy Bridge 
and later Intel processors, more power to them. We shouldn't care.

>    . This change means that we now load GnuTLS at startup, even if no
>      TLS connections are or will be used.

That's already true on GNU and POSIXish platforms, so it's not a problem there. 
It is an issue on MS-Windows, though, so your change to avoid GnuTLS here on 
MS-Windows makes sense.

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

Merely because we're in the neighborhood anyway and it's the first time I 
noticed that this detail was documented. The detail doesn't belong in the 
documentation; Emacs shouldn't promise that it'll use the PID, for example.

One other thing, while we're nearby: the doc shouldn't assume that readers know 
that time-of-day etc. is less random.

How about the attached patch? It should address these documentation concerns.

[-- Attachment #2: t.diff --]
[-- Type: text/x-diff, Size: 1316 bytes --]

diff --git a/doc/lispref/numbers.texi b/doc/lispref/numbers.texi
index 3a9483a..28eb6b1 100644
--- a/doc/lispref/numbers.texi
+++ b/doc/lispref/numbers.texi
@@ -1252,9 +1252,9 @@ Random Numbers
 (@pxref{Integer Basics}).
 
 If @var{limit} is @code{t}, it means to choose a new seed as if Emacs
-were restarting.  The new seed will be set from the system entropy, if
-that is available, or from the current time and Emacs process's PID
-(@pxref{System Environment, emacs-pid}) if not.
+were restarting, typically from the system entropy.  On systems
+lacking entropy pools, choose the seed from less-random volatile data
+such as the current time.
 
 If @var{limit} is a string, it means to choose a new seed based on the
 string's contents.
diff --git a/src/fns.c b/src/fns.c
index 19fa440..86ad333 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -51,7 +51,7 @@ and `most-positive-fixnum', inclusive, are equally likely.
 
 With positive integer LIMIT, return random number in interval [0,LIMIT).
 With argument t, set the random number seed from the system's entropy
-pool, or from the current time and pid if entropy is unavailable.
+pool if available, otherwise from less-random volatile data such as the time.
 With a string argument, set the seed based on the string's contents.
 Other values of LIMIT are ignored.
 

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

[Eli Zaretskii]

> Cc: 22202@debbugs.gnu.org, rcopley@gmail.com, deng@randomsample.de
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 18 Jan 2016 12:50:12 -0800
> 
> > I wish we'd discuss such things before committing and not after.
> 
> Sorry, I missed the part of the discussion that talked about reading 
> /dev/urandom in the first place.

That's not what I meant.  You saw the code I committed just a few days
ago; it would be prudent to discuss your plans to change that.  We all
silently fix blunders and other trivial problems; this wasn't one of
them.  It wasn't a bug fix, either.  So there was no need to rush with
the commit.  Such conduct doesn't make us feel like a community.

> >    . I see nothing wrong with having 2 (or more) independent reads from
> >      /dev/urandom:
> 
> It's one more thing to worry about when auditing an Emacs trace.

Why is that a worry?  We use many libraries, some of them open and
read files.  What's to worry about?

> Also, it's two file descriptors (both open at the same time) when we
> can get by with just one.

AFAICS, we close the file descriptor as soon as we finished reading.
So unless GnuTLS initialization is run in another thread, there won't
be 2 descriptors at the same time.

> >      . GnuTLS is a separate library, so it's free to do that for its
> >        own purposes; we shouldn't care.
> 
> Yes and no. Yes, we shouldn't care how GnuTLS gets entropy; and no, because if 
> GnuTLS is available we should be better off using its entropy source than 
> rolling our own. The GnuTLS guys are far more expert in this stuff; why reinvent 
> the wheel?

It's a very simple wheel.  If you've seen their sources (I have), you
know that they do exactly the same, both on GNU/Linux and on
MS-Windows.

> And if the GnuTLS entropy source is busted, Emacs is already
> insecure in dozens of important ways, so using that source here
> shouldn't make matters significantly worse.

I don't think we should become experts on external libraries, and I
don't think we should track their development.  Where GnuTLS needs
security for its internal use, we should let it do what they see fit;
if they do that wrongly, the word will spread, and we will upgrade or
switch to another library.

But where we need to seed our own PRNG, we better had a good idea of
what we do and what kind of randomness we get.  This is the core of
this bug report.  I don't think we should delegate that to an external
library whose main business is secure communications.  It's a
different discipline, different trade-offs, etc.

> >	 Besides, what if tomorrow
> >        there will be a 3rd library that would need to access
> >        /dev/urandom?
> 
> Not our problem. As you say, libraries are free to do that for their own 
> purposes, and we shouldn't care.

So what is special about GnuTLS?

> > Why is it suddenly a concern that users will know that we use time and
> > PID as fallback?
> 
> Merely because we're in the neighborhood anyway and it's the first time I 
> noticed that this detail was documented. The detail doesn't belong in the 
> documentation; Emacs shouldn't promise that it'll use the PID, for example.

We did that since 1993.  Isn't it a tad too late to worry about it?

> One other thing, while we're nearby: the doc shouldn't assume that readers know 
> that time-of-day etc. is less random.

A fallback is always worse than Plan A.  Everyone understands that,
even if they know nothing about randomness and PRNGs.

> How about the attached patch? It should address these documentation concerns.

Frankly, it feels silly, but since you are so enthusiastic, who am I
to stand on your way?

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

[John Wiegley]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

>>>>> Andy Moreton <andrewjmoreton@gmail.com> writes:

> This is rudeness from both of you: please solve this disagreement by talking
> to each other instead of adding pointless churn to the source tree.

Whenever possible, resolving arguments by words rather than commits is truly
preferable.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

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

[Paul Eggert]

Eli Zaretskii wrote:

> We all silently fix blunders and other trivial problems; this wasn't one of
> them.

I thought it a trivial matter; evidently I was mistaken. My apologies.

> AFAICS, we close the file descriptor as soon as we finished reading.
> So unless GnuTLS initialization is run in another thread, there won't
> be 2 descriptors at the same time.

GnuTLS keeps /dev/urandom open indefinitely. If Emacs opens /dev/urandom 
independently it can have two file descriptors open to the same file. Yes, it's 
not a huge deal performance-wise; but it is strange, and when doing security 
audits it will be one more thing to explain.

> But where we need to seed our own PRNG, we better had a good idea of
> what we do and what kind of randomness we get.

Any worries we might have about GnuTLS's randomness apply with equal force to 
/dev/urandom's. After all, /dev/urandom is not guaranteed to be random.

Really, though, if we can't trust GnuTLS to give us random data, we should not 
trust it for communications security at all. Nonces are that basic.

> So what is special about GnuTLS?

GnuTLS already has the random data we need; other libraries don't.

I installed the documentation patch, since it does seem a minor improvement. 
Yes, the doc could have been improved ages ago, but late is better than never.

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

[Eli Zaretskii]

> Cc: 22202@debbugs.gnu.org, rcopley@gmail.com, deng@randomsample.de
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 18 Jan 2016 21:34:12 -0800
> 
>     AFAICS, we close the file descriptor as soon as we finished reading.
>     So unless GnuTLS initialization is run in another thread, there won't
>     be 2 descriptors at the same time.
> 
> GnuTLS keeps /dev/urandom open indefinitely.

So it's a bug or misfeature in GnuTLS.  Not our concern.

> If Emacs opens /dev/urandom independently it can have two file descriptors open to the same file. Yes, it's not a huge deal performance-wise; but it is strange, and when doing security audits it will be one more thing to explain.

GnuTLS guys need to explain this, not us.

>     But where we need to seed our own PRNG, we better had a good idea of
>     what we do and what kind of randomness we get.
> 
> Any worries we might have about GnuTLS's randomness apply with equal force to /dev/urandom's. After all, /dev/urandom is not guaranteed to be random.

No, /dev/urandom is random enough for our purposes.

> Really, though, if we can't trust GnuTLS to give us random data, we should not trust it for communications security at all. Nonces are that basic.

We could stop trusting GnuTLS for communications security, but we
still need the secure random seed for server-start.  I see no reasons
to tie these two together.  The Emacs server is not about TLS
communications, at least not by default.

If GnuTLS were a library of RNGs, it would have been a different
matter.  But it isn't.  We shouldn't depend so critically on 3rd party
libraries for functionality that is unrelated or secondary to their
goal.

>     So what is special about GnuTLS?
> 
> GnuTLS already has the random data we need; other libraries don't.

We have what we need; calling gnutls_rnd changes nothing in this
regard.  It's just a more complex way of issuing the same system
calls.  It buys us nothing in terms of security and performance, while
we sustain the price of having core functionality that must run at
startup crucially depending on a 3rd party library we don't control.

John, I feel this decision is wrong and the changes that prefer
gnutls_rnd should be reverted.  Maybe I'm the only one who cares, but
then Paul is the only one who felt the need to make that change.  I'd
like to hear your take on this, please.

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

[John Wiegley]

[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> We have what we need; calling gnutls_rnd changes nothing in this regard.
> It's just a more complex way of issuing the same system calls. It buys us
> nothing in terms of security and performance, while we sustain the price of
> having core functionality that must run at startup crucially depending on a
> 3rd party library we don't control.

> John, I feel this decision is wrong and the changes that prefer gnutls_rnd
> should be reverted. Maybe I'm the only one who cares, but then Paul is the
> only one who felt the need to make that change. I'd like to hear your take
> on this, please.

From what I've read, I agree with you Eli. If we can open /dev/urandom, why do
we need a dependency on GnuTLS to effectively do the same thing?

What critical feature is GnuTLS buying for us that would make this worthwhile,
Paul?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

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

[Paul Eggert]

On 01/19/2016 08:24 AM, Eli Zaretskii wrote:
> So it's a bug or misfeature in GnuTLS.

GnuTLS has been operating that way for a while, and it works. Calling 
its behavior a "bug or misfeature" seems a stretch.

If we change Emacs back to always read /dev/urandom by hand as well has 
have GnuTLS read /dev/urandom at startup, this will cause Emacs to 
exhaust the GNU/Linux entropy pool more quickly. This may slow down 
other programs that read /dev/random (a device that blocks until entropy 
is available). So there is an overall system benefit to minimizing the 
use of /dev/urandom, which was the point of my original patch.

>> If Emacs opens /dev/urandom independently it can have two file descriptors open to the same file. Yes, it's not a huge deal performance-wise; but it is strange, and when doing security audits it will be one more thing to explain.
> GnuTLS guys need to explain this, not us.

Any explanation they come up with will have to be part of our 
explanation, since we're responsible for Emacs. Our explanation will 
also have to cover Emacs's added accesses, so minimizing them will be a win.

>>      But where we need to seed our own PRNG, we better had a good idea of
>>      what we do and what kind of randomness we get.
>>
>> Any worries we might have about GnuTLS's randomness apply with equal force to /dev/urandom's. After all, /dev/urandom is not guaranteed to be random.
> No, /dev/urandom is random enough for our purposes.

In that case GnuTLS's nonce generator is random enough for our purposes, 
and we have a good idea of what kind of randomness we get.

>
>> Really, though, if we can't trust GnuTLS to give us random data, we should not trust it for communications security at all. Nonces are that basic.
> We could stop trusting GnuTLS for communications security, but we
> still need the secure random seed for server-start.

If we stop trusting or using GnuTLS, the code will still get a secure 
random seed by hand, so that's not a problem. But currently we do trust 
and use GnuTLS by default, and there are no plans to change this.

> We have what we need; calling gnutls_rnd changes nothing in this 
> regard. It's just a more complex way of issuing the same system calls.

They are not the same system calls. If they were the same, you would be 
right and we shouldn't bother with GnuTLS here. They are different 
sequences of system calls, and the sequence that uses GnuTLS lessens 
entropy consumption and simplifies audits.

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

[Paul Eggert]

On 01/19/2016 09:03 AM, John Wiegley wrote:
> What critical feature is GnuTLS buying for us that would make this worthwhile,
> Paul?

There is nothing "critical" here. This is just a minor issue, one that 
has been blown all out of proportion. Using GnuTLS when available 
lessens use of system resources and simplifies auditing, but Emacs could 
get by without this minor bugfix-improvement.

> why do we need a dependency on GnuTLS

There isn't a dependency on GnuTLS in the usual sense: that is, if 
GnuTLS is absent, the code still works as before. The only dependency is 
that we trust the GnuTLS library to work when it is present, and to 
report an error if one occurs. This is a reasonable assumption, both 
here and elsewhere in Emacs.

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

[Eli Zaretskii]

> Cc: 22202@debbugs.gnu.org, rcopley@gmail.com, deng@randomsample.de
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 19 Jan 2016 09:07:15 -0800
> 
> On 01/19/2016 08:24 AM, Eli Zaretskii wrote:
> > So it's a bug or misfeature in GnuTLS.
> 
> GnuTLS has been operating that way for a while, and it works. Calling 
> its behavior a "bug or misfeature" seems a stretch.

You said it was a problem to have one more handle open, not I.  So
it's you who maintains this is a problem (a.k.a. "misfeature").  If
you are now saying it's okay to have the device open, then it's not a
problem to have it open twice for a short while.

IOW, please make up your mind whether this is an issue or not.

> If we change Emacs back to always read /dev/urandom by hand as well has 
> have GnuTLS read /dev/urandom at startup, this will cause Emacs to 
> exhaust the GNU/Linux entropy pool more quickly. This may slow down 
> other programs that read /dev/random (a device that blocks until entropy 
> is available). So there is an overall system benefit to minimizing the 
> use of /dev/urandom, which was the point of my original patch.

Now, _that's_ a stretch.  We only read /dev/urandom once, that's all.
You cannot be serious saying that a single call will deplete the
system entropy.

> >> If Emacs opens /dev/urandom independently it can have two file descriptors open to the same file. Yes, it's not a huge deal performance-wise; but it is strange, and when doing security audits it will be one more thing to explain.
> > GnuTLS guys need to explain this, not us.
> 
> Any explanation they come up with will have to be part of our 
> explanation

No, it doesn't.  We cannot be responsible for the internals of
3rd-party libraries.

> >>      But where we need to seed our own PRNG, we better had a good idea of
> >>      what we do and what kind of randomness we get.
> >>
> >> Any worries we might have about GnuTLS's randomness apply with equal force to /dev/urandom's. After all, /dev/urandom is not guaranteed to be random.
> > No, /dev/urandom is random enough for our purposes.
> 
> In that case GnuTLS's nonce generator is random enough for our purposes, 
> and we have a good idea of what kind of randomness we get.

Today, we do.  Tomorrow, it's anyone's guess.  Unless we audit their
code all the time.  Why should we?

> >> Really, though, if we can't trust GnuTLS to give us random data, we should not trust it for communications security at all. Nonces are that basic.
> > We could stop trusting GnuTLS for communications security, but we
> > still need the secure random seed for server-start.
> 
> If we stop trusting or using GnuTLS, the code will still get a secure 
> random seed by hand, so that's not a problem. But currently we do trust 
> and use GnuTLS by default, and there are no plans to change this.

That trust needs now to be ascertained even if we don't use secure
communications from Emacs.  So things got worse.

> > We have what we need; calling gnutls_rnd changes nothing in this 
> > regard. It's just a more complex way of issuing the same system calls.
> 
> They are not the same system calls. If they were the same, you would be 
> right and we shouldn't bother with GnuTLS here. They are different 
> sequences of system calls, and the sequence that uses GnuTLS lessens 
> entropy consumption and simplifies audits.

I've read the code and saw no differences.

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

[Eli Zaretskii]

> Cc: 22202@debbugs.gnu.org, rcopley@gmail.com, deng@randomsample.de
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Tue, 19 Jan 2016 09:38:23 -0800
> 
> This is just a minor issue, one that has been blown all out of
> proportion.

This is unfair: if this minor issue was important enough for you to
make those changes, then objecting to them cannot be considered
"blowing it all out of proportion".

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

[Andy Moreton]

On Tue 19 Jan 2016, John Wiegley wrote:

>>>>>> Eli Zaretskii <eliz@gnu.org> writes:
>
>> We have what we need; calling gnutls_rnd changes nothing in this regard.
>> It's just a more complex way of issuing the same system calls. It buys us
>> nothing in terms of security and performance, while we sustain the price of
>> having core functionality that must run at startup crucially depending on a
>> 3rd party library we don't control.
>
>> John, I feel this decision is wrong and the changes that prefer gnutls_rnd
>> should be reverted. Maybe I'm the only one who cares, but then Paul is the
>> only one who felt the need to make that change. I'd like to hear your take
>> on this, please.
>
> From what I've read, I agree with you Eli. If we can open /dev/urandom, why do
> we need a dependency on GnuTLS to effectively do the same thing?
>
> What critical feature is GnuTLS buying for us that would make this worthwhile,
> Paul?

As far as I can see, this set of patches attempted to fix a minor
problem, but in doing so:
 - added unnecessary dependencies on gnutls libraries
 - broke the Windows builds (which use runtime linking for gnutls)
 - broke all builds configured with "--without-gnutls"

I am happy for the original issue to be addressed, but only if all of
the issues listed above are addressed.

In particular, it must remain possible to build on a system that does
not have gnutls headers and libraries installed, or to disable gnutls
support even if the headers and libraries are present.

    AndyM

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

[Paul Eggert]

On 01/19/2016 10:16 AM, Eli Zaretskii wrote:
>> Cc: 22202@debbugs.gnu.org, rcopley@gmail.com, deng@randomsample.de
>> From: Paul Eggert <eggert@cs.ucla.edu>
>> Date: Tue, 19 Jan 2016 09:07:15 -0800
>>
>> On 01/19/2016 08:24 AM, Eli Zaretskii wrote:
>>> So it's a bug or misfeature in GnuTLS.
>> GnuTLS has been operating that way for a while, and it works. Calling
>> its behavior a "bug or misfeature" seems a stretch.
> You said it was a problem to have one more handle open, not I.  So
> it's you who maintains this is a problem (a.k.a. "misfeature").  If
> you are now saying it's okay to have the device open, then it's not a
> problem to have it open twice for a short while.

There is no contradiction here. Although having multiple file 
descriptors for the same file is only a minor problem, that does not 
mean it is not a problem at all, nor does it mean that the minor problem 
is entirely in GnuTLS as opposed to being in the combination of GnuTLS 
and Emacs. I did not say GnuTLS's behavior is a misfeature or bug; such 
a characterization would go too far.

> You cannot be serious saying that a single call will deplete the
> system entropy.

Although the entropy will not be completely depleted, reading excess 
bytes from /dev/urandom can exhaust the entropy pool more quickly. On 
GNU/Linux, processes reading /dev/urandom can fairly quickly drain 
system entropy down to a couple hundred bits. Reads of /dev/random can 
then drain the remaining bits and subsequent reads will hang. Although 
the kernel eventually will reacquire system entropy, acquisition rates 
can be less than one bit per second, so reading excess data from 
/dev/urandom is not cost-free here.

>
>>>> If Emacs opens /dev/urandom independently it can have two file descriptors open to the same file. Yes, it's not a huge deal performance-wise; but it is strange, and when doing security audits it will be one more thing to explain.
>>> GnuTLS guys need to explain this, not us.
>> Any explanation they come up with will have to be part of our
>> explanation
> No, it doesn't.  We cannot be responsible for the internals of
> 3rd-party libraries.

We are not responsible for GnuTLS internals, but for audits we are 
responsible for explaining unusual behaviors. It's like many other 
aspects of libraries that Emacs uses: for example, although we are not 
responsible for ImageMagick internals, if those internals allocate a lot 
of memory and cause Emacs to hang or crash, we are responsible for the 
overall behavior.

>>>>       But where we need to seed our own PRNG, we better had a good idea of
>>>>       what we do and what kind of randomness we get.
>>>>
>>>> Any worries we might have about GnuTLS's randomness apply with equal force to /dev/urandom's. After all, /dev/urandom is not guaranteed to be random.
>>> No, /dev/urandom is random enough for our purposes.
>> In that case GnuTLS's nonce generator is random enough for our purposes,
>> and we have a good idea of what kind of randomness we get.
> Today, we do.  Tomorrow, it's anyone's guess.  Unless we audit their
> code all the time.  Why should we?

We never really needed to audit the GnuTLS code in the first place. We 
went into it only because of concerns that GnuTLS might not do the right 
thing, due to lack of understanding about GnuTLS. These concerns have 
been addressed, and there should be no need to keep revisiting the issue.

> That trust needs now to be ascertained even if we don't use secure 
> communications from Emacs.

Sorry, I'm not following. If the point is that GnuTLS should be trusted 
for secure communications (which is relatively complicated) but not for 
generating nonces (which is relatively simple) then I don't see a sound 
basis for this worry. If GnuTLS can't be trusted for simple building 
blocks, why trust it for complex things? Especially when the complex 
things are based on the simple building blocks?

>>> We have what we need; calling gnutls_rnd changes nothing in this
>>> regard. It's just a more complex way of issuing the same system calls.
>> They are not the same system calls. If they were the same, you would be
>> right and we shouldn't bother with GnuTLS here. They are different
>> sequences of system calls, and the sequence that uses GnuTLS lessens
>> entropy consumption and simplifies audits.
> I've read the code and saw no differences.

You can try running it both ways under "strace" in GNU/Linux. You should 
see different sequences of system calls involving /dev/urandom. That's 
what happened for me.

>> This is just a minor issue, one that has been blown all out of
>> proportion.
> This is unfair: if this minor issue was important enough for you to
> make those changes, then objecting to them cannot be considered
> "blowing it all out of proportion".

Yes, a simple objection to a minor change would not blow it all out of 
proportion. But this long thread is more than that.

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

[Glenn Morris]

Andy Moreton wrote:

>  - broke all builds configured with "--without-gnutls"

AFAICS, you are mistaken.

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

[Andy Moreton]

On Tue 19 Jan 2016, Glenn Morris wrote:

> Andy Moreton wrote:
>
>>  - broke all builds configured with "--without-gnutls"
>
> AFAICS, you are mistaken.

I may well be, but please explain.
src/sysdep.c (at emacs-25 commit c5ee6de21d4b) contains:

#include "gnutls.h"
#if 0x020c00 <= GNUTLS_VERSION_NUMBER && !defined WINDOWSNT
# include <gnutls/crypto.h>
#else
# define emacs_gnutls_global_init() Qnil
# define gnutls_rnd(level, data, len) (-1)
#endif

How can this build properly on a non-Windows system which does not
contain an installed "gnutls.h" header ?

    AndyM
 

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

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Wed, 20 Jan 2016 14:06:42 +0000
> 
> On Tue 19 Jan 2016, Glenn Morris wrote:
> 
> > Andy Moreton wrote:
> >
> >>  - broke all builds configured with "--without-gnutls"
> >
> > AFAICS, you are mistaken.
> 
> I may well be, but please explain.
> src/sysdep.c (at emacs-25 commit c5ee6de21d4b) contains:
> 
> #include "gnutls.h"
> #if 0x020c00 <= GNUTLS_VERSION_NUMBER && !defined WINDOWSNT
> # include <gnutls/crypto.h>
> #else
> # define emacs_gnutls_global_init() Qnil
> # define gnutls_rnd(level, data, len) (-1)
> #endif
> 
> How can this build properly on a non-Windows system which does not
> contain an installed "gnutls.h" header ?

That's src/gnutls.h header that comes with Emacs sources.

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

[Andy Moreton]

On Wed 20 Jan 2016, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Wed, 20 Jan 2016 14:06:42 +0000
>> 
>> On Tue 19 Jan 2016, Glenn Morris wrote:
>> 
>> > Andy Moreton wrote:
>> >
>> >>  - broke all builds configured with "--without-gnutls"
>> >
>> > AFAICS, you are mistaken.
>> 
>> I may well be, but please explain.
>> src/sysdep.c (at emacs-25 commit c5ee6de21d4b) contains:
>> 
>> #include "gnutls.h"
>> #if 0x020c00 <= GNUTLS_VERSION_NUMBER && !defined WINDOWSNT
>> # include <gnutls/crypto.h>
>> #else
>> # define emacs_gnutls_global_init() Qnil
>> # define gnutls_rnd(level, data, len) (-1)
>> #endif
>> 
>> How can this build properly on a non-Windows system which does not
>> contain an installed "gnutls.h" header ?
>
> That's src/gnutls.h header that comes with Emacs sources.

Thanks - I knew it had to be something obvious. Sorry for the noise.

    AndyM