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

[Robert Pluim]

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.

>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