auth: classify $p$ lookups by account state#25
Open
hauke wants to merge 1 commit into
Open
Conversation
uh_auth_add resolves '$p$<user>' password lines by reading the named
account's stored credential from /etc/shadow (or /etc/passwd as a
fallback). The previous code copied that string verbatim into
realm->pass without checking whether it was a usable crypt(3) hash.
This broke for two distinct shadow states:
1. Locked or placeholder credentials ("*", "x", "!", "!!", "*LK*",
"*NP*", "!hash" lock prefix). In uh_auth_check the plaintext
compare branch fires whenever realm->pass does not start with '$',
so any client sending the placeholder verbatim as the password
matched and authenticated. On OpenWrt every system account except
root has '*' or 'x' in /etc/shadow by default, so a realm configured as
'/cgi-bin/admin:adminuser:$p$daemon' (the pattern the example UCI
in package/network/services/uhttpd/files/uhttpd.config documents)
authenticated any request with password '*' and ran the CGI as
root.
2. Empty password (the default OpenWrt root entry, "root:::..."):
handled correctly by the existing empty-pass drop but silently,
leaving admins unaware that '$p$root' on a fresh image produces a
public URL.
Match login(1) semantics: locked or placeholder accounts deny all
access, accounts with no password set permit access without
authentication, accounts with a real hash require it. For each
non-hash case log a distinct warning so admins notice the silent
state. A '$p$' reference to a non-existent account is treated as a
config typo: deny all access (consistent with the admin's stated
intent of requiring auth) and log a warning.
For the locked case, bind the realm to a sentinel that starts with
'$' (skipping the plaintext compare branch) and cannot be produced by
crypt(3) (failing the hash branch), so every request returns 401
instead of falling through to a missing-realm public response.
Reported-by: Amit Pinchasi <amitpinchasi123@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Link: openwrt#25
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
uh_auth_add resolves '$p$' password lines by reading the named account's stored credential from /etc/shadow (or /etc/passwd as a fallback). The previous code copied that string verbatim into realm->pass without checking whether it was a usable crypt(3) hash. This broke for two distinct shadow states:
Locked or placeholder credentials ("", "x", "!", "!!", "LK", "NP", "!hash" lock prefix). In uh_auth_check the plaintext compare branch fires whenever realm->pass does not start with '$', so any client sending the placeholder verbatim as the password matched and authenticated. On OpenWrt every system account except root has '' or 'x' in /etc/shadow by default, so a realm configured as '/cgi-bin/admin:adminuser:$p$daemon' (the pattern the example UCI in package/network/services/uhttpd/files/uhttpd.config documents) authenticated any request with password '*' and ran the CGI as root.
Empty password (the default OpenWrt root entry, "root:::..."): handled correctly by the existing empty-pass drop but silently, leaving admins unaware that '$p$root' on a fresh image produces a public URL.
Match login(1) semantics: locked or placeholder accounts deny all access, accounts with no password set permit access without authentication, accounts with a real hash require it. For each non-hash case log a distinct warning so admins notice the silent state. A '$p$' reference to a non-existent account is treated as a config typo: deny all access (consistent with the admin's stated intent of requiring auth) and log a warning.
For the locked case, bind the realm to a sentinel that starts with '$' (skipping the plaintext compare branch) and cannot be produced by crypt(3) (failing the hash branch), so every request returns 401 instead of falling through to a missing-realm public response.
Reported-by: Amit Pinchasi amitpinchasi123@gmail.com