[PATCH 0/2] Check for misplaced secure mml tags

[PATCH 0/2] Check for misplaced secure mml tags

[Mark Walters]

This is new (essentially completely rewritten) version of
id:1475008491-28175-1-git-send-email-markwalters1009@gmail.com

This version is stricter in its checking. I believe emacs only
processes a secure tag if it as the start of the body and followed by
a newline. Thus if there is a secure tag anywhere else (including in
the headers), or it is not followed by a newline we query the user.

The logic is a little convoluted but it seems to work in all cases I
have tried.

The extra strictness over the previous version is partly based on
experience from my current (not yet posted) version of the postpone
code. I will store the secure tag in a header while it is saved (so
checking the header seems worth doing), and one version restored the
secure tag. but not on its own line and that caused problems.

We could consider adding other checks later -- generally I think
sending a malformed email is bad but not terrible, but accidentally
sending a message unencrypted is terrible so we should be stricter
here.

Finally, there are other possible corruptions of a secure tag, but
this seems a good start.

Best wishes

Mark


Mark Walters (2):
  emacs: mua: extract a common message-send function.
  emacs: mua: check for misplaced secure mml tags

 emacs/notmuch-mua.el | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

-- 
2.1.4

[PATCH 1/2] emacs: mua: extract a common message-send function.

[Mark Walters]

This commit adds a common message-send function for message-send and
message-send-and-exit. At the moment the overlap is small, but the
message-send function will get more complex.
---
 emacs/notmuch-mua.el | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 55bc267..72fb770 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -490,15 +490,20 @@ will be addressed to all recipients of the source message."
     (notmuch-mua-reply query-string sender reply-all)
     (deactivate-mark)))
 
-(defun notmuch-mua-send-and-exit (&optional arg)
+(defun notmuch-mua-send-common (arg &optional exit)
   (interactive "P")
   (letf (((symbol-function 'message-do-fcc) #'notmuch-maildir-message-do-fcc))
-	(message-send-and-exit arg)))
+	(if exit
+	    (message-send-and-exit arg)
+	  (message-send arg))))
+
+(defun notmuch-mua-send-and-exit (&optional arg)
+  (interactive "P")
+  (notmuch-mua-send-common arg 't))
 
 (defun notmuch-mua-send (&optional arg)
   (interactive "P")
-  (letf (((symbol-function 'message-do-fcc) #'notmuch-maildir-message-do-fcc))
-	(message-send arg)))
+  (notmuch-mua-send-common arg))
 
 (defun notmuch-mua-kill-buffer ()
   (interactive)
-- 
2.1.4

[PATCH 2/2] emacs: mua: check for misplaced secure mml tags

[Mark Walters]

Emacs message-send seems to ignore a secure mml tag anywhere except at
the start of the body, and it must be followed by a newline. Since
this is almost certainly not desired we check for it, and require user
confirmation before sending.

As the setup before message-send or message-send-and-exit is getting
more complicated it is convenient to unify the two correspoinding
notmuch functions.
---
 emacs/notmuch-mua.el | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-mua.el b/emacs/notmuch-mua.el
index 72fb770..bae95f3 100644
--- a/emacs/notmuch-mua.el
+++ b/emacs/notmuch-mua.el
@@ -490,12 +490,37 @@ will be addressed to all recipients of the source message."
     (notmuch-mua-reply query-string sender reply-all)
     (deactivate-mark)))
 
+(defun notmuch-mua-misplaced-secure-tag ()
+  "Query user if there is a misplaced secure mml tag.
+
+Emacs message-send will (probably) ignore a secure mml tag unless
+it is at the start of the body and followed by a newline. Since
+this is almost certainly not desired we check for it, and get
+confirmation from the user if there is such a tag. Returns t if
+there is such a tag unless the user confirms they mean it."
+  (save-excursion
+    (let ((body-start (progn (message-goto-body) (point))))
+      (goto-char (point-max))
+      ;; We are always fine if there is no secure tag.
+      (when (search-backward "<#secure" nil 't)
+	;; There is a secure tag, so it must be at the start of the
+	;; body, with no secure tag earlier (i.e., in the headers) and
+	;; it must be followed by a newline.
+	(unless (and (= (point) body-start)
+		     (not (search-backward "<#secure" nil 't))
+		     (looking-at "<#secure[^\n>]*>\n"))
+	  (not (yes-or-no-p "\
+There is a <#secure> tag not at the start of the body. It is
+likely that the message will be sent unsigned and unencrypted.
+Really send? ")))))))
+
 (defun notmuch-mua-send-common (arg &optional exit)
   (interactive "P")
   (letf (((symbol-function 'message-do-fcc) #'notmuch-maildir-message-do-fcc))
-	(if exit
-	    (message-send-and-exit arg)
-	  (message-send arg))))
+	(unless (notmuch-mua-misplaced-secure-tag)
+	  (if exit
+	      (message-send-and-exit arg)
+	    (message-send arg)))))
 
 (defun notmuch-mua-send-and-exit (&optional arg)
   (interactive "P")
-- 
2.1.4

Re: [PATCH 2/2] emacs: mua: check for misplaced secure mml tags

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> +  (save-excursion
> +    (let ((body-start (progn (message-goto-body) (point))))
> +      (goto-char (point-max))
> +      ;; We are always fine if there is no secure tag.
> +      (when (search-backward "<#secure" nil 't)
> +	;; There is a secure tag, so it must be at the start of the
> +	;; body, with no secure tag earlier (i.e., in the headers) and
> +	;; it must be followed by a newline.
> +	(unless (and (= (point) body-start)
> +		     (not (search-backward "<#secure" nil 't))
> +		     (looking-at "<#secure[^\n>]*>\n"))

I believe the tag is actually "#secure" not "#!secure"

> +	  (not (yes-or-no-p "\
> +There is a <#secure> tag not at the start of the body. It is
> +likely that the message will be sent unsigned and unencrypted.
> +Really send? ")))))))
> +

This is message is a bit misleading if tag is at the begining of the
body but is not followed by a newline

d