all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Konstantin Kliakhandler <kosta@slumpy.org>
To: 23759@debbugs.gnu.org
Subject: bug#23759: 25.1.50; 25.1.50; open-tls-stream creates malformed gnutls-cli command if trusted cert files don't exist
Date: Sat, 2 Jul 2016 10:09:50 +0300	[thread overview]
Message-ID: <CAH+LVpk1DoqS5mdOX9-5YZ6RqntM+4+UQC9BPhwAZmAb=n6zWA@mail.gmail.com> (raw)
In-Reply-To: <87y46ahz23.fsf@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 2467 bytes --]

Hello,

First, I apologize for the double posting - I realize that I sent the
previous message with a messed up subject and this caused it not to be
grouped with the rest of the messages in this bug, and to not appear in the
tracker. I hope this second one will work now.

I am using emacs on OSX 10.12 from https://emacsformacosx.com/builds:

(emacs-version)
"GNU Emacs 25.1.50.1 (x86_64-apple-darwin13.4.0, NS appkit-1265.21 Version
10.9.5 (Build 13F1603))
 of 2016-05-30"

And experience the same problem when running emacs -Q. Furthermore, I
diagnosed the cause to be incorrect building of formatted-cmd in
open-tls-stream from the given arguments. Attached is a patch that fixes
the problem on my end. To make the patch smaller, I did not reindent the
whole function - I hope this is alright.

Finally, I'd like to reply to:


> Fran <flitterio <at> gmail.com> writes:

> If Cygwin is installed, open-tls-stream works, because gnutls-cli and
> openssl
> > are available. It has worked for some time. I routinely use this feature
> to
> > enable ERC to connect to servers using TLS using function erc-tls.



> At some point, tls.el will be deprecated. Why aren't you just using
> Emacs with the built-in TLS support?


In my honest opinion, a feature is either deprecated or not, and while it
is not yet deprecated, bugs should not be ignored. I hope you would agree
at least to the point of testing the patch and incorporating it if it works
well :-)

Fuller description of the problem and the fix:

The problem: `open-tls-stream' replaces %t with exactly one element, which
is nil if none of gnutls-trustfiles is readable, and the first element of
gnutls-trustfiles is more than one is readable.
The Solution: In the patch I make the test iterate on all the trustfiles as
a user might have more than one relevant. In addition, I made the default
setting for tls-program have entries that do not explicitly specify the
trustfile.

One thing to note here perhaps, is that if (gnutls-trustfiles) returns an
empty list and one has the %t substitution in one of the tls-program
entries, then that entry will not be run at all. I feel that this is
reasonable since by setting --x509cafile nil one makes gnutls-cli fail
anyway.

Finally, I'm experiencing the above behavior, as far as I can tell, by
default in e.g. erc-tls. What is the proper way to move to the built in
TLS? Is it likely to be something in my config or in the implementation of
ERC?

Thanks,
Kosta

[-- Attachment #1.2: Type: text/html, Size: 3695 bytes --]

[-- Attachment #2: 0001-tls-Make-open-tls-stream-try-all-gnutls-trustfiles-a.patch --]
[-- Type: application/octet-stream, Size: 3501 bytes --]

From 057fc9158e816e5220b9303a2b634eab1d7c735e Mon Sep 17 00:00:00 2001
From: Konstantin Kliakhandler <kosta@slumpy.org>
Date: Sat, 2 Jul 2016 02:42:33 +0300
Subject: [PATCH] tls: Make open-tls-stream try all gnutls-trustfiles and set
 better default tls-program

This fixes the bug reported in https://lists.gnu.org/archive/html/bug-gnu-emacs/2016-06/msg00553.html

* lisp/net/tls.el (open-tls-stream): Iterate on all found
  gnutls-trustfiles instead of sticking with the first one found

(tls-program): Set the default to a list that includes both
gnutls-trustfiles substitution and without it.
---
 lisp/net/tls.el | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/lisp/net/tls.el b/lisp/net/tls.el
index f1219fd..7890ac0 100644
--- a/lisp/net/tls.el
+++ b/lisp/net/tls.el
@@ -77,9 +77,11 @@ and `gnutls-cli' (version 2.0.1) output."
   :group 'tls)
 
 (defcustom tls-program
-  '("gnutls-cli --x509cafile %t -p %p %h"
+  '("gnutls-cli -p %p %h"
+    "gnutls-cli --x509cafile %t -p %p %h"
     "gnutls-cli --x509cafile %t -p %p %h --protocols ssl3"
-    "openssl s_client -connect %h:%p -no_ssl2 -ign_eof")
+    "openssl s_client -connect %h:%p -no_ssl2 -ign_eof"
+    "openssl s_client -CAfile %t -connect %h:%p -no_ssl2 -ign_eof")
   "List of strings containing commands to start TLS stream to a host.
 Each entry in the list is tried until a connection is successful.
 %h is replaced with the server hostname, %p with the port to
@@ -93,14 +95,18 @@ successful negotiation."
   :type
   '(choice
     (const :tag "Default list of commands"
-	   ("gnutls-cli --x509cafile %t -p %p %h"
+	   ("gnutls-cli -p %p %h"
+            "gnutls-cli --x509cafile %t -p %p %h"
 	    "gnutls-cli --x509cafile %t -p %p %h --protocols ssl3"
+            "openssl s_client -connect %h:%p -no_ssl2 -ign_eof"
 	    "openssl s_client -CAfile %t -connect %h:%p -no_ssl2 -ign_eof"))
     (list :tag "Choose commands"
 	  :value
-	  ("gnutls-cli --x509cafile %t -p %p %h"
+	  ("gnutls-cli -p %p %h"
+           "gnutls-cli --x509cafile %t -p %p %h"
 	   "gnutls-cli --x509cafile %t -p %p %h --protocols ssl3"
-	   "openssl s_client -connect %h:%p -no_ssl2 -ign_eof")
+           "openssl s_client -connect %h:%p -no_ssl2 -ign_eof"
+	   "openssl s_client -CAfile %t -connect %h:%p -no_ssl2 -ign_eof")
 	  (set :inline t
 	       ;; FIXME: add brief `:tag "..."' descriptions.
 	       ;; (repeat :inline t :tag "Other" (string))
@@ -227,12 +233,15 @@ Fourth arg PORT is an integer specifying a port to connect to."
     (with-current-buffer buffer
       (message "Opening TLS connection to `%s'..." host)
       (while (and (not done) (setq cmd (pop cmds)))
-	(let ((process-connection-type tls-process-connection-type)
+        (let ((trustfiles (gnutls-trustfiles))
+	      (trustfile nil))
+          (while (and (not done) (setq trustfile (pop trustfiles)))
+        (let ((process-connection-type tls-process-connection-type)
 	      (formatted-cmd
 	       (format-spec
 		cmd
 		(format-spec-make
-                 ?t (car (gnutls-trustfiles))
+                 ?t trustfile
 		 ?h host
 		 ?p (if (integerp port)
 			(int-to-string port)
@@ -269,7 +278,7 @@ Fourth arg PORT is an integer specifying a port to connect to."
 	      (if start-of-data
 		  ;; move point to start of client data
 		  (goto-char start-of-data)))
-	    (setq done process))))
+	    (setq done process))))))
       (when (and done
 		 (or
 		  (and tls-checktrust
-- 
2.7.4 (Apple Git-66)


  parent reply	other threads:[~2016-07-02  7:09 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-12 21:32 bug#23759: 25.1.50; open-tls-stream creates malformed gnutls-cli command if trusted cert files don't exist Francis Litterio
2016-06-13  3:42 ` Eli Zaretskii
2016-06-13 10:18   ` Lars Ingebrigtsen
     [not found]     ` <CAGQpP8QFu3zx9_3SLf5tVRhGC7bV0hUiA8=OJm8HpA5H-hTfwA@mail.gmail.com>
     [not found]       ` <CAGQpP8QWYaxgE0=VGshhxDW=U3yT_kXsNq178m6zPGq15Ets9g@mail.gmail.com>
2016-06-13 11:33         ` Fran
2016-06-13 11:40           ` Lars Ingebrigtsen
2016-06-13 11:49             ` Fran
2016-06-13 14:03     ` Eli Zaretskii
2016-07-02  0:09 ` bug#23759: 25.1.50; Konstantin Kliakhandler
2016-07-02  7:09 ` Konstantin Kliakhandler [this message]
2016-07-05 14:36   ` bug#23759: 25.1.50; 25.1.50; open-tls-stream creates malformed gnutls-cli command if trusted cert files don't exist Ted Zlatanov
2016-07-05 14:49     ` Noam Postavsky
2016-07-05 16:54       ` Konstantin Kliakhandler
2016-07-05 17:59         ` Noam Postavsky
2016-07-05 21:17         ` Ted Zlatanov
2016-07-06 22:24           ` Richard Stallman
2016-07-07  3:31             ` Ted Zlatanov
2016-07-07  6:11               ` Konstantin Kliakhandler
2016-07-07 22:01                 ` Richard Stallman
2016-07-07 17:10           ` Ted Zlatanov
2016-07-07 22:40             ` Konstantin Kliakhandler
2016-07-08 13:43               ` Ted Zlatanov
2019-05-13 19:42 ` bug#23759: " Lars Ingebrigtsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAH+LVpk1DoqS5mdOX9-5YZ6RqntM+4+UQC9BPhwAZmAb=n6zWA@mail.gmail.com' \
    --to=kosta@slumpy.org \
    --cc=23759@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.