[BlueSonicBoy]

Introduction

Servers should always be IP Filtered, in light of the number of fake servers around.

Enabling the Filter servers too (rejects dynIP-servers) option in Options->security->IP Filter causes eMule not to add IP Filtered servers and
dynIP-servers to the server list Previous post {edit} Regardless of its state, servers in the server list are not IP Filtered,(I was under the erroneous belief that they were always filtered!!)
I think the following code changes make sense given the facts outlined above. Code changes are indicated in bold red unless change From and To are shown.

Code changes

Firstly servers should always be IP Filtered so:

IMHO IDS_FILTERSERVERBYIPFILTER should be changed from
Filter servers too (rejects dynIP-servers)
to something like
Reject all dynIP-servers

and the code change to reflect this in ServerList.cpp AddServer() thus:
From:

bool CServerList::AddServer(const CServer* pServer) said:

        if (thePrefs.FilterServerByIP()){
                if (pServer->HasDynIP())
                        return false;
                if (theApp.ipfilter->IsFiltered(pServer->GetIP())){
                        if (thePrefs.GetLogFilteredIPs())
                                AddDebugLogLine(false, _T("Ignored server (IP=%s) - IP filter (%s)"), ipstr(pServer->GetIP()), theApp.ipfilter->GetLastHit());
                      return false;
                }
        }

To:

bool CServerList::AddServer(const CServer* pServer) said:

    if (thePrefs.FilterServerByIP() && pServer->HasDynIP())
            return false;
    if (theApp.ipfilter->IsFiltered(pServer->GetIP())){
            if (thePrefs.GetLogFilteredIPs())
                    AddDebugLogLine(false, _T("Ignored server (IP=%s) - IP filter (%s)"), ipstr(pServer->GetIP()), theApp.ipfilter->GetLastHit());
                    return false;
    }

Then to make sure we never connect to an IP Filtered server, and delete any filtered servers as we try them*
*Additional code to always filter servers!!

In ServerSocket.cpp (Old solution fixed by SiRoB! The fix fixes this issue! found by XMan)*

void CServerSocket::ConnectToServer() said:

void CServerSocket::ConnectToServer(CServer* server){
if (cur_server){
  ASSERT(0);
  delete cur_server;
  cur_server = NULL;
}

//Wizard's fix. Remove the server now!
if(theApp.ipfilter->IsFiltered(server->GetIP()))
    {
      theApp.emuledlg->serverwnd->serverlistctrl.RemoveServer(server);
      SetConnectionState(CS_ERROR);  //SiRoB's Fix
      return;
    }


cur_server = new CServer(server);

OR XMan's Solution!*

In Socket.cpp so we can call IsFiltered()

Socket.cpp said:

#include "IPFilter.h"

In Socket.cpp

void CServerConnect::ConnectToServer(CServer* server, on bool multiconnect), said:

void CServerConnect::ConnectToServer(CServer* server, bool multiconnect)
{
if (!multiconnect) {
  StopConnectionTry();
  Disconnect();
}

//Xman filter outgoing server connections
//it makes no sence to allow to connect to any ipfiltered server, because you'll only get lowID
if(theApp.ipfilter->IsFiltered(server->GetIP()))
{
  AddLogLine(true,_T("you can't connect to filtered server: %s, %s"),ipstr(server->GetIP()), server->GetDescription() );
  if(thePrefs.FilterServerByIP())
  {
  theApp.emuledlg->serverwnd->serverlistctrl.RemoveServer(server);
  }
  return;
}
//Xman end

connecting = true;
singleconnecting = !multiconnect;

If you don't add IP Filtered servers, doesn't it also make sense to remove existing IP Filtered servers
and any servers that become IP Filtered,(after you update your IPFilter.dat)?
Remove IP Filtered servers as well as dead servers:

In ServerListCtrl.cpp so we can call IsFiltered()

ServerListCtrl.cpp said:

#include "IPFilter.h"

Remove IP Filtered servers. in ServerListCtrl.cpp RemoveAllDeadServers()

ServerListCtrl.cpp said:

void CServerListCtrl::RemoveAllDeadServers()
{
        ShowWindow(SW_HIDE);
        for(POSITION pos = server_list->list.GetHeadPosition(); pos != NULL; server_list->list.GetNext(pos))
        {
              const CServer* cur_server = server_list->list.GetAt(pos);

                if(cur_server->GetFailedCount() >= thePrefs.GetDeadServerRetries() || theApp.ipfilter->IsFiltered(cur_server->GetIP()) )                                   
                  {
                    RemoveServer(cur_server);
                    pos = server_list->list.GetHeadPosition();
                  }
          }
ShowWindow(SW_SHOW);
}

(not related) * Here you could also add the option of not removing static servers!

ServerListCtrl.cpp and ServerList.cpp said:

In void CServerListCtrl::RemoveAllDeadServers()

if( (cur_server->GetFailedCount() >= thePrefs.GetDeadServerRetries() && (!cur_server->IsStaticMember() || !thePrefs.DontRemoveStaticServers)) || theApp.ipfilter->IsFiltered(cur_server->GetIP()))//don't remove static servers option
        {
            RemoveServer(cur_server);
            pos = server_list->list.GetHeadPosition();
        }

In ServerList.cpp void CServerList::ServerStats()

if (ping_server->GetFailedCount() >= thePrefs.GetDeadServerRetries()) {
    if(!ping_server->IsStaticMember() || !thePrefs.DontRemoveStaticServers)
        {
          theApp.emuledlg->serverwnd->serverlistctrl.RemoveServer(ping_server);
          return;
        }
    }

In addition to this, I believe it makes sense to show IP Filtered servers in the same way you show dead servers, as you cannot connect to them! Of course removing them on connect to a good server or *on connection attempt to a bad server. That way you get to see which servers your new IPFilter.dat has "taken out" before they are removed! Users can then see what is happening with regard to fake servers.

Show IP Filtered Server greyed out as per dead servers in ServerListCtrl.cpp OnNMCustomdraw()

ServerListCtrl.cpp said:

void CServerListCtrl::OnNMCustomdraw(NMHDR *pNMHDR, LRESULT *plResult)
{
        LPNMLVCUSTOMDRAW pnmlvcd = (LPNMLVCUSTOMDRAW)pNMHDR;

        if (pnmlvcd->nmcd.dwDrawStage == CDDS_PREPAINT)
            {
              *plResult = CDRF_NOTIFYITEMDRAW;
              return;
            }

        if (pnmlvcd->nmcd.dwDrawStage == CDDS_ITEMPREPAINT)
          {
              const CServer* pServer = (const CServer*)pnmlvcd->nmcd.lItemlParam;
              const CServer* pConnectedServer = theApp.serverconnect->GetCurrentServer();
              // the server which we are connected to always has a valid numerical IP member assigned,
              // therefore we do not need to call CServer::IsEqual (which is little expensive)
              //if (pConnectedServer && pConnectedServer->IsEqual(pServer))
                if (pConnectedServer && pConnectedServer->GetIP() == pServer->GetIP() && pConnectedServer->GetPort() == pServer->GetPort())
                      pnmlvcd->clrText = RGB(32,32,255);
                else if (pServer->GetFailedCount() >= thePrefs.GetDeadServerRetries() || theApp.ipfilter->IsFiltered(pServer->GetIP()))// IP Filtered indication
                        pnmlvcd->clrText = RGB(192,192,192);
                else if (pServer->GetFailedCount() >= 2)
                        pnmlvcd->clrText = RGB(128,128,128);
            }

        *plResult = CDRF_DODEFAULT;
}

And finally a very minor semi-related visual fix. The Load button in options->security->IP Filter should be greyed out on init and isn't in 47a
In PPgSecurity.cpp

PPgSecurity.cpp said:

void CPPgSecurity::LoadSettings(void)
{
        CString strBuffer;

        GetDlgItem(IDC_LOADURL)->EnableWindow(false);

Edit:to include Wizard's optimize!
Edit:*To fix my stupid oversight! Thanks Aenarion[ITA]
Edit:*Wizard's fix - Not only don't connect to an IP Filtered server but remove it from our list straight away!
Edit:*XMan & SiRoB's's fixes - For This Issue.
Edit:* Fix for "Don't remove static servers"

Done! Thank you for reading...

This post has been edited by BlueSonicBoy: 07 August 2006 - 03:13 AM

[tHeWiZaRdOfDoS]

One small thing... I propose that you do the other checks first because ipfiltering eats up your CPU, e.g.:

Quote

void CServerListCtrl::RemoveAllDeadServers()
{
        ShowWindow(SW_HIDE);
        for(POSITION pos = server_list->list.GetHeadPosition(); pos != NULL; server_list->list.GetNext(pos))
        {
              const CServer* cur_server = server_list->list.GetAt(pos);

                if(cur_server->GetFailedCount() >= thePrefs.GetDeadServerRetries() || (thePrefs.FilterServerByIP() && theApp.ipfilter->IsFiltered(cur_server->GetIP())))                                                                 
                  {
                    RemoveServer(cur_server);
                    pos = server_list->list.GetHeadPosition();
                  }
          }
ShowWindow(SW_SHOW);
}

This post has been edited by tHeWiZaRdOfDoS: 21 February 2006 - 10:42 AM

[BlueSonicBoy]

tHeWiZaRdOfDoS, on Feb 21 2006, 05:41 AM, said:

One small thing... I propose that you do the other checks first because ipfiltering eats up your CPU, e.g.:

Thanks I've added that! Unfortunately as it's an OR statement we will only save CPU cycles when the first condition is true, eg: a dead server. But any saving is a good saving!

[tHeWiZaRdOfDoS]

Yeah I know but it might be worth it *g*
I love such optimizations...

[Aenarion[ITA]]

In my Ackronic Mod, I create the AcKroNiC/EneTec Direct Servers Filter:

The feature is about the utilization system of the IP filters loaded in Emule's memory, both in the various official versions and in the other mods. If we have loaded an IP filter and we have in the server list servers which have IP addresses that are in the filter, nothing happens, making us vulnerable to an hypotetic risk of connecting to a fake/spy server and making us to cancel the servers list and download it again, to be sure we have no spies in the servers list, because eMule filters servers only during download.
In our mod, instead, servers are filtered every time you start the Mule, every time you change the options [only with the filter system activated], every time is pressed the "Filter Servers Now!" button; this is to avoid the waste of resources we would have if we filtered the list in real time. To avoid user's omissions the mod will show red "almost in real time" servers having listed IP's.

If you want, you can watch the code searching the comments "ServerFilter " and "Servers filter"

[tHeWiZaRdOfDoS]

This seems to be a useless waste of resources as you will simply not connect/deny connects to/from an ipfiltered server... that's how the ipfilter works?

[BlueSonicBoy]

Aenarion[ITA said:

,Feb 23 2006, 02:40 AM]In my Ackronic Mod, I create the AcKroNiC/EneTec Direct Servers Filter:

Sh*t!

I can't find where servers are filtered!
I could swear I'd seen servers were filtered....


You are right, this isn't good!
I'm thinking something like this??? It seems ok!

void CServerSocket::ConnectToServer() said:

void CServerSocket::ConnectToServer(CServer* server){
if (cur_server){
  ASSERT(0);
  delete cur_server;
  cur_server = NULL;
}

cur_server = new CServer(server);
if(theApp.ipfilter->IsFiltered(server->GetIP()))
    {
      SetConnectionState(CS_SERVERDEAD);
      return;
    }

This post has been edited by BlueSonicBoy: 24 February 2006 - 01:53 AM

[tHeWiZaRdOfDoS]

Eurrrr....

Quote

void CServerSocket::ConnectToServer(CServer* server){
if (cur_server){
  ASSERT(0);
  delete cur_server;
  cur_server = NULL;
}

//cur_server = new CServer(server);
if(theApp.ipfilter->IsFiltered(server->GetIP()))
    {
    // SetConnectionState(CS_SERVERDEAD);
  theApp.emuledlg->serverwnd->serverlistctrl.RemoveServer(server);
  delete server;
  server = NULL;
      return;
    }
cur_server = new CServer(server);

Better that way, don't you think? Such a server should not stay in our list (imho)

[BlueSonicBoy]

tHeWiZaRdOfDoS, on Feb 24 2006, 05:18 AM, said:

Quote

//cur_server = new CServer(server);
if(theApp.ipfilter->IsFiltered(server->GetIP()))
    {
    // SetConnectionState(CS_SERVERDEAD);
  theApp.emuledlg->serverwnd->serverlistctrl.RemoveServer(server);
  delete server;
  server = NULL;
      return;
    }
cur_server = new CServer(server);

Better that way, don't you think? Such a server should not stay in our list (imho)

Yes, excellent idea!
I was just doing a return here with the view that all IP Filtered servers can be removed when we get rid of dead servers on connect.
But your idea makes good sense. You can't delete server though as it's a pointer to the item that has just been removed. So this is the code as I see it.

void CServerSocket::ConnectToServer(CServer* server) said:

if(theApp.ipfilter->IsFiltered(server->GetIP()))
    {
      theApp.emuledlg->serverwnd->serverlistctrl.RemoveServer(server);
      cur_server = NULL;
      return;
    }

Thanks Wizard!

This post has been edited by BlueSonicBoy: 25 February 2006 - 03:12 AM

[tHeWiZaRdOfDoS]

BlueSonicBoy, on Feb 25 2006, 01:44 AM, said:

Yes, excellent idea!
I was just doing a return here with the view that all IP Filtered servers can be removed when we get rid of dead servers on connect.
But your idea makes good sense. You can't delete server though as it's a pointer to the item that has just been removed. So this is the code as I see it.  

Huh? Of course I can delete that server and I should do so otherwise I will keep that (unused) trash in memory... if I remove it from the list then that will only delete the added LPARAM not the CServer* object (at least I think so *lol* - did not test it, yet )


Uh-hu - now I see... that leads to

Quote

void CServerList::RemoveServer(const CServer* pServer)
{
for (POSITION pos = list.GetHeadPosition(); pos != NULL; )
{
  POSITION pos2 = pos;
  const CServer* test_server = list.GetNext(pos);
  if (test_server == pServer){
  if (theApp.downloadqueue->cur_udpserver == pServer)
    theApp.downloadqueue->cur_udpserver = NULL;
  list.RemoveAt(pos2);
  delservercount++;
  delete test_server;
  return;
  }
}
}

and then deletes the item... but... am I the only one who thinks that this code is way too complicated!?

Quote

void CServerList::RemoveServer(const CServer* pServer)
{
POSITION pos = list.Find(pServer);
if(pos)
{
  if (theApp.downloadqueue->cur_udpserver == pServer)
  theApp.downloadqueue->cur_udpserver = NULL;
  list.RemoveAt(pos);
  delservercount++;
  delete pServer;
}
}

Should do fine, too, what do you think?

This post has been edited by tHeWiZaRdOfDoS: 25 February 2006 - 10:07 AM

[BlueSonicBoy]

tHeWiZaRdOfDoS, on Feb 25 2006, 04:59 AM, said:

and then deletes the item... but... am I the only one who thinks that this code is way too complicated!?

Quote

void CServerList::RemoveServer(const CServer* pServer)
{
POSITION pos = list.Find(pServer);
if(pos)
{
  if (theApp.downloadqueue->cur_udpserver == pServer)
  theApp.downloadqueue->cur_udpserver = NULL;
  list.RemoveAt(pos);
  delservercount++;
  delete pServer;
}
}

Should do fine, too, what do you think?

Yes, that works great!
You need to put a cast in for it to compile.

Quote

POSITION pos = list.Find((void*)pServer);

[SiRoB]

@BlueSonicBoy
You still need to change the connectionstate of the server into ConnectToServer( to work arround some trouble reported here.

[Xman1]

ok, here is my working solution.. I think this is the simplest way:

Quote

void CServerConnect::ConnectToServer(CServer* server, bool multiconnect)
{
if (!multiconnect) {
  StopConnectionTry();
  Disconnect();
}

//Xman filter outgoing server connections
//it makes no sence to allow to connect to any ipfiltered server, because you'll only get lowID
if(theApp.ipfilter->IsFiltered(server->GetIP()))
{
  AddLogLine(true,_T("you can't connect to filtered server: %s, %s"),ipstr(server->GetIP()), server->GetDescription() );
  if(thePrefs.FilterServerByIP())
  {
  theApp.emuledlg->serverwnd->serverlistctrl.RemoveServer(server);
  }
  return;
}
//Xman end

connecting = true;
singleconnecting = !multiconnect;
theApp.emuledlg->ShowConnectionState();

CServerSocket* newsocket = new CServerSocket(this);
m_lstOpenSockets.AddTail(/*(void*&)*/newsocket); //Xman
newsocket->Create(0, SOCK_STREAM, FD_READ | FD_WRITE | FD_CLOSE | FD_CONNECT, thePrefs.GetBindAddrA());
newsocket->ConnectToServer(server);
connectionattemps.SetAt(GetTickCount(), newsocket);
}

and you must include the ipfilter.h

[BlueSonicBoy]

SiRoB, on Jun 6 2006, 11:25 AM, said:

@BlueSonicBoy
You still need to change the connectionstate of the server into ConnectToServer( to work arround some trouble reported here.

Thank You, fix added!!!

[BlueSonicBoy]

Xman1, on Jun 6 2006, 11:39 AM, said:

ok, here is my working solution.. I think this is the simplest way:

Thank you, for all your work finding and fixing this issue!
I have added your fix to my original post!

This post has been edited by BlueSonicBoy: 06 June 2006 - 09:27 PM

[SiRoB]

@BlueSonicBoy
I've got no trouble to help anyone (many could confirm).

Anyway, you added both, one is enough.
But i'm not sur how the Xman one may respect the connection process to additional server, i didn't check this part of code.

With my little change i tried to keep the normal sequence, but who know maybe both are ok.

[BlueSonicBoy]

My static servers disappeared, even though I though I had prevented them from being removed!
I had another look at the code and finally I believe I have found the culprit!

The fix below as been added to the original post.

void CServerList::ServerStats() said:

if (ping_server->GetFailedCount() >= thePrefs.GetDeadServerRetries()) {
    if(!ping_server->IsStaticMember() || !thePrefs.DontRemoveStaticServers)
        {
          theApp.emuledlg->serverwnd->serverlistctrl.RemoveServer(ping_server);
          return;
        }
    }

This post has been edited by BlueSonicBoy: 07 August 2006 - 02:56 AM