33eb806d3707661d8a4d9af1e0673f90578d86bc
max
  Mon Sep 21 06:12:21 2026 -0700
hgHubConnect: the api key sync has to answer before the cart, and its signature needed a timestamp, refs #38323

Six things, all in the syncHubApiKeys path, which is still off everywhere.

The sync never reached its handler on a real mirror.  It was registered as
a cartJson command, so building the cart ran forceUserIdOrCaptcha() first;
a peer's request carries no hguid cookie and no apiKey= variable, so any
site with cloudFlareSiteKey set -- which is every site that needs api keys
in the first place -- answered it with a captcha page.  Only hgwdev, where
the captcha is commented out, ever ran the handler, which is why this was
not caught earlier.  doApiKeySyncIfRequested() now answers the request in
main() before any cart exists.  That also stops each sync leaving a junk
userDb and sessionDb row behind: measured, 15 syncs now create none.

The signature is HMAC-SHA256 over the salt instead of md5(salt-user-key),
so it does not rest on md5 resisting length extension, and it is compared
in constant time.  It now covers a timestamp too, and a peer refuses
anything signed more than HUB_APIKEY_SYNC_WINDOW (300s) either side of
now, so a captured sync cannot be replayed later to reinstate a key its
owner has since revoked.  The fields are joined with newlines and a
newline in a userName or apiKey is refused, so one signature cannot cover
two different splits of the same string.

syncApiKeyToOtherNodes() is called inside an errCatch of its own now.  It
runs after the local key is already written and in the response, so an
errAbort there (login.cookieSalt unset) used to reach cartJsonExecute's
outer catch, which threw the response away -- the user saw a failure over
a key that was sitting in the database.

geoMirrorNotifyOtherNodes() returns each peer's response instead of
discarding it, and uses netUrlMustOpenPastHeader, which errAborts on
anything but a 200 and hands back the body alone.  A peer answering 'bad
signature' or 'not enabled on this site' is no longer indistinguishable
from success; it is logged to error_log, not warn()ed at the person who
clicked the button.

Verified against the built CGI run as Apache runs it, with the captcha
turned on: a signed sync is accepted and writes the row, a signed revoke
removes it, and a replay, a swapped userName, a forged signature and
malformed json are all refused.  hubSpaceKeysTester covers the signature
rules; it still cannot run its database half until hgcentralregress gets
the UPDATE and DELETE grant.

diff --git src/hg/hgHubConnect/hgHubConnect.h src/hg/hgHubConnect/hgHubConnect.h
index d8ed20243fe..717218c00b0 100644
--- src/hg/hgHubConnect/hgHubConnect.h
+++ src/hg/hgHubConnect/hgHubConnect.h
@@ -1,50 +1,52 @@
 /* hgHubConnect - User interfaces for connecting and managing track hubs */
 
 /* Copyright (C) 2008 The Regents of the University of California 
  * See kent/LICENSE or http://genome.ucsc.edu/license/ for licensing information. */
 
 #ifndef HGHUBCONNECT_H
 #define HGHUBCONNECT_H
 
 #include "cart.h"
 #include "cartJson.h"
 
 //extern struct cart *cart;	/* This holds cgi and other variables between clicks. */
 
 // the variables for various track hub wizard methods:
 #define hgHubGetHubSpaceUIState "getHubSpaceUIState"
 #define hgHubDeleteFile "deleteFile"
 #define hgHubCreateHub "createHub"
 #define hgHubEditHub "editHub"
 #define hgHubMoveFile "moveFile"
 #define hgHubGenerateApiKey "generateApiKey"
 #define hgHubRevokeApiKey "revokeApiKey"
 #define hgHubSyncApiKey "hgHubSyncApiKey"
 
 void cjRevokeApiKey(struct cartJson *cj, struct hash *paramHash);
 /* Remove any api keys for the user */
 
 void cjGenerateApiKey(struct cartJson *cj, struct hash *paramHash);
 /* Make a random (but not crypto-secure api key for use of hubtools to upload to hubspace */
 
-void cjSyncApiKey(struct cartJson *cj, struct hash *paramHash);
-/* Adopt an api key (or a revocation) that a peer geo mirror is telling us about.  Only ever
- * called by another mirror's geoMirrorNotifyOtherNodes(), never by a browser. */
+boolean doApiKeySyncIfRequested();
+/* If this request is a peer geo mirror telling us about an api key (or a revocation), answer
+ * it and return TRUE, else return FALSE and leave the request to the normal cart path.  Only
+ * ever sent by another mirror's geoMirrorNotifyOtherNodes(), never by a browser, so it must
+ * be answered before a cart is built: see the comment on the definition. */
 
 void doRemoveFile(struct cartJson *cj, struct hash *paramHash);
 /* Process the request to remove a file */
 
 void doMoveFile(struct cartJson *cj, struct hash *paramHash);
 /* Move a file to a new hub */
 
 void getHubSpaceUIState(struct cartJson *cj, struct hash *paramHash);
 /* Get all the data we need to make a users hubSpace UI table. The cartJson library
  * deals with printing the json */
 
 void doEditHub(struct cartJson *cj, struct hash *paramHash);
 /* Edit the hub.txt for a hub */
 
 void doTrackHubWizard(char *database);
 /* Print out the html to allow a user to upload some files from their machine to us */
 
 #endif /* HGHUBCONNECT_H */