1 file changed, 32 insertions, 95 deletions

diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.luaindex a03e1ea..ab56ee1 100644--- a/custom/extensions/auth-file.lua+++ b/custom/extensions/auth-file.lua@@ -53,7 +53,6 @@ -- still lists a protected repository with its description unless cgitrc sets -- repo.hide=1 for it. --- The values that follow are the configuration and are meant to be edited.  -- Accounts live one per line as username:hash, where the hash is any kind this -- system's crypt(3) checks. yescrypt ($y$) is what libxcrypt recommends, as on@@ -79,8 +78,8 @@ local groups_filename = "/etc/cgit-auth/groups" -- 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 regardless of case. While this file cannot be read every repository--- is refused, since nothing then tells the protected ones apart.+-- match regardless of case. While this file cannot be read every page, the+-- index too, is refused, since nothing then tells the protected ones apart. local repos_filename = "/etc/cgit-auth/repos"  -- Where the cookie-signing secret is stored, created on first use. It must be@@ -105,6 +104,7 @@ local cookie_insecure = false -- the password's length, and one longer than this is turned down unchecked. local max_password_bytes = 256 + -- A browser accepts a cookie named __Host- only when it is Secure, covers the -- whole host and was set by this host, so no sibling subdomain can plant a -- session in its place. Either cookie setting above can rule that out.@@ -113,12 +113,18 @@ if not cookie_insecure and cookie_path == "/" then 	session_cookie = "__Host-" .. cookie_name end +-- A cookie carries its expiry and Max-Age as whole seconds, while on Lua 5.3+-- and later a length worked out with a fraction, such as 1.5 days, is a float+-- that tostring would write with a trailing ".0".+session_seconds = math.floor(session_seconds)+ -- 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 --- The two lookups below, account_hash and repo_userset, are the only part--- of this script that differs from auth-inline.lua.+-- Apart from the configuration, the lookups below, account_hash and+-- repo_userset with their helpers, are the only code that differs from+-- auth-inline.lua, which defines trim later, beside get_cookie.  -- Lua patterns backtrack, so "^%s*(.-)%s*$" costs the square of a run of -- spaces inside the text. Finding each end directly stays linear, which@@ -152,9 +158,6 @@ end -- 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 	local wanted = user:lower() 	local users_file = io.open(users_filename, "r") 	if users_file == nil then@@ -180,9 +183,6 @@ end -- 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 repos_file, err = io.open(repos_filename, "r") 	if repos_file == nil then@@ -215,25 +215,17 @@ function repo_userset(repo) 	return users end --- The URL helpers below are adapted from 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(hex) 		return string.char(tonumber(hex, 16)) 	end)-	str = string.gsub(str, "\r\n", "\n") 	return str end  function url_encode(str)-	if not str then-		return ""-	end-	str = string.gsub(str, "\n", "\r\n") 	str = string.gsub(str, "([^%w ])", function(char) 		return string.format("%%%02X", string.byte(char)) 	end)@@ -245,13 +237,13 @@ end -- '=', for example a base64 password, so the value runs to the next '&'. function parse_query(query) 	local params = {}-	for key, value in string.gmatch(query or "", "([^&=]+)=([^&]*)") do+	for key, value in string.gmatch(query, "([^&=]+)=([^&]*)") do 		params[url_decode(key)] = url_decode(value) 	end 	return params end --- The token comes back still URL encoded by secure_value. Decoding it here+-- The token comes back still url encoded by secure_value. Decoding it here -- would break the signature check for any value carrying a percent escape. function get_cookie(cookies, name) 	for raw in string.gmatch(cookies or "", "[^;]+") do@@ -350,46 +342,9 @@ end -- 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 = ""-	local expiration = 0-	local salt = ""-	local signature = ""--	if cookie == nil or cookie:len() < 3 or cookie:sub(1, 1) == "|" then-		return nil-	end--	for component in string.gmatch(cookie, "[^|]+") do-		if i == 0 then-			field = component-		elseif i == 1 then-			value = component-		elseif i == 2 then-			-- 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-			expiration = tonumber(component)-		elseif i == 3 then-			salt = component-		elseif i == 4 then-			signature = component-		else-			break-		end-		i = i + 1-	end--	if signature == nil or signature:len() == 0 then-		return nil-	end--	if url_decode(field) ~= expected_field then+	local field, value, expiration, salt, signature =+		string.match(cookie or "", "^([^|]+)|([^|]+)|(%d+)|([^|]+)|([^|]+)$")+	if signature == nil or url_decode(field) ~= expected_field then 		return nil 	end @@ -408,12 +363,13 @@ function validate_value(expected_field, cookie, bind) 		end 	end -	local payload = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt+	local payload = field .. "|" .. value .. "|" .. expiration .. "|" .. salt 	if not constant_equals(signature, sign(payload, binding)) then 		return nil 	end  	-- An expiration of 0 never expires and is used for the redirect token.+	expiration = tonumber(expiration) 	if expiration ~= 0 and expiration <= os.time() then 		return nil 	end@@ -435,12 +391,6 @@ function secure_value(field, value, expiration, binding) 	return payload .. "|" .. sign(payload, binding) end --- 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- -- Compare two strings in time that does not depend on how many leading bytes -- match, so a mismatch position is not revealed by timing. function constant_equals(a, b)@@ -456,7 +406,8 @@ function constant_equals(a, b) end  -- An empty value is how a login is cleared, since Max-Age=0 tells the browser--- to drop the cookie it already holds.+-- to drop the cookie it already holds. Any other value is a token secure_value+-- built, which holds no control character to split the response with. function set_cookie(cookie, value) 	local attributes = "; HttpOnly; SameSite=Lax; Path=" .. cookie_path 	if not cookie_insecure then@@ -464,16 +415,18 @@ function set_cookie(cookie, value) 	end 	if value == "" then 		attributes = attributes .. "; Max-Age=0"-	elseif session_seconds > 0 then+	else 		attributes = attributes .. "; Max-Age=" .. tostring(session_seconds) 	end-	html("Set-Cookie: " .. cookie .. "=" .. strip_controls(value) .. attributes .. "\n")+	html("Set-Cookie: " .. cookie .. "=" .. value .. attributes .. "\n") end +-- A control character in the location would let it split the response, so+-- it is dropped rather than escaped. function redirect_to(url) 	html("Status: 302 Found\n") 	html("Cache-Control: no-cache, no-store\n")-	html("Location: " .. strip_controls(url) .. "\n")+	html("Location: " .. url:gsub("%c", "") .. "\n") end  -- A failed login lands back on the page it came from with this flag, which@@ -501,7 +454,7 @@ end function authenticate_post() 	local redirect = validate_value("redirect", post["redirect"]) -	if redirect == nil or not is_safe_redirect(redirect) then+	if not is_safe_redirect(redirect) then 		not_found() 		return 0 	end@@ -581,33 +534,17 @@ function body() 	return 0 end --- filter_open unpacks the action and request fields cgit passes in fixed--- order, filter_write collects a post body, and filter_close runs the action.- local actions = {} actions["authenticate-post"] = authenticate_post actions["authenticate-cookie"] = authenticate_cookie actions["body"] = body -function filter_open(...)-	action = actions[select(1, ...)]-+function filter_open(name, cookie, method, query, referer, path, host, https,+		repo, page, url, login)+	action = actions[name] 	post = {}--	http = {}-	http["cookie"] = select(2, ...)-	http["method"] = select(3, ...)-	http["query"] = select(4, ...)-	http["referer"] = select(5, ...)-	http["path"] = select(6, ...)-	http["host"] = select(7, ...)-	http["https"] = select(8, ...)--	cgit = {}-	cgit["repo"] = select(9, ...)-	cgit["page"] = select(10, ...)-	cgit["url"] = select(11, ...)-	cgit["login"] = select(12, ...)+	http = { cookie = cookie }+	cgit = { repo = repo, url = url, login = login } end  function filter_close()
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua
index a03e1ea..ab56ee1 100644
--- a/custom/extensions/auth-file.lua
+++ b/custom/extensions/auth-file.lua
@@ -53,7 +53,6 @@
 -- still lists a protected repository with its description unless cgitrc sets
 -- repo.hide=1 for it.
 
--- The values that follow are the configuration and are meant to be edited.
 
 -- Accounts live one per line as username:hash, where the hash is any kind this
 -- system's crypt(3) checks. yescrypt ($y$) is what libxcrypt recommends, as on
@@ -79,8 +78,8 @@ local groups_filename = "/etc/cgit-auth/groups"
 -- 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 regardless of case. While this file cannot be read every repository
--- is refused, since nothing then tells the protected ones apart.
+-- match regardless of case. While this file cannot be read every page, the
+-- index too, is refused, since nothing then tells the protected ones apart.
 local repos_filename = "/etc/cgit-auth/repos"
 
 -- Where the cookie-signing secret is stored, created on first use. It must be
@@ -105,6 +104,7 @@ local cookie_insecure = false
 -- the password's length, and one longer than this is turned down unchecked.
 local max_password_bytes = 256
 
+
 -- A browser accepts a cookie named __Host- only when it is Secure, covers the
 -- whole host and was set by this host, so no sibling subdomain can plant a
 -- session in its place. Either cookie setting above can rule that out.
@@ -113,12 +113,18 @@ if not cookie_insecure and cookie_path == "/" then
 	session_cookie = "__Host-" .. cookie_name
 end
 
+-- A cookie carries its expiry and Max-Age as whole seconds, while on Lua 5.3
+-- and later a length worked out with a fraction, such as 1.5 days, is a float
+-- that tostring would write with a trailing ".0".
+session_seconds = math.floor(session_seconds)
+
 -- 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
 
--- The two lookups below, account_hash and repo_userset, are the only part
--- of this script that differs from auth-inline.lua.
+-- Apart from the configuration, the lookups below, account_hash and
+-- repo_userset with their helpers, are the only code that differs from
+-- auth-inline.lua, which defines trim later, beside get_cookie.
 
 -- Lua patterns backtrack, so "^%s*(.-)%s*$" costs the square of a run of
 -- spaces inside the text. Finding each end directly stays linear, which
@@ -152,9 +158,6 @@ end
 -- 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
 	local wanted = user:lower()
 	local users_file = io.open(users_filename, "r")
 	if users_file == nil then
@@ -180,9 +183,6 @@ end
 -- 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 repos_file, err = io.open(repos_filename, "r")
 	if repos_file == nil then
@@ -215,25 +215,17 @@ function repo_userset(repo)
 	return users
 end
 
--- The URL helpers below are adapted from 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(hex)
 		return string.char(tonumber(hex, 16))
 	end)
-	str = string.gsub(str, "\r\n", "\n")
 	return str
 end
 
 function url_encode(str)
-	if not str then
-		return ""
-	end
-	str = string.gsub(str, "\n", "\r\n")
 	str = string.gsub(str, "([^%w ])", function(char)
 		return string.format("%%%02X", string.byte(char))
 	end)
@@ -245,13 +237,13 @@ end
 -- '=', for example a base64 password, so the value runs to the next '&'.
 function parse_query(query)
 	local params = {}
-	for key, value in string.gmatch(query or "", "([^&=]+)=([^&]*)") do
+	for key, value in string.gmatch(query, "([^&=]+)=([^&]*)") do
 		params[url_decode(key)] = url_decode(value)
 	end
 	return params
 end
 
--- The token comes back still URL encoded by secure_value. Decoding it here
+-- The token comes back still url encoded by secure_value. Decoding it here
 -- would break the signature check for any value carrying a percent escape.
 function get_cookie(cookies, name)
 	for raw in string.gmatch(cookies or "", "[^;]+") do
@@ -350,46 +342,9 @@ end
 -- 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 = ""
-	local expiration = 0
-	local salt = ""
-	local signature = ""
-
-	if cookie == nil or cookie:len() < 3 or cookie:sub(1, 1) == "|" then
-		return nil
-	end
-
-	for component in string.gmatch(cookie, "[^|]+") do
-		if i == 0 then
-			field = component
-		elseif i == 1 then
-			value = component
-		elseif i == 2 then
-			-- 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
-			expiration = tonumber(component)
-		elseif i == 3 then
-			salt = component
-		elseif i == 4 then
-			signature = component
-		else
-			break
-		end
-		i = i + 1
-	end
-
-	if signature == nil or signature:len() == 0 then
-		return nil
-	end
-
-	if url_decode(field) ~= expected_field then
+	local field, value, expiration, salt, signature =
+		string.match(cookie or "", "^([^|]+)|([^|]+)|(%d+)|([^|]+)|([^|]+)$")
+	if signature == nil or url_decode(field) ~= expected_field then
 		return nil
 	end
 
@@ -408,12 +363,13 @@ function validate_value(expected_field, cookie, bind)
 		end
 	end
 
-	local payload = field .. "|" .. value .. "|" .. tostring(expiration) .. "|" .. salt
+	local payload = field .. "|" .. value .. "|" .. expiration .. "|" .. salt
 	if not constant_equals(signature, sign(payload, binding)) then
 		return nil
 	end
 
 	-- An expiration of 0 never expires and is used for the redirect token.
+	expiration = tonumber(expiration)
 	if expiration ~= 0 and expiration <= os.time() then
 		return nil
 	end
@@ -435,12 +391,6 @@ function secure_value(field, value, expiration, binding)
 	return payload .. "|" .. sign(payload, binding)
 end
 
--- 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
-
 -- Compare two strings in time that does not depend on how many leading bytes
 -- match, so a mismatch position is not revealed by timing.
 function constant_equals(a, b)
@@ -456,7 +406,8 @@ function constant_equals(a, b)
 end
 
 -- An empty value is how a login is cleared, since Max-Age=0 tells the browser
--- to drop the cookie it already holds.
+-- to drop the cookie it already holds. Any other value is a token secure_value
+-- built, which holds no control character to split the response with.
 function set_cookie(cookie, value)
 	local attributes = "; HttpOnly; SameSite=Lax; Path=" .. cookie_path
 	if not cookie_insecure then
@@ -464,16 +415,18 @@ function set_cookie(cookie, value)
 	end
 	if value == "" then
 		attributes = attributes .. "; Max-Age=0"
-	elseif session_seconds > 0 then
+	else
 		attributes = attributes .. "; Max-Age=" .. tostring(session_seconds)
 	end
-	html("Set-Cookie: " .. cookie .. "=" .. strip_controls(value) .. attributes .. "\n")
+	html("Set-Cookie: " .. cookie .. "=" .. value .. attributes .. "\n")
 end
 
+-- A control character in the location would let it split the response, so
+-- it is dropped rather than escaped.
 function redirect_to(url)
 	html("Status: 302 Found\n")
 	html("Cache-Control: no-cache, no-store\n")
-	html("Location: " .. strip_controls(url) .. "\n")
+	html("Location: " .. url:gsub("%c", "") .. "\n")
 end
 
 -- A failed login lands back on the page it came from with this flag, which
@@ -501,7 +454,7 @@ end
 function authenticate_post()
 	local redirect = validate_value("redirect", post["redirect"])
 
-	if redirect == nil or not is_safe_redirect(redirect) then
+	if not is_safe_redirect(redirect) then
 		not_found()
 		return 0
 	end
@@ -581,33 +534,17 @@ function body()
 	return 0
 end
 
--- filter_open unpacks the action and request fields cgit passes in fixed
--- order, filter_write collects a post body, and filter_close runs the action.
-
 local actions = {}
 actions["authenticate-post"] = authenticate_post
 actions["authenticate-cookie"] = authenticate_cookie
 actions["body"] = body
 
-function filter_open(...)
-	action = actions[select(1, ...)]
-
+function filter_open(name, cookie, method, query, referer, path, host, https,
+		repo, page, url, login)
+	action = actions[name]
 	post = {}
-
-	http = {}
-	http["cookie"] = select(2, ...)
-	http["method"] = select(3, ...)
-	http["query"] = select(4, ...)
-	http["referer"] = select(5, ...)
-	http["path"] = select(6, ...)
-	http["host"] = select(7, ...)
-	http["https"] = select(8, ...)
-
-	cgit = {}
-	cgit["repo"] = select(9, ...)
-	cgit["page"] = select(10, ...)
-	cgit["url"] = select(11, ...)
-	cgit["login"] = select(12, ...)
+	http = { cookie = cookie }
+	cgit = { repo = repo, url = url, login = login }
 end
 
 function filter_close()