Re: [RFC] certfp for rcirc

[Omar Polo <op@omarpolo.com>]

Philip Kaludercic <philipk@posteo.net> writes:

> Omar Polo <op@omarpolo.com> writes:
>
>> For some reason I don't know yet, the NickServ still says that I've got
>> 30 seconds to identify myself, but in reality I'm already logged in.  I
>> don't know basically anything about how the irc protocol works, so I'm
>> probably missing something incredibly obvious.
>
> Have you experienced any issues since? It might also be that this is a
> server side issue?  What do other clients say?

I've been happily using it for the last three day.  The "auto-magic"
login is nice and works reliably :D

I've only used circe with this setup, and iirc nickserv didn't send the
message (or maybe it was hidden.)  I read the circe code, but nothing
caught my eye really.  I'll try some other client to see what's the
output like.

>> What do you think?
>
> I think this would be a good addition.  One might even want to go
> further and add functions to automate the certfp authentication.  But
> that might be a too much for rcirc.
>
> Also, the manual should be updated to explain how this works.

I'll send an updated diff soon.  I've never touched an emacs manual, it
may take a bit to figure things out :)

>> Cheers,
>>
>> Omar Polo
>>
>>
>> diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
>> index 52d74a3394..070218ef0a 100644
>> --- a/lisp/net/rcirc.el
>> +++ b/lisp/net/rcirc.el
>> @@ -262,10 +262,12 @@ The ARGUMENTS for each METHOD symbol are:
>>    `bitlbee': NICK PASSWORD
>>    `quakenet': ACCOUNT PASSWORD
>>    `sasl': NICK PASSWORD
>> +  `certfp': KEY CERT
>>  
>>  Examples:
>>   ((\"Libera.Chat\" nickserv \"bob\" \"p455w0rd\")
>>    (\"Libera.Chat\" chanserv \"bob\" \"#bobland\" \"passwd99\")
>> +  (\"Libera.Chat\" certfp \"/path/to/key.pem\" \"/path/to/cert.pem\")
>>    (\"bitlbee\" bitlbee \"robert\" \"sekrit\")
>>    (\"dal.net\" nickserv \"bob\" \"sekrit\" \"NickServ@services.dal.net\")
>>    (\"quakenet.org\" quakenet \"bobby\" \"sekrit\")
>> @@ -291,7 +293,11 @@ Examples:
>>                                      (list :tag "SASL"
>>                                            (const sasl)
>>                                            (string :tag "Nick")
>> -                                          (string :tag "Password")))))
>> +                                          (string :tag "Password"))
>> +                                    (list :tag "CertFP"
>> +                                          (const certfp)
>> +                                          (string :tag "Key")
>> +                                          (string :tag "Certificate")))))
>>  
>>  (defcustom rcirc-auto-authenticate-flag t
>>    "Non-nil means automatically send authentication string to server.
>> @@ -547,6 +553,9 @@ If ARG is non-nil, instead prompt for connection parameters."
>>                (password (plist-get (cdr c) :password))
>>                (encryption (plist-get (cdr c) :encryption))
>>                (server-alias (plist-get (cdr c) :server-alias))
>> +              (client-cert (when (eq (rcirc-get-server-method (car c))
>> +                                     'certfp)
>> +                             (rcirc-get-server-cert (car c))))
>>                contact)
>>            (when-let (((not password))
>>                       (auth (auth-source-search :host server
>> @@ -563,7 +572,7 @@ If ARG is non-nil, instead prompt for connection parameters."
>>  		  (condition-case nil
>>  		      (let ((process (rcirc-connect server port nick user-name
>>                                                      full-name channels password encryption
>> -                                                    server-alias)))
>> +                                                    client-cert server-alias)))
>>                          (when rcirc-display-server-buffer
>>                            (pop-to-buffer-same-window (process-buffer process))))
>>  		    (quit (message "Quit connecting to %s"
>> @@ -662,13 +671,22 @@ See `rcirc-connect' for more details on these variables.")
>>  	(when (string-match server-i server)
>>            (throw 'pass (car args)))))))
>>  
>> +(defun rcirc-get-server-cert (server)
>> +  "Return a list of key and certificate for SERVER."
>> +  (catch 'pass
>> +    (dolist (i rcirc-authinfo)
>> +      (let ((server-i (car i))
>> +            (args (cddr i)))
>> +        (when (string-match server-i server)
>> +          (throw 'pass args))))))
>
> Why not use alist-get with a test function?

Agreed, and usually I would have written like that, but the other
function around did exactly that so for coherence I stick with that
pattern.

I can send a follow-up diff to improve
rcirc-get-server-{method,password,cert}.

>>  ;;;###autoload
>>  (defun rcirc-connect (server &optional port nick user-name
>>                               full-name startup-channels password encryption
>> -                             server-alias)
>> +                             certfp server-alias)
>>    "Connect to SERVER.
>>  The arguments PORT, NICK, USER-NAME, FULL-NAME, PASSWORD,
>> -ENCRYPTION, SERVER-ALIAS are interpreted as in
>> +ENCRYPTION, CERTFP, SERVER-ALIAS are interpreted as in
>>  `rcirc-server-alist'.  STARTUP-CHANNELS is a list of channels
>>  that are joined after authentication."
>>    (save-excursion
>> @@ -692,10 +710,16 @@ that are joined after authentication."
>>          (delete-process process))
>>  
>>        ;; Set up process
>> -      (setq process (open-network-stream
>> -                     (or server-alias server) nil server port-number
>> -                     :type (or encryption 'plain)
>> -                     :nowait t))
>> +      (setq process (if certfp
>> +                        (open-network-stream
>> +                         (or server-alias server) nil server port-number
>> +                         :type 'tls
>> +                         :nowait t
>> +                         :client-certificate certfp)
>
> Is this case-distinction necessary?  If `certfp' is nil, then
> open-network-stream should just ignore the argument if I am not
> mistaken.

(I think you meant `tls' rather then `certfp', implying that I could
simply have added `:client-certificate certfp' argument to
open-network-stream.)

It's an attempt to being user-friendly (the wrong way maybe), i.e. by
implicitly use tls if the user asks for certfp.

Now that I think it better, one has to set the correct port anyway so
maybe it's better to be less clever and require the user to specify
`:encryption tls' in rcirc-server-alist if certfp is requested.

>> +                      (open-network-stream
>> +                       (or server-alias server) nil server port-number
>> +                       :type (or encryption 'plain)
>> +                       :nowait t)))
>>        (set-process-coding-system process 'raw-text 'raw-text)
>>        (with-current-buffer (get-buffer-create (rcirc-generate-new-buffer-name process nil))
>>          (set-process-buffer process (current-buffer))
>>
>>

I'll send an improved diff with the manual bits later,

Thanks!