Re: [PATCH] emacs: add check for encryption before saving.

[Mark Walters]

On Sat, 05 Nov 2016, David Bremner  wrote:
> Mark Walters  writes:
>
>>
>> I think this is an excellent thing to add. I agree that false positives
>> aren't much of a worry. If someone bumps into them a lot then they can
>> complain or come up with a better regex.
>>
>
> Should the regex also be a defcustom?

I think not a defcustom, but perhaps a defvar (the difference being that
defvar's are hidden from most users -- anyone who can write a better
regex can use setq in their init file). I think even if we don't expect
users to change it making it a defvar is quite clean so that might be
best.

Best wishes

Mark





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

Re: [PATCH] emacs: add check for encryption before saving.

[David Bremner]

Mark Walters  writes:

>
> I think this is an excellent thing to add. I agree that false positives
> aren't much of a worry. If someone bumps into them a lot then they can
> complain or come up with a better regex.
>

Should the regex also be a defcustom?

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

Re: [PATCH] emacs: add check for encryption before saving.

[Mark Walters]

On Sat, 05 Nov 2016, David Bremner  wrote:
> This is intended to decrease the chance of people ending up with a bunch
> of plaintext drafts of encrypted messages without knowing it.
>
> The check is intentionally overcautious; I think the false positive of
> misplaced #secure tag is probably OK here.
> ---
>
> This is somewhat RFC. The regex needs to be double checked, and the
> variable name is not ideal. However it does solve reduce a worry I
> have about this code saving drafts of sensitive messages in plaintext
> that are effectively invisible because they are tagged deleted.

Hi

I think this is an excellent thing to add. I agree that false positives
aren't much of a worry. If someone bumps into them a lot then they can
complain or come up with a better regex.

>  emacs/notmuch-message.el | 25 +
>  1 file changed, 25 insertions(+)
>
> diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
> index a503296..a2b079d 100644
> --- a/emacs/notmuch-message.el
> +++ b/emacs/notmuch-message.el
> @@ -80,6 +80,12 @@ postponing and resuming a message."
>:type '(repeat string)
>:group 'notmuch-send)
>  
> +(defcustom notmuch-message-warn-encryption t
> +  "Warn if the user postpones or saves a message with an mml encryption tag 
> in it"
> +  :type 'boolean
> +  :group 'notmuch-send
> +  :group 'notmuch-crypto)

I think it would be good if the variable name contained postpone or save
in it as it is not part of the normal send message route. Perhaps
notmuch-message-warn-unencrypted-save ? (not perfect I know)

Maybe change the docstring to something like "Warn if the user postpones
or saves a message that would be encrypted if sent (i.e., has an mml
encryption tag)."

> +(defun notmuch-message-check-encryption ()
> +  "Query user if there an mml tag that looks like it might indicate 
> encryption.

Maybe a name like notmuch-message-check-has-encrypt-tag (or omit "check")?

But this is probably excessive bikeshedding on my part. In any case the
only one I care about above is the name of the defcustom variable

(and there is one trivial typo below)

Best wishes

Mark

> +Returns t if there is no such tag, or the user confirms they mean
> +it."
> +  (save-excursion
> +(message-goto-body)
> +  (or
> +   ;; We fine if there is no secure tag, and no #part encryption
 ^^^
 are
  

> +   (not (re-search-forward "<#\\(part 
> encrypt\\|secure.*mode=.*encrypt>\\)" nil 't))
> +   ;; The user confirms they means it.
> +   (yes-or-no-p "\
> +This message contains mml tags that suggest it is intended to be encrypted.
> +Really save and index an unencrypted copy?
> +(Customize `notmuch-message-warn-encrypted' to avoid this warning)"
> +
>  (defun notmuch-message-save-draft ()
>"Save the current draft message in the notmuch database.
>  
> @@ -147,6 +169,9 @@ This saves the current message in the database with tags
>  `notmuch-message-draft-tags` (in addition to any default tags
>  applied to newly inserted messages)."
>(interactive)
> +  (when (and notmuch-message-warn-encryption
> +  (not (notmuch-message-check-encryption))
> +  (error "Save aborted")))
>(let (;; We need the message id as we need it for tagging. Note
>   ;; message-make-message-id gives the id inside a "<" ">" pair,
>   ;; but notmuch doesn't want that form, so remove them.
> -- 
> 2.10.1
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] emacs: add check for encryption before saving.

[David Bremner]

This is intended to decrease the chance of people ending up with a bunch
of plaintext drafts of encrypted messages without knowing it.

The check is intentionally overcautious; I think the false positive of
misplaced #secure tag is probably OK here.
---

This is somewhat RFC. The regex needs to be double checked, and the
variable name is not ideal. However it does solve reduce a worry I
have about this code saving drafts of sensitive messages in plaintext
that are effectively invisible because they are tagged deleted.

 emacs/notmuch-message.el | 25 +
 1 file changed, 25 insertions(+)

diff --git a/emacs/notmuch-message.el b/emacs/notmuch-message.el
index a503296..a2b079d 100644
--- a/emacs/notmuch-message.el
+++ b/emacs/notmuch-message.el
@@ -80,6 +80,12 @@ postponing and resuming a message."
   :type '(repeat string)
   :group 'notmuch-send)
 
+(defcustom notmuch-message-warn-encryption t
+  "Warn if the user postpones or saves a message with an mml encryption tag in 
it"
+  :type 'boolean
+  :group 'notmuch-send
+  :group 'notmuch-crypto)
+
 (defvar notmuch-message-draft-id nil
   "Message-id of the most recent saved draft of this message")
 (make-variable-buffer-local 'notmuch-message-draft-id)
@@ -140,6 +146,22 @@ Used when a new version is saved, or the message is sent."
   (when secure-tag
(insert secure-tag "\n")
 
+(defun notmuch-message-check-encryption ()
+  "Query user if there an mml tag that looks like it might indicate encryption.
+
+Returns t if there is no such tag, or the user confirms they mean
+it."
+  (save-excursion
+(message-goto-body)
+  (or
+   ;; We fine if there is no secure tag, and no #part encryption
+   (not (re-search-forward "<#\\(part 
encrypt\\|secure.*mode=.*encrypt>\\)" nil 't))
+   ;; The user confirms they means it.
+   (yes-or-no-p "\
+This message contains mml tags that suggest it is intended to be encrypted.
+Really save and index an unencrypted copy?
+(Customize `notmuch-message-warn-encrypted' to avoid this warning)"
+
 (defun notmuch-message-save-draft ()
   "Save the current draft message in the notmuch database.
 
@@ -147,6 +169,9 @@ This saves the current message in the database with tags
 `notmuch-message-draft-tags` (in addition to any default tags
 applied to newly inserted messages)."
   (interactive)
+  (when (and notmuch-message-warn-encryption
+(not (notmuch-message-check-encryption))
+(error "Save aborted")))
   (let (;; We need the message id as we need it for tagging. Note
;; message-make-message-id gives the id inside a "<" ">" pair,
;; but notmuch doesn't want that form, so remove them.
-- 
2.10.1

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