955721 - Login fails using znc without setting serverPassword

Status: RESOLVED FIXED

(Chat Core :: IRC, defect)

Description

[Instantbot]

*** Original post on bio 2270 by Alexis Metaireau <alexis+bugs AT mozilla.com> at 2013-12-09 14:54:00 UTC ***

When configuring instantbird for the first time, if you're using a znc bouncer, it's hard to figure out what's going wrong.

Part of the reason is that the options that has to be sent to the bouncer aren't exposed in the ui.

The current way to solve this is to go in the about:config section and enter

messenger.account.account1.options.username : username/network
messenger.account.account1.options.serverPassword : pass

Here is the message you get from the server:

[09/12/2013 15:36:57] DEBUG (@ prpl-irc resource://gre/components/irc.js:687)
{"rawMessage":":irc.znc.in 464 alexis :Password required","command":"464","params":["alexis","Password required"],"servername":"irc.znc.in"}
[09/12/2013 15:36:57] LOG (@ prpl-irc resource:///modules/socket.jsm:182)
Disconnect

Comment 1

[Patrick Cloke [:clokep]]

*** Original post on bio 2270 at 2013-12-09 15:01:40 UTC ***

We can probably send PASS in response to 464, but I don't know if the connection is still open at this point. (I'm not sure if that disconnect is from us or from the server.) We'd probably want to ensure we didn't already send PASS (since 464 is sent for both missing and wrong passwords).

Comment 2

[Instantbot]

*** Original post on bio 2270 by qlum <qlumreg AT gmail.com> at 2013-12-25 16:24:17 UTC ***

I can confirm the situation above, here is a debug log of me trying to connect using the method described on the znc wiki:http://wiki.znc.in/Pidgin to connect with pidgin: http://pastebin.mozilla.org/3860411

and this is what happens after using the fix described in the first post: http://pastebin.mozilla.org/3860413

Comment 3

[Patrick Cloke [:clokep]]

Fallen gave me a ZNC account, so I'll try to reproduce this tonight.

Comment 4

[Patrick Cloke [:clokep]]

My guess is that https://github.com/znc/znc/commit/981963a41e8b47f8a4f4ed16b5d8d90ae0626f54 is the commit that broke us. Still investigating though.

Comment 5

[Patrick Cloke [:clokep]]

Attachment: Debug log

In my debug logs the account was disconnected before the NOTICE was received since we actually force Instantbird to disconnect when receiving a 464 call [1]. You can see this in the stack of the disconnect:
> Socket.disconnect@resource:///modules/socket.jsm:185:1
> ircSocket.prototype.disconnect@file:///c:/Users/clokep/comm-central/obj-i686-pc-mingw32/mozilla/dist/bin/components/irc.js:707:5
> ircAccount.prototype.gotDisconnected@file:///c:/Users/clokep/comm-central/obj-i686-pc-mingw32/mozilla/dist/bin/components/irc.js:1651:5
> ircBase.commands[464]@resource:///modules/ircBase.jsm:1282:1
> ircHandlers._handleMessage@resource:///modules/ircHandlers.jsm:102:1
> ircHandlers.handleMessage@resource:///modules/ircHandlers.jsm:117:1
> ircSocket.prototype.onDataReceived@file:///c:/Users/clokep/comm-central/obj-i686-pc-mingw32/mozilla/dist/bin/components/irc.js:688:11
> Socket._handleQueue@resource:///modules/socket.jsm:463:7
> Socket._activateQueue@resource:///modules/socket.jsm:457:5
> Socket.onDataAvailable@resource:///modules/socket.jsm:447:7

I've attached a debug log to help with review.

[1] http://mxr.mozilla.org/comm-central/source/chat/protocols/irc/ircBase.jsm#1279

Comment 6

[Patrick Cloke [:clokep]]

Attachment: Parse 464 not NOTICE

This switches from parsing the NOTICE call to handling the 464 call.

Comment 7

[Patrick Cloke [:clokep]]

Attachment: Debug log with patch

This is the debug log with the patch applied. You can see we successfully connect (001). I find it odd that we receive two 464 calls and thus send the PASS command twice. But I'd like to keep the ZNC hacks to a minimum so I'm going to go with "this is OK" unless aleth or Florian greatly disagree.

I have a feeling SendRequiredPasswordNotice() is being called twice [1][2].

By the way, it seems that ZNC supports SASL. This would be a "safer" mechanism to use than the PASS command. [3] (We currently only support PLAIN, but if someone points me to an IRC server that supports other mechanisms and files a new bug I'll happily add them!)

[1] https://github.com/znc/znc/blob/a8e7b908c0225d3bcc398b51547dee060cde8d4b/src/Client.cpp#L852
[2] https://github.com/znc/znc/blob/a8e7b908c0225d3bcc398b51547dee060cde8d4b/src/Client.cpp#L157
[3] http://wiki.znc.in/Sasl

Comment 8

[Florian Quèze [:florian]]

(In reply to Patrick Cloke [:clokep] from comment #7)

> But I'd like to keep the ZNC hacks to a minimum so I'm
> going to go with "this is OK" unless aleth or Florian greatly disagree.

I don't understand much of the ZNC-specific situation so I'll trust you and aleth, but the one question I had when reading quickly here is: it seems a change in ZNC caused our code to no longer work, and you are adapting our code to work again with newer ZNC versions. Is this at the cost of breaking compatibility with older ZNCs, or have you found a way that works for both old and new versions of ZNC?

Thanks for debugging this!

Comment 9

[Patrick Cloke [:clokep]]

(In reply to Florian Quèze [:florian] [:flo] from comment #8)
> but the one question I had when reading quickly here is: it seems a
> change in ZNC caused our code to no longer work, and you are adapting our
> code to work again with newer ZNC versions. Is this at the cost of breaking
> compatibility with older ZNCs, or have you found a way that works for both
> old and new versions of ZNC?

I thought about this a bit and just assumed we had been ignoring the 464...but now that I think about it more, that doesn't make sense. I went back through the ZNC logs a bit more and found a commit [1] which is clearly what was breaking us. They added a 464 response about 2 years ago.

This patch would NOT work with versions of ZNC before that, I'll put up a new patch.

[1] https://github.com/znc/znc/commit/283fe7a72b6d6f081063fcfe5d4c597729ecaa35

Comment 10

[Patrick Cloke [:clokep]]

Attachment: Ignore 464 if from znc and "Password required" message

This is a more backwards compatible version which eats the new 464 message and lets the old NOTICE handler do what it always has. Note the extra check for "Password required" is because if you type in the WRONG password the same numeric is used, but the message is "Invalid Password". These exact wordings will be specific to ZNC, but that's OK since there's a check if it's from ZNC already.

Comment 11

[aleth [:aleth]]

Comment on attachment 8389772 [details] [diff] [review]
Ignore 464 if from znc and "Password required" message

Review of attachment 8389772 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good! r+ with the expanded comment.

::: chat/protocols/irc/ircNonStandard.jsm
@@ +121,5 @@
>  
> +    "464": function(aMessage) {
> +      // :Password required
> +      // If we receive a ZNC error message requesting a password, eat it since
> +      // a NOTICE AUTH will follow causing us to send the password.

Please add something like "the extra check for "Password required" is because if you type in the WRONG password the same numeric is used, but the message is "Invalid Password", in which case the standard 464 handler will disconnect." to avoid confusion in the future ;)

Comment 12

[Patrick Cloke [:clokep]]

Attachment: Ignore 464 if from znc and "Password required" message v2

Updated comment.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/ebe146fc7ac2