Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions src/m_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -361,10 +361,23 @@ m_server_estab(aClient *cptr)
aClient *acptr;

char *inpath, *host, *s, *encr;
struct userBan *ban;

inpath = get_client_name(cptr, HIDEME); /* "refresh" inpath with host */
host = cptr->name;

ban = check_userbanned(cptr, UBAN_IP|UBAN_CIDR4|UBAN_WILDUSER, 0);

if (ban)
{
int loc = (ban->flags & UBAN_LOCAL) ? 1 : 0;

ircstp->is_ref++;
ircstp->is_ref_2++;

return exit_banned_client(cptr, loc, loc? 'K' : 'A', ban->reason, 0);
}

if (!(aconn = cptr->serv->aconn))
{
ircstp->is_ref++;
Expand Down
13 changes: 0 additions & 13 deletions src/s_bsd.c
Original file line number Diff line number Diff line change
Expand Up @@ -1328,7 +1328,6 @@ aClient *add_connection(aListener *lptr, int fd)
struct sockaddr_in6 addr6;
} addr;
unsigned int len = sizeof(addr);
struct userBan *ban;

if (getpeername(fd, &addr.sa, &len) == -1)
{
Expand Down Expand Up @@ -1390,18 +1389,6 @@ aClient *add_connection(aListener *lptr, int fd)
acptr->lstn = lptr;
add_client_to_list(acptr);

ban = check_userbanned(acptr, UBAN_IP|UBAN_CIDR4|UBAN_WILDUSER, 0);
if(ban)
{
int loc = (ban->flags & UBAN_LOCAL) ? 1 : 0;

ircstp->is_ref++;
ircstp->is_ref_1++;
exit_banned_client(acptr, loc, loc ? 'K' : 'A', ban->reason, 0);

return NULL;
}

Comment on lines -1393 to -1404
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like check_userbanned will be postponed until SERVER or USER is parsed even for non-SSL connections. Couldn't we just silently return here if it is a banned SSL connection, and add another ban check when the SSL handshake is finished?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. By skipping this part, one less look through the banlist for a match, especially if the ban is placed on user@host, as we don't have the user info here yet.

if(call_hooks(CHOOK_PREACCESS, acptr) == FLUSH_BUFFER)
return NULL;

Expand Down
2 changes: 1 addition & 1 deletion src/s_user.c
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ register_user(aClient *cptr, aClient *sptr, char *nick, char *username,
: "Too many connections from your site");
}

if(!(ban = check_userbanned(sptr, UBAN_IP|UBAN_CIDR4, UBAN_WILDUSER)))
if(!(ban = check_userbanned(sptr, UBAN_IP|UBAN_CIDR4, 0)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is removing UBAN_WILDUSER necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you look in check_userbanned, with the flag, during my tests, only bans on *@host instead of both and *user@host are being matched.

ban = check_userbanned(sptr, UBAN_HOST, 0);

if(ban)
Expand Down