End a session when its account's hash changes

A session stayed good for its whole week after the password changed or
the account went away, which is when ending it matters most.

1 file changed, 44 insertions, 29 deletions

diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.luaindex 1b46fba..bce2472 100644--- a/custom/extensions/auth-inline.lua+++ b/custom/extensions/auth-inline.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. @@ -284,8 +286,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 = ""@@ -325,46 +338,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@@ -431,7 +448,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)@@ -441,15 +458,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@@ -464,7 +478,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
diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua
index 1b46fba..bce2472 100644
--- a/custom/extensions/auth-inline.lua
+++ b/custom/extensions/auth-inline.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.
 
@@ -284,8 +286,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 = ""
@@ -325,46 +338,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
@@ -431,7 +448,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)
@@ -441,15 +458,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
@@ -464,7 +478,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