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

[Mark Walters]

On Sat, 05 Nov 2016, David Bremner <david at tethera.net> 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