[PATCH 1/3] emacs: test notmuch-show during message decryption

[PATCH 1/3] emacs: test notmuch-show during message decryption

[Daniel Kahn Gillmor]

We did not have a test showing what message decryption looks like
within notmuch-emacs.  This change gives us a baseline for future work
on the notmuch-emacs interface.
---
 test/T450-emacs-show.sh                       | 15 ++++++++++
 test/corpora/crypto/basic-encrypted.eml       | 28 +++++++++++++++++++
 .../notmuch-show-decrypted-message            | 11 ++++++++
 .../notmuch-show-undecryptable-message        | 10 +++++++
 4 files changed, 64 insertions(+)
 create mode 100644 test/corpora/crypto/basic-encrypted.eml
 create mode 100644 test/emacs-show.expected-output/notmuch-show-decrypted-message
 create mode 100644 test/emacs-show.expected-output/notmuch-show-undecryptable-message

diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 3555a939..5d4df443 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -208,4 +208,19 @@ test_emacs '(notmuch-show "id:'$gen_msg_id'")
 output=$(head -1 OUTPUT.raw|cut -f1-4 -d' ')
 test_expect_equal "$output" "Notmuch Test Suite <test_suite@notmuchmail.org>"
 
+
+# switching to the crypto corpus, using gpg from here on:
+add_gnupg_home
+add_email_corpus crypto
+
+test_begin_subtest "show decrypted message"
+test_emacs '(notmuch-show "id:basic-encrypted@crypto.notmuchmail.org")
+            (test-visible-output)'
+test_expect_equal_file $EXPECTED/notmuch-show-decrypted-message OUTPUT
+
+test_begin_subtest "show undecryptable message"
+test_emacs '(notmuch-show "id:simple-encrypted@crypto.notmuchmail.org")
+            (test-visible-output)'
+test_expect_equal_file $EXPECTED/notmuch-show-undecryptable-message OUTPUT
+
 test_done
diff --git a/test/corpora/crypto/basic-encrypted.eml b/test/corpora/crypto/basic-encrypted.eml
new file mode 100644
index 00000000..1ba4698a
--- /dev/null
+++ b/test/corpora/crypto/basic-encrypted.eml
@@ -0,0 +1,28 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Here is the password
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <basic-encrypted@crypto.notmuchmail.org>
+MIME-Version: 1.0
+Content-Type: multipart/encrypted; boundary="=-=-=";
+	protocol="application/pgp-encrypted"
+
+--=-=-=
+Content-Type: application/pgp-encrypted
+
+Version: 1
+
+--=-=-=
+Content-Type: application/octet-stream
+
+-----BEGIN PGP MESSAGE-----
+
+hIwDxE023q1UqxYBBACp70e7KPy9OYaheIrkLzmhq1lRqmy51aL1jBL0K/qN7rfK
+BZEG1cR8jeLjTFdPKPLVKJI80r7FgKI0ywvWvl6R1aE1Ty5BnVXT9XzCrEH7fqCl
+SKK82EvolXTohAZHUrh6K66eQQTTIAC1n7B0A8hErzkgaM4+seN3LlvezT6TLNKM
+ATpqsEbM2MVrGgw0b3oUsGGAPEt2MmjNEYsriKnqwt6dJDZc//XyhjgMQayiD8da
+N1gT3oqgu/gKCpBZDYzHf9OtVi2UnlFDWy6rrMZLjWDnIv4ve9Pn/qolwHVjzdJ1
+ZfjNC5t0z3XADKGrjN9wutr4qm7STW1rHAXHP68TQTxI0qgJKjPXNKWEw6g=
+=pJG4
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/emacs-show.expected-output/notmuch-show-decrypted-message b/test/emacs-show.expected-output/notmuch-show-decrypted-message
new file mode 100644
index 00000000..08a9e4f6
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-decrypted-message
@@ -0,0 +1,11 @@
+test_suite@notmuchmail.org (2000-01-01) (encrypted inbox)
+Subject: Here is the password
+To: test_suite@notmuchmail.org
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+
+[ multipart/encrypted ]
+[ Decryption successful ]
+[ Unknown signature status ]
+[ application/pgp-encrypted ]
+[ text/plain ]
+The password is "abcd1234!", please do not tell anyone.
diff --git a/test/emacs-show.expected-output/notmuch-show-undecryptable-message b/test/emacs-show.expected-output/notmuch-show-undecryptable-message
new file mode 100644
index 00000000..530ff286
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-undecryptable-message
@@ -0,0 +1,10 @@
+Daniel Kahn Gillmor <dkg@fifthhorseman.net> (2016-12-22) (encrypted inbox)
+Subject: encrypted message
+To: dkg@fifthhorseman.net
+Date: Thu, 22 Dec 2016 08:34:56 -0400
+
+[ multipart/encrypted ]
+[ Decryption error ]
+[ Unknown signature status ]
+[ application/pgp-encrypted ]
+[ application/octet-stream ]
-- 
2.20.1

[PATCH 2/3] test: show what emacs sees of an encrypted message when crypto is disabled

[Daniel Kahn Gillmor]

Some users may set notmuch-crypto-process-mime to nil, disabling all
crypto use.  We should have a baseline for what that looks like.
---
 test/T450-emacs-show.sh                                |  6 ++++++
 .../notmuch-show-decrypted-message-no-crypto           | 10 ++++++++++
 2 files changed, 16 insertions(+)
 create mode 100644 test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto

diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 5d4df443..de1755d2 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -223,4 +223,10 @@ test_emacs '(notmuch-show "id:simple-encrypted@crypto.notmuchmail.org")
             (test-visible-output)'
 test_expect_equal_file $EXPECTED/notmuch-show-undecryptable-message OUTPUT
 
+test_begin_subtest "show encrypted message when not processing crypto"
+test_emacs '(let ((notmuch-crypto-process-mime nil))
+             (notmuch-show "id:basic-encrypted@crypto.notmuchmail.org")
+             (test-visible-output))'
+test_expect_equal_file $EXPECTED/notmuch-show-decrypted-message-no-crypto OUTPUT
+
 test_done
diff --git a/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto b/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto
new file mode 100644
index 00000000..e302e452
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto
@@ -0,0 +1,10 @@
+test_suite@notmuchmail.org (2000-01-01) (encrypted inbox)
+Subject: Here is the password
+To: test_suite@notmuchmail.org
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+
+[ multipart/encrypted ]
+[ Unknown encryption status ]
+[ Unknown signature status ]
+[ application/pgp-encrypted ]
+[ application/octet-stream ]
-- 
2.20.1

[PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[Daniel Kahn Gillmor]

When we have not been able to evaluate the signature status of a given
MIME part, showing a content-free (and interaction-free) "[ Unknown
signature status ]" button doesn't really help the user at all, and
takes up valuable screen real-estate.

A visual reminder that a given message is *not* signed isn't helpful
unless it is always present, in which case we'd want to see "[ Unknown
signature status ]" buttons on all messages, even ones that don't have
a signing structure, but i don't think we want that.
---
 emacs/notmuch-crypto.el                       | 27 ++++++++++---------
 .../notmuch-show-decrypted-message            |  1 -
 .../notmuch-show-decrypted-message-no-crypto  |  1 -
 ...notmuch-show-process-crypto-mime-parts-off |  1 -
 .../notmuch-show-undecryptable-message        |  1 -
 5 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 353f721e..68171153 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -93,6 +93,7 @@ mode."
 (defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
   (let* ((status (plist-get sigstatus :status))
 	 (help-msg nil)
+	 (show-button t)
 	 (label "Signature not processed")
 	 (face 'notmuch-crypto-signature-unknown)
 	 (button-action (lambda (button) (message (button-get button 'help-echo)))))
@@ -118,19 +119,21 @@ mode."
       (let ((keyid (concat "0x" (plist-get sigstatus :keyid))))
 	(setq label (concat "Bad signature (claimed key ID " keyid ")"))
 	(setq face 'notmuch-crypto-signature-bad)))
+     (status
+      (setq label (concat "Unknown signature status: " status)))
      (t
-      (setq label (concat "Unknown signature status"
-			  (if status (concat ": " status))))))
-    (insert-button
-     (concat "[ " label " ]")
-     :type 'notmuch-crypto-status-button-type
-     'help-echo help-msg
-     'face face
-     'mouse-face face
-     'action button-action
-     :notmuch-sigstatus sigstatus
-     :notmuch-from from)
-    (insert "\n")))
+      (setq show-button nil)))
+    (when show-button
+      (insert-button
+       (concat "[ " label " ]")
+       :type 'notmuch-crypto-status-button-type
+       'help-echo help-msg
+       'face face
+       'mouse-face face
+       'action button-action
+       :notmuch-sigstatus sigstatus
+       :notmuch-from from)
+      (insert "\n"))))
 
 (declare-function notmuch-show-refresh-view "notmuch-show" (&optional reset-state))
 
diff --git a/test/emacs-show.expected-output/notmuch-show-decrypted-message b/test/emacs-show.expected-output/notmuch-show-decrypted-message
index 08a9e4f6..c4ec4b32 100644
--- a/test/emacs-show.expected-output/notmuch-show-decrypted-message
+++ b/test/emacs-show.expected-output/notmuch-show-decrypted-message
@@ -5,7 +5,6 @@ Date: Sat, 01 Jan 2000 12:00:00 +0000
 
 [ multipart/encrypted ]
 [ Decryption successful ]
-[ Unknown signature status ]
 [ application/pgp-encrypted ]
 [ text/plain ]
 The password is "abcd1234!", please do not tell anyone.
diff --git a/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto b/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto
index e302e452..49d85337 100644
--- a/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto
+++ b/test/emacs-show.expected-output/notmuch-show-decrypted-message-no-crypto
@@ -5,6 +5,5 @@ Date: Sat, 01 Jan 2000 12:00:00 +0000
 
 [ multipart/encrypted ]
 [ Unknown encryption status ]
-[ Unknown signature status ]
 [ application/pgp-encrypted ]
 [ application/octet-stream ]
diff --git a/test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off b/test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off
index ce2892a0..3282c7b1 100644
--- a/test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off
+++ b/test/emacs-show.expected-output/notmuch-show-process-crypto-mime-parts-off
@@ -9,7 +9,6 @@ Subject: [notmuch] Working with Maildir storage?
 
   [ multipart/mixed ]
   [ multipart/signed ]
-  [ Unknown signature status ]
   [ text/plain ]
   > See the patch just posted here.
 
diff --git a/test/emacs-show.expected-output/notmuch-show-undecryptable-message b/test/emacs-show.expected-output/notmuch-show-undecryptable-message
index 530ff286..95c1646b 100644
--- a/test/emacs-show.expected-output/notmuch-show-undecryptable-message
+++ b/test/emacs-show.expected-output/notmuch-show-undecryptable-message
@@ -5,6 +5,5 @@ Date: Thu, 22 Dec 2016 08:34:56 -0400
 
 [ multipart/encrypted ]
 [ Decryption error ]
-[ Unknown signature status ]
 [ application/pgp-encrypted ]
 [ application/octet-stream ]
-- 
2.20.1

Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

The first two patches in this series should be uncontroversial (they're
just establishing a baseline), and i'd appreciate it if they could be
considered for merging regardless of whether anyone raises an objection
to this last patch of the series.

On Mon 2019-04-22 13:18:14 -0400, Daniel Kahn Gillmor wrote:
> When we have not been able to evaluate the signature status of a given
> MIME part, showing a content-free (and interaction-free) "[ Unknown
> signature status ]" button doesn't really help the user at all, and
> takes up valuable screen real-estate.
>
> A visual reminder that a given message is *not* signed isn't helpful
> unless it is always present, in which case we'd want to see "[ Unknown
> signature status ]" buttons on all messages, even ones that don't have
> a signing structure, but i don't think we want that.

This is a small step down the path of making notmuch-emacs friendlier
with regards to encrypted messages, but it's one that will have an
effect on future patch series that work with encrypted messages.  I'd be
happy to hear any concerns people have about this change, but i find
notmuch-emacs is more pleasant to use this way.

     --dkg

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

[PATCH v2 1/3] emacs: test notmuch-show during message decryption

[Daniel Kahn Gillmor]

We did not have a test showing what message decryption looks like
within notmuch-emacs.  This change gives us a baseline for future work
on the notmuch-emacs interface.
---

The difference between this and the prior version of the patch is that
it updates T357-index-decryption.sh as well, to ensure all tests pass.

 test/T357-index-decryption.sh                 |  3 +-
 test/T450-emacs-show.sh                       | 15 ++++++++++
 test/corpora/crypto/basic-encrypted.eml       | 28 +++++++++++++++++++
 .../notmuch-show-decrypted-message            | 11 ++++++++
 .../notmuch-show-undecryptable-message        | 10 +++++++
 5 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 test/corpora/crypto/basic-encrypted.eml
 create mode 100644 test/emacs-show.expected-output/notmuch-show-decrypted-message
 create mode 100644 test/emacs-show.expected-output/notmuch-show-undecryptable-message

diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index c5435f4f..e191f6ff 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -236,7 +236,8 @@ notmuch reindex --decrypt=true id:simple-encrypted@crypto.notmuchmail.org
 output=$(notmuch dump )
 expected='#notmuch-dump batch-tag:3 config,properties,tags
 +encrypted +inbox +unread -- id:simple-encrypted@crypto.notmuchmail.org
-#= simple-encrypted@crypto.notmuchmail.org index.decryption=failure'
+#= simple-encrypted@crypto.notmuchmail.org index.decryption=failure
++encrypted +inbox +unread -- id:basic-encrypted@crypto.notmuchmail.org'
 test_expect_equal \
     "$output" \
     "$expected"
diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 3555a939..5d4df443 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -208,4 +208,19 @@ test_emacs '(notmuch-show "id:'$gen_msg_id'")
 output=$(head -1 OUTPUT.raw|cut -f1-4 -d' ')
 test_expect_equal "$output" "Notmuch Test Suite <test_suite@notmuchmail.org>"
 
+
+# switching to the crypto corpus, using gpg from here on:
+add_gnupg_home
+add_email_corpus crypto
+
+test_begin_subtest "show decrypted message"
+test_emacs '(notmuch-show "id:basic-encrypted@crypto.notmuchmail.org")
+            (test-visible-output)'
+test_expect_equal_file $EXPECTED/notmuch-show-decrypted-message OUTPUT
+
+test_begin_subtest "show undecryptable message"
+test_emacs '(notmuch-show "id:simple-encrypted@crypto.notmuchmail.org")
+            (test-visible-output)'
+test_expect_equal_file $EXPECTED/notmuch-show-undecryptable-message OUTPUT
+
 test_done
diff --git a/test/corpora/crypto/basic-encrypted.eml b/test/corpora/crypto/basic-encrypted.eml
new file mode 100644
index 00000000..1ba4698a
--- /dev/null
+++ b/test/corpora/crypto/basic-encrypted.eml
@@ -0,0 +1,28 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Here is the password
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <basic-encrypted@crypto.notmuchmail.org>
+MIME-Version: 1.0
+Content-Type: multipart/encrypted; boundary="=-=-=";
+	protocol="application/pgp-encrypted"
+
+--=-=-=
+Content-Type: application/pgp-encrypted
+
+Version: 1
+
+--=-=-=
+Content-Type: application/octet-stream
+
+-----BEGIN PGP MESSAGE-----
+
+hIwDxE023q1UqxYBBACp70e7KPy9OYaheIrkLzmhq1lRqmy51aL1jBL0K/qN7rfK
+BZEG1cR8jeLjTFdPKPLVKJI80r7FgKI0ywvWvl6R1aE1Ty5BnVXT9XzCrEH7fqCl
+SKK82EvolXTohAZHUrh6K66eQQTTIAC1n7B0A8hErzkgaM4+seN3LlvezT6TLNKM
+ATpqsEbM2MVrGgw0b3oUsGGAPEt2MmjNEYsriKnqwt6dJDZc//XyhjgMQayiD8da
+N1gT3oqgu/gKCpBZDYzHf9OtVi2UnlFDWy6rrMZLjWDnIv4ve9Pn/qolwHVjzdJ1
+ZfjNC5t0z3XADKGrjN9wutr4qm7STW1rHAXHP68TQTxI0qgJKjPXNKWEw6g=
+=pJG4
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/emacs-show.expected-output/notmuch-show-decrypted-message b/test/emacs-show.expected-output/notmuch-show-decrypted-message
new file mode 100644
index 00000000..08a9e4f6
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-decrypted-message
@@ -0,0 +1,11 @@
+test_suite@notmuchmail.org (2000-01-01) (encrypted inbox)
+Subject: Here is the password
+To: test_suite@notmuchmail.org
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+
+[ multipart/encrypted ]
+[ Decryption successful ]
+[ Unknown signature status ]
+[ application/pgp-encrypted ]
+[ text/plain ]
+The password is "abcd1234!", please do not tell anyone.
diff --git a/test/emacs-show.expected-output/notmuch-show-undecryptable-message b/test/emacs-show.expected-output/notmuch-show-undecryptable-message
new file mode 100644
index 00000000..530ff286
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-undecryptable-message
@@ -0,0 +1,10 @@
+Daniel Kahn Gillmor <dkg@fifthhorseman.net> (2016-12-22) (encrypted inbox)
+Subject: encrypted message
+To: dkg@fifthhorseman.net
+Date: Thu, 22 Dec 2016 08:34:56 -0400
+
+[ multipart/encrypted ]
+[ Decryption error ]
+[ Unknown signature status ]
+[ application/pgp-encrypted ]
+[ application/octet-stream ]
-- 
2.20.1

Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]

On Mon 2019-04-22 13:26:05 -0400, Daniel Kahn Gillmor wrote:
> On Mon 2019-04-22 13:18:14 -0400, Daniel Kahn Gillmor wrote:
>> When we have not been able to evaluate the signature status of a given
>> MIME part, showing a content-free (and interaction-free) "[ Unknown
>> signature status ]" button doesn't really help the user at all, and
>> takes up valuable screen real-estate.
>>
>> A visual reminder that a given message is *not* signed isn't helpful
>> unless it is always present, in which case we'd want to see "[ Unknown
>> signature status ]" buttons on all messages, even ones that don't have
>> a signing structure, but i don't think we want that.
>
> This is a small step down the path of making notmuch-emacs friendlier
> with regards to encrypted messages, but it's one that will have an
> effect on future patch series that work with encrypted messages.  I'd be
> happy to hear any concerns people have about this change, but i find
> notmuch-emacs is more pleasant to use this way.

I've heard from some people that they don't like this final patch
because the "[ Unknown signature status ]" button is at least an
indication that the message appears to be signed (even if we decided not
to -- or were unable to -- evaluate the signature).

I considered argument this when writing the patch initially, and i don't
think it's a good argument for two reasons:

 a) without actual signature verification, the user experience is
    trivially scammable by an adversary who knows how to craft a MIME
    message, and it's basically encouraging a user pattern that is
    something along the lines of:

      https://xkcd.com/1181/

    (though maybe a bit more subtle, based on MIME structure instead of
    the inline-signing that xkcd is mocking)

    This is a particularly bad security indicator and user experience.
    The thing isn't reliable, and it's not actionable in most cases.

 b) In the current state of the codebase, the presence of the button
    does *not* indicate that a signature-like thing is even present.  If
    you look at
    test/emacs-show.expected-output/notmuch-show-decrypted-message, that
    shows the cleartext view of a decrypted message which *does not*
    have an OpenPGP signature on it at all
    (test/corpora/crypto/basic-encrypted.eml is encrypted but unsigned).


I could imagine changing notmuch to fix concern (b) -- that is, hiding
the button just in the case where no signature-looking thing is present
at all.  But i haven't seen anyone even identify that problem publicly
yet, let alone offer a fix for it.

But i think that (a) is at least as big of a concern as (b); the fix i'm
proposing in this series is actually simpler than such a targeted fix
would be; and the fix in this series actually solves both problems.

If someone wants to offer a fix just for (b) on top of the first two
patches in this series, i'd happily advocate for it as better than the
status quo, which would let us put off (a) to a more interesting and
targeted discussion.

        --dkg

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

Re: [PATCH v2 1/3] emacs: test notmuch-show during message decryption

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> We did not have a test showing what message decryption looks like
> within notmuch-emacs.  This change gives us a baseline for future work
> on the notmuch-emacs interface.
> ---
>
> The difference between this and the prior version of the patch is that
> it updates T357-index-decryption.sh as well, to ensure all tests pass.
>

I don't _think_ this is gmime3 related, but I get

T357-index-decryption: Testing indexing decrypted mail
 FAIL   indexing message fails when secret key not available
	--- T357-index-decryption.31.expected	2019-05-03 17:57:26.701988487 +0000
	+++ T357-index-decryption.31.output	2019-05-03 17:57:26.701988487 +0000
	@@ -1,4 +1,4 @@
	 #notmuch-dump batch-tag:3 config,properties,tags
	++encrypted +inbox +unread -- id:basic-encrypted@crypto.notmuchmail.org
	 +encrypted +inbox +unread -- id:simple-encrypted@crypto.notmuchmail.org
	 #= simple-encrypted@crypto.notmuchmail.org index.decryption=failure
	-+encrypted +inbox +unread -- id:basic-encrypted@crypto.notmuchmail.org
 FAIL   cleartext index recovery on reindexing with stashed session keys
	--- T357-index-decryption.33.expected	2019-05-03 17:57:26.817988197 +0000
	+++ T357-index-decryption.33.output	2019-05-03 17:57:26.817988197 +0000
	@@ -1 +1 @@
	-thread:0000000000000001   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)
	+thread:0000000000000002   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)

Applying on top of 103c11822

d

Re: [PATCH v2 1/3] emacs: test notmuch-show during message decryption

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 2006 bytes --]

On Fri 2019-05-03 17:59:49 +0000, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> We did not have a test showing what message decryption looks like
>> within notmuch-emacs.  This change gives us a baseline for future work
>> on the notmuch-emacs interface.
>> ---
>>
>> The difference between this and the prior version of the patch is that
>> it updates T357-index-decryption.sh as well, to ensure all tests pass.
>>
>
> I don't _think_ this is gmime3 related, but I get
>
> T357-index-decryption: Testing indexing decrypted mail
>  FAIL   indexing message fails when secret key not available
> 	--- T357-index-decryption.31.expected	2019-05-03 17:57:26.701988487 +0000
> 	+++ T357-index-decryption.31.output	2019-05-03 17:57:26.701988487 +0000
> 	@@ -1,4 +1,4 @@
> 	 #notmuch-dump batch-tag:3 config,properties,tags
> 	++encrypted +inbox +unread -- id:basic-encrypted@crypto.notmuchmail.org
> 	 +encrypted +inbox +unread -- id:simple-encrypted@crypto.notmuchmail.org
> 	 #= simple-encrypted@crypto.notmuchmail.org index.decryption=failure
> 	-+encrypted +inbox +unread -- id:basic-encrypted@crypto.notmuchmail.org
>  FAIL   cleartext index recovery on reindexing with stashed session keys
> 	--- T357-index-decryption.33.expected	2019-05-03 17:57:26.817988197 +0000
> 	+++ T357-index-decryption.33.output	2019-05-03 17:57:26.817988197 +0000
> 	@@ -1 +1 @@
> 	-thread:0000000000000001   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)
> 	+thread:0000000000000002   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)
>
> Applying on top of 103c11822


hm, right, thanks.  The issue here is likely that your filesystem's
readdir puts the messages in the corpus in a different order than mine
does.    So these failures are due only to the order in which "notmuch
new" encounters the messages in corpus/crypto.

I'll send a revised patch that should be ordering-independent.  thanks
for catching this!

    --dkg

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

[PATCH v3 1/3] emacs: test notmuch-show during message decryption

[Daniel Kahn Gillmor]

We did not have a test showing what message decryption looks like
within notmuch-emacs.  This change gives us a baseline for future work
on the notmuch-emacs interface.

This differs from previous revisions of this patch in that it should
be insensitive to the order in which the local filesystem readdir()s
the underlying maildir.

Signed-off-by: Daniel Kahn Gillmor <dkg@fifthhorseman.net>
---
 test/T357-index-decryption.sh                 | 13 +++++----
 test/T450-emacs-show.sh                       | 15 ++++++++++
 test/corpora/crypto/basic-encrypted.eml       | 28 +++++++++++++++++++
 .../notmuch-show-decrypted-message            | 11 ++++++++
 .../notmuch-show-undecryptable-message        | 10 +++++++
 5 files changed, 71 insertions(+), 6 deletions(-)
 create mode 100644 test/corpora/crypto/basic-encrypted.eml
 create mode 100644 test/emacs-show.expected-output/notmuch-show-decrypted-message
 create mode 100644 test/emacs-show.expected-output/notmuch-show-undecryptable-message

diff --git a/test/T357-index-decryption.sh b/test/T357-index-decryption.sh
index 0a602e50..c9cd5e30 100755
--- a/test/T357-index-decryption.sh
+++ b/test/T357-index-decryption.sh
@@ -224,10 +224,11 @@ add_email_corpus crypto
 
 test_begin_subtest "indexing message fails when secret key not available"
 notmuch reindex --decrypt=true id:simple-encrypted@crypto.notmuchmail.org
-output=$(notmuch dump )
-expected='#notmuch-dump batch-tag:3 config,properties,tags
-+encrypted +inbox +unread -- id:simple-encrypted@crypto.notmuchmail.org
-#= simple-encrypted@crypto.notmuchmail.org index.decryption=failure'
+output=$(notmuch dump | LC_ALL=C sort)
+expected='#= simple-encrypted@crypto.notmuchmail.org index.decryption=failure
+#notmuch-dump batch-tag:3 config,properties,tags
++encrypted +inbox +unread -- id:basic-encrypted@crypto.notmuchmail.org
++encrypted +inbox +unread -- id:simple-encrypted@crypto.notmuchmail.org'
 test_expect_equal \
     "$output" \
     "$expected"
@@ -245,8 +246,8 @@ notmuch restore <<EOF
 #= simple-encrypted@crypto.notmuchmail.org session-key=9%3AFC09987F5F927CC0CC0EE80A96E4C5BBF4A499818FB591207705DFDDD6112CF9
 EOF
 notmuch reindex id:simple-encrypted@crypto.notmuchmail.org
-output=$(notmuch search sekrit)
-expected='thread:0000000000000001   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)'
+output=$(notmuch search sekrit | notmuch_search_sanitize)
+expected='thread:XXX   2016-12-22 [1/1] Daniel Kahn Gillmor; encrypted message (encrypted inbox unread)'
 test_expect_equal \
     "$output" \
     "$expected"
diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index 3555a939..5d4df443 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -208,4 +208,19 @@ test_emacs '(notmuch-show "id:'$gen_msg_id'")
 output=$(head -1 OUTPUT.raw|cut -f1-4 -d' ')
 test_expect_equal "$output" "Notmuch Test Suite <test_suite@notmuchmail.org>"
 
+
+# switching to the crypto corpus, using gpg from here on:
+add_gnupg_home
+add_email_corpus crypto
+
+test_begin_subtest "show decrypted message"
+test_emacs '(notmuch-show "id:basic-encrypted@crypto.notmuchmail.org")
+            (test-visible-output)'
+test_expect_equal_file $EXPECTED/notmuch-show-decrypted-message OUTPUT
+
+test_begin_subtest "show undecryptable message"
+test_emacs '(notmuch-show "id:simple-encrypted@crypto.notmuchmail.org")
+            (test-visible-output)'
+test_expect_equal_file $EXPECTED/notmuch-show-undecryptable-message OUTPUT
+
 test_done
diff --git a/test/corpora/crypto/basic-encrypted.eml b/test/corpora/crypto/basic-encrypted.eml
new file mode 100644
index 00000000..1ba4698a
--- /dev/null
+++ b/test/corpora/crypto/basic-encrypted.eml
@@ -0,0 +1,28 @@
+From: test_suite@notmuchmail.org
+To: test_suite@notmuchmail.org
+Subject: Here is the password
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+Message-ID: <basic-encrypted@crypto.notmuchmail.org>
+MIME-Version: 1.0
+Content-Type: multipart/encrypted; boundary="=-=-=";
+	protocol="application/pgp-encrypted"
+
+--=-=-=
+Content-Type: application/pgp-encrypted
+
+Version: 1
+
+--=-=-=
+Content-Type: application/octet-stream
+
+-----BEGIN PGP MESSAGE-----
+
+hIwDxE023q1UqxYBBACp70e7KPy9OYaheIrkLzmhq1lRqmy51aL1jBL0K/qN7rfK
+BZEG1cR8jeLjTFdPKPLVKJI80r7FgKI0ywvWvl6R1aE1Ty5BnVXT9XzCrEH7fqCl
+SKK82EvolXTohAZHUrh6K66eQQTTIAC1n7B0A8hErzkgaM4+seN3LlvezT6TLNKM
+ATpqsEbM2MVrGgw0b3oUsGGAPEt2MmjNEYsriKnqwt6dJDZc//XyhjgMQayiD8da
+N1gT3oqgu/gKCpBZDYzHf9OtVi2UnlFDWy6rrMZLjWDnIv4ve9Pn/qolwHVjzdJ1
+ZfjNC5t0z3XADKGrjN9wutr4qm7STW1rHAXHP68TQTxI0qgJKjPXNKWEw6g=
+=pJG4
+-----END PGP MESSAGE-----
+--=-=-=--
diff --git a/test/emacs-show.expected-output/notmuch-show-decrypted-message b/test/emacs-show.expected-output/notmuch-show-decrypted-message
new file mode 100644
index 00000000..08a9e4f6
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-decrypted-message
@@ -0,0 +1,11 @@
+test_suite@notmuchmail.org (2000-01-01) (encrypted inbox)
+Subject: Here is the password
+To: test_suite@notmuchmail.org
+Date: Sat, 01 Jan 2000 12:00:00 +0000
+
+[ multipart/encrypted ]
+[ Decryption successful ]
+[ Unknown signature status ]
+[ application/pgp-encrypted ]
+[ text/plain ]
+The password is "abcd1234!", please do not tell anyone.
diff --git a/test/emacs-show.expected-output/notmuch-show-undecryptable-message b/test/emacs-show.expected-output/notmuch-show-undecryptable-message
new file mode 100644
index 00000000..530ff286
--- /dev/null
+++ b/test/emacs-show.expected-output/notmuch-show-undecryptable-message
@@ -0,0 +1,10 @@
+Daniel Kahn Gillmor <dkg@fifthhorseman.net> (2016-12-22) (encrypted inbox)
+Subject: encrypted message
+To: dkg@fifthhorseman.net
+Date: Thu, 22 Dec 2016 08:34:56 -0400
+
+[ multipart/encrypted ]
+[ Decryption error ]
+[ Unknown signature status ]
+[ application/pgp-encrypted ]
+[ application/octet-stream ]
-- 
2.20.1

Re: [PATCH v3 1/3] emacs: test notmuch-show during message decryption

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> We did not have a test showing what message decryption looks like
> within notmuch-emacs.  This change gives us a baseline for future work
> on the notmuch-emacs interface.
>
> This differs from previous revisions of this patch in that it should
> be insensitive to the order in which the local filesystem readdir()s
> the underlying maildir.

pushed to master

d

Re: [PATCH 2/3] test: show what emacs sees of an encrypted message when crypto is disabled

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> Some users may set notmuch-crypto-process-mime to nil, disabling all
> crypto use.  We should have a baseline for what that looks like.
> ---

pushed to master.

Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

On Mon 2019-04-22 13:18:14 -0400, Daniel Kahn Gillmor wrote:
> When we have not been able to evaluate the signature status of a given
> MIME part, showing a content-free (and interaction-free) "[ Unknown
> signature status ]" button doesn't really help the user at all, and
> takes up valuable screen real-estate.

I haven't heard any public complaints about this proposed change, and it
is a concrete improvement to the usability of encrypted-but-unsigned
messages.  Please consider merging!

           --dkg

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

Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
> index 353f721e..68171153 100644
> --- a/emacs/notmuch-crypto.el
> +++ b/emacs/notmuch-crypto.el
> @@ -93,6 +93,7 @@ mode."
>  (defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
>    (let* ((status (plist-get sigstatus :status))
>  	 (help-msg nil)
> +	 (show-button t)
>  	 (label "Signature not processed")

This should probably be nil, since that particular value is never used,
iiuc. I can amend it if you agree.

Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[Daniel Kahn Gillmor]

[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]

On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
>> index 353f721e..68171153 100644
>> --- a/emacs/notmuch-crypto.el
>> +++ b/emacs/notmuch-crypto.el
>> @@ -93,6 +93,7 @@ mode."
>>  (defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
>>    (let* ((status (plist-get sigstatus :status))
>>  	 (help-msg nil)
>> +	 (show-button t)
>>  	 (label "Signature not processed")
>
> This should probably be nil, since that particular value is never used,
> iiuc. I can amend it if you agree.

I don't think i understand your suggestion.  that value *is* used, by
the "(when show-button …)" construct later in the code.

The idea was to make the button shown by default, unless it was disabled
further down in the patch (in the "t" clause of the main "cond"
construct in notmuch-crypto-insert-sigstatus-button, which only triggers
if we legitimately don't have any status information (if "status" is
nil)).

So no, i think it's supposed to start off "t" and then get disabled in
the specific condition this patch tests for.

If i've misunderstood the e-lisp (entirely possible!) i would be happy
to be corrected.

   --dkg

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

Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote:
>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>
>>> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
>>> index 353f721e..68171153 100644
>>> --- a/emacs/notmuch-crypto.el
>>> +++ b/emacs/notmuch-crypto.el
>>> @@ -93,6 +93,7 @@ mode."
>>>  (defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
>>>    (let* ((status (plist-get sigstatus :status))
>>>  	 (help-msg nil)
>>> +	 (show-button t)
>>>  	 (label "Signature not processed")
>>
>> This should probably be nil, since that particular value is never used,
>> iiuc. I can amend it if you agree.
>
[snip]
>
> If i've misunderstood the e-lisp (entirely possible!) i would be happy
> to be corrected.

No, you've just misunderstood my reply. I refer to the existing initialization
of "label", which now seems obsolete to me.

d

Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[Daniel Kahn Gillmor]

On Fri 2019-05-24 22:38:12 -0300, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote:
>>> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>>>
>>>> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
>>>> index 353f721e..68171153 100644
>>>> --- a/emacs/notmuch-crypto.el
>>>> +++ b/emacs/notmuch-crypto.el
>>>> @@ -93,6 +93,7 @@ mode."
>>>>  (defun notmuch-crypto-insert-sigstatus-button (sigstatus from)
>>>>    (let* ((status (plist-get sigstatus :status))
>>>>  	 (help-msg nil)
>>>> +	 (show-button t)
>>>>  	 (label "Signature not processed")
>>>
>>> This should probably be nil, since that particular value is never used,
>>> iiuc. I can amend it if you agree.
>>
> [snip]
>>
>> If i've misunderstood the e-lisp (entirely possible!) i would be happy
>> to be corrected.
>
> No, you've just misunderstood my reply. I refer to the existing initialization
> of "label", which now seems obsolete to me.

Ah, sorry!  i understand now, and yes, i agree that label shold be
initialized to nil.  If you're willing to amend it that sounds good to
me.

Thanks for catching this.

        --dkg

Re: [PATCH 3/3] emacs: Drop content-free "Unknown signature status" button

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> When we have not been able to evaluate the signature status of a given
> MIME part, showing a content-free (and interaction-free) "[ Unknown
> signature status ]" button doesn't really help the user at all, and
> takes up valuable screen real-estate.
>
> A visual reminder that a given message is *not* signed isn't helpful
> unless it is always present, in which case we'd want to see "[ Unknown
> signature status ]" buttons on all messages, even ones that don't have
> a signing structure, but i don't think we want that.

amended version pushed to master

d