From 26662db315dd5775f7beaad6ab04e9ca4c97f372 Mon Sep 17 00:00:00 2001
From: Bryce Kwon <bryce@brycekwon.com>
Date: Fri, 16 Oct 2026 22:11:55 -1000
Subject: Spend a missing account's login on a real hash

The fixed SHA-512 stand-in matched only the documented setup, so a site
hashing with yescrypt, other rounds or bcrypt answered for missing and
real names in different times, and on OpenBSD it failed at once.
---
 custom/extensions/auth-file.lua   | 29 +++++++------
 custom/extensions/auth-inline.lua | 23 +++++++----
 tests/extensions/harness.lua      | 17 ++++++++
 tests/extensions/test-auth.lua    | 87 +++++++++++++++++++++++----------------
 4 files changed, 99 insertions(+), 57 deletions(-)

diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua
index f43eb82..59e05cc 100644
--- a/custom/extensions/auth-file.lua
+++ b/custom/extensions/auth-file.lua
@@ -90,11 +90,6 @@ local cookie_insecure = false
 -- the password's length, and one longer than this is turned down unchecked.
 local max_password_bytes = 256
 
--- A throwaway hash of the documented shape, used only to spend the same work
--- on a missing account as on a present one, so a failed login does not reveal
--- by timing whether the user name exists.
-local dummy_hash = "$6$rounds=300000$0000000000000000$"
-
 -- One request reaches this script as an open, a write and a close, so what the
 -- open decodes is kept here for the calls that follow.
 local action, http, cgit, post
@@ -129,6 +124,10 @@ end
 -- The hash is trimmed as well as the name because reading by line strips
 -- the newline but not a carriage return, so a CRLF users file would hand
 -- crypt a hash with a trailing \r and fail every login.
+--
+-- A name with no account comes back as nil and another account's hash, so
+-- a failed login can spend the same work on a hash of the same kind and
+-- cost, and its timing does not tell whether the name exists.
 function account_hash(user)
 	if user == nil then
 		return nil
@@ -138,15 +137,19 @@ function account_hash(user)
 	if users_file == nil then
 		return nil
 	end
+	local stand_in = nil
 	for line in users_file:lines() do
 		local name, hash = string.match(line, "(.-):(.+)")
-		if name ~= nil and trim(name):lower() == wanted then
-			users_file:close()
-			return trim(hash)
+		if name ~= nil then
+			if trim(name):lower() == wanted then
+				users_file:close()
+				return trim(hash)
+			end
+			stand_in = stand_in or trim(hash)
 		end
 	end
 	users_file:close()
-	return nil
+	return nil, stand_in
 end
 
 -- The users allowed into a repository come back keyed by lowercased name, and
@@ -454,10 +457,12 @@ function authenticate_post()
 	local password = post["password"]
 	local ok = false
 	if username ~= nil and password ~= nil and #password <= max_password_bytes then
-		local hash = account_hash(username)
+		local hash, stand_in = account_hash(username)
 		if hash == nil then
-			-- Spend the work anyway, see dummy_hash.
-			unistd.crypt(password, dummy_hash)
+			-- Spend the work anyway, see account_hash.
+			if stand_in ~= nil then
+				unistd.crypt(password, stand_in)
+			end
 		elseif constant_equals(hash, unistd.crypt(password, hash)) then
 			ok = true
 		end
diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua
index 53b1efc..3355b5c 100644
--- a/custom/extensions/auth-inline.lua
+++ b/custom/extensions/auth-inline.lua
@@ -90,11 +90,6 @@ local cookie_insecure = false
 -- the password's length, and one longer than this is turned down unchecked.
 local max_password_bytes = 256
 
--- A throwaway hash of the documented shape, used only to spend the same work
--- on a missing account as on a present one, so a failed login does not reveal
--- by timing whether the user name exists.
-local dummy_hash = "$6$rounds=300000$0000000000000000$"
-
 -- One request reaches this script as an open, a write and a close, so what the
 -- open decodes is kept here for the calls that follow.
 local action, http, cgit, post
@@ -120,11 +115,19 @@ do
 	end
 end
 
+-- A name with no account comes back as nil and another account's hash, so
+-- a failed login can spend the same work on a hash of the same kind and
+-- cost, and its timing does not tell whether the name exists.
 function account_hash(user)
 	if user == nil then
 		return nil
 	end
-	return users[user:lower()]
+	local hash = users[user:lower()]
+	if hash ~= nil then
+		return hash
+	end
+	local _, stand_in = next(users)
+	return nil, stand_in
 end
 
 -- The users allowed into a repository come back keyed by lowercased name, and
@@ -416,10 +419,12 @@ function authenticate_post()
 	local password = post["password"]
 	local ok = false
 	if username ~= nil and password ~= nil and #password <= max_password_bytes then
-		local hash = account_hash(username)
+		local hash, stand_in = account_hash(username)
 		if hash == nil then
-			-- Spend the work anyway, see dummy_hash.
-			unistd.crypt(password, dummy_hash)
+			-- Spend the work anyway, see account_hash.
+			if stand_in ~= nil then
+				unistd.crypt(password, stand_in)
+			end
 		elseif constant_equals(hash, unistd.crypt(password, hash)) then
 			ok = true
 		end
diff --git a/tests/extensions/harness.lua b/tests/extensions/harness.lua
index fc83a73..3df4719 100644
--- a/tests/extensions/harness.lua
+++ b/tests/extensions/harness.lua
@@ -78,6 +78,23 @@ function harness.load(script)
 	dofile(script)
 end
 
+-- Load a script with some of its own text replaced first, for checks that
+-- need the commented-out examples of its configuration switched on. Each
+-- replacement has to match exactly once, so a reworded example fails here
+-- rather than leaving the check to test the unedited configuration.
+function harness.load_with(script, replacements)
+	local file = assert(io.open(script, "rb"))
+	local source = file:read("*a")
+	file:close()
+	for _, pair in ipairs(replacements) do
+		local start = source:find(pair[1], 1, true)
+		assert(start and not source:find(pair[1], start + 1, true),
+			"expected exactly one " .. pair[1])
+		source = source:sub(1, start - 1) .. pair[2] .. source:sub(start + #pair[1])
+	end
+	assert((loadstring or load)(source, "@" .. script))()
+end
+
 local unpack_args = unpack or table.unpack
 
 -- One whole filter round trip, an open with the given arguments, a write per
diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua
index 97638bd..e0d2fe5 100644
--- a/tests/extensions/test-auth.lua
+++ b/tests/extensions/test-auth.lua
@@ -5,11 +5,11 @@
 -- every check about this script's own logic, the cookie layout, the expiry
 -- and field rules, the redirect vetting and the action flows, rather than
 -- about OpenSSL. The signing secret is pinned by replacing get_secret, and
--- the account and access lookups are populated through each variant's own
--- channel, the documented account_hash and repo_userset swap points for the
--- inline script and fixture files reached through a redirected io.open for
--- the file one. Both variants carry the same data so the flow checks read
--- identically for the two.
+-- the accounts and access lists are filled through each variant's own
+-- channel, its commented-out examples switched on for the inline script and
+-- fixture files reached through a redirected io.open for the file one. Both
+-- variants carry the same data so the flow checks read identically for the
+-- two.
 
 local test_directory = arg[0]:match("^(.*)/") or "."
 local h = dofile(test_directory .. "/harness.lua")
@@ -43,31 +43,31 @@ if variant == "file" then
 	h.redirect_file("/etc/cgit-auth/repos", "auth-repos")
 end
 
-h.load(script)
+if variant == "inline" then
+	h.load_with(script, {
+		{ "local protected_repos = {\n", "local protected_repos = {\n" ..
+			"\t[\"secret-repo\"] = { alice = true, Bob = true },\n" ..
+			"\t[\"empty-repo\"] = {},\n" ..
+			"\t[\"bare-repo\"] = {},\n" },
+		{ "local users = {\n", "local users = {\n" ..
+			"\tAlice = \"" .. stored_hash .. "\",\n" },
+	})
+else
+	h.load(script)
+end
 
 function get_secret()
 	return string.rep("0123456789abcdef", 4)
 end
 
-if variant == "inline" then
-	local accounts = { alice = stored_hash }
-	local access = {
-		["secret-repo"] = { alice = true, bob = true },
-		["empty-repo"] = {},
-		["bare-repo"] = {},
-	}
-	function account_hash(user)
-		if user == nil then
-			return nil
-		end
-		return accounts[user:lower()]
-	end
-	function repo_userset(repo)
-		if repo == nil then
-			return nil
-		end
-		return access[repo]
-	end
+-- crypt is watched through its stand-in module, so a check can see whether
+-- a login spent it and on which hash.
+local unistd = require("posix.unistd")
+local real_crypt = unistd.crypt
+local crypt_settings = {}
+unistd.crypt = function(trypass, setting)
+	crypt_settings[#crypt_settings + 1] = setting
+	return real_crypt(trypass, setting)
 end
 
 -- The lookups themselves, which are the part the two variants do not share.
@@ -75,6 +75,8 @@ end
 local hash = account_hash("ALICE")
 h.equals("an account is found whatever its case", hash, stored_hash)
 h.equals("an unknown account is not", account_hash("nobody"), nil)
+local _, stand_in = account_hash("nobody")
+h.equals("it brings another account's hash to spend instead", stand_in, stored_hash)
 h.equals("a nil user is not", account_hash(nil), nil)
 
 local userset = repo_userset("secret-repo")
@@ -278,27 +280,40 @@ out, ret = run_action("authenticate-post", {
 })
 h.contains("an unknown user is told nothing different", out, "Set-Cookie: cgitauth=; ")
 
+-- A failed login for a name with no account spends crypt on a configured
+-- hash, the same work a real check costs, so timing tells nothing.
+crypt_settings = {}
+run_action("authenticate-post", {
+	method = "POST",
+	body = "username=nobody&password=x&redirect=" .. url_encode(way_back),
+})
+h.equals("a missing account spends crypt on a configured hash", crypt_settings[1], stored_hash)
+
+if variant == "file" then
+	h.redirect_file("/etc/cgit-auth/users", "auth-users-missing")
+	crypt_settings = {}
+	run_action("authenticate-post", {
+		method = "POST",
+		body = "username=nobody&password=x&redirect=" .. url_encode(way_back),
+	})
+	h.equals("with no accounts at all nothing is spent", #crypt_settings, 0)
+	h.redirect_file("/etc/cgit-auth/users", "auth-users")
+end
+
 -- crypt's work grows with the password, so one past the cap is turned down
 -- before crypt sees it.
-local unistd = require("posix.unistd")
-local real_crypt = unistd.crypt
-local crypt_calls = 0
-unistd.crypt = function(...)
-	crypt_calls = crypt_calls + 1
-	return real_crypt(...)
-end
+crypt_settings = {}
 out, ret = run_action("authenticate-post", {
 	method = "POST",
 	body = "username=Alice&password=" .. string.rep("a", 257) .. "&redirect=" .. url_encode(way_back),
 })
 h.contains("an overlong password is turned down", out, "Set-Cookie: cgitauth=; ")
-h.equals("without spending crypt on it", crypt_calls, 0)
-out, ret = run_action("authenticate-post", {
+h.equals("without spending crypt on it", #crypt_settings, 0)
+run_action("authenticate-post", {
 	method = "POST",
 	body = "username=Alice&password=" .. string.rep("a", 256) .. "&redirect=" .. url_encode(way_back),
 })
-h.equals("a password at the cap is still checked", crypt_calls, 1)
-unistd.crypt = real_crypt
+h.equals("a password at the cap is still checked", #crypt_settings, 1)
 
 out, ret = run_action("authenticate-post", {
 	method = "POST",
-- 
cgit