[PATCH] emacs: change default for notmuch-crypto-process-mime to t

[PATCH] emacs: change default for notmuch-crypto-process-mime to t

[David Bremner]

There are some cases like remote usage where this might cause
problems, but those users can easily customize the variable. The
inconvenience seems to be outweighed by the security benefit for most
users.
---
 emacs/notmuch-crypto.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 0af727ef..fc2b5301 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -24,7 +24,7 @@
 (require 'epg)
 (require 'notmuch-lib)
 
-(defcustom notmuch-crypto-process-mime nil
+(defcustom notmuch-crypto-process-mime t
   "Should cryptographic MIME parts be processed?
 
 If this variable is non-nil signatures in multipart/signed
@@ -40,6 +40,7 @@ providing a prefix when viewing a signed or encrypted message, or
 by providing a prefix when reloading the message in notmuch-show
 mode."
   :type 'boolean
+  :package-version '(notmuch . "0.25")
   :group 'notmuch-crypto)
 
 (defface notmuch-crypto-part-header
-- 
2.13.2

Re: [PATCH] emacs: change default for notmuch-crypto-process-mime to t

[Tomi Ollila]

On Sun, Jul 09 2017, David Bremner wrote:

> There are some cases like remote usage where this might cause
> problems, but those users can easily customize the variable. The
> inconvenience seems to be outweighed by the security benefit for most
> users.
> ---

Trivial enough to LGTM codewise -- too little crypto usage for me to
comment on functionality (other that stronger crypto by default FTW)

Tomi



>  emacs/notmuch-crypto.el | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
> index 0af727ef..fc2b5301 100644
> --- a/emacs/notmuch-crypto.el
> +++ b/emacs/notmuch-crypto.el
> @@ -24,7 +24,7 @@
>  (require 'epg)
>  (require 'notmuch-lib)
>  
> -(defcustom notmuch-crypto-process-mime nil
> +(defcustom notmuch-crypto-process-mime t
>    "Should cryptographic MIME parts be processed?
>  
>  If this variable is non-nil signatures in multipart/signed
> @@ -40,6 +40,7 @@ providing a prefix when viewing a signed or encrypted message, or
>  by providing a prefix when reloading the message in notmuch-show
>  mode."
>    :type 'boolean
> +  :package-version '(notmuch . "0.25")
>    :group 'notmuch-crypto)
>  
>  (defface notmuch-crypto-part-header
> -- 
> 2.13.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: change default for notmuch-crypto-process-mime to t

[Daniel Kahn Gillmor]

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

On Sun 2017-07-09 07:46:14 -0300, David Bremner wrote:
> There are some cases like remote usage where this might cause
> problems, but those users can easily customize the variable. The
> inconvenience seems to be outweighed by the security benefit for most
> users.

lgtm.  i'm not sure that this change is technically a "security
benefit", though, it looks more like a "usability benefit", since the
main use of process-crypto is likely to be decrypting messages.

for signature verification, there's some small security benefit, but
since it's mainly exposure of interesting information to the user (as
opposed to blocking users from doing unsafe things) it's still probably
more on the usability side than security.

still, i think it's a good change.  If it uncovers performance problems
on use cases that normal people care about, hopefully we can get
examples of those use cases and get the performance problems fixed
(rather than just encouraging those users to set the flag to nil).

     --dkg

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] emacs: change default for notmuch-crypto-process-mime to t

[Brian Sniffen]

Gpg is exposed to some zip bomb problems last I looked. But the worst that could do is fill your disk or crash your Emacs, right?  And I suspect the MIME library exposes similar issues in quantity. 

-- 
Brian Sniffen

> On Jul 10, 2017, at 4:42 PM, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> 
>> On Sun 2017-07-09 07:46:14 -0300, David Bremner wrote:
>> There are some cases like remote usage where this might cause
>> problems, but those users can easily customize the variable. The
>> inconvenience seems to be outweighed by the security benefit for most
>> users.
> 
> lgtm.  i'm not sure that this change is technically a "security
> benefit", though, it looks more like a "usability benefit", since the
> main use of process-crypto is likely to be decrypting messages.
> 
> for signature verification, there's some small security benefit, but
> since it's mainly exposure of interesting information to the user (as
> opposed to blocking users from doing unsafe things) it's still probably
> more on the usability side than security.
> 
> still, i think it's a good change.  If it uncovers performance problems
> on use cases that normal people care about, hopefully we can get
> examples of those use cases and get the performance problems fixed
> (rather than just encouraging those users to set the flag to nil).
> 
>     --dkg
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: change default for notmuch-crypto-process-mime to t

[Daniel Kahn Gillmor]

On Mon 2017-07-10 20:48:40 -0400, Brian Sniffen wrote:
> Gpg is exposed to some zip bomb problems last I looked. But the worst
> that could do is fill your disk or crash your Emacs, right?  And I
> suspect the MIME library exposes similar issues in quantity.

Could you point to the zip bomb problem, Brian?  the quine (infinite
zipbomb) i think is limited by some sort of hard-coded depth constant.
are you referring to an infinite blowup, or "just" a finite expansion?

i agree that i expect GMime to be subject to finite expansions as well
(i haven't experimented with them though), but i think neither gpg nor
GMime should be subject to infinite expansion.

if you think otherwise, i'd be happy to read pointers.

thanks for raising this concern!

       --dkg

[PATCH] emacs: change default for notmuch-crypto-process-mime to t

[David Bremner]

There are some cases like remote usage where this might cause
problems, but those users can easily customize the variable. The
inconvenience seems to be outweighed by the security benefit for most
users.
---

 This version with test suite fixes.
 
 emacs/notmuch-crypto.el                                           | 3 ++-
 test/T450-emacs-show.sh                                           | 2 +-
 .../notmuch-show-elide-non-matching-messages-off                  | 6 +++---
 .../notmuch-show-elide-non-matching-messages-on                   | 6 +++---
 .../notmuch-show-indent-thread-content-off                        | 6 +++---
 test/emacs.expected-output/notmuch-show-thread-maildir-storage    | 8 ++++----
 .../notmuch-show-thread-maildir-storage-with-fourfold-indentation | 8 ++++----
 .../notmuch-show-thread-maildir-storage-without-indentation       | 8 ++++----
 test/tree.expected-output/notmuch-tree-show-window                | 2 +-
 9 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/emacs/notmuch-crypto.el b/emacs/notmuch-crypto.el
index 0af727ef..fc2b5301 100644
--- a/emacs/notmuch-crypto.el
+++ b/emacs/notmuch-crypto.el
@@ -24,7 +24,7 @@
 (require 'epg)
 (require 'notmuch-lib)
 
-(defcustom notmuch-crypto-process-mime nil
+(defcustom notmuch-crypto-process-mime t
   "Should cryptographic MIME parts be processed?
 
 If this variable is non-nil signatures in multipart/signed
@@ -40,6 +40,7 @@ providing a prefix when viewing a signed or encrypted message, or
 by providing a prefix when reloading the message in notmuch-show
 mode."
   :type 'boolean
+  :package-version '(notmuch . "0.25")
   :group 'notmuch-crypto)
 
 (defface notmuch-crypto-part-header
diff --git a/test/T450-emacs-show.sh b/test/T450-emacs-show.sh
index c4bc5ce0..db48c7d5 100755
--- a/test/T450-emacs-show.sh
+++ b/test/T450-emacs-show.sh
@@ -191,7 +191,7 @@ This is an error (see *Notmuch errors* for more details)
 === ERROR ===
 [XXX]
 This is an error
-command: YYY/notmuch_fail show --format\\=sexp --format-version\\=4 --exclude\\=false \\' \\* \\'
+command: YYY/notmuch_fail show --format\\=sexp --format-version\\=4 --decrypt --exclude\\=false \\' \\* \\'
 exit status: 1
 stderr:
 This is an error
diff --git a/test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-off b/test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-off
index 9f1e91f0..e0bd2c73 100644
--- a/test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-off
+++ b/test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-off
@@ -5,7 +5,7 @@ Date: Tue, 17 Nov 2009 14:00:54 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 I saw the LWN article and decided to take a look at notmuch.  I'm
 currently using mutt and mairix to index and read a collection of
@@ -40,7 +40,7 @@ Cheers,
 
   [ multipart/mixed ]
   [ multipart/signed ]
-  [ Unknown signature status ]
+  [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
   [ text/plain ]
   > See the patch just posted here.
 
@@ -67,7 +67,7 @@ Cheers,
 
     [ multipart/mixed ]
     [ multipart/signed ]
-    [ Unknown signature status ]
+    [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
     [ text/plain ]
     > I've also pushed a slightly more complicated (and complete) fix to my
     > private notmuch repository
diff --git a/test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-on b/test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-on
index 118053ba..d76d0952 100644
--- a/test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-on
+++ b/test/emacs-show.expected-output/notmuch-show-elide-non-matching-messages-on
@@ -5,7 +5,7 @@ Date: Tue, 17 Nov 2009 14:00:54 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 I saw the LWN article and decided to take a look at notmuch.  I'm
 currently using mutt and mairix to index and read a collection of
@@ -39,7 +39,7 @@ Cheers,
 
   [ multipart/mixed ]
   [ multipart/signed ]
-  [ Unknown signature status ]
+  [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
   [ text/plain ]
   > See the patch just posted here.
 
@@ -64,7 +64,7 @@ Cheers,
 
     [ multipart/mixed ]
     [ multipart/signed ]
-    [ Unknown signature status ]
+    [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
     [ text/plain ]
     > I've also pushed a slightly more complicated (and complete) fix to my
     > private notmuch repository
diff --git a/test/emacs-show.expected-output/notmuch-show-indent-thread-content-off b/test/emacs-show.expected-output/notmuch-show-indent-thread-content-off
index 2cb12118..1a06374d 100644
--- a/test/emacs-show.expected-output/notmuch-show-indent-thread-content-off
+++ b/test/emacs-show.expected-output/notmuch-show-indent-thread-content-off
@@ -5,7 +5,7 @@ Date: Tue, 17 Nov 2009 14:00:54 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 I saw the LWN article and decided to take a look at notmuch.  I'm
 currently using mutt and mairix to index and read a collection of
@@ -40,7 +40,7 @@ Date: Tue, 17 Nov 2009 15:33:01 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 > See the patch just posted here.
 
@@ -67,7 +67,7 @@ Date: Tue, 17 Nov 2009 19:50:40 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 > I've also pushed a slightly more complicated (and complete) fix to my
 > private notmuch repository
diff --git a/test/emacs.expected-output/notmuch-show-thread-maildir-storage b/test/emacs.expected-output/notmuch-show-thread-maildir-storage
index 35998922..1f89dbef 100644
--- a/test/emacs.expected-output/notmuch-show-thread-maildir-storage
+++ b/test/emacs.expected-output/notmuch-show-thread-maildir-storage
@@ -5,7 +5,7 @@ Date: Tue, 17 Nov 2009 14:00:54 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 I saw the LWN article and decided to take a look at notmuch.  I'm
 currently using mutt and mairix to index and read a collection of
@@ -46,7 +46,7 @@ http://notmuchmail.org/mailman/listinfo/notmuch
 
  [ multipart/mixed ]
  [ multipart/signed ]
- [ Unknown signature status ]
+ [ Unknown key ID 0x9D20F6503E338888 or unsupported algorithm ]
  [ text/plain ]
 
  Twas brillig at 14:00:54 17.11.2009 UTC-05 when lars@seas.harvard.edu did
@@ -79,7 +79,7 @@ http://notmuchmail.org/mailman/listinfo/notmuch
 
   [ multipart/mixed ]
   [ multipart/signed ]
-  [ Unknown signature status ]
+  [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
   [ text/plain ]
   > See the patch just posted here.
 
@@ -162,7 +162,7 @@ http://notmuchmail.org/mailman/listinfo/notmuch
 
     [ multipart/mixed ]
     [ multipart/signed ]
-    [ Unknown signature status ]
+    [ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
     [ text/plain ]
     > I've also pushed a slightly more complicated (and complete) fix to my
     > private notmuch repository
diff --git a/test/emacs.expected-output/notmuch-show-thread-maildir-storage-with-fourfold-indentation b/test/emacs.expected-output/notmuch-show-thread-maildir-storage-with-fourfold-indentation
index 3bbb114a..5c4ec979 100644
--- a/test/emacs.expected-output/notmuch-show-thread-maildir-storage-with-fourfold-indentation
+++ b/test/emacs.expected-output/notmuch-show-thread-maildir-storage-with-fourfold-indentation
@@ -5,7 +5,7 @@ Date: Tue, 17 Nov 2009 14:00:54 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 I saw the LWN article and decided to take a look at notmuch.  I'm
 currently using mutt and mairix to index and read a collection of
@@ -46,7 +46,7 @@ http://notmuchmail.org/mailman/listinfo/notmuch
 
     [ multipart/mixed ]
     [ multipart/signed ]
-    [ Unknown signature status ]
+    [ Unknown key ID 0x9D20F6503E338888 or unsupported algorithm ]
     [ text/plain ]
 
     Twas brillig at 14:00:54 17.11.2009 UTC-05 when lars@seas.harvard.edu did
@@ -79,7 +79,7 @@ http://notmuchmail.org/mailman/listinfo/notmuch
 
 	[ multipart/mixed ]
 	[ multipart/signed ]
-	[ Unknown signature status ]
+	[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 	[ text/plain ]
 	> See the patch just posted here.
 
@@ -165,7 +165,7 @@ http://notmuchmail.org/mailman/listinfo/notmuch
 
 		[ multipart/mixed ]
 		[ multipart/signed ]
-		[ Unknown signature status ]
+		[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 		[ text/plain ]
 		> I've also pushed a slightly more complicated (and complete)
 		> fix to my
diff --git a/test/emacs.expected-output/notmuch-show-thread-maildir-storage-without-indentation b/test/emacs.expected-output/notmuch-show-thread-maildir-storage-without-indentation
index 620caa00..24cdd56e 100644
--- a/test/emacs.expected-output/notmuch-show-thread-maildir-storage-without-indentation
+++ b/test/emacs.expected-output/notmuch-show-thread-maildir-storage-without-indentation
@@ -5,7 +5,7 @@ Date: Tue, 17 Nov 2009 14:00:54 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 I saw the LWN article and decided to take a look at notmuch.  I'm
 currently using mutt and mairix to index and read a collection of
@@ -46,7 +46,7 @@ Date: Wed, 18 Nov 2009 01:02:38 +0600
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0x9D20F6503E338888 or unsupported algorithm ]
 [ text/plain ]
 
 Twas brillig at 14:00:54 17.11.2009 UTC-05 when lars@seas.harvard.edu did gyre
@@ -79,7 +79,7 @@ Date: Tue, 17 Nov 2009 15:33:01 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 > See the patch just posted here.
 
@@ -162,7 +162,7 @@ Date: Tue, 17 Nov 2009 19:50:40 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 > I've also pushed a slightly more complicated (and complete) fix to my
 > private notmuch repository
diff --git a/test/tree.expected-output/notmuch-tree-show-window b/test/tree.expected-output/notmuch-tree-show-window
index ab7205b7..7d860c6f 100644
--- a/test/tree.expected-output/notmuch-tree-show-window
+++ b/test/tree.expected-output/notmuch-tree-show-window
@@ -5,7 +5,7 @@ Date: Tue, 17 Nov 2009 14:00:54 -0500
 
 [ multipart/mixed ]
 [ multipart/signed ]
-[ Unknown signature status ]
+[ Unknown key ID 0xD74695063141ACD8 or unsupported algorithm ]
 [ text/plain ]
 I saw the LWN article and decided to take a look at notmuch.  I'm
 currently using mutt and mairix to index and read a collection of
-- 
2.13.2

Re: [PATCH] emacs: change default for notmuch-crypto-process-mime to t

[David Bremner]

David Bremner <david@tethera.net> writes:

> There are some cases like remote usage where this might cause
> problems, but those users can easily customize the variable. The
> inconvenience seems to be outweighed by the security benefit for most
> users.

pushed to master

d