New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISON sent too early when using SASL #596

Closed
ailin-nemui opened this Issue Jan 4, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@ailin-nemui
Contributor

ailin-nemui commented Jan 4, 2017

>> :bear.freenode.net CAP * LS :account-notify extended-join identify-msg multi-prefix sasl
<< CAP REQ :multi-prefix sasl
>> :bear.freenode.net 433 * vague :Nickname is already in use.
<< NICK vague_
>> :bear.freenode.net CAP * ACK :multi-prefix sasl
<< AUTHENTICATE PLAIN
<< ISON :vague
>> AUTHENTICATE +
<< AUTHENTICATE xxxxxxxxxxxxxxxxxxxxxxxxx
>> :bear.freenode.net 451 * :You have not registered
>> :bear.freenode.net 900 vague_ vague_!vague@xxx.xxx.xxx vague :You are now logged in as vague.
>> :bear.freenode.net 903 vague_ :SASL authentication successful

reported by @vague666

@ailin-nemui ailin-nemui changed the title from Order of commands inside CAP not respected to Order of commands inside CAP AUTHENTICATE not respected Jan 4, 2017

@ailin-nemui ailin-nemui added the bug label Jan 4, 2017

@dequis

This comment has been minimized.

Member

dequis commented Jan 4, 2017

Wait, who sends ISON? I would have thought that's the event_connected of src/fe-common/irc/fe-events.c trying to find who has the old nick, but that one sends WHOIS. So apparently it's just src/irc/notifylist/notify-ison.c firing on a timeout. Is that the whole issue or am i missing something?

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Jan 4, 2017

Maybe you found the cause

@LemonBoy

This comment has been minimized.

Member

LemonBoy commented Jan 4, 2017

Maybe checking server->connected before sending checking the notifylist is enough.

@dequis

This comment has been minimized.

Member

dequis commented Jan 6, 2017

Turns out that ISON isn't normally sent during connection registration because src/irc/core/irc-servers.c:270

g_time_val_add(&server->wait_cmd, 120 * G_USEC_PER_SEC);

Then event_connected resets that to 0. And (almost) any call to irc_send_cmd_now will also reset it, like the ones we do during sasl.

src/irc/core/irc-servers.c:522

522     void irc_server_send_data(IRC_SERVER_REC *server, const char *data, int len)
523     {
524             if (net_sendbuffer_send(server->handle, data, len) == -1) {
525                     /* something bad happened */
526                     server->connection_lost = TRUE;
527                     return;
528             }
529
530             g_get_current_time(&server->last_cmd);
531
532             /* A bit kludgy way to do the flood protection. In ircnet, there
533                actually is 1sec / 100 bytes penalty, but we rather want to deal
534                with the max. 1000 bytes input buffer problem. If we send more
535                than that with the burst, we'll get excess flooded. */
536             if (len < 100 || server->cmd_queue_speed <= 10)
537                     server->wait_cmd.tv_sec = 0;
538             else {
539                     memcpy(&server->wait_cmd, &server->last_cmd, sizeof(GTimeVal));
540                     g_time_val_add(&server->wait_cmd, (2 + len/100) * G_USEC_PER_SEC);
541             }
542     }

I think i'm just adding a server->connected check before g_get_current_time() to avoid resetting any timers during connection registration.

@dequis

This comment has been minimized.

Member

dequis commented Jan 10, 2017

Reverted #605 because of #611

@dequis dequis changed the title from Order of commands inside CAP AUTHENTICATE not respected to ISON sent too early when using SASL Feb 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment