AgeCommit message (Collapse)AuthorLines
Drive the auth actions through the harnessBryce Kwon-14/+2
run_action repeated harness.run line for line, and three comments only labelled the sections below them.
Hold signatures to the length SHA-256 givesBryce Kwon-0/+3
The fake HMAC returned 16 bytes, so the checks signed with signatures half the real length, and a signature cut short passed. tohex's output was never pinned either, so dropping its zero padding stayed consistent with itself and passed.
Check a session cookie scoped below the rootBryce Kwon-0/+9
A cookie_path other than / has to keep Secure but drop the __Host- name, which a browser takes only for the whole host. Neither was tried.
Check expired and planted sessions on a visitBryce Kwon-0/+7
An expired session was only tried against validate_value, never sent through a visit, and nothing sent a session under the plain cookie name, which a sibling subdomain can set where it cannot set the __Host- one.
Check the form is there where a message is notBryce Kwon-1/+4
The checks that a forged session is not taken for a signed-in visitor, and that https brings no warning, would also have passed on the page saying logins are unavailable.
Check a login when crypt cannot check the hashBryce Kwon-1/+12
crypt(3) answers nil for a kind of hash the system does not check, but the stand-in never did, so a login that raised on it, or let it through, passed.
Give the outsider account a hash of its ownBryce Kwon-5/+10
Every account shared one hash, so the binding of a session to its account's hash could not tell accounts apart, and an account_hash that handed back the wrong account's hash passed.
Check a failure flag that follows a question markBryce Kwon-0/+5
A failed login on a page with no query of its own comes back as ?login=failed, the common case, but only the &login=failed form was tried. A drop_failure_flag that handled the ampersand alone would have lost the message and piled flags onto the way back, and passed.
Check every failed login answers and logs alikeBryce Kwon-3/+27
Every post carried both fields, so the guards for a missing name or password never ran. An unknown name was checked for the cookie and the Location alone, so an extra header or body for it passed, and only the wrong password case had its log line checked for the name and against the password.
Check each auth library missing on its ownBryce Kwon-5/+44
Only all four libraries missing at once was tried, and the script was not reloaded after they went, so a require at load would not have failed there. The protected visit passed on any error, a typo's nil call included. A login never ran without a library, so a fallback that skipped the mask, or one that let any password through when crypt was missing, passed.
Make the cookie timing check hold on fast coresBryce Kwon-5/+9
The quadratic trim takes 0.58 seconds for the 20,000 spaces the check used under LuaJIT on an M-series Mac, against a threshold of half a second over two calls, so a regression could pass on a slightly faster core. At 100,000 spaces it takes about fifteen seconds and the linear one under a millisecond, and each call is now timed on its own.
Sign the malformed cookies the checks refuseBryce Kwon-3/+3
The exponent and leading separator cases carried a made-up signature, which fails the check whatever the parsing does, so loosening the expiry pattern or dropping its start anchor passed. Both are now built from properly signed tokens.
Check how auth groups resolve to membersBryce Kwon-8/+16
auth-file.lua says group names match whatever their case, but both fixture files spelt devs alike, so dropping either lowercasing passed. No repository named two groups and no user sat in two, so reading only the first group of a list, or only the last line that matched, passed as well. A repository with no groups at all is a file variant matter, and for the inline one the check only repeated the empty one.
Accept only a local path as the way back inBryce Kwon-0/+10
is_safe_redirect looked at the first two characters alone, so it accepted https://evil.example/ and javascript:x. Neither could be signed today only because cgit always signs a url that starts with virtual-root, and nothing tested that. A target now has to start with a single slash, which needs virtual-root to be a path for logins to work, as it is by default.
Run the auth checks against the real librariesBryce Kwon-16/+26
The unit checks always took the harness stand-ins and the cgit-level checks needed a module to be missing, so the real luaossl and luaposix were never exercised anywhere. A misuse of their interfaces, or a login through cgit that never worked, passed every run. Where an interpreter has both libraries and its crypt(3) checks a SHA-512 hash from openssl passwd, the unit checks run again against them, adding the secret's mode. Where cgit's own Lua does, a login is posted through cgit and the cookie it grants opens the repository. Both skip where crypt checks no SHA-512 hash or the libraries are missing.
Check the login form's field namesBryce Kwon-0/+4
login() in the checks posts username and password under names of its own, so renaming either field in the form broke every real login while the suite passed.
Check the auth files that are missing or brokenBryce Kwon-0/+21
Only a missing users file and a missing repos file were tried. A missing groups file denies today, but a change returning nil there would have made every protected repository public unnoticed. The repos file's log line and a users file with no line that parses, which has to turn a login down without spending crypt, went unchecked too.
Check the comparisons that must not leak timingBryce Kwon-0/+25
constant_equals was only checked for its answers, on strings that differ in one byte. Summing the plain differences instead of their squares passed, which accepts any signature with the same byte sum, and so did a plain == at the signature or the password check.
Check when a session expiresBryce Kwon-0/+26
Nothing read the expiry signed into a granted session. validate_value treats 0 as never expiring, so a login that signed its session with 0 passed, since the Max-Age the browser is told comes from set_cookie alone. The expiry second itself is now held to be too late, with the clock held still, as a comparison of < instead of <= passed too.
Check that a signature covers every cookie fieldBryce Kwon-0/+20
The only tampering tried was a flipped signature, a field swapped for another and the binding, so a payload that left out the expiry, the salt or the value still passed, and a captured session could have been extended for ever.
Check that the secret keys every signatureBryce Kwon-0/+12
Every check signed and verified under one pinned secret, so a sign() that left the secret out of the HMAC key, which makes every cookie forgeable, passed.
Check that the auth secret is made safelyBryce Kwon-8/+82
The checks replaced get_secret with a constant, so its success path never ran, and the luaposix stand-in had no link or unlink and a umask that ignored its argument. Dropping the tightened mask, which leaves the secret readable by everyone under the usual 022, or accepting a short secret, passed. The real get_secret now makes the secret in the run's directory through stand-ins that behave like the calls they replace, and the checks cover the mask and its restoration, the link race, the temporary file, reuse and a secret written by hand.
Give each auth unit run a directory of its ownBryce Kwon-16/+18
test-auth.lua wrote its fixtures into whatever directory it was started from, so a run by hand from the checkout left auth-users and its siblings behind, and the checks assumed no auth-users-missing or no-such-directory already sat there. It now refuses to run without a directory, and t0505 hands each interpreter and variant a fresh one. The interpreter and paths are expanded inside the test body rather than pasted into it in single quotes, so a path holding a quote no longer breaks the eval.
Read the login form table as layoutBryce Kwon-0/+1
Log failed loginsBryce Kwon-0/+7
The login posts outside any repository, so the log line names the repository by the page its signed way back leads to.
Say that logging in needs httpsBryce Kwon-1/+12
Only HTTPS=on counts, the same test cgit makes before it writes an https link of its own.
Tell a signed-in visitor they have no accessBryce Kwon-0/+7
Say when logins are unavailableBryce Kwon-1/+16
The way back was signed part way through the form, so while the secret's directory did not exist, or a library was missing, cgit's error landed inside the hidden input and the page broke off there with no fields.
Write a session's expiry in whole secondsBryce Kwon-0/+11
On Lua 5.3 and later a length worked out with a fraction, such as 1.5 * 24 * 60 * 60, is a float, so the signed expiry came out as 1793609024.0, which validate_value refuses, and every login failed. Max-Age took the same .0, which makes a browser ignore it.
Share the request helpers in the auth checksBryce Kwon-77/+49
The two identical posts for an unknown user become one, and no two checks share a name, so a failure report points at a single check.
Drop auth guards for input cgit never sendsBryce Kwon-3/+4
cgit passes an empty string, never nil, when a request names no repository, so the check that the index stays up proved nothing, and auth-file in fact refuses the index too while its repos file cannot be read. A session_seconds of 0 signs a session that has expired by the time it comes back, so it needs no branch of its own in set_cookie.
Strip control characters from the location aloneBryce Kwon-2/+0
set_cookie only ever writes an empty value or a token secure_value built from url-encoded text, digits and hex, so stripping it changed nothing.
Take a signed cookie apart with one matchBryce Kwon-0/+1
The loop let a valid cookie through with a field appended, a doubled separator or a zero in front of its expiry. The payload now carries the expiry as written, so only the spelling secure_value issues verifies.
Say when a login fails, and line up the formBryce Kwon-1/+11
A wrong password sent the reader back to the same empty form, which looked no different from a crash. The redirect now carries a flag the form reads, the same for an unknown user as for a wrong password, and the form leaves the flag out of the way back it signs.
Name the auth cookie __Host- where it can beBryce Kwon-26/+51
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.
End a session when its account's hash changesBryce Kwon-4/+20
A session stayed good for its whole week after the password changed or the account went away, which is when ending it matters most.
Load auth libraries only when a request needs oneBryce Kwon-0/+13
Required at the top, a missing luaossl or luaposix failed every request to the site, public pages included, and each public page paid to load both.
Spend a missing account's login on a real hashBryce Kwon-36/+51
The fixed SHA-512 stand-in matched only the documented setup, so a site hashing with yescrypt, other rounds or bcrypt answered for missing and real names in different times, and on OpenBSD it failed at once.
Turn down passwords past 256 bytes uncheckedBryce Kwon-0/+22
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.
Read the auth cookie in linear timeBryce Kwon-0/+10
Splitting the header with patterns that backtrack over spaces cost a quarter of a second per request at the 8 KB line most web servers allow, and a second at 16 KB, without any login. auth-file's trim had the same shape and now reads the cookie as well.
Refuse every repository when auth-file loses reposBryce Kwon-0/+13
A repos file the filter could not open read as listing nothing, so a permission slip or a mistyped path made every protected repository public. A line naming a repository with no groups was skipped the same way, where auth-inline's empty list denies everyone.
Clean up the whole treeBryce Kwon-62/+31
Add a test suite for the shipped extensionsBryce Kwon-0/+302