Page MenuHome GnuPG
Create Task
Maniphest T1439

Windows: race condition on random_seed
Closed, ResolvedPublic
Actions

Description

gpg sporadically fails with a fatal error accessing the random_seed file if
another gpg process accesses that file at the same time. Here's a Cygwin bash
script to reproduce the problem:

$ cat rungpg.sh
#!/bin/bash
i=0
while test $i -lt $1 ; do

echo test | '/c/Program Files (x86)/GNU/GnuPG-1/gpg.exe' --batch --yes

-c --passphrase-fd 0 -o NUL: NUL:

i=$(( $i + 1 ))

done
$ time (./rungpg.sh 10000 & ./rungpg.sh 10000 & ./rungpg.sh 10000 & ./rungpg.sh
10000 & wait)
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: fatal: 'C:/Dokumente und
Einstellungen/jechternach/Anwendungsdaten/GnuPG\random_seed' ist unlesbar: No
such file or directory
secmem usage: 2496/2496 bytes in 8/8 blocks of pool 2496/32768
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: fatal: 'C:/Dokumente und
Einstellungen/jechternach/Anwendungsdaten/GnuPG\random_seed' ist unlesbar: No
such file or directory
secmem usage: 2496/2496 bytes in 8/8 blocks of pool 2496/32768
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: fatal: 'C:/Dokumente und
Einstellungen/jechternach/Anwendungsdaten/GnuPG\random_seed' ist unlesbar: No
such file or directory
secmem usage: 2496/2496 bytes in 8/8 blocks of pool 2496/32768
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: fatal: 'C:/Dokumente und
Einstellungen/jechternach/Anwendungsdaten/GnuPG\random_seed' ist unlesbar: No
such file or directory
secmem usage: 2496/2496 bytes in 8/8 blocks of pool 2496/32768
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer
gpg: Hinweis: 'random_seed'-Datei ist leer

real 23m0.147s
user 5m38.009s
sys 20m13.843s
$ '/c/Program Files (x86)/GNU/GnuPG-1/gpg.exe' --version
gpg (GnuPG) 1.4.12
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Home: C:/Dokumente und Einstellungen/jechternach/Anwendungsdaten/GnuPG
Unterstützte Verfahren:
Öff. Schlüssel: RSA, RSA-E, RSA-S, ELG-E, DSA
Verschlü.: 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH, CAMELLIA128,

CAMELLIA192, CAMELLIA256

Hash: MD5, SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224
Komprimierung: nicht komprimiert, ZIP, ZLIB, BZIP2

This problem was fixed for Unix-like systems by adding file locking to
cipher/random.c, but file locking is also necessary on Windows. Maybe random.c
assumes that open() implicitly locks the file on Windows, but Process Monitor
shows that the random_seed file is opened with FILE_SHARE_READ |
FILE_SHARE_WRITE flags behind the scenes and these flags turn off implicit locking.

BTW, error handling in read_seed_file() after read() isn't quite right: Short
reads (caused by a truncated random_seed file) are treated like read errors. I
guess that this has caused the "No such file or directory" error messages shown
above because they look bogus -- the random_seed file should always be present
because update_random_seed_file() only truncates it but doesn't delete it.

Details

Due Date
Oct 31 2012, 1:00 AM
Version
all

Related Objects

Event Timeline

jegrp added projects: Windows, Windows 32, Bug Report.Aug 29 2012, 6:03 PM
jegrp set Version to 1.4.12.
jegrp added a subscriber: jegrp.
• werner lowered the priority of this task from High to Normal.Sep 17 2012, 2:59 PM
• werner added a project: gnupg.
• werner added a subscriber: • werner.

This is known but a problem from a security POV. The seed file is a cache. If
the seed file can't be read a new one is created. Right, there might be a
performance issue but at least on Windows this is not as severe as on certain
Linux systems.

The non-locking read is on purpose - if it works: okay. Otherwise we
re-generate a seed file. Yes, we could do the file locking and iirc, we do this
in libgcrypt (GnuPG-2).

The error on short reads is also on purpose. We want those 600 bytes at once
and nothing else. If another process is writing the seed file we may see the
short reads. But in this case there is no clear answer waht to do - thus we
assume the "no seed file" case.

jegrp added a comment.Sep 18 2012, 7:52 PM

This is known but a problem from a security POV.

I suppose you mean "NOT a problem"? I think it might be a problem in
opportunistic encryption scenarios if gpg encryption failures caused by
random_seed access conflicts are ignored like failures caused by missing keys.
But usually it's just a nuisance like any other randomly failing program.

The non-locking read is on purpose - if it works: okay. Otherwise we
re-generate a seed file.

I see that the code tries to tolerate access conflicts, but there's still a race
condition if the random_seed file is truncated between fstat() and read(). The
read() error handling is incomplete. Maybe this pseudo-patch explains best what
I mean:

diff -ru gnupg-1.4.12/cipher/random.c gnupg-1.4.12/cipher/random.c

  • gnupg-1.4.12/cipher/random.c 2012-01-24 09:45:41.000000000 +0100

+++ gnupg-1.4.12/cipher/random.c 2012-09-18 19:47:54.449578800 +0200
@@ -492,11 +492,17 @@

    do {
	n = read( fd, buffer, POOLSIZE );
    } while( n == -1 && errno == EINTR );
  • if( n != POOLSIZE ) {

+ if( n == -1 ) {

	log_fatal(_("can't read `%s': %s\n"), seed_file_name,strerror(errno) );
	close(fd);
	return 0;
    }

+ else if ( n == 0 ) {
+ ... handle like sb.st_size == 0
+ }
+ else if ( n != POOLSIZE ) {
+ ... handle like sb.st_size != POOLSIZ
+ }

     close(fd);

Yes, we could do the file locking

Proper locking would be the ideal solution, but a better read() error handling
would already be sufficient to avoid the sporadic fatal errors on random_seed
accesses.

and iirc, we do this in libgcrypt (GnuPG-2).

I'm just checking this and ... sorry, no. The gpg2.exe from Gpg4win 2.1.0 shows
the very same error:

note: random_seed file is empty
note: random_seed file is empty
Fatal: can't read `C:/Dokumente und
Einstellungen/jechternach/Anwendungsdaten/GnuPG /random_seed': No such file or
directory

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.
note: random_seed file is empty

• werner added a comment.Sep 26 2012, 3:15 PM

Yeah sure, I meant "NOT a problem".

Yes I know what you mean. But without locking you will never be able
to get it right.

As you noted there is actually a problem with Libgcrypt under
Windows. In Libgcrypt we lock the seed file and thus the fatal error
is the right thing to do. But.....

  #ifdef __GCC__
  #warning Check whether we can lock on Windows.
  #endif
  #if LOCK_SEED_FILE

Thus we should implement locking for Windows. The problem here is
that there is no portable advisory locking in Windows. And frankly our
fcntl locking approach does not work on all Unices either and worse it
does not work if the home partition is on certain remote file systems.

The only solution I see is to employ the new dotlock code from GnuPG
here. It is slower than fcntl locking but very portable.

I don't think we will fix this in gpg 1.4.

• werner removed projects: gnupg, Windows 32, Windows.Sep 26 2012, 3:15 PM
• werner added a project: libgcrypt.
• werner set Due Date to Oct 31 2012, 1:00 AM.
• werner changed Version from 1.4.12 to all.
jegrp added a comment.Sep 26 2012, 5:52 PM

I'm not sure what you mean by "no portable advisory locking in Windows".
There's LockFile()/UnlockFile() as used in the new dotlock code. Would it be a
problem if the locks on random_seed were mandatory locks? If so, then using the
dotlock code is certainly the shortest path to a perfect solution.

Regarding gpg 1.4, however: The reason why I opened this bug is that gpg
randomly fails to encrypt what it was told to encrypt. This is as bad as if it
would be randomly crashing. I don't mind an occasional "note: random_seed file
is empty" message nor a performance penalty from needlessly re-generating the
seed file. Modifying the error handling after read() as I tried to outline in
my previous message ought to be enough to get rid of the random fatal errors.
Do you consider such a change suitable for gpg 1.4?

• werner added a project: In Progress.Nov 7 2012, 5:21 PM

Yes, mandatory locks are a problem. LockFile works on a separate lockfile to
implement such an advisory locking. If we would lock the random_seed file
directly reads or writes will fail and we won't be able to lock the creation of
that file.

You suggestion for 1.4. makes sense.

• werner added a project: Restricted Project.Nov 7 2012, 6:13 PM
• werner removed a project: In Progress.

I have implemented your suggestion for 1.4. Check out commit b1abc01 of GnuPG.

• werner removed a project: Restricted Project.Nov 7 2012, 6:15 PM
• werner added a project: In Progress.
jegrp added a comment.Nov 9 2012, 6:48 PM

I'm not able to reproduce the problem with commit b1abc01. It was reproducible
with the preceding revision (commit a74f05c), so I can confirm that b1abc01 has
fixed it.

• werner added a project: Restricted Project.Nov 13 2012, 11:01 AM
• werner removed a project: In Progress.
• werner closed this task as Resolved.Mar 18 2013, 9:23 AM
• werner claimed this task.
• werner removed a project: Restricted Project.