Restyle the filter extension headers and markup

1 file changed, 183 insertions, 158 deletions

diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.luaindex 2553233..8c731a3 100644--- a/custom/extensions/auth-file.lua+++ b/custom/extensions/auth-file.lua@@ -1,40 +1,28 @@--- cgit auth-filter that gates repositories behind a login form and a signed--- session cookie.+-- A cgit auth filter that puts chosen repositories behind a login form and a+-- signed session cookie. cgit consults it on every request once cgitrc names+-- it with auth-filter=lua:/path/to/auth-file.lua, and it answers the+-- authenticate-cookie, authenticate-post and body actions the filter API+-- defines. This variant keeps the accounts, the groups and the per-repository+-- access lists in files on disk, which suits a user set that is large or+-- maintained by something else. The companion auth-inline.lua behaves+-- identically but carries the same lists inside the script.++-- The script runs on Lua 5.1, 5.2, 5.3, 5.4 and LuaJIT, and the runtime has to+-- be the same Lua that cgit was built against. Lua 5.5 will not do, because+-- luaossl has no 5.5 build. ----- This is the FILE-BACKED variant. The user accounts, the groups, and the--- per-repository access lists are read from files on disk, whose paths are set--- among the configuration values below. Edit those files without touching this--- script. This suits larger or externally managed user sets.+-- Serve cgit over HTTPS and terminate TLS in the web server in front of it.+-- The session cookie is marked Secure by default, so a browser only sends it+-- back over HTTPS, and on an instance served over plain HTTP with no TLS+-- anywhere the cookie never comes back and login appears to loop until+-- cookie_insecure below is set. ----- The companion auth-inline.lua behaves identically but keeps its accounts and--- access lists inline in the script itself, which suits a small fixed set of--- users.------ Enable it in cgitrc with---     auth-filter=lua:/path/to/auth-file.lua------ SUPPORTED LUA------ Lua 5.1, 5.2, 5.3, 5.4 and LuaJIT. Lua 5.5 is not supported, because luaossl--- has no 5.5 build. Match the runtime to the Lua that cgit is built against.------ HTTPS IS RECOMMENDED------ Serve cgit over HTTPS. Terminate TLS at the web server in front of cgit. The--- session cookie is marked Secure by default, so a browser only sends it back--- over HTTPS. If you genuinely run cgit over plain HTTP with no TLS anywhere,--- set cookie_insecure below, otherwise the cookie is never returned and login--- appears to loop.------ DEPENDENCIES------     luaossl    OpenSSL binding, provides openssl.rand and openssl.hmac---                <https://github.com/wahern/luaossl>---     luaposix   POSIX binding, provides posix.sys.stat and posix.unistd---                <https://github.com/luaposix/luaposix>------ The reliable cross-platform install is LuaRocks, matched to your Lua--- version. luaossl also needs the OpenSSL development headers present.+-- Two libraries are needed. luaossl provides openssl.rand and openssl.hmac and+-- lives at <https://github.com/wahern/luaossl>, and luaposix provides+-- posix.sys.stat and posix.unistd and lives at+-- <https://github.com/luaposix/luaposix>. The reliable cross-platform way to+-- install them is LuaRocks matched to your Lua version, and luaossl also needs+-- the OpenSSL development headers present. -- --     # Debian and Ubuntu --     sudo apt install luarocks libssl-dev@@ -54,152 +42,158 @@ --     luarocks install luaossl OPENSSL_DIR="$(brew --prefix openssl)" --     luarocks install luaposix ----- Some distributions also package these, for example lua-luaossl and lua-posix--- on Debian. If you use a distribution package, make sure it is built for the--- same Lua version as cgit.------ SECURITY NOTES+-- Some distributions package both as well, for example lua-luaossl and+-- lua-posix on Debian, and such a package has to be built for the same Lua+-- version as cgit. ----- The cookie carries only a username with no server-side session store, so--- deleting an account does not revoke a cookie already issued until it--- expires, and instances that share a secret file accept each other's cookies.--- The login form carries no CSRF token. Both are acceptable for gating read--- access to a git browser. Weigh them before guarding anything more sensitive.+-- The cookie carries only a user name and there is no server-side session+-- store, so deleting an account does not revoke a cookie already issued until+-- it expires, and instances that share a secret file accept each other's+-- cookies. The login form carries no CSRF token. Both are acceptable for+-- gating read access to a git browser, so weigh them before guarding anything+-- more sensitive.  local sysstat = require("posix.sys.stat") local unistd = require("posix.unistd") local rand = require("openssl.rand") local hmac = require("openssl.hmac") --- Configuration, edit these values. Nothing below them needs changing for--- ordinary use.+-- The values that follow are the configuration and are meant to be edited.+-- Nothing below them needs changing for ordinary use. --- Accounts, one per line, as username:hash. Generate a hash with+-- Accounts live one per line as username:hash. Generate a hash with --     mkpasswd -m sha-512 -R 300000 -- This file should not be world-readable. local users_filename = "/etc/cgit-auth/users" --- Group membership, one per line, as groupname:user1,user2,user3,...+-- Group membership lives one per line as groupname:user1,user2,user3 and so+-- on. local groups_filename = "/etc/cgit-auth/groups" --- Per-repository access, one per line, as reponame:group1,group2,...--- A repository listed here is protected. One not listed is public.+-- Per-repository access lives one per line as reponame:group1,group2 and so+-- on. A repository named here is protected and one that is not named is+-- public. The repository name has to match exactly, while group and user names+-- match whatever their case. local repos_filename = "/etc/cgit-auth/repos" --- Where the cookie-signing secret is stored. It is created on first use. It--- must be persistent and writable by cgit. Prefer a path OUTSIDE the cache+-- Where the cookie-signing secret is stored, created on first use. It must be+-- persistent and writable by cgit, and it is worth keeping outside the cache -- root, because pruning the cache would delete a secret kept inside it and -- invalidate every live session. This file should not be world-readable. local secret_filename = "/var/lib/cgit/auth-secret" --- How long a login stays valid, in seconds. Default one week.+-- How long a login stays valid, in seconds. The default is one week. local session_seconds = 7 * 24 * 60 * 60 --- Name of the session cookie. local cookie_name = "cgitauth" --- Path the cookie is scoped to. "/" covers the whole host. Set it to the cgit--- root to scope the cookie more tightly.+-- "/" covers the whole host. Set this to the cgit root to scope the cookie+-- more tightly. local cookie_path = "/" --- Leave false so the cookie is marked Secure and only travels over HTTPS. Set--- it true ONLY if cgit is served over plain HTTP with no TLS anywhere, see the--- HTTPS note in the header.+-- Leave this false so the cookie is marked Secure and only travels over HTTPS.+-- Set it true only if cgit is served over plain HTTP with no TLS anywhere. local cookie_insecure = false  -- 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 username exists.+-- by timing whether the user name exists. local dummy_hash = "$6$rounds=300000$0000000000000000$" --- Module state shared across the open, write and close calls of one request.+-- 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 --- Account and access-list storage. This is the ONLY part that differs from--- auth-inline.lua. Swap these two functions to change where accounts live.+-- The two lookups below, account_hash and repo_userset, are the only part of+-- this script that differs from auth-inline.lua. Replacing them is all it+-- takes to keep accounts somewhere else.  local function trim(s) 	return (string.gsub(s, "^%s*(.-)%s*$", "%1")) end --- Return the stored password hash for a user, or nil. Reads the users file--- fresh each call. A missing or unreadable file, and any unparsable line, are--- skipped rather than fatal.+local function add_names(list, set)+	for name in string.gmatch(list, "([^,]+)") do+		set[trim(name):lower()] = true+	end+end++-- A missing or unreadable users file is not fatal, and neither is a line that+-- does not parse, so a broken file turns every login down rather than failing+-- the request outright. ----- The hash is trimmed as well as the name. f:lines() strips the newline but--- not a carriage return, so a users file saved with CRLF endings would--- otherwise hand crypt a hash with a trailing \r and fail every login with--- nothing in the log to say why.+-- The hash is trimmed as well as the name because reading by line strips the+-- newline but not a carriage return, so a users file saved with CRLF endings+-- would otherwise hand crypt a hash with a trailing \r and fail every login+-- with nothing in the log to say why. function account_hash(user) 	if user == nil then 		return nil 	end 	local wanted = user:lower()-	local f = io.open(users_filename, "r")-	if f == nil then+	local users_file = io.open(users_filename, "r")+	if users_file == nil then 		return nil 	end-	for line in f:lines() do-		local u, h = string.match(line, "(.-):(.+)")-		if u ~= nil and trim(u):lower() == wanted then-			f:close()-			return trim(h)+	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) 		end 	end-	f:close()+	users_file:close() 	return nil end --- Return the set of users allowed to access a repository, keyed by lowercased--- username, or nil if the repository is not protected. A protected repository--- whose groups resolve to no users returns an EMPTY table, which denies--- everyone rather than falling through to public.+-- The users allowed into a repository come back keyed by lowercased name, and+-- a repository the repos file does not name comes back as nil. A protected+-- repository whose groups resolve to nobody comes back as an empty table+-- instead, so it denies everyone rather than falling through to public. function repo_userset(repo) 	if repo == nil then 		return nil 	end 	local groups = nil-	local f = io.open(repos_filename, "r")-	if f ~= nil then-		for line in f:lines() do-			local r, g = string.match(line, "(.-):(.+)")-			if r ~= nil and trim(r) == repo then+	local repos_file = io.open(repos_filename, "r")+	if repos_file ~= nil then+		for line in repos_file:lines() do+			local name, list = string.match(line, "(.-):(.+)")+			if name ~= nil and trim(name) == repo then 				groups = {}-				for group in string.gmatch(g, "([^,]+)") do-					groups[trim(group):lower()] = true-				end+				add_names(list, groups) 				break 			end 		end-		f:close()+		repos_file:close() 	end 	if groups == nil then 		return nil 	end 	local users = {}-	local gf = io.open(groups_filename, "r")-	if gf ~= nil then-		for line in gf:lines() do-			local g, u = string.match(line, "(.-):(.+)")-			if g ~= nil and groups[trim(g):lower()] then-				for user in string.gmatch(u, "([^,]+)") do-					users[trim(user):lower()] = true-				end+	local groups_file = io.open(groups_filename, "r")+	if groups_file ~= nil then+		for line in groups_file:lines() do+			local name, list = string.match(line, "(.-):(.+)")+			if name ~= nil and groups[trim(name):lower()] then+				add_names(list, users) 			end 		end-		gf:close()+		groups_file:close() 	end 	return users end --- Utility functions based on keplerproject/wsapi.+-- The URL helpers below are adapted from keplerproject/wsapi.  function url_decode(str) 	if not str then 		return "" 	end 	str = string.gsub(str, "+", " ")-	str = string.gsub(str, "%%(%x%x)", function(h) return string.char(tonumber(h, 16)) end)+	str = string.gsub(str, "%%(%x%x)", function(hex)+		return string.char(tonumber(hex, 16))+	end) 	str = string.gsub(str, "\r\n", "\n") 	return str end@@ -209,30 +203,31 @@ function url_encode(str) 		return "" 	end 	str = string.gsub(str, "\n", "\r\n")-	str = string.gsub(str, "([^%w ])", function(c) return string.format("%%%02X", string.byte(c)) end)+	str = string.gsub(str, "([^%w ])", function(char)+		return string.format("%%%02X", string.byte(char))+	end) 	str = string.gsub(str, " ", "+") 	return str end  -- Parse an application/x-www-form-urlencoded body. A value may itself contain -- '=', for example a base64 password, so the value runs to the next '&'.-function parse_qs(qs)-	local tab = {}-	for key, val in string.gmatch(qs or "", "([^&=]+)=([^&]*)") do-		tab[url_decode(key)] = url_decode(val)+function parse_query(query)+	local params = {}+	for key, value in string.gmatch(query or "", "([^&=]+)=([^&]*)") do+		params[url_decode(key)] = url_decode(value) 	end-	return tab+	return params end --- Escape the Lua pattern magic characters, so a name is matched literally. local function pattern_escape(s) 	return (string.gsub(s, "([%^%$%(%)%%%.%[%]%*%+%-%?])", "%%%1")) end --- Return the value of the named cookie, or nil. The stored token was already--- url-encoded by secure_value, so it is returned verbatim, which keeps the--- write path (set_cookie) and the read path symmetric. Decoding it here would--- break the signature check for any value carrying a percent escape.+-- The stored token was already URL encoded by secure_value, so it comes back+-- verbatim and the write path in set_cookie stays symmetric with this read+-- path. Decoding it here would break the signature check for any value+-- carrying a percent escape. -- -- The name is escaped because it lands in a pattern. A cookie_name holding a -- magic character, say "cgit-auth", would otherwise read as a pattern and stop@@ -242,16 +237,14 @@ function get_cookie(cookies, name) 	return string.match(cookies, ";" .. pattern_escape(name) .. "=(.-);") end -function tohex(b)-	local x = ""-	for i = 1, #b do-		x = x .. string.format("%.2x", string.byte(b, i))+function tohex(bytes)+	local hex = ""+	for i = 1, #bytes do+		hex = hex .. string.format("%.2x", string.byte(bytes, i)) 	end-	return x+	return hex end --- Cookie construction and validation helpers.- local secret = nil  -- Load the cookie-signing secret, creating it on first use. Failures raise,@@ -263,21 +256,30 @@ function get_secret() 	end 	local secret_file = io.open(secret_filename, "r") 	if secret_file == nil then+		-- The secret is written under a tightened mask so it is not+		-- created readable by anyone but the user cgit runs as, and+		-- the old mask goes back on every way out. 		local old_umask = sysstat.umask(63)-		local temporary_filename = secret_filename .. ".tmp." .. tohex(rand.bytes(16))+		local temporary_filename = secret_filename .. ".tmp." ..+			tohex(rand.bytes(16)) 		local temporary_file = io.open(temporary_filename, "w") 		if temporary_file == nil then 			sysstat.umask(old_umask)-			error("cgit auth: cannot create secret file " .. secret_filename)+			error("cgit auth: cannot create secret file " ..+				secret_filename) 		end 		local wrote = temporary_file:write(tohex(rand.bytes(32))) 		local closed = temporary_file:close() 		if not wrote or not closed then 			os.remove(temporary_filename) 			sysstat.umask(old_umask)-			error("cgit auth: failed writing secret file " .. secret_filename)+			error("cgit auth: failed writing secret file " ..+				secret_filename) 		end-		unistd.link(temporary_filename, secret_filename) -- Intentionally fails if another worker won the race.+		-- The link is meant to fail when another worker won the race,+		-- which leaves that worker's secret in place rather than+		-- replacing it and invalidating the sessions it just signed.+		unistd.link(temporary_filename, secret_filename) 		unistd.unlink(temporary_filename) 		sysstat.umask(old_umask) 		secret_file = io.open(secret_filename, "r")@@ -289,7 +291,8 @@ function get_secret() 	secret_file:close() 	if secret == nil or secret:len() ~= 64 then 		secret = nil-		error("cgit auth: secret file " .. secret_filename .. " is malformed, expected 64 hex characters")+		error("cgit auth: secret file " .. secret_filename ..+			" is malformed, expected 64 hex characters") 	end 	return secret end@@ -315,7 +318,7 @@ function validate_value(expected_field, cookie) 	local field = "" 	local expiration = 0 	local salt = ""-	local chmac = ""+	local signature = ""  	if cookie == nil or cookie:len() < 3 or cookie:sub(1, 1) == "|" then 		return nil@@ -327,9 +330,10 @@ function validate_value(expected_field, cookie) 		elseif i == 1 then 			value = component 		elseif i == 2 then-			-- The expiration must be a plain integer. Rejecting other forms-			-- keeps the signed bytes canonical, since tonumber and tostring of-			-- "1e9" or "100.0" differ across Lua versions.+			-- The expiration must be a plain integer, since+			-- tonumber and tostring of "1e9" or "100.0" differ+			-- across Lua versions and the signed bytes have to+			-- come back byte for byte. 			if not string.match(component, "^%d+$") then 				return nil 			end@@ -337,19 +341,22 @@ function validate_value(expected_field, cookie) 		elseif i == 3 then 			salt = component 		elseif i == 4 then-			chmac = component+			signature = component 		else 			break 		end 		i = i + 1 	end -	if chmac == nil or chmac:len() == 0 then+	if signature == nil or signature:len() == 0 then 		return nil 	end -	-- Compare the HMAC without short-circuiting on the first mismatch.-	if not constant_equals(chmac, tohex(hmac.new(get_secret(), "sha256"):final(field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt))) then+	local payload = field .. "|" .. value .. "|" ..+		tostring(expiration) .. "|" .. salt+	local expected_signature =+		tohex(hmac.new(get_secret(), "sha256"):final(payload))+	if not constant_equals(signature, expected_signature) then 		return nil 	end @@ -371,6 +378,8 @@ function validate_value(expected_field, cookie) 	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) 	if value == nil or value:len() <= 0 then 		return ""@@ -379,13 +388,15 @@ function secure_value(field, value, expiration) 	local salt = tohex(rand.bytes(16)) 	value = url_encode(value) 	field = url_encode(field)-	local authstr = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt-	authstr = authstr .. "|" .. tohex(hmac.new(get_secret(), "sha256"):final(authstr))-	return authstr+	local payload = field .. "|" .. value .. "|" ..+		tostring(expiration) .. "|" .. salt+	local signature = tohex(hmac.new(get_secret(), "sha256"):final(payload))+	return payload .. "|" .. signature end --- Strip control characters that could split an HTTP response header.-function strip_ctl(s)+-- A control character in a header value would let that value split the+-- response, so it is dropped rather than escaped.+function strip_controls(s) 	return (string.gsub(s or "", "%c", "")) end @@ -397,29 +408,33 @@ function constant_equals(a, b) 	end 	local diff = 0 	for i = 1, #a do-		local d = a:byte(i) - b:byte(i)-		diff = diff + d * d+		local delta = a:byte(i) - b:byte(i)+		diff = diff + delta * delta 	end 	return diff == 0 end +-- An empty value is how a login is cleared, since Max-Age=0 tells the browser+-- to drop the cookie it already holds. function set_cookie(cookie, value)-	local attrs = "; HttpOnly; SameSite=Lax; Path=" .. cookie_path+	local attributes = "; HttpOnly; SameSite=Lax; Path=" .. cookie_path 	if not cookie_insecure then-		attrs = attrs .. "; Secure"+		attributes = attributes .. "; Secure" 	end 	if value == "" then-		attrs = attrs .. "; Max-Age=0"+		attributes = attributes .. "; Max-Age=0" 	elseif session_seconds > 0 then-		attrs = attrs .. "; Max-Age=" .. tostring(session_seconds)+		attributes = attributes ..+			"; Max-Age=" .. tostring(session_seconds) 	end-	html("Set-Cookie: " .. cookie .. "=" .. strip_ctl(value) .. attrs .. "\n")+	html("Set-Cookie: " .. cookie .. "=" ..+		strip_controls(value) .. attributes .. "\n") end  function redirect_to(url) 	html("Status: 302 Redirect\n") 	html("Cache-Control: no-cache, no-store\n")-	html("Location: " .. strip_ctl(url) .. "\n")+	html("Location: " .. strip_controls(url) .. "\n") end  function not_found()@@ -427,9 +442,12 @@ function not_found() 	html("Cache-Control: no-cache, no-store\n\n") end --- Authentication actions. Identical to auth-inline.lua from here down.+-- The three actions cgit can ask for follow, and they are the same in both+-- variants, so a change to one belongs in the other. --- Sets HTTP cookie headers based on post and sets up redirection.+-- The redirect goes out before the password is checked, so a wrong password+-- and a right one answer with the same status and location and differ only in+-- the cookie. function authenticate_post() 	local redirect = validate_value("redirect", post["redirect"]) @@ -454,7 +472,8 @@ function authenticate_post() 	end  	if ok then-		set_cookie(cookie_name, secure_value("username", username, os.time() + session_seconds))+		set_cookie(cookie_name, secure_value("username", username,+			os.time() + session_seconds)) 	else 		set_cookie(cookie_name, "") 	end@@ -463,22 +482,26 @@ function authenticate_post() 	return 0 end --- Returns 1 if the cookie is valid and 0 if it is not.+-- cgit reads the answer, where 1 lets the request through and 0 sends it to+-- the login form. function authenticate_cookie() 	local accepted_users = repo_userset(cgit["repo"]) 	if accepted_users == nil then-		-- The repository is not protected.+		-- A repository nothing lists is public. 		return 1 	end -	local username = validate_value("username", get_cookie(http["cookie"], cookie_name))+	local username = validate_value("username",+		get_cookie(http["cookie"], cookie_name)) 	if username == nil or not accepted_users[username:lower()] then 		return 0 	end 	return 1 end --- Prints the html for the login form.+-- cgit calls this to fill the page body once the cookie has been turned down.+-- The form carries a signed redirect token, so the browser lands back on the+-- page that was asked for. function body() 	local target = cgit["url"] 	if not is_safe_redirect(target) then@@ -501,8 +524,10 @@ function body() 	return 0 end --- Wrapper around the filter API, exposing the http, cgit and post tables to--- the functions above.+-- cgit calls filter_open with the action name followed by the request fields+-- in a fixed order, so they are unpacked here into the tables the functions+-- above read. Only a post reaches filter_write, carrying the form body, and+-- filter_close is where the action finally runs and answers cgit.  local actions = {} actions["authenticate-post"] = authenticate_post@@ -532,12 +557,12 @@ end  function filter_close() 	if action == nil then-		-- Unknown action, deny rather than raise.+		-- An unknown action denies rather than raising. 		return 0 	end 	return action() end  function filter_write(str)-	post = parse_qs(str)+	post = parse_query(str) end
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua
index 2553233..8c731a3 100644
--- a/custom/extensions/auth-file.lua
+++ b/custom/extensions/auth-file.lua
@@ -1,40 +1,28 @@
--- cgit auth-filter that gates repositories behind a login form and a signed
--- session cookie.
+-- A cgit auth filter that puts chosen repositories behind a login form and a
+-- signed session cookie. cgit consults it on every request once cgitrc names
+-- it with auth-filter=lua:/path/to/auth-file.lua, and it answers the
+-- authenticate-cookie, authenticate-post and body actions the filter API
+-- defines. This variant keeps the accounts, the groups and the per-repository
+-- access lists in files on disk, which suits a user set that is large or
+-- maintained by something else. The companion auth-inline.lua behaves
+-- identically but carries the same lists inside the script.
+
+-- The script runs on Lua 5.1, 5.2, 5.3, 5.4 and LuaJIT, and the runtime has to
+-- be the same Lua that cgit was built against. Lua 5.5 will not do, because
+-- luaossl has no 5.5 build.
 --
--- This is the FILE-BACKED variant. The user accounts, the groups, and the
--- per-repository access lists are read from files on disk, whose paths are set
--- among the configuration values below. Edit those files without touching this
--- script. This suits larger or externally managed user sets.
+-- Serve cgit over HTTPS and terminate TLS in the web server in front of it.
+-- The session cookie is marked Secure by default, so a browser only sends it
+-- back over HTTPS, and on an instance served over plain HTTP with no TLS
+-- anywhere the cookie never comes back and login appears to loop until
+-- cookie_insecure below is set.
 --
--- The companion auth-inline.lua behaves identically but keeps its accounts and
--- access lists inline in the script itself, which suits a small fixed set of
--- users.
---
--- Enable it in cgitrc with
---     auth-filter=lua:/path/to/auth-file.lua
---
--- SUPPORTED LUA
---
--- Lua 5.1, 5.2, 5.3, 5.4 and LuaJIT. Lua 5.5 is not supported, because luaossl
--- has no 5.5 build. Match the runtime to the Lua that cgit is built against.
---
--- HTTPS IS RECOMMENDED
---
--- Serve cgit over HTTPS. Terminate TLS at the web server in front of cgit. The
--- session cookie is marked Secure by default, so a browser only sends it back
--- over HTTPS. If you genuinely run cgit over plain HTTP with no TLS anywhere,
--- set cookie_insecure below, otherwise the cookie is never returned and login
--- appears to loop.
---
--- DEPENDENCIES
---
---     luaossl    OpenSSL binding, provides openssl.rand and openssl.hmac
---                <https://github.com/wahern/luaossl>
---     luaposix   POSIX binding, provides posix.sys.stat and posix.unistd
---                <https://github.com/luaposix/luaposix>
---
--- The reliable cross-platform install is LuaRocks, matched to your Lua
--- version. luaossl also needs the OpenSSL development headers present.
+-- Two libraries are needed. luaossl provides openssl.rand and openssl.hmac and
+-- lives at <https://github.com/wahern/luaossl>, and luaposix provides
+-- posix.sys.stat and posix.unistd and lives at
+-- <https://github.com/luaposix/luaposix>. The reliable cross-platform way to
+-- install them is LuaRocks matched to your Lua version, and luaossl also needs
+-- the OpenSSL development headers present.
 --
 --     # Debian and Ubuntu
 --     sudo apt install luarocks libssl-dev
@@ -54,152 +42,158 @@
 --     luarocks install luaossl OPENSSL_DIR="$(brew --prefix openssl)"
 --     luarocks install luaposix
 --
--- Some distributions also package these, for example lua-luaossl and lua-posix
--- on Debian. If you use a distribution package, make sure it is built for the
--- same Lua version as cgit.
---
--- SECURITY NOTES
+-- Some distributions package both as well, for example lua-luaossl and
+-- lua-posix on Debian, and such a package has to be built for the same Lua
+-- version as cgit.
 --
--- The cookie carries only a username with no server-side session store, so
--- deleting an account does not revoke a cookie already issued until it
--- expires, and instances that share a secret file accept each other's cookies.
--- The login form carries no CSRF token. Both are acceptable for gating read
--- access to a git browser. Weigh them before guarding anything more sensitive.
+-- The cookie carries only a user name and there is no server-side session
+-- store, so deleting an account does not revoke a cookie already issued until
+-- it expires, and instances that share a secret file accept each other's
+-- cookies. The login form carries no CSRF token. Both are acceptable for
+-- gating read access to a git browser, so weigh them before guarding anything
+-- more sensitive.
 
 local sysstat = require("posix.sys.stat")
 local unistd = require("posix.unistd")
 local rand = require("openssl.rand")
 local hmac = require("openssl.hmac")
 
--- Configuration, edit these values. Nothing below them needs changing for
--- ordinary use.
+-- The values that follow are the configuration and are meant to be edited.
+-- Nothing below them needs changing for ordinary use.
 
--- Accounts, one per line, as username:hash. Generate a hash with
+-- Accounts live one per line as username:hash. Generate a hash with
 --     mkpasswd -m sha-512 -R 300000
 -- This file should not be world-readable.
 local users_filename = "/etc/cgit-auth/users"
 
--- Group membership, one per line, as groupname:user1,user2,user3,...
+-- Group membership lives one per line as groupname:user1,user2,user3 and so
+-- on.
 local groups_filename = "/etc/cgit-auth/groups"
 
--- Per-repository access, one per line, as reponame:group1,group2,...
--- A repository listed here is protected. One not listed is public.
+-- Per-repository access lives one per line as reponame:group1,group2 and so
+-- on. A repository named here is protected and one that is not named is
+-- public. The repository name has to match exactly, while group and user names
+-- match whatever their case.
 local repos_filename = "/etc/cgit-auth/repos"
 
--- Where the cookie-signing secret is stored. It is created on first use. It
--- must be persistent and writable by cgit. Prefer a path OUTSIDE the cache
+-- Where the cookie-signing secret is stored, created on first use. It must be
+-- persistent and writable by cgit, and it is worth keeping outside the cache
 -- root, because pruning the cache would delete a secret kept inside it and
 -- invalidate every live session. This file should not be world-readable.
 local secret_filename = "/var/lib/cgit/auth-secret"
 
--- How long a login stays valid, in seconds. Default one week.
+-- How long a login stays valid, in seconds. The default is one week.
 local session_seconds = 7 * 24 * 60 * 60
 
--- Name of the session cookie.
 local cookie_name = "cgitauth"
 
--- Path the cookie is scoped to. "/" covers the whole host. Set it to the cgit
--- root to scope the cookie more tightly.
+-- "/" covers the whole host. Set this to the cgit root to scope the cookie
+-- more tightly.
 local cookie_path = "/"
 
--- Leave false so the cookie is marked Secure and only travels over HTTPS. Set
--- it true ONLY if cgit is served over plain HTTP with no TLS anywhere, see the
--- HTTPS note in the header.
+-- Leave this false so the cookie is marked Secure and only travels over HTTPS.
+-- Set it true only if cgit is served over plain HTTP with no TLS anywhere.
 local cookie_insecure = false
 
 -- 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 username exists.
+-- by timing whether the user name exists.
 local dummy_hash = "$6$rounds=300000$0000000000000000$"
 
--- Module state shared across the open, write and close calls of one request.
+-- 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
 
--- Account and access-list storage. This is the ONLY part that differs from
--- auth-inline.lua. Swap these two functions to change where accounts live.
+-- The two lookups below, account_hash and repo_userset, are the only part of
+-- this script that differs from auth-inline.lua. Replacing them is all it
+-- takes to keep accounts somewhere else.
 
 local function trim(s)
 	return (string.gsub(s, "^%s*(.-)%s*$", "%1"))
 end
 
--- Return the stored password hash for a user, or nil. Reads the users file
--- fresh each call. A missing or unreadable file, and any unparsable line, are
--- skipped rather than fatal.
+local function add_names(list, set)
+	for name in string.gmatch(list, "([^,]+)") do
+		set[trim(name):lower()] = true
+	end
+end
+
+-- A missing or unreadable users file is not fatal, and neither is a line that
+-- does not parse, so a broken file turns every login down rather than failing
+-- the request outright.
 --
--- The hash is trimmed as well as the name. f:lines() strips the newline but
--- not a carriage return, so a users file saved with CRLF endings would
--- otherwise hand crypt a hash with a trailing \r and fail every login with
--- nothing in the log to say why.
+-- The hash is trimmed as well as the name because reading by line strips the
+-- newline but not a carriage return, so a users file saved with CRLF endings
+-- would otherwise hand crypt a hash with a trailing \r and fail every login
+-- with nothing in the log to say why.
 function account_hash(user)
 	if user == nil then
 		return nil
 	end
 	local wanted = user:lower()
-	local f = io.open(users_filename, "r")
-	if f == nil then
+	local users_file = io.open(users_filename, "r")
+	if users_file == nil then
 		return nil
 	end
-	for line in f:lines() do
-		local u, h = string.match(line, "(.-):(.+)")
-		if u ~= nil and trim(u):lower() == wanted then
-			f:close()
-			return trim(h)
+	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)
 		end
 	end
-	f:close()
+	users_file:close()
 	return nil
 end
 
--- Return the set of users allowed to access a repository, keyed by lowercased
--- username, or nil if the repository is not protected. A protected repository
--- whose groups resolve to no users returns an EMPTY table, which denies
--- everyone rather than falling through to public.
+-- The users allowed into a repository come back keyed by lowercased name, and
+-- a repository the repos file does not name comes back as nil. A protected
+-- repository whose groups resolve to nobody comes back as an empty table
+-- instead, so it denies everyone rather than falling through to public.
 function repo_userset(repo)
 	if repo == nil then
 		return nil
 	end
 	local groups = nil
-	local f = io.open(repos_filename, "r")
-	if f ~= nil then
-		for line in f:lines() do
-			local r, g = string.match(line, "(.-):(.+)")
-			if r ~= nil and trim(r) == repo then
+	local repos_file = io.open(repos_filename, "r")
+	if repos_file ~= nil then
+		for line in repos_file:lines() do
+			local name, list = string.match(line, "(.-):(.+)")
+			if name ~= nil and trim(name) == repo then
 				groups = {}
-				for group in string.gmatch(g, "([^,]+)") do
-					groups[trim(group):lower()] = true
-				end
+				add_names(list, groups)
 				break
 			end
 		end
-		f:close()
+		repos_file:close()
 	end
 	if groups == nil then
 		return nil
 	end
 	local users = {}
-	local gf = io.open(groups_filename, "r")
-	if gf ~= nil then
-		for line in gf:lines() do
-			local g, u = string.match(line, "(.-):(.+)")
-			if g ~= nil and groups[trim(g):lower()] then
-				for user in string.gmatch(u, "([^,]+)") do
-					users[trim(user):lower()] = true
-				end
+	local groups_file = io.open(groups_filename, "r")
+	if groups_file ~= nil then
+		for line in groups_file:lines() do
+			local name, list = string.match(line, "(.-):(.+)")
+			if name ~= nil and groups[trim(name):lower()] then
+				add_names(list, users)
 			end
 		end
-		gf:close()
+		groups_file:close()
 	end
 	return users
 end
 
--- Utility functions based on keplerproject/wsapi.
+-- The URL helpers below are adapted from keplerproject/wsapi.
 
 function url_decode(str)
 	if not str then
 		return ""
 	end
 	str = string.gsub(str, "+", " ")
-	str = string.gsub(str, "%%(%x%x)", function(h) return string.char(tonumber(h, 16)) end)
+	str = string.gsub(str, "%%(%x%x)", function(hex)
+		return string.char(tonumber(hex, 16))
+	end)
 	str = string.gsub(str, "\r\n", "\n")
 	return str
 end
@@ -209,30 +203,31 @@ function url_encode(str)
 		return ""
 	end
 	str = string.gsub(str, "\n", "\r\n")
-	str = string.gsub(str, "([^%w ])", function(c) return string.format("%%%02X", string.byte(c)) end)
+	str = string.gsub(str, "([^%w ])", function(char)
+		return string.format("%%%02X", string.byte(char))
+	end)
 	str = string.gsub(str, " ", "+")
 	return str
 end
 
 -- Parse an application/x-www-form-urlencoded body. A value may itself contain
 -- '=', for example a base64 password, so the value runs to the next '&'.
-function parse_qs(qs)
-	local tab = {}
-	for key, val in string.gmatch(qs or "", "([^&=]+)=([^&]*)") do
-		tab[url_decode(key)] = url_decode(val)
+function parse_query(query)
+	local params = {}
+	for key, value in string.gmatch(query or "", "([^&=]+)=([^&]*)") do
+		params[url_decode(key)] = url_decode(value)
 	end
-	return tab
+	return params
 end
 
--- Escape the Lua pattern magic characters, so a name is matched literally.
 local function pattern_escape(s)
 	return (string.gsub(s, "([%^%$%(%)%%%.%[%]%*%+%-%?])", "%%%1"))
 end
 
--- Return the value of the named cookie, or nil. The stored token was already
--- url-encoded by secure_value, so it is returned verbatim, which keeps the
--- write path (set_cookie) and the read path symmetric. Decoding it here would
--- break the signature check for any value carrying a percent escape.
+-- The stored token was already URL encoded by secure_value, so it comes back
+-- verbatim and the write path in set_cookie stays symmetric with this read
+-- path. Decoding it here would break the signature check for any value
+-- carrying a percent escape.
 --
 -- The name is escaped because it lands in a pattern. A cookie_name holding a
 -- magic character, say "cgit-auth", would otherwise read as a pattern and stop
@@ -242,16 +237,14 @@ function get_cookie(cookies, name)
 	return string.match(cookies, ";" .. pattern_escape(name) .. "=(.-);")
 end
 
-function tohex(b)
-	local x = ""
-	for i = 1, #b do
-		x = x .. string.format("%.2x", string.byte(b, i))
+function tohex(bytes)
+	local hex = ""
+	for i = 1, #bytes do
+		hex = hex .. string.format("%.2x", string.byte(bytes, i))
 	end
-	return x
+	return hex
 end
 
--- Cookie construction and validation helpers.
-
 local secret = nil
 
 -- Load the cookie-signing secret, creating it on first use. Failures raise,
@@ -263,21 +256,30 @@ function get_secret()
 	end
 	local secret_file = io.open(secret_filename, "r")
 	if secret_file == nil then
+		-- The secret is written under a tightened mask so it is not
+		-- created readable by anyone but the user cgit runs as, and
+		-- the old mask goes back on every way out.
 		local old_umask = sysstat.umask(63)
-		local temporary_filename = secret_filename .. ".tmp." .. tohex(rand.bytes(16))
+		local temporary_filename = secret_filename .. ".tmp." ..
+			tohex(rand.bytes(16))
 		local temporary_file = io.open(temporary_filename, "w")
 		if temporary_file == nil then
 			sysstat.umask(old_umask)
-			error("cgit auth: cannot create secret file " .. secret_filename)
+			error("cgit auth: cannot create secret file " ..
+				secret_filename)
 		end
 		local wrote = temporary_file:write(tohex(rand.bytes(32)))
 		local closed = temporary_file:close()
 		if not wrote or not closed then
 			os.remove(temporary_filename)
 			sysstat.umask(old_umask)
-			error("cgit auth: failed writing secret file " .. secret_filename)
+			error("cgit auth: failed writing secret file " ..
+				secret_filename)
 		end
-		unistd.link(temporary_filename, secret_filename) -- Intentionally fails if another worker won the race.
+		-- The link is meant to fail when another worker won the race,
+		-- which leaves that worker's secret in place rather than
+		-- replacing it and invalidating the sessions it just signed.
+		unistd.link(temporary_filename, secret_filename)
 		unistd.unlink(temporary_filename)
 		sysstat.umask(old_umask)
 		secret_file = io.open(secret_filename, "r")
@@ -289,7 +291,8 @@ function get_secret()
 	secret_file:close()
 	if secret == nil or secret:len() ~= 64 then
 		secret = nil
-		error("cgit auth: secret file " .. secret_filename .. " is malformed, expected 64 hex characters")
+		error("cgit auth: secret file " .. secret_filename ..
+			" is malformed, expected 64 hex characters")
 	end
 	return secret
 end
@@ -315,7 +318,7 @@ function validate_value(expected_field, cookie)
 	local field = ""
 	local expiration = 0
 	local salt = ""
-	local chmac = ""
+	local signature = ""
 
 	if cookie == nil or cookie:len() < 3 or cookie:sub(1, 1) == "|" then
 		return nil
@@ -327,9 +330,10 @@ function validate_value(expected_field, cookie)
 		elseif i == 1 then
 			value = component
 		elseif i == 2 then
-			-- The expiration must be a plain integer. Rejecting other forms
-			-- keeps the signed bytes canonical, since tonumber and tostring of
-			-- "1e9" or "100.0" differ across Lua versions.
+			-- The expiration must be a plain integer, since
+			-- tonumber and tostring of "1e9" or "100.0" differ
+			-- across Lua versions and the signed bytes have to
+			-- come back byte for byte.
 			if not string.match(component, "^%d+$") then
 				return nil
 			end
@@ -337,19 +341,22 @@ function validate_value(expected_field, cookie)
 		elseif i == 3 then
 			salt = component
 		elseif i == 4 then
-			chmac = component
+			signature = component
 		else
 			break
 		end
 		i = i + 1
 	end
 
-	if chmac == nil or chmac:len() == 0 then
+	if signature == nil or signature:len() == 0 then
 		return nil
 	end
 
-	-- Compare the HMAC without short-circuiting on the first mismatch.
-	if not constant_equals(chmac, tohex(hmac.new(get_secret(), "sha256"):final(field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt))) then
+	local payload = field .. "|" .. value .. "|" ..
+		tostring(expiration) .. "|" .. salt
+	local expected_signature =
+		tohex(hmac.new(get_secret(), "sha256"):final(payload))
+	if not constant_equals(signature, expected_signature) then
 		return nil
 	end
 
@@ -371,6 +378,8 @@ function validate_value(expected_field, cookie)
 	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)
 	if value == nil or value:len() <= 0 then
 		return ""
@@ -379,13 +388,15 @@ function secure_value(field, value, expiration)
 	local salt = tohex(rand.bytes(16))
 	value = url_encode(value)
 	field = url_encode(field)
-	local authstr = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt
-	authstr = authstr .. "|" .. tohex(hmac.new(get_secret(), "sha256"):final(authstr))
-	return authstr
+	local payload = field .. "|" .. value .. "|" ..
+		tostring(expiration) .. "|" .. salt
+	local signature = tohex(hmac.new(get_secret(), "sha256"):final(payload))
+	return payload .. "|" .. signature
 end
 
--- Strip control characters that could split an HTTP response header.
-function strip_ctl(s)
+-- A control character in a header value would let that value split the
+-- response, so it is dropped rather than escaped.
+function strip_controls(s)
 	return (string.gsub(s or "", "%c", ""))
 end
 
@@ -397,29 +408,33 @@ function constant_equals(a, b)
 	end
 	local diff = 0
 	for i = 1, #a do
-		local d = a:byte(i) - b:byte(i)
-		diff = diff + d * d
+		local delta = a:byte(i) - b:byte(i)
+		diff = diff + delta * delta
 	end
 	return diff == 0
 end
 
+-- An empty value is how a login is cleared, since Max-Age=0 tells the browser
+-- to drop the cookie it already holds.
 function set_cookie(cookie, value)
-	local attrs = "; HttpOnly; SameSite=Lax; Path=" .. cookie_path
+	local attributes = "; HttpOnly; SameSite=Lax; Path=" .. cookie_path
 	if not cookie_insecure then
-		attrs = attrs .. "; Secure"
+		attributes = attributes .. "; Secure"
 	end
 	if value == "" then
-		attrs = attrs .. "; Max-Age=0"
+		attributes = attributes .. "; Max-Age=0"
 	elseif session_seconds > 0 then
-		attrs = attrs .. "; Max-Age=" .. tostring(session_seconds)
+		attributes = attributes ..
+			"; Max-Age=" .. tostring(session_seconds)
 	end
-	html("Set-Cookie: " .. cookie .. "=" .. strip_ctl(value) .. attrs .. "\n")
+	html("Set-Cookie: " .. cookie .. "=" ..
+		strip_controls(value) .. attributes .. "\n")
 end
 
 function redirect_to(url)
 	html("Status: 302 Redirect\n")
 	html("Cache-Control: no-cache, no-store\n")
-	html("Location: " .. strip_ctl(url) .. "\n")
+	html("Location: " .. strip_controls(url) .. "\n")
 end
 
 function not_found()
@@ -427,9 +442,12 @@ function not_found()
 	html("Cache-Control: no-cache, no-store\n\n")
 end
 
--- Authentication actions. Identical to auth-inline.lua from here down.
+-- The three actions cgit can ask for follow, and they are the same in both
+-- variants, so a change to one belongs in the other.
 
--- Sets HTTP cookie headers based on post and sets up redirection.
+-- The redirect goes out before the password is checked, so a wrong password
+-- and a right one answer with the same status and location and differ only in
+-- the cookie.
 function authenticate_post()
 	local redirect = validate_value("redirect", post["redirect"])
 
@@ -454,7 +472,8 @@ function authenticate_post()
 	end
 
 	if ok then
-		set_cookie(cookie_name, secure_value("username", username, os.time() + session_seconds))
+		set_cookie(cookie_name, secure_value("username", username,
+			os.time() + session_seconds))
 	else
 		set_cookie(cookie_name, "")
 	end
@@ -463,22 +482,26 @@ function authenticate_post()
 	return 0
 end
 
--- Returns 1 if the cookie is valid and 0 if it is not.
+-- cgit reads the answer, where 1 lets the request through and 0 sends it to
+-- the login form.
 function authenticate_cookie()
 	local accepted_users = repo_userset(cgit["repo"])
 	if accepted_users == nil then
-		-- The repository is not protected.
+		-- A repository nothing lists is public.
 		return 1
 	end
 
-	local username = validate_value("username", get_cookie(http["cookie"], cookie_name))
+	local username = validate_value("username",
+		get_cookie(http["cookie"], cookie_name))
 	if username == nil or not accepted_users[username:lower()] then
 		return 0
 	end
 	return 1
 end
 
--- Prints the html for the login form.
+-- cgit calls this to fill the page body once the cookie has been turned down.
+-- The form carries a signed redirect token, so the browser lands back on the
+-- page that was asked for.
 function body()
 	local target = cgit["url"]
 	if not is_safe_redirect(target) then
@@ -501,8 +524,10 @@ function body()
 	return 0
 end
 
--- Wrapper around the filter API, exposing the http, cgit and post tables to
--- the functions above.
+-- cgit calls filter_open with the action name followed by the request fields
+-- in a fixed order, so they are unpacked here into the tables the functions
+-- above read. Only a post reaches filter_write, carrying the form body, and
+-- filter_close is where the action finally runs and answers cgit.
 
 local actions = {}
 actions["authenticate-post"] = authenticate_post
@@ -532,12 +557,12 @@ end
 
 function filter_close()
 	if action == nil then
-		-- Unknown action, deny rather than raise.
+		-- An unknown action denies rather than raising.
 		return 0
 	end
 	return action()
 end
 
 function filter_write(str)
-	post = parse_qs(str)
+	post = parse_query(str)
 end