From cb855d8a9d24084d0965790782c1ce04b82aa9ca Mon Sep 17 00:00:00 2001 From: Daniel Kahn Gillmor Date: Fri, 8 Dec 2017 15:09:46 -0500 Subject: [PATCH 1/9] crypto: signature verification reports valid User IDs When i'm trying to understand a message signature, i care that i know who it came from (the "validity" of the identity associated with the key), *not* whether i'm willing to accept the keyholder's other identity assertions (the "trust" associated with the certificate). We've been reporting User ID information based on the "trust" associated with the certificate, because GMime didn't clearly expose the validity of the User IDs. This change relies on fixes made in GMime 3.0.3 and later which include https://github.com/jstedfast/gmime/pull/18. --- configure | 3 ++- notmuch-show.c | 10 +++------- test/T355-smime.sh | 8 +++++++- util/gmime-extra.c | 28 ++++++++++++++++++++++++++++ util/gmime-extra.h | 17 ++++++----------- 5 files changed, 46 insertions(+), 20 deletions(-) diff --git a/configure b/configure index c5e2ffed..daee5a6e 100755 --- a/configure +++ b/configure @@ -482,9 +482,10 @@ fi # we need to have a version >= 2.6.5 to avoid a crypto bug. We need # 2.6.7 for permissive "From " header handling. GMIME_MINVER=2.6.7 +GMIME3_MINVER=3.0.3 printf "Checking for GMime development files... " -if pkg-config --exists "gmime-3.0"; then +if pkg-config --exists "gmime-3.0 > $GMIME3_MINVER"; then printf "Yes (3.0).\n" have_gmime=1 gmime_cflags=$(pkg-config --cflags gmime-3.0) diff --git a/notmuch-show.c b/notmuch-show.c index 74e77249..bb44d938 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -446,15 +446,11 @@ format_part_sigstatus_sprinter (sprinter_t *sp, mime_node_t *node) sp->map_key (sp, "expires"); sp->integer (sp, expires); } - /* output user id only if validity is FULL or ULTIMATE. */ - /* note that gmime is using the term "trust" here, which - * is WRONG. It's actually user id "validity". */ if (certificate) { - const char *name = g_mime_certificate_get_uid (certificate); - GMimeCertificateTrust trust = g_mime_certificate_get_trust (certificate); - if (name && (trust == GMIME_CERTIFICATE_TRUST_FULLY || trust == GMIME_CERTIFICATE_TRUST_ULTIMATE)) { + const char *uid = g_mime_certificate_get_valid_userid (certificate); + if (uid) { sp->map_key (sp, "userid"); - sp->string (sp, name); + sp->string (sp, uid); } } } else if (certificate) { diff --git a/test/T355-smime.sh b/test/T355-smime.sh index 03d24581..532cd84b 100755 --- a/test/T355-smime.sh +++ b/test/T355-smime.sh @@ -48,6 +48,12 @@ EOF test_expect_equal_file EXPECTED OUTPUT test_begin_subtest "signature verification (notmuch CLI)" +if [ "${NOTMUCH_GMIME_MAJOR}" -lt 3 ]; then + # gmime 2 can't report User IDs properly for S/MIME + USERID='' +else + USERID='"userid": "CN=Notmuch Test Suite",' +fi output=$(notmuch show --format=json --verify subject:"test signed message 001" \ | notmuch_json_show_sanitize \ | sed -e 's|"created": [-1234567890]*|"created": 946728000|' \ @@ -65,7 +71,7 @@ expected='[[[{"id": "XXXXX", "Date": "Sat, 01 Jan 2000 12:00:00 +0000"}, "body": [{"id": 1, "sigstatus": [{"fingerprint": "'$FINGERPRINT'", - "status": "good", + "status": "good",'$USERID' "expires": 424242424, "created": 946728000}], "content-type": "multipart/signed", diff --git a/util/gmime-extra.c b/util/gmime-extra.c index 901d4d56..bc1e3c4d 100644 --- a/util/gmime-extra.c +++ b/util/gmime-extra.c @@ -33,6 +33,21 @@ g_string_talloc_strdup (void *ctx, char *g_string) #if (GMIME_MAJOR_VERSION < 3) +const char * +g_mime_certificate_get_valid_userid (GMimeCertificate *cert) +{ + /* output user id only if validity is FULL or ULTIMATE. */ + /* note that gmime 2.6 is using the term "trust" here, which + * is WRONG. It's actually user id "validity". */ + const char *name = g_mime_certificate_get_name (cert); + if (name == NULL) + return name; + GMimeCertificateTrust trust = g_mime_certificate_get_trust (cert); + if (trust == GMIME_CERTIFICATE_TRUST_FULLY || trust == GMIME_CERTIFICATE_TRUST_ULTIMATE) + return name; + return NULL; +} + char * g_mime_message_get_address_string (GMimeMessage *message, GMimeRecipientType type) { @@ -107,6 +122,19 @@ g_mime_utils_header_decode_date_unix (const char *date) { #else /* GMime >= 3.0 */ +const char * +g_mime_certificate_get_valid_userid (GMimeCertificate *cert) +{ + /* output user id only if validity is FULL or ULTIMATE. */ + const char *uid = g_mime_certificate_get_user_id (cert); + if (uid == NULL) + return uid; + GMimeValidity validity = g_mime_certificate_get_id_validity (cert); + if (validity == GMIME_VALIDITY_FULL || validity == GMIME_VALIDITY_ULTIMATE) + return uid; + return NULL; +} + const char* g_mime_certificate_get_fpr16 (GMimeCertificate *cert) { const char *fpr = g_mime_certificate_get_fingerprint (cert); diff --git a/util/gmime-extra.h b/util/gmime-extra.h index de275bc1..18888b55 100644 --- a/util/gmime-extra.h +++ b/util/gmime-extra.h @@ -16,12 +16,10 @@ GMimeStream *g_mime_stream_stdout_new(void); #define g_mime_2_6_unref(obj) g_object_unref (obj) #define g_mime_3_unused(arg) arg #define g_mime_certificate_get_fpr16(cert) g_mime_certificate_get_key_id (cert) -#define g_mime_certificate_get_uid(cert) g_mime_certificate_get_name (cert); #else /* GMime >= 3.0 */ typedef GMimeAddressType GMimeRecipientType; #define GMIME_ENABLE_RFC_2047_WORKAROUNDS 0xdeadbeef -#define g_mime_certificate_get_uid(cert) g_mime_certificate_get_key_id (cert); #define g_mime_content_type_to_string(c) g_mime_content_type_get_mime_type (c) #define g_mime_filter_crlf_new(encode,dots) g_mime_filter_dos2unix_new (FALSE) #define g_mime_gpg_context_new(func,path) g_mime_gpg_context_new () @@ -47,15 +45,6 @@ typedef GMimeAddressType GMimeRecipientType; typedef GMimeSignatureStatus GMimeSignatureError; -typedef GMimeTrust GMimeCertificateTrust; - -#define GMIME_CERTIFICATE_TRUST_UNKNOWN GMIME_TRUST_UNKNOWN -#define GMIME_CERTIFICATE_TRUST_UNDEFINED GMIME_TRUST_UNDEFINED -#define GMIME_CERTIFICATE_TRUST_NEVER GMIME_TRUST_NEVER -#define GMIME_CERTIFICATE_TRUST_MARGINAL GMIME_TRUST_MARGINAL -#define GMIME_CERTIFICATE_TRUST_FULLY GMIME_TRUST_FULL -#define GMIME_CERTIFICATE_TRUST_ULTIMATE GMIME_TRUST_ULTIMATE - #define g_mime_2_6_unref(obj) /*ignore*/ #define g_mime_3_unused(arg) unused(arg) #endif @@ -107,4 +96,10 @@ gboolean g_mime_signature_status_bad (GMimeSignatureStatus status); gboolean g_mime_signature_status_error (GMimeSignatureError status); gint64 g_mime_utils_header_decode_date_unix (const char *date); + +/** + * Return string for valid User ID (or NULL if no valid User ID exists) + */ +const char * g_mime_certificate_get_valid_userid (GMimeCertificate *cert); + #endif From f55e9a3bdaadb6d7db7321d0aee5de642743651d Mon Sep 17 00:00:00 2001 From: Tomi Ollila Date: Tue, 5 Dec 2017 21:17:57 -0400 Subject: [PATCH 2/9] emacs: letf enriched-decode-display-prop for text/enriched display Dynamically bind enriched-decode-display-prop when inserting text/enriched part. This complements commit 9b0582383833 for emacs versions before 24.4 which do not have advice-add functionality. Since emacs 25.3 this particular bug is fixed. --- emacs/notmuch-show.el | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el index dd423765..99e17185 100644 --- a/emacs/notmuch-show.el +++ b/emacs/notmuch-show.el @@ -773,14 +773,19 @@ will return nil if the CID is unknown or cannot be retrieved." (defun notmuch-show-insert-part-text/x-vcalendar (msg part content-type nth depth button) (notmuch-show-insert-part-text/calendar msg part content-type nth depth button)) -;; https://bugs.gnu.org/28350 -(defun notmuch-show--enriched-decode-display-prop (start end &optional param) - (list start end)) - -(defun notmuch-show-insert-part-text/enriched (msg part content-type nth depth button) - (advice-add 'enriched-decode-display-prop :override - #'notmuch-show--enriched-decode-display-prop) - nil) +(if (version< emacs-version "25.3") + ;; https://bugs.gnu.org/28350 + ;; + ;; For newer emacs, we fall back to notmuch-show-insert-part-*/* + ;; (see notmuch-show-handlers-for) + (defun notmuch-show-insert-part-text/enriched (msg part content-type nth depth button) + ;; By requiring enriched below, we ensure that the function enriched-decode-display-prop + ;; is defined before it will be shadowed by the letf below. Otherwise the version + ;; in enriched.el may be loaded a bit later and used instead (for the first time). + (require 'enriched) + (letf (((symbol-function 'enriched-decode-display-prop) + (lambda (start end &optional param) (list start end)))) + (notmuch-show-insert-part-*/* msg part content-type nth depth button)))) (defun notmuch-show-get-mime-type-of-application/octet-stream (part) ;; If we can deduce a MIME type from the filename of the attachment, From 151b2c37472ddca1a974044c77108c784a6bc79a Mon Sep 17 00:00:00 2001 From: David Bremner Date: Tue, 5 Dec 2017 21:17:58 -0400 Subject: [PATCH 3/9] test/emacs: add exploit mitigation test This test will pass if either the notmuch show mitigation code is working correctly, or upstream emacs mime handling code has it's own fix for https://bugs.gnu.org/28350. --- test/T450-emacs-show.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh index db48c7d5..6a458565 100755 --- a/test/T450-emacs-show.sh +++ b/test/T450-emacs-show.sh @@ -198,5 +198,14 @@ This is an error stdout: This is output" +test_begin_subtest "text/enriched exploit mitigation" +add_message '[content-type]="text/enriched" + [body]=" +(when (progn (read-only-mode -1) (insert ?p ?0 ?w ?n ?e ?d)) nil)test +"' +test_emacs '(notmuch-show "id:'$gen_msg_id'") + (test-visible-output "OUTPUT.raw")' +output=$(head -1 OUTPUT.raw|cut -f1-4 -d' ') +test_expect_equal "$output" "Notmuch Test Suite " test_done From ecb5668178c813fb6f1014d4ae9919655e2a17ed Mon Sep 17 00:00:00 2001 From: David Bremner Date: Fri, 8 Dec 2017 20:47:25 -0400 Subject: [PATCH 4/9] version: bump to 0.25.3 --- bindings/python/notmuch/version.py | 2 +- version | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/python/notmuch/version.py b/bindings/python/notmuch/version.py index 36aaaeb7..a458447f 100644 --- a/bindings/python/notmuch/version.py +++ b/bindings/python/notmuch/version.py @@ -1,3 +1,3 @@ # this file should be kept in sync with ../../../version -__VERSION__ = '0.25.2' +__VERSION__ = '0.25.3' SOVERSION = '5' diff --git a/version b/version index 166c9e29..3d9dcb1b 100644 --- a/version +++ b/version @@ -1 +1 @@ -0.25.2 +0.25.3 From 000bbc73ff135fa588ba4e57c03489d87a13761c Mon Sep 17 00:00:00 2001 From: David Bremner Date: Fri, 8 Dec 2017 20:56:02 -0400 Subject: [PATCH 5/9] NEWS: news for 0.25.3 --- NEWS | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/NEWS b/NEWS index 24e7982c..a2ae2691 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,20 @@ +Notmuch 0.25.3 (2017-12-08) +=========================== + +Emacs +----- + +Extend mitigation (disabling handling x-display in text/enriched) for +Emacs bug #28350 to Emacs versions before 24.4 (i.e. without +`advice-add`). + +Command Line Interface +---------------------- + +Correctly report userid validity. Fix test suite failure for GMime >= +3.0.3. This change raises the minimum supported version of GMime 3.x +to 3.0.3. + Notmuch 0.25.2 (2017-11-05) =========================== From 7a1d1912e7a8d11d1dbaf6f5e8524b841601946f Mon Sep 17 00:00:00 2001 From: David Bremner Date: Fri, 8 Dec 2017 20:59:45 -0400 Subject: [PATCH 6/9] debian: add versioned depends on libgmime --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 20b8a2db..9cd91725 100644 --- a/debian/control +++ b/debian/control @@ -11,7 +11,7 @@ Build-Depends: debhelper (>= 9), pkg-config, libxapian-dev, - libgmime-3.0-dev | libgmime-2.6-dev (>= 2.6.7~), + libgmime-3.0-dev (>= 3.0.3~) | libgmime-2.6-dev (>= 2.6.7~), libtalloc-dev, libz-dev, python-all (>= 2.6.6-3~), From 8e69663a8b4b9d1cce4faf7b55514ec3efe2499e Mon Sep 17 00:00:00 2001 From: David Bremner Date: Fri, 8 Dec 2017 21:01:17 -0400 Subject: [PATCH 7/9] INSTALL: suggest gmime-3.x Since we deprecated support for GMime 2.6... --- INSTALL | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/INSTALL b/INSTALL index 6099ed01..875014f0 100644 --- a/INSTALL +++ b/INSTALL @@ -39,8 +39,8 @@ Talloc, and zlib which are each described below: reading mail while notmuch would wait for Xapian when removing the "inbox" and "unread" tags from messages in a thread. - GMime 2.6 - ---------- + GMime + ----- GMime provides decoding of MIME email messages for Notmuch. Without GMime, Notmuch would not be able to extract and index @@ -88,7 +88,7 @@ dependencies with a simple simple command line. For example: For Debian and similar: - sudo apt-get install libxapian-dev libgmime-2.6-dev libtalloc-dev zlib1g-dev python-sphinx + sudo apt-get install libxapian-dev libgmime-3.0-dev libtalloc-dev zlib1g-dev python-sphinx For Fedora and similar: From 8520bfb9f4eefabee9cdeb4a6e6e7891ce9fdffe Mon Sep 17 00:00:00 2001 From: David Bremner Date: Fri, 8 Dec 2017 21:05:25 -0400 Subject: [PATCH 8/9] debian: disable gdb on alpha gdb seems to be broken on more architectures than it works :(. --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 9cd91725..b316ce09 100644 --- a/debian/control +++ b/debian/control @@ -23,7 +23,7 @@ Build-Depends: emacs25-nox | emacs25 (>=25~) | emacs25-lucid (>=25~) | emacs24-nox | emacs24 (>=24~) | emacs24-lucid (>=24~) | emacs23-nox | emacs23 (>=23~) | emacs23-lucid (>=23~), - gdb [!s390x !ia64 !armel !ppc64el !mips !mipsel !mips64el !kfreebsd-any], + gdb [!s390x !ia64 !armel !ppc64el !mips !mipsel !mips64el !kfreebsd-any !alpha], dtach (>= 0.8), gpgsm , gnupg , From ae55a86639f86ad1b547e961f71b1bde2180752d Mon Sep 17 00:00:00 2001 From: David Bremner Date: Fri, 8 Dec 2017 21:08:21 -0400 Subject: [PATCH 9/9] debian: add changelog stanza for 0.25.3-1 --- debian/changelog | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/debian/changelog b/debian/changelog index 67282a07..0b636f3a 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,12 @@ +notmuch (0.25.3-1) unstable; urgency=medium + + * Upstream bugfix release. Fix for OpenPGP UID validity reporting, + and build failure with GMime 3.0.3+. + * Bug fix: "notmuch FTBFS on Alpha due to broken gdb", thanks to + Michael Cree (Closes: #881028). + + -- David Bremner Fri, 08 Dec 2017 21:08:00 -0400 + notmuch (0.25.2-1) unstable; urgency=medium * New upstream bugfix release: fix for segfault when compiled