After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 668085 - GPG signature verification fails if signer's public key is missing
GPG signature verification fails if signer's public key is missing
Status: RESOLVED FIXED
Product: gmime
Classification: Other
Component: general
2.6.x
Other Linux
: Normal major
: ---
Assigned To: Jeffrey Stedfast
Jeffrey Stedfast
Depends on:
Blocks:
 
 
Reported: 2012-01-17 11:17 UTC by Thomas Jost
Modified: 2012-02-21 14:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
quick workaround (749 bytes, patch)
2012-01-17 13:00 UTC, Thomas Jost
none Details | Review

Description Thomas Jost 2012-01-17 11:17:26 UTC
(This bug was found while trying to add gmime-2.6 support to notmuch: http://notmuchmail.org/pipermail/notmuch/2012/007937.html)

When verifying signatures of a signed message, g_mime_multipart_signed_verify() returns NULL if a signer's public key is missing. As a result, a program using gmime can't tell why the signature verification fails.

The expected behaviour would be to return a valid GMimeSignatureList*, with the signature having status GMIME_SIGNATURE_STATUS_ERROR and error GMIME_SIGNATURE_ERROR_NO_PUBKEY.

The problem comes from the gpg_verify() function in gmime/gmime-gpg-context.c. In case of NO_PUBKEY, gpg exits with code 2, which results in entering this branch:

	if (gpg_ctx_op_wait (gpg) != 0) {
		const char *diagnostics;
		int save;
		
		save = errno;
		diagnostics = gpg_ctx_get_diagnostics (gpg);
		errno = save;
		
		g_set_error_literal (err, GMIME_ERROR, errno, diagnostics);
		gpg_ctx_free (gpg);
		
		return NULL;
	}

A quick and dirty workaround is to remove the last 2 lines (gpg_ctx_free() and return NULL), which results in gpg_verify() returning the GMimeSignatureList* it has built before regardless of gpg exit code. By doing this I get the expected behaviour in notmuch. But I don't know much about gmime so I can't tell if this results in a correct behaviour in case of a real error in gpg.

Could you please have a look at this issue? This is an annoying regression compared to gmime 2.4. Please don't hesitate to ask me if you need testing.
Comment 1 Thomas Jost 2012-01-17 13:00:30 UTC
Created attachment 205441 [details] [review]
quick workaround
Comment 2 Jeffrey Stedfast 2012-02-17 15:01:01 UTC
Hey Thomas, sorry for the delay in getting back to you.

With this failure, is there anything in the diagnostics string that is at all useful? Or can I just drop setting the GError altogether?

I'd rather not return a list and at the same time set the GError argument as that will likely lead programmers to not check if err is set.

I tend to like my APIs to only set err if we return a value that is clearly an error (e.g. NULL).


Here's what I'm thinking:

diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
index 439b023..411a23b 100644
--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -1898,7 +1898,8 @@ gpg_verify (GMimeCryptoContext *context, GMimeDigestAlgo digest,
                }
        }
        
-       if (gpg_ctx_op_wait (gpg) != 0) {
+       /* Only set the GError if we got no signature information from gpg */
+       if (gpg_ctx_op_wait (gpg) != 0 && !gpg->signatures) {
                const char *diagnostics;
                int save;


Would this work for you?
Comment 3 Jeffrey Stedfast 2012-02-17 15:05:41 UTC
I've committed the above patch, let me know if this doesn't fix the issue for you.
Comment 4 Thomas Jost 2012-02-21 14:43:30 UTC
It seems to work fine, thanks!