irc icon indicating copy to clipboard operation
irc copied to clipboard

real_nickname is not properly set when connecting to ZNC bouncers

Open sbraz opened this issue 6 years ago • 12 comments

Hi, I noticed that real_nickname is always wrong when I connect to ZNC bouncers:

#!/usr/bin/env python3

import ssl
import logging
import sys

import irc.connection
import irc.client

def on_welcome(c, e):
    print(c.get_nickname())

if __name__ == "__main__":
    #logging.basicConfig(level=logging.DEBUG)
    ssl_factory = irc.connection.Factory(wrapper=ssl.wrap_socket)
    reactor = irc.client.Reactor()
    reactor.server().connect(
            server,
            port,
            "", # nick doesn't matter
            connect_factory=ssl_factory,
            password=password,
    )
    reactor.add_global_handler("welcome", on_welcome)
    reactor.process_forever()

This will print Welcome to the freenode Internet Relay Chat Network sbraz. The debug log shows the following:

DEBUG:irc.client:FROM SERVER: :barjavel.freenode.net 001 :Welcome to the freenode Internet Relay Chat Network sbraz                                                                                                

sbraz avatar Oct 04 '18 22:10 sbraz

It's unclear to me exactly what's going on.

It seems to me that the client is expecting the argument to the 001 (welcome) command to be the real nickname. The code is here.

So I guess the questions are - What is the spec for 001? What does that imply for irc.client? Even if ZNC is violating the spec, should the client account for this deviation?

Can you do more investigation into what the IRC specs say about the 001 Welcome message?

jaraco avatar Oct 23 '18 16:10 jaraco

ZNC version? debug logs from ZNC?

DarthGandalf avatar Oct 23 '18 18:10 DarthGandalf

I looked at rfc2812 and it contains the following example:

   001    RPL_WELCOME
         "Welcome to the Internet Relay Network
          <nick>!<user>@<host>"

@DarthGandalf I'm running 1.7.1.

[2018-10-23 21:08:29.978562] (sbraz/freenode) ZNC -> CLI [:kornbluth.freenode.net 001 :Welcome to the freenode Internet Relay Chat Network sbraz]                                                                  

sbraz avatar Oct 23 '18 19:10 sbraz

Can you show the full logs from the moment when client connects? Obviously, replace the password with XXX

DarthGandalf avatar Oct 23 '18 19:10 DarthGandalf

I get it now, it's because I passed it an empty nickname.

[2018-10-23 21:08:29.836468] _LISTENER == ConnectionFrom(xx, xx) [Allowed]
[2018-10-23 21:08:29.836618] There are [0] clients from [xx]
[2018-10-23 21:08:29.909490] (xx) CLI -> ZNC [PASS sbraz/freenode:<censored>]
[2018-10-23 21:08:29.978129] (xx) CLI -> ZNC [NICK]
[2018-10-23 21:08:29.978370] (xx) CLI -> ZNC [USER  0 * :]
[2018-10-23 21:08:29.978562] (sbraz/freenode) ZNC -> CLI [:kornbluth.freenode.net 001 :Welcome to the freenode Internet Relay Chat Network sbraz]

If I use a non-empty nickname, I see it in the reply. However, it doesn't help the library find out what the real nickname is.

sbraz avatar Oct 23 '18 19:10 sbraz

IRC protocol doesn't allow empty parameters like that, CLI -> ZNC [USER 0 * :] is broken.

DarthGandalf avatar Oct 23 '18 19:10 DarthGandalf

A few more logs after setting my nickname to notreal:

DEBUG:irc.client:TO SERVER: NICK notreal
DEBUG:irc.client:TO SERVER: USER notreal 0 * :notreal
DEBUG:irc.client:process_forever(timeout=0.2)
DEBUG:irc.client:FROM SERVER: :kornbluth.freenode.net 001 notreal :Welcome to the freenode Internet Relay Chat Network sbraz
…
DEBUG:irc.client:FROM SERVER: :notreal!~sbraz@gentoo/developer/sbraz NICK :sbraz
DEBUG:irc.client:command: nick, source: notreal!~sbraz@gentoo/developer/sbraz, target: sbraz, arguments: [], tags: None

In this case, real_nickname is set to notreal, it looks like the client is missing a callback to change its value when we receive the nick change.

To summarize:

  • the client shouldn't allow an empty nickname (and maybe ZNC shouldn't either)
  • the client should update the nickname when it receives the NICK command

sbraz avatar Oct 23 '18 19:10 sbraz

After performing some more tests, I noticed that real_nickname is properly updated if I start the connection with a non-empty nickname: https://github.com/jaraco/irc/blob/dd16a701cb357cbd547dd631e7f942473a0dd897/irc/client.py#L288 The only remaining problem is that the client allows us to use an empty nick and this should be banned.

sbraz avatar Oct 26 '18 23:10 sbraz

the client allows us to use an empty nick and this should be banned.

I haven't looked into it, but if you can say with confidence that an empty nick is never valid, then I agree - the client should disallow invalid inputs.

jaraco avatar Mar 29 '19 15:03 jaraco

the client allows us to use an empty nick and this should be banned.

I haven't looked into it, but if you can say with confidence that an empty nick is never valid, then I agree - the client should disallow invalid inputs.

I don't see anything about an empty nick in the RFC so perhaps it's allowed: https://tools.ietf.org/html/rfc2812#section-3.1.2

@DarthGandalf what do you think?

sbraz avatar Apr 02 '19 08:04 sbraz

Hey, how do you imagine someone talking on IRC with an empty nick? RFC doesn't mention all possible corner cases, yes, but this is one where it's obvious what shouldn't be done.

The section you need here is https://tools.ietf.org/html/rfc1459#section-2.3.1 - it says that there can be multiple spaces between parameters, and it says that the last parameter can be empty.

Btw, the RFC you linked is not used much; the reality is somewhere between RFC 1459, and RFC 2812, closer to RFC 1459. And there is stuff coming from non-RFC, some of which is described at https://modern.ircdocs.horse/ or in https://ircv3.net

DarthGandalf avatar Apr 02 '19 20:04 DarthGandalf

So the conclusion is that empty nicks should be rejected? Can someone send a PR?

jaraco avatar Oct 27 '19 19:10 jaraco