Subject: bug#23759: 25.1.50; 25.1.50; open-tls-stream creates malformed gnutls-cli command if trusted cert files don't exist

From: Ted Zlatanov <[email-masked]>
Date: Fri, 08 Jul 2016 09:43:35 -0400

On Fri, 8 Jul 2016 01:40:13 +0300 Konstantin Kliakhandler <[email-masked]> wrote: 

T> Perhaps there can be a way to say
T> "if this %t is empty, remove the preceding --argument as well"
T> in the format string? That would simplify the whole thing, like so:

T> "gnutls-cli --x509cafile %T -p %p %h"

T> ...becomes "gnutls-cli -p PORT HOST"
T> when the %T parameter is nil. Just an idea...

KK> I toyed with this idea, and even implemented something of the sort, but
KK> from a bit different different direction - I added another replacement
KK> variable - %c - and made the list tls-program now contain pairs with
KK> (string . value-of-c), e.g. ("gnutls-cli %c %T -p %p %h" . "--x509cafile")
KK> have both %c and %T replaced (together) as appropriate.

KK> The problem with this approach is, what about people who customized this
KK> setting? So, I made it backward compatible with the old standard.
KK> Eventually however, it turned into a an ugly big mess due to the backward
KK> compatability and I decided against submitting.

KK> There is a similar problem of backward compatibility in your approach -
KK> what if someone customized it in such a way that wasn't expecting an
KK> argument to be removed, and it would create a vulnerability in their setup?
KK> I also don't see a simple way to do it nicely, but have no objections on
KK> those grounds, of course.

Hmm, right, yeah... well %t is always preceded by an argument, right? So
maybe the backwards-compatible solution is that if %t is nil, delete the
preceding option?

Another option is to throw an error when %t is nil, explaining what
happened and how to fix it. That's not a terrible inconvenience for the
user, compared to running an insecure connection unknowingly. I slightly
prefer this.

KK> Finally, I would do the patch but am uncertain whether it would be better
KK> to wait for your results from emacs-devel and remove the ssl3 bit as well
KK> (or just go ahead and do it). Let me know and I'll send the appropriate
KK> patch.

Go ahead and remove it, we have agreement that it's a Bad Thing. Thank you!

Ted