]> git.netwichtig.de Git - user/henk/code/inspircd.git/blobdiff - src/modules/m_ident.cpp
DOH! Fix my muppetry of a segfault, and fix some warnings
[user/henk/code/inspircd.git] / src / modules / m_ident.cpp
index f08f8c868635d34be7cd4ca12fe05ba6d05bd43d..8f251cb467253d3ede7fee8734bcb1eda2ce5dc2 100644 (file)
@@ -2,7 +2,7 @@
  *       | Inspire Internet Relay Chat Daemon |
  *       +------------------------------------+
  *
- *  InspIRCd: (C) 2002-2007 InspIRCd Development Team
+ *  InspIRCd: (C) 2002-2008 InspIRCd Development Team
  * See: http://www.inspircd.org/wiki/index.php/Credits
  *
  * This program is free but copyrighted software; see
 
 /* $ModDesc: Provides support for RFC1413 ident lookups */
 
+/* --------------------------------------------------------------
+ * Note that this is the third incarnation of m_ident. The first
+ * two attempts were pretty crashy, mainly due to the fact we tried
+ * to use InspSocket/BufferedSocket to make them work. This class
+ * is ok for more heavyweight tasks, it does a lot of things behind
+ * the scenes that are not good for ident sockets and it has a huge
+ * memory footprint!
+ *
+ * To fix all the issues that we had in the old ident modules (many
+ * nasty race conditions that would cause segfaults etc) we have
+ * rewritten this module to use a simplified socket object based
+ * directly off EventHandler. As EventHandler only has low level
+ * readability, writeability and error events tied directly to the
+ * socket engine, this makes our lives easier as nothing happens to
+ * our ident lookup class that is outside of this module, or out-
+ * side of the control of the class. There are no timers, internal
+ * events, or such, which will cause the socket to be deleted,
+ * queued for deletion, etc. In fact, theres not even any queueing!
+ *
+ * Using this framework we have a much more stable module.
+ *
+ * A few things to note:
+ * 
+ *   O  The only place that may *delete* an active or inactive
+ *      ident socket is OnUserDisconnect in the module class.
+ *      Because this is out of scope of the socket class there is
+ *      no possibility that the socket may ever try to delete
+ *      itself.
+ *
+ *   O  Closure of the ident socket with the Close() method will
+ *      not cause removal of the socket from memory or detatchment
+ *      from its 'parent' User class. It will only flag it as an 
+ *      inactive socket in the socket engine.
+ *
+ *   O  Timeouts are handled in OnCheckReaady at the same time as
+ *      checking if the ident socket has a result. This is done
+ *      by checking if the age the of the class (its instantiation
+ *      time) plus the timeout value is greater than the current time.
+ *
+ *  O   The ident socket is able to but should not modify its
+ *      'parent' user directly. Instead the ident socket class sets
+ *      a completion flag and during the next call to OnCheckReady,
+ *      the completion flag will be checked and any result copied to
+ *      that user's class. This again ensures a single point of socket
+ *      deletion for safer, neater code.
+ *
+ *  O   The code in the constructor of the ident socket is taken from
+ *      BufferedSocket but majorly thinned down. It works for both
+ *      IPv4 and IPv6.
+ *
+ *  O   In the event that the ident socket throws a ModuleException,
+ *      nothing is done. This is counted as total and complete
+ *      failure to create a connection.
+ * --------------------------------------------------------------
+ */
+
 class IdentRequestSocket : public EventHandler
 {
  private:
 
-        User *user;
-        InspIRCd* ServerInstance;
-        bool done;
-        std::string result;
+        User *user;                    /* User we are attached to */
+        InspIRCd* ServerInstance;      /* Server instance */
+        bool done;                     /* True if lookup is finished */
+        std::string result;            /* Holds the ident string if done */
 
  public:
 
        IdentRequestSocket(InspIRCd *Server, User* u, const std::string &bindip) : user(u), ServerInstance(Server), result(u->ident)
        {
-               /* connect here on instantiation, throw on immediate failure */
-
                socklen_t size = 0;
-
 #ifdef IPV6
+               /* Does this look like a v6 ip address? */
                bool v6 = false;
                if ((bindip.empty()) || bindip.find(':') != std::string::npos)
                v6 = true;
@@ -49,10 +103,12 @@ class IdentRequestSocket : public EventHandler
                if (GetFd() == -1)
                        throw ModuleException("Could not create socket");
 
+               /* We allocate two of these because sizeof(sockaddr_in6) > sizeof(sockaddr_in) */
                sockaddr* s = new sockaddr[2];
                sockaddr* addr = new sockaddr[2];
        
 #ifdef IPV6
+               /* Horrid icky nasty ugly berkely socket crap. */
                if (v6)
                {
                        in6_addr addy;
@@ -63,7 +119,7 @@ class IdentRequestSocket : public EventHandler
                                memcpy(&((sockaddr_in6*)addr)->sin6_addr, &addy, sizeof(addy));
                                ((sockaddr_in6*)addr)->sin6_port = htons(113);
                                size = sizeof(sockaddr_in6);
-                               inet_pton(AF_INET6, IP.c_str(), &n);
+                               inet_pton(AF_INET6, bindip.c_str(), &n);
                                memcpy(&((sockaddr_in6*)s)->sin6_addr, &n, sizeof(sockaddr_in6));
                                ((sockaddr_in6*)s)->sin6_port = 0;
                                ((sockaddr_in6*)s)->sin6_family = AF_INET6;
@@ -87,16 +143,19 @@ class IdentRequestSocket : public EventHandler
                        }
                }
 
+               /* Attempt to bind (ident requests must come from the ip the query is referring to */
                if (ServerInstance->SE->Bind(GetFd(), s, size) < 0)
                {
                        this->Close();
                        delete[] s;
+                       delete[] addr;
                        throw ModuleException("failed to bind()");
                }
 
                delete[] s;
                ServerInstance->SE->NonBlocking(GetFd());
 
+               /* Attempt connection (nonblocking) */
                if (ServerInstance->SE->Connect(this, (sockaddr*)addr, size) == -1 && errno != EINPROGRESS)
                {
                        this->Close();
@@ -106,23 +165,26 @@ class IdentRequestSocket : public EventHandler
 
                delete[] addr;
 
+               /* Add fd to socket engine */
                if (!ServerInstance->SE->AddFd(this))
                {
                        this->Close();
                        throw ModuleException("out of fds");
                }
 
+               /* Important: We set WantWrite immediately after connect()
+                * because a successful connection will trigger a writability event
+                */
                ServerInstance->SE->WantWrite(this);
        }
 
        virtual void OnConnected()
        {
-               ServerInstance->Log(DEBUG,"OnConnected()");
+               ServerInstance->Logs->Log("m_ident",DEBUG,"OnConnected()");
 
                /* Both sockaddr_in and sockaddr_in6 can be safely casted to sockaddr, especially since the
                 * only members we use are in a part of the struct that should always be identical (at the
                 * byte level). */
-
                #ifndef IPV6
                sockaddr_in laddr, raddr;
                #else
@@ -140,12 +202,16 @@ class IdentRequestSocket : public EventHandler
 
                char req[32];
 
+               /* Build request in the form 'localport,remoteport\r\n' */
                #ifndef IPV6
                int req_size = snprintf(req, sizeof(req), "%d,%d\r\n", ntohs(raddr.sin_port), ntohs(laddr.sin_port));
                #else
                int req_size = snprintf(req, sizeof(req), "%d,%d\r\n", ntohs(raddr.sin6_port), ntohs(laddr.sin6_port));
                #endif
                
+               /* Send failed if we didnt write the whole ident request --
+                * might as well give up if this happens!
+                */
                if (ServerInstance->SE->Send(this, req, req_size, 0) < req_size)
                        done = true;
        }
@@ -164,7 +230,10 @@ class IdentRequestSocket : public EventHandler
                        break;
                        case EVENT_ERROR:
                                /* fd error event, ohshi- */
-                               ServerInstance->Log(DEBUG,"EVENT_ERROR");
+                               ServerInstance->Logs->Log("m_ident",DEBUG,"EVENT_ERROR");
+                               /* We *must* Close() here immediately or we get a
+                                * huge storm of EVENT_ERROR events!
+                                */
                                Close();
                                done = true;
                        break;
@@ -173,11 +242,16 @@ class IdentRequestSocket : public EventHandler
 
        void Close()
        {
+               /* Remove ident socket from engine, and close it, but dont detatch it
+                * from its parent user class, or attempt to delete its memory.
+                */
                if (GetFd() > -1)
                {
+                       ServerInstance->Logs->Log("m_ident",DEBUG,"Close ident socket %d", GetFd());
+                       ServerInstance->SE->DelFd(this);
                        ServerInstance->SE->Close(GetFd());
                        ServerInstance->SE->Shutdown(GetFd(), SHUT_WR);
-                       ServerInstance->SE->DelFd(this);
+                       this->SetFd(-1);
                }
        }
 
@@ -186,6 +260,9 @@ class IdentRequestSocket : public EventHandler
                return done;
        }
 
+       /* Note: if the lookup succeeded, will contain 'ident', otherwise
+        * will contain '~ident'. Use *GetResult() to determine lookup success.
+        */
        const char* GetResult()
        {
                return result.c_str();
@@ -193,39 +270,42 @@ class IdentRequestSocket : public EventHandler
 
        void ReadResponse()
        {
-               ServerInstance->Log(DEBUG,"ReadResponse()");
-
-               // We don't really need to buffer for incomplete replies here, since IDENT replies are
-               // extremely short - there is *no* sane reason it'd be in more than one packet
-
+               /* We don't really need to buffer for incomplete replies here, since IDENT replies are
+                * extremely short - there is *no* sane reason it'd be in more than one packet
+                */
                char ibuf[MAXBUF];
                int recvresult = ServerInstance->SE->Recv(this, ibuf, MAXBUF-1, 0);
 
-               /* Cant possibly be a valid response shorter than 3 chars */
+               /* Cant possibly be a valid response shorter than 3 chars,
+                * because the shortest possible response would look like: '1,1'
+                */
                if (recvresult < 3)
                {
+                       Close();
                        done = true;
                        return;
                }
 
+               ServerInstance->Logs->Log("m_ident",DEBUG,"ReadResponse()");
+
                irc::sepstream sep(ibuf, ':');
                std::string token;
                for (int i = 0; sep.GetToken(token); i++)
                {
-                       // We only really care about the 4th portion
+                       /* We only really care about the 4th portion */
                        if (i < 3)
                                continue;
 
                        char ident[IDENTMAX + 2];
 
-                       // Truncate the ident at any characters we don't like, skip leading spaces
+                       /* Truncate the ident at any characters we don't like, skip leading spaces */
                        int k = 0;
                        for (const char *j = token.c_str(); *j && (k < IDENTMAX + 1); j++)
                        {
                                if (*j == ' ')
                                        continue;
 
-                               // Rules taken from InspIRCd::IsIdent
+                               /* Rules taken from InspIRCd::IsIdent */
                                if (((*j >= 'A') && (*j <= '}')) || ((*j >= '0') && (*j <= '9')) || (*j == '-') || (*j == '.'))
                                {
                                        ident[k++] = *j;
@@ -237,16 +317,18 @@ class IdentRequestSocket : public EventHandler
 
                        ident[k] = '\0';
 
-                       // Redundant check with IsIdent, in case that changes and this doesn't (paranoia!)
+                       /* Re-check with IsIdent, in case that changes and this doesn't (paranoia!) */
                        if (*ident && ServerInstance->IsIdent(ident))
                        {
                                result = ident;
-                               ServerInstance->next_call = ServerInstance->Time();
                        }
 
                        break;
                }
 
+               /* Close (but dont delete from memory) our socket
+                * and flag as done
+                */
                Close();
                done = true;
                return;
@@ -262,17 +344,15 @@ class ModuleIdent : public Module
                : Module(Me)
        {
                OnRehash(NULL, "");
+               Implementation eventlist[] = { I_OnRehash, I_OnUserRegister, I_OnCheckReady, I_OnCleanup, I_OnUserDisconnect };
+               ServerInstance->Modules->Attach(eventlist, this, 5);
        }
        
        virtual Version GetVersion()
        {
-               return Version(1, 1, 1, 0, VF_VENDOR, API_VERSION);
+               return Version(1, 2, 1, 0, VF_VENDOR, API_VERSION);
        }
        
-       virtual void Implements(char *List)
-       {
-               List[I_OnRehash] = List[I_OnUserRegister] = List[I_OnCheckReady] = List[I_OnCleanup] = List[I_OnUserDisconnect] = 1;
-       }
        
        virtual void OnRehash(User *user, const std::string &param)
        {
@@ -289,7 +369,7 @@ class ModuleIdent : public Module
                 * should be preceded by a ~. The field is actually IDENTMAX+2 characters wide. */
                memmove(user->ident + 1, user->ident, IDENTMAX);
                user->ident[0] = '~';
-               // Ensure that it is null terminated
+               /* Ensure that it is null terminated */
                user->ident[IDENTMAX + 1] = '\0';
 
                user->WriteServ("NOTICE Auth :*** Looking up your ident...");
@@ -322,7 +402,7 @@ class ModuleIdent : public Module
                }
                catch (ModuleException &e)
                {
-                       ServerInstance->Log(DEBUG,"Ident exception: %s", e.GetReason());
+                       ServerInstance->Logs->Log("m_ident",DEBUG,"Ident exception: %s", e.GetReason());
                        return 0;
                }
 
@@ -330,25 +410,36 @@ class ModuleIdent : public Module
                return 0;
        }
 
+       /* This triggers pretty regularly, we can use it in preference to
+        * creating a Timer object and especially better than creating a
+        * Timer per ident lookup!
+        */
        virtual bool OnCheckReady(User *user)
        {
-               ServerInstance->Log(DEBUG,"OnCheckReady %s", user->nick);
+               ServerInstance->Logs->Log("m_ident",DEBUG,"OnCheckReady %s", user->nick);
 
                /* Does user have an ident socket attached at all? */
                IdentRequestSocket *isock = NULL;
                if (!user->GetExt("ident_socket", isock))
                {
-                       ServerInstance->Log(DEBUG, "No ident socket :(");
+                       ServerInstance->Logs->Log("m_ident",DEBUG, "No ident socket :(");
                        return true;
                }
 
-               ServerInstance->Log(DEBUG, "Has ident_socket");
+               ServerInstance->Logs->Log("m_ident",DEBUG, "Has ident_socket");
 
-               if (isock->age + RequestTimeout > ServerInstance->Time() && !isock->HasResult())
+               time_t compare = isock->age;
+               compare += RequestTimeout;
+
+               /* Check for timeout of the socket */
+               if (ServerInstance->Time() >= compare)
                {
                        /* Ident timeout */
                        user->WriteServ("NOTICE Auth :*** Ident request timed out.");
-                       ServerInstance->Log(DEBUG, "Timeout");
+                       ServerInstance->Logs->Log("m_ident",DEBUG, "Timeout");
+                       /* The user isnt actually disconnecting,
+                        * we call this to clean up the user
+                        */
                        OnUserDisconnect(user);
                        return true;
                }
@@ -356,19 +447,22 @@ class ModuleIdent : public Module
                /* Got a result yet? */
                if (!isock->HasResult())
                {
-                       ServerInstance->Log(DEBUG, "No result yet");
+                       ServerInstance->Logs->Log("m_ident",DEBUG, "No result yet");
                        return false;
                }
 
-               ServerInstance->Log(DEBUG, "Yay, result!");
+               ServerInstance->Logs->Log("m_ident",DEBUG, "Yay, result!");
 
-               /* wooo, got a result! */
+               /* wooo, got a result (it will be good, or bad) */
                if (*(isock->GetResult()) != '~')
                        user->WriteServ("NOTICE Auth :*** Found your ident, '%s'", isock->GetResult());
                else
                        user->WriteServ("NOTICE Auth :*** Could not find your ident, using %s instead.", isock->GetResult());
 
+               /* Copy the ident string to the user */
                strlcpy(user->ident, isock->GetResult(), IDENTMAX+1);
+
+               /* The user isnt actually disconnecting, we call this to clean up the user */
                OnUserDisconnect(user);
                return true;
        }
@@ -389,10 +483,10 @@ class ModuleIdent : public Module
                        isock->Close();
                        delete isock;
                        user->Shrink("ident_socket");
-                       ServerInstance->Log(DEBUG, "Removed ident socket from %s", user->nick);
+                       ServerInstance->Logs->Log("m_ident",DEBUG, "Removed ident socket from %s", user->nick);
                }
        }
 };
 
-MODULE_INIT(ModuleIdent);
+MODULE_INIT(ModuleIdent)