Merge of two starttls.el's

Merge of two starttls.el's

[Simon Josefsson]

This is a followup on an old thread where I wanted to replace
lisp/gnus/starttls.el with a version that uses GNUTLS instead of the
OpenSSL based (and not widely installed) "starttls" tool.  The
discussion suggested that it was too drastic, and it should have been
backwards compatible instead, i.e. support both GNUTLS and starttls.
This patch implement that.

I'd really like to install this patch soon.  If the user has the
"starttls" tool installed, the patch should not change functionality
in ANY way.  Please review the patch to verify this.

If "starttls" is not installed, however, the new code will try to use
GNUTLS.  The code may be buggy, but my point is that in any case the
user is not worse of than before, where it would just simply fail when
"starttls" was not installed.

I will update the SMTP manual, write NEWS entries, and commit the
patch once someone has reviewed this.  Any suggestions or comments
appreciated.

Thanks.

(Naturally, I'm sending this message using this code, with GNUTLS.)

2004-05-26  Simon Josefsson  <jas@extundo.com>

	* starttls.el: Merge with my GNUTLS based starttls.el.
	(starttls-gnutls-program, starttls-use-gnutls)
	(starttls-extra-arguments, starttls-process-connection-type)
	(starttls-connect, starttls-failure, starttls-success): New
	variables.
	(starttls-program, starttls-extra-args): Doc fix.
	(starttls-negotiate-gnutls, starttls-open-stream-gnutls): New
	functions.
	(starttls-negotiate, starttls-open-stream): Check
	`starttls-use-gnutls' and pass on to corresponding *-gnutls
	function if it is set.

Index: starttls.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/gnus/starttls.el,v
retrieving revision 1.3
diff -u -p -u -w -r1.3 starttls.el
--- starttls.el	1 Sep 2003 15:45:28 -0000	1.3
+++ starttls.el	26 May 2004 17:28:24 -0000
@@ -1,10 +1,11 @@
 ;;; starttls.el --- STARTTLS functions
 
-;; Copyright (C) 1999, 2000 Free Software Foundation, Inc.
+;; Copyright (C) 1999, 2000, 2004 Free Software Foundation, Inc.
 
 ;; Author: Daiki Ueno <ueno@unixuser.org>
+;; Author: Simon Josefsson <simon@josefsson.org>
 ;; Created: 1999/11/20
-;; Keywords: TLS, SSL, OpenSSL, mail, news
+;; Keywords: TLS, SSL, OpenSSL, GNUTLS, mail, news
 
 ;; This file is part of GNU Emacs.
 
@@ -30,6 +31,88 @@
 ;; [RFC 2595] "Using TLS with IMAP, POP3 and ACAP"
 ;;	by Chris Newman <chris.newman@innosoft.com> (1999/06)
 
+;; This file now contain a combination of the two previous
+;; implementations both called "starttls.el".  The first one is Daiki
+;; Ueno's starttls.el which uses his own "starttls" command line tool,
+;; and the second one is Simon Josefsson's starttls.el which uses
+;; "gnutls-cli" from GNUTLS.
+;;
+;; If "gnutls-cli" is available, it is prefered by the code over
+;; "starttls".  Use `starttls-use-gnutls' to toggle between
+;; implementations if you have both tools installed.
+
+;; The GNUTLS support require GNUTLS 0.9.90 (released 2003-10-08) or
+;; later, from <http://www.gnu.org/software/gnutls/>, or "starttls"
+;; from <ftp://ftp.opaopa.org/pub/elisp/>.
+
+;; Usage is similar to `open-network-stream'.  For example:
+;;
+;; (when (setq tmp (starttls-open-stream
+;;			"test" (current-buffer) "yxa.extundo.com" 25))
+;;   (accept-process-output tmp 15)
+;;   (process-send-string tmp "STARTTLS\n")
+;;   (accept-process-output tmp 15)
+;;   (message "STARTTLS output:\n%s" (starttls-negotiate tmp))
+;;   (process-send-string tmp "EHLO foo\n"))
+
+;; An example run yield the following output:
+;;
+;; 220 yxa.extundo.com ESMTP Sendmail 8.12.11/8.12.11/Debian-3; Wed, 26 May 2004 19:12:29 +0200; (No UCE/UBE) logging access from: c494102a.s-bi.bostream.se(OK)-c494102a.s-bi.bostream.se [217.215.27.65]
+;; 220 2.0.0 Ready to start TLS
+;; 250-yxa.extundo.com Hello c494102a.s-bi.bostream.se [217.215.27.65], pleased to meet you
+;; 250-ENHANCEDSTATUSCODES
+;; 250-PIPELINING
+;; 250-EXPN
+;; 250-VERB
+;; 250-8BITMIME
+;; 250-SIZE
+;; 250-DSN
+;; 250-ETRN
+;; 250-AUTH DIGEST-MD5 CRAM-MD5 PLAIN LOGIN
+;; 250-DELIVERBY
+;; 250 HELP
+;; nil
+;;
+;; With the message buffer containing:
+;;
+;; STARTTLS output:
+;; *** Starting TLS handshake
+;; - Server's trusted authorities:
+;;    [0]: C=SE,ST=Stockholm,L=Stockholm,O=YXA,OU=CA,CN=yxa.extundo.com,EMAIL=staff@yxa.extundo.com
+;; - Certificate type: X.509
+;;  - Got a certificate list of 2 certificates.
+;;
+;;  - Certificate[0] info:
+;;  # The hostname in the certificate matches 'yxa.extundo.com'.
+;;  # valid since: Wed May 26 12:16:00 CEST 2004
+;;  # expires at: Wed Jul 26 12:16:00 CEST 2023
+;;  # serial number: 04
+;;  # fingerprint: 7c 04 4b c1 fa 26 9b 5d 90 22 52 3c 65 3d 85 3a
+;;  # version: #1
+;;  # public key algorithm: RSA
+;;  #   Modulus: 1024 bits
+;;  # Subject's DN: C=SE,ST=Stockholm,L=Stockholm,O=YXA,OU=Mail server,CN=yxa.extundo.com,EMAIL=staff@yxa.extundo.com
+;;  # Issuer's DN: C=SE,ST=Stockholm,L=Stockholm,O=YXA,OU=CA,CN=yxa.extundo.com,EMAIL=staff@yxa.extundo.com
+;;
+;;  - Certificate[1] info:
+;;  # valid since: Sun May 23 11:35:00 CEST 2004
+;;  # expires at: Sun Jul 23 11:35:00 CEST 2023
+;;  # serial number: 00
+;;  # fingerprint: fc 76 d8 63 1a c9 0b 3b fa 40 fe ed 47 7a 58 ae
+;;  # version: #3
+;;  # public key algorithm: RSA
+;;  #   Modulus: 1024 bits
+;;  # Subject's DN: C=SE,ST=Stockholm,L=Stockholm,O=YXA,OU=CA,CN=yxa.extundo.com,EMAIL=staff@yxa.extundo.com
+;;  # Issuer's DN: C=SE,ST=Stockholm,L=Stockholm,O=YXA,OU=CA,CN=yxa.extundo.com,EMAIL=staff@yxa.extundo.com
+;;
+;; - Peer's certificate issuer is unknown
+;; - Peer's certificate is NOT trusted
+;; - Version: TLS 1.0
+;; - Key Exchange: RSA
+;; - Cipher: ARCFOUR 128
+;; - MAC: SHA
+;; - Compression: NULL
+
 ;;; Code:
 
 (defgroup starttls nil
@@ -37,18 +120,141 @@
   :version "21.1"
   :group 'mail)
 
+(defcustom starttls-gnutls-program "gnutls-cli"
+  "Name of GNUTLS command line tool.
+This program is used when GNUTLS is used, i.e. when
+`starttls-use-gnutls' is non-nil."
+  :type 'string
+  :group 'starttls)
+
 (defcustom starttls-program "starttls"
-  "The program to run in a subprocess to open an TLSv1 connection."
+  "The program to run in a subprocess to open an TLSv1 connection.
+This program is used when the `starttls' command is used,
+i.e. when `starttls-use-gnutls' is nil."
   :type 'string
   :group 'starttls)
 
+(defcustom starttls-use-gnutls (not (executable-find starttls-program))
+  "*Whether to use GNUTLS instead of the `starttls' command."
+  :type 'boolean
+  :group 'starttls)
+
 (defcustom starttls-extra-args nil
-  "Extra arguments to `starttls-program'."
+  "Extra arguments to `starttls-program'.
+This program is used when the `starttls' command is used,
+i.e. when `starttls-use-gnutls' is nil."
   :type '(repeat string)
   :group 'starttls)
 
+(defcustom starttls-extra-arguments nil
+  "Extra arguments to `starttls-program'.
+This program is used when GNUTLS is used, i.e. when
+`starttls-use-gnutls' is non-nil.
+
+For example, non-TLS compliant servers may require
+'(\"--protocols\" \"ssl3\").  Invoke \"gnutls-cli --help\" to
+find out which parameters are available."
+  :type '(repeat string)
+  :group 'starttls)
+
+(defcustom starttls-process-connection-type nil
+  "*Value for `process-connection-type' to use when starting STARTTLS process."
+  :type 'boolean
+  :group 'starttls)
+
+(defcustom starttls-connect "- Simple Client Mode:\n\n"
+  "*Regular expression indicating successful connection.
+The default is what GNUTLS's \"gnutls-cli\" outputs."
+  ;; GNUTLS cli.c:main() print this string when it is starting to run
+  ;; in the application read/write phase.  If the logic, or the string
+  ;; itself, is modified, this must be updated.
+  :type 'regexp
+  :group 'starttls)
+
+(defcustom starttls-failure "\\*\\*\\* Handshake has failed"
+  "*Regular expression indicating failed TLS handshake.
+The default is what GNUTLS's \"gnutls-cli\" outputs."
+  ;; GNUTLS cli.c:do_handshake() print this string on failure.  If the
+  ;; logic, or the string itself, is modified, this must be updated.
+  :type 'regexp
+  :group 'starttls)
+
+(defcustom starttls-success "- Compression: "
+  "*Regular expression indicating completed TLS handshakes.
+The default is what GNUTLS's \"gnutls-cli\" outputs."
+  ;; GNUTLS cli.c:do_handshake() calls, on success,
+  ;; common.c:print_info(), that unconditionally print this string
+  ;; last.  If that logic, or the string itself, is modified, this
+  ;; must be updated.
+  :type 'regexp
+  :group 'starttls)
+
+(defun starttls-negotiate-gnutls (process)
+  "Negotiate TLS on process opened by `open-starttls-stream'.
+This should typically only be done once.  It typically return a
+multi-line informational message with information about the
+handshake, or NIL on failure."
+  (let (buffer info old-max done-ok done-bad)
+    (if (null (setq buffer (process-buffer process)))
+	;; XXX How to remove/extract the TLS negotiation junk?
+	(signal-process (process-id process) 'SIGALRM)
+      (with-current-buffer buffer
+	(save-excursion
+	  (setq old-max (goto-char (point-max)))
+	  (signal-process (process-id process) 'SIGALRM)
+	  (while (and (processp process)
+		      (eq (process-status process) 'run)
+		      (save-excursion
+			(goto-char old-max)
+			(not (or (setq done-ok (re-search-forward
+						starttls-success nil t))
+				 (setq done-bad (re-search-forward
+						 starttls-failure nil t))))))
+	    (accept-process-output process 1 100)
+	    (sit-for 0.1))
+	  (setq info (buffer-substring-no-properties old-max (point-max)))
+	  (delete-region old-max (point-max))
+	  (if (or (and done-ok (not done-bad))
+		  ;; Prevent mitm that fake success msg after failure msg.
+		  (and done-ok done-bad (< done-ok done-bad)))
+	      info
+	    (message "STARTTLS negotiation failed: %s" info)
+	    nil))))))
+
 (defun starttls-negotiate (process)
-  (signal-process (process-id process) 'SIGALRM))
+  (if starttls-use-gnutls
+      (starttls-negotiate-gnutls process)
+    (signal-process (process-id process) 'SIGALRM)))
+
+(defun starttls-open-stream-gnutls (name buffer host service)
+  (message "Opening STARTTLS connection to `%s'..." host)
+  (let* (done
+	 (old-max (with-current-buffer buffer (point-max)))
+	 (process-connection-type starttls-process-connection-type)
+	 (process (apply #'start-process name buffer
+			 starttls-gnutls-program "-s" host
+			 "-p" (if (integerp service)
+				  (int-to-string service)
+				service)
+			 starttls-extra-arguments)))
+    (process-kill-without-query process)
+    (while (and (processp process)
+		(eq (process-status process) 'run)
+		(save-excursion
+		  (set-buffer buffer)
+		  (goto-char old-max)
+		  (not (setq done (re-search-forward
+				   starttls-connect nil t)))))
+      (accept-process-output process 0 100)
+      (sit-for 0.1))
+    (if done
+	(with-current-buffer buffer
+	  (delete-region old-max done))
+      (delete-process process)
+      (setq process nil))
+    (message "Opening STARTTLS connection to `%s'...%s"
+	     host (if done "done" "failed"))
+    process))
 
 (defun starttls-open-stream (name buffer host service)
   "Open a TLS connection for a service to a host.
@@ -64,13 +270,15 @@ BUFFER is the buffer (or `buffer-name') 
 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."
-  (let* ((process-connection-type nil)
+  (if starttls-use-gnutls
+      (starttls-open-stream-gnutls name buffer host service)
+    (let* ((process-connection-type starttls-process-connection-type)
 	 (process (apply #'start-process
 			 name buffer starttls-program
 			 host (format "%s" service)
 			 starttls-extra-args)))
     (process-kill-without-query process)
-    process))
+      process)))
 
 (provide 'starttls)

Re: Merge of two starttls.el's

[Simon Josefsson]

Simon Josefsson <jas@extundo.com> writes:

> (Naturally, I'm sending this message using this code, with GNUTLS.)

Well, actually I wasn't, because smtpmail.el disable STARTTLS
functionality if the binary "starttls" isn't present.  That should be
fixed as well, but can wait.  _This_ message should be sent using
GNUTLS, though.

Re: Merge of two starttls.el's

[Simon Josefsson]

Thanks for quick review!  I'm adding emacs-devel to Cc, I hope you
don't mind.  Several of the issues can probably be regarded as open
for discussion.

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +(defcustom starttls-gnutls-program "gnutls-cli"
>> +  "Name of GNUTLS command line tool.
>> +This program is used when GNUTLS is used, i.e. when
>> +`starttls-use-gnutls' is non-nil."
>> +  :type 'string
>> +  :group 'starttls)
>> +
>>  (defcustom starttls-program "starttls"
>> -  "The program to run in a subprocess to open an TLSv1 connection."
>> +  "The program to run in a subprocess to open an TLSv1 connection.
>> +This program is used when the `starttls' command is used,
>> +i.e. when `starttls-use-gnutls' is nil."
>>    :type 'string
>>    :group 'starttls)
>  
>> +(defcustom starttls-use-gnutls (not (executable-find starttls-program))
>> +  "*Whether to use GNUTLS instead of the `starttls' command."
>> +  :type 'boolean
>> +  :group 'starttls)
>
> I haven't looked enough at the code to understand whether it would make
> sense, but I would naturally be tempted to replace the above with
>
> (defcustom starttls-use-gnutls (not (executable-find starttls-program))
> ...
> (defcustom starttls-program (if starttls-use-gnutls "gnutls-cli" "starttls")

The reason for doing it the other way is that people can't re-define
the name of both tools, and switch between them using only
starttls-use-gnutls.  I.e., using your proposal, if the user toggle
starttls-use-gnutls, she must also toggle starttls-program.

Perhaps I'm the only one, but I'd like to toggle between both
implementations using starttls-use-gnutls (because GNUTLS and Openssl
are very different, with different default ciphers etc), and I often
have both tools installed at weird locations (not in PATH).

>>  (defcustom starttls-extra-args nil
>> -  "Extra arguments to `starttls-program'."
>> +  "Extra arguments to `starttls-program'.
>> +This program is used when the `starttls' command is used,
>> +i.e. when `starttls-use-gnutls' is nil."
>>    :type '(repeat string)
>>    :group 'starttls)
>  
>> +(defcustom starttls-extra-arguments nil
>> +  "Extra arguments to `starttls-program'.
>> +This program is used when GNUTLS is used, i.e. when
>> +`starttls-use-gnutls' is non-nil.
>
> Similarly here I would just use a single variable.

Here there is another reason: the command line arguments to the two
tools are incompatible.  Look at smtpmail.el:

      (let* ((cred-key (smtpmail-cred-key cred))
	     (cred-cert (smtpmail-cred-cert cred))
	     (starttls-extra-args
	      (when (and (stringp cred-key) (stringp cred-cert)
			 (file-regular-p
			  (setq cred-key (expand-file-name cred-key)))
			 (file-regular-p
			  (setq cred-cert (expand-file-name cred-cert))))
		(list "--key-file" cred-key "--cert-file" cred-cert))))
	(starttls-open-stream "SMTP" process-buffer host port)))))

These command line parameters do not work with gnutls-cli.  I expect
that smtpmail will have to add a new let variable
`starttls-extra-arguments' with the GNUTLS command line parameters
too.

I should have mentioned this complexity.  The variable names do not
suggest the GNUTLS/starttls incompatibility was the motivation for the
two variables.

It would be nicer to not require callers of starttls.el to know the
command line syntax of various implementations that starttls.el uses.
Hm.  Perhaps add temporary variables for the cert/key filenames?
I.e.:

(defvar starttls-key-file nil)
(defvar starttls-cert-file nil)

Then elisp code calling starttls-open-stream could bind those
variables, and the code for gnutls-cli and starttls could make sure
the proper parameter is used.

The starttls-extra-args variable would need to be kept, though, for
backwards compatibility.

>> +(defcustom starttls-process-connection-type nil
>> +  "*Value for `process-connection-type' to use when starting STARTTLS process."
>> +  :type 'boolean
>> +  :group 'starttls)
>
> This seems unrelated to adding support for gnutls.  Is that right?

Yes.

> What is its purpose?

I haven't needed the variable now, but my experience with imap.el
suggest it sometimes work better to toggle that variable.  Note that
the default value isn't changed, it just make it possible for users to
toggle it, which can be useful to suggest when people report problems.

(Looking at my own .emacs, I have been running with
imap-process-connection-type set to t for ages.  Perhaps the problems
that made me use that setting has been fixed, though...)

>> +(defcustom starttls-connect "- Simple Client Mode:\n\n"
>> +(defcustom starttls-failure "\\*\\*\\* Handshake has failed"
>> +(defcustom starttls-success "- Compression: "
>
> Those shouldn't be a defcustom.  I'd make it a defconst or maybe defvar.
> Are users ever expected to fiddle with this?

I'm not sure.  There are two concerns.

1) GNUTLS is still under heavy development, and I'm not sure those
strings will be stable.  So it would be nice for users to change them,
as Emacs have a longer release cycle than GNUTLS.

2) It might allow advanced users to swap in a completely different
implementation than "gnutls-cli", change those strings to something
else, and have things work.  Given the above discussion about hard
coded command line parameters, this argument is not a strong one,
since the parameters would likely be incompatible as well.

> Other than that, it looks safe, so I have no objection to your installing
> this patch,

Great.  I'll wait a few days to let other people comment.