Closed Bug 1618061 Opened 4 years ago Closed 6 days ago

Don't enable SASL authentication by default for IRC

Categories

(Chat Core :: IRC, defect)

defect

Tracking

(thunderbird127 fixed)

RESOLVED FIXED
127 Branch
Tracking Status
thunderbird127 --- fixed

People

(Reporter: opal, Assigned: clokep)

References

()

Details

Attachments

(1 file)

Currently Thunderbird will, by default, attempt authenticating to IRC via SASL, using the user's configured username and password. This is fine assuming the Thunderbird user actually owns the username on IRC, but counterintuitive for users who may not be aware of this default behaviour, and may not have an account on IRC at all.

This manifests itself especially on freenode IRC network: users with common or short nicknames registered on freenode may get several "SASL authentication failed" messages a day from unwitting Thunderbird users. Specifically the messages may look like this (I just got this today, when identifying to my bot's NickServ account on freenode):

00:32.18 [Freenode2] -NickServ(NickServ@services.)- You are now identified for Nori.
00:32.18 [Freenode2] -NickServ(NickServ@services.)- 5 failed logins since last login.
00:32.18 [Freenode2] -NickServ(NickServ@services.)- Last failed attempt from: nori!~Thunderbi@110-131-22-187.rev.home.ne.jp on Feb 24 04:23:44 2020.

There is no way to disable these "failed attempt" messages on freenode, nor would I suggest a way to ignore it, because these are valid security alerts that serve useful for auditing. Thunderbird is the only client I've seen that produces noise at such a scale.

Suggested action: disable SASL authentication by default, and make it more obvious in the IRC configuration interface that it is not required to have a username and password set.

I'm not a Thunderbird user so I don't know the specific UI changes that this would entail, so maybe others can pick up the conversation from here and figure out a user-friendly way to deal with this.

Should this be considered a bug, or enhancement?

Flags: needinfo?(clokep)

I think we need to make it clearer that a password should only be provided for IRC accounts in some situations, I don't think disabling SASL authentication is the way to go.

Flags: needinfo?(clokep)

(In reply to Wayne Mery (:wsmwk) from comment #1)

Should this be considered a bug, or enhancement?

A bug, especially considering how this negatively impacts "random" users on freenode, and since, to my knowledge, this is a deficit of the current UI.

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

I don't think disabling SASL authentication is the way to go.

For those who have services accounts on freenode or other supported IRC networks, I agree that SASL is beneficial, but I don't think it should be suggested by default in the IRC connection configuration, because it creates confusion for those who do not know what SASL is and who think that IRC unconditionally requires a username and password to participate.

(In reply to opal hart from comment #4)

I don't think it should be suggested by default in the IRC connection configuration, because it creates confusion for those who do not know what SASL is and who think that IRC unconditionally requires a username and password to participate.

I think this matches what I suggested in comment 2.

Just to be clear, the current behaviour of Thunderbird makes it appear to be an attack tool to people who don't even use it.

The problem isn't that "SASL is enabled by default"; rather the problem is that by default it attempts to connect and authenticate using what amount to fake credentials.

We as the victims of these attacks cannot even send the Thunderbird user concerned a message to ask them to stop, either because Thunderbird disconnects after the failure (rather than falling back to an unauthenticated session), or because there is nothing to draw such messages to the user's attention.

There are jurisdictions where "unauthorised access to a computer system" is punishable by years of jail time, and the onus would be on the Thunderbird user to prove that they didn't know this was happening on their behalf, since clearly they took the action to configure the credentials.

THIS MUST STOP.

If disabling "SASL by default" is not an option, then some other action must be taken to curtail these "attacks". (I'm not exaggerating; it really does feel like an attack when some unknown random person tries to hijack your IRC account.)

The user must be shown an alert when the login attempt fails, without the option to avoid seeing the alert in future.

Automatic IRC login must be disabled after a reasonable number of consecutive failures, and not reenabled until the user chooses to. It must not be reenabled simply as a side effect of shutting down and restarting Thunderbird.

Flags: needinfo?(vseerror)

(In reply to Martin K from comment #6)

rather the problem is that by default it attempts to connect and authenticate using what amount to fake credentials.

Thunderbird attempts to login using the provided credentials, unfortunately the password page when configuring an account doesn't really explain that a user might not have an IRC account (and doesn't need one).

either because Thunderbird disconnects after the failure (rather than falling back to an unauthenticated session), or because there is nothing to draw such messages to the user's attention.

It falls back to a different nickname, as is common for IRC clients.

There is two slightly different situations that can be happening:

  1. The attempted nickname is currently in use by the owner.
  2. The attempted nickname is not currently in use by the owner.

The main difference with these is that in scenario 1, Thunderbird will choose another nick, while in scenario 2 it will login and then forcibly have its nick changed. (How exactly that happens likely varies by the IRC services in use.)

A few things that are odd about this:

  1. In scenario 1, we still try to login in, even if the second nickname is a Thunderbird generated one (not one of the ones configured by the user). This seems incorrect.
  2. The failure (code 904) isn't surface to the user (I'm not sure if that decision was thought through or if there just isn't a good way to let the user know something happened? You do still get logged in, even though your authentication failed, so it doesn't seem great to disconnect the account, but maybe that's what we should do.)
  3. We still try an IDENTIFY command even though the login failed with SASL. This seems weird and I have no idea why we would have done that (again, maybe it is just due to decoupled code). This shouldn't cause any further issues, I don't think. (Although could count as an additional failed login attempt?)
Flags: needinfo?(vseerror)
Severity: normal → S3

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

  1. In scenario 1, we still try to login in, even if the second nickname is a Thunderbird generated one (not one of the ones configured by the user). This seems incorrect.

I think this is probably correct behavior here. For people legitimately using Thunderbird as their IRC client, they would likely wish to be logged into their account regardless of which fallback nickname is used (perhaps due to a flapping connection to the IRC network). Having a user fill out login details but having that be ignored sometimes seems like it is rife for surprises, some of which are detailed below.

  1. The failure (code 904) isn't surface to the user (I'm not sure if that decision was thought through or if there just isn't a good way to let the user know something happened? You do still get logged in, even though your authentication failed, so it doesn't seem great to disconnect the account, but maybe that's what we should do.)
  2. We still try an IDENTIFY command even though the login failed with SASL. This seems weird and I have no idea why we would have done that (again, maybe it is just due to decoupled code). This shouldn't cause any further issues, I don't think. (Although could count as an additional failed login attempt?)

I would suggest aborting the connection on SASL failure, for a variety of reasons:

  1. A potential privacy concern where you may have a spoofed vHost while logged in, but your real IP is exposed while logged out
  2. Certain channels on your autojoin may require you to be logged in before you are able to join them, so continuing despite login failure means part of your autojoin list just breaks on you
  3. If we know SASL fails, sending IDENTIFY to NickServ isn't going to work particularly well either. Aborting on SASL failure is a very easy way of preventing this dual-attempt logic without needing to couple different parts of the code.

All of the above is largely tangential to this particular bug though, as I think it mostly boils down to an area for UX improvement with how IRC credentials are entered. I'm not sure what the right play for that is, but some ideas I had include a field of some sort where the user positively confirms that they have an IRC account (dropdown for authentication mechanism, checkbox to enable the fields, something else?), a note that IRC accounts are optional and are not the same as the user's email account credentials, or a connection test button that attempts the connection behind-the-scenes and flags an error if it can't connect or authentication fails. While the test button would still lead to the failed logins, it would be more of a one-time thing and less of a repeated thing.

Thanks for your thoughtful response!

I would suggest aborting the connection on SASL failure, for a variety of reasons:

I think the reason the code originally did this was that SASL login wasn't super common when this code was originally written, perhaps it is common enough now that it shouldn't fallback (or we should have an advance option, similar to XMPP, to try less-secure logins).

I think it mostly boils down to an area for UX improvement with how IRC credentials are entered.

I agree, the account creation flow is being reworked a bit (bug 1789966; bug 1731221 for the chat specific bits). It could make sense to consider this while doing that redesign.

Alex / Martin -- from a UI/UX POV do you have any thoughts on how we can make it obvious that IRC accounts don't need a password unless a user has registered with NickServ?

Flags: needinfo?(alessandro)

My preliminary thoughts for IRC were to have two separate optional password fields (with descriptive labels that contain more information than just "password"). Unlike the current flow there wouldn't be a separate step that makes you enter a password, so it would feel much more optional in general.

I would suggest aborting the connection on SASL failure

From https://ircv3.net/specs/extensions/sasl-3.2.html

If a client requires pre-authentication and is unable to obtain the sasl capability, then the client MUST disconnect and MAY retry the connection.

Aborting connection is required, not a choice.

My preliminary thoughts for IRC were to have two separate optional password fields

This worsens IRC clients' already inherently bad UX, I don't see what practical use an additional password field has.

from a UI/UX POV do you have any thoughts on how we can make it obvious that IRC accounts don't need a password unless a user has registered with NickServ?

It's a hard question to answer but I'd think the best way to sidestep this flaw in IRC's login flow would be to hide the password field behind a "this nickname requires authentication" checkbox so that it's more obvious to the user that the password field is not necessarily required to connect to most networks.

ugh, sorry for the comment spam...

Aborting connection is required, not a choice.

Reading that closer, it seems like it's conditionally required, so yes, your suggestion is entirely correct given that users would expect a login to succeed in order to ensure expected usage of the configured IRC network with their account and not have authentication be conditional and unpredictable. I'd go ahead and say it's de-facto required because of this, and the spec kind of ignores these practical issues around failed auth.

(In reply to opal hart from comment #11)

My preliminary thoughts for IRC were to have two separate optional password fields

This worsens IRC clients' already inherently bad UX, I don't see what practical use an additional password field has.

Ah, I haven't given enough context for this specific ticket. The second field would be for a potential server password, another issue with our current account setup flow.

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

Alex / Martin -- from a UI/UX POV do you have any thoughts on how we can make it obvious that IRC accounts don't need a password unless a user has registered with NickServ?

The first thing would be to add some descriptive text making it clear that IRC isn't part of email, and that the user should entirely skip this step if they don't already have an account on any IRC server.
Maybe ask a question "Do you have an existing IRC account that you want to use with Thunderbird? If you don't already have such an account you should skip this section. If you would like to try out IRC, please read (LINK TO EXPLANATION).". Unless they affirmatively answer "yes", all the other fields should be greyed out.

The second thing would be to make it clearer that the IRC server name isn't an arbitrary choice, but rather asking which service they already have established an account on. Make it default to "I don't have any IRC account"

I like the idea of testing the provided credentials when the user tries to save them. Care must be taken to understand that "current nick" and "username for SASL authentication" don't have to be the same; if SASL says you can't log in that's definitive that your chosen credentials are wrong, but if not using SASL then the user should be given the option to try a list of nicks, not just one, before declaring that the credentials are invalid.

I'm not an expert with IRC or any adjacent protocol, so apologies if I'm gonna write something stupid.

So, here's a bunch of silly questions:

  • Could we detect if an IRC server requires a password during initial setup?
  • Can a user log into an IRC server with or without a password? If I remember correctly, I could log into an IRC server and the authenticate myself later on, is that correct?

In general, I'm always for an approach where we only show things when needed, so if we can autodetect if the user needs to go through SASL auth or not, we should return different UI, alongside options to allow skipping or ignoring steps.

We will circle back to this in the coming months as we rebuild the chat account creation flow, so the more info the better.

Flags: needinfo?(alessandro)

(In reply to Alessandro Castellani [:aleca] from comment #17)

  • Could we detect if an IRC server requires a password during initial setup?

Not reliably. Passwords are only rarely required at connection time, and if they are, it'd fall into one of two categories:

  1. Getting into a privileged server or under a privileged connection (e.g. staff or a flood-exempt bot) may require a server password on connect. This is completely different from SASL and probably doesn't apply to the vast majority of users here. For the ones it does apply to, they'll know to set it up themselves.
  2. Connections from IP ranges prone to abuse may require that the user authenticate to an already-registered account via SASL. This is possible to detect, but there's a handful of other rejection reasons that indicate other requirements or that the user is simply banned. None of these reasons use standardized wording, which is why detection is not reliable here.

There may be other times where a password is required in order to join particular channels that the user is interested in, should those channels restrict themselves to registered users. This is not possible to detect reliably during initial setup either.

  • Can a user log into an IRC server with or without a password? If I remember correctly, I could log into an IRC server and the authenticate myself later on, is that correct?

Correct. Usually a password is not required in order to connect, and a user can authenticate later on. For users with accounts already, providing a SASL password during connection yields benefits mostly due to the avoidance of race conditions between authentication and automatically joining channels.

If someone is setting up IRC for the first time, they likely don't have an account or password on that particular network yet.

(In reply to moonmoon from comment #18)

If someone is setting up IRC for the first time, they likely don't have an account or password on that particular network yet.

Right, and so fooling them into reusing some other username and password from a different service on IRC nets seems like it's just going to share the user's login details with untrusted systems.

(In reply to opal hart from comment #12)

from a UI/UX POV do you have any thoughts on how we can make it obvious that IRC accounts don't need a password unless a user has registered with NickServ?

It's a hard question to answer but I'd think the best way to sidestep this flaw in IRC's login flow would be to hide the password field behind a "this nickname requires authentication" checkbox so that it's more obvious to the user that the password field is not necessarily required to connect to most networks.

This is just some food for thought for how we've overhauled the login process for our new IRCv3 plugin in Pidgin 3. Maybe it makes sense for Thunderbird, maybe it doesn't.

For protocols like IRC that have optional passwords we added an account option that the user can set saying the account requires a password . This was original done to determine if we need to lookup a password for an account from a password manager, but is also used by our new IRCv3 plugin to determine if we should attempt to use SASL for a given account.

With that setup done, we demoted the PASS command to it's own account option field. Private IRC servers can and do have a server password in addition to user accounts via sasl. We then only use the password field for SASL which is the much more common situation nowadays. If the server doesn't advertise sasl in CAP LS and doesn't CAP ACK a CAP REQ sasl we just try to connect like normal changing nicks if there's a collision. But if we request sasl and the server has it, we hard fail on any failures or mechanism exhaustion.

We continue to get what I assume are people's email passwords submitted to IRC very regularly. Fortunately I have a solution to cut down on a significant amount of the noise generated by Thunderbird!

Once as password is affirmatively rejected by SASL with an affirmative ERR_SASLFAIL(904) message from the server, that password should be removed from the user's settings. It's not going to become more valid in 15 minute with the user's client reconnects. Once the user has an account they can install the new, unique password they selected for their IRC account!

If thunderbird was previously reconnecting on failed authentication (instead of ignoring that and proceeding unauthenticated) it should set the account to not automatically reconnect at this point.

There are cases where SASL can time out but these won't result in services sending fail, so the credentials won't get discarded by this strategy due to simple downtimes.

Assignee: nobody → clokep
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Component: Instant Messaging → IRC
Product: Thunderbird → Chat Core
Version: unspecified → trunk

Pushed by martin@humanoids.be:
https://hg.mozilla.org/comm-central/rev/c13f5c8d2501
Error if SASL results in an authentication error. r=freaktechnik

Status: ASSIGNED → RESOLVED
Closed: 6 days ago
Resolution: --- → FIXED
Target Milestone: --- → 127 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: