From 4bcef46eca3083a48d32f55de55fa24337cea904 Mon Sep 17 00:00:00 2001 From: Ed Kellett Date: Sat, 6 Apr 2019 01:34:08 +0100 Subject: [PATCH 1/6] Add UFNC --- include/s_serv.h | 46 +++++++++++---------- modules/core/m_nick.c | 95 +++++++++++++++++++++++++++++++++++++++---- src/s_serv.c | 1 + 3 files changed, 113 insertions(+), 29 deletions(-) diff --git a/include/s_serv.h b/include/s_serv.h index 80b7e3f6b..86493a355 100644 --- a/include/s_serv.h +++ b/include/s_serv.h @@ -54,34 +54,36 @@ struct Capability unsigned int required; /* 1 if required, 0 if not */ }; -#define CAP_CAP 0x000001 /* received a CAP to begin with */ -#define CAP_QS 0x000002 /* Can handle quit storm removal */ -#define CAP_EX 0x000004 /* Can do channel +e exemptions */ -#define CAP_CHW 0x000008 /* Can do channel wall @# */ -#define CAP_IE 0x000010 /* Can do invite exceptions */ -#define CAP_KLN 0x000040 /* Can do KLINE message */ -#define CAP_ZIP 0x000100 /* Can do ZIPlinks */ -#define CAP_KNOCK 0x000400 /* supports KNOCK */ -#define CAP_TB 0x000800 /* supports TBURST */ -#define CAP_UNKLN 0x001000 /* supports remote unkline */ -#define CAP_CLUSTER 0x002000 /* supports cluster stuff */ -#define CAP_ENCAP 0x004000 /* supports ENCAP */ -#define CAP_TS6 0x008000 /* supports TS6 or above */ -#define CAP_SERVICE 0x010000 -#define CAP_RSFNC 0x020000 /* rserv FNC */ -#define CAP_SAVE 0x040000 /* supports SAVE (nick collision FNC) */ -#define CAP_EUID 0x080000 /* supports EUID (ext UID + nonencap CHGHOST) */ -#define CAP_REMOVE 0x100000 /* supports REMOVE */ -#define CAP_EOPMOD 0x200000 /* supports EOPMOD (ext +z + ext topic) */ -#define CAP_BAN 0x400000 /* supports propagated bans */ -#define CAP_MLOCK 0x800000 /* supports MLOCK messages */ +#define CAP_CAP 0x0000001 /* received a CAP to begin with */ +#define CAP_QS 0x0000002 /* Can handle quit storm removal */ +#define CAP_EX 0x0000004 /* Can do channel +e exemptions */ +#define CAP_CHW 0x0000008 /* Can do channel wall @# */ +#define CAP_IE 0x0000010 /* Can do invite exceptions */ +#define CAP_KLN 0x0000040 /* Can do KLINE message */ +#define CAP_ZIP 0x0000100 /* Can do ZIPlinks */ +#define CAP_KNOCK 0x0000400 /* supports KNOCK */ +#define CAP_TB 0x0000800 /* supports TBURST */ +#define CAP_UNKLN 0x0001000 /* supports remote unkline */ +#define CAP_CLUSTER 0x0002000 /* supports cluster stuff */ +#define CAP_ENCAP 0x0004000 /* supports ENCAP */ +#define CAP_TS6 0x0008000 /* supports TS6 or above */ +#define CAP_SERVICE 0x0010000 +#define CAP_RSFNC 0x0020000 /* rserv FNC */ +#define CAP_SAVE 0x0040000 /* supports SAVE (nick collision FNC) */ +#define CAP_EUID 0x0080000 /* supports EUID (ext UID + nonencap CHGHOST) */ +#define CAP_REMOVE 0x0100000 /* supports REMOVE */ +#define CAP_EOPMOD 0x0200000 /* supports EOPMOD (ext +z + ext topic) */ +#define CAP_BAN 0x0400000 /* supports propagated bans */ +#define CAP_MLOCK 0x0800000 /* supports MLOCK messages */ +#define CAP_UFNC 0x1000000 /* supports UFNC messages */ #define CAP_MASK (CAP_QS | CAP_EX | CAP_CHW | \ CAP_IE | CAP_KLN | CAP_SERVICE |\ CAP_CLUSTER | CAP_ENCAP | \ CAP_ZIP | CAP_KNOCK | CAP_UNKLN | \ CAP_RSFNC | CAP_SAVE | CAP_EUID | \ - CAP_REMOVE | CAP_EOPMOD | CAP_BAN | CAP_MLOCK) + CAP_REMOVE | CAP_EOPMOD | CAP_BAN | \ + CAP_MLOCK | CAP_UFNC) #ifdef HAVE_LIBZ #define CAP_ZIP_SUPPORTED CAP_ZIP diff --git a/modules/core/m_nick.c b/modules/core/m_nick.c index effd2034d..15e375b6c 100644 --- a/modules/core/m_nick.c +++ b/modules/core/m_nick.c @@ -62,8 +62,10 @@ static int ms_nick(struct Client *, struct Client *, int, const char **); static int ms_uid(struct Client *, struct Client *, int, const char **); static int ms_euid(struct Client *, struct Client *, int, const char **); static int ms_save(struct Client *, struct Client *, int, const char **); +static int ms_ufnc(struct Client *, struct Client *, int, const char **); static int can_save(struct Client *); static void save_user(struct Client *, struct Client *, struct Client *); +static void fnc_user(struct Client *, struct Client *, struct Client *, const char *); static void bad_nickname(struct Client *, const char *); static int h_local_nick_change; @@ -85,9 +87,13 @@ struct Message save_msgtab = { "SAVE", 0, 0, 0, MFLG_SLOW, {mg_ignore, mg_ignore, mg_ignore, {ms_save, 3}, mg_ignore, mg_ignore} }; +struct Message ufnc_msgtab = { + "UFNC", 0, 0, 0, MFLG_SLOW, + {mg_ignore, mg_ignore, mg_ignore, {ms_ufnc, 4}, mg_ignore, mg_ignore} +}; mapi_clist_av1 nick_clist[] = { &nick_msgtab, &uid_msgtab, &euid_msgtab, - &save_msgtab, NULL }; + &save_msgtab, &ufnc_msgtab, NULL }; mapi_hlist_av1 nick_hlist[] = { { "local_nick_change", &h_local_nick_change }, @@ -249,7 +255,7 @@ m_nick(struct Client *client_p, struct Client *source_p, int parc, const char *p } /* mc_nick() - * + * * server -> server nick change * parv[1] = nickname * parv[2] = TS when nick change @@ -270,17 +276,22 @@ mc_nick(struct Client *client_p, struct Client *source_p, int parc, const char * newts = atol(parv[2]); target_p = find_named_client(parv[1]); + if (newts == source_p->tsinfo && target_p != source_p) + { + /* doesn't change the TS, so it's a case change. ignore it + * if the nick has changed materially */ + } /* if the nick doesnt exist, allow it and process like normal */ - if(target_p == NULL) + else if (target_p == NULL) { change_remote_nick(client_p, source_p, newts, parv[1], 1); } - else if(IsUnknown(target_p)) + else if (IsUnknown(target_p)) { exit_client(NULL, target_p, &me, "Overridden"); change_remote_nick(client_p, source_p, newts, parv[1], 1); } - else if(target_p == source_p) + else if (target_p == source_p) { /* client changing case of nick */ if(strcmp(target_p->name, parv[1])) @@ -546,11 +557,38 @@ ms_save(struct Client *client_p, struct Client *source_p, int parc, const char * return 0; } +/* ms_ufnc() + * parv[1] - UID + * parv[2] - newnick + * parv[3] - TS + */ +static int +ms_ufnc(struct Client *client_p, struct Client *source_p, int parc, const char *parv[]) +{ + struct Client *target_p; + + target_p = find_id(parv[1]); + char *newnick = parv[2]; + if (target_p == NULL) + return 0; + if (!IsPerson(target_p)) + sendto_realops_snomask(SNO_GENERAL, L_NETWIDE, + "Ignored UFNC message for non-person %s from %s", + target_p->name, source_p->name); + else if (target_p->tsinfo == atol(parv[3])) + fnc_user(client_p, source_p, target_p, newnick); + else + sendto_realops_snomask(SNO_SKILL, L_NETWIDE, + "Ignored UFNC message for %s from %s", + target_p->name, source_p->name); + return 0; +} + /* clean_nick() * * input - nickname to check * output - 0 if erroneous, else 1 - * side effects - + * side effects - */ static int clean_nick(const char *nick, int loc_client) @@ -764,7 +802,7 @@ change_local_nick(struct Client *client_p, struct Client *source_p, monitor_signon(source_p); /* Make sure everyone that has this client on its accept list - * loses that reference. + * loses that reference. */ /* we used to call del_all_accepts() here, but theres no real reason * to clear a clients own list of accepted clients. So just remove @@ -1265,6 +1303,49 @@ save_user(struct Client *client_p, struct Client *source_p, change_remote_nick(target_p, target_p, SAVE_NICKTS, target_p->id, 0); } +static void +fnc_user(struct Client *client_p, struct Client *source_p, + struct Client *target_p, const char *newnick) +{ + struct Client *exist_p = find_named_client(newnick); + + if (exist_p) { + char buf[BUFSIZE]; + + if (exist_p == target_p) return; + + if(MyClient(exist_p)) + sendto_one(exist_p, ":%s KILL %s :(Nickname regained by services)", + me.name, exist_p->name); + + exist_p->flags |= FLAGS_KILLED; + /* Do not send kills to servers for unknowns -- jilles */ + if(IsClient(exist_p)) + kill_client_serv_butone(NULL, exist_p, + "%s (Nickname regained by services)", me.name); + + rb_snprintf(buf, sizeof(buf), "Killed (%s (Nickname regained by services))", + me.name); + exit_client(NULL, exist_p, &me, buf); + } + + sendto_server(client_p, NULL, CAP_UFNC|CAP_TS6, NOCAPS, ":%s UFNC %s %s %ld", + source_p->id, target_p->id, newnick, (long)target_p->tsinfo); + sendto_server(client_p, NULL, CAP_TS6, CAP_UFNC, ":%s NICK %s :%ld", + target_p->id, target_p->id, (long)target_p->tsinfo); + if (MyClient(target_p)) + { + time_t tsinfo = target_p->tsinfo; + change_local_nick(target_p, target_p, newnick, 0); + target_p->tsinfo = tsinfo; + } + else + { + change_remote_nick(target_p, target_p, target_p->tsinfo, newnick, 0); + } +} + + static void bad_nickname(struct Client *client_p, const char *nick) { char squitreason[100]; diff --git a/src/s_serv.c b/src/s_serv.c index e24f5cce1..7647c8ba3 100644 --- a/src/s_serv.c +++ b/src/s_serv.c @@ -91,6 +91,7 @@ struct Capability captab[] = { { "EOPMOD", CAP_EOPMOD }, { "BAN", CAP_BAN }, { "MLOCK", CAP_MLOCK }, + { "UFNC", CAP_UFNC }, {0, 0} }; From 22f42ed53fe033cb9dd091162157e5257d897bdb Mon Sep 17 00:00:00 2001 From: Ed Kellett Date: Sun, 7 Apr 2019 23:10:09 +0100 Subject: [PATCH 2/6] Enforce UFNC restrictions --- modules/core/m_nick.c | 42 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/modules/core/m_nick.c b/modules/core/m_nick.c index 15e375b6c..2863106e2 100644 --- a/modules/core/m_nick.c +++ b/modules/core/m_nick.c @@ -67,6 +67,7 @@ static int can_save(struct Client *); static void save_user(struct Client *, struct Client *, struct Client *); static void fnc_user(struct Client *, struct Client *, struct Client *, const char *); static void bad_nickname(struct Client *, const char *); +static void bad_fnc(struct Client *, const char *); static int h_local_nick_change; static int h_remote_nick_change; @@ -1308,6 +1309,9 @@ fnc_user(struct Client *client_p, struct Client *source_p, struct Client *target_p, const char *newnick) { struct Client *exist_p = find_named_client(newnick); + struct Client *server_p = target_p->servptr; + + char squit_reason[300]; if (exist_p) { char buf[BUFSIZE]; @@ -1329,18 +1333,35 @@ fnc_user(struct Client *client_p, struct Client *source_p, exit_client(NULL, exist_p, &me, buf); } + if (!(source_p->flags & FLAGS_SERVICE)) { + snprintf(squit_reason, sizeof squit_reason, + "Invalid UFNC (%s -> %s) from %s (source server lacks U:line)", + target_p->name, newnick, + source_p->name); + bad_fnc(client_p, squit_reason); + return 0; + } + + for (; server_p && server_p != &me; server_p = server_p->servptr) { + if (server_p->serv->caps & CAP_UFNC) + continue; + snprintf(squit_reason, sizeof squit_reason, + "Invalid UFNC (%s -> %s) from %s (server %s on path lacks support)", + target_p->name, newnick, + source_p->name, server_p->name); + bad_fnc(client_p, squit_reason); + return 0; + } + sendto_server(client_p, NULL, CAP_UFNC|CAP_TS6, NOCAPS, ":%s UFNC %s %s %ld", source_p->id, target_p->id, newnick, (long)target_p->tsinfo); sendto_server(client_p, NULL, CAP_TS6, CAP_UFNC, ":%s NICK %s :%ld", target_p->id, target_p->id, (long)target_p->tsinfo); - if (MyClient(target_p)) - { + if (MyClient(target_p)) { time_t tsinfo = target_p->tsinfo; change_local_nick(target_p, target_p, newnick, 0); target_p->tsinfo = tsinfo; - } - else - { + } else { change_remote_nick(target_p, target_p, target_p->tsinfo, newnick, 0); } } @@ -1359,3 +1380,14 @@ static void bad_nickname(struct Client *client_p, const char *nick) "Bad nickname introduced [%s]", nick); exit_client(client_p, client_p, &me, squitreason); } + + +static void bad_fnc(struct Client *client_p, const char *reason) +{ + sendto_realops_snomask(SNO_GENERAL, L_NETWIDE, "Squitting %s: %s", + client_p->name, reason); + ilog(L_SERVER, "Link %s cancelled: %s", + client_p->name, reason); + + exit_client(client_p, client_p, &me, reason); +} From 134eebdfa8cb4b7964e4a6f1766e871bcce7a002 Mon Sep 17 00:00:00 2001 From: Ed Kellett Date: Sun, 7 Apr 2019 23:56:26 +0100 Subject: [PATCH 3/6] Add ufnc.txt --- doc/technical/ufnc.txt | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 doc/technical/ufnc.txt diff --git a/doc/technical/ufnc.txt b/doc/technical/ufnc.txt new file mode 100644 index 000000000..49d84d420 --- /dev/null +++ b/doc/technical/ufnc.txt @@ -0,0 +1,81 @@ +UFNC: FNC as a real nick change +=============================== + +Motivation +---------- + +seven, like charybdis, uses the RSFNC mechanism to enable services to change +user nicknames. RSFNC looks like this: + + ENCAP victims-server RSFNC victim-uid newnick oldTS newTS + +and doesn't do anything by itself: it just asks victim's server to perform the +equivalent of a /nick. + +This causes a race condition when RSFNCs depend on each other. Starting with +001TARGET on the nick 'foobar', consider a typical services REGAIN: + + ENCAP serv1 RSFNC 001TARGET Guest12345 1000 1234 + ENCAP serv2 RSFNC 002TARGET foobar 1001 1234 + +serv1 will receive the first RSFNC, changing 001TARGET's nick to Guest12345. +However, if it doesn't manage to do that and send a NICK to serv2 before serv2 +receives the second RSFNC, serv2 will change 002TARGET's nick to foobar while +still seeing 001TARGET's nick as foobar. The RSFNC spec requires that in this +case 001TARGET is killed. + +The kill could be avoided by issuing a SAVE, or by saving the first target +in the first place instead of RSFNC-ing them. There is another solution, +however: instead of _requesting_ a nick change, have services unilaterally +_propagate_ a nick change. As commands from any given server cannot be +reordered, the race condition is avoided. + + +UFNC +------ + +Our addition is as follows: + + UFNC targetUID newnick nickTS + +UFNC should only be issued by a services server. However, if attempts to enforce +this are implemented, they must account for the fact that the nick change has +already propagated; SAVE, SQUIT or KILL would be okay, while just ignoring the +UFNC would not. + +UFNC must be silently ignored if nickTS does not match the target's TS. +Otherwise, a server receiving a UFNC changes the target's nick to newnick, +leaving its TS unchanged. It is propagated as a UFNC to servers that support it, +or as a NICK to servers that do not. + +If newnick already exists, its existing owner is killed, regardless of their TS, +as with RSFNC. + +We introduce a new server capability, also named UFNC, representing the ability +of a server to process and propagate UFNC commands. Servers issuing UFNC must +ensure their entire path to the vicitm's server has UFNC support. Servers +receiving a UFNC message may enforce this. + + +Desync resistance +----------------- + +UFNC is not designed to handle the case where two different U-lined servers send +conflicting UFNC messages. + +In other cases, we believe UFNC cannot lead to nick desyncs: + + - Normal nick changes, including those generated by RSFNC, are either case + changes or are guaranteed to change the nickTS, no matter how fast they + are sent. + + For regular nick changes which change the TS, a NICK racing with a UFNC will + override the UFNC it arrives second, and invalidate the UFNC if it + arrives first. + + A case change NICK racing with a UFNC will be ignored if it arrives second + (requiring a small modification to the NICK logic), and overridden if it + arrives first. + + - SAVEs always change the TS, so a SAVE racing with a UFNC will win everywhere + by the same logic as for regular NICK. From 6815e56652dbe4f2007bbf98ae0001b4f92f8c7e Mon Sep 17 00:00:00 2001 From: Ed Kellett Date: Mon, 15 Apr 2019 15:58:21 +0100 Subject: [PATCH 4/6] Disallow UFNC for saved nicks --- doc/technical/ufnc.txt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/technical/ufnc.txt b/doc/technical/ufnc.txt index 49d84d420..0af0707bf 100644 --- a/doc/technical/ufnc.txt +++ b/doc/technical/ufnc.txt @@ -43,6 +43,8 @@ this are implemented, they must account for the fact that the nick change has already propagated; SAVE, SQUIT or KILL would be okay, while just ignoring the UFNC would not. +UFNC may not be issued for a UID (saved) nick. + UFNC must be silently ignored if nickTS does not match the target's TS. Otherwise, a server receiving a UFNC changes the target's nick to newnick, leaving its TS unchanged. It is propagated as a UFNC to servers that support it, @@ -77,5 +79,8 @@ In other cases, we believe UFNC cannot lead to nick desyncs: (requiring a small modification to the NICK logic), and overridden if it arrives first. - - SAVEs always change the TS, so a SAVE racing with a UFNC will win everywhere - by the same logic as for regular NICK. + - Initial SAVEs change the TS, so a SAVE racing with a UFNC will win + everywhere by the same logic as for a regular NICK. + + A second SAVE does not change the TS, so a second SAVE racing with a UFNC + could desync; it is therefore prohibited to send a UFNC for a saved nick. From 88805e14d1a0b8d5dc433210a593aaa7bc92d091 Mon Sep 17 00:00:00 2001 From: Ed Kellett Date: Mon, 15 Apr 2019 16:03:47 +0100 Subject: [PATCH 5/6] Add argument for downgrade to NICK --- doc/technical/ufnc.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/technical/ufnc.txt b/doc/technical/ufnc.txt index 0af0707bf..be8c35903 100644 --- a/doc/technical/ufnc.txt +++ b/doc/technical/ufnc.txt @@ -84,3 +84,12 @@ In other cases, we believe UFNC cannot lead to nick desyncs: A second SAVE does not change the TS, so a second SAVE racing with a UFNC could desync; it is therefore prohibited to send a UFNC for a saved nick. + + - When a UFNC is propagated as a NICK, the downgrade to NICK is performed by a + server on the path between the UFNC originator and the owner of the nick. + Therefore, from the point of view of a server without UFNC support, all nick + changes for a given target pass through the downgrading server. If UFNC + without downgrade is safe from desyncs, the downgrading server must have a + consistent view of the target's nick, and since nick changes for the target + can only come from the downgrading server, the non-UFNC server must have the + same consistent view. From e0eacd0215a58cc1f3b96200a570febc512dd71b Mon Sep 17 00:00:00 2001 From: Ed Kellett Date: Mon, 15 Apr 2019 18:06:48 +0100 Subject: [PATCH 6/6] Ignore UFNC for saved nicks --- modules/core/m_nick.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/core/m_nick.c b/modules/core/m_nick.c index 2863106e2..dcbb20c06 100644 --- a/modules/core/m_nick.c +++ b/modules/core/m_nick.c @@ -576,6 +576,10 @@ ms_ufnc(struct Client *client_p, struct Client *source_p, int parc, const char * sendto_realops_snomask(SNO_GENERAL, L_NETWIDE, "Ignored UFNC message for non-person %s from %s", target_p->name, source_p->name); + else if (IsDigit(target_p->name[0])) + sendto_realops_snomask(SNO_GENERAL, L_NETWIDE, + "Ignored UFNC message for saved user %s from %s", + target_p->name, source_p->name); else if (target_p->tsinfo == atol(parv[3])) fnc_user(client_p, source_p, target_p, newnick); else