bug#18718: Encrypted messages expose Bcc identities

bug#18718: Encrypted messages expose Bcc identities

[Lars Ingebrigtsen]

Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:

> 4. Send an encrypted e-mail to yourself, with one To address and one
>    Bcc address.  Read the mail received under the To address, where
>    you should not be able to identify the Bcc recipient.
>    Note that buffer *epg-debug* mentions that the message was
>    encrypted to two keys (including both key IDs and e-mail
>    addresses).  The Bcc recipient is clearly visible.

[...]

> On 2014-09-21 I posted some suggestions to the ding mailing list
> concerning my package DefaultEncrypt, which contains a workaround.

Would it be possible for you to create a patch for this against the
version of Message in 25.1?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18718: Encrypted messages expose Bcc identities

[Jens Lechtenboerger]

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

On 2015-12-26, at 22:34, Lars Ingebrigtsen wrote:

> Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:
>
>> 4. Send an encrypted e-mail to yourself, with one To address and one
>>    Bcc address.  Read the mail received under the To address, where
>>    you should not be able to identify the Bcc recipient.
>>    Note that buffer *epg-debug* mentions that the message was
>>    encrypted to two keys (including both key IDs and e-mail
>>    addresses).  The Bcc recipient is clearly visible.
>
> [...]
>
>> On 2014-09-21 I posted some suggestions to the ding mailing list
>> concerning my package DefaultEncrypt, which contains a workaround.
>
> Would it be possible for you to create a patch for this against the
> version of Message in 25.1?

A patch is attached.  The new function mml-secure-bcc-is-safe does
nothing on its own but can be added to message-send-hook or called
from message-send and friends.

Concerning documentation: I’m currently involved in a refactoring
effort for encryption related functionality, which takes place in
the Gnus git under branch mml-refactoring.  There, Message
documentation is already extended with a section “Bcc Warning” which
could be extended.

(Also, gnus-subsetp, which is part of this patch, is already present
in the branch mml-refactoring.)

Best wishes
Jens


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Identify-unsafe-combinations-of-Bcc-and-encryption.patch --]
[-- Type: text/x-diff, Size: 4308 bytes --]

From f9fb01a6b013963e0d8021b5da587cc548c1ea9a Mon Sep 17 00:00:00 2001
From: Jens Lechtenboerger <jens.lechtenboerger@fsfe.org>
Date: Sun, 27 Dec 2015 16:29:02 +0100
Subject: [PATCH] Identify unsafe combinations of Bcc and encryption

---
 ChangeLog.2            |  8 ++++++++
 lisp/gnus/gnus-util.el | 10 ++++++++++
 lisp/gnus/mml-sec.el   | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+)

diff --git a/ChangeLog.2 b/ChangeLog.2
index 6d72663..971a3b5 100644
--- a/ChangeLog.2
+++ b/ChangeLog.2
@@ -1,3 +1,11 @@
+2015-12-27  Jens Lechtenboerger  <jens.lechtenboerger@fsfe.org>
+
+	Identify unsafe combinations of Bcc and encryption
+
+	* lisp/gnus/gnus-util.el (gnus-subsetp): New function
+	* lisp/gnus/mml-sec.el (mml-secure-safe-bcc-list): New variable
+	* lisp/gnus/mml-sec.el (mml-secure-bcc-is-safe): New function
+
 2015-12-27  Lars Ingebrigtsen  <larsi@gnus.org>
 
 	* shr.el (shr-descend): Allow using lambdas in external functions.
diff --git a/lisp/gnus/gnus-util.el b/lisp/gnus/gnus-util.el
index 40e2dcf..933387d 100644
--- a/lisp/gnus/gnus-util.el
+++ b/lisp/gnus/gnus-util.el
@@ -1989,6 +1989,16 @@ to case differences."
   (defun gnus-timer--function (timer)
     (elt timer 5)))
 
+(defun gnus-subsetp (list1 list2)
+  "Return t if LIST1 is a subset of LIST2.
+Similar to `subsetp' but use member for element test so that this works for
+lists of strings."
+  (when (and (listp list1) (listp list2))
+    (if list1
+	(and (member (car list1) list2)
+	     (gnus-subsetp (cdr list1) list2))
+      t)))
+
 (provide 'gnus-util)
 
 ;;; gnus-util.el ends here
diff --git a/lisp/gnus/mml-sec.el b/lisp/gnus/mml-sec.el
index 45da937..dbae280 100644
--- a/lisp/gnus/mml-sec.el
+++ b/lisp/gnus/mml-sec.el
@@ -122,6 +122,21 @@ Whether the passphrase is cached at all is controlled by
   :group 'message
   :type 'integer)
 
+(defcustom mml-secure-safe-bcc-list nil
+  "List of e-mail addresses that are safe to use in Bcc headers.
+EasyPG encrypts e-mails to Bcc addresses, and the encrypted e-mail
+by default identifies the used encryption keys, giving away the
+Bcc'ed identities.  Clearly, this contradicts the original goal of
+*blind* copies.
+For an academic paper explaining the problem, see URL
+`http://crypto.stanford.edu/portia/papers/bb-bcc.pdf'.
+Use this variable to specify e-mail addresses whose owners do not
+mind if they are identifiable as recipients.  This may be useful if
+you use Bcc headers to encrypt e-mails to yourself."
+  :version "25.1"
+  :group 'message
+  :type '(repeat string))
+
 ;;; Configuration/helper functions
 
 (defun mml-signencrypt-style (method &optional style)
@@ -272,6 +287,37 @@ Use METHOD if given.  Else use `mml-secure-method' or
   (interactive)
   (mml-secure-part "smime"))
 
+(defun mml-secure-is-encrypted-p ()
+  "Check whether secure encrypt tag is present."
+  (save-excursion
+    (goto-char (point-min))
+    (re-search-forward
+     (concat "^" (regexp-quote mail-header-separator) "\n"
+	     "<#secure[^>]+encrypt")
+     nil t)))
+
+(defun mml-secure-bcc-is-safe ()
+  "Check whether usage of Bcc is safe (or absent).
+Bcc usage is safe in two cases: first, if the current message does
+not contain an MML secure encrypt tag;
+second, if the Bcc addresses are a subset of `mml-secure-safe-bcc-list'.
+In all other cases, ask the user whether Bcc usage is safe.
+Raise error if user answers no.
+Note that this function does not produce a meaningful return value:
+either an error is raised or not."
+  (when (mml-secure-is-encrypted-p)
+    (let ((bcc (mail-strip-quoted-names (message-fetch-field "bcc"))))
+      (when bcc
+	;; Split recipients at "," boundary, omit empty strings (t),
+	;; and strip whitespace.
+	(let ((bcc-list (split-string hdr "," t "\\s-+")))
+	  (unless (gnus-subsetp bcc-list mml-secure-safe-bcc-list)
+	    (unless (yes-or-no-p "Message for encryption contains Bcc header.\
+  This may give away all Bcc'ed identities to all recipients.\
+  Are you sure that this is safe?\
+  (Customize `mml-secure-safe-bcc-list' to avoid this warning.) ")
+	      (error "Aborted"))))))))
+
 ;; defuns that add the proper <#secure ...> tag to the top of the message body
 (defun mml-secure-message (method &optional modesym)
   (let ((mode (prin1-to-string modesym))
-- 
1.9.1

bug#18718: Encrypted messages expose Bcc identities

[Lars Ingebrigtsen]

Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:

> A patch is attached.  The new function mml-secure-bcc-is-safe does
> nothing on its own but can be added to message-send-hook or called
> from message-send and friends.

Looks good.  Do you have Emacs copyright assignment papers on file?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18718: Encrypted messages expose Bcc identities

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Sun, 27 Dec 2015 18:59:08 +0100
> Cc: 18718@debbugs.gnu.org
> 
> Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:
> 
> > A patch is attached.  The new function mml-secure-bcc-is-safe does
> > nothing on its own but can be added to message-send-hook or called
> > from message-send and friends.
> 
> Looks good.  Do you have Emacs copyright assignment papers on file?

He does.

bug#18718: Encrypted messages expose Bcc identities

[Lars Ingebrigtsen]

Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:

> A patch is attached.  The new function mml-secure-bcc-is-safe does
> nothing on its own but can be added to message-send-hook or called
> from message-send and friends.

I've applied the patch, but there were bugs.  It referred to an unbound
variable called "hdr", which I've changed to bcc.  Please look over the
resulting code.

Also, I get these warnings:

In end of data:
gnus/mml-sec.el:429:1:Warning: the following functions are not known to be
    defined: mail-strip-quoted-names, message-fetch-field, gnus-subsetp


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18718: Encrypted messages expose Bcc identities

[Jens Lechtenboerger]

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

On 2015-12-27, at 19:26, Lars Ingebrigtsen wrote:

> Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:
>
>> A patch is attached.  The new function mml-secure-bcc-is-safe does
>> nothing on its own but can be added to message-send-hook or called
>> from message-send and friends.
>
> I've applied the patch, but there were bugs.  It referred to an unbound
> variable called "hdr", which I've changed to bcc.  Please look over the
> resulting code.

You are right.  I tested against the wrong load-path.  Sorry.

> Also, I get these warnings:
>
> In end of data:
> gnus/mml-sec.el:429:1:Warning: the following functions are not known to be
>     defined: mail-strip-quoted-names, message-fetch-field, gnus-subsetp

Indeed.  Actually, when should I use require, when autoload?  In
particular, for gnus-util both variants are used in different files,
and I fail to see a pattern.  As mml-sec just uses autoloads, the
attached patch adds more of them to avoid the warnings.

Best wishes
Jens


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-More-autoloads-to-avoid-compile-warnings.patch --]
[-- Type: text/x-diff, Size: 904 bytes --]

From 1f54b417fd487880f794cfff2eecceb87a07d4d8 Mon Sep 17 00:00:00 2001
From: Jens Lechtenboerger <jens.lechtenboerger@fsfe.org>
Date: Sun, 27 Dec 2015 20:40:15 +0100
Subject: [PATCH 2/2] More autoloads to avoid compile warnings

---
 lisp/gnus/mml-sec.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/gnus/mml-sec.el b/lisp/gnus/mml-sec.el
index dbae280..d7702d7 100644
--- a/lisp/gnus/mml-sec.el
+++ b/lisp/gnus/mml-sec.el
@@ -25,10 +25,13 @@
 
 (eval-when-compile (require 'cl))
 
+(autoload 'gnus-subsetp "gnus-util")
+(autoload 'mail-strip-quoted-names "mail-utils")
 (autoload 'mml2015-sign "mml2015")
 (autoload 'mml2015-encrypt "mml2015")
 (autoload 'mml1991-sign "mml1991")
 (autoload 'mml1991-encrypt "mml1991")
+(autoload 'message-fetch-field "message")
 (autoload 'message-goto-body "message")
 (autoload 'mml-insert-tag "mml")
 (autoload 'mml-smime-sign "mml-smime")
-- 
1.9.1

bug#18718: Encrypted messages expose Bcc identities

[Lars Ingebrigtsen]

Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:

> Indeed.  Actually, when should I use require, when autoload?  In
> particular, for gnus-util both variants are used in different files,
> and I fail to see a pattern.  As mml-sec just uses autoloads, the
> attached patch adds more of them to avoid the warnings.

Thanks; applied.

There's no hard and fast rule, especially with these libraries that tend
to infloop if you add too many requires.  :-)  (That is, a requires b
that requires c that requires a...)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18718: acknowledged by developer (control message for bug #18718)

[Jens Lechtenboerger]

I don’t think that mml-secure-bcc-is-safe gets called so far, which
means that the bug still exists.

As I wrote concerning the patch, the function could be added to
message-send-hook or called from message-send.  I don’t know what
would be preferable.

bug#18718: acknowledged by developer (control message for bug #18718)

[Lars Magne Ingebrigtsen]

Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:

> I don’t think that mml-secure-bcc-is-safe gets called so far, which
> means that the bug still exists.
>
> As I wrote concerning the patch, the function could be added to
> message-send-hook or called from message-send.  I don’t know what
> would be preferable.

Calling from message-send sounds better, I think.  Could you send a
patch?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18718: acknowledged by developer (control message for bug #18718)

[Jens Lechtenboerger]

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

On 2016-01-03, at 10:08, Lars Magne Ingebrigtsen wrote:

> Calling from message-send sounds better, I think.  Could you send a
> patch?

This should do it.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Call-mml-secure-bcc-is-safe-for-bug-18718.patch --]
[-- Type: text/x-diff, Size: 1238 bytes --]

From c1cae98181cb05a001a4b0b3216f4aa072aaed6c Mon Sep 17 00:00:00 2001
From: Jens Lechtenboerger <jens.lechtenboerger@fsfe.org>
Date: Sun, 3 Jan 2016 15:48:43 +0100
Subject: [PATCH] Call mml-secure-bcc-is-safe for bug#18718

* lisp/gnus/message.el (message-send): Call mml-secure-bcc-is-safe
---
 lisp/gnus/message.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/gnus/message.el b/lisp/gnus/message.el
index a6c8282..1e2e3bd 100644
--- a/lisp/gnus/message.el
+++ b/lisp/gnus/message.el
@@ -4227,6 +4227,8 @@ Instead, just auto-save the buffer and then bury it."
   (if message-return-action
       (apply (car message-return-action) (cdr message-return-action))))
 
+(autoload 'mml-secure-bcc-is-safe "mml-sec")
+
 (defun message-send (&optional arg)
   "Send the message in the current buffer.
 If `message-interactive' is non-nil, wait for success indication or
@@ -4241,6 +4243,7 @@ It should typically alter the sending method in some way or other."
   (let ((inhibit-read-only t))
     (put-text-property (point-min) (point-max) 'read-only nil))
   (message-fix-before-sending)
+  (mml-secure-bcc-is-safe)
   (run-hooks 'message-send-hook)
   (when message-confirm-send
     (or (y-or-n-p "Send message? ")
-- 
1.9.1

bug#18718: acknowledged by developer (control message for bug #18718)

[Lars Magne Ingebrigtsen]

Jens Lechtenboerger <jens.lechtenboerger@fsfe.org> writes:

> On 2016-01-03, at 10:08, Lars Magne Ingebrigtsen wrote:
>
>> Calling from message-send sounds better, I think.  Could you send a
>> patch?
>
> This should do it.

Thanks; applied to Emacs 25.1.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no