690cc018255362c9d1af6ed5f4fa15ae7a0fc81d
max
  Wed Aug 5 11:26:22 2026 -0700
changes after automated code review

hgLogin: sign the pending social identity with hmacMd5() instead of a hand-built
cookieSalt-prefix + MD5. The salt now keys the HMAC rather than being prepended to
the hashed message. This is the signature that closes the account-takeover hole in
the OAuth account chooser, so it should use a real MAC. It also now fails closed:
with login.cookieSalt unset the old code hashed an empty salt, so anyone could
compute a valid signature and the protection was silently absent - hgLogin refuses
to run a social login without the salt. Ordinary login pages never reach this check.
Pending identities in flight across the upgrade no longer validate; those users are
asked to sign in again.

lib/hmac.c: hmacSha1() and hmacMd5() sized their hex buffers at exactly 40 and 32
chars, so the last sprintf wrote its terminating zero one byte past the array and
the following strlen read out of bounds. Grow both by one and use cloneString().
Output is unchanged, verified against openssl dgst -hmac.

userAccounts.css: drop the #helpBox rule, unused since the last id="helpBox" was
removed from hgLogin.c.

mirrorManual, ex.hg.conf: the login section claimed "three" extra ways to sign in
when there are two (external provider, one-time email link). Also document that
login.cookieSalt is required for social sign-in, now that the check fails closed.
Edited mirrorManual.txt and regenerated the html with mirrorDocs/make, since the
html is pandoc output and hand edits there do not survive.

refs #37984

diff --git src/hg/hgLogin/hgLogin.c src/hg/hgLogin/hgLogin.c
index 4ff08174b69..c5779d26d75 100644
--- src/hg/hgLogin/hgLogin.c
+++ src/hg/hgLogin/hgLogin.c
@@ -1,26 +1,27 @@
 /* hgLogin - Administer UCSC Genome Browser membership - signup, lost password, etc. */
 
 /* Copyright (C) 2014 The Regents of the University of California 
  * See kent/LICENSE or http://genome.ucsc.edu/license/ for licensing information. */
 
 #include <openssl/evp.h>
 #include <openssl/opensslv.h>
 #include <openssl/md5.h>
 
 #include "common.h"
 #include "hash.h"
+#include "hmac.h"
 #include "obscure.h"
 #include "hgConfig.h"
 #include "cheapcgi.h"
 #include "memalloc.h"
 #include "jksql.h"
 #include "htmshell.h"
 #include "cart.h"
 #include "hPrint.h"
 #include "hdb.h"
 #include "hui.h"
 #include "web.h"
 #include "ra.h"
 #include "hgColors.h"
 #include "net.h"
 #include "wikiLink.h"
@@ -1663,41 +1664,45 @@
 static struct gbMembers *memberForIdentity(struct sqlConnection *conn, struct oauthIdentity *id)
 /* Return the gbMembers account already linked to this provider identity, or NULL. */
 {
 char query[512];
 sqlSafef(query, sizeof(query),
     "SELECT idx FROM gbMemberIdentity WHERE provider='%s' AND subject='%s'",
     id->provider, id->subject);
 uint idx = (uint)sqlQuickLongLong(conn, query);
 if (idx == 0)
     return NULL;
 sqlSafef(query, sizeof(query), "SELECT * FROM gbMembers WHERE idx=%u", idx);
 return gbMembersLoadByQuery(conn, query);
 }
 
 static char *oauthPendingSig(char *provider, char *subject, char *email)
-/* Signature over a pending social identity, keyed by the secret login.cookieSalt.  Only
+/* HMAC-MD5 over a pending social identity, keyed by the secret login.cookieSalt.  Only
  * resolveIdentity (which runs after a genuine provider verification) can produce a valid one,
  * so a pending identity injected through cart/CGI variables will not validate.  (Not bound to
  * the session id: the hgsid is regenerated across the provider redirect, so a session-bound
  * signature would never match on the way back.)  Result is allocd. */
 {
+char *salt = cfgOption(CFG_LOGIN_COOKIE_SALT);
+if (isEmpty(salt))
+    errAbort("Signing in with an external identity provider requires %s in hg.conf, set to a "
+        "secret random string.  Without a secret we cannot sign the pending identity, and the "
+        "account chooser would accept a forged one.", CFG_LOGIN_COOKIE_SALT);
 char buf[1024];
-safef(buf, sizeof(buf), "%s|%s|%s|%s",
-    emptyForNull(cfgOption(CFG_LOGIN_COOKIE_SALT)),
+safef(buf, sizeof(buf), "%s|%s|%s",
     emptyForNull(provider), emptyForNull(subject), emptyForNull(email));
-return generateTokenMD5(buf);
+return hmacMd5(salt, buf);
 }
 
 static boolean pendingIdentityValid()
 /* TRUE only if the pending-identity cart variables carry a signature we minted this session.
  * Guards the OAuth chooser and completeAccount against forged/injected pending identities. */
 {
 char *sig = cartUsualString(cart, "oauth_pending_sig", "");
 if (isEmpty(sig))
     return FALSE;
 char *expected = oauthPendingSig(cartUsualString(cart, "oauth_pending_provider", ""),
                                  cartUsualString(cart, "oauth_pending_subject", ""),
                                  cartUsualString(cart, "oauth_pending_email", ""));
 boolean ok = sameString(sig, expected);
 freeMem(expected);
 return ok;