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

[Robert Pluim <rpluim@gmail.com>]

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