From e725c4d65335d18dea6b855726ee7c57afd4a60a Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 27 Sep 2017 17:18:55 +0200 Subject: [PATCH 01/21] doc: Make --check-sigs more prominent. -- It seems people are using --list-sigs instead of --check-sigs and do not realize that the signatures are not checked at all. We better highlight the use of --check-sigs to avoid this UI problem. Suggested-by: Andrew Gallagher Signed-off-by: Werner Koch --- doc/gpg.texi | 81 +++++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/doc/gpg.texi b/doc/gpg.texi index b6a9b2d70..b14cb371b 100644 --- a/doc/gpg.texi +++ b/doc/gpg.texi @@ -309,43 +309,36 @@ the key using the command @option{--export-secret-subkeys}). A @code{>} after these tags indicate that the key is stored on a smartcard. See also @option{--list-keys}. -@item --list-signatures -@opindex list-signatures -@itemx --list-sigs -@opindex list-sigs -Same as @option{--list-keys}, but the signatures are listed too. -This command has the same effect as -using @option{--list-keys} with @option{--with-sig-list}. - -For each signature listed, there are several flags in between the "sig" -tag and keyid. These flags give additional information about each -signature. From left to right, they are the numbers 1-3 for certificate -check level (see @option{--ask-cert-level}), "L" for a local or -non-exportable signature (see @option{--lsign-key}), "R" for a -nonRevocable signature (see the @option{--edit-key} command "nrsign"), -"P" for a signature that contains a policy URL (see -@option{--cert-policy-url}), "N" for a signature that contains a -notation (see @option{--cert-notation}), "X" for an eXpired signature -(see @option{--ask-cert-expire}), and the numbers 1-9 or "T" for 10 and -above to indicate trust signature levels (see the @option{--edit-key} -command "tsign"). - @item --check-signatures @opindex check-signatures @itemx --check-sigs @opindex check-sigs -Same as @option{--list-signatures}, but the signatures are verified. Note -that for performance reasons the revocation status of a signing key is -not shown. -This command has the same effect as +Same as @option{--list-keys}, but the key signatures are verified and +listed too. Note that for performance reasons the revocation status +of a signing key is not shown. This command has the same effect as using @option{--list-keys} with @option{--with-sig-check}. -The status of the verification is indicated by a flag directly following -the "sig" tag (and thus before the flags described above for -@option{--list-signatures}). A "!" indicates that the signature has been -successfully verified, a "-" denotes a bad signature and a "%" is used -if an error occurred while checking the signature (e.g. a non supported -algorithm). +The status of the verification is indicated by a flag directly +following the "sig" tag (and thus before the flags described below. A +"!" indicates that the signature has been successfully verified, a "-" +denotes a bad signature and a "%" is used if an error occurred while +checking the signature (e.g. a non supported algorithm). Signatures +where the public key is not availabale are not listed; to see their +keyids the command @option{--list-sigs} can be used. + +For each signature listed, there are several flags in between the +signature status flag and keyid. These flags give additional +information about each key signature. From left to right, they are +the numbers 1-3 for certificate check level (see +@option{--ask-cert-level}), "L" for a local or non-exportable +signature (see @option{--lsign-key}), "R" for a nonRevocable signature +(see the @option{--edit-key} command "nrsign"), "P" for a signature +that contains a policy URL (see @option{--cert-policy-url}), "N" for a +signature that contains a notation (see @option{--cert-notation}), "X" +for an eXpired signature (see @option{--ask-cert-expire}), and the +numbers 1-9 or "T" for 10 and above to indicate trust signature levels +(see the @option{--edit-key} command "tsign"). + @item --locate-keys @opindex locate-keys @@ -360,7 +353,7 @@ be used to locate a key. Only public keys are listed. List all keys (or the specified ones) along with their fingerprints. This is the same output as @option{--list-keys} but with the additional output of a line with the fingerprint. May also be -combined with @option{--list-signatures} or @option{--check-signatures}. If this +combined with @option{--check-signatures}. If this command is given twice, the fingerprints of all secondary keys are listed too. This command also forces pretty printing of fingerprints if the keyid format has been set to "none". @@ -1254,7 +1247,7 @@ Assume "no" on most questions. @opindex list-options This is a space or comma delimited string that gives options used when listing keys and signatures (that is, @option{--list-keys}, -@option{--list-signatures}, @option{--list-public-keys}, +@option{--check-signatures}, @option{--list-public-keys}, @option{--list-secret-keys}, and the @option{--edit-key} functions). Options can be prepended with a @option{no-} (after the two dashes) to give the opposite meaning. The options are: @@ -1263,7 +1256,7 @@ give the opposite meaning. The options are: @item show-photos @opindex list-options:show-photos - Causes @option{--list-keys}, @option{--list-signatures}, + Causes @option{--list-keys}, @option{--check-signatures}, @option{--list-public-keys}, and @option{--list-secret-keys} to display any photo IDs attached to the key. Defaults to no. See also @option{--photo-viewer}. Does not work with @option{--with-colons}: @@ -1279,7 +1272,7 @@ give the opposite meaning. The options are: @item show-policy-urls @opindex list-options:show-policy-urls - Show policy URLs in the @option{--list-signatures} or @option{--check-signatures} + Show policy URLs in the @option{--check-signatures} listings. Defaults to no. @item show-notations @@ -1289,11 +1282,11 @@ give the opposite meaning. The options are: @opindex list-options:show-std-notations @opindex list-options:show-user-notations Show all, IETF standard, or user-defined signature notations in the - @option{--list-signatures} or @option{--check-signatures} listings. Defaults to no. + @option{--check-signatures} listings. Defaults to no. @item show-keyserver-urls @opindex list-options:show-keyserver-urls - Show any preferred keyserver URL in the @option{--list-signatures} or + Show any preferred keyserver URL in the @option{--check-signatures} listings. Defaults to no. @item show-uid-validity @@ -1316,7 +1309,7 @@ give the opposite meaning. The options are: @item show-sig-expire @opindex list-options:show-sig-expire - Show signature expiration dates (if any) during @option{--list-signatures} or + Show signature expiration dates (if any) during @option{--check-signatures} listings. Defaults to no. @item show-sig-subpackets @@ -1325,7 +1318,7 @@ give the opposite meaning. The options are: optional argument list of the subpackets to list. If no argument is passed, list all subpackets. Defaults to no. This option is only meaningful when using @option{--with-colons} along with - @option{--list-signatures} or @option{--check-signatures}. + @option{--check-signatures}. @end table @@ -3224,6 +3217,16 @@ verification is not needed. Print key listings delimited by colons (like @option{--with-colons}) and print the public key data. +@item --list-signatures +@opindex list-signatures +@itemx --list-sigs +@opindex list-sigs +Same as @option{--list-keys}, but the signatures are listed too. This +command has the same effect as using @option{--list-keys} with +@option{--with-sig-list}. Note that in contrast to +@option{--check-signatures} the key signatures are not verified. + + @item --fast-list-mode @opindex fast-list-mode Changes the output of the list commands to work faster; this is achieved From b509d81cab030cca6abf0d878e1fc884eda344e6 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 28 Sep 2017 14:10:12 +0200 Subject: [PATCH 02/21] gpg: Workaround for junk after --trusted-key. * g10/trust.c (register_trusted_key): Cut off everthing starting as a hash sign. -- This problem is fallout from commit f99830b72812395da5451152bdd2f2d90a7cb7fb which fixes GnuPG-bug-id: 1206 The same could happen with other options taking keyids but we won't change that because a trailing '#' does not indicate a comment. So this is really only a workaround and eventually we will deprecate --trusted-key anyway or require a fingerprint as a value. Signed-off-by: Werner Koch --- g10/trust.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/g10/trust.c b/g10/trust.c index ee6078b5a..6d4f0e74b 100644 --- a/g10/trust.c +++ b/g10/trust.c @@ -66,6 +66,26 @@ register_trusted_key (const char *string) #ifdef NO_TRUST_MODELS (void)string; #else + + /* Some users have conf files with entries like + * trusted-key 0x1234567812345678 # foo + * That is obviously wrong. Before fixing bug#1206 trailing garbage + * on a key specification if was ignored. We detect the above use case + * here and cut off the junk-looking-like-a comment. */ + if (strchr (string, '#')) + { + char *buf; + + buf = xtrystrdup (string); + if (buf) + { + *strchr (buf, '#') = 0; + tdb_register_trusted_key (buf); + xfree (buf); + return; + } + } + tdb_register_trusted_key (string); #endif } From 1bf5cbd3ef01b7f5fdcfa30c882047b924dcf3f0 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 17 Oct 2017 20:56:55 +0200 Subject: [PATCH 03/21] sm: Fix colon listing of fields > 12 in crt records. * sm/keylist.c (print_capabilities): Move colon printing ... (list_cert_colon): to here. -- Fixes-commit: 7af008bfe1641938a6c2c995cb065829fa05a693 Signed-off-by: Werner Koch --- sm/keylist.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sm/keylist.c b/sm/keylist.c index 24c86e18a..9997da812 100644 --- a/sm/keylist.c +++ b/sm/keylist.c @@ -289,8 +289,6 @@ print_capabilities (ksba_cert_t cert, estream_t fp) es_putc ('S', fp); if ((use & KSBA_KEYUSAGE_KEY_CERT_SIGN)) es_putc ('C', fp); - - es_putc (':', fp); } @@ -503,6 +501,7 @@ list_cert_colon (ctrl_t ctrl, ksba_cert_t cert, unsigned int validity, es_putc (':', fp); /* Field 12, capabilities: */ print_capabilities (cert, fp); + es_putc (':', fp); /* Field 13, not used: */ es_putc (':', fp); /* Field 14, not used: */ From 752cae6dd2ee8982a34c796a3f168ae538f7938c Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 18 Oct 2017 13:09:47 +0200 Subject: [PATCH 04/21] gpg: Simplify keydb handling of the main import function. * g10/import.c (import_keys_internal): Return gpg_error_t instead of int. Change var names. (import_keys_es_stream): Ditto. (import_one): Ditto. Use a single keydb_new and simplify the use of of keydb_release. -- Note that this opens a keydb handle before we call get_pubkey_byfprint_fast which internally uses another key db handle. A further patch will cleanup this double use. Note that we also disable the keydb caching for the insert case. The s/int/gpg_error_t/ has been done while checking the call chains of the import functions and making sure that gpg_err_code is always used. Signed-off-by: Werner Koch --- g10/import.c | 159 +++++++++++++++++++++++++-------------------------- g10/main.h | 2 +- 2 files changed, 80 insertions(+), 81 deletions(-) diff --git a/g10/import.c b/g10/import.c index 5b55f8f82..e7850023e 100644 --- a/g10/import.c +++ b/g10/import.c @@ -102,7 +102,7 @@ static int import (ctrl_t ctrl, static int read_block (IOBUF a, int with_meta, PACKET **pending_pkt, kbnode_t *ret_root, int *r_v3keys); static void revocation_present (ctrl_t ctrl, kbnode_t keyblock); -static int import_one (ctrl_t ctrl, +static gpg_error_t import_one (ctrl_t ctrl, kbnode_t keyblock, struct import_stats_s *stats, unsigned char **fpr, size_t *fpr_len, @@ -436,7 +436,7 @@ read_key_from_file (ctrl_t ctrl, const char *fname, kbnode_t *r_keyblock) * * Key revocation certificates have special handling. */ -static int +static gpg_error_t import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames, import_stats_t stats_handle, unsigned char **fpr, size_t *fpr_len, @@ -445,7 +445,7 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames, int origin, const char *url) { int i; - int rc = 0; + gpg_error_t err = 0; struct import_stats_s *stats = stats_handle; if (!stats) @@ -453,8 +453,8 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames, if (inp) { - rc = import (ctrl, inp, "[stream]", stats, fpr, fpr_len, options, - screener, screener_arg, origin, url); + err = import (ctrl, inp, "[stream]", stats, fpr, fpr_len, options, + screener, screener_arg, origin, url); } else { @@ -478,14 +478,14 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames, log_error (_("can't open '%s': %s\n"), fname, strerror (errno)); else { - rc = import (ctrl, inp2, fname, stats, fpr, fpr_len, options, + err = import (ctrl, inp2, fname, stats, fpr, fpr_len, options, screener, screener_arg, origin, url); iobuf_close (inp2); /* Must invalidate that ugly cache to actually close it. */ iobuf_ioctl (NULL, IOBUF_IOCTL_INVALIDATE_CACHE, 0, (char*)fname); - if (rc) + if (err) log_error ("import from '%s' failed: %s\n", - fname, gpg_strerror (rc) ); + fname, gpg_strerror (err) ); } if (!fname) break; @@ -507,7 +507,7 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames, if (!(options & IMPORT_FAST)) check_or_update_trustdb (ctrl); - return rc; + return err; } @@ -521,7 +521,7 @@ import_keys (ctrl_t ctrl, char **fnames, int nnames, } -int +gpg_error_t import_keys_es_stream (ctrl_t ctrl, estream_t fp, import_stats_t stats_handle, unsigned char **fpr, size_t *fpr_len, @@ -529,23 +529,23 @@ import_keys_es_stream (ctrl_t ctrl, estream_t fp, import_screener_t screener, void *screener_arg, int origin, const char *url) { - int rc; + gpg_error_t err; iobuf_t inp; inp = iobuf_esopen (fp, "rb", 1); if (!inp) { - rc = gpg_error_from_syserror (); - log_error ("iobuf_esopen failed: %s\n", gpg_strerror (rc)); - return rc; + err = gpg_error_from_syserror (); + log_error ("iobuf_esopen failed: %s\n", gpg_strerror (err)); + return err; } - rc = import_keys_internal (ctrl, inp, NULL, 0, stats_handle, + err = import_keys_internal (ctrl, inp, NULL, 0, stats_handle, fpr, fpr_len, options, screener, screener_arg, origin, url); iobuf_close (inp); - return rc; + return err; } @@ -1608,7 +1608,7 @@ update_key_origin (kbnode_t keyblock, u32 curtime, int origin, const char *url) * even most error messages are suppressed. ORIGIN is the origin of * the key (0 for unknown) and URL the corresponding URL. */ -static int +static gpg_error_t import_one (ctrl_t ctrl, kbnode_t keyblock, struct import_stats_s *stats, unsigned char **fpr, size_t *fpr_len, unsigned int options, @@ -1616,6 +1616,7 @@ import_one (ctrl_t ctrl, import_screener_t screener, void *screener_arg, int origin, const char *url) { + gpg_error_t err = 0; PKT_public_key *pk; PKT_public_key *pk_orig = NULL; kbnode_t node, uidnode; @@ -1623,7 +1624,6 @@ import_one (ctrl_t ctrl, byte fpr2[MAX_FINGERPRINT_LEN]; size_t fpr2len; u32 keyid[2]; - int rc = 0; int new_key = 0; int mod_key = 0; int same_key = 0; @@ -1632,6 +1632,7 @@ import_one (ctrl_t ctrl, char pkstrbuf[PUBKEY_STRING_SIZE]; int merge_keys_done = 0; int any_filter = 0; + KEYDB_HANDLE hd = NULL; /* Get the key and print some info about it. */ node = find_kbnode( keyblock, PKT_PUBLIC_KEY ); @@ -1791,7 +1792,7 @@ import_one (ctrl_t ctrl, merge_keys_and_selfsig (ctrl, keyblock); merge_keys_done = 1; } - rc = write_keyblock_to_output (keyblock, opt.armor, opt.export_options); + err = write_keyblock_to_output (keyblock, opt.armor, opt.export_options); goto leave; } @@ -1799,37 +1800,37 @@ import_one (ctrl_t ctrl, goto leave; /* Do we have this key already in one of our pubrings ? */ + hd = keydb_new (); + if (!hd) + return gpg_error_from_syserror (); + keydb_disable_caching (hd); pk_orig = xmalloc_clear( sizeof *pk_orig ); - rc = get_pubkey_byfprint_fast (pk_orig, fpr2, fpr2len); - if (rc && gpg_err_code (rc) != GPG_ERR_NO_PUBKEY - && gpg_err_code (rc) != GPG_ERR_UNUSABLE_PUBKEY ) + err = get_pubkey_byfprint_fast (pk_orig, fpr2, fpr2len); + if (err + && gpg_err_code (err) != GPG_ERR_NO_PUBKEY + && gpg_err_code (err) != GPG_ERR_UNUSABLE_PUBKEY ) { if (!silent) log_error (_("key %s: public key not found: %s\n"), - keystr(keyid), gpg_strerror (rc)); + keystr(keyid), gpg_strerror (err)); } - else if ( rc && (opt.import_options&IMPORT_MERGE_ONLY) ) + else if (err && (opt.import_options&IMPORT_MERGE_ONLY) ) { if (opt.verbose && !silent ) log_info( _("key %s: new key - skipped\n"), keystr(keyid)); - rc = 0; + err = 0; stats->skipped_new_keys++; } - else if (rc ) /* Insert this key. */ + else if (err) /* Insert this key. */ { - KEYDB_HANDLE hd; int n_sigs_cleaned, n_uids_cleaned; - hd = keydb_new (); - if (!hd) - return gpg_error_from_syserror (); - - rc = keydb_locate_writable (hd); - if (rc) + err = keydb_locate_writable (hd); + if (err) { - log_error (_("no writable keyring found: %s\n"), gpg_strerror (rc)); - keydb_release (hd); - return GPG_ERR_GENERAL; + log_error (_("no writable keyring found: %s\n"), gpg_strerror (err)); + err = gpg_error (GPG_ERR_GENERAL); + goto leave; } if (opt.verbose > 1 ) log_info (_("writing to '%s'\n"), keydb_get_resource_name (hd) ); @@ -1843,19 +1844,19 @@ import_one (ctrl_t ctrl, * and thus the address of KEYBLOCK won't change. */ if ( !(options & IMPORT_RESTORE) ) { - rc = insert_key_origin (keyblock, origin, url); - if (rc) + err = insert_key_origin (keyblock, origin, url); + if (err) { - log_error ("insert_key_origin failed: %s\n", gpg_strerror (rc)); - keydb_release (hd); - return GPG_ERR_GENERAL; + log_error ("insert_key_origin failed: %s\n", gpg_strerror (err)); + err = gpg_error (GPG_ERR_GENERAL); + goto leave; } } - rc = keydb_insert_keyblock (hd, keyblock ); - if (rc) + err = keydb_insert_keyblock (hd, keyblock ); + if (err) log_error (_("error writing keyring '%s': %s\n"), - keydb_get_resource_name (hd), gpg_strerror (rc)); + keydb_get_resource_name (hd), gpg_strerror (err)); else if (!(opt.import_options & IMPORT_KEEP_OWNERTTRUST)) { /* This should not be possible since we delete the @@ -1868,7 +1869,10 @@ import_one (ctrl_t ctrl, if (non_self) revalidation_mark (ctrl); } + + /* Release the handle and thus unlock the keyring asap. */ keydb_release (hd); + hd = NULL; /* We are ready. */ if (!opt.quiet && !silent) @@ -1890,7 +1894,6 @@ import_one (ctrl_t ctrl, } else /* Merge the key. */ { - KEYDB_HANDLE hd; int n_uids, n_sigs, n_subk, n_sigs_cleaned, n_uids_cleaned; u32 curtime = make_timestamp (); @@ -1903,29 +1906,22 @@ import_one (ctrl_t ctrl, goto leave; } - /* Now read the original keyblock again so that we can use - that handle for updating the keyblock. */ - hd = keydb_new (); - if (!hd) - { - rc = gpg_error_from_syserror (); - goto leave; - } - keydb_disable_caching (hd); - rc = keydb_search_fpr (hd, fpr2); - if (rc ) + /* Now read the original keyblock again so that we can use the + * handle for updating the keyblock. FIXME: Why not let + * get_pubkey_byfprint_fast do that - it fetches the keyblock + * anyway. */ + err = keydb_search_fpr (hd, fpr2); + if (err) { log_error (_("key %s: can't locate original keyblock: %s\n"), - keystr(keyid), gpg_strerror (rc)); - keydb_release (hd); + keystr(keyid), gpg_strerror (err)); goto leave; } - rc = keydb_get_keyblock (hd, &keyblock_orig); - if (rc) + err = keydb_get_keyblock (hd, &keyblock_orig); + if (err) { log_error (_("key %s: can't read original keyblock: %s\n"), - keystr(keyid), gpg_strerror (rc)); - keydb_release (hd); + keystr(keyid), gpg_strerror (err)); goto leave; } @@ -1938,14 +1934,11 @@ import_one (ctrl_t ctrl, clear_kbnode_flags( keyblock_orig ); clear_kbnode_flags( keyblock ); n_uids = n_sigs = n_subk = n_uids_cleaned = 0; - rc = merge_blocks (ctrl, options, keyblock_orig, keyblock, keyid, - curtime, origin, url, - &n_uids, &n_sigs, &n_subk ); - if (rc ) - { - keydb_release (hd); - goto leave; - } + err = merge_blocks (ctrl, options, keyblock_orig, keyblock, keyid, + curtime, origin, url, + &n_uids, &n_sigs, &n_subk ); + if (err) + goto leave; if ((options & IMPORT_CLEAN)) clean_key (ctrl, keyblock_orig, opt.verbose, (options&IMPORT_MINIMAL), @@ -1958,25 +1951,28 @@ import_one (ctrl_t ctrl, * and thus the address of KEYBLOCK won't change. */ if ( !(options & IMPORT_RESTORE) ) { - rc = update_key_origin (keyblock_orig, curtime, origin, url); - if (rc) + err = update_key_origin (keyblock_orig, curtime, origin, url); + if (err) { log_error ("update_key_origin failed: %s\n", - gpg_strerror (rc)); - keydb_release (hd); + gpg_strerror (err)); goto leave; } } mod_key = 1; /* KEYBLOCK_ORIG has been updated; write */ - rc = keydb_update_keyblock (ctrl, hd, keyblock_orig); - if (rc) + err = keydb_update_keyblock (ctrl, hd, keyblock_orig); + if (err) log_error (_("error writing keyring '%s': %s\n"), - keydb_get_resource_name (hd), gpg_strerror (rc) ); + keydb_get_resource_name (hd), gpg_strerror (err)); else if (non_self) revalidation_mark (ctrl); + /* Release the handle and thus unlock the keyring asap. */ + keydb_release (hd); + hd = NULL; + /* We are ready. */ if (!opt.quiet && !silent) { @@ -2025,6 +2021,10 @@ import_one (ctrl_t ctrl, } else { + /* Release the handle and thus unlock the keyring asap. */ + keydb_release (hd); + hd = NULL; + /* Fixme: we do not track the time we last checked a key for * updates. To do this we would need to rewrite even the * keys which have no changes. */ @@ -2041,11 +2041,10 @@ import_one (ctrl_t ctrl, stats->unchanged++; } - - keydb_release (hd); hd = NULL; } leave: + keydb_release (hd); if (mod_key || new_key || same_key) { /* A little explanation for this: we fill in the fingerprint @@ -2094,7 +2093,7 @@ import_one (ctrl_t ctrl, release_kbnode( keyblock_orig ); free_public_key( pk_orig ); - return rc; + return err; } diff --git a/g10/main.h b/g10/main.h index 87417ee37..6c15a2a8d 100644 --- a/g10/main.h +++ b/g10/main.h @@ -354,7 +354,7 @@ gpg_error_t read_key_from_file (ctrl_t ctrl, const char *fname, void import_keys (ctrl_t ctrl, char **fnames, int nnames, import_stats_t stats_hd, unsigned int options, int origin, const char *url); -int import_keys_es_stream (ctrl_t ctrl, estream_t fp, +gpg_error_t import_keys_es_stream (ctrl_t ctrl, estream_t fp, import_stats_t stats_handle, unsigned char **fpr, size_t *fpr_len, unsigned int options, From 8448347b5bdee56e6f9938a93ea92fe4d3c8800c Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 18 Oct 2017 17:52:41 +0200 Subject: [PATCH 05/21] gpg: Improve keydb handling in the main import function. * g10/getkey.c (get_pubkey_byfprint_fast): Factor most code out to ... (get_keyblock_byfprint_fast): .. new function. * g10/import.c (revocation_present): s/int rc/gpg_error_t err/. (import_one): Use get_keyblock_byfprint_fast to get the keyblock and a handle. Remove the now surplus keyblock fetch in the merge branch. Signed-off-by: Werner Koch --- g10/getkey.c | 76 +++++++++++++++++++++++++++++++++++++++++----------- g10/import.c | 64 +++++++++++++++---------------------------- g10/keydb.h | 13 +++++++-- 3 files changed, 94 insertions(+), 59 deletions(-) diff --git a/g10/getkey.c b/g10/getkey.c index 852c53286..5ce580541 100644 --- a/g10/getkey.c +++ b/g10/getkey.c @@ -1828,16 +1828,47 @@ get_pubkey_byfprint (ctrl_t ctrl, PKT_public_key *pk, kbnode_t *r_keyblock, * * Like get_pubkey_byfprint, PK may be NULL. In that case, this * function effectively just checks for the existence of the key. */ -int +gpg_error_t get_pubkey_byfprint_fast (PKT_public_key * pk, const byte * fprint, size_t fprint_len) { - int rc = 0; - KEYDB_HANDLE hd; + gpg_error_t err; KBNODE keyblock; + + err = get_keyblock_byfprint_fast (&keyblock, NULL, fprint, fprint_len, 0); + if (!err) + { + if (pk) + copy_public_key (pk, keyblock->pkt->pkt.public_key); + release_kbnode (keyblock); + } + + return err; +} + + +/* This function is similar to get_pubkey_byfprint_fast but returns a + * keydb handle at R_HD and the keyblock at R_KEYBLOCK. R_KEYBLOCK or + * R_HD may be NULL. If LOCK is set the handle has been opend in + * locked mode and keydb_disable_caching () has been called. On error + * R_KEYBLOCK is set to NULL but R_HD must be released by the caller; + * it may have a value of NULL, though. This allows to do an insert + * operation on a locked keydb handle. */ +gpg_error_t +get_keyblock_byfprint_fast (kbnode_t *r_keyblock, KEYDB_HANDLE *r_hd, + const byte *fprint, size_t fprint_len, int lock) +{ + gpg_error_t err; + KEYDB_HANDLE hd; + kbnode_t keyblock; byte fprbuf[MAX_FINGERPRINT_LEN]; int i; + if (r_keyblock) + *r_keyblock = NULL; + if (r_hd) + *r_hd = NULL; + for (i = 0; i < MAX_FINGERPRINT_LEN && i < fprint_len; i++) fprbuf[i] = fprint[i]; while (i < MAX_FINGERPRINT_LEN) @@ -1846,33 +1877,48 @@ get_pubkey_byfprint_fast (PKT_public_key * pk, hd = keydb_new (); if (!hd) return gpg_error_from_syserror (); + if (r_hd) + *r_hd = hd; - rc = keydb_search_fpr (hd, fprbuf); - if (gpg_err_code (rc) == GPG_ERR_NOT_FOUND) + if (lock) { - keydb_release (hd); - return GPG_ERR_NO_PUBKEY; + keydb_disable_caching (hd); } - rc = keydb_get_keyblock (hd, &keyblock); - keydb_release (hd); - if (rc) + + err = keydb_search_fpr (hd, fprbuf); + if (gpg_err_code (err) == GPG_ERR_NOT_FOUND) { - log_error ("keydb_get_keyblock failed: %s\n", gpg_strerror (rc)); - return GPG_ERR_NO_PUBKEY; + if (!r_hd) + keydb_release (hd); + return gpg_error (GPG_ERR_NO_PUBKEY); + } + err = keydb_get_keyblock (hd, &keyblock); + if (err) + { + log_error ("keydb_get_keyblock failed: %s\n", gpg_strerror (err)); + if (!r_hd) + keydb_release (hd); + return gpg_error (GPG_ERR_NO_PUBKEY); } log_assert (keyblock->pkt->pkttype == PKT_PUBLIC_KEY || keyblock->pkt->pkttype == PKT_PUBLIC_SUBKEY); - if (pk) - copy_public_key (pk, keyblock->pkt->pkt.public_key); - release_kbnode (keyblock); /* Not caching key here since it won't have all of the fields properly set. */ + if (r_keyblock) + *r_keyblock = keyblock; + else + release_kbnode (keyblock); + + if (!r_hd) + keydb_release (hd); + return 0; } + const char * parse_def_secret_key (ctrl_t ctrl) { diff --git a/g10/import.c b/g10/import.c index e7850023e..8dd6b501e 100644 --- a/g10/import.c +++ b/g10/import.c @@ -1618,7 +1618,6 @@ import_one (ctrl_t ctrl, { gpg_error_t err = 0; PKT_public_key *pk; - PKT_public_key *pk_orig = NULL; kbnode_t node, uidnode; kbnode_t keyblock_orig = NULL; byte fpr2[MAX_FINGERPRINT_LEN]; @@ -1800,16 +1799,15 @@ import_one (ctrl_t ctrl, goto leave; /* Do we have this key already in one of our pubrings ? */ - hd = keydb_new (); - if (!hd) - return gpg_error_from_syserror (); - keydb_disable_caching (hd); - pk_orig = xmalloc_clear( sizeof *pk_orig ); - err = get_pubkey_byfprint_fast (pk_orig, fpr2, fpr2len); - if (err - && gpg_err_code (err) != GPG_ERR_NO_PUBKEY - && gpg_err_code (err) != GPG_ERR_UNUSABLE_PUBKEY ) + err = get_keyblock_byfprint_fast (&keyblock_orig, &hd, + fpr2, fpr2len, 1/*locked*/); + if ((err + && gpg_err_code (err) != GPG_ERR_NO_PUBKEY + && gpg_err_code (err) != GPG_ERR_UNUSABLE_PUBKEY) + || !hd) { + /* The !hd above is to catch a misbehaving function which + * returns NO_PUBKEY for failing to allocate a handle. */ if (!silent) log_error (_("key %s: public key not found: %s\n"), keystr(keyid), gpg_strerror (err)); @@ -1823,6 +1821,7 @@ import_one (ctrl_t ctrl, } else if (err) /* Insert this key. */ { + /* Note: ERR can only be NO_PUBKEY or UNUSABLE_PUBKEY. */ int n_sigs_cleaned, n_uids_cleaned; err = keydb_locate_writable (hd); @@ -1892,45 +1891,26 @@ import_one (ctrl_t ctrl, stats->imported++; new_key = 1; } - else /* Merge the key. */ + else /* Key already exists - merge. */ { int n_uids, n_sigs, n_subk, n_sigs_cleaned, n_uids_cleaned; u32 curtime = make_timestamp (); /* Compare the original against the new key; just to be sure nothing * weird is going on */ - if (cmp_public_keys( pk_orig, pk ) ) + if (cmp_public_keys (keyblock_orig->pkt->pkt.public_key, pk)) { if (!silent) log_error( _("key %s: doesn't match our copy\n"),keystr(keyid)); goto leave; } - /* Now read the original keyblock again so that we can use the - * handle for updating the keyblock. FIXME: Why not let - * get_pubkey_byfprint_fast do that - it fetches the keyblock - * anyway. */ - err = keydb_search_fpr (hd, fpr2); - if (err) - { - log_error (_("key %s: can't locate original keyblock: %s\n"), - keystr(keyid), gpg_strerror (err)); - goto leave; - } - err = keydb_get_keyblock (hd, &keyblock_orig); - if (err) - { - log_error (_("key %s: can't read original keyblock: %s\n"), - keystr(keyid), gpg_strerror (err)); - goto leave; - } - /* Make sure the original direct key sigs are all sane. */ n_sigs_cleaned = fix_bad_direct_key_sigs (ctrl, keyblock_orig, keyid); if (n_sigs_cleaned) commit_kbnode (&keyblock_orig); - /* and try to merge the block */ + /* Try to merge KEYBLOCK into KEYBLOCK_ORIG. */ clear_kbnode_flags( keyblock_orig ); clear_kbnode_flags( keyblock ); n_uids = n_sigs = n_subk = n_uids_cleaned = 0; @@ -2091,7 +2071,6 @@ import_one (ctrl_t ctrl, } release_kbnode( keyblock_orig ); - free_public_key( pk_orig ); return err; } @@ -3266,14 +3245,15 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock) /* Okay, we have a revocation key, and a * revocation issued by it. Do we have the key * itself? */ - int rc; + gpg_error_t err; - rc=get_pubkey_byfprint_fast (NULL,sig->revkey[idx].fpr, - MAX_FINGERPRINT_LEN); - if (gpg_err_code (rc) == GPG_ERR_NO_PUBKEY - || gpg_err_code (rc) == GPG_ERR_UNUSABLE_PUBKEY) + err = get_pubkey_byfprint_fast (NULL, + sig->revkey[idx].fpr, + MAX_FINGERPRINT_LEN); + if (gpg_err_code (err) == GPG_ERR_NO_PUBKEY + || gpg_err_code (err) == GPG_ERR_UNUSABLE_PUBKEY) { - char *tempkeystr=xstrdup(keystr_from_pk(pk)); + char *tempkeystr = xstrdup (keystr_from_pk (pk)); /* No, so try and get it */ if ((opt.keyserver_options.options @@ -3289,13 +3269,13 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock) opt.keyserver, 0); /* Do we have it now? */ - rc=get_pubkey_byfprint_fast (NULL, + err = get_pubkey_byfprint_fast (NULL, sig->revkey[idx].fpr, MAX_FINGERPRINT_LEN); } - if (gpg_err_code (rc) == GPG_ERR_NO_PUBKEY - || gpg_err_code (rc) == GPG_ERR_UNUSABLE_PUBKEY) + if (gpg_err_code (err) == GPG_ERR_NO_PUBKEY + || gpg_err_code (err) == GPG_ERR_UNUSABLE_PUBKEY) log_info(_("WARNING: key %s may be revoked:" " revocation key %s not present.\n"), tempkeystr,keystr(keyid)); diff --git a/g10/keydb.h b/g10/keydb.h index f503c9990..b173751ca 100644 --- a/g10/keydb.h +++ b/g10/keydb.h @@ -339,8 +339,17 @@ int get_pubkey_byfprint (ctrl_t ctrl, PKT_public_key *pk, kbnode_t *r_keyblock, /* This function is similar to get_pubkey_byfprint, but it doesn't merge the self-signed data into the public key and subkeys or into the user ids. */ -int get_pubkey_byfprint_fast (PKT_public_key *pk, - const byte *fprint, size_t fprint_len); +gpg_error_t get_pubkey_byfprint_fast (PKT_public_key *pk, + const byte *fprint, size_t fprint_len); + +/* This function is similar to get_pubkey_byfprint, but it doesn't + merge the self-signed data into the public key and subkeys or into + the user ids. */ +gpg_error_t get_keyblock_byfprint_fast (kbnode_t *r_keyblock, + KEYDB_HANDLE *r_hd, + const byte *fprint, size_t fprint_len, + int lock); + /* Returns true if a secret key is available for the public key with key id KEYID. */ From 7c73db3d31c6457dfbdc82a8dc89951c023f0603 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 18 Oct 2017 18:28:52 +0200 Subject: [PATCH 06/21] gpg: Keep a lock during the read-update/insert cycle in import. * g10/keydb.c (keydb_handle): New field 'keep_lock'. (keydb_release): Clear that flag. (keydb_lock): New function. (unlock_all): Skip if KEEP_LOCK is set. * g10/getkey.c (get_keyblock_byfprint_fast): Call keep_lock if requested. -- That change is straightforward. It helps to avoid the race condition that another gpg process inserts a key while the first process is between the search and the insert. A similar change is due for gpgsm. Note that the key edit operations may still suffer from a race. GnuPG-bug-id: 3446 --- g10/getkey.c | 15 +++++++++++++-- g10/keydb.c | 25 ++++++++++++++++++++++++- g10/keydb.h | 4 ++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/g10/getkey.c b/g10/getkey.c index 5ce580541..c58e8ff2c 100644 --- a/g10/getkey.c +++ b/g10/getkey.c @@ -1877,14 +1877,25 @@ get_keyblock_byfprint_fast (kbnode_t *r_keyblock, KEYDB_HANDLE *r_hd, hd = keydb_new (); if (!hd) return gpg_error_from_syserror (); - if (r_hd) - *r_hd = hd; if (lock) { + err = keydb_lock (hd); + if (err) + { + /* If locking did not work, we better don't return a handle + * at all - there was a reason that locking has been + * requested. */ + keydb_release (hd); + return err; + } keydb_disable_caching (hd); } + /* Fo all other errors we return the handle. */ + if (r_hd) + *r_hd = hd; + err = keydb_search_fpr (hd, fprbuf); if (gpg_err_code (err) == GPG_ERR_NOT_FOUND) { diff --git a/g10/keydb.c b/g10/keydb.c index 0f28bc301..58a14a83d 100644 --- a/g10/keydb.c +++ b/g10/keydb.c @@ -96,6 +96,10 @@ struct keydb_handle / keybox_lock, as appropriate). */ int locked; + /* If this flag is set a lock will only be released by + * keydb_release. */ + int keep_lock; + /* The index into ACTIVE of the resources in which the last search result was found. Initially -1. */ int found; @@ -964,6 +968,7 @@ keydb_release (KEYDB_HANDLE hd) log_assert (active_handles > 0); active_handles--; + hd->keep_lock = 0; unlock_all (hd); for (i=0; i < hd->used; i++) { @@ -985,6 +990,24 @@ keydb_release (KEYDB_HANDLE hd) } +/* Take a lock on the files immediately and not only during insert or + * update. This lock is released with keydb_release. */ +gpg_error_t +keydb_lock (KEYDB_HANDLE hd) +{ + gpg_error_t err; + + if (!hd) + return gpg_error (GPG_ERR_INV_ARG); + + err = lock_all (hd); + if (!err) + hd->keep_lock = 1; + + return err; +} + + /* Set a flag on the handle to suppress use of cached results. This * is required for updating a keyring and for key listings. Fixme: * Using a new parameter for keydb_new might be a better solution. */ @@ -1098,7 +1121,7 @@ unlock_all (KEYDB_HANDLE hd) { int i; - if (!hd->locked) + if (!hd->locked || hd->keep_lock) return; for (i=hd->used-1; i >= 0; i--) diff --git a/g10/keydb.h b/g10/keydb.h index b173751ca..739376838 100644 --- a/g10/keydb.h +++ b/g10/keydb.h @@ -154,6 +154,10 @@ KEYDB_HANDLE keydb_new (void); /* Free all resources owned by the database handle. */ void keydb_release (KEYDB_HANDLE hd); +/* Take a lock on the files immediately and not only during insert or + * update. This lock is released with keydb_release. */ +gpg_error_t keydb_lock (KEYDB_HANDLE hd); + /* Set a flag on the handle to suppress use of cached results. This is required for updating a keyring and for key listings. Fixme: Using a new parameter for keydb_new might be a better solution. */ From d07de3862710d88bc80d6f6c5ca8da5cf38ff0eb Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Thu, 19 Oct 2017 11:08:24 +0900 Subject: [PATCH 07/21] g10: Fix find_and_check_key for multiple keyrings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * g10/pkclist.c (find_and_check_key): Call get_validity on a specific keyblock. -- When we have multiple keyrings, get_validity after get_best_pubkey_byname should access same keyring. Or else, the situation of an expired key in keyring A but valid key in keyring B causes SEGV. Thanks to Guido Günther for the use case and the log. Debian-bug-id: 878812 Signed-off-by: NIIBE Yutaka --- g10/pkclist.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/g10/pkclist.c b/g10/pkclist.c index 67d932e2a..220936c56 100644 --- a/g10/pkclist.c +++ b/g10/pkclist.c @@ -826,6 +826,7 @@ find_and_check_key (ctrl_t ctrl, const char *name, unsigned int use, { int rc; PKT_public_key *pk; + KBNODE keyblock = NULL; if (!name || !*name) return gpg_error (GPG_ERR_INV_USER_ID); @@ -838,7 +839,7 @@ find_and_check_key (ctrl_t ctrl, const char *name, unsigned int use, if (from_file) rc = get_pubkey_fromfile (ctrl, pk, name); else - rc = get_best_pubkey_byname (ctrl, NULL, pk, name, NULL, 0, 0); + rc = get_best_pubkey_byname (ctrl, NULL, pk, name, &keyblock, 0, 0); if (rc) { int code; @@ -861,6 +862,7 @@ find_and_check_key (ctrl_t ctrl, const char *name, unsigned int use, if (rc) { /* Key found but not usable for us (e.g. sign-only key). */ + release_kbnode (keyblock); send_status_inv_recp (3, name); /* Wrong key usage */ log_error (_("%s: skipped: %s\n"), name, gpg_strerror (rc) ); free_public_key (pk); @@ -872,7 +874,8 @@ find_and_check_key (ctrl_t ctrl, const char *name, unsigned int use, { int trustlevel; - trustlevel = get_validity (ctrl, NULL, pk, pk->user_id, NULL, 1); + trustlevel = get_validity (ctrl, keyblock, pk, pk->user_id, NULL, 1); + release_kbnode (keyblock); if ( (trustlevel & TRUST_FLAG_DISABLED) ) { /* Key has been disabled. */ From 1ba308aa0356a57c21c4c8c2dac75b4d62b8aac3 Mon Sep 17 00:00:00 2001 From: Damien Goutte-Gattat Date: Sun, 8 Oct 2017 17:30:52 +0100 Subject: [PATCH 08/21] dirmngr: Do not follow https-to-http redirects. * dirmngr/ks-engine-http.c (ks_http_fetch): Forbid redirects from a https URI to a http URI. -- GnuPG-bug-id: 3436 Signed-off-by: Damien Goutte-Gattat --- dirmngr/ks-engine-http.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dirmngr/ks-engine-http.c b/dirmngr/ks-engine-http.c index 7fb77312d..6492dda8a 100644 --- a/dirmngr/ks-engine-http.c +++ b/dirmngr/ks-engine-http.c @@ -73,12 +73,13 @@ ks_http_fetch (ctrl_t ctrl, const char *url, estream_t *r_fp) estream_t fp = NULL; char *request_buffer = NULL; parsed_uri_t uri = NULL; - int is_onion; + int is_onion, is_https; err = http_parse_uri (&uri, url, 0); if (err) goto leave; is_onion = uri->onion; + is_https = uri->use_tls; once_more: /* Note that we only use the system provided certificates with the @@ -152,17 +153,18 @@ ks_http_fetch (ctrl_t ctrl, const char *url, estream_t *r_fp) url, s?s:"[none]", http_get_status_code (http)); if (s && *s && redirects_left-- ) { - if (is_onion) + if (is_onion || is_https) { /* Make sure that an onion address only redirects to - * another onion address. */ + * another onion address, or that a https address + * only redirects to a https address. */ http_release_parsed_uri (uri); uri = NULL; err = http_parse_uri (&uri, s, 0); if (err) goto leave; - if (! uri->onion) + if ((is_onion && ! uri->onion) || (is_https && ! uri->use_tls)) { err = gpg_error (GPG_ERR_FORBIDDEN); goto leave; From 68c8619114fd5f24cb6bfb9e0f25c428a8805323 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 19 Oct 2017 17:05:39 +0200 Subject: [PATCH 09/21] gpg: Make --dry-run and show-only work for secret keys. * g10/import.c (import_secret_one): Check for dry-run before transferring keys. -- The use of --dry-run or --import-option show-only had no effect when importing a secret key and the public key already existed. If the public key did not exist an error message inhibited the import of the secret key. Signed-off-by: Werner Koch --- g10/import.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/g10/import.c b/g10/import.c index 8dd6b501e..255e48fb2 100644 --- a/g10/import.c +++ b/g10/import.c @@ -2532,7 +2532,8 @@ import_secret_one (ctrl_t ctrl, kbnode_t keyblock, /* At least we cancel the secret key import when the public key import was skipped due to MERGE_ONLY option and a new key. */ - if (stats->skipped_new_keys <= nr_prev) + if (!(opt.dry_run || (options & IMPORT_DRY_RUN)) + && stats->skipped_new_keys <= nr_prev) { /* Read the keyblock again to get the effects of a merge. */ /* Fixme: we should do this based on the fingerprint or From 2c7dccca9b617780a3ea760adf460bb3b77f90f3 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 19 Oct 2017 17:12:36 +0200 Subject: [PATCH 10/21] gpg: Print sec/sbb with --import-option import-show or show-only. * g10/import.c (import_one): Pass FROM_SK to list_keyblock_direct. -- Note that this will likely add the suffix '#' top "sec" because the secret key has not yet (or will not be) imported. If the secret key already exists locally another suffix might be printed. The upshot is that the suffix has no usefulness. GnuPG-bug-id: 3431 Signed-off-by: Werner Koch --- doc/gpg.texi | 3 ++- g10/import.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/gpg.texi b/doc/gpg.texi index b14cb371b..bd45b0422 100644 --- a/doc/gpg.texi +++ b/doc/gpg.texi @@ -2306,7 +2306,8 @@ opposite meaning. The options are: Show a listing of the key as imported right before it is stored. This can be combined with the option @option{--dry-run} to only look at keys; the option @option{show-only} is a shortcut for this - combination. + combination. Note that suffixes like '#' for "sec" and "sbb" lines + may or may not be printed. @item import-export Run the entire import code but instead of storing the key to the diff --git a/g10/import.c b/g10/import.c index 255e48fb2..71e39557c 100644 --- a/g10/import.c +++ b/g10/import.c @@ -1778,7 +1778,7 @@ import_one (ctrl_t ctrl, merge_keys_done = 1; /* Note that we do not want to show the validity because the key * has not yet imported. */ - list_keyblock_direct (ctrl, keyblock, 0, 0, + list_keyblock_direct (ctrl, keyblock, from_sk, 0, opt.fingerprint || opt.with_fingerprint, 1); es_fflush (es_stdout); } From 44fb3fbc85b32552c91f32f099b6b246c12ce0cc Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 19 Oct 2017 18:10:37 +0200 Subject: [PATCH 11/21] gpg: Fix creating on-disk subkey with on-card primary key. * g10/keygen.c (generate_subkeypair): Ignore error code issued for trying to verify a card based key. -- We try to verify the primary key and thus seed the passphrase cache before generating the subkey. However, the verification does not yet work for on-card keys and thus the PASSWD --verify send to the agent returns an error. This patch detects this error and continues without a seeded passphrase cache. After all that pre-seeding is just a convenience. GnuPG-bug-id: 3280 Signed-off-by: Werner Koch --- g10/keygen.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/g10/keygen.c b/g10/keygen.c index 2b17a1e09..8f30b7ecc 100644 --- a/g10/keygen.c +++ b/g10/keygen.c @@ -5051,6 +5051,9 @@ generate_subkeypair (ctrl_t ctrl, kbnode_t keyblock, const char *algostr, err = agent_passwd (ctrl, hexgrip, desc, 1 /*=verify*/, &cache_nonce, &passwd_nonce); xfree (desc); + if (gpg_err_code (err) == GPG_ERR_NOT_IMPLEMENTED + && gpg_err_source (err) == GPG_ERR_SOURCE_GPGAGENT) + err = 0; /* Very likely that the key is on a card. */ if (err) goto leave; } From 9e3f2a7e0b7a8713084bbdd835bc5c65410a402f Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 20 Oct 2017 08:56:39 +0200 Subject: [PATCH 12/21] doc: Fix "SEE ALSO" section of gpgv. -- --- doc/gpgv.texi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/gpgv.texi b/doc/gpgv.texi index 5336c98db..a05286171 100644 --- a/doc/gpgv.texi +++ b/doc/gpgv.texi @@ -187,6 +187,6 @@ The default keyring with the allowed keys. @end table @mansect see also -@command{gpg2}(1) +@command{gpg}(1) @include see-also-note.texi From 016538d82867c40a21bc7cbf44ec386f4699077f Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 24 Oct 2017 09:31:49 +0200 Subject: [PATCH 13/21] gpg: Remove unused flags from keyedit.c. * g10/keyedit.c (KEYEDIT_NOT_SK, KEYEDIT_ONLY_SK): Remove. (cmds): Remove them. -- These flags were cruft from the time we had to switch between secret and public key view. Signed-off-by: Werner Koch --- g10/keyedit.c | 74 ++++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 39 deletions(-) diff --git a/g10/keyedit.c b/g10/keyedit.c index 38cdbce3a..75c52afb3 100644 --- a/g10/keyedit.c +++ b/g10/keyedit.c @@ -1223,10 +1223,6 @@ parse_sign_type (const char *str, int *localsig, int *nonrevokesig, /* Need an SK for this command */ #define KEYEDIT_NEED_SK 1 -/* Cannot be viewing the SK for this command */ -#define KEYEDIT_NOT_SK 2 -/* Must be viewing the SK for this command */ -#define KEYEDIT_ONLY_SK 4 /* Match the tail of the string */ #define KEYEDIT_TAIL_MATCH 8 @@ -1268,12 +1264,12 @@ static struct { "key", cmdSELKEY, 0, N_("select subkey N")}, { "check", cmdCHECK, 0, N_("check signatures")}, { "c", cmdCHECK, 0, NULL}, - { "change-usage", cmdCHANGEUSAGE, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, NULL}, - { "cross-certify", cmdBACKSIGN, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, NULL}, - { "backsign", cmdBACKSIGN, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, NULL}, - { "sign", cmdSIGN, KEYEDIT_NOT_SK | KEYEDIT_TAIL_MATCH, + { "change-usage", cmdCHANGEUSAGE, KEYEDIT_NEED_SK, NULL}, + { "cross-certify", cmdBACKSIGN, KEYEDIT_NEED_SK, NULL}, + { "backsign", cmdBACKSIGN, KEYEDIT_NEED_SK, NULL}, + { "sign", cmdSIGN, KEYEDIT_TAIL_MATCH, N_("sign selected user IDs [* see below for related commands]")}, - { "s", cmdSIGN, KEYEDIT_NOT_SK, NULL}, + { "s", cmdSIGN, 0, NULL}, /* "lsign" and friends will never match since "sign" comes first and it is a tail match. They are just here so they show up in the help menu. */ @@ -1282,62 +1278,62 @@ static struct { "nrsign", cmdNOP, 0, N_("sign selected user IDs with a non-revocable signature")}, { "debug", cmdDEBUG, 0, NULL}, - { "adduid", cmdADDUID, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, N_("add a user ID")}, - { "addphoto", cmdADDPHOTO, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "adduid", cmdADDUID, KEYEDIT_NEED_SK, N_("add a user ID")}, + { "addphoto", cmdADDPHOTO, KEYEDIT_NEED_SK, N_("add a photo ID")}, - { "deluid", cmdDELUID, KEYEDIT_NOT_SK, N_("delete selected user IDs")}, + { "deluid", cmdDELUID, 0, N_("delete selected user IDs")}, /* delphoto is really deluid in disguise */ - { "delphoto", cmdDELUID, KEYEDIT_NOT_SK, NULL}, - { "addkey", cmdADDKEY, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, N_("add a subkey")}, + { "delphoto", cmdDELUID, 0, NULL}, + { "addkey", cmdADDKEY, KEYEDIT_NEED_SK, N_("add a subkey")}, #ifdef ENABLE_CARD_SUPPORT - { "addcardkey", cmdADDCARDKEY, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "addcardkey", cmdADDCARDKEY, KEYEDIT_NEED_SK, N_("add a key to a smartcard")}, - { "keytocard", cmdKEYTOCARD, KEYEDIT_NEED_SK | KEYEDIT_ONLY_SK, + { "keytocard", cmdKEYTOCARD, KEYEDIT_NEED_SK, N_("move a key to a smartcard")}, - { "bkuptocard", cmdBKUPTOCARD, KEYEDIT_NEED_SK | KEYEDIT_ONLY_SK, + { "bkuptocard", cmdBKUPTOCARD, KEYEDIT_NEED_SK, N_("move a backup key to a smartcard")}, #endif /*ENABLE_CARD_SUPPORT */ - { "delkey", cmdDELKEY, KEYEDIT_NOT_SK, N_("delete selected subkeys")}, - { "addrevoker", cmdADDREVOKER, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "delkey", cmdDELKEY, 0, N_("delete selected subkeys")}, + { "addrevoker", cmdADDREVOKER, KEYEDIT_NEED_SK, N_("add a revocation key")}, - { "delsig", cmdDELSIG, KEYEDIT_NOT_SK, + { "delsig", cmdDELSIG, 0, N_("delete signatures from the selected user IDs")}, - { "expire", cmdEXPIRE, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "expire", cmdEXPIRE, KEYEDIT_NEED_SK, N_("change the expiration date for the key or selected subkeys")}, - { "primary", cmdPRIMARY, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "primary", cmdPRIMARY, KEYEDIT_NEED_SK, N_("flag the selected user ID as primary")}, { "toggle", cmdTOGGLE, KEYEDIT_NEED_SK, NULL}, /* Dummy command. */ { "t", cmdTOGGLE, KEYEDIT_NEED_SK, NULL}, - { "pref", cmdPREF, KEYEDIT_NOT_SK, N_("list preferences (expert)")}, - { "showpref", cmdSHOWPREF, KEYEDIT_NOT_SK, N_("list preferences (verbose)")}, - { "setpref", cmdSETPREF, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "pref", cmdPREF, 0, N_("list preferences (expert)")}, + { "showpref", cmdSHOWPREF, 0, N_("list preferences (verbose)")}, + { "setpref", cmdSETPREF, KEYEDIT_NEED_SK, N_("set preference list for the selected user IDs")}, - { "updpref", cmdSETPREF, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, NULL}, - { "keyserver", cmdPREFKS, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "updpref", cmdSETPREF, KEYEDIT_NEED_SK, NULL}, + { "keyserver", cmdPREFKS, KEYEDIT_NEED_SK, N_("set the preferred keyserver URL for the selected user IDs")}, - { "notation", cmdNOTATION, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "notation", cmdNOTATION, KEYEDIT_NEED_SK, N_("set a notation for the selected user IDs")}, - { "passwd", cmdPASSWD, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "passwd", cmdPASSWD, KEYEDIT_NEED_SK, N_("change the passphrase")}, - { "password", cmdPASSWD, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, NULL}, + { "password", cmdPASSWD, KEYEDIT_NEED_SK, NULL}, #ifndef NO_TRUST_MODELS - { "trust", cmdTRUST, KEYEDIT_NOT_SK, N_("change the ownertrust")}, + { "trust", cmdTRUST, 0, N_("change the ownertrust")}, #endif /*!NO_TRUST_MODELS*/ - { "revsig", cmdREVSIG, KEYEDIT_NOT_SK, + { "revsig", cmdREVSIG, 0, N_("revoke signatures on the selected user IDs")}, - { "revuid", cmdREVUID, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "revuid", cmdREVUID, KEYEDIT_NEED_SK, N_("revoke selected user IDs")}, - { "revphoto", cmdREVUID, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, NULL}, - { "revkey", cmdREVKEY, KEYEDIT_NOT_SK | KEYEDIT_NEED_SK, + { "revphoto", cmdREVUID, KEYEDIT_NEED_SK, NULL}, + { "revkey", cmdREVKEY, KEYEDIT_NEED_SK, N_("revoke key or selected subkeys")}, #ifndef NO_TRUST_MODELS - { "enable", cmdENABLEKEY, KEYEDIT_NOT_SK, N_("enable key")}, - { "disable", cmdDISABLEKEY, KEYEDIT_NOT_SK, N_("disable key")}, + { "enable", cmdENABLEKEY, 0, N_("enable key")}, + { "disable", cmdDISABLEKEY, 0, N_("disable key")}, #endif /*!NO_TRUST_MODELS*/ { "showphoto", cmdSHOWPHOTO, 0, N_("show selected photo IDs")}, - { "clean", cmdCLEAN, KEYEDIT_NOT_SK, + { "clean", cmdCLEAN, 0, N_("compact unusable user IDs and remove unusable signatures from key")}, - { "minimize", cmdMINIMIZE, KEYEDIT_NOT_SK, + { "minimize", cmdMINIMIZE, 0, N_("compact unusable user IDs and remove all signatures from key")}, { NULL, cmdNONE, 0, NULL} From 560d85ecff4246133d185dc29395f07c918b5556 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 24 Oct 2017 10:56:13 +0200 Subject: [PATCH 14/21] gpg: Improve the "secret key available" notice in keyedit.c * g10/keyedit.c (KEYEDIT_NEED_SUBSK): New. (cmds): Add this flag to keytocard, bkuptocard, expire, and passwd. (keyedit_menu): Check whether only subkeys are available and take care of that in the command check and in the HELP listing. Also print a different notice if only subkeys are available. -- Print "Secret key is available" and the bailing out in all commands which require the _primary_ secret key was surprising. Now we print another notice and adjust the checks. GnuPG-bug-id: 3463 Signed-off-by: Werner Koch --- g10/keyedit.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/g10/keyedit.c b/g10/keyedit.c index 75c52afb3..4acb2de5f 100644 --- a/g10/keyedit.c +++ b/g10/keyedit.c @@ -1223,6 +1223,8 @@ parse_sign_type (const char *str, int *localsig, int *nonrevokesig, /* Need an SK for this command */ #define KEYEDIT_NEED_SK 1 +/* Need an SUB KEY for this command */ +#define KEYEDIT_NEED_SUBSK 2 /* Match the tail of the string */ #define KEYEDIT_TAIL_MATCH 8 @@ -1288,9 +1290,9 @@ static struct #ifdef ENABLE_CARD_SUPPORT { "addcardkey", cmdADDCARDKEY, KEYEDIT_NEED_SK, N_("add a key to a smartcard")}, - { "keytocard", cmdKEYTOCARD, KEYEDIT_NEED_SK, + { "keytocard", cmdKEYTOCARD, KEYEDIT_NEED_SK | KEYEDIT_NEED_SUBSK, N_("move a key to a smartcard")}, - { "bkuptocard", cmdBKUPTOCARD, KEYEDIT_NEED_SK, + { "bkuptocard", cmdBKUPTOCARD, KEYEDIT_NEED_SK | KEYEDIT_NEED_SUBSK, N_("move a backup key to a smartcard")}, #endif /*ENABLE_CARD_SUPPORT */ { "delkey", cmdDELKEY, 0, N_("delete selected subkeys")}, @@ -1298,7 +1300,7 @@ static struct N_("add a revocation key")}, { "delsig", cmdDELSIG, 0, N_("delete signatures from the selected user IDs")}, - { "expire", cmdEXPIRE, KEYEDIT_NEED_SK, + { "expire", cmdEXPIRE, KEYEDIT_NEED_SK | KEYEDIT_NEED_SUBSK, N_("change the expiration date for the key or selected subkeys")}, { "primary", cmdPRIMARY, KEYEDIT_NEED_SK, N_("flag the selected user ID as primary")}, @@ -1313,9 +1315,9 @@ static struct N_("set the preferred keyserver URL for the selected user IDs")}, { "notation", cmdNOTATION, KEYEDIT_NEED_SK, N_("set a notation for the selected user IDs")}, - { "passwd", cmdPASSWD, KEYEDIT_NEED_SK, + { "passwd", cmdPASSWD, KEYEDIT_NEED_SK | KEYEDIT_NEED_SUBSK, N_("change the passphrase")}, - { "password", cmdPASSWD, KEYEDIT_NEED_SK, NULL}, + { "password", cmdPASSWD, KEYEDIT_NEED_SK | KEYEDIT_NEED_SUBSK, NULL}, #ifndef NO_TRUST_MODELS { "trust", cmdTRUST, 0, N_("change the ownertrust")}, #endif /*!NO_TRUST_MODELS*/ @@ -1402,6 +1404,7 @@ keyedit_menu (ctrl_t ctrl, const char *username, strlist_t locusr, KBNODE keyblock = NULL; KEYDB_HANDLE kdbhd = NULL; int have_seckey = 0; + int have_anyseckey = 0; char *answer = NULL; int redisplay = 1; int modified = 0; @@ -1444,9 +1447,18 @@ keyedit_menu (ctrl_t ctrl, const char *username, strlist_t locusr, /* See whether we have a matching secret key. */ if (seckey_check) { - have_seckey = !agent_probe_any_secret_key (ctrl, keyblock); + have_anyseckey = !agent_probe_any_secret_key (ctrl, keyblock); + if (have_anyseckey + && !agent_probe_secret_key (ctrl, keyblock->pkt->pkt.public_key)) + { + /* The primary key is also available. */ + have_seckey = 1; + } + if (have_seckey && !quiet) - tty_printf (_("Secret key is available.\n")); + tty_printf (_("Secret key is available.\n")); + else if (have_anyseckey && !quiet) + tty_printf (_("Secret subkeys are available.\n")); } /* Main command loop. */ @@ -1544,12 +1556,14 @@ keyedit_menu (ctrl_t ctrl, const char *username, strlist_t locusr, else if (!ascii_strcasecmp (answer, cmds[i].name)) break; } - if ((cmds[i].flags & KEYEDIT_NEED_SK) && !have_seckey) + if ((cmds[i].flags & (KEYEDIT_NEED_SK|KEYEDIT_NEED_SUBSK)) + && !(((cmds[i].flags & KEYEDIT_NEED_SK) && have_seckey) + || ((cmds[i].flags & KEYEDIT_NEED_SUBSK) && have_anyseckey))) { tty_printf (_("Need the secret key to do this.\n")); cmd = cmdNOP; } - else + else cmd = cmds[i].id; } @@ -1559,7 +1573,9 @@ keyedit_menu (ctrl_t ctrl, const char *username, strlist_t locusr, case cmdHELP: for (i = 0; cmds[i].name; i++) { - if ((cmds[i].flags & KEYEDIT_NEED_SK) && !have_seckey) + if ((cmds[i].flags & (KEYEDIT_NEED_SK|KEYEDIT_NEED_SUBSK)) + && !(((cmds[i].flags & KEYEDIT_NEED_SK) && have_seckey) + ||((cmds[i].flags&KEYEDIT_NEED_SUBSK)&&have_anyseckey))) ; /* Skip those item if we do not have the secret key. */ else if (cmds[i].desc) tty_printf ("%-11s %s\n", cmds[i].name, _(cmds[i].desc)); From 6e808ae4700dc5e95bf4cc2d5c063df582c234d0 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 24 Oct 2017 12:01:07 +0200 Subject: [PATCH 15/21] gpgconf: Ignore non-installed components with --apply-profile. * tools/gpgconf-comp.c (retrieve_options_from_program): Add arg only_installed. (gc_component_retrieve_options): Use this if we want to process all components. -- Note that this also also ignores them in --with-defaults. This is useful for systems which come without scdaemon. GnuPG-bug-id: 3313 Signed-off-by: Werner Koch --- tools/gpgconf-comp.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tools/gpgconf-comp.c b/tools/gpgconf-comp.c index e6ef4f4e3..9ce752b18 100644 --- a/tools/gpgconf-comp.c +++ b/tools/gpgconf-comp.c @@ -2085,9 +2085,12 @@ get_config_filename (gc_component_t component, gc_backend_t backend) /* Retrieve the options for the component COMPONENT from backend - BACKEND, which we already know is a program-type backend. */ + * BACKEND, which we already know is a program-type backend. With + * ONLY_INSTALLED set components which are not installed are silently + * ignored. */ static void -retrieve_options_from_program (gc_component_t component, gc_backend_t backend) +retrieve_options_from_program (gc_component_t component, gc_backend_t backend, + int only_installed) { gpg_error_t err; const char *pgmname; @@ -2107,6 +2110,11 @@ retrieve_options_from_program (gc_component_t component, gc_backend_t backend) argv[0] = "--gpgconf-list"; argv[1] = NULL; + if (only_installed && access (pgmname, X_OK)) + { + return; /* The component is not installed. */ + } + err = gnupg_spawn_process (pgmname, argv, NULL, NULL, 0, NULL, &outfp, NULL, &pid); if (err) @@ -2378,7 +2386,7 @@ retrieve_options_from_file (gc_component_t component, gc_backend_t backend) /* Retrieve the currently active options and their defaults from all involved backends for this component. Using -1 for component will - retrieve all options from all components. */ + retrieve all options from all installed components. */ void gc_component_retrieve_options (int component) { @@ -2420,7 +2428,8 @@ gc_component_retrieve_options (int component) assert (backend != GC_BACKEND_ANY); if (gc_backend[backend].program) - retrieve_options_from_program (component, backend); + retrieve_options_from_program (component, backend, + process_all); else retrieve_options_from_file (component, backend); } From 1067403c8a7fb51decf30059e46901b5ee9f5b37 Mon Sep 17 00:00:00 2001 From: Rainer Perske Date: Tue, 24 Oct 2017 17:29:04 +0200 Subject: [PATCH 16/21] sm: Do not expect X.509 keyids to be unique * sm/certlist.c (gpgsm_find_cert): Add arg allow_ambiguous and use it. * sm/call-dirmngr.c (inq_certificate): Pass true to ALLOW_AMBIGUOUS (run_command_inq_cb): Ditto. * sm/gpgsm.c (main): Pass false. * sm/server.c (cmd_passwd): Pass false. -- As described in my report T1644, it is possible that multiple certificates exist with the same Distinguished Name and the same key. In this case, verifying S/MIME signatures and other actions fail with "certificate not found: Ambiguous name". For details see the bug report. To circumvent the problem, I am patching GnuPG since 2014 so that in this case the newest of the ambiguous certificates is used. This is not an ultimate solution of the problem: You should try every certificate with the same DN until verification succeeds or until all certificates fail, and if multiple certificates of a chain are ambiguous you even have to check every combination. You may even consider checking the keyUsage attributes of the ambiguous certificates to reduce the number of combinations. But in the existing case of the certificates in the German Research Network (DFN) PKI where the newest one is the valid one and all ambiguous certificates have the same keyUsage attributes, this patch has proven to be sufficient over the last three years. With every GnuPG update, I have adapted the patch, luckily I never needed to change anything except line numbers. GnuPG-bug-id: 1644 ChangeLog log written by wk, comment taken from mail. Signed-off line was missing in the plain diff. However the mail with the patch and the DCO posted as reply to that mail were both signed. Signed-off-by: Werner Koch --- sm/call-dirmngr.c | 4 ++-- sm/certlist.c | 40 +++++++++++++++++++++++++++++++++++++++- sm/gpgsm.c | 2 +- sm/gpgsm.h | 2 +- sm/server.c | 2 +- 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/sm/call-dirmngr.c b/sm/call-dirmngr.c index 930194076..e94311892 100644 --- a/sm/call-dirmngr.c +++ b/sm/call-dirmngr.c @@ -415,7 +415,7 @@ inq_certificate (void *opaque, const char *line) ksba_cert_t cert; - err = gpgsm_find_cert (parm->ctrl, line, ski, &cert); + err = gpgsm_find_cert (parm->ctrl, line, ski, &cert, 1); if (err) { log_error ("certificate not found: %s\n", gpg_strerror (err)); @@ -936,7 +936,7 @@ run_command_inq_cb (void *opaque, const char *line) if (!*line) return gpg_error (GPG_ERR_ASS_PARAMETER); - err = gpgsm_find_cert (parm->ctrl, line, NULL, &cert); + err = gpgsm_find_cert (parm->ctrl, line, NULL, &cert, 1); if (err) { log_error ("certificate not found: %s\n", gpg_strerror (err)); diff --git a/sm/certlist.c b/sm/certlist.c index 39ab03c5d..c9e275e9d 100644 --- a/sm/certlist.c +++ b/sm/certlist.c @@ -489,7 +489,8 @@ gpgsm_release_certlist (certlist_t list) subjectKeyIdentifier. */ int gpgsm_find_cert (ctrl_t ctrl, - const char *name, ksba_sexp_t keyid, ksba_cert_t *r_cert) + const char *name, ksba_sexp_t keyid, ksba_cert_t *r_cert, + int allow_ambiguous) { int rc; KEYDB_SEARCH_DESC desc; @@ -537,6 +538,16 @@ gpgsm_find_cert (ctrl_t ctrl, won't lead to ambiguous names. */ if (!rc && !keyid) { + ksba_isotime_t notbefore = ""; + const unsigned char *image = NULL; + size_t length = 0; + if (allow_ambiguous) + { + /* We want to return the newest certificate */ + if (ksba_cert_get_validity (*r_cert, 0, notbefore)) + *notbefore = '\0'; + image = ksba_cert_get_image (*r_cert, &length); + } next_ambiguous: rc = keydb_search (ctrl, kh, &desc, 1); if (rc == -1) @@ -546,6 +557,10 @@ gpgsm_find_cert (ctrl_t ctrl, if (!rc) { ksba_cert_t cert2 = NULL; + ksba_isotime_t notbefore2 = ""; + const unsigned char *image2 = NULL; + size_t length2 = 0; + int cmp = 0; if (!keydb_get_cert (kh, &cert2)) { @@ -554,6 +569,29 @@ gpgsm_find_cert (ctrl_t ctrl, ksba_cert_release (cert2); goto next_ambiguous; } + if (allow_ambiguous) + { + if (ksba_cert_get_validity (cert2, 0, notbefore2)) + *notbefore2 = '\0'; + image2 = ksba_cert_get_image (cert2, &length2); + cmp = strcmp (notbefore, notbefore2); + /* use certificate image bits as last resort for stable ordering */ + if (!cmp) + cmp = memcmp (image, image2, length < length2 ? length : length2); + if (!cmp) + cmp = length < length2 ? -1 : length > length2 ? 1 : 0; + if (cmp < 0) + { + ksba_cert_release (*r_cert); + *r_cert = cert2; + strcpy (notbefore, notbefore2); + image = image2; + length = length2; + } + else + ksba_cert_release (cert2); + goto next_ambiguous; + } ksba_cert_release (cert2); } rc = gpg_error (GPG_ERR_AMBIGUOUS_NAME); diff --git a/sm/gpgsm.c b/sm/gpgsm.c index 10eff0a84..0feda90b1 100644 --- a/sm/gpgsm.c +++ b/sm/gpgsm.c @@ -2052,7 +2052,7 @@ main ( int argc, char **argv) ksba_cert_t cert = NULL; char *grip = NULL; - rc = gpgsm_find_cert (&ctrl, *argv, NULL, &cert); + rc = gpgsm_find_cert (&ctrl, *argv, NULL, &cert, 0); if (rc) ; else if (!(grip = gpgsm_get_keygrip_hexstring (cert))) diff --git a/sm/gpgsm.h b/sm/gpgsm.h index 8c1f520de..cd4fc995f 100644 --- a/sm/gpgsm.h +++ b/sm/gpgsm.h @@ -328,7 +328,7 @@ int gpgsm_add_to_certlist (ctrl_t ctrl, const char *name, int secret, certlist_t *listaddr, int is_encrypt_to); void gpgsm_release_certlist (certlist_t list); int gpgsm_find_cert (ctrl_t ctrl, const char *name, ksba_sexp_t keyid, - ksba_cert_t *r_cert); + ksba_cert_t *r_cert, int allow_ambiguous); /*-- keylist.c --*/ gpg_error_t gpgsm_list_keys (ctrl_t ctrl, strlist_t names, diff --git a/sm/server.c b/sm/server.c index 64a3add9a..568e51b17 100644 --- a/sm/server.c +++ b/sm/server.c @@ -1179,7 +1179,7 @@ cmd_passwd (assuan_context_t ctx, char *line) line = skip_options (line); - err = gpgsm_find_cert (ctrl, line, NULL, &cert); + err = gpgsm_find_cert (ctrl, line, NULL, &cert, 0); if (err) ; else if (!(grip = gpgsm_get_keygrip_hexstring (cert))) From e417aaf69817fcb4a73c38077853dc940a2deabc Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 24 Oct 2017 18:34:28 +0200 Subject: [PATCH 17/21] build: Do not mess with CFLAGS in configure. * configure.ac: Do not mess with the user provided CFLAGS. -- A problem was claimed with some configure tests if the user provided CFLAGS=-Werror. The commit introducing this Fixes-commit: 02eb9fc9d5863abcfed6af704e618f8cac7cc2e8 does not mention a concrete case. Anyway, messing with CFLAGS is a bad idea because configure tests will then test something different than what is used later (cf. autoconf manual). Tests which depend on the whether -Werror is used needsto be fixed. Note that in certain cases we modify CFLAGS. This is only done for some configure options or if the platform requires the use of special compiler flags (e.g. on HP/UX). GnuPG-bug-id: 2423 --- configure.ac | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/configure.ac b/configure.ac index 70c122615..1f933a729 100644 --- a/configure.ac +++ b/configure.ac @@ -89,12 +89,6 @@ AB_INIT AC_GNU_SOURCE -# Before we do anything with the C compiler, we first save the user's -# CFLAGS (they are restored at the end of the configure script). This -# is because some configure checks don't work with -Werror, but we'd -# like to use -Werror with our build. -CFLAGS_orig=$CFLAGS -CFLAGS= # Some status variables. have_gpg_error=no @@ -1700,11 +1694,6 @@ if test x"$gnupg_builddir_envvar" = x"yes"; then [This is only used with "make distcheck"]) fi -# -# Add user CFLAGS. -# -CFLAGS="$CFLAGS $CFLAGS_orig" - # # Decide what to build # From 812fe29bff42cf7dbd07e0becc55b2ada340dd97 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 24 Oct 2017 18:42:37 +0200 Subject: [PATCH 18/21] build: New configure option --enable-werror * configure.ac: Implement that option. -- This can be used as a workaround in case of bogus autoconf tests. GnuPG-bug-id: 2423 Signed-off-by: Werner Koch --- configure.ac | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/configure.ac b/configure.ac index 1f933a729..551c970b3 100644 --- a/configure.ac +++ b/configure.ac @@ -1665,6 +1665,17 @@ AC_ARG_ENABLE(optimization, CFLAGS=`echo $CFLAGS | sed s/-O[[1-9]]\ /-O0\ /g` fi]) +# +# Add -Werror to CFLAGS. This hack can be used to avoid problems with +# misbehaving autoconf tests in case the user supplied -Werror. +# +AC_ARG_ENABLE(werror, + AC_HELP_STRING([--enable-werror], + [append -Werror to CFLAGS]), + [if test $enableval = yes ; then + CFLAGS="$CFLAGS -Werror" + fi]) + # # Configure option --enable-all-tests # From 84af859e391a757877c9a1d78e35face983e6d23 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 24 Oct 2017 21:11:38 +0200 Subject: [PATCH 19/21] gpg: Avoid superfluous sig check info during import. * g10/key-check.c (print_info): New. (key_check_all_keysigs): Print sig checking results only in debug mode. Prettify the stats info and suppress them in quiet mode. -- This also makes usable stats by prefixing them with the key and the program name. GnuPG-bug-id: 3397 Signed-off-by: Werner Koch --- g10/key-check.c | 139 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 94 insertions(+), 45 deletions(-) diff --git a/g10/key-check.c b/g10/key-check.c index d32067b99..86b1e769d 100644 --- a/g10/key-check.c +++ b/g10/key-check.c @@ -32,6 +32,27 @@ #include "key-check.h" + +/* Print PREFIX followed by TEXT. With mode > 0 use log_info, with + * mode < 0 use ttyio, else print to stdout. If TEXT is not NULL, it + * may be modified by this function. */ +static void +print_info (int mode, const char *prefix, char *text) +{ + char *p; + + if (!text) + text = ""; + else if ((p = strchr (text,'\n'))) + *p = 0; /* Strip LF. */ + + if (mode > 0) + log_info ("%s %s\n", prefix, text); + else + tty_fprintf (mode? NULL:es_stdout, "%s %s\n", prefix, text); +} + + /* Order two signatures. The actual ordering isn't important. Our * goal is to ensure that identical signatures occur together. */ static int @@ -100,7 +121,6 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb, int only_selected, int only_selfsigs) { gpg_error_t err; - estream_t fp = mode < 0? NULL : mode ? log_get_stream () : es_stdout; PKT_public_key *pk; KBNODE n, n_next, *n_prevp, n2; char *pending_desc = NULL; @@ -476,8 +496,9 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb, has_selfsig = 1; } - if ((n2 && n2 != last_printed_component) - || (! n2 && last_printed_component != current_component)) + if (DBG_PACKET + && ((n2 && n2 != last_printed_component) + || (! n2 && last_printed_component != current_component))) { int is_reordered = n2 && n2 != current_component; if (n2) @@ -489,36 +510,34 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb, ; else if (last_printed_component->pkt->pkttype == PKT_USER_ID) { - tty_fprintf (fp, "uid "); - tty_print_utf8_string2 (fp, - last_printed_component - ->pkt->pkt.user_id->name, - last_printed_component - ->pkt->pkt.user_id->len, 0); + log_debug ("uid "); + print_utf8_buffer (log_get_stream (), + last_printed_component + ->pkt->pkt.user_id->name, + last_printed_component + ->pkt->pkt.user_id->len); + log_flush (); } else if (last_printed_component->pkt->pkttype == PKT_PUBLIC_KEY) - tty_fprintf (fp, "pub %s", - pk_keyid_str (last_printed_component + log_debug ("pub %s\n", + pk_keyid_str (last_printed_component ->pkt->pkt.public_key)); else - tty_fprintf (fp, "sub %s", - pk_keyid_str (last_printed_component - ->pkt->pkt.public_key)); + log_debug ("sub %s\n", + pk_keyid_str (last_printed_component + ->pkt->pkt.public_key)); if (modified) { if (is_reordered) - tty_fprintf (fp, _(" (reordered signatures follow)")); - if (mode > 0) - log_printf ("\n"); - else - tty_fprintf (fp, "\n"); + log_debug ("%s\n", _(" (reordered signatures follow)")); } } - if (modified) - keyedit_print_one_sig (ctrl, fp, rc, kb, n, NULL, NULL, NULL, + if (DBG_PACKET && modified) + keyedit_print_one_sig (ctrl, log_get_stream (), + rc, kb, n, NULL, NULL, NULL, has_selfsig, 0, only_selfsigs); } @@ -624,32 +643,62 @@ key_check_all_keysigs (ctrl_t ctrl, int mode, kbnode_t kb, } } - if (dups || missing_issuer || bad_signature || reordered) - tty_fprintf (fp, _("key %s:\n"), pk_keyid_str (pk)); + if (!opt.quiet) + { + char prefix[100]; + char *p; - if (dups) - tty_fprintf (fp, - ngettext ("%d duplicate signature removed\n", - "%d duplicate signatures removed\n", dups), dups); - if (missing_issuer) - tty_fprintf (fp, - ngettext ("%d signature not checked due to a missing key\n", - "%d signatures not checked due to missing keys\n", - missing_issuer), missing_issuer); - if (bad_signature) - tty_fprintf (fp, - ngettext ("%d bad signature\n", - "%d bad signatures\n", - bad_signature), bad_signature); - if (reordered) - tty_fprintf (fp, - ngettext ("%d signature reordered\n", - "%d signatures reordered\n", - reordered), reordered); + /* To avoid string changes in 2.2 we strip the LF here. */ + snprintf (prefix, sizeof prefix, _("key %s:\n"), pk_keyid_str (pk)); + p = strrchr (prefix, '\n'); + if (p) + *p = 0; - if (only_selfsigs && (bad_signature || reordered)) - tty_fprintf (fp, _("Warning: errors found and only checked self-signatures," - " run '%s' to check all signatures.\n"), "check"); + if (dups) + { + p = xtryasprintf + (ngettext ("%d duplicate signature removed\n", + "%d duplicate signatures removed\n", dups), dups); + print_info (mode, prefix, p); + xfree (p); + } + + if (missing_issuer) + { + p = xtryasprintf + (ngettext ("%d signature not checked due to a missing key\n", + "%d signatures not checked due to missing keys\n", + missing_issuer), missing_issuer); + print_info (mode, prefix, p); + xfree (p); + } + if (bad_signature) + { + p = xtryasprintf (ngettext ("%d bad signature\n", + "%d bad signatures\n", + bad_signature), bad_signature); + print_info (mode, prefix, p); + xfree (p); + } + + if (reordered) + { + p = xtryasprintf (ngettext ("%d signature reordered\n", + "%d signatures reordered\n", + reordered), reordered); + print_info (mode, prefix, p); + xfree (p); + } + + if (only_selfsigs && (bad_signature || reordered)) + { + p = xtryasprintf + (_("Warning: errors found and only checked self-signatures," + " run '%s' to check all signatures.\n"), "check"); + print_info (mode, prefix, p); + xfree (p); + } + } return modified; } From b13972dfbf7224478652038725ab0d2cb41b7303 Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Thu, 26 Oct 2017 11:19:45 +0900 Subject: [PATCH 20/21] Fix comment of configure. * configure.ac (BUILD_WITH_DIRMNGR): Comment fix. Signed-off-by: NIIBE Yutaka --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 551c970b3..dc1fc1afd 100644 --- a/configure.ac +++ b/configure.ac @@ -1754,7 +1754,7 @@ if test "$build_scdaemon" = yes ; then AC_DEFINE(BUILD_WITH_SCDAEMON,1,[Defined if SCDAEMON is to be build]) fi if test "$build_dirmngr" = yes ; then - AC_DEFINE(BUILD_WITH_DIRMNGR,1,[Defined if SCDAEMON is to be build]) + AC_DEFINE(BUILD_WITH_DIRMNGR,1,[Defined if DIRMNGR is to be build]) fi if test "$build_g13" = yes ; then AC_DEFINE(BUILD_WITH_G13,1,[Defined if G13 is to be build]) From 05cb87276c21c3a47226c75026fa46a955553dd9 Mon Sep 17 00:00:00 2001 From: NIIBE Yutaka Date: Thu, 26 Oct 2017 11:24:39 +0900 Subject: [PATCH 21/21] agent, tests: Support --disable-scdaemon build case. * agent/command.c (cmd_scd): Support !BUILD_WITH_SCDAEMON. * tests/openpgp/defs.scm (create-gpghome): Likewise. * tests/gpgsm/gpgsm-defs.scm (create-gpgsmhome): Likewise. -- We could modify gpg-agent to remove all support of scdaemon, with no inclusion of call-scd.c, divert-scd.c, and learncard.c, but it would not be worth to do that. GnuPG-bug-id: 3316 Signed-off-by: NIIBE Yutaka --- agent/command.c | 9 ++++++--- tests/gpgsm/gpgsm-defs.scm | 4 +++- tests/openpgp/defs.scm | 4 +++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/agent/command.c b/agent/command.c index f2a668306..e20361a11 100644 --- a/agent/command.c +++ b/agent/command.c @@ -1988,14 +1988,17 @@ static const char hlp_scd[] = static gpg_error_t cmd_scd (assuan_context_t ctx, char *line) { - ctrl_t ctrl = assuan_get_pointer (ctx); int rc; - +#ifdef BUILD_WITH_SCDAEMON + ctrl_t ctrl = assuan_get_pointer (ctx); if (ctrl->restricted) return leave_cmd (ctx, gpg_error (GPG_ERR_FORBIDDEN)); rc = divert_generic_cmd (ctrl, line, ctx); - +#else + (void)ctx; (void)line; + rc = gpg_error (GPG_ERR_NOT_SUPPORTED); +#endif return rc; } diff --git a/tests/gpgsm/gpgsm-defs.scm b/tests/gpgsm/gpgsm-defs.scm index d99d7dad4..c78a12797 100644 --- a/tests/gpgsm/gpgsm-defs.scm +++ b/tests/gpgsm/gpgsm-defs.scm @@ -67,7 +67,9 @@ "faked-system-time 1008241200") (create-file "gpg-agent.conf" (string-append "pinentry-program " (tool 'pinentry)) - (string-append "scdaemon-program " (tool 'scdaemon)) + (if (assoc "scdaemon" gpg-components) + (string-append "scdaemon-program " (tool 'scdaemon)) + "# No scdaemon available") ) (start-agent) (create-file diff --git a/tests/openpgp/defs.scm b/tests/openpgp/defs.scm index f52f31614..a6347fe1f 100644 --- a/tests/openpgp/defs.scm +++ b/tests/openpgp/defs.scm @@ -354,7 +354,9 @@ (if (flag "--extended-key-format" *args*) "enable-extended-key-format" "#enable-extended-key-format") (string-append "pinentry-program " (tool 'pinentry)) - (string-append "scdaemon-program " (tool 'scdaemon)) + (if (assoc "scdaemon" gpg-components) + (string-append "scdaemon-program " (tool 'scdaemon)) + "# No scdaemon available") )) ;; Initialize the test environment, install appropriate configuration