Re: master 91c732f: Always check for client-certificates

Re: master 91c732f: Always check for client-certificates

[Dmitry Alexandrov]

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

rpluim@gmail.com (Robert Pluim) wrote:
> branch: master
> commit 91c732f687a61ba130acf38d5142bec6369ebd68
> Author: Robert Pluim <rpluim@gmail.com>
> Commit: Robert Pluim <rpluim@gmail.com>
>
>     Always check for client-certificates
>     
>     * lisp/net/network-stream.el
>     (network-stream-use-client-certificates): New user option.
>     (open-network-stream): If 'network-stream-use-client-certificates'
>     is t, and the user hasn't specified :client-certificate, do
>     certificate lookups via 'auth-source'.
>     (network-stream-certificate): Only return key and certificate
>     files that exist.

From userʼs point of view it means: M-x eww RET https://gnu.org or M-x list-packages or something else equally anonymous by nature may eventually request a passphrase to decrypt private GPG key (that one, which was used to encrypt ~/.authinfo.gpg), and fail if request is rejected.

Iʼm afraid, this deeply violates the principle of the least astonishment.

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

Re: master 91c732f: Always check for client-certificates

[Robert Pluim]

>>>>> On Sat, 16 Nov 2019 10:07:52 +0300, Dmitry Alexandrov <321942@gmail.com> said:

    Dmitry> From userʼs point of view it means: M-x eww RET https://gnu.org or M-x
    Dmitry> list-packages or something else equally anonymous by nature may
    Dmitry> eventually request a passphrase to decrypt private GPG key (that one,
    Dmitry> which was used to encrypt ~/.authinfo.gpg), and fail if request is
    Dmitry> rejected.

If the user already trusts Emacs with access to .authinfo.gpg, then
nothing new is happening. If they donʼt then they should customize
'auth-sources'.

If they specifically want to prevent Emacs performing decryption for
network access, then they can customize
'network-stream-use-client-certificates'. We can of course discuss the
default value, but nobody objected during the development of the
feature.

The failure when access is refused is a bug. How does the following
work for you:

commit 41e31c45519b0df6846e73557fba718f1ee29394
Author:     Robert Pluim <rpluim@gmail.com>
AuthorDate: Sun Nov 17 21:21:48 2019 +0100
Commit:     Robert Pluim <rpluim@gmail.com>
CommitDate: Sun Nov 17 21:25:22 2019 +0100

    Handle auth-source-search failures in open-network-stream
    
    If the user cancels the gpg decryption pop-up, auth-source-search
    fails *and* epa pops up an error buffer.  Fix epa to allow suppressing
    that, and ignore errors returned from auth-source-search.
    
    * lisp/epa.el (epa-suppress-error-buffer): New defvar.  Bind non-nil
    to stop epa popping up an error buffer.
    
    * lisp/net/network-stream.el: require epa when byte-compiling.
    (network-stream-certificate): ignore errors when calling
    auth-source-search, and suppress the epa error buffer.

diff --git a/lisp/epa.el b/lisp/epa.el
index a2be9a3dbd..13708d046d 100644
--- a/lisp/epa.el
+++ b/lisp/epa.el
@@ -179,6 +179,7 @@ epa-key
 (defvar epa-list-keys-arguments nil)
 (defvar epa-info-buffer nil)
 (defvar epa-error-buffer nil)
+(defvar epa-suppress-error-buffer nil)
 (defvar epa-last-coding-system-specified nil)
 
 (defvar epa-key-list-mode-map
@@ -578,7 +579,8 @@ epa-display-info
     (message "%s" info)))
 
 (defun epa-display-error (context)
-  (unless (equal (epg-context-error-output context) "")
+  (unless (or (equal (epg-context-error-output context) "")
+              epa-suppress-error-buffer)
     (let ((buffer (get-buffer-create "*Error*")))
       (save-selected-window
 	(unless (and epa-error-buffer (buffer-live-p epa-error-buffer))
diff --git a/lisp/net/network-stream.el b/lisp/net/network-stream.el
index 1571c76189..4c6056e0c8 100644
--- a/lisp/net/network-stream.el
+++ b/lisp/net/network-stream.el
@@ -46,6 +46,9 @@
 (require 'nsm)
 (require 'puny)
 
+(eval-when-compile
+  (require 'epa)) ; for epa-suppress-error-buffer
+
 (declare-function starttls-available-p "starttls" ())
 (declare-function starttls-negotiate "starttls" (process))
 (declare-function starttls-open-stream "starttls" (name buffer host port))
@@ -225,10 +228,12 @@ network-stream-certificate
       ;; Either nil or a list with a key/certificate pair.
       spec)
      ((eq spec t)
-      (let* ((auth-info
-	      (car (auth-source-search :max 1
-				       :host host
-				       :port service)))
+      (let* ((epa-suppress-error-buffer t)
+             (auth-info
+              (ignore-errors
+                (car (auth-source-search :max 1
+                                         :host host
+                                         :port service))))
 	     (key (plist-get auth-info :key))
 	     (cert (plist-get auth-info :cert)))
 	(and key cert (file-readable-p key) (file-readable-p cert)

Re: master 91c732f: Always check for client-certificates

[Lars Ingebrigtsen]

Robert Pluim <rpluim@gmail.com> writes:

> If they specifically want to prevent Emacs performing decryption for
> network access, then they can customize
> 'network-stream-use-client-certificates'. We can of course discuss the
> default value, but nobody objected during the development of the
> feature.

I didn't realise that this would mean accessing the .authinfo.gpg file
by default for https connections.  I don't think that's a good idea, so
network-stream-use-client-certificates has to default to nil.

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

Re: master 91c732f: Always check for client-certificates

[Robert Pluim]

>>>>> On Mon, 18 Nov 2019 09:32:34 +0100, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> Robert Pluim <rpluim@gmail.com> writes:
    >> If they specifically want to prevent Emacs performing decryption for
    >> network access, then they can customize
    >> 'network-stream-use-client-certificates'. We can of course discuss the
    >> default value, but nobody objected during the development of the
    >> feature.

    Lars> I didn't realise that this would mean accessing the .authinfo.gpg file
    Lars> by default for https connections.  I don't think that's a good idea, so
    Lars> network-stream-use-client-certificates has to default to nil.

I can flip the default if thatʼs the consensus.

Robert

Re: master 91c732f: Always check for client-certificates

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Date: Mon, 18 Nov 2019 10:06:19 +0100
> Cc: Dmitry Alexandrov <321942@gmail.com>, emacs-devel@gnu.org
> 
>     Lars> I didn't realise that this would mean accessing the .authinfo.gpg file
>     Lars> by default for https connections.  I don't think that's a good idea, so
>     Lars> network-stream-use-client-certificates has to default to nil.
> 
> I can flip the default if thatʼs the consensus.

If everyone agrees with Lars, then we have a consensus.  But if you
disagree, I'd like to hear your arguments (and anyone else's really),
before we decide what is the consensus.

Thanks.

Re: master 91c732f: Always check for client-certificates

[Robert Pluim]

>>>>> On Mon, 18 Nov 2019 17:38:42 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Mon, 18 Nov 2019 10:06:19 +0100
    >> Cc: Dmitry Alexandrov <321942@gmail.com>, emacs-devel@gnu.org
    >> 
    Lars> I didn't realise that this would mean accessing the .authinfo.gpg file
    Lars> by default for https connections.  I don't think that's a good idea, so
    Lars> network-stream-use-client-certificates has to default to nil.
    >> 
    >> I can flip the default if thatʼs the consensus.

    Eli> If everyone agrees with Lars, then we have a consensus.  But if you
    Eli> disagree, I'd like to hear your arguments (and anyone else's really),
    Eli> before we decide what is the consensus.

I'm doubly biased: I implemented it, and I read email in Emacs, so
.authinfo.gpg gets decrypted for me anyway, so having it done for eww
or package-list-packages is a no-op, which means I disagree, but not
strongly.

The reason for the feature is to make it easy to use certificates:
just add the right stuff to .authinfo.gpg, and everything else happens
by itself, much like usernames/passwords when sending
email.

Defaulting it to off means more configuration burden on the user.
Defaulting it to on means that some people who object to it need to
customize auth-sources and/or network-stream-use-client-certificates.

I canʼt judge the relative sizes of those two groups, although the
second one is highly likely to be more vocal.

Having said that, I donʼt think weʼre looking for unanimity anyway,
just rough consensus, and so far Iʼm outnumbered at least 2-1.

Robert

Re: master 91c732f: Always check for client-certificates

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: 321942@gmail.com,  larsi@gnus.org,  emacs-devel@gnu.org
> Date: Mon, 18 Nov 2019 17:05:09 +0100
> 
> The reason for the feature is to make it easy to use certificates:
> just add the right stuff to .authinfo.gpg, and everything else happens
> by itself, much like usernames/passwords when sending
> email.
> 
> Defaulting it to off means more configuration burden on the user.
> Defaulting it to on means that some people who object to it need to
> customize auth-sources and/or network-stream-use-client-certificates.
> 
> I canʼt judge the relative sizes of those two groups, although the
> second one is highly likely to be more vocal.
> 
> Having said that, I donʼt think weʼre looking for unanimity anyway,
> just rough consensus, and so far Iʼm outnumbered at least 2-1.

OK, thanks.  Let's wait for a few days to give others a chance to
chime in.  If no other opinions are voiced, I'm okay to go with the
"majority".

Re: master 91c732f: Always check for client-certificates

[Michael Welsh Duggan]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Mon, 18 Nov 2019 17:38:42 +0200, Eli Zaretskii <eliz@gnu.org> said:
>
>     >> From: Robert Pluim <rpluim@gmail.com>
>     >> Date: Mon, 18 Nov 2019 10:06:19 +0100
>     >> Cc: Dmitry Alexandrov <321942@gmail.com>, emacs-devel@gnu.org
>     >> 
>     Lars> I didn't realise that this would mean accessing the .authinfo.gpg file
>     Lars> by default for https connections.  I don't think that's a
>     Lars> good idea, so
>     Lars> network-stream-use-client-certificates has to default to nil.
>     >> 
>     >> I can flip the default if thatʼs the consensus.
>
>     Eli> If everyone agrees with Lars, then we have a consensus.  But if you
>     Eli> disagree, I'd like to hear your arguments (and anyone else's really),
>     Eli> before we decide what is the consensus.
>
> I'm doubly biased: I implemented it, and I read email in Emacs, so
> .authinfo.gpg gets decrypted for me anyway, so having it done for eww
> or package-list-packages is a no-op, which means I disagree, but not
> strongly.
>
> The reason for the feature is to make it easy to use certificates:
> just add the right stuff to .authinfo.gpg, and everything else happens
> by itself, much like usernames/passwords when sending
> email.
>
> Defaulting it to off means more configuration burden on the user.
> Defaulting it to on means that some people who object to it need to
> customize auth-sources and/or network-stream-use-client-certificates.

Would it be difficult (or a bad idea) to make it such that the first
time someone uses a package that might want to use .authinfo.gpg for
private information, a separate prompt comes up asking whether people
want to load their .authinfo.gpg this time, not this time, every time
(and don't ask again), or never (and don't ask again)?  This one prompt
can be verbose, popping up a window with an explanation, with the
understanding that the user can make an informed choice and not have to
do this again.  This may be clunky, but this is the simplest way I can
think of to "have your cake and eat it too."

This seems similar to the "how do I set up email to work the first time
when I send an Emacs bug report" problem.  It also is similar to the
sort of thing that is done when someone visits a site with self-signed
certificates and suchlike.

-- 
Michael Welsh Duggan
(md5i@md5i.com)

Re: master 91c732f: Always check for client-certificates

[Lars Ingebrigtsen]

Michael Welsh Duggan <mwd@md5i.com> writes:

> Would it be difficult (or a bad idea) to make it such that the first
> time someone uses a package that might want to use .authinfo.gpg for
> private information, a separate prompt comes up asking whether people
> want to load their .authinfo.gpg this time, not this time, every time
> (and don't ask again), or never (and don't ask again)?

Yes, it's a usability stumble.  Why would opening https://fsf.org/ ask
to open your password store?  It just sounds suspicious.

Very few people use client certificates, and it's not too much to ask
those people that do to set a variable.

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