From a138b218f63864f9e1194be8bfd139c2f6e3f672 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Fri, 5 Jun 2020 15:40:46 +0200 Subject: [PATCH 1/2] Fix S/MIME keylist and improve error handling This fixes an issue where iterating over the key list could get stuck in a loop and after fetching the key for the email address. Also, more GPGME calls are now checked for errors. --- util/gpgmeutils.c | 127 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 95 insertions(+), 32 deletions(-) diff --git a/util/gpgmeutils.c b/util/gpgmeutils.c index b80096ace..0e5e2b010 100644 --- a/util/gpgmeutils.c +++ b/util/gpgmeutils.c @@ -300,17 +300,32 @@ static gpgme_key_t find_email_encryption_key (gpgme_ctx_t ctx, const char *uid_email) { gchar *bracket_email; - gpgme_key_t key; - gboolean recipient_found = FALSE; + gpgme_key_t key, found_key; + gpgme_error_t err; if (uid_email == NULL) return NULL; bracket_email = g_strdup_printf ("<%s>", uid_email); - gpgme_op_keylist_start (ctx, NULL, 0); + err = gpgme_op_keylist_start (ctx, NULL, 0); + if (err) + { + g_free (bracket_email); + g_warning ("gpgme_op_keylist_start failed: %s", gpgme_strerror (err)); + return NULL; + } + gpgme_op_keylist_next (ctx, &key); - while (key && recipient_found == FALSE) + if (err) + { + g_free (bracket_email); + g_warning ("gpgme_op_keylist_next failed: %s", gpgme_strerror (err)); + return NULL; + } + + found_key = NULL; + while (key) { if (key->can_encrypt) { @@ -319,7 +334,7 @@ find_email_encryption_key (gpgme_ctx_t ctx, const char *uid_email) gpgme_user_id_t uid; uid = key->uids; - while (uid && recipient_found == FALSE) + while (uid && found_key == NULL) { g_debug ("%s: UID email: %s", __func__, uid->email); @@ -328,7 +343,7 @@ find_email_encryption_key (gpgme_ctx_t ctx, const char *uid_email) { g_message ("%s: Found matching UID for %s", __func__, uid_email); - recipient_found = TRUE; + found_key = key; } uid = uid->next; } @@ -339,17 +354,21 @@ find_email_encryption_key (gpgme_ctx_t ctx, const char *uid_email) key->subkeys->fpr); } - if (recipient_found == FALSE) - gpgme_op_keylist_next (ctx, &key); + err = gpgme_op_keylist_next (ctx, &key); + if (err & GPG_ERR_EOF) + break; + else if (err) + { + g_free (bracket_email); + g_warning ("gpgme_op_keylist_next failed: %s", gpgme_strerror (err)); + return NULL; + } } - if (recipient_found) - return key; - else - { - g_warning ("%s: No suitable key found for %s", __func__, uid_email); - return NULL; - } + if (found_key == NULL) + g_warning ("%s: No suitable key found for %s", __func__, uid_email); + + return found_key; } /** @@ -394,6 +413,14 @@ gvm_gpgme_fwrite (void *handle, const void *buffer, size_t size) return ret; } +#define CHECK_ERR(func) \ + if (err) \ + { \ + printf ("%s: %s failed: %s\n", \ + __func__, func, gpgme_strerror (err)); \ + return -1; \ + } + /** * @brief Adds a trust list of all current certificates to a GPG homedir. * @@ -414,6 +441,7 @@ create_all_certificates_trustlist (gpgme_ctx_t ctx, const char *homedir) gchar *trustlist_filename; GString *trustlist_content; GError *g_err; + gpgme_error_t err; g_err = NULL; gpgme_set_pinentry_mode (ctx, GPGME_PINENTRY_MODE_CANCEL); @@ -422,12 +450,18 @@ create_all_certificates_trustlist (gpgme_ctx_t ctx, const char *homedir) trustlist_content = g_string_new (""); - gpgme_op_keylist_start (ctx, NULL, 0); + err = gpgme_op_keylist_start (ctx, NULL, 0); + CHECK_ERR ("gpgme_op_keylist_start") gpgme_op_keylist_next (ctx, &key); + CHECK_ERR ("gpgme_op_keylist_next") while (key) { g_string_append_printf (trustlist_content, "%s S\n", key->fpr); - gpgme_op_keylist_next (ctx, &key); + err = gpgme_op_keylist_next (ctx, &key); + if (err & GPG_ERR_EOF) + break; + else + CHECK_ERR ("gpgme_op_keylist_next") } if (g_file_set_contents (trustlist_filename, trustlist_content->str, @@ -447,6 +481,22 @@ create_all_certificates_trustlist (gpgme_ctx_t ctx, const char *homedir) return 0; } +#undef CHECK_ERR +#define CHECK_ERR(func) \ + if (err) \ + { \ + printf ("%s: %s failed: %s\n", \ + __func__, func, gpgme_strerror (err)); \ + if (plain_data) \ + gpgme_data_release (plain_data); \ + if (encrypted_data) \ + gpgme_data_release (encrypted_data); \ + if (ctx) \ + gpgme_release (ctx); \ + gvm_file_remove_recurse (gpg_temp_dir); \ + return -1; \ + } + /** * @brief Encrypt a stream for a PGP public key, writing to another stream. * @@ -478,6 +528,10 @@ encrypt_stream_internal (FILE *plain_file, FILE *encrypted_file, const char *key_type_str; struct gpgme_data_cbs callbacks; + ctx = NULL; + plain_data = NULL; + encrypted_data = NULL; + if (uid_email == NULL || strcmp (uid_email, "") == 0) { g_warning ("%s: No email address for user identification given", @@ -485,6 +539,12 @@ encrypt_stream_internal (FILE *plain_file, FILE *encrypted_file, return -1; } + if (gpgme_check_version (NULL) == NULL) + { + g_warning ("%s: gpgme_check_version failed", __func__); + return -1; + } + if (protocol == GPGME_PROTOCOL_CMS) key_type_str = "certificate"; else @@ -497,15 +557,25 @@ encrypt_stream_internal (FILE *plain_file, FILE *encrypted_file, return -1; } - gpgme_new (&ctx); + err = gpgme_new (&ctx); + CHECK_ERR ("gpgme_new") if (protocol == GPGME_PROTOCOL_CMS) gpgme_set_armor (ctx, 0); else gpgme_set_armor (ctx, 1); - gpgme_ctx_set_engine_info (ctx, protocol, NULL, gpg_temp_dir); - gpgme_set_protocol (ctx, protocol); + err = gpgme_ctx_set_engine_info (ctx, protocol, NULL, gpg_temp_dir); + CHECK_ERR ("gpgme_ctx_set_engine_info") + + err = gpgme_set_protocol (ctx, protocol); + CHECK_ERR ("gpgme_set_protocol") + + err = gpgme_set_keylist_mode (ctx, GPGME_KEYLIST_MODE_LOCAL); + CHECK_ERR ("gpgme_set_keylist_mode") + + gpgme_set_offline (ctx, 1); + encrypt_flags = GPGME_ENCRYPT_ALWAYS_TRUST | GPGME_ENCRYPT_NO_COMPRESS; // Import public key into context @@ -530,7 +600,8 @@ encrypt_stream_internal (FILE *plain_file, FILE *encrypted_file, keys[0] = key; // Set up data objects for input and output streams - gpgme_data_new_from_stream (&plain_data, plain_file); + err = gpgme_data_new_from_stream (&plain_data, plain_file); + CHECK_ERR ("gpgme_data_new_from_stream for plain text") /* Create a GPGME data buffer with custom read and write functions. * @@ -539,7 +610,8 @@ encrypt_stream_internal (FILE *plain_file, FILE *encrypted_file, memset (&callbacks, 0, sizeof (callbacks)); callbacks.read = gvm_gpgme_fread; callbacks.write = gvm_gpgme_fwrite; - gpgme_data_new_from_cbs (&encrypted_data, &callbacks, encrypted_file); + err = gpgme_data_new_from_cbs (&encrypted_data, &callbacks, encrypted_file); + CHECK_ERR ("gpgme_data_new_from_stream for encrypted text") if (protocol == GPGME_PROTOCOL_CMS) { @@ -557,16 +629,7 @@ encrypt_stream_internal (FILE *plain_file, FILE *encrypted_file, // Encrypt data err = gpgme_op_encrypt (ctx, keys, encrypt_flags, plain_data, encrypted_data); - - if (err) - { - g_warning ("%s: Encryption failed: %s", __func__, gpgme_strerror (err)); - gpgme_data_release (plain_data); - gpgme_data_release (encrypted_data); - gpgme_release (ctx); - gvm_file_remove_recurse (gpg_temp_dir); - return -1; - } + CHECK_ERR ("gpgme_op_encrypt") gpgme_data_release (plain_data); gpgme_data_release (encrypted_data); From 0e562446a4d2ec2daaa6c44645d95a643c7804f8 Mon Sep 17 00:00:00 2001 From: Timo Pollmeier Date: Fri, 5 Jun 2020 16:32:52 +0200 Subject: [PATCH 2/2] Add S/MIME fix to CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9b66c8f9..6f3b0911b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Fix is_cidr_block(). [#322](https://github.com/greenbone/gvm-libs/pull/322) - Fix is_cidr6_block() and is_short_range_network(). [#337](https://github.com/greenbone/gvm-libs/pull/337) +- Fix S/MIME keylist and improve error handling [#345](https://github.com/greenbone/gvm-libs/pull/345) [20.8]: https://github.com/greenbone/gvm-libs/compare/gvm-libs-11.0...master