Name the auth cookie __Host- where it can be

Without the prefix any host under the same domain could set a cgitauth
cookie of its own, signing a visitor in as someone else or out.

3 files changed, 71 insertions, 30 deletions

diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.luaindex 2ef2328..f5d250c 100644--- a/custom/extensions/auth-file.lua+++ b/custom/extensions/auth-file.lua@@ -101,6 +101,14 @@ 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.+local session_cookie = cookie_name+if not cookie_insecure and cookie_path == "/" then+	session_cookie = "__Host-" .. cookie_name+end+ -- 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@@ -502,7 +510,7 @@ function authenticate_post() 	end  	-- A failed login answers with an empty session, which clears the cookie.-	set_cookie(cookie_name, session)+	set_cookie(session_cookie, session)  	html("\n") 	return 0@@ -517,7 +525,7 @@ function authenticate_cookie() 		return 1 	end -	local cookie = get_cookie(http["cookie"], cookie_name)+	local cookie = get_cookie(http["cookie"], session_cookie) 	local username = validate_value("username", cookie, account_hash) 	if username == nil or not accepted_users[username:lower()] then 		return 0
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua
index 2ef2328..f5d250c 100644
--- a/custom/extensions/auth-file.lua
+++ b/custom/extensions/auth-file.lua
@@ -101,6 +101,14 @@ 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.
+local session_cookie = cookie_name
+if not cookie_insecure and cookie_path == "/" then
+	session_cookie = "__Host-" .. cookie_name
+end
+
 -- 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
@@ -502,7 +510,7 @@ function authenticate_post()
 	end
 
 	-- A failed login answers with an empty session, which clears the cookie.
-	set_cookie(cookie_name, session)
+	set_cookie(session_cookie, session)
 
 	html("\n")
 	return 0
@@ -517,7 +525,7 @@ function authenticate_cookie()
 		return 1
 	end
 
-	local cookie = get_cookie(http["cookie"], cookie_name)
+	local cookie = get_cookie(http["cookie"], session_cookie)
 	local username = validate_value("username", cookie, account_hash)
 	if username == nil or not accepted_users[username:lower()] then
 		return 0
diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.luaindex bce2472..ae21c7a 100644--- a/custom/extensions/auth-inline.lua+++ b/custom/extensions/auth-inline.lua@@ -100,6 +100,14 @@ 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.+local session_cookie = cookie_name+if not cookie_insecure and cookie_path == "/" then+	session_cookie = "__Host-" .. cookie_name+end+ -- 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@@ -463,7 +471,7 @@ function authenticate_post() 	end  	-- A failed login answers with an empty session, which clears the cookie.-	set_cookie(cookie_name, session)+	set_cookie(session_cookie, session)  	html("\n") 	return 0@@ -478,7 +486,7 @@ function authenticate_cookie() 		return 1 	end -	local cookie = get_cookie(http["cookie"], cookie_name)+	local cookie = get_cookie(http["cookie"], session_cookie) 	local username = validate_value("username", cookie, account_hash) 	if username == nil or not accepted_users[username:lower()] then 		return 0
diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua
index bce2472..ae21c7a 100644
--- a/custom/extensions/auth-inline.lua
+++ b/custom/extensions/auth-inline.lua
@@ -100,6 +100,14 @@ 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.
+local session_cookie = cookie_name
+if not cookie_insecure and cookie_path == "/" then
+	session_cookie = "__Host-" .. cookie_name
+end
+
 -- 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
@@ -463,7 +471,7 @@ function authenticate_post()
 	end
 
 	-- A failed login answers with an empty session, which clears the cookie.
-	set_cookie(cookie_name, session)
+	set_cookie(session_cookie, session)
 
 	html("\n")
 	return 0
@@ -478,7 +486,7 @@ function authenticate_cookie()
 		return 1
 	end
 
-	local cookie = get_cookie(http["cookie"], cookie_name)
+	local cookie = get_cookie(http["cookie"], session_cookie)
 	local username = validate_value("username", cookie, account_hash)
 	if username == nil or not accepted_users[username:lower()] then
 		return 0
diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.luaindex 92e82eb..0115716 100644--- a/tests/extensions/test-auth.lua+++ b/tests/extensions/test-auth.lua@@ -44,23 +44,38 @@ if variant == "file" then 	h.redirect_file("/etc/cgit-auth/repos", "auth-repos") end -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" ..-			"\tcarol = \"" .. stored_hash .. "\",\n" },-	})-else-	h.load(script)+-- The inline script carries its lists in the code, so its commented-out+-- examples are switched on with the same data the fixture files hold.+local inline_config = {+	{ "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" ..+		"\tcarol = \"" .. stored_hash .. "\",\n" },+}++local function load_script(changes)+	local all = {}+	if variant == "inline" then+		for _, change in ipairs(inline_config) do+			all[#all + 1] = change+		end+	end+	for _, change in ipairs(changes or {}) do+		all[#all + 1] = change+	end+	h.load_with(script, all)+	function get_secret()+		return string.rep("0123456789abcdef", 4)+	end end -function get_secret()-	return string.rep("0123456789abcdef", 4)-end+load_script()++-- Served over https at the root, the session cookie takes the __Host- name.+local session_name = "__Host-cgitauth"  -- crypt is watched through its stand-in module, so a check can see whether -- a login spent it and on which hash.@@ -231,32 +246,32 @@ out, ret = run_action("authenticate-cookie", { repo = "secret-repo" }) h.equals("a protected repository turns a bare request away", ret, 0)  local session = secure_value("username", "Alice", now + 3600, stored_hash)-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. session })+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. session }) h.equals("a signed session for a member is let through", ret, 1)  -- A session is signed with its account's hash, so a new password or a -- removed account ends it. local stale = secure_value("username", "Alice", now + 3600, "$6$rounds=300000$old$hash")-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. stale })+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. stale }) h.equals("a session from before a password change is turned away", ret, 0)  local orphan = secure_value("username", "bob", now + 3600, stored_hash)-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. orphan })+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. orphan }) h.equals("a session for a member with no account is turned away", ret, 0)  local unbound = secure_value("username", "Alice", now + 3600) h.equals("a session signed without its binding does not verify", 	validate_value("username", unbound, account_hash), nil) -out, ret = run_action("authenticate-cookie", { repo = "empty-repo", cookie = "cgitauth=" .. session })+out, ret = run_action("authenticate-cookie", { repo = "empty-repo", cookie = session_name .. "=" .. session }) h.equals("a memberless repository denies even a valid session", ret, 0)  local outsider = secure_value("username", "carol", now + 3600, stored_hash)-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. outsider })+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. outsider }) h.equals("a signed session for an outsider is turned away", ret, 0)  local forged = session:sub(1, -2) .. (session:sub(-1) == "0" and "1" or "0")-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. forged })+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. forged }) h.equals("a forged session is turned away", ret, 0)  out, ret = run_action("no-such-action", {})@@ -280,7 +295,7 @@ out, ret = run_action("authenticate-post", { }) h.contains("a good login redirects back", out, "Status: 302") h.contains("to where the form said", out, "Location: /repo/\n")-local granted = out:match("Set%-Cookie: cgitauth=([^;]*);")+local granted = out:match("Set%-Cookie: __Host%-cgitauth=([^;]*);") h.equals("and grants a session that verifies", granted and validate_value("username", granted, account_hash), "Alice")  out, ret = run_action("authenticate-post", {@@ -288,13 +303,13 @@ out, ret = run_action("authenticate-post", { 	body = "username=Alice&password=wrong&redirect=" .. url_encode(way_back), }) h.contains("a bad password redirects the same way", out, "Status: 302")-h.contains("but clears the session cookie", out, "Set-Cookie: cgitauth=; ")+h.contains("but clears the session cookie", out, "Set-Cookie: " .. session_name .. "=; ")  out, ret = run_action("authenticate-post", { 	method = "POST", 	body = "username=nobody&password=x&redirect=" .. url_encode(way_back), })-h.contains("an unknown user is told nothing different", out, "Set-Cookie: cgitauth=; ")+h.contains("an unknown user is told nothing different", out, "Set-Cookie: " .. session_name .. "=; ")  -- 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.@@ -323,7 +338,7 @@ 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.contains("an overlong password is turned down", out, "Set-Cookie: " .. session_name .. "=; ") h.equals("without spending crypt on it", #crypt_settings, 0) run_action("authenticate-post", { 	method = "POST",@@ -344,6 +359,16 @@ out, ret = run_action("authenticate-post", { }) h.contains("even a signed unsafe destination is not followed", out, "Status: 404") +-- Over plain http the cookie cannot be Secure, and so loses the __Host- name.+load_script({ { "local cookie_insecure = false", "local cookie_insecure = true" } })+out, ret = run_action("authenticate-post", {+	method = "POST",+	body = "username=Alice&password=" .. url_encode(password) .. "&redirect=" .. url_encode(way_back),+})+h.contains("an insecure session cookie keeps its plain name", out, "Set-Cookie: cgitauth=")+h.excludes("and is not marked Secure", out, "; Secure")+load_script()+ -- The libraries load only once a request needs one, so with every one of -- them gone a public repository is still served, while a protected one -- fails closed instead of reaching its content.@@ -354,7 +379,7 @@ end out, ret = run_action("authenticate-cookie", { repo = "unlisted" }) h.equals("without the libraries a public repository is served", ret, 1) h.check("while a protected one fails closed", not pcall(run_action,-	"authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. session }))+	"authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. session })) h.check("and so does its login form", not pcall(run_action, "body", {}))  h.finish()
diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua
index 92e82eb..0115716 100644
--- a/tests/extensions/test-auth.lua
+++ b/tests/extensions/test-auth.lua
@@ -44,23 +44,38 @@ if variant == "file" then
 	h.redirect_file("/etc/cgit-auth/repos", "auth-repos")
 end
 
-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" ..
-			"\tcarol = \"" .. stored_hash .. "\",\n" },
-	})
-else
-	h.load(script)
+-- The inline script carries its lists in the code, so its commented-out
+-- examples are switched on with the same data the fixture files hold.
+local inline_config = {
+	{ "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" ..
+		"\tcarol = \"" .. stored_hash .. "\",\n" },
+}
+
+local function load_script(changes)
+	local all = {}
+	if variant == "inline" then
+		for _, change in ipairs(inline_config) do
+			all[#all + 1] = change
+		end
+	end
+	for _, change in ipairs(changes or {}) do
+		all[#all + 1] = change
+	end
+	h.load_with(script, all)
+	function get_secret()
+		return string.rep("0123456789abcdef", 4)
+	end
 end
 
-function get_secret()
-	return string.rep("0123456789abcdef", 4)
-end
+load_script()
+
+-- Served over https at the root, the session cookie takes the __Host- name.
+local session_name = "__Host-cgitauth"
 
 -- crypt is watched through its stand-in module, so a check can see whether
 -- a login spent it and on which hash.
@@ -231,32 +246,32 @@ out, ret = run_action("authenticate-cookie", { repo = "secret-repo" })
 h.equals("a protected repository turns a bare request away", ret, 0)
 
 local session = secure_value("username", "Alice", now + 3600, stored_hash)
-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. session })
+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. session })
 h.equals("a signed session for a member is let through", ret, 1)
 
 -- A session is signed with its account's hash, so a new password or a
 -- removed account ends it.
 local stale = secure_value("username", "Alice", now + 3600, "$6$rounds=300000$old$hash")
-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. stale })
+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. stale })
 h.equals("a session from before a password change is turned away", ret, 0)
 
 local orphan = secure_value("username", "bob", now + 3600, stored_hash)
-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. orphan })
+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. orphan })
 h.equals("a session for a member with no account is turned away", ret, 0)
 
 local unbound = secure_value("username", "Alice", now + 3600)
 h.equals("a session signed without its binding does not verify",
 	validate_value("username", unbound, account_hash), nil)
 
-out, ret = run_action("authenticate-cookie", { repo = "empty-repo", cookie = "cgitauth=" .. session })
+out, ret = run_action("authenticate-cookie", { repo = "empty-repo", cookie = session_name .. "=" .. session })
 h.equals("a memberless repository denies even a valid session", ret, 0)
 
 local outsider = secure_value("username", "carol", now + 3600, stored_hash)
-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. outsider })
+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. outsider })
 h.equals("a signed session for an outsider is turned away", ret, 0)
 
 local forged = session:sub(1, -2) .. (session:sub(-1) == "0" and "1" or "0")
-out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. forged })
+out, ret = run_action("authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. forged })
 h.equals("a forged session is turned away", ret, 0)
 
 out, ret = run_action("no-such-action", {})
@@ -280,7 +295,7 @@ out, ret = run_action("authenticate-post", {
 })
 h.contains("a good login redirects back", out, "Status: 302")
 h.contains("to where the form said", out, "Location: /repo/\n")
-local granted = out:match("Set%-Cookie: cgitauth=([^;]*);")
+local granted = out:match("Set%-Cookie: __Host%-cgitauth=([^;]*);")
 h.equals("and grants a session that verifies", granted and validate_value("username", granted, account_hash), "Alice")
 
 out, ret = run_action("authenticate-post", {
@@ -288,13 +303,13 @@ out, ret = run_action("authenticate-post", {
 	body = "username=Alice&password=wrong&redirect=" .. url_encode(way_back),
 })
 h.contains("a bad password redirects the same way", out, "Status: 302")
-h.contains("but clears the session cookie", out, "Set-Cookie: cgitauth=; ")
+h.contains("but clears the session cookie", out, "Set-Cookie: " .. session_name .. "=; ")
 
 out, ret = run_action("authenticate-post", {
 	method = "POST",
 	body = "username=nobody&password=x&redirect=" .. url_encode(way_back),
 })
-h.contains("an unknown user is told nothing different", out, "Set-Cookie: cgitauth=; ")
+h.contains("an unknown user is told nothing different", out, "Set-Cookie: " .. session_name .. "=; ")
 
 -- 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.
@@ -323,7 +338,7 @@ 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.contains("an overlong password is turned down", out, "Set-Cookie: " .. session_name .. "=; ")
 h.equals("without spending crypt on it", #crypt_settings, 0)
 run_action("authenticate-post", {
 	method = "POST",
@@ -344,6 +359,16 @@ out, ret = run_action("authenticate-post", {
 })
 h.contains("even a signed unsafe destination is not followed", out, "Status: 404")
 
+-- Over plain http the cookie cannot be Secure, and so loses the __Host- name.
+load_script({ { "local cookie_insecure = false", "local cookie_insecure = true" } })
+out, ret = run_action("authenticate-post", {
+	method = "POST",
+	body = "username=Alice&password=" .. url_encode(password) .. "&redirect=" .. url_encode(way_back),
+})
+h.contains("an insecure session cookie keeps its plain name", out, "Set-Cookie: cgitauth=")
+h.excludes("and is not marked Secure", out, "; Secure")
+load_script()
+
 -- The libraries load only once a request needs one, so with every one of
 -- them gone a public repository is still served, while a protected one
 -- fails closed instead of reaching its content.
@@ -354,7 +379,7 @@ end
 out, ret = run_action("authenticate-cookie", { repo = "unlisted" })
 h.equals("without the libraries a public repository is served", ret, 1)
 h.check("while a protected one fails closed", not pcall(run_action,
-	"authenticate-cookie", { repo = "secret-repo", cookie = "cgitauth=" .. session }))
+	"authenticate-cookie", { repo = "secret-repo", cookie = session_name .. "=" .. session }))
 h.check("and so does its login form", not pcall(run_action, "body", {}))
 
 h.finish()