1 file changed, 44 insertions, 29 deletions
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua
index 0573549..2ef2328 100644
--- a/custom/extensions/auth-file.lua
+++ b/custom/extensions/auth-file.lua
@@ -42,10 +42,12 @@
 --     luarocks install luaossl OPENSSL_DIR="$(brew --prefix openssl)"
 --     luarocks install luaposix
 --
--- There is no server-side session store and no CSRF token, so a deleted
--- account's cookie stays valid until it expires and instances sharing a
--- secret file accept each other's cookies. That is acceptable for gating
--- read access to a git browser, so weigh it before guarding anything more.
+-- A session is signed together with its account's hash, so changing the
+-- password or deleting the account ends it. There is no server-side session
+-- store and no CSRF token, so instances sharing a secret file accept each
+-- other's cookies, and only a new secret file ends every session at once.
+-- That is acceptable for gating read access to a git browser, so weigh it
+-- before guarding anything more.
 
 -- The values that follow are the configuration and are meant to be edited.
 
@@ -323,8 +325,19 @@ function is_safe_redirect(url)
 	return true
 end
 
+-- The signature covers the payload and a binding that the cookie never
+-- carries, so a value only verifies while its binding stays the same.
+local function sign(payload, binding)
+	local hmac = require("openssl.hmac")
+	local signed = payload .. "|" .. (binding or "")
+	return tohex(hmac.new(get_secret(), "sha256"):final(signed))
+end
+
 -- Return the value carried by a signed cookie, or nil if it does not verify.
-function validate_value(expected_field, cookie)
+-- bind, when given, maps the value to the binding its signature has to cover,
+-- or to nil to turn it down, which is how a session meets its account's
+-- current hash.
+function validate_value(expected_field, cookie, bind)
 	local i = 0
 	local value = ""
 	local field = ""
@@ -364,46 +377,50 @@ function validate_value(expected_field, cookie)
 		return nil
 	end
 
-	local payload = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt
-	local hmac = require("openssl.hmac")
-	local expected_signature = tohex(hmac.new(get_secret(), "sha256"):final(payload))
-	if not constant_equals(signature, expected_signature) then
+	if url_decode(field) ~= expected_field then
 		return nil
 	end
 
-	-- An expiration of 0 never expires and is used for the redirect token.
-	if expiration ~= 0 and expiration <= os.time() then
+	local decoded = url_decode(value)
+	-- Reject values carrying control characters so a signed value cannot
+	-- smuggle CR or LF into a response header.
+	if decoded:find("%c") then
 		return nil
 	end
 
-	if url_decode(field) ~= expected_field then
+	local binding = ""
+	if bind ~= nil then
+		binding = bind(decoded)
+		if binding == nil then
+			return nil
+		end
+	end
+
+	local payload = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt
+	if not constant_equals(signature, sign(payload, binding)) then
 		return nil
 	end
 
-	local decoded = url_decode(value)
-	-- Reject values carrying control characters so a signed value cannot
-	-- smuggle CR or LF into a response header.
-	if decoded:find("%c") then
+	-- An expiration of 0 never expires and is used for the redirect token.
+	if expiration ~= 0 and expiration <= os.time() then
 		return nil
 	end
 	return decoded
 end
 
 -- The layout built here is what validate_value takes apart again, so the two
--- have to move together.
-function secure_value(field, value, expiration)
+-- have to move together. The binding, if any, is signed but left out.
+function secure_value(field, value, expiration, binding)
 	if value == nil or value:len() <= 0 then
 		return ""
 	end
 
 	local rand = require("openssl.rand")
-	local hmac = require("openssl.hmac")
 	local salt = tohex(rand.bytes(16))
 	value = url_encode(value)
 	field = url_encode(field)
 	local payload = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt
-	local signature = tohex(hmac.new(get_secret(), "sha256"):final(payload))
-	return payload .. "|" .. signature
+	return payload .. "|" .. sign(payload, binding)
 end
 
 -- A control character in a header value would let that value split the
@@ -470,7 +487,7 @@ function authenticate_post()
 
 	local username = post["username"]
 	local password = post["password"]
-	local ok = false
+	local session = ""
 	if username ~= nil and password ~= nil and #password <= max_password_bytes then
 		local unistd = require("posix.unistd")
 		local hash, stand_in = account_hash(username)
@@ -480,15 +497,12 @@ function authenticate_post()
 				unistd.crypt(password, stand_in)
 			end
 		elseif constant_equals(hash, unistd.crypt(password, hash)) then
-			ok = true
+			session = secure_value("username", username, os.time() + session_seconds, hash)
 		end
 	end
 
-	if ok then
-		set_cookie(cookie_name, secure_value("username", username, os.time() + session_seconds))
-	else
-		set_cookie(cookie_name, "")
-	end
+	-- A failed login answers with an empty session, which clears the cookie.
+	set_cookie(cookie_name, session)
 
 	html("\n")
 	return 0
@@ -503,7 +517,8 @@ function authenticate_cookie()
 		return 1
 	end
 
-	local username = validate_value("username", get_cookie(http["cookie"], cookie_name))
+	local cookie = get_cookie(http["cookie"], cookie_name)
+	local username = validate_value("username", cookie, account_hash)
 	if username == nil or not accepted_users[username:lower()] then
 		return 0
 	end