fba55c7cdca98634dd406e5a0b58ce09f98593d3 angie Mon Dec 12 16:46:03 2016 -0800 Having absolutely no new security is a bummer, so for now, by default, check incoming cookie vs. gbMembers.idx, but add a setting login.acceptAnyId that we can enable in case there is a problem with checking idx. Also add a new setting login.acceptIdx that can be disabled (along with login.acceptAnyId) in order to absolutely require a login.cookieSalt match for better security. refs #17327 diff --git src/hg/lib/wikiLink.c src/hg/lib/wikiLink.c index 94f8949..c41515b 100644 --- src/hg/lib/wikiLink.c +++ src/hg/lib/wikiLink.c @@ -175,64 +175,100 @@ cookieStrings = NULL; slAddHead(&cookieStrings, wikiLinkLoggedInCookieString(0, NULL)); slAddHead(&cookieStrings, wikiLinkUserNameCookieString(NULL)); return cookieStrings; } static char *getLoginUserName() /* Get the (CGI-decoded) value of the login userName cookie. */ { char *userName = cloneString(findCookieData(wikiLinkUserNameCookie())); if (isNotEmpty(userName)) cgiDecodeFull(userName, userName, strlen(userName)); return userName; } +static boolean loginIsRemoteClient() +/* Return TRUE if wikiHost is non-empty and not the same as this host. */ +{ +char *wikiHost = cfgOption(CFG_WIKI_HOST); +return (isNotEmpty(wikiHost) && + differentString(wikiHost, "HTTPHOST") && + differentString(wikiHost, hHttpHost())); +} + +static boolean idxIsValid(char *userName, uint idx) +/* If login is local, return TRUE if idx is the same as hgcentral.gbMembers.idx for userName. + * If remote, just return TRUE. */ +{ +if (loginIsRemoteClient()) + return TRUE; +// Look up idx for userName in gbMembers and compare to idx +struct sqlConnection *conn = hConnectCentral(); +char query[512]; +sqlSafef(query, sizeof(query), "select idx from gbMembers where userName='%s'", userName); +uint memberIdx = (uint)sqlQuickLongLong(conn, query); +hDisconnectCentral(&conn); +return (idx == memberIdx); +} + +static void sendNewCookies(char *userName, char *cookieSalt) +/* Compute key from userName and cookieSalt, and add a cookie string with the new key. */ +{ +char *newKey = makeUserKey(userName, cookieSalt); +slAddHead(&cookieStrings, wikiLinkLoggedInCookieString(0, newKey)); +slAddHead(&cookieStrings, wikiLinkUserNameCookieString(userName)); +} + struct slName *loginValidateCookies() /* Return possibly empty list of cookie strings for the caller to set. * If login cookies are obsolete but (formerly) valid, the results sets updated cookies. * If login cookies are present but invalid, the result deletes/expires the cookies. * Otherwise returns NULL (no change to cookies). */ { alreadyAuthenticated = TRUE; authenticated = FALSE; char *userName = getLoginUserName(); char *cookieKey = NULL; uint cookieIdx = getCookieIdxOrKey(&cookieKey); char *cookieSalt = getLoginCookieSalt(); if (userName && (cookieIdx > 0 || isNotEmpty(cookieKey))) { if (isNotEmpty(cookieSalt)) { if (cookieKey && sameString(makeUserKey(userName, cookieSalt), cookieKey)) { authenticated = TRUE; } -// BEGIN TODO: remove in Feb 2017 - else + else if (cfgOptionBooleanDefault(CFG_LOGIN_ACCEPT_ANY_ID, FALSE)) { - // For the first couple months, accept any value of cookieKey like we used to. - // It's possible for different systems to have different gbMembers.idx for the - // same userName, so checking gbMembers.idx would risk logging some users out - // every time they switch systems. + // Don't perform any checks on the incoming cookie. authenticated = TRUE; - // Create and store a new key, and make a cookie string with the new key. - char *newKey = makeUserKey(userName, cookieSalt); - slAddHead(&cookieStrings, wikiLinkLoggedInCookieString(cookieIdx, newKey)); - slAddHead(&cookieStrings, wikiLinkUserNameCookieString(userName)); + // Replace with improved cookie, in preparation for when better security is enabled. + sendNewCookies(userName, cookieSalt); + } +// TODO: change default to FALSE in v344 Jan 2017: + else if (cfgOptionBooleanDefault(CFG_LOGIN_ACCEPT_IDX, TRUE) && + idxIsValid(userName, cookieIdx)) + { + // Compare cookieIdx vs. gbMembers.idx (if login is local) -- a little more secure + // than before, but might cause some trouble if a userName has different idx values + // on different systems (e.g. RR vs genome-preview/genome-text). + authenticated = TRUE; + // Replace with improved cookie, in preparation for when better security is enabled. + sendNewCookies(userName, cookieSalt); } -// END TODO: remove in Feb 2017 } else { // hg.conf doesn't specify login.cookieSalt -- no checking. authenticated = TRUE; } if (!authenticated) { // Invalid key; delete cookies slAddHead(&cookieStrings, wikiLinkLoggedInCookieString(0, NULL)); slAddHead(&cookieStrings, wikiLinkUserNameCookieString(NULL)); } } return cookieStrings; }