From: aheinecke (Andre Heinecke)
Sent: Montag, 28. Januar 2019 19:25

fwiw. Your patch is beautiful in which it follows our coding style and
debug output. I'm confident that we will accept it but currently I have
to read up on Job's a bit.

Is there a way I could help you with this? This issue is hampering adoption
of GnuPG 2 here.

--

Jan Echternach

I've developed a simple patch that sets the CREATE_BREAKAWAY_FROM_JOB flag when creating a new background process. This flag requires a special permission on the job object, which is tested first. This means that the patch only works if the parent process sets JOB_OBJECT_LIMIT_BREAKAWAY_OK on the job object, otherwise the behavior should be as without the patch.

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.

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?

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