fix bug where client connecting before server would result in attempt

to forward packet to host not yet available.  Drop such packets now.

xwords4/relay/cref.cpp

@@ -438,6 +438,23 @@ CookieRef::pushForwardEvent( HostID src, HostID dest,
     m_eventQueue.push_front( evt );
 }
 
+void
+CookieRef::pushDestBadEvent()
+{
+    CRefEvent evt;
+    evt.type = XW_EVENT_DESTBAD;
+    m_eventQueue.push_front( evt );
+}
+
+void
+CookieRef::pushDestOkEvent( const CRefEvent* oldEvt )
+{
+    CRefEvent evt;
+    memcpy( &evt, oldEvt, sizeof(evt) );
+    evt.type = XW_EVENT_DESTOK;
+    m_eventQueue.push_front( evt );
+}
+
 void
 CookieRef::handleEvents()
 {
@@ -462,6 +479,10 @@ CookieRef::handleEvents()
                 forward( &evt );
                 break;
 
+            case XW_ACTION_CHECKDEST:
+                checkDest( &evt );
+                break;
+
             case XW_ACTION_DISCONNECTALL:
                 disconnectAll( &evt );
                 break;
@@ -475,10 +496,10 @@ CookieRef::handleEvents()
                 break;
 
             case XW_ACTION_HEARTOK:
-                /* nothing to do for this */
+            case XW_ACTION_NONE: 
+                /* nothing to do for these */
                 break;
 
-            case XW_ACTION_NONE: 
             default:
                 assert(0); 
                 break;
@@ -552,14 +573,30 @@ CookieRef::forward( const CRefEvent* evt )
 
     int destSocket = SocketForHost( dest );
 
-    /* This is an ugly hack!!!! */
+    if ( destSocket != -1 ) {
-    *buf = XWRELAY_MSG_FROMRELAY;
+        /* This is an ugly hack!!!! */
-    send_with_length( destSocket, buf, buflen );
+        *buf = XWRELAY_MSG_FROMRELAY;
+        send_with_length( destSocket, buf, buflen );
 
-    /* also note that we've heard from src recently */
+        /* also note that we've heard from src recently */
-    pushHeartbeatEvent( src, SocketForHost(src) );
+        pushHeartbeatEvent( src, SocketForHost(src) );
+    } else {
+        /* We're not really connected yet! */
+    }
 } /* forward */
 
+void
+CookieRef::checkDest( const CRefEvent* evt )
+{
+    HostID dest = evt->u.fwd.dest;
+    int destSocket = SocketForHost( dest );
+    if ( destSocket == -1 ) {
+        pushDestBadEvent();
+    } else {
+        pushDestOkEvent( evt );
+    }
+} /* checkDest */
+
 void
 CookieRef::disconnectAll( const CRefEvent* evt )
 {

xwords4/relay/cref.h

@@ -99,11 +99,16 @@ class CookieRef {
     void pushHeartTimerEvent( time_t now, vector<int>* victims );
     void pushForwardEvent( HostID src, HostID dest, unsigned char* buf, 
                            int buflen );
+    void pushDestBadEvent();
+    void pushDestOkEvent( const CRefEvent* evt );
+
 
     void handleEvents();
 
     void sendResponse(const CRefEvent* evt);
     void forward(const CRefEvent* evt);
+    void checkDest( const CRefEvent* evt );
+
     void disconnectAll(const CRefEvent* evt);
     void noteHeartbeat(const CRefEvent* evt);
     void checkHeartbeats(const CRefEvent* evt);

xwords4/relay/states.cpp

@@ -29,6 +29,33 @@ typedef struct StateTable {
                                    performs the action determine the state? */
 } StateTable;
 
+/* Connecting.  The problem is that we don't know how many devices to expect.
+   So hosts connect and get a response.  Hosts send messages to be forwarded.
+   We may or may not have an ID for the recipient host.  If we do, we forward.
+   If we don't, we drop.  No big deal.  So is there any difference between
+   CONNECTING and CONNECTED?  I don't think so.  Messages come in and we try
+   to forward.  Connection requests come in and we accept them if the host in
+   question is unknown.  NOT QUITE.  There's a DOS vulnerability there.  It's
+   best if we can put the game in a state where others can't connect, if the
+   window where new devices can sign in using a given cookie is fairly small.
+
+   Perhaps a better algorithm is needed for determining when the game is
+   closed.  It's not when the first XW_EVENT_FORWARDMSG comes along, since
+   that can happen before all hosts have arrived (e.g. if a client signs up
+   before the server.)  The goal has been to avoid having the relay know about
+   the xwords protocol.  But it already knows about hostID 0 (I think).  So
+   maybe when that host sends a message for forwarding we know we're set?  Or
+   maybe we add a message the server can send saying "All are present now;
+   lock the game!"  It'd suck if that were spoofed though.
+
+   The problem we want to avoid is bogus connections.  Two are playing using
+   cookie FOO.  Somebody looks over a shoulder and sees FOO, and tries to
+   connect.  The relay must not start forwarding packets from the interloper
+   into the game associated with FOO.
+
+
+ */
+
 StateTable g_stateTable[] = {
 
     /* Initial msg comes in.  Managing object created in init state, sends response */
@@ -36,9 +63,17 @@ StateTable g_stateTable[] = {
 
     /* Another connect msg comes in */
     { XW_ST_CONNECTING,        XW_EVENT_CONNECTMSG,  XW_ACTION_SENDRSP,       XW_ST_CONNECTING },
-    /* Fwd message comes in, tells us we're now fully connected */
+
-    { XW_ST_CONNECTING,        XW_EVENT_FORWARDMSG,  XW_ACTION_FWD,           XW_ST_ALLCONNECTED },
+    /* Server can lock game when all players are accounted for.  Not required
-    { XW_ST_CONNECTING,        XW_EVENT_HEARTMSG,    XW_ACTION_NOTEHEART,     XW_ST_CONNECTING },
+       yet, though.  In fact I'm not sure how to require it. :-) */
+    { XW_ST_CONNECTING,        XW_EVENT_ALLHEREMSG,  XW_ACTION_LOCKGAME,      XW_ST_ALLCONNECTED },
+
+    /* Forward requests while not locked are ok -- but we must check that the
+       target is actually present.  If no socket available must drop the message */
+    { XW_ST_CONNECTING,        XW_EVENT_FORWARDMSG,  XW_ACTION_CHECKDEST,     XW_ST_CHECKINGDEST },
+    { XW_ST_CHECKINGDEST,      XW_EVENT_DESTOK,      XW_ACTION_FWD,           XW_ST_CONNECTING },
+    { XW_ST_CHECKINGDEST,      XW_EVENT_DESTBAD,     XW_ACTION_NONE,          XW_ST_CONNECTING },
+
     /* Timeout before all connected */
     { XW_ST_CONNECTING,        XW_EVENT_CONNTIMER,   XW_ACTION_DISCONNECTALL, XW_ST_DEAD },
     { XW_ST_CONNECTING,        XW_EVENT_HEARTTIMER,  XW_ACTION_CHECKHEART,    XW_ST_HEARTCHECK_CONNECTING },
@@ -46,6 +81,7 @@ StateTable g_stateTable[] = {
     /* This is the entry we'll use most of the time */
     { XW_ST_ALLCONNECTED,      XW_EVENT_FORWARDMSG,  XW_ACTION_FWD,           XW_ST_ALLCONNECTED },
     /* Heartbeat arrived */
+    { XW_ST_CONNECTING,        XW_EVENT_HEARTMSG,    XW_ACTION_NOTEHEART,     XW_ST_CONNECTING },
     { XW_ST_ALLCONNECTED,      XW_EVENT_HEARTMSG,    XW_ACTION_NOTEHEART,     XW_ST_ALLCONNECTED },
 
     /* Heartbeat timer means check for dead connections.  Post event to self if there's a problem. */
@@ -102,6 +138,8 @@ stateString( XW_RELAY_STATE state )
         CASESTR(XW_ST_HEARTCHECK_CONNECTING);
         CASESTR(XW_ST_HEARTCHECK_CONNECTED);
         CASESTR(XW_ST_DEAD);
+        CASESTR(XW_ST_CHECKING_CONN);
+        CASESTR(XW_ST_CHECKINGDEST);
     }
     assert(0);
     return "";
@@ -121,6 +159,10 @@ eventString( XW_RELAY_EVENT evt )
         CASESTR(XW_EVENT_CONNTIMER);
         CASESTR(XW_EVENT_HEARTOK);
         CASESTR(XW_EVENT_HEARTFAILED);
+        CASESTR(XW_EVENT_ALLHEREMSG);
+        CASESTR(XW_EVENT_DESTOK);
+        CASESTR(XW_EVENT_DESTBAD);
+
     }
     assert(0);
     return "";

xwords4/relay/states.h

@@ -35,6 +35,9 @@ typedef enum {
                                    packets have yet arrived to be
                                    forwarded. */
 
+    ,XW_ST_CHECKING_CONN        /* While we're still not fully connected a
+                                   message comes in */
+
     ,XW_ST_ALLCONNECTED         /* All devices are connected and ready for the
                                    relay to do its work.  This is the state
                                    we're in most of the time.  */
@@ -55,6 +58,8 @@ typedef enum {
 
     ,XW_ST_DISCONNECTED         /* We're about to kill this object. */
 
+    ,XW_ST_CHECKINGDEST         /* Checking for valid socket */
+
     ,XW_ST_DEAD                 /* About to kill the object */
 } XW_RELAY_STATE;
 
@@ -81,8 +86,17 @@ typedef enum {
     ,XW_EVENT_CONNTIMER         /* timer for did we get all players hooked
                                    up  */
 
+    ,XW_EVENT_ALLHEREMSG        /* message from server that all expected
+                                   player reg messages have been received and
+                                   no new hosts should be registering selvs
+                                   with cookie. */
+
     ,XW_EVENT_HEARTOK
 
+    ,XW_EVENT_DESTOK
+
+    ,XW_EVENT_DESTBAD
+
     ,XW_EVENT_HEARTFAILED
 } XW_RELAY_EVENT;
 
@@ -103,6 +117,10 @@ typedef enum {
 
     ,XW_ACTION_HEARTOK          /* allows transition back to stationary state */
 
+    ,XW_ACTION_LOCKGAME         /* lock the cref/cookie session to new hosts */
+
+    ,XW_ACTION_CHECKDEST        /* check that a given hostID has a socket */
+
 } XW_RELAY_ACTION;