[
https://issues.apache.org/jira/browse/OAK-12259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18089103#comment-18089103
]
Julian Reschke commented on OAK-12259:
--------------------------------------
While at it, we should also handle the whitespace after "Basic" correctly
(https://www.greenbytes.de/tech/specs/rfc7235.html#challenge.and.response),
case sensitivity, decoding non-ASCII, invalid username/passwords.
For obvious reasons, I'll take this.
Thanks for the report.
> oak-http: OakServlet mis-parses HTTP Basic credentials - passwords containing
> a colon are silently truncated and malformed headers cause an HTTP 500
> ----------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: OAK-12259
> URL: https://issues.apache.org/jira/browse/OAK-12259
> Project: Jackrabbit Oak
> Issue Type: Bug
> Components: oak-http
> Environment: Any deployment that exposes the repository over HTTP via
> {{org.apache.jackrabbit.oak.http.OakServlet}} using HTTP Basic authentication.
> Reporter: Herman Ciechanowiec
> Assignee: Julian Reschke
> Priority: Major
>
> h2. Description
> {{OakServlet}} authenticates incoming HTTP requests by decoding the
> {{Authorization: Basic <base64>}} header and parsing the decoded
> {{user-id:password}} string. The current implementation is:
> {code:java}
> String authorization = request.getHeader("Authorization");
> if (authorization != null && authorization.startsWith("Basic ")) {
> String[] basic =
> Base64.decode(authorization.substring("Basic
> ".length())).split(":");
> credentials = new SimpleCredentials(basic[0], basic[1].toCharArray());
> } else {
> throw new LoginException();
> }
> {code}
> (File:
> {{{}oak-http/src/main/java/org/apache/jackrabbit/oak/http/OakServlet.java{}}},
> inside {{{}service(...){}}}.)
> This violates [RFC 7617|https://www.rfc-editor.org/rfc/rfc7617] ("The 'Basic'
> HTTP Authentication Scheme"), which states that the credentials are the
> concatenation of the user-id, a single colon ({{{}:{}}}) character, and the
> password. The user-id _must not_ contain a colon, but the password _may_
> contain any character, including colons. Therefore everything after the
> *first* colon is the password.
> Two distinct defects result from using {{String.split(":")}} with no limit:
> h3. 1. Password truncation / weakened authentication (CWE-287)
> {{split(":")}} splits on *every* colon and the code keeps only element
> {{{}[1]{}}}. A password that contains one or more colons is therefore
> silently truncated to the substring between the first and second colon. The
> repository then authenticates the user against this shortened secret.
> *Example:* a user whose password is {{p4ss:w0rd:!}} is authenticated using
> only {{p4ss}} (the substring between the first and second colon). The
> effective secret the server verifies is shorter than the one the user
> actually set, reducing the keyspace an attacker must search and breaking the
> user's expectation about password strength.
> h3. 2. Unhandled exception -> HTTP 500 (instead of 401)
> If the decoded value contains no colon (e.g. a client sends just a user-id,
> or an empty password {{user:}} - {{String.split}} drops trailing empty
> strings), accessing {{basic[1]}} throws
> {{{}ArrayIndexOutOfBoundsException{}}}. This runtime exception is *not*
> caught by the surrounding handlers (which only catch {{LoginException}} and
> {{{}NoSuchWorkspaceException{}}}), so it escapes as an HTTP 500 with a stack
> trace rather than a proper 401 Unauthorized challenge.
> h2. Steps to reproduce
> *Defect 1 (password truncation):*
> # Create a user whose password contains a colon, e.g. {{{}p4ss:w0rd:!{}}}.
> # Send a request with: {{Authorization: Basic <base64 of
> "user:p4ss:w0rd:!">}}
> # Observe that authentication is evaluated against {{{}p4ss{}}}, not the
> full password. A login that should fail with the wrong password can succeed,
> and the full password is never the one actually checked.
> *Defect 2 (HTTP 500):*
> # Send a request with an Authorization header whose decoded value has no
> colon, e.g. Base64 of {{{}nocolon{}}}: {{Authorization: Basic bm9jb2xvbg==}}
> # Observe an HTTP 500 ({{{}ArrayIndexOutOfBoundsException{}}}) instead of a
> clean 401 Unauthorized.
> h2. Expected behaviour
> * The password is taken as everything after the first colon, preserving any
> colons it contains (RFC 7617 compliant).
> * A missing/empty/non-Basic Authorization header, or a decoded value with no
> colon, results in a 401 Unauthorized challenge - never an HTTP 500.
> h2. Actual behaviour
> * Passwords containing colons are truncated at the first colon, weakening
> authentication.
> * A decoded value without a colon yields HTTP 500
> ({{{}ArrayIndexOutOfBoundsException{}}}) instead of 401.
> h2. Impact
> * *Security:* effective passwords are silently weakened whenever they
> contain a colon; the secret actually verified is shorter than the one
> configured.
> * *Robustness/availability:* malformed credential headers produce 500 errors
> and leak a stack trace instead of returning a standard 401 challenge.
> h2. Proposed fix
> Split only on the first colon ({{{}indexOf(':'){}}} + {{{}substring{}}}),
> reject a missing colon with a {{LoginException}} (mapped to 401 by the
> existing handler), and reject a missing/non-Basic header the same way. The
> credential parsing could be extracted into a small private helper for
> clarity, for example:
> {code:java}
> private Credentials parseBasicCredentials(String authorization)
> throws LoginException {
> if (authorization == null || !authorization.startsWith("Basic ")) {
> throw new LoginException(
> "Missing or unsupported Authorization header; expected Basic
> scheme");
> }
> String decoded = Base64.decode(authorization.substring("Basic
> ".length()));
> int colon = decoded.indexOf(':');
> if (colon < 0) {
> throw new LoginException(
> "Malformed Basic credentials: missing ':' separator");
> }
> String userId = decoded.substring(0, colon);
> String password = decoded.substring(colon + 1);
> return new SimpleCredentials(userId, password.toCharArray());
> }
> {code}
> This is a contained bug fix with no API change, so no feature toggle is
> required.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
