Page MenuHome GnuPG
Create Task
Maniphest T403

Public keyring cache not being updated correctly under Windows
Closed, ResolvedPublic
Actions

Description

Release: gnupg-w32cli-1.4.0a.zip

Environment

Intel Celeron 4A 2.4Ghz, 256Mb RAM, Windows 2000 Pro SP4, MSVCRT.DLL 6.1.9844.0

Description

When doing a "gpg --list-keys" for the first time after having just altered the trust setting of a public key, gpg will produce the following output:

>gpg --list-keys
F:/Documents and Settings/user/Application Data/GnuPG\pubring.gpg

gpg: checking the trustdb
gpg: renaming `F:/Documents and Settings/user/Application Data/GnuPG\pubring.gpg
' to `F:/Documents and Settings/user/Application Data/GnuPG\pubring.bak' failed:

Permission denied

gpg: failed to rebuild keyring cache: file rename error
gpg: 3 marginal(s) needed, 1 complete(s) needed, PGP trust model
gpg: depth: 0 valid: 2 signed: 0 trust: 0-, 0q, 0n, 0m, 0f, 2u
<normal key list followed and was suppressed>

Subsequent executions of "gpg --list-keys" will not suffer from this problem, until the trust setting of some public key is altered again (in which case the problem will occur the first time "gpg --list-keys" is executed after the change).

Debugging showed the problem occurs inside function rename_tmp_file(), at keyring.c:1229:
1226:#if defined(HAVE_DOSISH_SYSTEM) || defined(riscos)
1227: remove (bakfname);
1228:#endif
1229: if (rename (fname, bakfname) )
1230: {
1231: log_error ("renaming %s' to %s' failed: %s\n",

A full backtrace from gdb at this point:
#0 rename_tmp_file (

bakfname=0x3fdd60 "F:/Documents and Settings/user/Application Data/GnuPG\\pu

bring.bak",

tmpfname=0x3fddc8 "F:/Documents and Settings/user/Application Data/GnuPG\\pu

bring.tmp",

fname=0x3fc30c "F:/Documents and Settings/user/Application Data/GnuPG\\pubri

ng.gpg", secret=0) at keyring.c:1229
#1 0x00414344 in keyring_rebuild_cache (token=0x3fc2f8, noisy=0)

at keyring.c:1447

#2 0x00411a5b in keydb_rebuild_caches (noisy=0) at keydb.c:683
#3 0x0045c467 in validate_keys (interactive=0) at trustdb.c:1936
#4 0x0045adc1 in check_trustdb_stale () at trustdb.c:1024
#5 0x0042bde0 in list_keyblock_print (keyblock=0xbd6230, secret=0, fpr=0,

opaque=0x0) at keylist.c:711

#6 0x0042da51 in list_keyblock (keyblock=0xbd6230, secret=0, fpr=0,

opaque=0x0) at keylist.c:1364

#7 0x0042b0a9 in list_all (secret=0) at keylist.c:399
#8 0x0042a68b in public_key_list (list=0x0) at keylist.c:99
#9 0x004065d7 in main (argc=0, argv=0x3f25f8) at g10.c:3201

The rename() fails because there is still an open handle to fname (that is, to %HOMEDRIVE%%HOMEPATH%/GnuPG\pubring.gpg). Unlike in *nix, it is not possible to rename an open file in Windows. The file handle is _not_ being closed by the "invalidate close caches" block a few lines above (still inside rename_tmp_file()):

1218: /* invalidate close caches*/
1219: iobuf_ioctl (NULL, 2, 0, (char*)tmpfname );
1220: iobuf_ioctl (NULL, 2, 0, (char*)bakfname );
1221: iobuf_ioctl (NULL, 2, 0, (char*)fname );

This is because, at that point, the io buffer associated with the file handle has not been iobuf_close()'ed. In fact, it seems to never be iobuf_close()'ed, as the file handle associated with it remains open until the end of the program execution. This io buffer is opened inside prepare_search(), at keyring.c:659:

658: hd->current.eof = 0;
659: hd->current.iobuf = iobuf_open (hd->current.kr->fname);
660: if (!hd->current.iobuf)
661: {
662: log_error(_("can't open `%s'\n"), hd->current.kr->fname );

A full backtrace at this point:
#0 prepare_search (hd=0x3fc228) at keyring.c:659
#1 0x00413139 in keyring_search (hd=0x3fc228, desc=0x22fbf0, ndesc=1,

descindex=0x0) at keyring.c:916

#2 0x00411bda in keydb_search2 (hd=0x3fc370, desc=0x22fbf0, ndesc=1,

descindex=0x0) at keydb.c:740

#3 0x00411c5e in keydb_search_first (hd=0x3fc370) at keydb.c:760
#4 0x0042af84 in list_all (secret=0) at keylist.c:370
#5 0x0042a68b in public_key_list (list=0x0) at keylist.c:99
#6 0x004065d7 in main (argc=0, argv=0x3f25f8) at g10.c:3201

As can be noted in the first backtrace, the instance of rename_tmp_file() that fails is being called from inside keyring_rebuild_cache(), at keyring.c:1447:

rc = lastresname? rename_tmp_file (bakfilename, tmpfilename,
                                   lastresname, 0) : 0;
m_free (tmpfilename);  tmpfilename = NULL;
m_free (bakfilename);  bakfilename = NULL;

with bakfilename == "%HOMEDRIVE%%HOMEPATH%/GnuPG\pubring.bak", tmpfilename == "%HOMEDRIVE%%HOMEPATH%/GnuPG\pubring.tmp" and lastresname == "%HOMEDRIVE%%HOMEPATH%/GnuPG\pubring.gpg". rename_tmp_file() will fail when trying to rename pubring.gpg to pubring.bak (because pubring.gpg is still open, as noted above), causing it to error out with G10ERR_RENAME_FILE (keyring.c:1233) and _not_ renaming the new pubring.tmp to pubring.gpg. The old pubring.gpg is, therefore, _not_ updated with the new version from pubring.tmp.

How To Repeat

  1. Alter the trust setting of a given key, by running "gpg --edit-key xxxxxx" then using the "trust" command then the "quit" command.
  2. Run "gpg --list-keys", and the problem will manifestate.

Fix

I've not studied the code to sufficient detail to suggest a working solution, so shall only offer some ideas.

The most direct way would seem to be by issuing an iobuf_close() on the io buffer opened inside prepare_search(), somewhere before entering rename_tmp_file(). This way, the handle associated with that buffer will be closed along with the other previously iobuf_close()'ed ones inside rename_tmp_file(), by the iobuf_ioctl() at keyring.c:1221.

Perhaps some restructuring of the higher level callers (list_all() perhaps? would seem to be the lowest level common ground between the two call trees) would allow for this. Or perhaps placing the iobuf_close() at a lower level, wherever that io buffer ceases to be necessary.

Another idea might be to just move the backup portion to some place else in the code - which may not make much sense especially if later in the code we will be changing the original file and earlier on we don't know we want to back it up yet.

Yet another idea might be to wrap the affected rename_tmp_file() call with a quick and dirty hack that closes the file handle before calling the function, then reopens it and seeks to the proper location inside it (or something to that effect using the iobuf layer).

Release Note

Fixed in CVS; there will be a 1.4.1rc RSN

Event Timeline

ilugo added projects: gnupg, Bug Report.Jan 10 2005, 2:47 AM
• werner added a subscriber: • werner.Jan 18 2005, 11:47 AM

Fixed

• werner added a comment.Jan 18 2005, 11:47 AM

Thanks for the very good and detailed bug report. I really appreciated it.

Fortunately the solution is pretty easy:

  • keylist.c 21 Dec 2004 04:19:03 -0000 1.93

+++ keylist.c 18 Jan 2005 09:51:58 -0000 1.94
@@ -95,6 +95,13 @@

  printf("\n");
}

+ /* We need to do the stale check right here because it might need to
+ update the keyring while we already have the keyring open. This
+ is very bad for W32 because of a sharing violation. For real OSes
+ it might lead to false results if we are later listing a keyring
+ which is associated with the inode of a deleted file. */
+ check_trustdb_stale ();
+

if( !list )
  list_all(0);
else

@@ -104,6 +111,8 @@
void
secret_key_list( STRLIST list )
{
+ check_trustdb_stale ();
+

if( !list )
   list_all(1);
else  /* List by user id */
• werner added a project: Restricted Project.Jan 18 2005, 11:47 AM
• werner added a comment.Jan 27 2005, 12:24 PM

Got report from the Enigmail folks that the snapshot I
provided to them really fixed this bug.

• werner closed this task as Resolved.Jan 27 2005, 12:24 PM
• werner removed a project: Restricted Project.