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

2019-05-25 Thread David Bremner
Daniel Kahn Gillmor  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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


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

2019-05-25 Thread Daniel Kahn Gillmor
On Fri 2019-05-24 22:38:12 -0300, David Bremner wrote:
> Daniel Kahn Gillmor  writes:
>
>> On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote:
>>> Daniel Kahn Gillmor  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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


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

2019-05-24 Thread David Bremner
Daniel Kahn Gillmor  writes:

> On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote:
>> Daniel Kahn Gillmor  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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


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

2019-05-24 Thread Daniel Kahn Gillmor
On Thu 2019-05-23 22:13:59 -0300, David Bremner wrote:
> Daniel Kahn Gillmor  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


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


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

2019-05-23 Thread David Bremner
Daniel Kahn Gillmor  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.
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


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

2019-05-19 Thread Daniel Kahn Gillmor
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


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


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

2019-04-23 Thread Daniel Kahn Gillmor
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


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


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

2019-04-22 Thread Daniel Kahn Gillmor
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


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch


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

2019-04-22 Thread 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" ( 
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 +
 
 [ 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 +
 
 [ 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

___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch