unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* difficulty in rendering S/MIME signature status from some certificates
@ 2021-05-26 23:32 Daniel Kahn Gillmor
  2021-05-27  1:44 ` [PATCH 1/2] cli/show: produce "email" element in sigstatus Daniel Kahn Gillmor
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Kahn Gillmor @ 2021-05-26 23:32 UTC (permalink / raw)
  To: Notmuch Mail


[-- Attachment #1.1: Type: text/plain, Size: 2499 bytes --]

Hi folks--

id:e4a7efe92433f7c3e5dedeac0ea4efc003020296.camel@ericsson.com from the
IETF LAMPS WG mailing list <spasm@ietf.org> can be found at 
https://mailarchive.ietf.org/arch/msg/quic/FpbJTLXsoFLSNr--LgxCOC6IssY.
when rendering it, notmuch-emacs shows:

     [ Good signature by: 1.2.840.113549.1.9.1=#6D61676E75732E7765737465726C756E64406572696373736F6E2E636F6D,CN=Magnus Westerlund,O=Ericsson ]

1.2.840.113549.1.9.1 is the OID for emailAddress in distinguished names.
This field which is deprecated according to
https://www.oid-info.com/get/1.2.840.113549.1.9.1 and §5.2.1 of
https://datatracker.ietf.org/doc/html/rfc2985#page-7, but it is still
very much in use as evidenced by the message's X.509 certificate, which
was issued less than a year ago (in 2020-12-11) with a 3-year validity
window.

I wanted this to make this visible in notmuch as the more legible form
"EMAIL=magnus.westerlund@ericsson.com,CN=Magnus Westerlund,O=Ericsson".
i tracked it down to a constraint in libksba's parsing code, and
reported it to GnuPG (libksba's upstream) here:
https://dev.gnupg.org/T5450 and the fix was rejected.

I don't think that notmuch should try to contain any string-to-DN
parsing code, and notmuch's use of gmime here is basically a passthrough
from gpgme, so i'm a bit stuck.

It occurs to me that maybe notmuch should be identifying the e-mail
address (and only the e-mail address?) instead of the other elements of
the user ID, which are more dubious than the e-mail address anyway.

It seems possible to do this by using the g_mime_certificate's email
field in preference to the g_mime_certificate's user_id field, at least
for this particular certificate, because it contains a subjectAltName of
type rfc822name that is just the e-mail address.

I see a couple different options available to do this:

 a) add an "email?" field to the "signature" object in devel/schemata,
    and then teach notmuch-emacs to render that instead of the userid
    field in cases where it's present.

 b) replace the content of the userid field in the "signature" object
    with the e-mail address entirely.

I'm leaning toward (a), though it requires fiddling in more places.  At
the very least, the first step of (a) doesn't seem objectionable.

(note: GMimeCertificate's email field itself is of somewhat dubious
provenance, and i'm trying to clean that up at
https://github.com/jstedfast/gmime/pull/102)

What do folks think?

        --dkg

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] cli/show: produce "email" element in sigstatus
  2021-05-26 23:32 difficulty in rendering S/MIME signature status from some certificates Daniel Kahn Gillmor
@ 2021-05-27  1:44 ` Daniel Kahn Gillmor
  2021-05-27  1:44   ` [PATCH 2/2] emacs: Prefer email address over User ID when showing valid signature Daniel Kahn Gillmor
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Kahn Gillmor @ 2021-05-27  1:44 UTC (permalink / raw)
  To: Notmuch Mail

When the certificate that signs a message is known to be valid, GMime
is capable of reporting on the e-mail address embedded in the
certificate.

We pass this information along to the caller of "notmuch show", as
often only the e-mail address of the certificate has actually been
checked/verified.

Furthermore, signature verification should probably at some point
compare the e-mail address of the caller against the sender address of
the message itself.  Having to parse what gmime thinks is a "userid"
to extract an e-mail address seems clunky and unnecessary if gmime
already thinks it knows what the e-mail address is.

See id:878s41ax6t.fsf@fifthhorseman.net for more motivation and discussion.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 devel/schemata                 |  1 +
 notmuch-show.c                 |  5 +++++
 test/T350-crypto.sh            |  6 ++++--
 test/T355-smime.sh             |  3 ++-
 test/T356-protected-headers.sh |  8 ++++----
 test/test-lib.sh               |  1 +
 util/gmime-extra.c             | 15 +++++++++++++++
 util/gmime-extra.h             |  4 ++++
 8 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/devel/schemata b/devel/schemata
index 28332c6b..ae84a528 100644
--- a/devel/schemata
+++ b/devel/schemata
@@ -158,6 +158,7 @@ signature = {
     created?:       unix_time,
     expires?:       unix_time,
     userid?:        string
+    email?:         string
     # if status is not "good":
     keyid?:         string
     errors?: 	    sig_errors
diff --git a/notmuch-show.c b/notmuch-show.c
index bdb87321..232557d5 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -475,6 +475,11 @@ format_part_sigstatus_sprinter (sprinter_t *sp, GMimeSignatureList *siglist)
 		    sp->map_key (sp, "userid");
 		    sp->string (sp, uid);
 		}
+		const char *email = g_mime_certificate_get_valid_email (certificate);
+		if (email) {
+		    sp->map_key (sp, "email");
+		    sp->string (sp, email);
+		}
 	    }
 	} else if (certificate) {
 	    const char *key_id = g_mime_certificate_get_fpr16 (certificate);
diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 4508c984..a25c4b0b 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -35,7 +35,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
- "crypto": {"signed": {"status": [{ "status": "good", "created": 946728000, "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'"}]}},
+ "crypto": {"signed": {"status": [{ "status": "good", "created": 946728000, "email": "'"$SELF_EMAIL"'", "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'"}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -44,6 +44,7 @@ expected='[[[{"id": "XXXXX",
  "sigstatus": [{"status": "good",
  "fingerprint": "'$FINGERPRINT'",
  "created": 946728000,
+ "email": "'"$SELF_EMAIL"'",
  "userid": "'"$SELF_USERID"'"}],
  "content-type": "multipart/signed",
  "content": [{"id": 2,
@@ -367,7 +368,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["encrypted","inbox"],
- "crypto": {"signed": {"status": [{ "status": "good", "created": 946728000, "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'"}],
+ "crypto": {"signed": {"status": [{ "status": "good", "created": 946728000, "fingerprint": "'$FINGERPRINT'", "email": "'"$SELF_EMAIL"'", "userid": "'"$SELF_USERID"'"}],
                        "encrypted": true },
             "decrypted": {"status": "full"}},
  "headers": {"Subject": "test encrypted message 002",
@@ -379,6 +380,7 @@ expected='[[[{"id": "XXXXX",
  "sigstatus": [{"status": "good",
  "fingerprint": "'$FINGERPRINT'",
  "created": 946728000,
+ "email": "'"$SELF_EMAIL"'",
  "userid": "'"$SELF_USERID"'"}],
  "content-type": "multipart/encrypted",
  "content": [{"id": 2,
diff --git a/test/T355-smime.sh b/test/T355-smime.sh
index 69bdcfac..9c6e7340 100755
--- a/test/T355-smime.sh
+++ b/test/T355-smime.sh
@@ -46,7 +46,7 @@ expected='[[[{"id": "XXXXX",
  "timestamp": 946728000,
  "date_relative": "2000-01-01",
  "tags": ["inbox","signed"],
- "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite","expires": 424242424, "created": 946728000}]}},
+ "crypto": {"signed": {"status": [{"fingerprint": "'$FINGERPRINT'", "status": "good","userid": "CN=Notmuch Test Suite", "email": "<test_suite@notmuchmail.org>", "expires": 424242424, "created": 946728000}]}},
  "headers": {"Subject": "test signed message 001",
  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
  "To": "test_suite@notmuchmail.org",
@@ -55,6 +55,7 @@ expected='[[[{"id": "XXXXX",
  "sigstatus": [{"fingerprint": "'$FINGERPRINT'",
  "status": "good",
  "userid": "CN=Notmuch Test Suite",
+ "email": "<test_suite@notmuchmail.org>",
  "expires": 424242424,
  "created": 946728000}],
  "content-type": "multipart/signed",
diff --git a/test/T356-protected-headers.sh b/test/T356-protected-headers.sh
index 074a2345..f0aba14e 100755
--- a/test/T356-protected-headers.sh
+++ b/test/T356-protected-headers.sh
@@ -69,12 +69,12 @@ test_json_nodes <<<"$output" \
 test_begin_subtest "show cryptographic envelope on signed mail"
 output=$(notmuch show --verify --format=json id:simple-signed-mail@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
-                'crypto:[0][0][0]["crypto"]={"signed": {"status": [{"created": 1525609971, "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "status": "good"}]}}'
+                'crypto:[0][0][0]["crypto"]={"signed": {"status": [{"created": 1525609971, "fingerprint": "'$FINGERPRINT'", "email": "'"$SELF_EMAIL"'", "userid": "'"$SELF_USERID"'", "status": "good"}]}}'
 
 test_begin_subtest "verify signed protected header"
 output=$(notmuch show --verify --format=json id:signed-protected-header@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
-                'crypto:[0][0][0]["crypto"]={"signed": {"status": [{"created": 1525350527, "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "status": "good"}], "headers": ["Subject"]}}'
+                'crypto:[0][0][0]["crypto"]={"signed": {"status": [{"created": 1525350527, "fingerprint": "'$FINGERPRINT'", "email": "'"$SELF_EMAIL"'", "userid": "'"$SELF_USERID"'", "status": "good"}], "headers": ["Subject"]}}'
 
 test_begin_subtest "protected subject does not leak by default in replies"
 output=$(notmuch reply --decrypt=true --format=json id:protected-header@crypto.notmuchmail.org)
@@ -115,7 +115,7 @@ test_begin_subtest "verify protected header is both signed and encrypted"
 output=$(notmuch show --decrypt=true --format=json id:encrypted-signed@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]={
-                   "signed":{"status": [{"status": "good", "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "created": 1525812676}],
+                   "signed":{"status": [{"status": "good", "fingerprint": "'$FINGERPRINT'", "email": "'"$SELF_EMAIL"'", "userid": "'"$SELF_USERID"'", "created": 1525812676}],
                    "encrypted": true, "headers": ["Subject"]},"decrypted": {"status": "full", "header-mask": {"Subject": "Subject Unavailable"}}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="Rhinoceros dinner"'
 
@@ -123,7 +123,7 @@ test_begin_subtest "verify protected header is signed even when not masked"
 output=$(notmuch show --decrypt=true --format=json id:encrypted-signed-not-masked@crypto.notmuchmail.org)
 test_json_nodes <<<"$output" \
                 'crypto:[0][0][0]["crypto"]={
-                   "signed":{"status": [{"status": "good", "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "created": 1525812676}],
+                   "signed":{"status": [{"status": "good", "fingerprint": "'$FINGERPRINT'", "userid": "'"$SELF_USERID"'", "email": "'"$SELF_EMAIL"'", "created": 1525812676}],
                    "encrypted": true, "headers": ["Subject"]},"decrypted": {"status": "full"}}' \
                 'subject:[0][0][0]["headers"]["Subject"]="Rhinoceros dinner"'
 
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 0bca76df..7c7c3354 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -131,6 +131,7 @@ add_gnupg_home () {
     # Change this if we ship a new test key
     FINGERPRINT="5AEAB11F5E33DCE875DDB75B6D92612D94E46381"
     SELF_USERID="Notmuch Test Suite <test_suite@notmuchmail.org> (INSECURE!)"
+    SELF_EMAIL="test_suite@notmuchmail.org"
     printf '%s:6:\n' "$FINGERPRINT" | gpg --quiet --batch --no-tty --import-ownertrust
 }
 
diff --git a/util/gmime-extra.c b/util/gmime-extra.c
index 81a5b174..192cb078 100644
--- a/util/gmime-extra.c
+++ b/util/gmime-extra.c
@@ -107,6 +107,21 @@ g_mime_certificate_get_valid_userid (GMimeCertificate *cert)
     return NULL;
 }
 
+const char *
+g_mime_certificate_get_valid_email (GMimeCertificate *cert)
+{
+    /* output e-mail address only if validity is FULL or ULTIMATE. */
+    const char *email = g_mime_certificate_get_email(cert);
+
+    if (email == NULL)
+	return email;
+    GMimeValidity validity = g_mime_certificate_get_id_validity (cert);
+
+    if (validity == GMIME_VALIDITY_FULL || validity == GMIME_VALIDITY_ULTIMATE)
+	return email;
+    return NULL;
+}
+
 const char *
 g_mime_certificate_get_fpr16 (GMimeCertificate *cert)
 {
diff --git a/util/gmime-extra.h b/util/gmime-extra.h
index 094309ec..889e91f3 100644
--- a/util/gmime-extra.h
+++ b/util/gmime-extra.h
@@ -69,6 +69,10 @@ 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);
+/**
+ * Return string for valid e-mail address (or NULL if no valid e-mail address exists)
+ */
+const char *g_mime_certificate_get_valid_email (GMimeCertificate *cert);
 
 #ifdef __cplusplus
 }
-- 
2.30.2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] emacs: Prefer email address over User ID when showing valid signature
  2021-05-27  1:44 ` [PATCH 1/2] cli/show: produce "email" element in sigstatus Daniel Kahn Gillmor
@ 2021-05-27  1:44   ` Daniel Kahn Gillmor
  2021-06-07 12:07     ` David Edmondson
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Kahn Gillmor @ 2021-05-27  1:44 UTC (permalink / raw)
  To: Notmuch Mail

Most concrete verification steps are likely only taken on the e-mail
address in the first place, and e-mail addresses render more
intelligibly than arbitrary User IDs in the first place.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 emacs/notmuch-crypto.el | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index db7cb75d..5c260a7a 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -119,14 +119,19 @@ mode."
     (cond
      ((string= status "good")
       (let ((fingerprint (concat "0x" (plist-get sigstatus :fingerprint)))
+	    (email (plist-get sigstatus :email))
 	    (userid (plist-get sigstatus :userid)))
-	;; If userid is present it has full or greater validity.
-	(if userid
+	;; If email or userid are present, they have full or greater validity.
+	(if email
 	    (progn
-	      (setq label (concat "Good signature by: " userid))
+	      (setq label (concat "Good signature by: " email))
 	      (setq face 'notmuch-crypto-signature-good))
-	  (setq label (concat "Good signature by key: " fingerprint))
-	  (setq face 'notmuch-crypto-signature-good-key))
+	  (if userid
+	      (progn
+		(setq label (concat "Good signature by: " userid))
+		(setq face 'notmuch-crypto-signature-good))
+	    (setq label (concat "Good signature by key: " fingerprint))
+	    (setq face 'notmuch-crypto-signature-good-key)))
 	(setq button-action 'notmuch-crypto-sigstatus-good-callback)
 	(setq help-msg (concat "Click to list key ID 0x" fingerprint "."))))
      ((string= status "error")
-- 
2.30.2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] emacs: Prefer email address over User ID when showing valid signature
  2021-05-27  1:44   ` [PATCH 2/2] emacs: Prefer email address over User ID when showing valid signature Daniel Kahn Gillmor
@ 2021-06-07 12:07     ` David Edmondson
  2021-06-07 12:24       ` Daniel Kahn Gillmor
  0 siblings, 1 reply; 5+ messages in thread
From: David Edmondson @ 2021-06-07 12:07 UTC (permalink / raw)
  To: Daniel Kahn Gillmor, Notmuch Mail

On Wednesday, 2021-05-26 at 21:44:59 -04, Daniel Kahn Gillmor wrote:

> Most concrete verification steps are likely only taken on the e-mail
> address in the first place, and e-mail addresses render more
> intelligibly than arbitrary User IDs in the first place.
>
> Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
> ---
>  emacs/notmuch-crypto.el | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
> index db7cb75d..5c260a7a 100644
> --- a/emacs/notmuch-crypto.el
> +++ b/emacs/notmuch-crypto.el
> @@ -119,14 +119,19 @@ mode."
>      (cond
>       ((string= status "good")
>        (let ((fingerprint (concat "0x" (plist-get sigstatus :fingerprint)))
> +	    (email (plist-get sigstatus :email))
>  	    (userid (plist-get sigstatus :userid)))
> -	;; If userid is present it has full or greater validity.
> -	(if userid
> +	;; If email or userid are present, they have full or greater validity.
> +	(if email

Could this not be something like:

(when (or email userid)
  (setq label (concat "Good signature by: " (or email userid)))
  ...
  )

?

>  	    (progn
> -	      (setq label (concat "Good signature by: " userid))
> +	      (setq label (concat "Good signature by: " email))
>  	      (setq face 'notmuch-crypto-signature-good))
> -	  (setq label (concat "Good signature by key: " fingerprint))
> -	  (setq face 'notmuch-crypto-signature-good-key))
> +	  (if userid
> +	      (progn
> +		(setq label (concat "Good signature by: " userid))
> +		(setq face 'notmuch-crypto-signature-good))
> +	    (setq label (concat "Good signature by key: " fingerprint))
> +	    (setq face 'notmuch-crypto-signature-good-key)))
>  	(setq button-action 'notmuch-crypto-sigstatus-good-callback)
>  	(setq help-msg (concat "Click to list key ID 0x" fingerprint "."))))
>       ((string= status "error")
> -- 
> 2.30.2
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

dme.
-- 
If I could buy my reasoning, I'd pay to lose.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] emacs: Prefer email address over User ID when showing valid signature
  2021-06-07 12:07     ` David Edmondson
@ 2021-06-07 12:24       ` Daniel Kahn Gillmor
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Kahn Gillmor @ 2021-06-07 12:24 UTC (permalink / raw)
  To: David Edmondson, Notmuch Mail


[-- Attachment #1.1: Type: text/plain, Size: 363 bytes --]

On Mon 2021-06-07 13:07:12 +0100, David Edmondson wrote:
> Could this not be something like:
>
> (when (or email userid)
>   (setq label (concat "Good signature by: " (or email userid)))
>   ...
>   )
>
> ?

Sounds reasonable to me.  If you want to offer a revised patch for this,
i'd definitely defer to your expertise on idiomatic and concise elisp.

    --dkg

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-06-07 14:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-26 23:32 difficulty in rendering S/MIME signature status from some certificates Daniel Kahn Gillmor
2021-05-27  1:44 ` [PATCH 1/2] cli/show: produce "email" element in sigstatus Daniel Kahn Gillmor
2021-05-27  1:44   ` [PATCH 2/2] emacs: Prefer email address over User ID when showing valid signature Daniel Kahn Gillmor
2021-06-07 12:07     ` David Edmondson
2021-06-07 12:24       ` Daniel Kahn Gillmor

unofficial mirror of notmuch@notmuchmail.org

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/notmuch/0 notmuch/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 notmuch notmuch/ https://yhetil.org/notmuch \
		notmuch@notmuchmail.org
	public-inbox-index notmuch

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.mail.notmuch.general
	nntp://news.gmane.io/gmane.mail.notmuch.general


code repositories for project(s) associated with this inbox:

	notmuch.git.git (no URL configured)

AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git