GNU bug report logs - #47788
Add support for TLS client certificates to 'erc-tls'

Previous Next

Full log

Message #22 received at 47788-done <at> debbugs.gnu.org (full text, mbox):

From: Amin Bandali <bandali <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>
Cc: Robert Pluim <rpluim <at> gmail.com>, 47788-done <at> debbugs.gnu.org,
 emacs-erc <at> gnu.org
Subject: Re: Add support for TLS client certificates to 'erc-tls'
Date: Thu, 22 Apr 2021 20:45:29 -0400

[Message part 1 (text/plain, inline)]

J.P. writes:

> Amin Bandali <bandali <at> gnu.org> writes:
>
>>> It reconnected successfully with no hiccups, so I think that's one
>>> for the win column.
>
> This continues to be the case with the latest changes applied; ditto
> when using the authinfo variant of the new :client-certificate param.

Great!

>>> Beyond that, users may appreciate a mention of the new additions in the
>>> info manual and maybe the wiki as well (instead of just NEWS).
>>
>> Certainly; done in v2.
>
> Nice job. This sort of thing can be a real time sink (at least for me).

Thanks, and agreed; but it's good to take the time and do it anyway. :-)

> One question. And stop me if I've confused myself here, but the doc
> string and the tex-info section for `erc-tls' both offer the same
> example (borrowed from their plain `erc' counterparts) in which the
> function `erc-compute-full-name' is said to be consulted despite the
> :full-name "Harry S. Truman" keyword arg being present. While it's
> (technically) true that `erc-compute-full-name' always runs, I suspect
> its inclusion in the original example was inadvertent (or something
> about it has changed since 2007). Would it perhaps make sense to omit
> `erc-compute-full-name' from the Truman example?

Yeah that's fair!  I've tried to clarify/remedy that in the latest
revision.

[...]
>
> Thanks, but I shouldn't have proposed that tweak without a concrete use
> case in mind. In fact, I've only ever needed :nowait to be nil when
> using a custom opener. If my suggestion made things more confusing or
> distracting, my apologies. Perhaps it's best to just revert to what you
> had originally or even just force it to t. (Not that you should waste
> another second on this relative nothingburger.)

No worries :-).  And per our short discussion on #erc the other day,
there could be a legitimate use-case for that.  So I think it's safe to
say that all in all it was a net positive change.

>> Thanks for the suggestion.  After some thought, I decided to change the
>> interface of erc-tls from the two separate :client-key and :client-crt
>> arguments in v1 of the patch to just one :client-certificate in v2,
>> matching that of `open-network-stream'.  I tested the change with
>> `:client-certificate t' and confirm that it works fine for me.
>
> The interface change is a welcome improvement, IMO. Although dealing
> with a list may feel slightly more cumbersome to some, the multiple
> helpful examples make that a nonissue. (And sparing contributors an
> extra param to worry about is a nice bonus as well.) But I guess the
> real win is in accommodating the authinfo facility, which seems a rather
> obvious and essential inclusion in retrospect. Great work!
>

Awesome.  Yeah I couldn't think of a more straightforward and/or less
cumbersome way of doing it, except for using the same shape of argument
as expected by `open-network-stream'.  Per usual, I would be open to
reconsideration if a better way of doing it is suggested.

For now, I went ahead and finally installed the attached patch on the
master branch.  Thanks for all the feedback/suggestions. :-)

[0001-Add-support-for-using-a-TLS-client-certificate-with-.patch (text/x-diff, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.