bug#33780: network-stream.el: network-stream-certificate always returns nil

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Vinothan Shankar]

network-stream-certificate will *always* return nil, regardless of
whether there actually is a client-certificate value-pair specified,
because (plist-get :client-certificate parameters) is always nil.  This
is because plist-get takes the plist as the first argument, and the key
as the second; trying to find a list in a token is always going to be
nil.

This makes it impossible to use client certificates with Emacs's built-
in network-stream support, at least without overriding functions.

The error is in net/network-stream.el.  It has been there since the
function was first written in 2011, according to git blame.

I surmise that this, in combination with there being no support for
client certificates in network-stream-tls (though it's available in
network-stream-starttls) is part of the reason there are so many
conflicting guides on, for example, using client-certificate SASL with 
ERC.

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

Vinothan Shankar <darael@dracon.is> writes:

> network-stream-certificate will *always* return nil, regardless of
> whether there actually is a client-certificate value-pair specified,
> because (plist-get :client-certificate parameters) is always nil.  This
> is because plist-get takes the plist as the first argument, and the key
> as the second; trying to find a list in a token is always going to be
> nil.
>
> This makes it impossible to use client certificates with Emacs's built-
> in network-stream support, at least without overriding functions.
>
> The error is in net/network-stream.el.  It has been there since the
> function was first written in 2011, according to git blame.

Yes. Lars?

> I surmise that this, in combination with there being no support for
> client certificates in network-stream-tls (though it's available in
> network-stream-starttls) is part of the reason there are so many
> conflicting guides on, for example, using client-certificate SASL with 
> ERC.

Could you apply the following patch, and test something like

(open-network-stream
 "*tls*" (current-buffer) "server.example.com"
 "443"
 :type 'tls
 :warn-unless-encrypted t
 :return-list t
 :client-certificate t)

with the appropriate entries in your .authinfo (replace the servername
and port number as needed)? It works in my limited testing, and
doesnʼt appear to have broken Gnus (but none of my TLS connections
require client certificates).

It could be argued that this should all be transparent, i.e. we should
assume ":client-certificate t" unless itʼs explicitly nil, which would
avoid having to fix all the packages that just call
`open-network-stream', but that we can revisit once things actually
work.

diff --git i/lisp/gnus/nnimap.el w/lisp/gnus/nnimap.el
index 1a3b05ddb3..956c7144cb 100644
--- i/lisp/gnus/nnimap.el
+++ w/lisp/gnus/nnimap.el
@@ -456,6 +456,7 @@ nnimap-open-connection-1
                :always-query-capabilities t
 	       :end-of-command "\r\n"
 	       :success " OK "
+               :client-certificate t
 	       :starttls-function
 	       (lambda (capabilities)
 		 (when (string-match-p "STARTTLS" capabilities)
diff --git i/lisp/gnus/nntp.el w/lisp/gnus/nntp.el
index be9e495510..efb4912a8f 100644
--- i/lisp/gnus/nntp.el
+++ w/lisp/gnus/nntp.el
@@ -1266,6 +1266,7 @@ nntp-open-connection
 		     :end-of-command "^\\([2345]\\|[.]\\).*\n"
 		     :capability-command "HELP\r\n"
 		     :success "^3"
+                     :client-certificate t
 		     :starttls-function
 		     (lambda (capabilities)
 		       (if (not (string-match "STARTTLS" capabilities))
diff --git i/lisp/net/gnutls.el w/lisp/net/gnutls.el
index 315932b7e6..625f11caa5 100644
--- i/lisp/net/gnutls.el
+++ w/lisp/net/gnutls.el
@@ -38,6 +38,9 @@
 (require 'cl-lib)
 (require 'puny)
 
+(declare-function network-stream-certificate "network-stream"
+                  (host service parameters))
+
 (defgroup gnutls nil
   "Emacs interface to the GnuTLS library."
   :version "24.1"
@@ -138,7 +141,7 @@ gnutls-min-prime-bits
                  (integer :tag "Number of bits" 512))
   :group 'gnutls)
 
-(defun open-gnutls-stream (name buffer host service &optional nowait)
+(defun open-gnutls-stream (name buffer host service &optional parameters)
   "Open a SSL/TLS connection for a service to a host.
 Returns a subprocess-object to represent the connection.
 Input and output work as for subprocesses; `delete-process' closes it.
@@ -152,9 +155,14 @@ open-gnutls-stream
 Third arg is name of the host to connect to, or its IP address.
 Fourth arg SERVICE is name of the service desired, or an integer
 specifying a port number to connect to.
-Fifth arg NOWAIT (which is optional) means that the socket should
-be opened asynchronously.  The connection process will be
-returned to the caller before TLS negotiation has happened.
+Fifth arg PARAMETERS is a property list.  It is currently checked for:
+
+    :nowait which means that the socket should be opened
+    asynchronously.  The connection process will be returned to
+    the caller before TLS negotiation has happened.
+
+    :client-certificate which allows the specification of
+    client certificates and keys to use to set up the connection.
 
 Usage example:
 
@@ -168,19 +176,22 @@ open-gnutls-stream
 documentation for the specific parameters you can use to open a
 GnuTLS connection, including specifying the credential type,
 trust and key files, and priority string."
-  (let ((process (open-network-stream
-                  name buffer host service
-                  :nowait nowait
-                  :tls-parameters
-                  (and nowait
-                       (cons 'gnutls-x509pki
-                             (gnutls-boot-parameters
-                              :type 'gnutls-x509pki
-                              :hostname (puny-encode-domain host)))))))
+  (let* ((cert (network-stream-certificate host service parameters))
+         (nowait (plist-get parameters :nowait))
+         (process (open-network-stream
+                   name buffer host service
+                   :nowait nowait
+                   :tls-parameters
+                   (and nowait
+                        (cons 'gnutls-x509pki
+                              (gnutls-boot-parameters
+                               :type 'gnutls-x509pki
+                               :hostname (puny-encode-domain host)))))))
     (if nowait
         process
       (gnutls-negotiate :process process
                         :type 'gnutls-x509pki
+                        :keylist (and cert (list cert))
                         :hostname (puny-encode-domain host)))))
 
 (define-error 'gnutls-error "GnuTLS error")
diff --git i/lisp/net/network-stream.el w/lisp/net/network-stream.el
index a0589e25a4..8b813eef2c 100644
--- i/lisp/net/network-stream.el
+++ w/lisp/net/network-stream.el
@@ -196,7 +196,7 @@ open-network-stream
 	  (car result))))))
 
 (defun network-stream-certificate (host service parameters)
-  (let ((spec (plist-get :client-certificate parameters)))
+  (let ((spec (plist-get parameters :client-certificate)))
     (cond
      ((listp spec)
       ;; Either nil or a list with a key/certificate pair.
@@ -389,7 +389,7 @@ network-stream-open-tls
 	   (stream
             (if (gnutls-available-p)
                 (open-gnutls-stream name buffer host service
-                                    (plist-get parameters :nowait))
+                                    parameters)
               (require 'tls)
               (open-tls-stream name buffer host service)))
 	   (eoc (plist-get parameters :end-of-command)))

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Vinothan Shankar]

On Wed, 2018-12-19 at 18:19 +0100, Robert Pluim wrote:
> Could you apply the following patch, and test something like
> 
> (open-network-stream
>  "*tls*" (current-buffer) "server.example.com"
>  "443"
>  :type 'tls
>  :warn-unless-encrypted t
>  :return-list t
>  :client-certificate t)
> 
> with the appropriate entries in your .authinfo (replace the
> servername
> and port number as needed)? It works in my limited testing, and
> doesnʼt appear to have broken Gnus (but none of my TLS connections
> require client certificates).

OK, so a few minutes into the process of trying to do this, I came
across a snag: the syntax for using certificates in authinfo files
doesn't appear to be documented anywhere; I had to extract it from a
stackexchange question.  Docs bug, or lack of search-fu?  Moving on...

Results:

Initial failure, but this is because I've been testing with ERC, which
calls open-network-stream with ":nowait t".  If I add the ":keylist
(and cert (list cert))" stanza to the other branch of open-gnutls-
stream as well, in the gnutls-boot-parameters call, it works perfectly:
Freenode picks up my identity even when I supply a blank password.

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

Vinothan Shankar <darael@dracon.is> writes:

> OK, so a few minutes into the process of trying to do this, I came
> across a snag: the syntax for using certificates in authinfo files
> doesn't appear to be documented anywhere; I had to extract it from a
> stackexchange question.  Docs bug, or lack of search-fu?  Moving on...

Itʼs in the smptmail info manual, node 'Encryption'. It is linked from
the main Emacs manual, from the 'Mail Sending' node, but there appears
to be no description of the syntax in the auth-source manual. Patches
welcome :-)

> Results:
>
> Initial failure, but this is because I've been testing with ERC, which
> calls open-network-stream with ":nowait t".  If I add the ":keylist
> (and cert (list cert))" stanza to the other branch of open-gnutls-
> stream as well, in the gnutls-boot-parameters call, it works perfectly:
> Freenode picks up my identity even when I supply a blank password.

Thanks for testing. Iʼll update my patch (and write a ChangeLog, and a
NEWS entry)

By my count there are at least 11 calls to open-network-stream in
Emacs' sources which would need updating with ':client-certificate t'
in order to trigger transparent use of user-specified certificates.

By analogy to e.g. smtpmail looking up usernames and passwords by
default using auth-source, I think Emacs should do the same for
client-certificates by default. People without entries specifying
certificates would be unaffected, and third-party packages would not
need to be updated to take advantage of this new feature. Comments
welcome.

Robert

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

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

Robert Pluim <rpluim@gmail.com> writes:

> Vinothan Shankar <darael@dracon.is> writes:
>
>> OK, so a few minutes into the process of trying to do this, I came
>> across a snag: the syntax for using certificates in authinfo files
>> doesn't appear to be documented anywhere; I had to extract it from a
>> stackexchange question.  Docs bug, or lack of search-fu?  Moving on...
>
> Itʼs in the smptmail info manual, node 'Encryption'. It is linked from
> the main Emacs manual, from the 'Mail Sending' node, but there appears
> to be no description of the syntax in the auth-source manual. Patches
> welcome :-)
>

I was looking there anyway, so I updated the manual.

Proposed patch attached. At this time it just enables taking into
account ':client-certificate t' in calls to 'open-network-stream' and
applying any client certificates found, it doesnʼt change the default
behaviour. Iʼll follow up on emacs-devel afterwards about that.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Check-for-client-certificates-when-using-GnuTLS.patch --]
[-- Type: text/x-patch, Size: 8192 bytes --]

From 2f13e12882a32246d9b1d57e111ad17e0773ff54 Mon Sep 17 00:00:00 2001
From: Robert Pluim <rpluim@gmail.com>
Date: Fri, 21 Dec 2018 11:58:00 +0100
Subject: [PATCH] Check for client certificates when using GnuTLS
To: emacs-devel@gnu.org

This fixes Bug#33780, and extends the documentation to describe how to
enable use of client certificates.

* lisp/net/network-stream.el (network-stream-certificate): Correct
order of parameters to plist-get.
(network-stream-open-tls): Pass all received parameters to
open-gnutls-stream, not just :nowait.

* lisp/net/gnutls.el (open-gnutls-stream): Add optional plist to
arglist.  Derive client certificate(s) and keys(s) from plist (maybe
via auth-source) and pass to gnutls-boot-parameters and
gnutls-negotiate.
(network-stream-certificate): Add declare-function form for it.

* doc/misc/auth.texi (Help for users): Describe format to use for
client key/cert specification.

* doc/misc/emacs-gnutls.texi (Help For Developers): Describe usage
of new optional plist argument.  Add crossref to description of
.authinfo format for client key/cert specification.

* etc/NEWS: Describe new client certificate functionality for
  'open-network-stream'
---
 doc/misc/auth.texi         |  9 +++++++++
 doc/misc/emacs-gnutls.texi | 12 +++++++++++-
 etc/NEWS                   |  7 +++++++
 lisp/net/gnutls.el         | 31 +++++++++++++++++++++----------
 lisp/net/network-stream.el |  5 +++--
 5 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/doc/misc/auth.texi b/doc/misc/auth.texi
index fcbc83ead5..68b8553d58 100644
--- a/doc/misc/auth.texi
+++ b/doc/misc/auth.texi
@@ -109,6 +109,15 @@ Help for users
 @code{auth-source-search} queries.  You can also use @code{login} and
 @code{account}.
 
+You can also use this file to specify client certificates to use when
+setting up TLS connections.  The format is:
+@example
+machine @var{mymachine} port @var{myport} key "@var{key}" cert "@var{cert}"
+@end example
+
+@var{key} and @var{cert} are filenames containing the key and
+certificate to use respectively.
+
 You can use spaces inside a password or other token by surrounding the
 token with either single or double quotes.
 
diff --git a/doc/misc/emacs-gnutls.texi b/doc/misc/emacs-gnutls.texi
index a690ccfcce..90c2d217e2 100644
--- a/doc/misc/emacs-gnutls.texi
+++ b/doc/misc/emacs-gnutls.texi
@@ -179,7 +179,7 @@ Help For Developers
 You should not have to use the @file{gnutls.el} functions directly.
 But you can test them with @code{open-gnutls-stream}.
 
-@defun open-gnutls-stream name buffer host service &optional nowait
+@defun open-gnutls-stream name buffer host service &optional nowait parameters
 This function creates a buffer connected to a specific @var{host} and
 @var{service} (port number or service name).  The parameters and their
 syntax are the same as those given to @code{open-network-stream}
@@ -191,6 +191,16 @@ Help For Developers
 asynchronous, and the connection process will be returned to the
 caller before TLS negotiation has happened.
 
+@var{parameters} is a plist which is currently checked only for
+@code{:client-certificate}.  Any resulting client certificates are
+passed down to the lower TLS layers.  Set @code{:client certificate t}
+to trigger looking up of the certificates using the auth-source
+library.  The format used by @file{.authinfo} to specify the
+per-server keys is described in @xref{Help for users,,auth-source,
+auth, Emacs auth-source Library}.
+
+Example calls:
+
 @lisp
 ;; open a HTTPS connection
 (open-gnutls-stream "tls" "tls-buffer" "yourserver.com" "https")
diff --git a/etc/NEWS b/etc/NEWS
index 0624c5690b..74943fb2ff 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -199,6 +199,13 @@ issued), you can either set 'network-security-protocol-checks' to nil,
 or adjust the elements in that variable to only happen on the 'high'
 security level (assuming you use the 'medium' level).
 
++++
+** Native GnuTLS connections can now use client certificates.
+Previously, this support was only available when using the external
+gnutls-cli command.  Call 'open-network-stream' with
+':client-certificate t' to trigger looking up of per-server
+certificates via 'auth-source'.
+
 +++
 ** New function 'fill-polish-nobreak-p', to be used in 'fill-nobreak-predicate'.
 It blocks line breaking after a one-letter word, also in the case when
diff --git a/lisp/net/gnutls.el b/lisp/net/gnutls.el
index 315932b7e6..30f933fa48 100644
--- a/lisp/net/gnutls.el
+++ b/lisp/net/gnutls.el
@@ -38,6 +38,9 @@
 (require 'cl-lib)
 (require 'puny)
 
+(declare-function network-stream-certificate "network-stream"
+                  (host service parameters))
+
 (defgroup gnutls nil
   "Emacs interface to the GnuTLS library."
   :version "24.1"
@@ -138,7 +141,7 @@ gnutls-min-prime-bits
                  (integer :tag "Number of bits" 512))
   :group 'gnutls)
 
-(defun open-gnutls-stream (name buffer host service &optional nowait)
+(defun open-gnutls-stream (name buffer host service &optional nowait parameters)
   "Open a SSL/TLS connection for a service to a host.
 Returns a subprocess-object to represent the connection.
 Input and output work as for subprocesses; `delete-process' closes it.
@@ -155,6 +158,10 @@ open-gnutls-stream
 Fifth arg NOWAIT (which is optional) means that the socket should
 be opened asynchronously.  The connection process will be
 returned to the caller before TLS negotiation has happened.
+Sixth arg PARAMETERS is an optional property list.  It is currently
+checked for :client-certificate only. This allows specifying the
+client certificates and keys used to set up the connection.
+See `open-network-stream' for a complete description.
 
 Usage example:
 
@@ -168,19 +175,23 @@ open-gnutls-stream
 documentation for the specific parameters you can use to open a
 GnuTLS connection, including specifying the credential type,
 trust and key files, and priority string."
-  (let ((process (open-network-stream
-                  name buffer host service
-                  :nowait nowait
-                  :tls-parameters
-                  (and nowait
-                       (cons 'gnutls-x509pki
-                             (gnutls-boot-parameters
-                              :type 'gnutls-x509pki
-                              :hostname (puny-encode-domain host)))))))
+  (let* ((cert (network-stream-certificate host service parameters))
+         (keylist (and cert (list cert)))
+         (process (open-network-stream
+                   name buffer host service
+                   :nowait nowait
+                   :tls-parameters
+                   (and nowait
+                        (cons 'gnutls-x509pki
+                              (gnutls-boot-parameters
+                               :type 'gnutls-x509pki
+                               :keylist keylist
+                               :hostname (puny-encode-domain host)))))))
     (if nowait
         process
       (gnutls-negotiate :process process
                         :type 'gnutls-x509pki
+                        :keylist keylist
                         :hostname (puny-encode-domain host)))))
 
 (define-error 'gnutls-error "GnuTLS error")
diff --git a/lisp/net/network-stream.el b/lisp/net/network-stream.el
index a0589e25a4..26f92d5aa8 100644
--- a/lisp/net/network-stream.el
+++ b/lisp/net/network-stream.el
@@ -196,7 +196,7 @@ open-network-stream
 	  (car result))))))
 
 (defun network-stream-certificate (host service parameters)
-  (let ((spec (plist-get :client-certificate parameters)))
+  (let ((spec (plist-get parameters :client-certificate)))
     (cond
      ((listp spec)
       ;; Either nil or a list with a key/certificate pair.
@@ -389,7 +389,8 @@ network-stream-open-tls
 	   (stream
             (if (gnutls-available-p)
                 (open-gnutls-stream name buffer host service
-                                    (plist-get parameters :nowait))
+                                    (plist-get parameters :nowait)
+                                    parameters)
               (require 'tls)
               (open-tls-stream name buffer host service)))
 	   (eoc (plist-get parameters :end-of-command)))
-- 
2.19.1.816.gcd69ec8cde.dirty

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

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

Following discussion on emacs-devel on how to do this, latest version
of patch attached. This maintains backwards compatibility for
open-gnutls-stream (assuming I haven't screwed up the checks), and
updates the relevant documentation.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Check-for-client-certificates-when-using-GnuTLS.patch --]
[-- Type: text/x-patch, Size: 10477 bytes --]

From 6bdf3d94dc83e79394d109486f68810ef9f4b373 Mon Sep 17 00:00:00 2001
From: Robert Pluim <rpluim@gmail.com>
Date: Fri, 21 Dec 2018 11:58:00 +0100
Subject: [PATCH] Check for client certificates when using GnuTLS
To: emacs-devel@gnu.org

This fixes Bug#33780, and extends the documentation to describe how to
enable use of client certificates.

* lisp/net/network-stream.el (network-stream-certificate): Correct
order of parameters to plist-get.
(network-stream-open-tls): Pass all received parameters to
open-gnutls-stream as plist, not just :nowait.

* lisp/net/gnutls.el (open-gnutls-stream): Change optional nowait arg
to be plist.  Derive nowait and client certificate(s) and keys(s) from
plist (maybe via auth-source) and pass to gnutls-boot-parameters and
gnutls-negotiate.
(network-stream-certificate): Add declare-function form for it.

* doc/misc/auth.texi (Help for users): Describe format to use for
client key/cert specification.

* doc/misc/emacs-gnutls.texi (Help For Developers): Describe usage of
optional plist argument.  Add crossreference to description of
.authinfo format for client key/cert specification.

* etc/NEWS: Describe new client certificate functionality for
  'open-network-stream'.
---
 doc/misc/auth.texi         |  9 ++++++
 doc/misc/emacs-gnutls.texi | 38 ++++++++++++++++++-------
 etc/NEWS                   |  7 +++++
 lisp/net/gnutls.el         | 57 +++++++++++++++++++++++++-------------
 lisp/net/network-stream.el |  4 +--
 5 files changed, 84 insertions(+), 31 deletions(-)

diff --git a/doc/misc/auth.texi b/doc/misc/auth.texi
index 495d9f53e1..ddfeabcba7 100644
--- a/doc/misc/auth.texi
+++ b/doc/misc/auth.texi
@@ -109,6 +109,15 @@ Help for users
 @code{auth-source-search} queries.  You can also use @code{login} and
 @code{account}.
 
+You can also use this file to specify client certificates to use when
+setting up TLS connections.  The format is:
+@example
+machine @var{mymachine} port @var{myport} key @var{key} cert @var{cert}
+@end example
+
+@var{key} and @var{cert} are filenames containing the key and
+certificate to use respectively.
+
 You can use spaces inside a password or other token by surrounding the
 token with either single or double quotes.
 
diff --git a/doc/misc/emacs-gnutls.texi b/doc/misc/emacs-gnutls.texi
index aae583c641..0e2a9764a1 100644
--- a/doc/misc/emacs-gnutls.texi
+++ b/doc/misc/emacs-gnutls.texi
@@ -179,17 +179,35 @@ Help For Developers
 You should not have to use the @file{gnutls.el} functions directly.
 But you can test them with @code{open-gnutls-stream}.
 
-@defun open-gnutls-stream name buffer host service &optional nowait
+@defun open-gnutls-stream name buffer host service &optional parameters
 This function creates a buffer connected to a specific @var{host} and
-@var{service} (port number or service name).  The parameters and their
-syntax are the same as those given to @code{open-network-stream}
-(@pxref{Network,, Network Connections, elisp, The Emacs Lisp Reference
-Manual}).  The connection process is called @var{name} (made unique if
-necessary).  This function returns the connection process.
-
-The @var{nowait} parameter means that the socket should be
-asynchronous, and the connection process will be returned to the
-caller before TLS negotiation has happened.
+@var{service} (port number or service name).  The mandatory arguments
+and their syntax are the same as those given to
+@code{open-network-stream} (@pxref{Network,, Network Connections,
+elisp, The Emacs Lisp Reference Manual}).  The connection process is
+called @var{name} (made unique if necessary).  This function returns
+the connection process.
+
+The optional @var{parameters} argument is a list of keywords and
+values.  The only keywords which currently have any effect are
+@code{:client-certificate} and @code{:nowait}.
+
+Passing @code{:client certificate t} triggers looking up of client
+certificates matching @var{host} and @var{service} using the
+'auth-source' library.  Any resulting client certificates are passed
+down to the lower TLS layers.  The format used by @file{.authinfo} to
+specify the per-server keys is described in @xref{Help for
+users,,auth-source, auth, Emacs auth-source Library}.
+
+Passing @code{:nowait t} means that the socket should be asynchronous,
+and the connection process will be returned to the caller before TLS
+negotiation has happened.
+
+For historical reasons @var{parameters} can also be a symbol, which is
+interpreted the same as passing a list containing @code{:nowait} and
+the value of that symbol.
+
+Example calls:
 
 @lisp
 ;; open a HTTPS connection
diff --git a/etc/NEWS b/etc/NEWS
index 3670ab5bf4..43997f8418 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -199,6 +199,13 @@ issued), you can either set 'network-security-protocol-checks' to nil,
 or adjust the elements in that variable to only happen on the 'high'
 security level (assuming you use the 'medium' level).
 
++++
+** Native GnuTLS connections can now use client certificates.
+Previously, this support was only available when using the external
+gnutls-cli command.  Call 'open-network-stream' with
+':client-certificate t' to trigger looking up of per-server
+certificates via 'auth-source'.
+
 +++
 ** New function 'fill-polish-nobreak-p', to be used in 'fill-nobreak-predicate'.
 It blocks line breaking after a one-letter word, also in the case when
diff --git a/lisp/net/gnutls.el b/lisp/net/gnutls.el
index 78ac3fe35b..dae208e926 100644
--- a/lisp/net/gnutls.el
+++ b/lisp/net/gnutls.el
@@ -38,6 +38,9 @@
 (require 'cl-lib)
 (require 'puny)
 
+(declare-function network-stream-certificate "network-stream"
+                  (host service parameters))
+
 (defgroup gnutls nil
   "Emacs interface to the GnuTLS library."
   :version "24.1"
@@ -138,23 +141,25 @@ gnutls-min-prime-bits
                  (integer :tag "Number of bits" 512))
   :group 'gnutls)
 
-(defun open-gnutls-stream (name buffer host service &optional nowait)
+(defun open-gnutls-stream (name buffer host service &optional parameters)
   "Open a SSL/TLS connection for a service to a host.
 Returns a subprocess-object to represent the connection.
 Input and output work as for subprocesses; `delete-process' closes it.
 Args are NAME BUFFER HOST SERVICE.
 NAME is name for process.  It is modified if necessary to make it unique.
 BUFFER is the buffer (or `buffer-name') to associate with the process.
- Process output goes at end of that buffer, unless you specify
- a filter function to handle the output.
- BUFFER may be also nil, meaning that this process is not associated
- with any buffer
-Third arg is name of the host to connect to, or its IP address.
-Fourth arg SERVICE is name of the service desired, or an integer
+Process output goes at end of that buffer, unless you specify a
+filter function to handle the output.  BUFFER may be also nil,
+meaning that this process is not associated with any buffer
+Third arg HOST is the name of the host to connect to, or its IP address.
+Fourth arg SERVICE is the name of the service desired, or an integer
 specifying a port number to connect to.
-Fifth arg NOWAIT (which is optional) means that the socket should
-be opened asynchronously.  The connection process will be
-returned to the caller before TLS negotiation has happened.
+Fifth arg PARAMETERS is an optional list of keyword/value pairs.
+Only :client-certificate and :nowait keywords are recognized, and
+have the same meaning as for `open-network-stream'.
+For historical reasons PARAMETERS can also be a symbol, which is
+interpreted the same as passing a list containing :nowait and the
+value of that symbol.
 
 Usage example:
 
@@ -168,19 +173,33 @@ open-gnutls-stream
 documentation for the specific parameters you can use to open a
 GnuTLS connection, including specifying the credential type,
 trust and key files, and priority string."
-  (let ((process (open-network-stream
-                  name buffer host service
-                  :nowait nowait
-                  :tls-parameters
-                  (and nowait
-                       (cons 'gnutls-x509pki
-                             (gnutls-boot-parameters
-                              :type 'gnutls-x509pki
-                              :hostname (puny-encode-domain host)))))))
+  (let* ((parameters
+          (cond ((symbolp parameters)
+                 (list :nowait parameters))
+                ((not (cl-evenp (length parameters)))
+                 (error "Malformed keyword list"))
+                ((consp parameters)
+                 parameters)
+                (t
+                 (error "Unknown parameter type"))))
+         (cert (network-stream-certificate host service parameters))
+         (keylist (and cert (list cert)))
+         (nowait (plist-get parameters :nowait))
+         (process (open-network-stream
+                   name buffer host service
+                   :nowait nowait
+                   :tls-parameters
+                   (and nowait
+                        (cons 'gnutls-x509pki
+                              (gnutls-boot-parameters
+                               :type 'gnutls-x509pki
+                               :keylist keylist
+                               :hostname (puny-encode-domain host)))))))
     (if nowait
         process
       (gnutls-negotiate :process process
                         :type 'gnutls-x509pki
+                        :keylist keylist
                         :hostname (puny-encode-domain host)))))
 
 (define-error 'gnutls-error "GnuTLS error")
diff --git a/lisp/net/network-stream.el b/lisp/net/network-stream.el
index 98b2033790..1723931c67 100644
--- a/lisp/net/network-stream.el
+++ b/lisp/net/network-stream.el
@@ -196,7 +196,7 @@ open-network-stream
 	  (car result))))))
 
 (defun network-stream-certificate (host service parameters)
-  (let ((spec (plist-get :client-certificate parameters)))
+  (let ((spec (plist-get parameters :client-certificate)))
     (cond
      ((listp spec)
       ;; Either nil or a list with a key/certificate pair.
@@ -389,7 +389,7 @@ network-stream-open-tls
 	   (stream
             (if (gnutls-available-p)
                 (open-gnutls-stream name buffer host service
-                                    (plist-get parameters :nowait))
+                                    parameters)
               (require 'tls)
               (open-tls-stream name buffer host service)))
 	   (eoc (plist-get parameters :end-of-command)))
-- 
2.19.1.816.gcd69ec8cde.dirty

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Vinothan Shankar <darael@dracon.is>, eliz@gnu.org, Ted Zlatanov <tzz@lifelogs.com>
> Date: Wed, 09 Jan 2019 11:48:46 +0100

Thanks, a few comments regarding the Texinfo part:

> +Passing @code{:client certificate t} triggers looking up of client
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This should be wrapped in @w{..}, otherwise makeinfo might divide it
between two lines.

> +certificates matching @var{host} and @var{service} using the
> +'auth-source' library.  Any resulting client certificates are passed

auth-source should be in @file, and without the quotes.

> +down to the lower TLS layers.  The format used by @file{.authinfo} to
> +specify the per-server keys is described in @xref{Help for
> +users,,auth-source, auth, Emacs auth-source Library}.

@xref can only be used at the beginning of a sentence, as it generates
a capitalizes "See".  Use "see @ref" instead here.

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: Vinothan Shankar <darael@dracon.is>, eliz@gnu.org, Ted Zlatanov <tzz@lifelogs.com>
>> Date: Wed, 09 Jan 2019 11:48:46 +0100
>
> Thanks, a few comments regarding the Texinfo part:
>
>> +Passing @code{:client certificate t} triggers looking up of client
>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This should be wrapped in @w{..}, otherwise makeinfo might divide it
> between two lines.
>

Fixed, along with another instance later.

>> +certificates matching @var{host} and @var{service} using the
>> +'auth-source' library.  Any resulting client certificates are passed
>
> auth-source should be in @file, and without the quotes.
>
>> +down to the lower TLS layers.  The format used by @file{.authinfo} to
>> +specify the per-server keys is described in @xref{Help for
>> +users,,auth-source, auth, Emacs auth-source Library}.
>
> @xref can only be used at the beginning of a sentence, as it generates
> a capitalizes "See".  Use "see @ref" instead here.

Fixed. I was about to push, then got paranoid, so I wrote a few tests
for 'open-network-stream', which gave me a few suprises. I had to add
the following in network-stream-tests.el:

+(require 'network-stream)
+; The require above is needed for 'open-network-stream', but it pulls
+; in nsm, which then makes the :nowait tests fail unless we disable
+; the nsm.
+(setq network-security-level 'low)

otherwise both the old and my new ':nowait t' tests failed. Is that
expected?

Thanks

Robert

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: darael@dracon.is,  tzz@lifelogs.com,  33780@debbugs.gnu.org
> Date: Mon, 14 Jan 2019 14:27:15 +0100
> 
> Fixed. I was about to push, then got paranoid, so I wrote a few tests
> for 'open-network-stream', which gave me a few suprises. I had to add
> the following in network-stream-tests.el:
> 
> +(require 'network-stream)
> +; The require above is needed for 'open-network-stream', but it pulls
> +; in nsm, which then makes the :nowait tests fail unless we disable
> +; the nsm.
> +(setq network-security-level 'low)
> 
> otherwise both the old and my new ':nowait t' tests failed. Is that
> expected?

Not sure.  Did you understand why it failed?  IOW, what does nsm have
to do with the failures?

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: darael@dracon.is,  tzz@lifelogs.com,  33780@debbugs.gnu.org
>> Date: Mon, 14 Jan 2019 14:27:15 +0100
>> 
>> Fixed. I was about to push, then got paranoid, so I wrote a few tests
>> for 'open-network-stream', which gave me a few suprises. I had to add
>> the following in network-stream-tests.el:
>> 
>> +(require 'network-stream)
>> +; The require above is needed for 'open-network-stream', but it pulls
>> +; in nsm, which then makes the :nowait tests fail unless we disable
>> +; the nsm.
>> +(setq network-security-level 'low)
>> 
>> otherwise both the old and my new ':nowait t' tests failed. Is that
>> expected?
>
> Not sure.  Did you understand why it failed?  IOW, what does nsm have
> to do with the failures?

When I ran the equivalent 'open-network-stream' code to the tests
manually in 'emacs -Q', I get a prompt from nsm asking me whether to
accept the certificate of the server Iʼm connecting to.

When running the test suite, weʼre in batch mode, so thereʼs no way to
answer that question, as far as I know, so turning off the nsm is the
only way to go.

This only fails for the existing tests with ':nowait t', since then I
suspect nsm gets called automatically, whilst in the ':nowait nil'
case nsm never gets called (the existing tests all use
'make-network-process' directly, rather than 'open-network-stream', so
they bypass nsm).

This could be seen as a bug in nsm, I suppose, since naïvely you
wouldn't expect loading it to change the behaviour of
'make-network-process'.

Robert

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: darael@dracon.is,  tzz@lifelogs.com,  33780@debbugs.gnu.org
> Date: Mon, 14 Jan 2019 17:25:55 +0100
> 
> When running the test suite, weʼre in batch mode, so thereʼs no way to
> answer that question, as far as I know, so turning off the nsm is the
> only way to go.

You could also override the nsm-query-user function, I think.

> This only fails for the existing tests with ':nowait t', since then I
> suspect nsm gets called automatically, whilst in the ':nowait nil'
> case nsm never gets called (the existing tests all use
> 'make-network-process' directly, rather than 'open-network-stream', so
> they bypass nsm).

I don't think I understood why nsm gets called only in the ":nowait t"
case.  What did I miss?

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: darael@dracon.is,  tzz@lifelogs.com,  33780@debbugs.gnu.org
>> Date: Mon, 14 Jan 2019 17:25:55 +0100
>> 
>> When running the test suite, weʼre in batch mode, so thereʼs no way to
>> answer that question, as far as I know, so turning off the nsm is the
>> only way to go.
>
> You could also override the nsm-query-user function, I think.
>

True.

>> This only fails for the existing tests with ':nowait t', since then I
>> suspect nsm gets called automatically, whilst in the ':nowait nil'
>> case nsm never gets called (the existing tests all use
>> 'make-network-process' directly, rather than 'open-network-stream', so
>> they bypass nsm).
>
> I don't think I understood why nsm gets called only in the ":nowait t"
> case.  What did I miss?

process.c has:

#ifdef HAVE_GNUTLS
static void
finish_after_tls_connection (Lisp_Object proc)
{
  struct Lisp_Process *p = XPROCESS (proc);
  Lisp_Object contact = p->childp;
  Lisp_Object result = Qt;

  if (!NILP (Ffboundp (Qnsm_verify_connection)))
    result = call3 (Qnsm_verify_connection,
		    proc,
		    Fplist_get (contact, QChost),
		    Fplist_get (contact, QCservice));

so loading nsm.el causes nsm-verify-connection to get called in the
':nowait t' case. Presumably in the ':nowait nil' case gnutls-boot has
already completed the tls connection, and finish_after_tls_connection
never gets called (thatʼs speculation on my part). I donʼt know the
GnuTLS code well enough to know if this is a bug. Ted?

Robert

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: darael@dracon.is,  tzz@lifelogs.com,  33780@debbugs.gnu.org
> Date: Mon, 14 Jan 2019 18:40:59 +0100
> 
> > I don't think I understood why nsm gets called only in the ":nowait t"
> > case.  What did I miss?
> 
> process.c has:
> 
> #ifdef HAVE_GNUTLS
> static void
> finish_after_tls_connection (Lisp_Object proc)
> {
>   struct Lisp_Process *p = XPROCESS (proc);
>   Lisp_Object contact = p->childp;
>   Lisp_Object result = Qt;
> 
>   if (!NILP (Ffboundp (Qnsm_verify_connection)))
>     result = call3 (Qnsm_verify_connection,
> 		    proc,
> 		    Fplist_get (contact, QChost),
> 		    Fplist_get (contact, QCservice));
> 
> so loading nsm.el causes nsm-verify-connection to get called in the
> ':nowait t' case. Presumably in the ':nowait nil' case gnutls-boot has
> already completed the tls connection, and finish_after_tls_connection
> never gets called (thatʼs speculation on my part). I donʼt know the
> GnuTLS code well enough to know if this is a bug. Ted?

Ah, okay.  No, I don't think this is a bug.  So use some way to get
nsm to approve the connection.

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

Eli Zaretskii <eliz@gnu.org> writes:

>> so loading nsm.el causes nsm-verify-connection to get called in the
>> ':nowait t' case. Presumably in the ':nowait nil' case gnutls-boot has
>> already completed the tls connection, and finish_after_tls_connection
>> never gets called (thatʼs speculation on my part). I donʼt know the
>> GnuTLS code well enough to know if this is a bug. Ted?
>

I can confirm this is what happens: finish_after_tls_connection only
gets called when ':nowait t'.

> Ah, okay.  No, I don't think this is a bug.  So use some way to get
> nsm to approve the connection.

I do find it unexpected that the low level GnuTLS code only invokes
the nsm for ':nowait t' connections.  OTOH 'open-network-stream' works
fine, and uses the nsm, so itʼs not a big deal.

Overriding nsm-query appears not to be enough (itʼs enough when
running the tests interactively, but not in batch mode), I had to
override 'nsm-verify-connection'.

Robert

bug#33780: network-stream.el: network-stream-certificate always returns nil

[Robert Pluim]

tags 33780 fixed
close 33780 27.1
quit

Fix pushed to master as f3f9a3582e along with some tests.