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

[David Bremner]

David Bremner  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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[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
+++ 

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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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  
> 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

___
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 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


signature.asc
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[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

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