diff --git a/include/h.h b/include/h.h index c071d8295..eda6b9984 100644 --- a/include/h.h +++ b/include/h.h @@ -731,7 +731,7 @@ extern MODVAR unsigned char *(*StripColors)(unsigned char *text); extern MODVAR const char *(*StripControlCodes)(unsigned char *text); extern MODVAR void (*spamfilter_build_user_string)(char *buf, char *nick, Client *acptr); extern MODVAR void (*send_protoctl_servers)(Client *client, int response); -extern MODVAR int (*verify_link)(Client *client, char *servername, ConfigItem_link **link_out); +extern MODVAR int (*verify_link)(Client *client, ConfigItem_link **link_out); extern MODVAR void (*send_server_message)(Client *client); extern MODVAR void (*broadcast_md_client)(ModDataInfo *mdi, Client *acptr, ModData *md); extern MODVAR void (*broadcast_md_channel)(ModDataInfo *mdi, Channel *channel, ModData *md); diff --git a/src/api-efunctions.c b/src/api-efunctions.c index 9cd055ca7..0f94b4084 100644 --- a/src/api-efunctions.c +++ b/src/api-efunctions.c @@ -76,7 +76,7 @@ unsigned char *(*StripColors)(unsigned char *text); const char *(*StripControlCodes)(unsigned char *text); void (*spamfilter_build_user_string)(char *buf, char *nick, Client *client); void (*send_protoctl_servers)(Client *client, int response); -int (*verify_link)(Client *client, char *servername, ConfigItem_link **link_out); +int (*verify_link)(Client *client, ConfigItem_link **link_out); void (*introduce_user)(Client *to, Client *client); void (*send_server_message)(Client *client); void (*broadcast_md_client)(ModDataInfo *mdi, Client *client, ModData *md); diff --git a/src/modules/protoctl.c b/src/modules/protoctl.c index c1b3c4eda..17c81a8fd 100644 --- a/src/modules/protoctl.c +++ b/src/modules/protoctl.c @@ -236,7 +236,7 @@ CMD_FUNC(cmd_protoctl) } servername = strtoken(&p, buf, ","); - if (!servername || (strlen(servername) > HOSTLEN) || !strchr(servername, '.')) + if (!valid_server_name(servername)) { sendto_one(client, NULL, "ERROR :Bogus server name in EAUTH (%s)", servername ? servername : ""); sendto_snomask @@ -258,7 +258,12 @@ CMD_FUNC(cmd_protoctl) } } - if (!verify_link(client, servername, &aconf)) + /* Set client->name but don't add to hash list, this gives better + * log messages and should be safe. See CMTSRV941 in server.c. + */ + strlcpy(client->name, servername, sizeof(client->name)); + + if (!verify_link(client, &aconf)) return; /* note: software, protocol and flags may be NULL */ @@ -267,11 +272,6 @@ CMD_FUNC(cmd_protoctl) SetEAuth(client); make_server(client); /* allocate and set client->serv */ - /* Set client->name but don't add to hash list. The real work on - * that is done in cmd_server. We just set it here for display - * purposes of error messages (such as reject due to clock). - */ - strlcpy(client->name, servername, sizeof(client->name)); if (protocol) client->serv->features.protocol = atoi(protocol); if (software) diff --git a/src/modules/server.c b/src/modules/server.c index cb6eb81cd..2607616b0 100644 --- a/src/modules/server.c +++ b/src/modules/server.c @@ -45,7 +45,7 @@ EVENT(server_handshake_timeout); void send_channel_modes_sjoin3(Client *to, Channel *channel); CMD_FUNC(cmd_server); CMD_FUNC(cmd_sid); -int _verify_link(Client *client, char *servername, ConfigItem_link **link_out); +int _verify_link(Client *client, ConfigItem_link **link_out); void _send_protoctl_servers(Client *client, int response); void _send_server_message(Client *client); void _introduce_user(Client *to, Client *acptr); @@ -634,11 +634,10 @@ void _send_server_message(Client *client) * This does authentication and authorization checks. * @param cptr The client directly connected to us (cptr). * @param client The client which (originally) issued the server command (client). - * @param servername The server name provided by the client. * @param link_out Pointer-to-pointer-to-link block. Will be set when auth OK. Caller may pass NULL if he doesn't care. * @returns This function returns 1 on successful authentication, 0 otherwise - in which case the client has been killed. */ -int _verify_link(Client *client, char *servername, ConfigItem_link **link_out) +int _verify_link(Client *client, ConfigItem_link **link_out) { char xerrmsg[256]; ConfigItem_link *link; @@ -664,12 +663,6 @@ int _verify_link(Client *client, char *servername, ConfigItem_link **link_out) return 0; } - /* First check if the server is in the list */ - if (!servername) { - strcpy(xerrmsg, "Null servername"); - goto errlink; - } - if (client->serv && client->serv->conf) { /* This is an outgoing connect so we already know what link block we are @@ -677,17 +670,15 @@ int _verify_link(Client *client, char *servername, ConfigItem_link **link_out) */ /* Actually we still need to double check the servername to avoid confusion. */ - if (strcasecmp(servername, client->serv->conf->servername)) + if (strcasecmp(client->name, client->serv->conf->servername)) { - ircsnprintf(xerrmsg, sizeof(xerrmsg), "Outgoing connect from link block '%s' but server " - "introduced himself as '%s'. Server name mismatch.", - client->serv->conf->servername, - servername); - - sendto_one(client, NULL, "ERROR :%s", xerrmsg); - sendto_ops_and_log("Outgoing link aborted to %s(%s@%s) (%s) %s", - client->serv->conf->servername, client->ident, client->local->sockhost, xerrmsg, inpath); - exit_client(client, NULL, xerrmsg); + unreal_log(ULOG_INFO, "link", "LINK_SERVERNAME_MISMATCH", client, + "Link from server $client.details denied: " + "outgoing connect from link block '$link_block' but server " + "introduced itself as '$client'. Server name mismatch.", + log_data_link_block(client->serv->conf)); + exit_client_fmt(client, NULL, "Servername (%s) does not match name in my link block (%s)", + client->name, client->serv->conf->servername); return 0; } link = client->serv->conf; @@ -695,23 +686,23 @@ int _verify_link(Client *client, char *servername, ConfigItem_link **link_out) } else { /* Hunt the linkblock down ;) */ for(link = conf_link; link; link = link->next) - if (match_simple(link->servername, servername)) + if (match_simple(link->servername, client->name)) break; } if (!link) { - ircsnprintf(xerrmsg, sizeof(xerrmsg), "No link block named '%s'", servername); + ircsnprintf(xerrmsg, sizeof(xerrmsg), "No link block named '%s'", client->name); goto errlink; } if (!link->incoming.mask) { - ircsnprintf(xerrmsg, sizeof(xerrmsg), "Link block '%s' exists but has no link::incoming::mask", servername); + ircsnprintf(xerrmsg, sizeof(xerrmsg), "Link block '%s' exists but has no link::incoming::mask", client->name); goto errlink; } - link = find_link(servername, client); + link = find_link(client->name, client); if (!link) { @@ -720,10 +711,10 @@ errlink: /* Send the "simple" error msg to the server */ sendto_one(client, NULL, "ERROR :Link denied (No link block found named '%s' or link::incoming::mask did not match your IP %s) %s", - servername, GetIP(client), inpath); + client->name, GetIP(client), inpath); /* And send the "verbose" error msg only to locally connected ircops */ sendto_ops_and_log("Link denied for %s(%s@%s) (%s) %s", - servername, client->ident, client->local->sockhost, xerrmsg, inpath); + client->name, client->ident, client->local->sockhost, xerrmsg, inpath); exit_client(client, NULL, "Link denied (No link block found with your server name or link::incoming::mask did not match)"); return 0; } @@ -743,31 +734,31 @@ skip_host_check: ((link->auth->type != AUTHTYPE_PLAINTEXT) && client->local->passwd && strcmp(client->local->passwd, "*"))) { sendto_ops_and_log("Link denied for '%s' (Authentication failed due to different password types on both sides of the link) %s", - servername, inpath); + client->name, inpath); sendto_ops_and_log("Read https://www.unrealircd.org/docs/FAQ#auth-fail-mixed for more information"); } else if (link->auth->type == AUTHTYPE_SPKIFP) { sendto_ops_and_log("Link denied for '%s' (Authentication failed [spkifp mismatch]) %s", - servername, inpath); + client->name, inpath); } else if (link->auth->type == AUTHTYPE_TLS_CLIENTCERT) { sendto_ops_and_log("Link denied for '%s' (Authentication failed [tlsclientcert mismatch]) %s", - servername, inpath); + client->name, inpath); } else if (link->auth->type == AUTHTYPE_TLS_CLIENTCERTFP) { sendto_ops_and_log("Link denied for '%s' (Authentication failed [tlsclientcertfp mismatch]) %s", - servername, inpath); + client->name, inpath); } else { sendto_ops_and_log("Link denied for '%s' (Authentication failed [Bad password?]) %s", - servername, inpath); + client->name, inpath); } sendto_one(client, NULL, "ERROR :Link '%s' denied (Authentication failed) %s", - servername, inpath); + client->name, inpath); exit_client(client, NULL, "Link denied (Authentication failed)"); return 0; } @@ -781,9 +772,9 @@ skip_host_check: { sendto_one(client, NULL, "ERROR :Link '%s' denied (Not using SSL/TLS) %s", - servername, inpath); + client->name, inpath); sendto_ops_and_log("Link denied for '%s' (Not using SSL/TLS and verify-certificate is on) %s", - servername, inpath); + client->name, inpath); exit_client(client, NULL, "Link denied (Not using SSL/TLS)"); return 0; } @@ -791,9 +782,9 @@ skip_host_check: { sendto_one(client, NULL, "ERROR :Link '%s' denied (Certificate verification failed) %s", - servername, inpath); + client->name, inpath); sendto_ops_and_log("Link denied for '%s' (Certificate verification failed) %s", - servername, inpath); + client->name, inpath); sendto_ops_and_log("Reason for certificate verification failure: %s", errstr); exit_client(client, NULL, "Link denied (Certificate verification failed)"); return 0; @@ -804,7 +795,7 @@ skip_host_check: * Third phase, we check that the server does not exist * already */ - if ((acptr = find_server(servername, NULL))) + if ((acptr = find_server(client->name, NULL))) { /* Found. Bad. Quit. */ @@ -822,16 +813,16 @@ skip_host_check: acptr = (client->local->firsttime > acptr->local->firsttime) ? client : acptr; sendto_one(acptr, NULL, "ERROR :Server %s already exists from %s", - servername, + client->name, (ocptr->direction ? ocptr->direction->name : "")); sendto_ops_and_log ("Link %s cancelled, server %s already exists from %s", - get_client_name(acptr, TRUE), servername, + get_client_name(acptr, TRUE), client->name, (ocptr->direction ? ocptr->direction->name : "")); exit_client(acptr, NULL, "Server Exists"); return 0; } - if ((bconf = find_ban(NULL, servername, CONF_BAN_SERVER))) + if ((bconf = find_ban(NULL, client->name, CONF_BAN_SERVER))) { sendto_ops_and_log ("Cancelling link %s, banned server", @@ -934,14 +925,20 @@ CMD_FUNC(cmd_server) return; } - if (!verify_link(client, servername, &aconf)) + /* We set the client->name early here, even though it is not authenticated yet. + * Reason is that it makes the notices and logging more useful. + * This should be safe as it is not in the server linked list yet or hash table. + * CMTSRV941 -- Syzop. + */ + strlcpy(client->name, servername, sizeof(client->name)); + + if (!verify_link(client, &aconf)) return; /* Rejected */ /* From this point the server is authenticated, so we can be more verbose * with notices to ircops and in exit_client() and such. */ - strlcpy(client->name, servername, sizeof(client->name)); if (strlen(client->id) != 3) { diff --git a/src/serv.c b/src/serv.c index 20391ebfa..f41a010d7 100644 --- a/src/serv.c +++ b/src/serv.c @@ -1204,15 +1204,18 @@ int valid_server_name(char *name) { char *p; + if (!*name) + return 0; /* empty name */ + if (strlen(name) >= HOSTLEN) - return 0; + return 0; /* oversized */ for (p = name; *p; p++) if ((*p <= ' ') || (*p > '~')) - return 0; + return 0; /* forbidden chars */ if (!strchr(name, '.')) - return 0; + return 0; /* no dot */ return 1; }