client certs and CRL lists for GnuTLS (was: Opportunistic STARTTLS in smtpmail.el)

[Ted Zlatanov <tzz@lifelogs.com>]

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

On Mon, 02 May 2011 14:21:32 -0500 Ted Zlatanov <tzz@lifelogs.com> wrote: 

TZ> On Mon, 02 May 2011 20:59:18 +0200 Lars Magne Ingebrigtsen <larsi@gnus.org> wrote: 
LMI> Ted Zlatanov <tzz@lifelogs.com> writes:
LMI> "--x509keyfile" "--x509certfile"
>>> 
LMI> to gnutlc-cli.  `open-network-stream' has no concept of these things,
LMI> and I'm not sure gnutls.c has, either.  Ted?
>>> 
>>> Yes, definitely, with the :keyfiles and :trustfiles parameters to
>>> `gnutls-boot'.

LMI> Right.  Would "--x509keyfile" correspond to :keyfiles and
LMI> "--x509certfile" to :trustfiles?

TZ> Oh wait, I think I'm wrong.  The key+cert files (client-side SSL certs)
TZ> are not the same as the trust files (which verify the server's SSL
TZ> cert).  Let me take a look, this may require another parameter or I'm
TZ> missing something.

OK, I found the problem.  See the attached patch.  Before I was
processing client key files as CRL lists (I meant to do client key+cert
pairs PLUS CRL lists and somehow merged the two mentally).

The attached patch adds a :keylist parameter to `gnutls-boot' which is a
list of (client key file, client cert file) pairs.  It also renames the
:keyfiles parameter to :crlfiles since it's for CRL lists.  So now you
can specify any number of client certs.  If the key files require a
passphrase, the decoding won't work because we don't set a callback.

`gnutls-negotiate' also gets the parameter changes (should I just make
it take a plist?)

Please look it over and I'll commit if you agree it's correct.

Thanks
Ted


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gnutls-crl-and-client-certs.patch --]
[-- Type: text/x-diff, Size: 6400 bytes --]

=== modified file 'lisp/net/gnutls.el'
--- lisp/net/gnutls.el	2011-04-25 13:47:23 +0000
+++ lisp/net/gnutls.el	2011-05-03 15:14:03 +0000
@@ -86,7 +86,7 @@
 (declare-function gnutls-errorp "gnutls.c" (error))
 
 (defun gnutls-negotiate (proc type hostname &optional priority-string
-                              trustfiles keyfiles verify-flags
+                              trustfiles crlfiles keylist verify-flags
                               verify-error verify-hostname-error)
   "Negotiate a SSL/TLS connection.  Returns proc. Signals gnutls-error.
 TYPE is `gnutls-x509pki' (default) or `gnutls-anon'.  Use nil for the default.
@@ -94,7 +94,8 @@
 HOSTNAME is the remote hostname.  It must be a valid string.
 PRIORITY-STRING is as per the GnuTLS docs, default is \"NORMAL\".
 TRUSTFILES is a list of CA bundles.
-KEYFILES is a list of client keys.
+CRLFILES is a list of CRL files.
+KEYLIST is an alist of (client key file, client cert file) pairs.
 
 When VERIFY-HOSTNAME-ERROR is not nil, an error will be raised
 when the hostname does not match the presented certificate's host
@@ -141,7 +142,8 @@
                              :hostname ,hostname
                              :loglevel ,gnutls-log-level
                              :trustfiles ,trustfiles
-                             :keyfiles ,keyfiles
+                             :crlfiles ,crlfiles
+                             :keylist ,keylist
                              :verify-flags ,verify-flags
                              :verify-error ,verify-error
                              :verify-hostname-error ,verify-hostname-error

=== modified file 'src/gnutls.c'
--- src/gnutls.c	2011-05-02 02:49:06 +0000
+++ src/gnutls.c	2011-05-03 15:11:35 +0000
@@ -44,7 +44,8 @@
 /* The following are for the property list of `gnutls-boot'.  */
 static Lisp_Object Qgnutls_bootprop_priority;
 static Lisp_Object Qgnutls_bootprop_trustfiles;
-static Lisp_Object Qgnutls_bootprop_keyfiles;
+static Lisp_Object Qgnutls_bootprop_keylist;
+static Lisp_Object Qgnutls_bootprop_crlfiles;
 static Lisp_Object Qgnutls_bootprop_callbacks;
 static Lisp_Object Qgnutls_bootprop_loglevel;
 static Lisp_Object Qgnutls_bootprop_hostname;
@@ -412,7 +413,10 @@
 
 :trustfiles is a list of PEM-encoded trust files for `gnutls-x509pki'.
 
-:keyfiles is a list of PEM-encoded key files for `gnutls-x509pki'.
+:crlfiles is a list of PEM-encoded CRL lists for `gnutls-x509pki'.
+
+:keylist is an alist of PEM-encoded key files and PEM-encoded
+certificates for `gnutls-x509pki'.
 
 :callbacks is an alist of callback functions, see below.
 
@@ -471,7 +475,8 @@
   /* Placeholders for the property list elements.  */
   Lisp_Object priority_string;
   Lisp_Object trustfiles;
-  Lisp_Object keyfiles;
+  Lisp_Object crlfiles;
+  Lisp_Object keylist;
   /* Lisp_Object callbacks; */
   Lisp_Object loglevel;
   Lisp_Object hostname;
@@ -486,7 +491,8 @@
   hostname              = Fplist_get (proplist, Qgnutls_bootprop_hostname);
   priority_string       = Fplist_get (proplist, Qgnutls_bootprop_priority);
   trustfiles            = Fplist_get (proplist, Qgnutls_bootprop_trustfiles);
-  keyfiles              = Fplist_get (proplist, Qgnutls_bootprop_keyfiles);
+  keylist               = Fplist_get (proplist, Qgnutls_bootprop_keylist);
+  crlfiles              = Fplist_get (proplist, Qgnutls_bootprop_crlfiles);
   /* callbacks          = Fplist_get (proplist, Qgnutls_bootprop_callbacks); */
   loglevel              = Fplist_get (proplist, Qgnutls_bootprop_loglevel);
   verify_flags          = Fplist_get (proplist, Qgnutls_bootprop_verify_flags);
@@ -614,15 +620,41 @@
             }
         }
 
-      for (tail = keyfiles; !NILP (tail); tail = Fcdr (tail))
-	{
-	  Lisp_Object keyfile = Fcar (tail);
-          if (STRINGP (keyfile))
-            {
-              GNUTLS_LOG2 (1, max_log_level, "setting the keyfile: ",
+      for (tail = crlfiles; !NILP (tail); tail = Fcdr (tail))
+	{
+	  Lisp_Object crlfile = Fcar (tail);
+          if (STRINGP (crlfile))
+            {
+              GNUTLS_LOG2 (1, max_log_level, "setting the CRL file: ",
+                           SSDATA (crlfile));
+              ret = gnutls_certificate_set_x509_crl_file
+                (x509_cred,
+                 SSDATA (crlfile),
+                 file_format);
+
+              if (ret < GNUTLS_E_SUCCESS)
+                return gnutls_make_error (ret);
+            }
+          else
+            {
+              error ("Sorry, GnuTLS can't use non-string CRL file %s",
+                     SDATA (crlfile));
+            }
+        }
+
+      for (tail = keylist; !NILP (tail); tail = Fcdr (tail))
+	{
+	  Lisp_Object keyfile = Fcar (Fcar (tail));
+	  Lisp_Object certfile = Fcar (Fcdr (tail));
+          if (STRINGP (keyfile) && STRINGP (certfile))
+            {
+              GNUTLS_LOG2 (1, max_log_level, "setting the client key file: ",
                            SSDATA (keyfile));
-              ret = gnutls_certificate_set_x509_crl_file
+              GNUTLS_LOG2 (1, max_log_level, "setting the client cert file: ",
+                           SSDATA (certfile));
+              ret = gnutls_certificate_set_x509_key_file
                 (x509_cred,
+                 SSDATA (certfile),
                  SSDATA (keyfile),
                  file_format);
 
@@ -631,8 +663,12 @@
             }
           else
             {
-              error ("Sorry, GnuTLS can't use non-string keyfile %s",
-                     SDATA (keyfile));
+              if (STRINGP (keyfile))
+                error ("Sorry, GnuTLS can't use non-string client cert file %s",
+                       SDATA (certfile));
+              else
+                error ("Sorry, GnuTLS can't use non-string client key file %s",
+                       SDATA (keyfile));
             }
         }
     }
@@ -868,8 +904,11 @@
   Qgnutls_bootprop_trustfiles = intern_c_string (":trustfiles");
   staticpro (&Qgnutls_bootprop_trustfiles);
 
-  Qgnutls_bootprop_keyfiles = intern_c_string (":keyfiles");
-  staticpro (&Qgnutls_bootprop_keyfiles);
+  Qgnutls_bootprop_keylist = intern_c_string (":keylist");
+  staticpro (&Qgnutls_bootprop_keylist);
+
+  Qgnutls_bootprop_crlfiles = intern_c_string (":crlfiles");
+  staticpro (&Qgnutls_bootprop_crlfiles);
 
   Qgnutls_bootprop_callbacks = intern_c_string (":callbacks");
   staticpro (&Qgnutls_bootprop_callbacks);