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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Amin Bandali <bandali <at> gnu.org>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#47788: closed (Add support for TLS client certificates to
 'erc-tls')
Date: Fri, 23 Apr 2021 00:46:02 +0000

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

Your message dated Thu, 22 Apr 2021 20:45:29 -0400
with message-id <871rb1zv4m.fsf <at> gnu.org>
and subject line Re: Add support for TLS client certificates to 'erc-tls'
has caused the debbugs.gnu.org bug report #47788,
regarding Add support for TLS client certificates to 'erc-tls'
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
47788: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=47788
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Amin Bandali <bandali <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Cc: emacs-erc <at> gnu.org, jp <at> neverwas.me
Subject: Add support for TLS client certificates to 'erc-tls'
Date: Thu, 15 Apr 2021 00:16:38 -0400

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

Tags: patch

After a few recent changes in my IRC/ERC setup, I felt ERC's lack of
support for connecting to IRC networks using TLS client certificates as
an alternative to traditional password-based authentication.  So I spent
some time a few nights ago and wired things up in ERC to allow passing a
client certificate to `open-network-stream'.

Fellow ERC user 'neverwas' was kind enough to take my patch for a test
drive and provide feedback.  I have done some work on addressing their
feedback, but for now I'm attaching my very first draft here which I'd
sent to neverwas, and will attach the next version of my patch (along
with a proper commit message, NEWS entry, etc) shortly after neverwas's
reply with their feedback.

For anyone wanting to take this for a spin, the changes here should be
fully backward-compatible, and should not break anything in existing ERC
configurations.  To specify a client certificate, call `erc-tls' with
the new keywords :client-key and :client-crt with the file name of the
certificate key and certificate file itself respectively:

    (erc-tls :server "chat.freenode.net" :port 6697
             :client-key "~/my-freenode-cert.key"
             :client-crt "~/my-freenode-cert.crt")


In GNU Emacs 28.0.50 (build 43, x86_64-pc-linux-gnu, X toolkit, cairo version 1.15.10, Xaw3d scroll bars)
 of 2021-04-14 built on trisquel
Repository revision: 4ddad8f1db1f888f3365ac1330989dfbee605dd5
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description: Trisquel GNU/Linux Etiona (9.0)

Configured using:
 'configure --without-gconf --without-gsettings --without-gpm
 --with-x-toolkit=lucid --with-xaw3d --with-mailutils'

[0001-Add-support-for-TLS-client-certificates-to-erc-tls.patch (text/patch, attachment)]

[Message part 5 (message/rfc822, inline)]

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 6 (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.