Turn down passwords past 256 bytes unchecked

SHA-crypt hashes the password in most of its rounds, so the 4 KB that
cgit's POST limit lets through cost about fifteen ordinary logins per
anonymous attempt.

3 files changed, 32 insertions, 2 deletions

diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.luaindex 90f597b..f43eb82 100644--- a/custom/extensions/auth-file.lua+++ b/custom/extensions/auth-file.lua@@ -86,6 +86,10 @@ local cookie_path = "/" -- Set this true only if cgit is served over plain HTTP with no TLS anywhere. local cookie_insecure = false +-- crypt feeds the password into most of its rounds, so its work grows with+-- the password's length, and one longer than this is turned down unchecked.+local max_password_bytes = 256+ -- 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 user name exists.@@ -449,7 +453,7 @@ function authenticate_post() 	local username = post["username"] 	local password = post["password"] 	local ok = false-	if username ~= nil and password ~= nil then+	if username ~= nil and password ~= nil and #password <= max_password_bytes then 		local hash = account_hash(username) 		if hash == nil then 			-- Spend the work anyway, see dummy_hash.
diff --git a/custom/extensions/auth-file.lua b/custom/extensions/auth-file.lua
index 90f597b..f43eb82 100644
--- a/custom/extensions/auth-file.lua
+++ b/custom/extensions/auth-file.lua
@@ -86,6 +86,10 @@ local cookie_path = "/"
 -- Set this true only if cgit is served over plain HTTP with no TLS anywhere.
 local cookie_insecure = false
 
+-- crypt feeds the password into most of its rounds, so its work grows with
+-- the password's length, and one longer than this is turned down unchecked.
+local max_password_bytes = 256
+
 -- 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 user name exists.
@@ -449,7 +453,7 @@ function authenticate_post()
 	local username = post["username"]
 	local password = post["password"]
 	local ok = false
-	if username ~= nil and password ~= nil then
+	if username ~= nil and password ~= nil and #password <= max_password_bytes then
 		local hash = account_hash(username)
 		if hash == nil then
 			-- Spend the work anyway, see dummy_hash.
diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.luaindex de91f88..53b1efc 100644--- a/custom/extensions/auth-inline.lua+++ b/custom/extensions/auth-inline.lua@@ -86,6 +86,10 @@ local cookie_path = "/" -- Set this true only if cgit is served over plain HTTP with no TLS anywhere. local cookie_insecure = false +-- crypt feeds the password into most of its rounds, so its work grows with+-- the password's length, and one longer than this is turned down unchecked.+local max_password_bytes = 256+ -- 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 user name exists.@@ -411,7 +415,7 @@ function authenticate_post() 	local username = post["username"] 	local password = post["password"] 	local ok = false-	if username ~= nil and password ~= nil then+	if username ~= nil and password ~= nil and #password <= max_password_bytes then 		local hash = account_hash(username) 		if hash == nil then 			-- Spend the work anyway, see dummy_hash.
diff --git a/custom/extensions/auth-inline.lua b/custom/extensions/auth-inline.lua
index de91f88..53b1efc 100644
--- a/custom/extensions/auth-inline.lua
+++ b/custom/extensions/auth-inline.lua
@@ -86,6 +86,10 @@ local cookie_path = "/"
 -- Set this true only if cgit is served over plain HTTP with no TLS anywhere.
 local cookie_insecure = false
 
+-- crypt feeds the password into most of its rounds, so its work grows with
+-- the password's length, and one longer than this is turned down unchecked.
+local max_password_bytes = 256
+
 -- 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 user name exists.
@@ -411,7 +415,7 @@ function authenticate_post()
 	local username = post["username"]
 	local password = post["password"]
 	local ok = false
-	if username ~= nil and password ~= nil then
+	if username ~= nil and password ~= nil and #password <= max_password_bytes then
 		local hash = account_hash(username)
 		if hash == nil then
 			-- Spend the work anyway, see dummy_hash.
diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.luaindex 5d3541c..97638bd 100644--- a/tests/extensions/test-auth.lua+++ b/tests/extensions/test-auth.lua@@ -278,6 +278,28 @@ out, ret = run_action("authenticate-post", { }) h.contains("an unknown user is told nothing different", out, "Set-Cookie: cgitauth=; ") +-- crypt's work grows with the password, so one past the cap is turned down+-- before crypt sees it.+local unistd = require("posix.unistd")+local real_crypt = unistd.crypt+local crypt_calls = 0+unistd.crypt = function(...)+	crypt_calls = crypt_calls + 1+	return real_crypt(...)+end+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.equals("without spending crypt on it", crypt_calls, 0)+out, ret = run_action("authenticate-post", {+	method = "POST",+	body = "username=Alice&password=" .. string.rep("a", 256) .. "&redirect=" .. url_encode(way_back),+})+h.equals("a password at the cap is still checked", crypt_calls, 1)+unistd.crypt = real_crypt+ out, ret = run_action("authenticate-post", { 	method = "POST", 	body = "username=Alice&password=" .. url_encode(password),
diff --git a/tests/extensions/test-auth.lua b/tests/extensions/test-auth.lua
index 5d3541c..97638bd 100644
--- a/tests/extensions/test-auth.lua
+++ b/tests/extensions/test-auth.lua
@@ -278,6 +278,28 @@ out, ret = run_action("authenticate-post", {
 })
 h.contains("an unknown user is told nothing different", out, "Set-Cookie: cgitauth=; ")
 
+-- crypt's work grows with the password, so one past the cap is turned down
+-- before crypt sees it.
+local unistd = require("posix.unistd")
+local real_crypt = unistd.crypt
+local crypt_calls = 0
+unistd.crypt = function(...)
+	crypt_calls = crypt_calls + 1
+	return real_crypt(...)
+end
+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.equals("without spending crypt on it", crypt_calls, 0)
+out, ret = run_action("authenticate-post", {
+	method = "POST",
+	body = "username=Alice&password=" .. string.rep("a", 256) .. "&redirect=" .. url_encode(way_back),
+})
+h.equals("a password at the cap is still checked", crypt_calls, 1)
+unistd.crypt = real_crypt
+
 out, ret = run_action("authenticate-post", {
 	method = "POST",
 	body = "username=Alice&password=" .. url_encode(password),