[PATCH] emacs: reply: remove wrong sig/enc status buttons

[PATCH] emacs: reply: remove wrong sig/enc status buttons

[Mark Walters]

This stopps the (usually incorrect) sigstatus and encstatus buttons
appearing when replying in emacs, and updates the test suite to match.

Overriding the status button functions is a little unusual but much
less intrusive than passing an argument all the way down the call
chain. It also makes it clear exactly what it does.
---

I dont't have any encrypted messages for testing but it seems to work
on signed messages (but even that is very lightly tested). 

Best wishes

Mark



emacs/notmuch-mua.el | 7 +++++--
 test/T310-emacs.sh   | 2 --
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index fadf20f..55bc267 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -253,8 +253,11 @@ mutiple parts get a header."
 		       (notmuch-show-insert-header-p-function notmuch-mua-reply-insert-header-p-function)
 		       ;; Don't indent multipart sub-parts.
 		       (notmuch-show-indent-multipart nil))
-		    (notmuch-show-insert-body original (plist-get original :body) 0)
-		    (buffer-substring-no-properties (point-min) (point-max)))))
+		    ;; We don't want sigstatus buttons (an information leak and usually wrong anyway).
+		    (letf (((symbol-function 'notmuch-crypto-insert-sigstatus-button) #'ignore)
+			   ((symbol-function 'notmuch-crypto-insert-encstatus-button) #'ignore))
+			  (notmuch-show-insert-body original (plist-get original :body) 0)
+			  (buffer-substring-no-properties (point-min) (point-max))))))
 
 	(set-mark (point))
 	(goto-char start)
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 202fc3b..21675b6 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -384,8 +384,6 @@ References: <20091118002059.067214ed@hikari>
 --text follows this line--
 Adrian Perez de Castro <aperez@igalia.com> writes:
 
-> [ Unknown signature status ]
->
 > Hello to all,
 >
 > I have just heard about Not Much today in some random Linux-related news
-- 
2.1.4

Re: [PATCH] emacs: reply: remove wrong sig/enc status buttons

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> This stopps the (usually incorrect) sigstatus and encstatus buttons
> appearing when replying in emacs, and updates the test suite to match.
>
> Overriding the status button functions is a little unusual but much
> less intrusive than passing an argument all the way down the call
> chain. It also makes it clear exactly what it does.
> ---
>
> I dont't have any encrypted messages for testing but it seems to work
> on signed messages (but even that is very lightly tested). 

In replies encrypted messages there is an extra line saying Version:
1. This is from the first part part of the multipart encrypted

└┬╴multipart/encrypted 6845 bytes
 ├─╴application/pgp-encrypted attachment [PGPMIME version identification] 11 bytes
 └─╴application/octet-stream inline [encrypted.asc] 2708 bytes

I'm not really sure it's useful to display in notmuch-show mode
either. If not, that would fix it here also. Perhaps Daniel can comment
on whether it makes sense to just drop it.

d

Re: [PATCH] emacs: reply: remove wrong sig/enc status buttons

[Daniel Kahn Gillmor]

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

On Mon 2016-09-12 03:11:44 +0200, David Bremner wrote:
> Mark Walters <markwalters1009@gmail.com> writes:
>
>> This stopps the (usually incorrect) sigstatus and encstatus buttons
>> appearing when replying in emacs, and updates the test suite to match.
>>
>> Overriding the status button functions is a little unusual but much
>> less intrusive than passing an argument all the way down the call
>> chain. It also makes it clear exactly what it does.
>> ---
>>
>> I dont't have any encrypted messages for testing but it seems to work
>> on signed messages (but even that is very lightly tested). 
>
> In replies encrypted messages there is an extra line saying Version:
> 1. This is from the first part part of the multipart encrypted
>
> └┬╴multipart/encrypted 6845 bytes
>  ├─╴application/pgp-encrypted attachment [PGPMIME version identification] 11 bytes
>  └─╴application/octet-stream inline [encrypted.asc] 2708 bytes
>
> I'm not really sure it's useful to display in notmuch-show mode
> either. If not, that would fix it here also. Perhaps Daniel can comment
> on whether it makes sense to just drop it.

Yes, it makes no sense to include the "Version: 1" in the
quoted/attributed text either.  It'd be great to remove that as well.

                  --dkg

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

[PATCH v2] emacs: reply: remove wrong sig/enc status buttons

[Mark Walters]

This stopps the (usually incorrect) sigstatus and encstatus buttons
appearing when replying in emacs, and updates the test suite to match.

Overriding the status button functions is a little unusual but much
less intrusive than passing an argument all the way down the call
chain. It also makes it clear exactly what it does.

We also hide the application/pgp-encrypted part as it can only contain
"Version: 1". We do this in notmuch show, which means it also happens
when replying.

---

This fixes the bug David mentioned of Version: 1 appearing in the
reply. We actually fix it by making notmuch-show.el not display this
either.

The patch is as before but with the addition of the 3 lines for
notmuch-show.

Best wishes

Mark



emacs/notmuch-mua.el  | 7 +++++--
 emacs/notmuch-show.el | 3 +++
 test/T310-emacs.sh    | 2 --
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index fadf20f..55bc267 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -253,8 +253,11 @@ mutiple parts get a header."
 		       (notmuch-show-insert-header-p-function notmuch-mua-reply-insert-header-p-function)
 		       ;; Don't indent multipart sub-parts.
 		       (notmuch-show-indent-multipart nil))
-		    (notmuch-show-insert-body original (plist-get original :body) 0)
-		    (buffer-substring-no-properties (point-min) (point-max)))))
+		    ;; We don't want sigstatus buttons (an information leak and usually wrong anyway).
+		    (letf (((symbol-function 'notmuch-crypto-insert-sigstatus-button) #'ignore)
+			   ((symbol-function 'notmuch-crypto-insert-encstatus-button) #'ignore))
+			  (notmuch-show-insert-body original (plist-get original :body) 0)
+			  (buffer-substring-no-properties (point-min) (point-max))))))
 
 	(set-mark (point))
 	(goto-char start)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index eb6877e..0c7d8a1 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -682,6 +682,9 @@ will return nil if the CID is unknown or cannot be retrieved."
       (indent-rigidly start (point) 1)))
   t)
 
+(defun notmuch-show-insert-part-application/pgp-encrypted (msg part content-type nth depth button)
+  t)
+
 (defun notmuch-show-insert-part-multipart/* (msg part content-type nth depth button)
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 202fc3b..21675b6 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -384,8 +384,6 @@ References: <20091118002059.067214ed@hikari>
 --text follows this line--
 Adrian Perez de Castro <aperez@igalia.com> writes:
 
-> [ Unknown signature status ]
->
 > Hello to all,
 >
 > I have just heard about Not Much today in some random Linux-related news
-- 
2.1.4

Re: [PATCH v2] emacs: reply: remove wrong sig/enc status buttons

[Daniel Kahn Gillmor]

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

Hi Mark--

On Tue 2016-09-13 00:12:19 +0200, Mark Walters wrote:
> This stopps the (usually incorrect) sigstatus and encstatus buttons
> appearing when replying in emacs, and updates the test suite to match.
>
> Overriding the status button functions is a little unusual but much
> less intrusive than passing an argument all the way down the call
> chain. It also makes it clear exactly what it does.
>
> We also hide the application/pgp-encrypted part as it can only contain
> "Version: 1". We do this in notmuch show, which means it also happens
> when replying.
>
> ---
>
> This fixes the bug David mentioned of Version: 1 appearing in the
> reply. We actually fix it by making notmuch-show.el not display this
> either.

hm, fwiw, i am still seeing Version: 1 in both the replies and in the
plaintext message body, even with this patch applied :/

not sure what i should do to give you more debugging info.  any
pointers?

        --dkg

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

(no subject)

[David Bremner]

I added a new test to to be "fixed" by Mark's patch. I wasn't 100%
sure about whether to add the test to the emacs set or the crypto set,
but this way doesn't introduce new dependencies to the test set (T350
is already using emacs). If people feel strongly we could move the
test to T310; this will be easier after add_gnupg_home moves into
test-lib.sh

[PATCH 1/2] test/crypto: test reply to encrypted message in emacs

[David Bremner]

This is the current button ridden format, to be cleaned up in a
subsequent commit.
---
 test/T350-crypto.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 96349fa..28ea9ad 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -316,6 +316,28 @@ test_expect_equal \
     "$output" \
     "$expected"
 
+test_begin_subtest "Reply within emacs to an encrypted message"
+test_emacs "(let ((message-hidden-headers '())
+      (notmuch-crypto-process-mime 't))
+  (notmuch-show \"subject:test.encrypted.message.002\")
+  (notmuch-show-reply)
+  (test-output))"
+# the empty To: is probably a bug, but it's not to do with encryption
+grep -v -e '^In-Reply-To:' -e '^References:' -e '^Fcc:' -e 'To:' < OUTPUT > OUTPUT.clean
+cat <<EOF >EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+Subject: Re: test encrypted message 002
+--text follows this line--
+<#secure method=pgpmime mode=signencrypt>
+Notmuch Test Suite <test_suite@notmuchmail.org> writes:
+
+> [ Decryption successful ]
+> [ Good signature by:  Notmuch Test Suite <test_suite@notmuchmail.org> (INSECURE!) ]
+> Version: 1
+> This is another test encrypted message.
+EOF
+test_expect_equal_file EXPECTED OUTPUT.clean
+
 test_begin_subtest "signature verification with revoked key"
 # generate revocation certificate and load it to revoke key
 echo "y
-- 
2.9.3

[PATCH 2/2] emacs: reply: remove wrong sig/enc status buttons

[David Bremner]

From: Mark Walters <markwalters1009@gmail.com>

This stops the (usually incorrect) sigstatus and encstatus buttons
appearing when replying in emacs, and updates the test suite to match.

Overriding the status button functions is a little unusual but much
less intrusive than passing an argument all the way down the call
chain. It also makes it clear exactly what it does.

We also hide the application/pgp-encrypted part as it can only contain
"Version: 1". We do this in notmuch show, which means it also happens
when replying.
---
 emacs/notmuch-mua.el  | 7 +++++--
 emacs/notmuch-show.el | 3 +++
 test/T310-emacs.sh    | 2 --
 test/T350-crypto.sh   | 3 ---
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index fadf20f..55bc267 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -253,8 +253,11 @@ mutiple parts get a header."
 		       (notmuch-show-insert-header-p-function notmuch-mua-reply-insert-header-p-function)
 		       ;; Don't indent multipart sub-parts.
 		       (notmuch-show-indent-multipart nil))
-		    (notmuch-show-insert-body original (plist-get original :body) 0)
-		    (buffer-substring-no-properties (point-min) (point-max)))))
+		    ;; We don't want sigstatus buttons (an information leak and usually wrong anyway).
+		    (letf (((symbol-function 'notmuch-crypto-insert-sigstatus-button) #'ignore)
+			   ((symbol-function 'notmuch-crypto-insert-encstatus-button) #'ignore))
+			  (notmuch-show-insert-body original (plist-get original :body) 0)
+			  (buffer-substring-no-properties (point-min) (point-max))))))
 
 	(set-mark (point))
 	(goto-char start)
diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 5a585f3..641398d 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -682,6 +682,9 @@ will return nil if the CID is unknown or cannot be retrieved."
       (indent-rigidly start (point) 1)))
   t)
 
+(defun notmuch-show-insert-part-application/pgp-encrypted (msg part content-type nth depth button)
+  t)
+
 (defun notmuch-show-insert-part-multipart/* (msg part content-type nth depth button)
   (let ((inner-parts (plist-get part :content))
 	(start (point)))
diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index 202fc3b..21675b6 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -384,8 +384,6 @@ References: <20091118002059.067214ed@hikari>
 --text follows this line--
 Adrian Perez de Castro <aperez@igalia.com> writes:
 
-> [ Unknown signature status ]
->
 > Hello to all,
 >
 > I have just heard about Not Much today in some random Linux-related news
diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 28ea9ad..df2dc74 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -331,9 +331,6 @@ Subject: Re: test encrypted message 002
 <#secure method=pgpmime mode=signencrypt>
 Notmuch Test Suite <test_suite@notmuchmail.org> writes:
 
-> [ Decryption successful ]
-> [ Good signature by:  Notmuch Test Suite <test_suite@notmuchmail.org> (INSECURE!) ]
-> Version: 1
 > This is another test encrypted message.
 EOF
 test_expect_equal_file EXPECTED OUTPUT.clean
-- 
2.9.3

Re: [PATCH v2] emacs: reply: remove wrong sig/enc status buttons

[Mark Walters]

On Tue, 13 Sep 2016, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> [ Unknown signature status ]
> Hi Mark--
>
> On Tue 2016-09-13 00:12:19 +0200, Mark Walters wrote:
>> This stopps the (usually incorrect) sigstatus and encstatus buttons
>> appearing when replying in emacs, and updates the test suite to match.
>>
>> Overriding the status button functions is a little unusual but much
>> less intrusive than passing an argument all the way down the call
>> chain. It also makes it clear exactly what it does.
>>
>> We also hide the application/pgp-encrypted part as it can only contain
>> "Version: 1". We do this in notmuch show, which means it also happens
>> when replying.
>>
>> ---
>>
>> This fixes the bug David mentioned of Version: 1 appearing in the
>> reply. We actually fix it by making notmuch-show.el not display this
>> either.
>
> hm, fwiw, i am still seeing Version: 1 in both the replies and in the
> plaintext message body, even with this patch applied :/
>
> not sure what i should do to give you more debugging info.  any
> pointers?

Hi

Can you post a screenshot of the message in notmuch-show, and the output
of printmimestructure for such a message

if it is hard to reproduce with a message you can make public, then the
main thing I care about is whether the "Version: 1" is in a part with
mime type application/pgp-encrypted.

Best wishes

Mark

Re: [PATCH v2] emacs: reply: remove wrong sig/enc status buttons

[David Bremner]

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

> hm, fwiw, i am still seeing Version: 1 in both the replies and in the
> plaintext message body, even with this patch applied :/
>
> not sure what i should do to give you more debugging info.  any
> pointers?

Maybe this is obvious, but I had the same experience until I recompiled
the emacs lisp (make).

Does the test I posted later in the thread pass for you? (after my
version of the patch)

d

Re: [PATCH 1/2] test/crypto: test reply to encrypted message in emacs

[David Bremner]

David Bremner <david@tethera.net> writes:

> This is the current button ridden format, to be cleaned up in a
> subsequent commit.

I pushed a slightly modified version of this series, where the test is
initially broken and then fixed by Mark's patch. This is closer to our
usual convention,

d