From 04bb95ce795519c36e7a5d4b0d5b29d4644623ce Mon Sep 17 00:00:00 2001 From: Art Cancro Date: Mon, 21 Nov 2011 15:17:43 -0500 Subject: [PATCH] Important fixes to session matching and reuse logic. --- webcit/auth.c | 6 --- webcit/context_loop.c | 89 ++++++++++++++++++++++--------------------- webcit/dav_main.c | 2 +- webcit/webcit.c | 72 ++++++++-------------------------- webcit/webcit.h | 1 - 5 files changed, 61 insertions(+), 109 deletions(-) diff --git a/webcit/auth.c b/webcit/auth.c index 76fb13dc1..31c000f52 100644 --- a/webcit/auth.c +++ b/webcit/auth.c @@ -477,7 +477,6 @@ void do_welcome(void) * Disconnect from the Citadel server, and end this WebCit session */ void end_webcit_session(void) { - serv_puts("QUIT"); WC->killthis = 1; /* close() of citadel socket will be done by do_housekeeping() */ @@ -950,11 +949,6 @@ void CheckAuthBasic(ParsedHttpHdrs *hdr) */ StrBufAppendBufPlain(hdr->HR.plainauth, HKEY(":"), 0); StrBufAppendBuf(hdr->HR.plainauth, hdr->HR.user_agent, 0); - hdr->HR.SessionKey = hashlittle(SKEY(hdr->HR.plainauth), 89479832); -/* - syslog(1, "CheckAuthBasic: calculated sessionkey %ld\n", - hdr->HR.SessionKey); -*/ } diff --git a/webcit/context_loop.c b/webcit/context_loop.c index 3b1f30ae3..5f2d2d6d3 100644 --- a/webcit/context_loop.c +++ b/webcit/context_loop.c @@ -56,7 +56,7 @@ void shutdown_sessions(void) wcsession *sptr; for (sptr = SessionList; sptr != NULL; sptr = sptr->next) { - sptr->killthis = 1; + sptr->killthis = 1; } } @@ -74,7 +74,7 @@ void do_housekeeping(void) /* Kill idle sessions */ if ((time(NULL) - (sptr->lastreq)) > (time_t) WEBCIT_TIMEOUT) { - syslog(3, "Timeout session %d\n", sptr->wc_session); + syslog(3, "Timeout session %d", sptr->wc_session); sptr->killthis = 1; } @@ -101,7 +101,7 @@ void do_housekeeping(void) * Now free up and destroy the culled sessions. */ while (sessions_to_kill != NULL) { - syslog(3, "Destroying session %d\n", sessions_to_kill->wc_session); + syslog(3, "Destroying session %d", sessions_to_kill->wc_session); sptr = sessions_to_kill->next; session_destroy_modules(&sessions_to_kill); sessions_to_kill = sptr; @@ -120,7 +120,7 @@ void check_thread_pool_size(void) (num_threads_executing >= num_threads_existing) && (num_threads_existing < MAX_WORKER_THREADS) ) { - syslog(3, "%d of %d threads are executing. Adding another worker thread.\n", + syslog(3, "%d of %d threads are executing. Adding another worker thread.", num_threads_executing, num_threads_existing ); @@ -174,33 +174,27 @@ wcsession *FindSession(wcsession **wclist, ParsedHttpHdrs *Hdr, pthread_mutex_t switch (Hdr->HR.got_auth) { case AUTH_BASIC: - if ( (Hdr->HR.SessionKey != sptr->SessionKey)) - continue; - if ((!strcasecmp(ChrPtr(Hdr->c_username), ChrPtr(sptr->wc_username))) && - (!strcasecmp(ChrPtr(Hdr->c_password), ChrPtr(sptr->wc_password))) ) { - syslog(LOG_DEBUG, "-- matched a session with the same http-auth"); + if ( (!strcasecmp(ChrPtr(Hdr->c_username), ChrPtr(sptr->wc_username))) + && (!strcasecmp(ChrPtr(Hdr->c_password), ChrPtr(sptr->wc_password))) + && (sptr->killthis == 0) + ) { + syslog(LOG_DEBUG, "\033[32m-- matched a session with the same http-auth\033[0m"); TheSession = sptr; } - if (TheSession == NULL) - syslog(1, "found sessionkey [%d], but credentials for [%s|%s] didn't match", - Hdr->HR.SessionKey, - ChrPtr(Hdr->c_username), - ChrPtr(sptr->wc_username) - ); break; case AUTH_COOKIE: /* If cookie-session, look for a session with matching session ID */ - if ( (Hdr->HR.desired_session != 0) && - (sptr->wc_session == Hdr->HR.desired_session)) - { - syslog(LOG_DEBUG, "-- matched a session with the same cookie"); + if ( (Hdr->HR.desired_session != 0) + && (sptr->wc_session == Hdr->HR.desired_session) + ) { + syslog(LOG_DEBUG, "\033[32m-- matched a session with the same cookie\033[0m"); TheSession = sptr; } break; case NO_AUTH: /* Any unbound session is a candidate */ if ( (sptr->wc_session == 0) && (sptr->inuse == 0) ) { - syslog(LOG_DEBUG, "-- reusing an unbound session"); + syslog(LOG_DEBUG, "\033[32m-- reusing an unbound session\033[0m"); TheSession = sptr; } break; @@ -208,9 +202,7 @@ wcsession *FindSession(wcsession **wclist, ParsedHttpHdrs *Hdr, pthread_mutex_t } CtdlLogResult(pthread_mutex_unlock(ListMutex)); if (TheSession == NULL) { - syslog(1, "didn't find sessionkey [%d] for user [%s]", - Hdr->HR.SessionKey, ChrPtr(Hdr->c_username) - ); + syslog(LOG_DEBUG, "\033[32m-- no existing session was matched\033[0m"); } return TheSession; } @@ -221,7 +213,6 @@ wcsession *CreateSession(int Lockable, int Static, wcsession **wclist, ParsedHtt TheSession = (wcsession *) malloc(sizeof(wcsession)); memset(TheSession, 0, sizeof(wcsession)); TheSession->Hdr = Hdr; - TheSession->SessionKey = Hdr->HR.SessionKey; TheSession->serv_sock = (-1); pthread_setspecific(MyConKey, (void *)TheSession); @@ -406,7 +397,7 @@ int ReadHTTPRequest (ParsedHttpHdrs *Hdr) memset(pHdr, 0, sizeof(OneHttpHeader)); pHdr->Val = Line; Put(Hdr->HTTPHeaders, HKEY("GET /"), pHdr, DestroyHttpHeaderHandler); - syslog(9, "%s\n", ChrPtr(Line)); + syslog(9, "%s", ChrPtr(Line)); isbogus = ReadHttpSubject(Hdr, Line, HeaderName); if (isbogus) break; continue; @@ -516,7 +507,7 @@ void context_loop(ParsedHttpHdrs *Hdr) wcsession *Bogus; Bogus = CreateSession(0, 1, NULL, Hdr, NULL); do_404(); - syslog(9, "HTTP: 404 [%ld.%06ld] %s %s \n", + syslog(9, "HTTP: 404 [%ld.%06ld] %s %s", ((tx_finish.tv_sec*1000000 + tx_finish.tv_usec) - (tx_start.tv_sec*1000000 + tx_start.tv_usec)) / 1000000, ((tx_finish.tv_sec*1000000 + tx_finish.tv_usec) - (tx_start.tv_sec*1000000 + tx_start.tv_usec)) % 1000000, ReqStrs[Hdr->HR.eReqType], @@ -537,7 +528,7 @@ void context_loop(ParsedHttpHdrs *Hdr) /* How long did this transaction take? */ gettimeofday(&tx_finish, NULL); - syslog(9, "HTTP: 200 [%ld.%06ld] %s %s \n", + syslog(9, "HTTP: 200 [%ld.%06ld] %s %s", ((tx_finish.tv_sec*1000000 + tx_finish.tv_usec) - (tx_start.tv_sec*1000000 + tx_start.tv_usec)) / 1000000, ((tx_finish.tv_sec*1000000 + tx_finish.tv_usec) - (tx_start.tv_sec*1000000 + tx_start.tv_usec)) % 1000000, ReqStrs[Hdr->HR.eReqType], @@ -557,30 +548,40 @@ void context_loop(ParsedHttpHdrs *Hdr) } /* - * See if there's an existing session open with the desired ID or user/pass + * See if there's an existing session open with any of: + * - The desired Session ID + * - A matching http-auth username and password + * - An unbound session flagged as reusable */ TheSession = FindSession(&SessionList, Hdr, &SessionListMutex); /* - * Create a new session if we have to + * If there were no qualifying sessions, then create a new one. */ if (TheSession == NULL) { TheSession = CreateSession(1, 0, &SessionList, Hdr, &SessionListMutex); + } - if ( (StrLength(Hdr->c_username) == 0) - && (!Hdr->HR.DontNeedAuth) - && (Hdr->HR.Handler != NULL) - && ((XHTTP_COMMANDS & Hdr->HR.Handler->Flags) == XHTTP_COMMANDS) - ) { - OverrideRequest(Hdr, HKEY("GET /401 HTTP/1.0")); - Hdr->HR.prohibit_caching = 1; - } - - if (StrLength(Hdr->c_language) > 0) { - syslog(9, "Session cookie requests language '%s'\n", ChrPtr(Hdr->c_language)); - set_selected_language(ChrPtr(Hdr->c_language)); - go_selected_language(); - } + /* + * If a language was requested via a cookie, select that language now. + */ + if (StrLength(Hdr->c_language) > 0) { + syslog(9, "Session cookie requests language '%s'", ChrPtr(Hdr->c_language)); + set_selected_language(ChrPtr(Hdr->c_language)); + go_selected_language(); + } + + /* + * Reject transactions which require http-auth, if http-auth was not provided + */ + if ( (StrLength(Hdr->c_username) == 0) + && (!Hdr->HR.DontNeedAuth) + && (Hdr->HR.Handler != NULL) + && ((XHTTP_COMMANDS & Hdr->HR.Handler->Flags) == XHTTP_COMMANDS) + ) { + syslog(LOG_DEBUG, "\033[35m -- http-auth required but not provided\033[0m"); + OverrideRequest(Hdr, HKEY("GET /401 HTTP/1.0")); + Hdr->HR.prohibit_caching = 1; } /* @@ -604,7 +605,7 @@ void context_loop(ParsedHttpHdrs *Hdr) /* How long did this transaction take? */ gettimeofday(&tx_finish, NULL); - syslog(9, "HTTP: 200 [%ld.%06ld] %s %s \n", + syslog(9, "HTTP: 200 [%ld.%06ld] %s %s", ((tx_finish.tv_sec*1000000 + tx_finish.tv_usec) - (tx_start.tv_sec*1000000 + tx_start.tv_usec)) / 1000000, ((tx_finish.tv_sec*1000000 + tx_finish.tv_usec) - (tx_start.tv_sec*1000000 + tx_start.tv_usec)) % 1000000, ReqStrs[Hdr->HR.eReqType], diff --git a/webcit/dav_main.c b/webcit/dav_main.c index 91610ac99..833e75e75 100644 --- a/webcit/dav_main.c +++ b/webcit/dav_main.c @@ -15,7 +15,7 @@ * * You should have received a copy of the GNU General Public License * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ #include "webcit.h" diff --git a/webcit/webcit.c b/webcit/webcit.c index 6d789dc48..94ff1e22e 100644 --- a/webcit/webcit.c +++ b/webcit/webcit.c @@ -358,9 +358,9 @@ void authorization_required(void) message ); wDumpContent(0); - end_webcit_session(); } + /* * Convenience functions to wrap around asynchronous ajax responses */ @@ -378,6 +378,7 @@ void begin_ajax_response(void) { begin_burst(); } + /* * print ajax response footer */ @@ -386,7 +387,6 @@ void end_ajax_response(void) { } - /* * Wraps a Citadel server command in an AJAX transaction. */ @@ -499,6 +499,7 @@ void push_destination(void) { wc_printf("OK"); } + /* * Go to the URL saved by push_destination() */ @@ -587,49 +588,6 @@ int ReadPostData(void) return 1; } -#if 0 -void ParseREST_URL(void) -{ - StrBuf *Buf; - StrBuf *pFloor = NULL; - wcsession *WCC = WC; - long i = 0; - const char *pCh = NULL; - HashList *Floors; - void *vFloor; - - syslog(1, "parsing rest URL: %s", ChrPtr(WCC->Hdr->HR.ReqLine)); - - WCC->Directory = NewHash(1, Flathash); - WCC->CurrentFloor = NULL; - - Buf = NewStrBuf(); - while (StrBufExtract_NextToken(Buf, WCC->Hdr->HR.ReqLine, &pCh, '/') >= 0) - { - if (StrLength(Buf) != 0) { - /* ignore empty path segments */ - StrBufUnescape(Buf, 1); - Put(WCC->Directory, IKEY(i), Buf, HFreeStrBuf); - if (i==0) - pFloor = Buf; - Buf = NewStrBuf(); - } - i++; - } - - FreeStrBuf(&Buf); - if (pFloor != NULL) - { - Floors = GetFloorListHash(NULL, NULL); - - if (Floors != NULL) - { - if (GetHash(WCC->FloorsByName, SKEY(pFloor), &vFloor)) - WCC->CurrentFloor = (Floor*) vFloor; - } - } -} -#endif int Conditional_REST_DEPTH(StrBuf *Target, WCTemplputParams *TP) { @@ -737,10 +695,10 @@ void session_loop(void) * If we're not logged in, but we have authentication data (either from * a cookie or from http-auth), try logging in to Citadel using that. */ - if ((!WCC->logged_in) - && (StrLength(WCC->Hdr->c_username) > 0) - && (StrLength(WCC->Hdr->c_password) > 0)) - { + if ( (!WCC->logged_in) + && (StrLength(WCC->Hdr->c_username) > 0) + && (StrLength(WCC->Hdr->c_password) > 0) + ) { long Status; FlushStrBuf(Buf); @@ -816,15 +774,13 @@ void session_loop(void) display_login(); } else { -#if 0 - if ((WCC->Hdr->HR.Handler->Flags & PARSE_REST_URL) != 0) - ParseREST_URL(); -#endif - if ((WCC->Hdr->HR.Handler->Flags & AJAX) != 0) + if ((WCC->Hdr->HR.Handler->Flags & AJAX) != 0) { begin_ajax_response(); + } WCC->Hdr->HR.Handler->F(); - if ((WCC->Hdr->HR.Handler->Flags & AJAX) != 0) + if ((WCC->Hdr->HR.Handler->Flags & AJAX) != 0) { end_ajax_response(); + } } } /* When all else fails, display the default landing page or a main menu. */ @@ -845,10 +801,12 @@ void session_loop(void) * Toplevel dav requests? or just a flat browser request? */ else { - if (xhttp) + if (xhttp) { dav_main(); - else + } + else { display_main_menu(); + } } } diff --git a/webcit/webcit.h b/webcit/webcit.h index 3951ea388..5166d0e86 100644 --- a/webcit/webcit.h +++ b/webcit/webcit.h @@ -490,7 +490,6 @@ struct wcsession { int killthis; /* Nonzero == purge this session */ int ctdl_pid; /* Session ID on the Citadel server */ int nonce; /* session nonce (to prevent session riding) */ - int SessionKey; int inuse; /* set to nonzero if bound to a running thread */ /* Session local Members */ -- 2.30.2